[PATCH] common: usb-hub: Reset hub port before scanning

[Marek Vasut]

On 11/24/23 01:37, Shantur Rathore wrote:
> Hi Marek,

Hi,

sorry for the late reply.

>>>>> In my case RockPro64, the power to usb ports onboard is controlled by
>>>>> a regulator.
>>>>> This regulator is enabled as part of init  as here
>>>>> https://github.com/u-boot/u-boot/blob/master/arch/arm/dts/rk3399-rockpro64.dtsi#L177
>>>>>
>>>>> On init, the usb devices connected to the port are powered up, in my
>>>>> case AM8180 (a RTL9210 based ) NVMe to USB enclosure with UAS.
>>>>> But the usb controller is only enabled at 'usb start' and by this time
>>>>> the device is in some state that it doesn't get detected.
>>>>>
>>>>> One way to make sure the devices connected to the ports are in an
>>>>> initialising state is by issuing a port reset before scanning the
>>>>> port.
>>>>
>>>> Why not remove this regulator-always-on and let the PHY driver turn the
>>>> VBUS ON only when it is needed ?
>>>>
>>>> Wouldn't that solve the problem too AND remove unnecessarily enabled
>>>> regulator ?
>>>>
>>>> [...]
>>>
>>> Removing the regulator-always-on does make it work but there are few issues
>>>
>>> 1. regulator is used to control power to multiple ports ( USB3.0 and
>>> USB2.0 in RockPro64 ),
>>> so this could lead to all ports turning off if a phy resets / turns off power
>>
>> I vaguely recall seeing some refcounting patch for regulator subsystem,
>> would that help ?
> 
> I don't think this would be a problem for u-boot, but linux maybe.

How so ?

>>> 2. Even with regulator-always-on and without this reset port patch,
>>> linux was always
>>> able to detect the drive on the port, so there is something missing in u-boot.
>>> 3. Looking at usb hub code in Linux, I found that for USB3.0 it tries
>>> to reset the port before
>>> scanning. The comment says
>>>
>>> /* Is a USB 3.0 port in the Inactive or Compliance Mode state?
>>>    * Port warm reset is required to recover
>>>    */
>>>
>>> i. https://github.com/torvalds/linux/blob/master/drivers/usb/core/hub.c#L5736
>>> ii. https://github.com/torvalds/linux/blob/master/drivers/usb/core/hub.c#L2836
>>>
>>> I believe this isn't implemented in u-boot and hence explicit reset of
>>> a port fixes the issue.
>>
>> I feel this is separate issue and what needs to be fixed first is the
>> hard-wired VBus control.
> 
> I beg to differ on this, couple of reasons for this
> 
> 1. The same drive works fine without being reset on the USB2.0 ports - this
> confirms that the issue is related to USB3.0 only.

This is a separate issue from the hard-wired VBus control. I agree the 
issue does exist, I would simply like to avoid conflating the hard-wired 
VBus control with device reset.

> 2. Linux is able to detect the same drive on USB3.0 when u-boot fails - this
> confirms issue doesn't lie with regulator-always-on

See above

> 3. The links I pasted earlier mentions that in USB3.0 the ports need reset
> and this is done before the port is scanned. Adding the similar reset in u-boot
> fixes all with the same regulator-always-on. AFAIK u-boot doesn't handle
> this special USB3.0 case and maybe this is why I was finding a few posts
> around the drive not being detected in the USB3.0 port in u-boot but works in
> a USB2.0 port.

I am not disputing the need for USB 3.0 device reset, I believe there 
are two issues here -- one is the hard-wired VBus regulator -- the other 
is this USB 3.0 reset. They should both be fixed.