Re: [PATCH] USB: Set hub depth after USB3 hub reset

[Elric Fu <elricfu1@xxxxxxxxx>]

2012/2/22 Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>:
> On Sat, Feb 18, 2012 at 01:32:27PM +0800, Elric Fu wrote:
>> The superspeed device attached to a USB 3.0 hub(such as VIA's)
>> doesn't respond the address device command after resume. The
>> root cause is the superspeed hub will miss the Hub Depth value
>> that is used as an offset into the route string to locate the
>> bits it uses to determine the downstream port number after
>> reset, and all packets can't be routed to the device attached
>> to the superspeed hub.
>>
>> Hub driver sends a Set Hub Depth request to the superspeed hub
>> except for USB 3.0 root hub when the hub is initialized and
>> doesn't send the request again after reset due to the resume
>> process. So moving the code that sends the Set Hub Depth request
>> to the superspeed hub from hub_configure() to hub_activate()
>> is to cover those situations include initialization and reset.
>>
>> The patch shoule be backported to kernels as old as 2.6.39.
>
> Perfect, thanks!  I'll send this off to Greg.  The only thing you needed
> was to Cc the stable mailing list.

Thank you very much. I learn a lot of things from you. About Cc the
stable mail list, I always thought I shouldn't cc stable and if the patch
is done I should send the patch to maintainer then the maintainer will
submit it to stable. It seems like a mistake. Do you mean I should cc
the stable if the patch is done?

I have another question about submitting patch. If I find a bug and
submit a patch to give a solution, Shall I send a mail that the prefix
of subject is [RFC] then send a patch when the patch is done, or
send the mail that the subject is prefixed by [PATCH] and after
discussion send the final patch the subject is prefixed by
[PATCH vx]?

Best Regards,
Elric Fu

>
>> Signed-off-by: Elric Fu <elricfu1@xxxxxxxxx>
>> Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
>
> That way it gets sent to the stable mailing list when it makes it into
> Linus' tree, and Greg's mail bot automatically picks it up for the
> stable trees then.
>
> Congrats on your first kernel patch!
>
> Sarah Sharp
>
>> ---
>>  drivers/usb/core/hub.c |   30 +++++++++++++++++-------------
>>  1 files changed, 17 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
>> index a0613d8..dbb19fe 100644
>> --- a/drivers/usb/core/hub.c
>> +++ b/drivers/usb/core/hub.c
>> @@ -705,10 +705,26 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
>>       if (type == HUB_INIT3)
>>               goto init3;
>>
>> -     /* After a resume, port power should still be on.
>> +     /* The superspeed hub except for root hub has to use Hub Depth
>> +      * value as an offset into the route string to locate the bits
>> +      * it uses to determine the downstream port number. So hub driver
>> +      * should send a set hub depth request to superspeed hub after
>> +      * the superspeed hub is set configuration in initialization or
>> +      * reset procedure.
>> +      *
>> +      * After a resume, port power should still be on.
>>        * For any other type of activation, turn it on.
>>        */
>>       if (type != HUB_RESUME) {
>> +             if (hdev->parent && hub_is_superspeed(hdev)) {
>> +                     ret = usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
>> +                                     HUB_SET_DEPTH, USB_RT_HUB,
>> +                                     hdev->level - 1, 0, NULL, 0,
>> +                                     USB_CTRL_SET_TIMEOUT);
>> +                     if (ret < 0)
>> +                             dev_err(hub->intfdev,
>> +                                             "set hub depth failed\n");
>> +             }
>>
>>               /* Speed up system boot by using a delayed_work for the
>>                * hub's initial power-up delays.  This is pretty awkward
>> @@ -987,18 +1003,6 @@ static int hub_configure(struct usb_hub *hub,
>>               goto fail;
>>       }
>>
>> -     if (hub_is_superspeed(hdev) && (hdev->parent != NULL)) {
>> -             ret = usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
>> -                             HUB_SET_DEPTH, USB_RT_HUB,
>> -                             hdev->level - 1, 0, NULL, 0,
>> -                             USB_CTRL_SET_TIMEOUT);
>> -
>> -             if (ret < 0) {
>> -                     message = "can't set hub depth";
>> -                     goto fail;
>> -             }
>> -     }
>> -
>>       /* Request the entire hub descriptor.
>>        * hub->descriptor can handle USB_MAXCHILDREN ports,
>>        * but the hub can/will return fewer bytes here.
>> --
>> 1.7.9
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html