On Thu, Feb 8, 2024 at 1:44 PM Dragan Simic <dsi...@manjaro.org> wrote:
>
> On 2024-02-08 14:33, Marek Vasut wrote:
> > On 2/8/24 12:30, Shantur Rathore wrote:
> >> On Wed, Feb 7, 2024 at 1:07 PM Marek Vasut <ma...@denx.de> wrote:
> >>> On 2/7/24 11:23, Shantur Rathore wrote:
> >>>> USB 3.0 spec requires hub to reset device while
> >>>> enumeration. Some USB 2.0 hubs / devices don't
> >>>> handle this well and after implementation of
> >>>> reset some USB 2.0 disks weren't detected on
> >>>> Allwinner based boards.
> >>>>
> >>>> Resetting only when hub is USB 3.0 fixes it.
> >>>
> >>> It would be good to include as many details about the faulty hardware
> >>> in
> >>> the commit message as possible, so that when someone else runs into
> >>> this, they would have all that information available.
> >>>
> >>>> Tested-by: Andre Przywara <andre.przyw...@arm.com>
> >>>>
> >>>> Signed-off-by: Shantur Rathore <i...@shantur.com>
> >>>> ---
> >>>>
> >>>> common/usb_hub.c | 6 ++++--
> >>>> 1 file changed, 4 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/common/usb_hub.c b/common/usb_hub.c
> >>>> index 3fb7e14d10..2e054eb935 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);
> >>>> + if (usb_hub_is_superspeed(dev)) {
> >>>
> >>> Should this condition be "all which are lower than superspeed"
> >>> instead ,
> >>> so when the next generation of USB comes, this problem won't trigger
> >>> ?
> >>>
> >>> What does Linux do btw ?
> >>
> >> As of now Linux checks if the hub is superspeed
> >> https://github.com/torvalds/linux/blob/master/drivers/usb/core/hub.c#L2859
> >>
> >> which is
> >> return hdev->descriptor.bDeviceProtocol == USB_HUB_PR_SS; //
> >> USB_HUB_PR_SS = 3
> >>
> >> This holds true for newer SuperSpeedPlus hubs as well.
> >> https://github.com/torvalds/linux/blob/master/drivers/usb/core/hub.h#L155
> >>
> >> We can change the check to be bDeviceProtocol > 2 but who knows if
> >> things change in the newer version of spec.
> >> I am open to suggestions.
> >
> > Please just include the ^ in the commit description. Use link to
> > git.kernel.org , not some mirror . This is extremely useful
> > information and, well, you already wrote the V2 commit message
> > addition in this answer.
>
> Shantur, if that would be easier or quicker for you, I can write
> a quite detailed patch description for you, in exchange for a
> "Helped-by" tag in the v2 patch submission. :)
That would be really kind of you Dragan.
I am down with the flu so that would really help me as my brain is
working at 15% capacity.
