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
- signature.asc [application/pgp-signature] 857 bytes