[FIX PATCH v1] Fix: common: usb_hub: Reset only USB3.0 hub

[Marek Vasut]

On 2/8/24 12:30, Shantur Rathore wrote:
> Hi Marek,
> 
> On Wed, Feb 7, 2024 at 1:07 PM Marek Vasut <marex at 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.przywara at arm.com>
>>>
>>> Signed-off-by: Shantur Rathore <i at 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.

Thanks