[UBOOT PATCH 1/3] usb: dwc3: core: improve reset sequence
Marek Vasut
marex at denx.de
Mon Jun 19 23:43:34 CEST 2023
On 6/19/23 19:04, Eugen Hristev wrote:
> On 6/19/23 19:07, Marek Vasut wrote:
>> On 6/19/23 15:26, Eugen Hristev wrote:
>>> On 5/8/23 06:00, Venkatesh Yadav Abbarapu wrote:
>>>> [ 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 at amd.com>
>>>> ---
>>>> drivers/usb/dwc3/core.c | 50
>>>> +++++++++++++++--------------------------
>>>> 1 file changed, 18 insertions(+), 32 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>> index 49f6a1900b..8702a54efa 100644
>>>> --- a/drivers/usb/dwc3/core.c
>>>> +++ b/drivers/usb/dwc3/core.c
>>>> @@ -60,42 +60,28 @@ 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);
>>>> -
>>>> - /* 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;
>>>> - dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>>>> -
>>>> - mdelay(100);
>>>> -
>>>> - /* Clear USB3 PHY reset */
>>>> - reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
>>>> - reg &= ~DWC3_GUSB3PIPECTL_PHYSOFTRST;
>>>> - dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
>>>> -
>>>> - /* Clear USB2 PHY reset */
>>>> - reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>>>> - reg &= ~DWC3_GUSB2PHYCFG_PHYSOFTRST;
>>>> - dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), 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;
>>>> - mdelay(100);
>>>> + reg = dwc3_readl(dwc->regs, DWC3_DCTL);
>>>> + reg |= DWC3_DCTL_CSFTRST;
>>>> + dwc3_writel(dwc->regs, DWC3_DCTL, reg);
>>>> - /* 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;
>>>> }
>>>> /*
>>>
>>> Hello Venkatesh, Michal,
>>>
>>> I randomly found this patch while testing the Dwc3 on rockchip RK3588
>>> , I noticed that you removed the code that resets the PHYs in this
>>> patch hence DWC3 is no longer starting in my case.
>>> Was this intentional with this patch ?
>>
>> All of these patches are NAK until the DWC3 is synchronized with
>> Linux. Picking random DWC3 patches and ignoring other fixes will turn
>> the whole driver into an unmaintainable mess and make the sync that
>> much harder in the future. I spent enormous amount of my spare time
>> trying to explain to Xilinx how to do that mostly automatically, but
>> that was all ignored or countered with reason after reason why this
>> cannot be done, but as far as I can tell, nobody in Xilinx actually
>> tried the proposal.
>>
>> Hence, NAK
>>
>> Hence, no need to worry these patches will be applied in their current
>> state.
>
> Hi Marek,
>
> that's fine. No argument from my side.
>
> What I wanted to actually tell/ask , is the fact that this patch
> actually helps in my case, just that it breaks something else, and I
> wanted to get more info from the patch authors.
>
> I am experiencing some situations where dwc3 does not correctly start
> unless I add some manual delays here and there, and I am trying to see why.
OK
More information about the U-Boot
mailing list