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

[Shantur Rathore]

Hi Marek,

On Sun, Dec 3, 2023 at 8:42 PM Marek Vasut <marex at denx.de> wrote:
>
> 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 ?

Actually, I am wrong. I wasn't sure if there is refcounting for the
regulator subsystem
in linux. just found it has so this is null and void.

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

Sure, agreed 100%.
Do we need to fix both at the same time?
Both patches seem to be independent.

Kind regards,
Shantur