Hi, Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes: >> (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
Attachment:
signature.asc
Description: PGP signature