[PATCH v1] HSD #18028953892: usb: xhci-dwc3: Fix USB3.1 controller register access in reset state

[Marek Vasut]

On 6/22/23 16:08, Lim, Jit Loon wrote:
>> -----Original Message-----
>> From: Marek Vasut <marex at denx.de>
>> Sent: Thursday, 22 June, 2023 5:35 PM
>> To: Lim, Jit Loon <jit.loon.lim at intel.com>; u-boot at lists.denx.de
>> Cc: Jagan Teki <jagan at amarulasolutions.com>; Simon
>> <simon.k.r.goldschmidt at gmail.com>; Chee, Tien Fong
>> <tien.fong.chee at intel.com>; Hea, Kok Kiang <kok.kiang.hea at intel.com>;
>> Lokanathan, Raaj <raaj.lokanathan at intel.com>; Maniyam, Dinesh
>> <dinesh.maniyam at intel.com>; Ng, Boon Khai <boon.khai.ng at intel.com>;
>> Yuslaimi, Alif Zakuan <alif.zakuan.yuslaimi at intel.com>; Chong, Teik Heng
>> <teik.heng.chong at intel.com>; Zamri, Muhammad Hazim Izzat
>> <muhammad.hazim.izzat.zamri at intel.com>; Tang, Sieu Mun
>> <sieu.mun.tang at intel.com>; Bin Meng <bmeng.cn at gmail.com>; Michal Simek
>> <monstr at monstr.eu>; Tom Rini <trini at konsulko.com>; Eugen Hristev
>> <eugen.hristev at microchip.com>
>> Subject: Re: [PATCH v1] HSD #18028953892: usb: xhci-dwc3: Fix USB3.1
>> controller register access in reset state
>>
>> On 6/22/23 04:48, Lim, Jit Loon wrote:
>>>> -----Original Message-----
>>>> From: Marek Vasut <marex at denx.de>
>>>> Sent: Wednesday, 21 June, 2023 10:19 PM
>>>> To: Lim, Jit Loon <jit.loon.lim at intel.com>; u-boot at lists.denx.de
>>>> Cc: Jagan Teki <jagan at amarulasolutions.com>; Simon
>>>> <simon.k.r.goldschmidt at gmail.com>; Chee, Tien Fong
>>>> <tien.fong.chee at intel.com>; Hea, Kok Kiang <kok.kiang.hea at intel.com>;
>>>> Lokanathan, Raaj <raaj.lokanathan at intel.com>; Maniyam, Dinesh
>>>> <dinesh.maniyam at intel.com>; Ng, Boon Khai <boon.khai.ng at intel.com>;
>>>> Yuslaimi, Alif Zakuan <alif.zakuan.yuslaimi at intel.com>; Chong, Teik
>>>> Heng <teik.heng.chong at intel.com>; Zamri, Muhammad Hazim Izzat
>>>> <muhammad.hazim.izzat.zamri at intel.com>; Tang, Sieu Mun
>>>> <sieu.mun.tang at intel.com>; Bin Meng <bmeng.cn at gmail.com>
>>>> Subject: Re: [PATCH v1] HSD #18028953892: usb: xhci-dwc3: Fix USB3.1
>>>> controller register access in reset state
>>>>
>>>> On 6/21/23 16:11, Jit Loon Lim wrote:
>>>>> From: Teik Heng Chong <teik.heng.chong at intel.com>
>>>>>
>>>>> The controller registers should not be accessed while the
>>>>> controller's vcc_reset_n is asserted.
>>>>>
>>>>> Signed-off-by: Teik Heng Chong <teik.heng.chong at intel.com>
>>>>
>>>> Is this patch ported from Linux or is this custom development ?
>>>>
>>>> Is there a matching patch/fix in Linux already ?
>>>
>>> In xhci_dwc3_probe(), the program sequence is vcc reset -> clk init ->
>>> phy setup -> xhci_register therefore, when xhci_dwc3_remove is called,
>>> the proper usb stop sequence should be xhci_register _> phy setup ->
>>> clk init -> vcc reset
>>>
>>> if we look at linux driver
>>> https://elixir.bootlin.com/linux/latest/source/drivers/usb/dwc3/dwc3-o
>>> f-simple.c#L33
>>>
>>> dwc3_of_simple_probe: The sequence is reset -> clock
>>> __dwc3_of_simple_teardown: Then, clock -> reset
>>>
>>> So based on the above, we have made changes and the uboot fix is now
>> aligned with linux driver.
>>
>> Instead of adding random patches to the U-Boot dwc3 driver, please just
>> synchronize the driver with Linux. You should be able to add the missing
>> patches to the DWC3 driver from Linux since the last synchronization point, the
>> process should be largely mechanical. Make sure to include commit ID of each
>> Linux commit in the new matching U-Boot patch.
>>
>> It shouldn't be difficult, one approach I can think of is roughly this:
>> - figure out the original merge base from which the DWC3 driver was imported
>> to U-Boot
>> - in U-Boot, revert all dwc3 patches on top of that import patch
>> - pick all Linux kernel dwc3 patches from that merge base and apply on top of
>> this U-Boot with reverts
>> - Run rebase and drop the reverts, let git drop duplicate patches
> 
> We believed the previous reply from us is a bit confusing.
> There is no exact same function/file for U-Boot to use in Linux.
> What we are doing is, we are referring to Linux sequence.
> 
> Linux:  (dwc3_of_simple_probe)
> https://elixir.bootlin.com/linux/latest/source/drivers/usb/dwc3/dwc3-of-simple.c#L33
> U-Boot: (xhci_dwc3_probe)
> https://elixir.bootlin.com/u-boot/latest/source/drivers/usb/host/xhci-dwc3.c#L159
> 
> Linux:  (__dwc3_of_simple_teardown)
> https://elixir.bootlin.com/linux/latest/source/drivers/usb/dwc3/dwc3-of-simple.c#L98
> U-Boot: (xhci_dwc3_remove)
> https://elixir.bootlin.com/u-boot/latest/source/drivers/usb/host/xhci-dwc3.c#L227
> 
> So we believed that we can't directly pickup all Linux kernel dwc3 patches and merge to U-Boot.

If you were to sync the driver from Linux to U-Boot, then the same 
sequence as Linux uses would be automatically used too, right ?

Sorry for the abysmal delay in my reply.