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.

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

usb: add new (experimental) USB device support #38679

Merged
merged 15 commits into from
Dec 2, 2022

Conversation

jfischer-no
Copy link
Collaborator

@jfischer-no jfischer-no commented Sep 20, 2021

Add new USB device controller API (UDC API)
Add new USB device controller API and nRF USBD controller driver.
Add new USB device stack implementation using new UDC API

Resolves #19713
Resolves #29132
Resolves #29133
Resolves #29134
Resolves #29135
Resolves #29136
Resolves #30042

Commits introducing USB host controller (UHC) driver API are here for reasons and will be moved to own PR later.

@jfischer-no jfischer-no added area: Drivers area: USB Universal Serial Bus area: API Changes to public APIs labels Sep 20, 2021
@jfischer-no jfischer-no added this to the v3.0.0 milestone Sep 20, 2021
@github-actions github-actions bot added the area: Tests Issues related to a particular existing or missing test label Sep 20, 2021
@zephyrbot zephyrbot requested a review from tejlmand September 20, 2021 14:17
include/usb/usb_ch9.h Outdated Show resolved Hide resolved
drivers/usb/udc/Kconfig.nrf Outdated Show resolved Hide resolved
drivers/usb/udc/udc_nrf.c Outdated Show resolved Hide resolved
drivers/usb/udc/udc_nrf.c Outdated Show resolved Hide resolved
drivers/usb/udc/Kconfig.nrf Outdated Show resolved Hide resolved
include/drivers/usb/udc.h Outdated Show resolved Hide resolved
drivers/usb/udc/udc_common.c Outdated Show resolved Hide resolved
drivers/usb/udc/udc_common.c Outdated Show resolved Hide resolved
drivers/usb/udc/udc_common.c Outdated Show resolved Hide resolved
include/drivers/usb/udc.h Outdated Show resolved Hide resolved
drivers/usb/udc/udc_nrf.c Outdated Show resolved Hide resolved
struct cdc_acm_uart_data *data = dev->data;

atomic_set_bit(&data->state, CDC_ACM_CLASS_ENABLED);
LOG_WRN("Configuration enabled");
Copy link

Choose a reason for hiding this comment

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

This definitely should be lower level than LOG_WRN. Changing this to LOG_INF would make it match the log level for configuration disabled message.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, have forgotten it again...

subsys/usb/device_next/class/usbd_cdc_acm.c Outdated Show resolved Hide resolved
subsys/usb/device_next/class/usbd_cdc_acm.c Outdated Show resolved Hide resolved
@@ -140,6 +161,7 @@ static void udc_event_xfer_in(const struct device *dev,
buf = udc_buf_get(dev, ep, true);
if (buf == NULL) {
LOG_ERR("ep 0x%02x queue is empty", ep);
__ASSERT(false, "ep 0x%02x queue is empty", ep);
Copy link

Choose a reason for hiding this comment

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

Asserting on false is not that useful. It would be better to make it __ASSERT(buf != NULL, "ep 0x%02x queue is empty", ep);. Also I think it would be better to place assert before if (buf == NULL)


if (!check_wq_ctx(dev)) {
LOG_WRN("Invoked by inappropriate context");
__ASSERT(true, "Invoked by inappropriate context");
Copy link

Choose a reason for hiding this comment

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

Asserting on true makes no sense. Condition true is always true and therefore always is fulfilled.

The assert should be __ASSERT(check_wq_ctx(dev), "Invoked by inappropriate context"); and be placed before the if statement.

Copy link
Collaborator Author

@jfischer-no jfischer-no Dec 2, 2022

Choose a reason for hiding this comment

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

No, it should be __ASSERT(false, "Invoked by inappropriate context");, alternative is __ASSERT_NO_MSG.

Add new USB device controller API and nRF USBD controller driver.
The new UDC API brings support for multiple instances and asynchronous
transfer model, transfers use net_buf and store methadata in the
user data area.

Signed-off-by: Johann Fischer <[email protected]>
Implement method to distiguish hal driver and UDC shim driver events.

Signed-off-by: Johann Fischer <[email protected]>
HAL events were also used for the shim driver's concerns
during the prototyping of API. Fix it now and use specific
shim driver events. That also allows new transfers to be
triggered from a single point.

Signed-off-by: Johann Fischer <[email protected]>
Expand ifdef by adding new Kconfig option UDC_KINETIS as
preparation for USBFSOTG UDC driver.

Signed-off-by: Johann Fischer <[email protected]>
Add USBFSOTG UDC driver for Kinetis SoCs.

Signed-off-by: Johann Fischer <[email protected]>
Simple test for API rules, allocation, queue, and dequeu
of the endpoint requests. USB device controller should not be
connected to the host as this state is not covered by this test.

Signed-off-by: Johann Fischer <[email protected]>
The device supprt brings support for multiple stack instances,
multiple configuration, asynchronous transfer model, ability to
change most of the properties of a device at runtime and
the composition of configuration and classes at runtime.

The stack requires new UDC driver API and is not compatible
with old driver API (usb_dc_). The classes (functions) of old
(current) USB device stack cannot be used with new ones and must
be ported.

Signed-off-by: Johann Fischer <[email protected]>
Add UDC driver API reference.

Signed-off-by: Johann Fischer <[email protected]>
The sample enables new experimental USB device support
and the shell function.

Signed-off-by: Johann Fischer <[email protected]>
Add experimental CDC ACM implementation for new USB device stack.
It currently implements only UART IRQ API support and is WIP.

Signed-off-by: Johann Fischer <[email protected]>
Add code and configuration to enable new USB device support.

Signed-off-by: Johann Fischer <[email protected]>
Add code and configuration to enable new USB device support.

Signed-off-by: Johann Fischer <[email protected]>
Copy link

@desowin desowin left a comment

Choose a reason for hiding this comment

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

Time to merge this, despite some yet unresolved issues. This is experimental after all and will stay experimental for a while.

@carlescufi carlescufi changed the title usb: add new USB device support usb: add new (experimental) USB device support Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: ARM ARM (32-bit) Architecture area: Devicetree Binding PR modifies or adds a Device Tree binding area: Devicetree area: Documentation area: Drivers area: Networking area: Process area: Shields Shields (add-on boards) area: UART Universal Asynchronous Receiver-Transmitter area: USB Universal Serial Bus platform: NXP NXP
Projects
Status: Done
8 participants