[UBOOT PATCH 1/3] usb: dwc3: core: improve reset sequence

Marek Vasut marex at denx.de
Mon Jun 19 18:07:52 CEST 2023


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.


More information about the U-Boot mailing list