Hi Elric,
Thanks for sending this patch. It has a couple issues, and I hope
you'll find the time to fix them up. :)
First, I would suggest you run your patch through scripts/checkpatch.pl.
You can do that with a command like this:
git diff HEAD^ | perl scripts/checkpatch.pl --no-signoff -
That script will make sure you are matching the kernel coding style.
The script is mostly complaining that you used spaces instead of tabs to
indent your code. You probably need to make sure that your editor is
using hard tab stops, instead of inserting spaces when you press tab.
There's also some trailing whitespace at the end of lines. I used to do
that all the time, until I discovered the magic .vimrc lines to
highlight trailing whitespace. Unfortunately, those lines don't work on
c files for newer vim versions anymore (but it works for .txt files, go
figure). In any case, seeing trailing white spaces in bright red seemed
to have trained me out of leaving them in the code, so maybe you can
figure out how to get your editor to highlight trailing white spaces. :)
Please fix any errors or warnings from checkpatch.pl and resubmit your patch.
Also, can you pick a shorter short description for your patch?
Descriptions shorter than 50 characters usually display well with our
tools, and don't cause mail client subject line wrap.
How about instead of
"fix bug of device attached to superspeed hub can't be re-enumerated after resume"
say
"USB: Set USB3 hub depth after device reset"?
We like to have the subsystem for the patch, although I sometimes have
to add it later.
Kudos for sending a patch that actually applies correctly, following the
SubmittingPatches documentation, and figuring out which stable kernel
this should apply to. :) You just need to fix up a few things and
resubmit.
Sarah Sharp
On Fri, Feb 17, 2012 at 04:03:16PM +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.
>
> Signed-off-by: Elric Fu <elricfu1@xxxxxxxxx>
> Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> ---
> drivers/usb/core/hub.c | 30 +++++++++++++++++-------------
> 1 file changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index a0613d8..5b3e918 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.1.231.g0364
>
--
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
