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(STARTXFER). This happens because DSTS[[21:18] wasn't updated to U0 yet, it's observed that the latency of DSTS can be in order of milli-seconds. Hence avoid calling gadget_wakeup during startxfer to prevent unnecessarily issuing remote wakeup to host.
Fixes: c36d8e947a56 ("usb: dwc3: gadget: put link to U0 before Start Transfer") Cc: stable@vger.kernel.org Suggested-by: Thinh Nguyen Thinh.Nguyen@synopsys.com Signed-off-by: Prashanth K quic_prashk@quicinc.com --- v2: Refactored the patch as suggested in v1 discussion.
drivers/usb/dwc3/gadget.c | 24 ------------------------ 1 file changed, 24 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 89fc690fdf34..3f634209c5b8 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -327,30 +327,6 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd, dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg); }
- if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_STARTTRANSFER) { - int link_state; - - /* - * Initiate remote wakeup if the link state is in U3 when - * operating in SS/SSP or L1/L2 when operating in HS/FS. If the - * link state is in U1/U2, no remote wakeup is needed. The Start - * Transfer command will initiate the link recovery. - */ - link_state = dwc3_gadget_get_link_state(dwc); - switch (link_state) { - case DWC3_LINK_STATE_U2: - if (dwc->gadget->speed >= USB_SPEED_SUPER) - break; - - fallthrough; - case DWC3_LINK_STATE_U3: - ret = __dwc3_gadget_wakeup(dwc, false); - dev_WARN_ONCE(dwc->dev, ret, "wakeup failed --> %d\n", - ret); - break; - } - } - /* * For some commands such as Update Transfer command, DEPCMDPARn * registers are reserved. Since the driver often sends Update Transfer
On Fri, Aug 16, 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(STARTXFER). This happens because DSTS[[21:18] wasn't updated to U0 yet, it's observed that the latency of DSTS can be in order of milli-seconds. Hence avoid calling gadget_wakeup during startxfer to prevent unnecessarily issuing remote wakeup to host.
Fixes: c36d8e947a56 ("usb: dwc3: gadget: put link to U0 before Start Transfer") Cc: stable@vger.kernel.org Suggested-by: Thinh Nguyen Thinh.Nguyen@synopsys.com Signed-off-by: Prashanth K quic_prashk@quicinc.com
v2: Refactored the patch as suggested in v1 discussion.
drivers/usb/dwc3/gadget.c | 24 ------------------------ 1 file changed, 24 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 89fc690fdf34..3f634209c5b8 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -327,30 +327,6 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd, dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
Can you capture the notes I provided explaining why we can issue Start Transfer without checking for L1/L2/U3 states on top of this function?
Thanks, Thinh
}
- if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_STARTTRANSFER) {
int link_state;/** Initiate remote wakeup if the link state is in U3 when* operating in SS/SSP or L1/L2 when operating in HS/FS. If the* link state is in U1/U2, no remote wakeup is needed. The Start* Transfer command will initiate the link recovery.*/link_state = dwc3_gadget_get_link_state(dwc);switch (link_state) {case DWC3_LINK_STATE_U2:if (dwc->gadget->speed >= USB_SPEED_SUPER)break;fallthrough;case DWC3_LINK_STATE_U3:ret = __dwc3_gadget_wakeup(dwc, false);dev_WARN_ONCE(dwc->dev, ret, "wakeup failed --> %d\n",ret);break;}- }
- /*
- For some commands such as Update Transfer command, DEPCMDPARn
- registers are reserved. Since the driver often sends Update Transfer
-- 2.25.1
On 17-08-24 03:19 am, Thinh Nguyen wrote:
On Fri, Aug 16, 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(STARTXFER). This happens because DSTS[[21:18] wasn't updated to U0 yet, it's observed that the latency of DSTS can be in order of milli-seconds. Hence avoid calling gadget_wakeup during startxfer to prevent unnecessarily issuing remote wakeup to host.
Fixes: c36d8e947a56 ("usb: dwc3: gadget: put link to U0 before Start Transfer") Cc: stable@vger.kernel.org Suggested-by: Thinh Nguyen Thinh.Nguyen@synopsys.com Signed-off-by: Prashanth K quic_prashk@quicinc.com
v2: Refactored the patch as suggested in v1 discussion.
drivers/usb/dwc3/gadget.c | 24 ------------------------ 1 file changed, 24 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 89fc690fdf34..3f634209c5b8 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -327,30 +327,6 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd, dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
Can you capture the notes I provided explaining why we can issue Start Transfer without checking for L1/L2/U3 states on top of this function?
I also thought the same initially, but didnt add since Greg usually adds the patch link to commit, so that the discussion would be captured in git log. I will add it on top of dwc3_send_gadget_ep_cmd and send V3, let me know if there's any other suggestion.
Thanks, Prashanth K
linux-stable-mirror@lists.linaro.org
-
Prashanth K -
Thinh Nguyen