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

[Marek Vasut]

On 11/23/23 12:24, Shantur Rathore wrote:
> Hi Marek,
> 
> On Thu, Nov 23, 2023 at 12:06 AM Marek Vasut <marex at denx.de> wrote:
>>
>> On 11/20/23 00:03, Shantur Rathore wrote:
>>> Hi Marek,
>>
>> Hi,
>>
>>> On Sun, Nov 19, 2023 at 8:53 PM Marek Vasut <marex at denx.de> wrote:
>>>>
>>>> On 11/10/23 15:13, Shantur Rathore wrote:
>>>>> Currently when a hub is turned on, all the ports are powered on.
>>>>> This works well for hubs which have individual power control.
>>>>>
>>>>> For the hubs without individual power control this has no effect.
>>>>
>>>> OK
>>>>
>>>>> Mostly in these scenarios the hub port is powered before the USB
>>>>> controller is enabled, this can lead to some devices in unexpected
>>>>> state.
>>>>
>>>> This ^ part needs clarification.
>>>>
>>>> Which devices are in incorrect state, the ones connected to the hub
>>>> downstream facing ports ?
>>>>
>>>
>>> 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 ?

> 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.