| Message ID | 20230622095849.498702-3-eugen.hristev@collabora.com |
|---|---|
| State | Superseded |
| Delegated to: | Kever Yang |
| Headers | show |
| Series | rockchip: rk3588: add support for DFU in SPL | expand |
On 6/22/23 12:58, Eugen Hristev wrote: > From: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com> > > [ 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 <venkatesh.abbarapu@amd.com> > [eugen.hristev@collabora.com: keep the PHY resets code] > Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com> > --- > > Just resending for consistency with my series. I readded the PHY resets > code because it breaks my PHYs if it's being removed > > Marek, I know you NAKed this, that's fine, I am simply adding this patch > (and the others here) for consistency >>> >>> Please sync the DWC3 patchset with Linux first, else this is a NAK. >>> >>> Please do not make the DWC3 driver an unmaintainable mess. >>> >>> I already explained this to AMD/Xilinx, multiple times, they ignored all my requests without even trying, so my NAK still stands. >>> >>> The sync should be easy and mechanical. >>> >>> Please do not try to sneak those patches in as part of another series. >> >> Hi Marek, >> >> I know, I saw the discussion. I am adding the patch in case someone wants to use it. Definitely I am not trying to sneak anything. Read the cover letter as well if you have doubts. > > I tried asking intel just now to fix what AMD couldn't, so let's see. I will try to do it myself, if I have some spare time in the following months, if nobody does it. ^^ I am resending the conversation above as I forgot to CC the mailing list on the initial thread
On 6/22/23 12:00, Eugen Hristev wrote: > On 6/22/23 12:58, Eugen Hristev wrote: >> From: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com> >> >> [ 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 <venkatesh.abbarapu@amd.com> >> [eugen.hristev@collabora.com: keep the PHY resets code] >> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com> >> --- >> >> Just resending for consistency with my series. I readded the PHY resets >> code because it breaks my PHYs if it's being removed >> >> Marek, I know you NAKed this, that's fine, I am simply adding this patch >> (and the others here) for consistency > > >>> > >>> Please sync the DWC3 patchset with Linux first, else this is a NAK. > >>> > >>> Please do not make the DWC3 driver an unmaintainable mess. > >>> > >>> I already explained this to AMD/Xilinx, multiple times, they > ignored all my requests without even trying, so my NAK still stands. > >>> > >>> The sync should be easy and mechanical. > >>> > >>> Please do not try to sneak those patches in as part of another series. > >> > >> Hi Marek, > >> > >> I know, I saw the discussion. I am adding the patch in case someone > wants to use it. Definitely I am not trying to sneak anything. Read the > cover letter as well if you have doubts. > > > > I tried asking intel just now to fix what AMD couldn't, so let's see. > > I will try to do it myself, if I have some spare time in the following > months, if nobody does it. > > ^^ > I am resending the conversation above as I forgot to CC the mailing list > on the initial thread I appreciate it. It really should be easy to do.
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; } /*