Re: [PATCH] usb: dwc3: Fix latency of DSTS while receiving wakeup event

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 07-08-24 05:21 am, Thinh Nguyen wrote:
Hi,

On Tue, Jul 30, 2024, Prashanth K wrote:
When operating in High-Speed, it is observed that DSTS[USBLNKST] doesn't
update link state immediately after receiving the wakeup interrupt. Since
wakeup event handler calls the resume callbacks, there is a chance that
function drivers can perform an ep queue. Which in turn tries to perform
remote wakeup from send_gadget_ep_cmd(), this happens because DSTS[[21:18]
wasn't updated to U0 yet. It is observed that the latency of DSTS can be
in order of milli-seconds. Hence update the dwc->link_state from evtinfo,
and use this variable to prevent calling remote wakup unnecessarily.

Fixes: ecba9bc9946b ("usb: dwc3: gadget: Check for L1/L2/U3 for Start Transfer")

This commit ID is corrupted. Please check.

Will fix it, was supposed to be 63c4c320ccf7, thanks for pointing out.

While operating in usb2 speed, if the device is in low power link state
(L1/L2), CMDACT may not complete and time out. The programming guide
suggested to initiate remote wakeup to bring the device to ON state,
allowing the command to go through. However, clearing the

Yea true, we need ensure that the linkstate is not in L1/L2/U3 for HS/SS. But since we are relying on DSTS for this, we may issue remote-wakeup to host even when not needed. During host initiated wakeup scenario, we get a wakeup interrupt which calls function driver resume calls. If function driver queues something, then startxfer has to be issued, but DSTS was still showing U3 instead of U0. When checked with our design team, they mentioned the latency in DSTS is expected since and latency would be in msec order from Resume to U0. Can you please confirm this once, I simply added a polling mechanism in wakeup handler.

@@ -4175,6 +4177,14 @@ static void dwc3_gadget_wakeup_interrupt(struct dwc3 *dwc, unsigned int evtinfo)
         * TODO take core out of low power mode when that's
         * implemented.
         */
+       while (retries++ < 20000) {
+               reg = dwc3_readl(dwc->regs, DWC3_DSTS);
+               /* in HS, means ON */
+               if (DWC3_DSTS_USBLNKST(reg) == DWC3_LINK_STATE_U0)
+                       break;
+               udelay(2);
+       }
+       pr_info("DWC3 Wakeup: %d", retries);

And turns out, retries 1500 to 15000 (worst case), which can range from 3ms to 30ms. By this time, control can reach startXfer, where it tries to perform remote-wakeup even if host just resumed the gadget.

For SS case, this retries count was consistently 1, it was passing in first try itself. But unfortunately doesn't behave the same way in HS.

GUSB2PHYCFG.suspendusb2 turns on the signal required to complete a
command within 50us. This happens within the timeout required for an
endpoint command. As a result, there's no need to perform remote wakeup.

For usb3 speed, if it's in U3, the gadget is in suspend anyway. There
will be no ep_queue to trigger the Start Transfer command.

You can just remove the whole Start Transfer check for remote wakeup
completely.

Sorry, i didnt understand your suggestion. The startxfer check is needed as per databook, but we also need to handle the latency seen in DSTS when operating in HS.

Thanks,
Prashanth K




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux