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

[Shantur Rathore]

On Tue, Jan 16, 2024 at 12:45 AM Marek Vasut <marex at denx.de> wrote:
>
> On 1/8/24 23:11, Shantur Rathore wrote:
> > Hi Marex,
> >
> > On Sat, Dec 9, 2023 at 6:12 PM Shantur Rathore <i at shantur.com> 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.
> >> Mostly in these scenarios the hub port is powered before the USB
> >> controller is enabled, this can lead to some devices in unexpected
> >> state.
> >>
> >> 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.
> >>
> >> 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.
> >>
> >> Tested working after this patch:
> >> 1. AMicro AM8180 based NVME to USB Adapter
> >> 2. Kingston DataTraveler 3.0
> >> 3. GenesysLogic USB3.0 Hub
> >>
> >> The drives were tested while connected directly and via the hub.
> >>
> >> Signed-off-by: Shantur Rathore <i at shantur.com>
> >>
> >> ---
> >>
> >> Changes in v3:
> >> - Split up patches as seperate series
> >>
> >> Changes in v2:
> >> - As requested, added fix for regulator-always-on in RockPro64
> >>
> >>   common/usb_hub.c | 4 +++-
> >>   1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/common/usb_hub.c b/common/usb_hub.c
> >> index 70279f301d..3fb7e14d10 100644
> >> --- a/common/usb_hub.c
> >> +++ b/common/usb_hub.c
> >> @@ -174,8 +174,10 @@ static void usb_hub_power_on(struct usb_hub_device *hub)
> >>
> >>          debug("enabling power on all ports\n");
> >>          for (i = 0; i < dev->maxchild; i++) {
> >> +               usb_set_port_feature(dev, i + 1, USB_PORT_FEAT_RESET);
> >> +               debug("Reset : port %d returns %lX\n", i + 1, dev->status);
> >>                  usb_set_port_feature(dev, i + 1, USB_PORT_FEAT_POWER);
> >> -               debug("port %d returns %lX\n", i + 1, dev->status);
> >> +               debug("PowerOn : port %d returns %lX\n", i + 1, dev->status);
> >>          }
> >>
> >>   #ifdef CONFIG_SANDBOX
> >> --
> >> 2.40.1
> >>
> >
> > Do you think it is the right time to get this in early so it can be
> > tested for a longer time before the next release ?
>
> Yes, I think so, sorry for the delay.
>
> I asked Tom to pick it directly and provided RB .

Thanks Marek,

Just realised Tom isn't included in this chain.

@Tom Rini - Can I request you to please merge this earlier in this release.

Kind regards,
Shantur