* [PATCH] usb: Allow USB device to be warm reset in suspended state
@ 2019-11-05 16:54 Kai-Heng Feng
2019-11-05 18:07 ` Alan Stern
0 siblings, 1 reply; 4+ messages in thread
From: Kai-Heng Feng @ 2019-11-05 16:54 UTC (permalink / raw)
To: gregkh; +Cc: mathias.nyman, stern, linux-usb, linux-kernel, Kai-Heng Feng
On Dell WD15 dock, sometimes USB ethernet cannot be detected after plugging
cable to the ethernet port, the hub and roothub get runtime resumed and
runtime suspended immediately:
...
[ 433.315169] xhci_hcd 0000:3a:00.0: hcd_pci_runtime_resume: 0
[ 433.315204] usb usb4: usb auto-resume
[ 433.315226] hub 4-0:1.0: hub_resume
[ 433.315239] xhci_hcd 0000:3a:00.0: Get port status 4-1 read: 0x10202e2, return 0x10343
[ 433.315264] usb usb4-port1: status 0343 change 0001
[ 433.315279] xhci_hcd 0000:3a:00.0: clear port1 connect change, portsc: 0x10002e2
[ 433.315293] xhci_hcd 0000:3a:00.0: Get port status 4-2 read: 0x2a0, return 0x2a0
[ 433.317012] xhci_hcd 0000:3a:00.0: xhci_hub_status_data: stopping port polling.
[ 433.422282] xhci_hcd 0000:3a:00.0: Get port status 4-1 read: 0x10002e2, return 0x343
[ 433.422307] usb usb4-port1: do warm reset
[ 433.422311] usb 4-1: device reset not allowed in state 8
[ 433.422339] hub 4-0:1.0: state 7 ports 2 chg 0002 evt 0000
[ 433.422346] xhci_hcd 0000:3a:00.0: Get port status 4-1 read: 0x10002e2, return 0x343
[ 433.422356] usb usb4-port1: do warm reset
[ 433.422358] usb 4-1: device reset not allowed in state 8
[ 433.422428] xhci_hcd 0000:3a:00.0: set port remote wake mask, actual port 0 status = 0xf0002e2
[ 433.422455] xhci_hcd 0000:3a:00.0: set port remote wake mask, actual port 1 status = 0xe0002a0
[ 433.422465] hub 4-0:1.0: hub_suspend
[ 433.422475] usb usb4: bus auto-suspend, wakeup 1
[ 433.426161] xhci_hcd 0000:3a:00.0: xhci_hub_status_data: stopping port polling.
[ 433.466209] xhci_hcd 0000:3a:00.0: port 0 polling in bus suspend, waiting
[ 433.510204] xhci_hcd 0000:3a:00.0: port 0 polling in bus suspend, waiting
[ 433.554051] xhci_hcd 0000:3a:00.0: port 0 polling in bus suspend, waiting
[ 433.598235] xhci_hcd 0000:3a:00.0: port 0 polling in bus suspend, waiting
[ 433.642154] xhci_hcd 0000:3a:00.0: port 0 polling in bus suspend, waiting
[ 433.686204] xhci_hcd 0000:3a:00.0: port 0 polling in bus suspend, waiting
[ 433.730205] xhci_hcd 0000:3a:00.0: port 0 polling in bus suspend, waiting
[ 433.774203] xhci_hcd 0000:3a:00.0: port 0 polling in bus suspend, waiting
[ 433.818207] xhci_hcd 0000:3a:00.0: port 0 polling in bus suspend, waiting
[ 433.862040] xhci_hcd 0000:3a:00.0: port 0 polling in bus suspend, waiting
[ 433.862053] xhci_hcd 0000:3a:00.0: xhci_hub_status_data: stopping port polling.
[ 433.862077] xhci_hcd 0000:3a:00.0: xhci_suspend: stopping port polling.
[ 433.862096] xhci_hcd 0000:3a:00.0: // Setting command ring address to 0x8578fc001
[ 433.862312] xhci_hcd 0000:3a:00.0: hcd_pci_runtime_suspend: 0
[ 433.862445] xhci_hcd 0000:3a:00.0: PME# enabled
[ 433.902376] xhci_hcd 0000:3a:00.0: restoring config space at offset 0xc (was 0x0, writing 0x20)
[ 433.902395] xhci_hcd 0000:3a:00.0: restoring config space at offset 0x4 (was 0x100000, writing 0x100403)
[ 433.902490] xhci_hcd 0000:3a:00.0: PME# disabled
[ 433.902504] xhci_hcd 0000:3a:00.0: enabling bus mastering
[ 433.902547] xhci_hcd 0000:3a:00.0: // Setting command ring address to 0x8578fc001
[ 433.902649] pcieport 0000:00:1b.0: PME: Spurious native interrupt!
[ 433.902839] xhci_hcd 0000:3a:00.0: Port change event, 4-1, id 3, portsc: 0xb0202e2
[ 433.902842] xhci_hcd 0000:3a:00.0: resume root hub
[ 433.902845] xhci_hcd 0000:3a:00.0: handle_port_status: starting port polling.
[ 433.902877] xhci_hcd 0000:3a:00.0: xhci_resume: starting port polling.
[ 433.902889] xhci_hcd 0000:3a:00.0: xhci_hub_status_data: stopping port polling.
[ 433.902891] xhci_hcd 0000:3a:00.0: hcd_pci_runtime_resume: 0
[ 433.902919] usb usb4: usb wakeup-resume
[ 433.902942] usb usb4: usb auto-resume
[ 433.902966] hub 4-0:1.0: hub_resume
...
As Mathias pointed out, the hub enters Cold Attach Status state and
requires a warm reset. However usb_reset_device() bails out early when
the device is in suspended state, as its callers port_event() and
hub_event() don't always resume the device.
Since there's nothing wrong to reset a suspended device, allow
usb_reset_device() to do so to solve the issue.
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
drivers/usb/core/hub.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 05a2d51bdbe0..f0194fdbc9b8 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -5877,8 +5877,7 @@ int usb_reset_device(struct usb_device *udev)
struct usb_host_config *config = udev->actconfig;
struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
- if (udev->state == USB_STATE_NOTATTACHED ||
- udev->state == USB_STATE_SUSPENDED) {
+ if (udev->state == USB_STATE_NOTATTACHED) {
dev_dbg(&udev->dev, "device reset not allowed in state %d\n",
udev->state);
return -EINVAL;
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] usb: Allow USB device to be warm reset in suspended state
2019-11-05 16:54 [PATCH] usb: Allow USB device to be warm reset in suspended state Kai-Heng Feng
@ 2019-11-05 18:07 ` Alan Stern
2019-11-05 18:18 ` Kai-Heng Feng
2019-11-06 11:23 ` Bjørn Mork
0 siblings, 2 replies; 4+ messages in thread
From: Alan Stern @ 2019-11-05 18:07 UTC (permalink / raw)
To: Kai-Heng Feng; +Cc: gregkh, mathias.nyman, linux-usb, linux-kernel
On Wed, 6 Nov 2019, Kai-Heng Feng wrote:
> On Dell WD15 dock, sometimes USB ethernet cannot be detected after plugging
> cable to the ethernet port, the hub and roothub get runtime resumed and
> runtime suspended immediately:
> ...
> ...
>
> As Mathias pointed out, the hub enters Cold Attach Status state and
> requires a warm reset. However usb_reset_device() bails out early when
> the device is in suspended state, as its callers port_event() and
> hub_event() don't always resume the device.
>
> Since there's nothing wrong to reset a suspended device, allow
> usb_reset_device() to do so to solve the issue.
I was sure I remembered reading somewhere that suspended devices were
not allowed to be reset, but now I can't find that requirement anywhere
in the USB spec.
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> drivers/usb/core/hub.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 05a2d51bdbe0..f0194fdbc9b8 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -5877,8 +5877,7 @@ int usb_reset_device(struct usb_device *udev)
> struct usb_host_config *config = udev->actconfig;
> struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
>
> - if (udev->state == USB_STATE_NOTATTACHED ||
> - udev->state == USB_STATE_SUSPENDED) {
> + if (udev->state == USB_STATE_NOTATTACHED) {
> dev_dbg(&udev->dev, "device reset not allowed in state %d\n",
> udev->state);
> return -EINVAL;
You forgot to update the kerneldoc for this function.
Alan Stern
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] usb: Allow USB device to be warm reset in suspended state
2019-11-05 18:07 ` Alan Stern
@ 2019-11-05 18:18 ` Kai-Heng Feng
2019-11-06 11:23 ` Bjørn Mork
1 sibling, 0 replies; 4+ messages in thread
From: Kai-Heng Feng @ 2019-11-05 18:18 UTC (permalink / raw)
To: Alan Stern
Cc: Greg Kroah-Hartman, Mathias Nyman, USB list, Kernel development list
> On Nov 6, 2019, at 02:07, Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Wed, 6 Nov 2019, Kai-Heng Feng wrote:
>
>> On Dell WD15 dock, sometimes USB ethernet cannot be detected after plugging
>> cable to the ethernet port, the hub and roothub get runtime resumed and
>> runtime suspended immediately:
>> ...
>
>> ...
>>
>> As Mathias pointed out, the hub enters Cold Attach Status state and
>> requires a warm reset. However usb_reset_device() bails out early when
>> the device is in suspended state, as its callers port_event() and
>> hub_event() don't always resume the device.
>>
>> Since there's nothing wrong to reset a suspended device, allow
>> usb_reset_device() to do so to solve the issue.
>
> I was sure I remembered reading somewhere that suspended devices were
> not allowed to be reset, but now I can't find that requirement anywhere
> in the USB spec.
I don't find it in the USB spec either.
That said, the following usb_autoresume_device() before reset may resume the device.
I've also tried using pm_runtime_get_noresume() and it works equally well for my case but I am not sure if we want to change the behavior here.
>
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> ---
>> drivers/usb/core/hub.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
>> index 05a2d51bdbe0..f0194fdbc9b8 100644
>> --- a/drivers/usb/core/hub.c
>> +++ b/drivers/usb/core/hub.c
>> @@ -5877,8 +5877,7 @@ int usb_reset_device(struct usb_device *udev)
>> struct usb_host_config *config = udev->actconfig;
>> struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
>>
>> - if (udev->state == USB_STATE_NOTATTACHED ||
>> - udev->state == USB_STATE_SUSPENDED) {
>> + if (udev->state == USB_STATE_NOTATTACHED) {
>> dev_dbg(&udev->dev, "device reset not allowed in state %d\n",
>> udev->state);
>> return -EINVAL;
>
> You forgot to update the kerneldoc for this function.
Ok, will do that in v2.
Kai-Heng
>
> Alan Stern
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] usb: Allow USB device to be warm reset in suspended state
2019-11-05 18:07 ` Alan Stern
2019-11-05 18:18 ` Kai-Heng Feng
@ 2019-11-06 11:23 ` Bjørn Mork
1 sibling, 0 replies; 4+ messages in thread
From: Bjørn Mork @ 2019-11-06 11:23 UTC (permalink / raw)
To: Alan Stern; +Cc: Kai-Heng Feng, gregkh, mathias.nyman, linux-usb, linux-kernel
Alan Stern <stern@rowland.harvard.edu> writes:
> I was sure I remembered reading somewhere that suspended devices were
> not allowed to be reset, but now I can't find that requirement anywhere
> in the USB spec.
I don't know anything about this, but "Reset From Suspended State" is
part of Appendix C in the USB 2.0 spec. Looks valid to me..
Quoting the relevant section for those who don't have that spec at hand:
C.2.1 Reset From Suspended State
As can be seen from Figure C-2, the device wakes up from the Suspended
state as soon as it sees a K or an SE0 on the bus. A J would be
indistinguishable from idle on the bus that a suspended device sees
normally. On seeing a K, the device will initiate a resume
process. For the details of this process, see Section 7.1.7.7. On
seeing an SE0, the device could enter the reset handshake procedure,
so it starts timer T0.
The actual reset handshake is only started after seeing a continuous
assertion of SE0 for at least 2.5 μs (T FILTSE0 ). The loop between
the blocks with “Clear timer T1” and “Run timer T1” represents this
filtering. If the device has not detected a continuous SE0 before
timer T0 exceeds the value of T UCHEND - T UCH , the device goes back
into the Suspended state.
A device coming from suspend most probably had its high-speed clock
stopped to meet the power requirements for a suspended device (see
Section 7.2.3). Therefore, it may take some time to let the clock
settle to a level of operation where it is able to perform the reset
detection and handshake with enough precision. In the state diagram, a
time symbol T WTCLK is used to have the device wait for a stable
clock. This symbol is not part of the USB 2.0 specification and does
not appear in Chapter 7. It is an implementation specific detail of
the reset detection state diagram for the upstream facing port, where
it is marked with a asterisk (*). T WTCLK should have a value
somewhere between 0 and 5.0 ms. This allows at least 1.0 ms time to
detect the continuous SE0.
If the device has seen an SE0 signal on the bus for at least T FILTSE0
, then it can safely assume to have detected a reset and can start the
reset handshake.
Bjørn
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-11-06 11:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-05 16:54 [PATCH] usb: Allow USB device to be warm reset in suspended state Kai-Heng Feng
2019-11-05 18:07 ` Alan Stern
2019-11-05 18:18 ` Kai-Heng Feng
2019-11-06 11:23 ` Bjørn Mork
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).