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

drivers: udc_mcux_ehci: fix typo in transfer handler #85004

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

jfischer-no
Copy link
Collaborator

@jfischer-no jfischer-no commented Feb 1, 2025

If the to-host data stage length is less than that requested by the
host, but equal to or a multiple of MPS, the device should send a ZLP,
not receive it.

Fixes: #84999
Tested on mimxrt1020_evk

udc_mcux_ip3511.c has similar type, but according to the reporter it would not fix similar issue there, so there should be something else.

@henrikbrixandersen
Copy link
Member

If the to-host data stage length is less than that requested by the host, but equal to or a multiple of MPS, the device should send a ZLP, not receive it.

Fixes: #84999 Tested on mimxrt1020_evk

Confirmed, tested with mimxrt1014_evk.

udc_mcux_ip3511.c has similar type, so it should also Fixes: #83306, not tested.

My lpcxpresso55s16 board still doesn't enumerate on MS Windows even with this fix.

@jfischer-no
Copy link
Collaborator Author

udc_mcux_ip3511.c has similar type, so it should also Fixes: #83306, not tested.

My lpcxpresso55s16 board still doesn't enumerate on MS Windows even with this fix.

From your description it looks like similar issue, can you please add LOG_ERRs like

diff --git a/drivers/usb/udc/udc_mcux_ip3511.c b/drivers/usb/udc/udc_mcux_ip3511.c
index 9b1665168c5..8800e7a8976 100644
--- a/drivers/usb/udc/udc_mcux_ip3511.c
+++ b/drivers/usb/udc/udc_mcux_ip3511.c
@@ -354,10 +354,12 @@ static bool udc_mcux_handler_zlt(const struct device *dev, uint8_t ep, struct ne
        struct udc_mcux_data *priv = udc_get_private(dev);
 
        /* The whole transfer is already done by MCUX controller driver. */
+       LOG_ERR("ep %x mcux_len %u buf->len %u", ep, mcux_len, buf->len);
        if (mcux_len >= buf->len) {
                if (udc_ep_buf_has_zlp(buf)) {
                        usb_status_t status;
 
+                       LOG_ERR("zlp");
                        udc_ep_buf_clear_zlp(buf);
                        status = mcux_if->deviceSend(priv->mcux_device.controllerHandle,
                                        ep, NULL, 0);

and post the logs here?

Copy link
Contributor

@tmon-nordic tmon-nordic left a comment

Choose a reason for hiding this comment

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

Don't include two pretty much independent commits in a single PR. :samples: hid-keyboard: initialize report buffer at startup" is pretty much completely unrelated to "drivers: udc_mcux_ehci: fix typo in transfer handler".

kartben
kartben previously approved these changes Feb 3, 2025
@jfischer-no
Copy link
Collaborator Author

Don't include two pretty much independent commits in a single PR. :samples: hid-keyboard: initialize report buffer at startup" is pretty much completely unrelated to "drivers: udc_mcux_ehci: fix typo in transfer handler".

It is related to the MCUX UDC drivers. But it needs more investigation.

kartben
kartben previously approved these changes Feb 3, 2025
@henrikbrixandersen
Copy link
Member

udc_mcux_ip3511.c has similar type, so it should also Fixes: #83306, not tested.

My lpcxpresso55s16 board still doesn't enumerate on MS Windows even with this fix.

From your description it looks like similar issue, can you please add LOG_ERRs like

diff --git a/drivers/usb/udc/udc_mcux_ip3511.c b/drivers/usb/udc/udc_mcux_ip3511.c
index 9b1665168c5..8800e7a8976 100644
--- a/drivers/usb/udc/udc_mcux_ip3511.c
+++ b/drivers/usb/udc/udc_mcux_ip3511.c
@@ -354,10 +354,12 @@ static bool udc_mcux_handler_zlt(const struct device *dev, uint8_t ep, struct ne
        struct udc_mcux_data *priv = udc_get_private(dev);
 
        /* The whole transfer is already done by MCUX controller driver. */
+       LOG_ERR("ep %x mcux_len %u buf->len %u", ep, mcux_len, buf->len);
        if (mcux_len >= buf->len) {
                if (udc_ep_buf_has_zlp(buf)) {
                        usb_status_t status;
 
+                       LOG_ERR("zlp");
                        udc_ep_buf_clear_zlp(buf);
                        status = mcux_if->deviceSend(priv->mcux_device.controllerHandle,
                                        ep, NULL, 0);

and post the logs here?


*** Booting Zephyr OS build v4.0.0-4411-g839bcdca6588 ***
[00:00:00.104,000] <inf> main: USBD message: Bus reset
[00:00:00.183,000] <err> udc_mcux: ep 80 mcux_len 18 buf->len 18
[00:00:00.184,000] <err> udc_mcux: ep 80 mcux_len 0 buf->len 0
[00:00:00.194,000] <err> udc_mcux: ep 80 mcux_len 18 buf->len 18
[00:00:00.203,000] <err> udc_mcux: ep 80 mcux_len 32 buf->len 32
[00:00:00.204,000] <err> udc_mcux: ep 80 mcux_len 64 buf->len 64
[00:00:00.204,000] <err> udc_mcux: zlp
[00:00:05.719,000] <err> udc_mcux: ep 80 mcux_len 65535 buf->len 64
[00:00:05.720,000] <inf> main: USBD message: Bus reset
[00:00:05.864,000] <err> udc_mcux: ep 80 mcux_len 18 buf->len 18
[00:00:05.864,000] <err> usbd_core: UDC error event
[00:00:05.864,000] <inf> main: USBD message: Controller error
[00:00:10.874,000] <err> udc_mcux: ep 80 mcux_len 0 buf->len 0
[00:00:10.884,000] <err> udc_mcux: ep 80 mcux_len 18 buf->len 18
[00:00:10.884,000] <err> usbd_core: UDC error event
[00:00:10.884,000] <inf> main: USBD message: Controller error
[00:00:15.912,000] <err> udc_mcux: ep 80 mcux_len 32 buf->len 32
[00:00:15.912,000] <err> usbd_core: UDC error event
[00:00:15.912,000] <inf> main: USBD message: Controller error
[00:00:21.418,000] <inf> main: USBD message: Bus reset
[00:00:21.563,000] <err> udc_mcux: ep 80 mcux_len 18 buf->len 18
[00:00:21.563,000] <err> usbd_core: UDC error event
[00:00:21.563,000] <inf> main: USBD message: Controller error
[00:00:26.572,000] <err> udc_mcux: ep 80 mcux_len 0 buf->len 0
[00:00:26.582,000] <err> udc_mcux: ep 80 mcux_len 18 buf->len 18
[00:00:26.583,000] <err> usbd_core: UDC error event
[00:00:26.583,000] <inf> main: USBD message: Controller error
[00:00:31.598,000] <err> udc_mcux: ep 80 mcux_len 32 buf->len 32
[00:00:31.598,000] <err> usbd_core: UDC error event
[00:00:31.599,000] <inf> main: USBD message: Controller error
[00:00:37.107,000] <inf> main: USBD message: Bus reset
[00:00:37.251,000] <err> udc_mcux: ep 80 mcux_len 18 buf->len 18
[00:00:37.251,000] <err> usbd_core: UDC error event
[00:00:37.252,000] <inf> main: USBD message: Controller error
[00:00:42.254,000] <err> udc_mcux: ep 80 mcux_len 0 buf->len 0
[00:00:42.264,000] <err> udc_mcux: ep 80 mcux_len 18 buf->len 18
[00:00:42.264,000] <err> usbd_core: UDC error event
[00:00:42.265,000] <inf> main: USBD message: Controller error
[00:00:47.286,000] <err> udc_mcux: ep 80 mcux_len 32 buf->len 32
[00:00:47.286,000] <err> usbd_core: UDC error event
[00:00:47.286,000] <inf> main: USBD message: Controller error
...

MarkWangChinese
MarkWangChinese previously approved these changes Feb 5, 2025
Copy link
Member

@henrikbrixandersen henrikbrixandersen left a comment

Choose a reason for hiding this comment

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

Blocking to prevent merge until the fix is working.

If the to-host data stage length is less than that requested by the
host, but equal to or a multiple of MPS, the device should send a ZLP,
not receive it.

Signed-off-by: Johann Fischer <[email protected]>
@zephyrbot zephyrbot added the size: XS A PR changing only a single line of code label Feb 5, 2025
@jfischer-no
Copy link
Collaborator Author

@MarkWangChinese udc_mcux_ip3511.c has similar typo, but according to the reporter it would not fix it, I do not have hardware to investigate it further, can you please look at the #83306? Thank you.

@jfischer-no
Copy link
Collaborator Author

Blocking to prevent merge until the fix is working.

@henrikbrixandersen I reduced the commit to fix #84999 only. This is tested and should not be blocked.

@MarkWangChinese
Copy link
Contributor

@MarkWangChinese udc_mcux_ip3511.c has similar typo, but according to the reporter it would not fix it, I do not have hardware to investigate it further, can you please look at the #83306? Thank you.

OK, I will look at it later.

@kartben kartben merged commit ea3dbe9 into zephyrproject-rtos:main Feb 6, 2025
27 checks passed
@jfischer-no jfischer-no deleted the pr-fix-84999 branch February 6, 2025 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Drivers area: Samples Samples area: USB Universal Serial Bus Experimental Experimental features not enabled by default platform: NXP Drivers NXP Semiconductors, drivers size: XS A PR changing only a single line of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

samples: usb: webusb-next: failure to enumerate on mimxrt1024_evk/mimxrt1024 under MS Windows
7 participants