Re: [PATCH v2 02/10] usb: dwc3: core: improve reset sequence

[Marek Vasut]

On 8/1/23 09:28, Eugen Hristev wrote:

From: Venkatesh Yadav Abbarapu 

[ Felipe: Ported from Linux kernel commit
  f59dcab17629 ("usb: dwc3: core: improve reset sequence") ]

According to Synopsys Databook, we shouldn't be relying on
GCTL.CORESOFTRESET bit as that's only for debugging purposes.
Instead, let's use DCTL.CSFTRST if we're OTG or PERIPHERAL mode.

Host side block will be reset by XHCI driver if necessary. Note that this
reduces amount of time spent on dwc3_probe() by a long margin.

We're still gonna wait for reset to finish for a long time
(default to 1ms max), but tests show that the reset polling loop executed
at most 19 times (modprobe dwc3 && modprobe -r dwc3 executed 1000
times in a row).

Without proper core reset, observing random issues like when the
USB(DWC3) is in device mode, the host device is not able to detect the
USB device.

Signed-off-by: Venkatesh Yadav Abbarapu 
[eugen.hris...@collabora.com: keep the PHY resets code]
Signed-off-by: Eugen Hristev 


NAK


---
Not to be merged, I know Marek does not apply any patches to DWC3.


That is not true and taken out of context.

What needs to happen is, someone needs to sync the DWC3 with Linux 
instead of picking random subsets patches and turning the driver into a 
total unmaintainable mess, which will be horribly difficult to sync in 
the future due to these random patches mixed in the history. I explained 
how to do it to Xilinx, it is a trivial thing to do, but it seems they 
did not even bother to try the method.

[PATCH v2 02/10] usb: dwc3: core: improve reset sequence

[Eugen Hristev]

From: Venkatesh Yadav Abbarapu 

[ Felipe: Ported from Linux kernel commit
  f59dcab17629 ("usb: dwc3: core: improve reset sequence") ]

According to Synopsys Databook, we shouldn't be relying on
GCTL.CORESOFTRESET bit as that's only for debugging purposes.
Instead, let's use DCTL.CSFTRST if we're OTG or PERIPHERAL mode.

Host side block will be reset by XHCI driver if necessary. Note that this
reduces amount of time spent on dwc3_probe() by a long margin.

We're still gonna wait for reset to finish for a long time
(default to 1ms max), but tests show that the reset polling loop executed
at most 19 times (modprobe dwc3 && modprobe -r dwc3 executed 1000
times in a row).

Without proper core reset, observing random issues like when the
USB(DWC3) is in device mode, the host device is not able to detect the
USB device.

Signed-off-by: Venkatesh Yadav Abbarapu 
[eugen.hris...@collabora.com: keep the PHY resets code]
Signed-off-by: Eugen Hristev 
---
Not to be merged, I know Marek does not apply any patches to DWC3.

 drivers/usb/dwc3/core.c | 31 +++
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 5a8c29424578..bdfe51c3df96 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -60,17 +60,24 @@ static void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
 static int dwc3_core_soft_reset(struct dwc3 *dwc)
 {
u32 reg;
+   int retries = 1000;
 
-   /* Before Resetting PHY, put Core in Reset */
-   reg = dwc3_readl(dwc->regs, DWC3_GCTL);
-   reg |= DWC3_GCTL_CORESOFTRESET;
-   dwc3_writel(dwc->regs, DWC3_GCTL, reg);
+   /*
+* We're resetting only the device side because, if we're in host mode,
+* XHCI driver will reset the host block. If dwc3 was configured for
+* host-only mode, then we can return early.
+*/
+   if (dwc->dr_mode == USB_DR_MODE_HOST)
+   return 0;
+
+   reg = dwc3_readl(dwc->regs, DWC3_DCTL);
+   reg |= DWC3_DCTL_CSFTRST;
+   dwc3_writel(dwc->regs, DWC3_DCTL, reg);
 
/* Assert USB3 PHY reset */
reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
reg |= DWC3_GUSB3PIPECTL_PHYSOFTRST;
dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
-
/* Assert USB2 PHY reset */
reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
reg |= DWC3_GUSB2PHYCFG_PHYSOFTRST;
@@ -88,14 +95,14 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
reg &= ~DWC3_GUSB2PHYCFG_PHYSOFTRST;
dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
 
-   mdelay(100);
-
-   /* After PHYs are stable we can take Core out of reset state */
-   reg = dwc3_readl(dwc->regs, DWC3_GCTL);
-   reg &= ~DWC3_GCTL_CORESOFTRESET;
-   dwc3_writel(dwc->regs, DWC3_GCTL, reg);
+   do {
+   reg = dwc3_readl(dwc->regs, DWC3_DCTL);
+   if (!(reg & DWC3_DCTL_CSFTRST))
+   return 0;
+   udelay(1);
+   } while (--retries);
 
-   return 0;
+   return -ETIMEDOUT;
 }
 
 /*
-- 
2.34.1