Thread (40 messages) 40 messages, 7 authors, 2021-04-23

Re: [PATCH v3] usb: dwc3: core: Do core softreset when switch mode

From: Felipe Balbi <balbi@kernel.org>
Date: 2021-04-19 09:47:37
Also in: stable

Hi,

Thinh Nguyen [off-list ref] writes:
quoted hunk ↗ jump to hunk
quoted
(Issue history:
https://urldefense.com/v3/__https://github.com/andy-shev/linux/issues/31__;!!A4F2R9G_pg!Oa6XGH3IqY3wwG5KK4FwPuNA0m3q5bRj7N6vdP-y4sAY6mya-96J90NJ0tJnXNc7KgAw$
)

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-next&id=475e8be53d0496f9bc6159f4abb3ff5f9b90e8de


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);
this makes sense. We used to track state using the state change
interrupts, but that's long since being disabled. I think, either way,
we need this fix.

-- 
balbi

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help