Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

usb audio changes #67261

Merged
merged 4 commits into from
Jan 10, 2024
Merged

usb audio changes #67261

merged 4 commits into from
Jan 10, 2024

Conversation

jzipperer-fb
Copy link
Contributor

Added device tree bindings for some addition audio usb descriptor values
Added start of frame notifications to nxp mcux usb

@zephyrbot zephyrbot added area: USB Universal Serial Bus area: Devicetree Binding PR modifies or adds a Device Tree binding platform: NXP NXP labels Jan 5, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 5, 2024

Hello @jzipperer-fb, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

@jzipperer-fb jzipperer-fb force-pushed the jzipperer-fb/usb-audio branch from 0680f47 to edbe1ac Compare January 5, 2024 22:15
@jzipperer-fb
modules: hal_nxp: usb: enable sof notifications
504bfe3 
When building with CONFIG_USB_DEVICE_AUDIO, enable start of frame
notifications from the mcux sdk usb middleware

Signed-off-by: James Zipperer <[email protected]>
@jzipperer-fb
drivers: usb: device: add start of frame notifications to nxp mcux dr…
54b6850 
…iver

When usb middleware sends a start of frame notification to this driver,
call status_cb with USB_DC_SOF.

Signed-off-by: James Zipperer <[email protected]>
@jzipperer-fb
usb: device: audio options for polling-interval and sample-rate-hz
74b07f1 
These descriptor values can now be configured via the device tree

Signed-off-by: James Zipperer <[email protected]>
@jzipperer-fb
usb: device: audio: fix compiler warning for usb audio microphone
34c6148 
audio_receive_cb is only used by headphones and headset, it is unused in
microphone-only configurations.  Gate compilation based on the device
tree.

Signed-off-by: James Zipperer <[email protected]>
@jzipperer-fb jzipperer-fb force-pushed the jzipperer-fb/usb-audio branch from edbe1ac to 34c6148 Compare January 9, 2024 15:08
@carlescufi carlescufi merged commit 273165c into zephyrproject-rtos:main Jan 10, 2024
16 checks passed
@github-actions GitHub Actions
Copy link

Hi @jzipperer-fb!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

mmahadevan108
mmahadevan108 reviewed Jan 10, 2024
View reviewed changes
drivers/usb/device/usb_dc_mcux.c
@@ -910,6 +910,9 @@ static void usb_mcux_thread_main(void *arg1, void *arg2, void *arg3)
case kUSB_DeviceNotifyResume:
dev_state.status_cb(USB_DC_RESUME, NULL);
break;
case kUSB_DeviceNotifySOF:
Copy link
Collaborator

@mmahadevan108 mmahadevan108 Jan 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jzipperer-fb Where is kUSB_DeviceNotifySOF defined. This is causing CI failures.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh no! It's in the nxp middleware usb sdk. device/usb_device_dci.h.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nxp-mcuxpresso/mcuxsdk-middleware-usb#5

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jzipperer-fb, the NXP SDK team has to review and integrate these changes first. We can then add these changes to Zephyr. For now, I have added a PR to revert the 2 commits to the NXP drivers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. sounds good.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how can i kick start that process?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have followed up with the SDK team via the PR you submitted. We can keep an eye on that PR.

Copy link
Contributor

@jsiverskog jsiverskog Jan 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey,
this was merged to main, causing build issues:

drivers/usb/device/usb_dc_mcux.c: In function 'usb_mcux_thread_main':
drivers/usb/device/usb_dc_mcux.c:913:22: error: 'kUSB_DeviceNotifySOF' undeclared (first use in this function); did you mean 'kUSB_DeviceNotifyError'?
  913 |                 case kUSB_DeviceNotifySOF:
      |                      ^~~~~~~~~~~~~~~~~~~~
      |                      kUSB_DeviceNotifyError

i believe these changes should be reverted, nxp-mcuxpresso/mcuxsdk-middleware-usb#5 patch be merged into https://github.com/zephyrproject-rtos/hal_nxp/, the manifest updated in this repo and finally revert again?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, saw #67459 now.

@mmahadevan108 mmahadevan108 mentioned this pull request Jan 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmahadevan108 mmahadevan108 mmahadevan108 left review comments

@jsiverskog jsiverskog jsiverskog left review comments

@tmon-nordic tmon-nordic tmon-nordic approved these changes

@jfischer-no jfischer-no jfischer-no approved these changes

@bperseghetti bperseghetti Awaiting requested review from bperseghetti

@danieldegrasse danieldegrasse Awaiting requested review from danieldegrasse

@dleach02 dleach02 Awaiting requested review from dleach02

@galak galak Awaiting requested review from galak

@manuargue manuargue Awaiting requested review from manuargue

@PetervdPerk-NXP PetervdPerk-NXP Awaiting requested review from PetervdPerk-NXP

Assignees

@jfischer-no jfischer-no

Labels
area: Devicetree Binding PR modifies or adds a Device Tree binding area: USB Universal Serial Bus platform: NXP NXP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@jzipperer-fb @jsiverskog @jfischer-no @mmahadevan108 @tmon-nordic @carlescufi @zephyrbot