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: rework usb_dc_mcux_ehci #21175

Merged

Conversation

jfischer-no
Copy link
Collaborator

@jfischer-no jfischer-no commented Dec 4, 2019

This is bug fix, rework, and style fix of usb_dc_mcux_ehci

Fixes #19763

@jfischer-no jfischer-no added bug The issue is a bug, or the PR is fixing a bug area: Drivers area: USB Universal Serial Bus area: Code Style labels Dec 4, 2019
@carlescufi carlescufi added this to the v2.1.0 milestone Dec 4, 2019
@jfischer-no jfischer-no force-pushed the pr-fix-usb_dc_mcux_ehci branch from 03b6bf6 to 808f7ad Compare December 4, 2019 11:14
@jfischer-no
Copy link
Collaborator Author

Tested on IMXRT1050-EVKB, passes USB 2 CV Chapter 9 and all testusb tests now.

@ioannisg
Copy link
Member

ioannisg commented Dec 5, 2019

We should perhaps hold this back, merge it after 2.1 release, we are in -rc3 already.

@ioannisg ioannisg modified the milestones: v2.1.0, v2.2.0 Dec 5, 2019
drivers/usb/device/usb_dc_mcux_ehci.c Outdated Show resolved Hide resolved
drivers/usb/device/usb_dc_mcux_ehci.c Outdated Show resolved Hide resolved
drivers/usb/device/usb_dc_mcux_ehci.c Outdated Show resolved Hide resolved
drivers/usb/device/usb_dc_mcux_ehci.c Show resolved Hide resolved
drivers/usb/device/usb_dc_mcux_ehci.c Show resolved Hide resolved
@MaureenHelm
Copy link
Member

@MarkWangChinese please review

@jfischer-no
Copy link
Collaborator Author

Rebased.

@MaureenHelm The header file modules/hal/nxp/mcux/middleware/usb/device/port/zephyr/usb_dc_mcux.h is a part of the driver. It hides a lot of things, has unnecessary complexity, and does not help to maintain the driver. This file should be moved to drivers/usb/device/usb_dc_mcux_hal.h and reworked/simplified. I would like to do that, this would also help to fix high speed support what I really would like to improve and test with imxrt1050.

@galak galak added the platform: NXP NXP label Jan 29, 2020
@jhedberg
Copy link
Member

jhedberg commented Feb 4, 2020

@MaureenHelm did you still want someone else to review this? I see you mentioned @MarkWangChinese back in December.

@jhedberg
Copy link
Member

@jfischer-phytec-iot there seems to be a conflict with drivers/usb/device/usb_dc_mcux_ehci.c so you'll need to rebase. @MaureenHelm is someone else still expected to review this?

Fix style, variables name, defines.

Signed-off-by: Johann Fischer <j.fischer@phytec.de>
Remove unnecessary cast, fix style.

Signed-off-by: Johann Fischer <j.fischer@phytec.de>
Fix style, rework usb_dc_attach.

Signed-off-by: Johann Fischer <j.fischer@phytec.de>
Fix style, rework usb_dc_detach().

Signed-off-by: Johann Fischer <j.fischer@phytec.de>
Do not reconfigure active endpoint. Fix style.

Signed-off-by: Johann Fischer <j.fischer@phytec.de>
Rework set/clear stall. Fix style.

Signed-off-by: Johann Fischer <j.fischer@phytec.de>
Rework endpoint enable/disable. Fix style.

Signed-off-by: Johann Fischer <j.fischer@phytec.de>
Rework ep_write/ep_read. Fix style.

Signed-off-by: Johann Fischer <j.fischer@phytec.de>
Rework USB_DeviceNotificationTrigger(). Fix style.

Drop messages from USB_DeviceEhciCancel().
MCUX EHCI driver notifies about canceled transfers,
but there is no specific code for this event in
usb_device_callback_message_struct_t, the only way to
recognize it is to check the length value.

Signed-off-by: Johann Fischer <j.fischer@phytec.de>
Add checks for valid endpoint.
Fix controllerHandle check in usb_dc_detach().

Fixes: zephyrproject-rtos#19763

Signed-off-by: Johann Fischer <j.fischer@phytec.de>
Re-enable reception after clear an endpoint stall.

Signed-off-by: Johann Fischer <j.fischer@phytec.de>
@jfischer-no jfischer-no force-pushed the pr-fix-usb_dc_mcux_ehci branch from b176570 to 26e1ad6 Compare February 13, 2020 12:00
@hakehuang
Copy link
Collaborator

test pass in this PR @MaureenHelm , @dlech

_descriptor_start 0x20002b5c
D: __usb_descriptor_end 0x20002bcd
D: Configuration descriptor 0x20002b6e
D: Interface descriptor 0x20002b77
D: Endpoint descriptor 0x20002b80
D: Fixing EP address 81 -> 81
D: endpoint 0x81
D: Now the wTotalLength is 25
D: idx_max 11, ascii_idx_max 5, buf 0x20002b8d
D: char R : 52, idx 5 -> 11
D: char Y : 59, idx 4 -> 9
D: char H : 48, idx 3 -> 7
D: char P : 50, idx 2 -> 5
D: char E : 45, idx 1 -> 3
D: char Z : 5a, idx 0 -> 1
D: idx_max 13, ascii_idx_max 6, buf 0x20002b9b
D: char V : 56, idx 6 -> 13
D: char E : 45, idx 5 -> 11
D: char D : 44, idx 4 -> 9
D: char - : 2d, idx 3 -> 7
D: char B : 42, idx 2 -> 5
D: char S : 53, idx 1 -> 3
D: char U : 55, idx 0 -> 1
D: Serial Number
D: d7 49 4c 5a 46 00 00 50 |.ILZF..P
D: idx_max 31, ascii_idx_max 15, buf 0x20002bab
D: char 7 : 37, idx 15 -> 31
D: char D : 44, idx 14 -> 29
D: char 9 : 39, idx 13 -> 27
D: char 4 : 34, idx 12 -> 25
D: char C : 43, idx 11 -> 23
D: char 4 : 34, idx 10 -> 21
D: char A : 41, idx 9 -> 19
D: char 5 : 35, idx 8 -> 17
D: char 6 : 36, idx 7 -> 15
D: char 4 : 34, idx 6 -> 13
D: char 0 : 30, idx 5 -> 11
D: char 0 : 30, idx 4 -> 9
D: char 0 : 30, idx 3 -> 7
D: char 0 : 30, idx 2 -> 5
D: char 0 : 30, idx 1 -> 3
D: char 5 : 35, idx 0 -> 1
*** Booting Zephyr OS version 2.2.0-rc1  ***
D: lock usb_enable_lock mutex
D: Attached
D: set cb, ep: 0x81
D: unlock usb_enable_lock mutex
Running test suite test_device
===================================================================
starting test - test_usb_dc_api_invalid
E: Wrong endpoint index/address
E: Wrong endpoint index/address
E: Wrong endpoint index/address
E: Wrong endpoint index/address
E: Wrong endpoint index/address
E: Wrong endpoint index/address
E: Wrong endpoint index/address
E: Wrong endpoint index/address
E: Wrong endpoint index/address
E: Wrong endpoint index/address/direction
E: Wrong endpoint index/address/direction
E: Wrong endpoint index/address
E: Wrong endpoint index/address
PASS - test_usb_dc_api_invalid
===================================================================
starting test - test_usb_dc_api
PASS - test_usb_dc_api
===================================================================
starting test - test_usb_dc_api_read_write
E: Wrong endpoint index/address/direction
E: Wrong endpoint index/address
PASS - test_usb_dc_api_read_write
===================================================================
starting test - test_usb_dc_api_invalid
E: Wrong endpoint index/address
E: Wrong endpoint index/address
E: Wrong endpoint index/address
E: Wrong endpoint index/address
E: Wrong endpoint index/address
E: Wrong endpoint index/address
E: Wrong endpoint index/address
E: Wrong endpoint index/address
E: Wrong endpoint index/address
E: Wrong endpoint index/address/direction
E: Wrong endpoint index/address/direction
E: Wrong endpoint index/address
E: Wrong endpoint index/address
PASS - test_usb_dc_api_invalid
===================================================================
starting test - test_usb_deconfig
PASS - test_usb_deconfig
===================================================================
starting test - test_usb_disable
W: Device not attached
PASS - test_usb_disable
===================================================================
Test suite test_device succeeded
===================================================================
PROJECT EXECUTION SUCCESSFUL

@carlescufi
Copy link
Member

@MaureenHelm this fixes a bug, has been opened for a while and is scheduled for this release. Could you review it so we can include it in the release?
cc @jhedberg

@jhedberg jhedberg modified the milestones: v2.2.0, v2.3.0 Mar 10, 2020
@carlescufi carlescufi merged commit 085e58f into zephyrproject-rtos:master Mar 10, 2020
@jfischer-no jfischer-no deleted the pr-fix-usb_dc_mcux_ehci branch September 15, 2021 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Drivers area: USB Universal Serial Bus bug The issue is a bug, or the PR is fixing a bug platform: NXP NXP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tests/subsys/usb/device/ failed on mimxrt1050_evk board.
8 participants