[PATCH] usb:solve resume usb device identification problem

[Pengcheng Li]

A usb device in the connection state. Then host is suspend and resume.
But the usb device could not be at the right speed. We should be reset
the reset.

Signed-off-by: Pengcheng Li <lpc...@hisilicon.com>
---
drivers/usb/core/hub.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index bee1351..cd71bb3 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3455,7 +3455,7 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
struct usb_port *port_dev = hub->ports[udev->portnum - 1];
int port1 = udev->portnum;
- int status;
+ int status, retval;
u16 portchange, portstatus;

if (!test_and_set_bit(port1, hub->child_usage_bits)) {
@@ -3512,6 +3512,10 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
}
}

+ retval = hub_port_reset(hub, port1, udev, HUB_ROOT_RESET_TIME, false);
+ if (retval < 0)
+ hub_port_disable(hub, port1, 0);
+
if (udev->persist_enabled)
status = wait_for_connected(udev, hub, &port1, &portchange,
&portstatus);
--
2.8.2

[Alan Stern]

On Mon, 11 Jul 2016, Pengcheng Li wrote:

> A usb device in the connection state. Then host is suspend and resume.
> But the usb device could not be at the right speed. We should be reset
> the reset.
>
> Signed-off-by: Pengcheng Li <lpc...@hisilicon.com>

Why wouldn't the USB device be at the right speed?

You should _not_ reset the device if it is at the right speed.

> @@ -3512,6 +3512,10 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
> }
> }
>
> + retval = hub_port_reset(hub, port1, udev, HUB_ROOT_RESET_TIME, false);
> + if (retval < 0)
> + hub_port_disable(hub, port1, 0);
> +

Most of the time (for example, for non-USB3 devices) this would be
wrong.

Alan Stern

[Lu Baolu]

Hi,

On 07/11/2016 08:57 PM, Pengcheng Li wrote:
> A usb device in the connection state. Then host is suspend and resume.
> But the usb device could not be at the right speed. We should be reset
> the reset.

Have you tried applying XHCI_RESET_ON_RESUME quirk to your
host controller driver? Is your usb device self powered?

Best regards,
Lu Baolu

[Lipengcheng]

Hi,

> -----Original Message-----
> From: Alan Stern [mailto:st...@rowland.harvard.edu]
> Sent: Monday, July 11, 2016 10:51 PM
> To: Lipengcheng
> Cc: gre...@linuxfoundation.org; baol...@linux.intel.com; chaseme...@gmail.com; mathia...@linux.intel.com;
> one...@suse.com; jun...@freescale.com; linu...@vger.kernel.org; linux-...@vger.kernel.org
> Subject: Re: [PATCH] usb:solve resume usb device identification problem
>
> On Mon, 11 Jul 2016, Pengcheng Li wrote:
>
> > A usb device in the connection state. Then host is suspend and resume.
> > But the usb device could not be at the right speed. We should be reset
> > the reset.
> >
> > Signed-off-by: Pengcheng Li <lpc...@hisilicon.com>
>
> Why wouldn't the USB device be at the right speed?
>

For example, some USB3 devices are resume, the device may be in USB 2.0 Device States. We should have USB 2.0 reset and
make the device into the right speed.

> You should _not_ reset the device if it is at the right speed.
>
> > @@ -3512,6 +3512,10 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
> > }
> > }
> >
> > + retval = hub_port_reset(hub, port1, udev, HUB_ROOT_RESET_TIME, false);
> > + if (retval < 0)
> > + hub_port_disable(hub, port1, 0);
> > +
>
> Most of the time (for example, for non-USB3 devices) this would be wrong.
>

Yes, USB3 devices have this problem. So far, USB2 device can not find this problem.
I mainly refer to the reset process of usb enumeration process.
> Alan Stern

Thanks,
Pengcheng Li

[Lipengcheng]

Hi,

> -----Original Message-----
> From: Lu Baolu [mailto:baol...@linux.intel.com]
> Sent: Tuesday, July 12, 2016 8:42 AM
> To: Lipengcheng; gre...@linuxfoundation.org; st...@rowland.harvard.edu; chaseme...@gmail.com; mathia...@linux.intel.com;
> one...@suse.com; jun...@freescale.com
> Cc: linu...@vger.kernel.org; linux-...@vger.kernel.org
> Subject: Re: [PATCH] usb:solve resume usb device identification problem
>

> Hi,
>
> On 07/11/2016 08:57 PM, Pengcheng Li wrote:
> > A usb device in the connection state. Then host is suspend and resume.
> > But the usb device could not be at the right speed. We should be reset
> > the reset.
>
> Have you tried applying XHCI_RESET_ON_RESUME quirk to your host controller driver? Is your usb device self powered?
>

I do not apply XHCI_RESET_ON_RESUME quir to my host controller driver. I select no pci platform. Our usb device is not self powered.

Best regards
Pengcheng Li

[Lu Baolu]

Hi,

On 07/12/2016 09:48 AM, Lipengcheng wrote:
> Hi,
>
>> -----Original Message-----
>> From: Lu Baolu [mailto:baol...@linux.intel.com]
>> Sent: Tuesday, July 12, 2016 8:42 AM
>> To: Lipengcheng; gre...@linuxfoundation.org; st...@rowland.harvard.edu; chaseme...@gmail.com; mathia...@linux.intel.com;
>> one...@suse.com; jun...@freescale.com
>> Cc: linu...@vger.kernel.org; linux-...@vger.kernel.org
>> Subject: Re: [PATCH] usb:solve resume usb device identification problem
>>
>> Hi,
>>
>> On 07/11/2016 08:57 PM, Pengcheng Li wrote:
>>> A usb device in the connection state. Then host is suspend and resume.
>>> But the usb device could not be at the right speed. We should be reset
>>> the reset.
>> Have you tried applying XHCI_RESET_ON_RESUME quirk to your host controller driver? Is your usb device self powered?
>>
> I do not apply XHCI_RESET_ON_RESUME quir to my host controller driver. I select no pci platform. Our usb device is not self powered.

This quirk is not pci specific.

>> Best regards,
>> Lu Baolu
>>
>>> Signed-off-by: Pengcheng Li <lpc...@hisilicon.com>
>>> ---
>>> drivers/usb/core/hub.c | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index
>>> bee1351..cd71bb3 100644
>>> --- a/drivers/usb/core/hub.c
>>> +++ b/drivers/usb/core/hub.c
>>> @@ -3455,7 +3455,7 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
>>> struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
>>> struct usb_port *port_dev = hub->ports[udev->portnum - 1];
>>> int port1 = udev->portnum;
>>> - int status;
>>> + int status, retval;
>>> u16 portchange, portstatus;
>>>
>>> if (!test_and_set_bit(port1, hub->child_usage_bits)) { @@ -3512,6
>>> +3512,10 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
>>> }
>>> }
>>>
>>> + retval = hub_port_reset(hub, port1, udev, HUB_ROOT_RESET_TIME, false);
>>> + if (retval < 0)
>>> + hub_port_disable(hub, port1, 0);
>>> +

If I understand it right, this is a "host + device" specific issue. This line of code
might solve your issue, but it impacts all other hosts and devices which don't
have such problem.

Best regards,
Lu Baolu

[Lipengcheng]

I'm sorry. I will try to it.

> >> Best regards,
> >> Lu Baolu
> >>
> >>> Signed-off-by: Pengcheng Li <lpc...@hisilicon.com>
> >>> ---
> >>> drivers/usb/core/hub.c | 6 +++++-
> >>> 1 file changed, 5 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index
> >>> bee1351..cd71bb3 100644
> >>> --- a/drivers/usb/core/hub.c
> >>> +++ b/drivers/usb/core/hub.c
> >>> @@ -3455,7 +3455,7 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
> >>> struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
> >>> struct usb_port *port_dev = hub->ports[udev->portnum - 1];
> >>> int port1 = udev->portnum;
> >>> - int status;
> >>> + int status, retval;
> >>> u16 portchange, portstatus;
> >>>
> >>> if (!test_and_set_bit(port1, hub->child_usage_bits)) { @@ -3512,6
> >>> +3512,10 @@ int usb_port_resume(struct usb_device *udev,

> >>> +pm_message_t msg)

> >>> }
> >>> }
> >>>
> >>> + retval = hub_port_reset(hub, port1, udev, HUB_ROOT_RESET_TIME, false);
> >>> + if (retval < 0)
> >>> + hub_port_disable(hub, port1, 0);
> >>> +
>
> If I understand it right, this is a "host + device" specific issue. This line of code might solve your issue, but it impacts all other hosts and devices
> which don't have such problem.
>

Yes. You are right. This line of code can solve my issue. But I suspect this is a common issue. At normal enumeration, the device has a reset operation. The resume
should have the same operation. In USB3.0 spec, superspeed device is at the wrong statue(u2 statue), and we should be reset the device.
> Best regards,
> Lu Baolu

Best regards,
Pengcheng Li