[PATCH] usb:solve resume usb device identification problem

[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.li@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

Re: [PATCH] usb:solve resume usb device identification problem

[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.li@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

Re: [PATCH] usb:solve resume usb device identification problem

[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

>
> Signed-off-by: Pengcheng Li <lpc.li@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);

RE: [PATCH] usb:solve resume usb device identification problem

[Lipengcheng]

Hi,
> -----Original Message-----
> From: Alan Stern [mailto:stern@rowland.harvard.edu]
> Sent: Monday, July 11, 2016 10:51 PM
> To: Lipengcheng
> Cc: gregkh@linuxfoundation.org; baolu.lu@linux.intel.com; chasemetzger15@gmail.com; mathias.nyman@linux.intel.com;
> oneukum@suse.com; jun.li@freescale.com; linux-usb@vger.kernel.org; linux-kernel@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.li@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

RE: [PATCH] usb:solve resume usb device identification problem

[Lipengcheng]

Hi,

> -----Original Message-----
> From: Lu Baolu [mailto:baolu.lu@linux.intel.com]
> Sent: Tuesday, July 12, 2016 8:42 AM
> To: Lipengcheng; gregkh@linuxfoundation.org; stern@rowland.harvard.edu; chasemetzger15@gmail.com; mathias.nyman@linux.intel.com;
> oneukum@suse.com; jun.li@freescale.com
> Cc: linux-usb@vger.kernel.org; linux-kernel@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,
> Lu  Baolu
> 
> >
> > Signed-off-by: Pengcheng Li <lpc.li@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);
Best regards
Pengcheng Li

Re: [PATCH] usb:solve resume usb device identification problem

[Lu Baolu]

Hi,

On 07/12/2016 09:48 AM, Lipengcheng wrote:
> Hi,
>
>> -----Original Message-----
>> From: Lu Baolu [mailto:baolu.lu@linux.intel.com]
>> Sent: Tuesday, July 12, 2016 8:42 AM
>> To: Lipengcheng; gregkh@linuxfoundation.org; stern@rowland.harvard.edu; chasemetzger15@gmail.com; mathias.nyman@linux.intel.com;
>> oneukum@suse.com; jun.li@freescale.com
>> Cc: linux-usb@vger.kernel.org; linux-kernel@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.li@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

RE: [PATCH] usb:solve resume usb device identification problem

[Lipengcheng]

> -----Original Message-----
> From: Lu Baolu [mailto:baolu.lu@linux.intel.com]
> Sent: Tuesday, July 12, 2016 10:24 AM
> To: Lipengcheng; gregkh@linuxfoundation.org; stern@rowland.harvard.edu; chasemetzger15@gmail.com; mathias.nyman@linux.intel.com;
> oneukum@suse.com; jun.li@freescale.com
> Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] usb:solve resume usb device identification problem
> 
> Hi,
> 
> On 07/12/2016 09:48 AM, Lipengcheng wrote:
> > Hi,
> >
> >> -----Original Message-----
> >> From: Lu Baolu [mailto:baolu.lu@linux.intel.com]
> >> Sent: Tuesday, July 12, 2016 8:42 AM
> >> To: Lipengcheng; gregkh@linuxfoundation.org;
> >> stern@rowland.harvard.edu; chasemetzger15@gmail.com;
> >> mathias.nyman@linux.intel.com; oneukum@suse.com; jun.li@freescale.com
> >> Cc: linux-usb@vger.kernel.org; linux-kernel@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.
> 
I'm sorry. I will try to it.
> >> Best regards,
> >> Lu  Baolu
> >>
> >>> Signed-off-by: Pengcheng Li <lpc.li@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