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

[Shantur Rathore]

Hi Marek,

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.

> > With this patch, we explicitly reset the port while powering up hub
> > This resets the port for hubs without port power control and has
> > no effect on hubs with port power control as the port is still off.
>
> Should common/usb_hub.c usb_hub_port_connect_change() trigger the reset?
>
> Could it be you do not get a connect change event ?

Yes, that's correct there is no connect change event while scanning the port.

>
> > Before this patch AMicro AM8180 based NVME to USB adapter won't be
> > detected as a USB3.0 Mass Storage device but with this it works as
> > expected.
>
> Do you have HUB power control ?
>
> I recall there was some companion hub discussion recently.

The onboard ports don't have separate power control, only the
regulator as explained above.

Kind regards,
Shantur