[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240816214941.l3el46ittrugxqp5@synopsys.com>
Date: Fri, 16 Aug 2024 21:49:48 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: Prashanth K <quic_prashk@...cinc.com>
CC: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [v2] usb: dwc3: Avoid waking up gadget during startxfer
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@...r.kernel.org>
> Suggested-by: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
> Signed-off-by: Prashanth K <quic_prashk@...cinc.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
>
Powered by blists - more mailing lists