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

[Shantur Rathore]

Hi Andre,

On Thu, Feb 1, 2024 at 4:46 PM Andre Przywara <andre.przywara at arm.com> wrote:
>
> On Thu, 1 Feb 2024 09:39:54 +0000
> Shantur Rathore <i at shantur.com> wrote:
>
> Hi Shantur,
>
> > On Tue, Jan 30, 2024 at 2:01 PM Andre Przywara <andre.przywara at arm.com> wrote:
> > >
> > > On Sat,  9 Dec 2023 18:10:56 +0000
> > > Shantur Rathore <i at shantur.com> wrote:
> > >
> > > Hi,
> > >
> > > > 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.
> > >
> > > so this broke USB operation on some Allwinner boards. The ports still
> > > enumerate correctly, but no devices are detected. With mainline
> > > (containing this patch), and a USB stick connected:
> > > starting USB...
> > > Bus usb at 5200000: USB EHCI 1.00
> > > Bus usb at 5200400: USB OHCI 1.0
> > > scanning bus usb at 5200000 for devices... 1 USB Device(s) found
> > > scanning bus usb at 5200400 for devices... 1 USB Device(s) found
> > >        scanning usb for storage devices... 0 Storage Device(s) found
> > >
> > > With this patch here reverted:
> > > starting USB...
> > > Bus usb at 5200000: USB EHCI 1.00
> > > Bus usb at 5200400: USB OHCI 1.0
> > > scanning bus usb at 5200000 for devices... 2 USB Device(s) found
> > > scanning bus usb at 5200400 for devices... 1 USB Device(s) found
> > >        scanning usb for storage devices... 1 Storage Device(s) found
> > >
> > > I have no clue why this happens, there is no discrete hub anywhere in this
> > > setup, so I guess the code runs for the root hub here?
> > > I am attaching a log with DEBUG enabled for common/usb_hub.c, for both
> > > cases.
> > >
> > > Any clues, debug suggestions, or even a fix are warmly welcome.
> > >
> >
> > Which USB disk are you using? Is it a USB 2.0 disk ?
> > Can you try with any other USB disk to confirm?
>
> Yes, this was some USB 2.0 memory stick, on an USB 2.0 port.
> Most sunxi boards only have USB 2.0, but there is one SoC with USB 3.0, I
> will try some combinations later tonight.
>
> Thanks,
> Andre

That would be awesome.
We need to test

USB 3.0 and 2.0 ports with USB 3.0 and 2.0 drives.

Once you have tested it, can you please try the patch below

        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);
+               if (usb_hub_is_superspeed(hub)) {
+                       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("PowerOn : port %d returns %lX\n", i + 1, dev->status);
        }


Thanks