2012/2/16 Elric Fu <elricfu1@xxxxxxxxx>:
> 2012/2/16 Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>:
>> On Wed, 15 Feb 2012, Sarah Sharp wrote:
>>
>>> On Wed, Feb 15, 2012 at 10:48:05AM -0500, Alan Stern wrote:
>>> > On Wed, 15 Feb 2012, Elric Fu wrote:
>>> >
>>> > > Hi all,
>>> > >
>>> > > I found a usb 3.0 hub issue. Now I use a NEC xHCI host card and a VIA VL810 3.0
>>> > > hub to test suspend/resume function. I found the device attached to a usb 3.0
>>> > > hub port didn't respond the address device command in the procedure of
>>> > > re-enumeration after resume. I tested linux kernel from 2.6.39 to 3.1.0+. All
>>> > > of them have the issue.
>>> >
>>> > > Then I found the root cause by USB CATC. USB core sends a set hub
>>> > > depth request to
>>> > > usb 3.0 hub in hub_configure() in superspeed hub initialization. And
>>> > > If we do a s3/s4,
>>> > > after resume usb core should reset the device and send the set hub
>>> > > depth request
>>> > > again. But before re-enumeration the usb core doesn't do it. So I
>>> > > added a set hub depth
>>> > > request after setting configuration in usb_reset_and_verify_device().
>>> > > Now it seems like
>>> > > the issue is gone.
>>> >
>>> > Assuming this is the right thing to do, the place to do it is in
>>> > hub_activate(), not usb_reset_and_verify_device().
>>>
>>> It might be the right thing to do for this particular hub, but it means
>>> the hub is very broken. Any USB device is supposed to respond to the
>>> Set Address command once it's link trained. It shouldn't matter when we
>>> send the set hub depth request, as long as we do so before we start
>>> using it as a hub.
>
> The hub I use is VIA USB 3.0 hub. As far as I know, VIA USB 3.0 hub is
> the first product of USB 3.0 hub and used as a sample to test by USB-IF.
> I don't have other USB 3.0 hubs (such as TI), so I don't know the others'
> behavior like this or not. I will buy a TI or Genesys USB 3.0 hub for
> testing later.
>
> According to USB 3.0 spec, After completion of a Hot Reset the hub
> returns to the default state except port configuration information is
> maintained for the upstream port. And the hub ignores the route string
> and assumes all packets are routes directly to the hub until it enters
> the configured state.
>
> So I think after completion of a Reset or Warm Reset we should send
> the Set Hub Depth request to superspeed hub again.
>
>>
>> You're missing the point. We send the Set-Depth request in
>> hub_configure(), which runs only once when the hub is first bound to
>> the hub driver. We do not send it when the hub is reset, in the
>> HUB_POST_RESET or HUB_RESET_RESUME cases of hub_activate().
>>
>> Therefore the correct solution is to move the code that sends the
>> Set-Depth request into hub_activate(), so that it will run on either an
>> init or reset path.
>
> I will try and test it in the lastest version of linux kernel.
I tried 3.3.0-rc3 and the issue occurred too. I modified the patch for merging
initialization and reset paths according to what Alan said. It can fix
the issue.
Signed-off-by: Elric Fu <elricfu1@xxxxxxxxx>
---
drivers/usb/core/hub.c | 22 ++++++++++------------
1 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index a0613d8..7654ffb 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -709,6 +709,16 @@ static void hub_activate(struct usb_hub *hub,
enum hub_activation_type type)
* For any other type of activation, turn it on.
*/
if (type != HUB_RESUME) {
+ if (hdev->parent && hub_is_superspeed(hdev) &&
+ hdev->state == USB_STATE_CONFIGURED) {
+ 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 +997,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.
--
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
