Ferry Toth wrote:
Hi
Op 17-04-2021 om 16:22 schreef Ferry Toth:
Hi
Op 17-04-2021 om 04:27 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 16-04-2021 om 00:23 schreef Thinh Nguyen:
Thinh Nguyen wrote:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115...
[2] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com
Changes in v3:
- Check if the desired mode is OTG, then keep the old flow
- Remove condition for OTG support only since the device can still be
configured DRD host/device mode only
- Remove redundant hw_mode check since __dwc3_set_mode() only applies
when hw_mode is DRD Changes in v2:
- Initialize mutex per device and not as global mutex.
- Add additional checks for DRD only mode
drivers/usb/dwc3/core.c | 27 +++++++++++++++++++++++++++ drivers/usb/dwc3/core.h | 5 +++++ 2 files changed, 32 insertions(+)
Hi John,
If possible, can you run a test with this version on your platform?
Thanks, Thinh
I tested this on edison-arduino with this patch on top of usb-next (5.12-rc7 + "increase BESL baseline to 6" to prevent throttling").
On this platform there is a physical switch to switch roles. With this patch I find:
switch to host mode always works fine
switch to gadget mode I need to flip the switch 3x
(gadget-host-gadget).
An error message appears on the gadget side "dwc3 dwc3.0.auto: timed out waiting for SETUP phase" appears, but then the device connects to my PC, no throttling.
- alternatively I can switch to gadget 1x and then unplug/replug the
cable.
No error message and connects fine.
- if I flip the switch only once, on the PC side I get:
kernel: usb 1-5: new high-speed USB device number 18 usingxhci_hcd kernel: usb 1-5: New USB device found, idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 kernel: usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 kernel: usb 1-5: Product: USBArmory Gadget kernel: usb 1-5: Manufacturer: USBArmory kernel: usb 1-5: SerialNumber: 0123456789abcdef kernel: usb 1-5: can't set config #1, error -110
The device failed at set_configuration() request and timed out. It probably timed out from the status stage looking at the device err print.
Then if I wait long enough on the gadget side I get:
root@yuna:~# ifconfig
usb0: flags=-28605<UP,BROADCAST,RUNNING,MULTICAST,DYNAMIC> mtu 1500 inet 169.254.119.239 netmask 255.255.0.0 broadcast 169.254.255.255 inet6 fe80::a8bb:ccff:fedd:eef1 prefixlen 64 scopeid 0x20<link> ether aa:bb:cc:dd:ee:f1 txqueuelen 1000 (Ethernet) RX packets 490424 bytes 735146578 (701.0 MiB) RX errors 0 dropped 191 overruns 0 frame 0 TX packets 35279 bytes 2532746 (2.4 MiB) TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0
(correct would be: inet 10.42.0.221 netmask 255.255.255.0 broadcast 10.42.0.255)
So much improved now, but it seems I am still missing something on plug.
That's great! We can look at it further. Can you capture the tracepoints of the issue. Also, can you try with mass_storage gadget to see if the result is the same?
I have already gser, eem, mass_storage and uac2 combo. When eem fails, the mass_storage and uac2 don't appear (on KDE you get all kind of popups when they appear).
So either all works, or all fails.
I'll trace this later today.
Trace capturing switch from host-> gadget here https://urldefense.com/v3/__https://github.com/andy-shev/linux/files/6329600...
(Issue history: https://urldefense.com/v3/__https://github.com/andy-shev/linux/issues/31__%3... )
On the PC side this resulted to:
apr 17 18:17:44 delfion kernel: usb 1-5: new high-speed USB device number 12 using xhci_hcd apr 17 18:17:44 delfion kernel: usb 1-5: New USB device found, idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 apr 17 18:17:44 delfion kernel: usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 apr 17 18:17:44 delfion kernel: usb 1-5: Product: USBArmory Gadget apr 17 18:17:44 delfion kernel: usb 1-5: Manufacturer: USBArmory apr 17 18:17:44 delfion kernel: usb 1-5: SerialNumber: 0123456789abcdef apr 17 18:17:49 delfion kernel: usb 1-5: can't set config #1, error -110
Thanks for all your help!
Looks like it's LPM related again. To confirm, try this: Disable LPM with this property "snps,usb2-gadget-lpm-disable" (Note that it's not the same as "snps,dis_enblslpm_quirk")
Make sure that your testing kernel has this patch [1] 475e8be53d04 ("usb: dwc3: gadget: Check for disabled LPM quirk")
[1] https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb...
The failure you saw was probably due the gadget function attempting to start a delayed status stage of the SET_CONFIGURATION request. By this time, the host already put the device in low power.
The START_TRANSFER command needs to be executed while the device is on "ON" state (or U0 if eSS). We shouldn't use dwc->link_state to check for link state because we only enable link state change interrupt for some controller versions.
Once you confirms disabling LPM works, try this fix:
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 6227641f2d31..06cdec79244e 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -309,10 +309,14 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd,
if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_STARTTRANSFER) { int needs_wakeup; + u8 link_state;
- needs_wakeup = (dwc->link_state == DWC3_LINK_STATE_U1 || - dwc->link_state == DWC3_LINK_STATE_U2 || - dwc->link_state == DWC3_LINK_STATE_U3); + reg = dwc3_readl(dwc->regs, DWC3_DSTS); + link_state = DWC3_DSTS_USBLNKST(reg); + + needs_wakeup = (link_state == DWC3_LINK_STATE_U1 || + link_state == DWC3_LINK_STATE_U2 || + link_state == DWC3_LINK_STATE_U3);
if (unlikely(needs_wakeup)) { ret = __dwc3_gadget_wakeup(dwc); @@ -1989,6 +1993,8 @@ static int __dwc3_gadget_wakeup(struct dwc3 *dwc) case DWC3_LINK_STATE_RESET: case DWC3_LINK_STATE_RX_DET: /* in HS, means Early Suspend */ case DWC3_LINK_STATE_U3: /* in HS, means SUSPEND */ + case DWC3_LINK_STATE_U2: /* in HS, means Sleep (L1) */ + case DWC3_LINK_STATE_U1: case DWC3_LINK_STATE_RESUME: break; default:
BR, Thinh