Hi Sarah and Mathias,
As the discussion in http://comments.gmane.org/gmane.linux.usb.general/107011,
I found that [AMD] FCH USB XHCI Controller [1022:7814] the USB 3.0 disk
can't work in SuperSpeed after several times of hotplug. After doing some
experiments and bisection, I found the bug is caused by
41e7e056cdc662f704fa9262e5c6e213b4ab45dd (USB: Allow USB 3.0 ports to be
disabled.). And the bug can be fixed by not executing the
hub_usb3_port_disable() function. I also found that the port status is
already in RxDetect before setting the port to Disabled in
hub_usb3_port_disable() function. So, there are 2 ways to fix the bug:
1) Check if the Vendor/Device id is [1022:7814] at the beginning of
hub_usb3_port_disable() function. If yes, return without executing the
remaining code.
2) Check if the port status is already in RxDetect, if yes, return without
executing the remaining code.
The second method seems more reasonable, so the patch is the implementation
of the second one. But it will affect more platforms and I don't know if
there'll be any negative result. Otherwise, if the first one is correct,
I can reimplement a new one.
I'm appreciated if you can give me some advice, or if there is any thing I missed.
Thanks,
Gavin
Gavin Guo (1):
usb: Check if port status is equal to RxDetect
drivers/usb/core/hub.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
--
2.0.0
When using USB 3.0 pen drive with the [AMD] FCH USB XHCI Controller
[1022:7814], the second hotplugging will experience the USB 3.0 pen
drive is recognized as high-speed device. After bisecting the kernel,
I found the commit number 41e7e056cdc662f704fa9262e5c6e213b4ab45dd
(USB: Allow USB 3.0 ports to be disabled.) causes the bug. After doing
some experiments, the bug can be fixed by avoiding executing the function
hub_usb3_port_disable(). Because the port status with [AMD] FCH USB
XHCI Controlleris [1022:7814] is already in RxDetect
(I tried printing out the port status before setting to Disabled state),
it's reasonable to check the port status before really executing
hub_usb3_port_disable().
Fixes: 41e7e056cdc6 (USB: Allow USB 3.0 ports to be disabled.)
Signed-off-by: Gavin Guo <[email protected]>
---
drivers/usb/core/hub.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 21b99b4..e02ab62 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -889,6 +889,25 @@ static int hub_usb3_port_disable(struct usb_hub *hub, int port1)
if (!hub_is_superspeed(hub->hdev))
return -EINVAL;
+ ret = hub_port_status(hub, port1, &portstatus, &portchange);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * USB controller Advanced Micro Devices,
+ * Inc. [AMD] FCH USB XHCI Controller [1022:7814] will have spurious result
+ * making the following usb 3.0 device hotplugging route to the 2.0 root hub
+ * and recognized as high-speed device if we set the usb 3.0 port link state
+ * to Disabled. Since it's already in USB_SS_PORT_LS_RX_DETECT state, we
+ * check the state here to avoid the bug.
+ */
+ if ((portstatus & USB_PORT_STAT_LINK_STATE) ==
+ USB_SS_PORT_LS_RX_DETECT) {
+ dev_dbg(&hub->ports[port1 - 1]->dev,
+ "The link state is already in USB_SS_PORT_LS_RX_DETECT\n");
+ return ret;
+ }
+
ret = hub_set_port_link_state(hub, port1, USB_SS_PORT_LS_SS_DISABLED);
if (ret)
return ret;
--
2.0.0
Hi all,
On Fri, Jul 11, 2014 at 2:22 PM, Gavin Guo <[email protected]> wrote:
> Hi Sarah and Mathias,
>
> As the discussion in http://comments.gmane.org/gmane.linux.usb.general/107011,
> I found that [AMD] FCH USB XHCI Controller [1022:7814] the USB 3.0 disk
> can't work in SuperSpeed after several times of hotplug. After doing some
> experiments and bisection, I found the bug is caused by
> 41e7e056cdc662f704fa9262e5c6e213b4ab45dd (USB: Allow USB 3.0 ports to be
> disabled.). And the bug can be fixed by not executing the
> hub_usb3_port_disable() function. I also found that the port status is
> already in RxDetect before setting the port to Disabled in
> hub_usb3_port_disable() function. So, there are 2 ways to fix the bug:
>
> 1) Check if the Vendor/Device id is [1022:7814] at the beginning of
> hub_usb3_port_disable() function. If yes, return without executing the
> remaining code.
>
> 2) Check if the port status is already in RxDetect, if yes, return without
> executing the remaining code.
>
> The second method seems more reasonable, so the patch is the implementation
> of the second one. But it will affect more platforms and I don't know if
> there'll be any negative result. Otherwise, if the first one is correct,
> I can reimplement a new one.
>
> I'm appreciated if you can give me some advice, or if there is any thing I missed.
>
> Thanks,
> Gavin
>
> Gavin Guo (1):
> usb: Check if port status is equal to RxDetect
>
> drivers/usb/core/hub.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> --
> 2.0.0
>
Add Greg k-h to the list.
Hi all,
On Fri, Jul 11, 2014 at 2:22 PM, Gavin Guo <[email protected]> wrote:
> When using USB 3.0 pen drive with the [AMD] FCH USB XHCI Controller
> [1022:7814], the second hotplugging will experience the USB 3.0 pen
> drive is recognized as high-speed device. After bisecting the kernel,
> I found the commit number 41e7e056cdc662f704fa9262e5c6e213b4ab45dd
> (USB: Allow USB 3.0 ports to be disabled.) causes the bug. After doing
> some experiments, the bug can be fixed by avoiding executing the function
> hub_usb3_port_disable(). Because the port status with [AMD] FCH USB
> XHCI Controlleris [1022:7814] is already in RxDetect
> (I tried printing out the port status before setting to Disabled state),
> it's reasonable to check the port status before really executing
> hub_usb3_port_disable().
>
> Fixes: 41e7e056cdc6 (USB: Allow USB 3.0 ports to be disabled.)
> Signed-off-by: Gavin Guo <[email protected]>
> ---
> drivers/usb/core/hub.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 21b99b4..e02ab62 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -889,6 +889,25 @@ static int hub_usb3_port_disable(struct usb_hub *hub, int port1)
> if (!hub_is_superspeed(hub->hdev))
> return -EINVAL;
>
> + ret = hub_port_status(hub, port1, &portstatus, &portchange);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * USB controller Advanced Micro Devices,
> + * Inc. [AMD] FCH USB XHCI Controller [1022:7814] will have spurious result
> + * making the following usb 3.0 device hotplugging route to the 2.0 root hub
> + * and recognized as high-speed device if we set the usb 3.0 port link state
> + * to Disabled. Since it's already in USB_SS_PORT_LS_RX_DETECT state, we
> + * check the state here to avoid the bug.
> + */
> + if ((portstatus & USB_PORT_STAT_LINK_STATE) ==
> + USB_SS_PORT_LS_RX_DETECT) {
> + dev_dbg(&hub->ports[port1 - 1]->dev,
> + "The link state is already in USB_SS_PORT_LS_RX_DETECT\n");
> + return ret;
> + }
> +
> ret = hub_set_port_link_state(hub, port1, USB_SS_PORT_LS_SS_DISABLED);
> if (ret)
> return ret;
> --
> 2.0.0
>
Add Greg k-h to the list.
On Sat, Jul 12, 2014 at 07:32:34AM +0800, Gavin Guo wrote:
> Hi all,
>
> On Fri, Jul 11, 2014 at 2:22 PM, Gavin Guo <[email protected]> wrote:
> > Hi Sarah and Mathias,
> >
> > As the discussion in http://comments.gmane.org/gmane.linux.usb.general/107011,
> > I found that [AMD] FCH USB XHCI Controller [1022:7814] the USB 3.0 disk
> > can't work in SuperSpeed after several times of hotplug. After doing some
> > experiments and bisection, I found the bug is caused by
> > 41e7e056cdc662f704fa9262e5c6e213b4ab45dd (USB: Allow USB 3.0 ports to be
> > disabled.). And the bug can be fixed by not executing the
> > hub_usb3_port_disable() function. I also found that the port status is
> > already in RxDetect before setting the port to Disabled in
> > hub_usb3_port_disable() function. So, there are 2 ways to fix the bug:
> >
> > 1) Check if the Vendor/Device id is [1022:7814] at the beginning of
> > hub_usb3_port_disable() function. If yes, return without executing the
> > remaining code.
> >
> > 2) Check if the port status is already in RxDetect, if yes, return without
> > executing the remaining code.
> >
> > The second method seems more reasonable, so the patch is the implementation
> > of the second one. But it will affect more platforms and I don't know if
> > there'll be any negative result. Otherwise, if the first one is correct,
> > I can reimplement a new one.
> >
> > I'm appreciated if you can give me some advice, or if there is any thing I missed.
> >
> > Thanks,
> > Gavin
> >
> > Gavin Guo (1):
> > usb: Check if port status is equal to RxDetect
> >
> > drivers/usb/core/hub.c | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > --
> > 2.0.0
> >
>
> Add Greg k-h to the list.
Why? Mathias can handle this just fine, right?
And I read all linux-usb@vger email anyway...
greg k-h
Hi Greg,
On Sat, Jul 12, 2014 at 8:04 AM, Greg KH <[email protected]> wrote:
> On Sat, Jul 12, 2014 at 07:32:34AM +0800, Gavin Guo wrote:
>> Hi all,
>>
>> On Fri, Jul 11, 2014 at 2:22 PM, Gavin Guo <[email protected]> wrote:
>> > Hi Sarah and Mathias,
>> >
>> > As the discussion in http://comments.gmane.org/gmane.linux.usb.general/107011,
>> > I found that [AMD] FCH USB XHCI Controller [1022:7814] the USB 3.0 disk
>> > can't work in SuperSpeed after several times of hotplug. After doing some
>> > experiments and bisection, I found the bug is caused by
>> > 41e7e056cdc662f704fa9262e5c6e213b4ab45dd (USB: Allow USB 3.0 ports to be
>> > disabled.). And the bug can be fixed by not executing the
>> > hub_usb3_port_disable() function. I also found that the port status is
>> > already in RxDetect before setting the port to Disabled in
>> > hub_usb3_port_disable() function. So, there are 2 ways to fix the bug:
>> >
>> > 1) Check if the Vendor/Device id is [1022:7814] at the beginning of
>> > hub_usb3_port_disable() function. If yes, return without executing the
>> > remaining code.
>> >
>> > 2) Check if the port status is already in RxDetect, if yes, return without
>> > executing the remaining code.
>> >
>> > The second method seems more reasonable, so the patch is the implementation
>> > of the second one. But it will affect more platforms and I don't know if
>> > there'll be any negative result. Otherwise, if the first one is correct,
>> > I can reimplement a new one.
>> >
>> > I'm appreciated if you can give me some advice, or if there is any thing I missed.
>> >
>> > Thanks,
>> > Gavin
>> >
>> > Gavin Guo (1):
>> > usb: Check if port status is equal to RxDetect
>> >
>> > drivers/usb/core/hub.c | 19 +++++++++++++++++++
>> > 1 file changed, 19 insertions(+)
>> >
>> > --
>> > 2.0.0
>> >
>>
>> Add Greg k-h to the list.
>
> Why? Mathias can handle this just fine, right?
>
> And I read all linux-usb@vger email anyway...
>
> greg k-h
I'm sorry if there is any misunderstanding. Because the code is under
drivers/usb/core/hub.c, I found that in MAINTAINERS you are the
maintainer of drivers/usb/*. so I think it's better to let you know
the change.
Thanks,
Gavin Guo
On Fri, 11 Jul 2014, Gavin Guo wrote:
> When using USB 3.0 pen drive with the [AMD] FCH USB XHCI Controller
> [1022:7814], the second hotplugging will experience the USB 3.0 pen
> drive is recognized as high-speed device. After bisecting the kernel,
> I found the commit number 41e7e056cdc662f704fa9262e5c6e213b4ab45dd
> (USB: Allow USB 3.0 ports to be disabled.) causes the bug. After doing
> some experiments, the bug can be fixed by avoiding executing the function
> hub_usb3_port_disable(). Because the port status with [AMD] FCH USB
> XHCI Controlleris [1022:7814] is already in RxDetect
> (I tried printing out the port status before setting to Disabled state),
> it's reasonable to check the port status before really executing
> hub_usb3_port_disable().
This seems like a race. Even if the port isn't in RxDetect when you
check it, it could switch to RxDetect before the kernel sets it to
Disabled.
But maybe this is the best we can do.
> Fixes: 41e7e056cdc6 (USB: Allow USB 3.0 ports to be disabled.)
> Signed-off-by: Gavin Guo <[email protected]>
> ---
> drivers/usb/core/hub.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 21b99b4..e02ab62 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -889,6 +889,25 @@ static int hub_usb3_port_disable(struct usb_hub *hub, int port1)
> if (!hub_is_superspeed(hub->hdev))
> return -EINVAL;
>
> + ret = hub_port_status(hub, port1, &portstatus, &portchange);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * USB controller Advanced Micro Devices,
> + * Inc. [AMD] FCH USB XHCI Controller [1022:7814] will have spurious result
> + * making the following usb 3.0 device hotplugging route to the 2.0 root hub
> + * and recognized as high-speed device if we set the usb 3.0 port link state
> + * to Disabled. Since it's already in USB_SS_PORT_LS_RX_DETECT state, we
> + * check the state here to avoid the bug.
> + */
Comments should not extend beyond column 80. And there should be only
one space, not two, after the '*' in the 6th and 7th lines.
> + if ((portstatus & USB_PORT_STAT_LINK_STATE) ==
> + USB_SS_PORT_LS_RX_DETECT) {
> + dev_dbg(&hub->ports[port1 - 1]->dev,
> + "The link state is already in USB_SS_PORT_LS_RX_DETECT\n");
This message does not explain what has happened. It should say
something like "Not disabling port; link state is RxDetect".
> + return ret;
> + }
> +
> ret = hub_set_port_link_state(hub, port1, USB_SS_PORT_LS_SS_DISABLED);
> if (ret)
> return ret;
Alan Stern