linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Revert "usb: Avoid unnecessary LPM enabling and disabling during suspend and resume"
@ 2019-10-02 15:15 Kai-Heng Feng
  2019-10-02 15:47 ` Alan Stern
  2019-10-03 15:56 ` [PATCH v2] usb: Add a new quirk to let buggy hub enable and disable LPM during suspend and resume Kai-Heng Feng
  0 siblings, 2 replies; 13+ messages in thread
From: Kai-Heng Feng @ 2019-10-02 15:15 UTC (permalink / raw)
  To: mathias.nyman, gregkh; +Cc: linux-usb, linux-kernel, Kai-Heng Feng

This reverts commit d590c23111505635e1beb01006612971e5ede8aa.

Dell WD15 dock has a topology like this:
/:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 10000M
    |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/7p, 5000M
            |__ Port 2: Dev 3, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M

Their IDs:
Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 004 Device 002: ID 0424:5537 Standard Microsystems Corp.
Bus 004 Device 004: ID 0bda:8153 Realtek Semiconductor Corp.

Ethernet cannot be detected after plugging ethernet cable to the dock,
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

At this point the SMSC hub (usb 4-1) goes into compliance mode
(USB_SS_PORT_LS_COMP_MOD), and USB core tries to warm-reset it,

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

However the warm-reset never success, the asserted PCI PME keeps the
runtime-resume, warm-reset and runtime-suspend loop which never bring it back
causing spurious interrupts floods.

After some trial and errors, the issue goes away if LPM on the SMSC hub
is disabled. Digging further, enabling and disabling LPM during runtime
resume and runtime suspend respectively can solve the issue.

So bring back the old LPM behavior, which the SMSC hub inside Dell WD15
depends on.

Fixes: d590c2311150 ("usb: Avoid unnecessary LPM enabling and disabling during suspend and resume")
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 drivers/usb/core/hub.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 236313f41f4a..22a96d279b14 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3269,6 +3269,12 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
 		if (PMSG_IS_AUTO(msg))
 			goto err_ltm;
 	}
+	if (usb_unlocked_disable_lpm(udev)) {
+		dev_err(&udev->dev, "Failed to disable LPM before suspend\n.");
+		status = -ENOMEM;
+		if (PMSG_IS_AUTO(msg))
+			goto err_lpm3;
+	}
 
 	/* see 7.1.7.6 */
 	if (hub_is_superspeed(hub->hdev))
@@ -3295,7 +3301,9 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
 	if (status) {
 		dev_dbg(&port_dev->dev, "can't suspend, status %d\n", status);
 
-		/* Try to enable USB3 LTM again */
+		/* Try to enable USB3 LPM and LTM again */
+		usb_unlocked_enable_lpm(udev);
+ err_lpm3:
 		usb_enable_ltm(udev);
  err_ltm:
 		/* Try to enable USB2 hardware LPM again */
@@ -3584,8 +3592,9 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
 		/* Try to enable USB2 hardware LPM */
 		usb_enable_usb2_hardware_lpm(udev);
 
-		/* Try to enable USB3 LTM */
+		/* Try to enable USB3 LTM and LPM */
 		usb_enable_ltm(udev);
+		usb_unlocked_enable_lpm(udev);
 	}
 
 	usb_unlock_port(port_dev);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] Revert "usb: Avoid unnecessary LPM enabling and disabling during suspend and resume"
  2019-10-02 15:15 [PATCH] Revert "usb: Avoid unnecessary LPM enabling and disabling during suspend and resume" Kai-Heng Feng
@ 2019-10-02 15:47 ` Alan Stern
  2019-10-03 11:55   ` Kai-Heng Feng
  2019-10-03 15:56 ` [PATCH v2] usb: Add a new quirk to let buggy hub enable and disable LPM during suspend and resume Kai-Heng Feng
  1 sibling, 1 reply; 13+ messages in thread
From: Alan Stern @ 2019-10-02 15:47 UTC (permalink / raw)
  To: Kai-Heng Feng; +Cc: mathias.nyman, gregkh, linux-usb, linux-kernel

On Wed, 2 Oct 2019, Kai-Heng Feng wrote:

> This reverts commit d590c23111505635e1beb01006612971e5ede8aa.
> 
> Dell WD15 dock has a topology like this:
> /:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 10000M
>     |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/7p, 5000M
>             |__ Port 2: Dev 3, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M
> 
> Their IDs:
> Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> Bus 004 Device 002: ID 0424:5537 Standard Microsystems Corp.
> Bus 004 Device 004: ID 0bda:8153 Realtek Semiconductor Corp.
> 
> Ethernet cannot be detected after plugging ethernet cable to the dock,
> the hub and roothub get runtime resumed and runtime suspended
> immediately:
> ...

> After some trial and errors, the issue goes away if LPM on the SMSC hub
> is disabled. Digging further, enabling and disabling LPM during runtime
> resume and runtime suspend respectively can solve the issue.
> 
> So bring back the old LPM behavior, which the SMSC hub inside Dell WD15
> depends on.
> 
> Fixes: d590c2311150 ("usb: Avoid unnecessary LPM enabling and disabling during suspend and resume")
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

Maybe it would be better to have a VID/PID-specific quirk for this?

Alan Stern


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Revert "usb: Avoid unnecessary LPM enabling and disabling during suspend and resume"
  2019-10-02 15:47 ` Alan Stern
@ 2019-10-03 11:55   ` Kai-Heng Feng
  2019-10-03 14:26     ` Alan Stern
  0 siblings, 1 reply; 13+ messages in thread
From: Kai-Heng Feng @ 2019-10-03 11:55 UTC (permalink / raw)
  To: Alan Stern; +Cc: mathias.nyman, gregkh, linux-usb, linux-kernel



> On Oct 2, 2019, at 23:47, Alan Stern <stern@rowland.harvard.edu> wrote:
> 
> On Wed, 2 Oct 2019, Kai-Heng Feng wrote:
> 
>> This reverts commit d590c23111505635e1beb01006612971e5ede8aa.
>> 
>> Dell WD15 dock has a topology like this:
>> /:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 10000M
>>    |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/7p, 5000M
>>            |__ Port 2: Dev 3, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M
>> 
>> Their IDs:
>> Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
>> Bus 004 Device 002: ID 0424:5537 Standard Microsystems Corp.
>> Bus 004 Device 004: ID 0bda:8153 Realtek Semiconductor Corp.
>> 
>> Ethernet cannot be detected after plugging ethernet cable to the dock,
>> the hub and roothub get runtime resumed and runtime suspended
>> immediately:
>> ...
> 
>> After some trial and errors, the issue goes away if LPM on the SMSC hub
>> is disabled. Digging further, enabling and disabling LPM during runtime
>> resume and runtime suspend respectively can solve the issue.
>> 
>> So bring back the old LPM behavior, which the SMSC hub inside Dell WD15
>> depends on.
>> 
>> Fixes: d590c2311150 ("usb: Avoid unnecessary LPM enabling and disabling during suspend and resume")
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> 
> Maybe it would be better to have a VID/PID-specific quirk for this?

Re-reading the spec, I think we need some clarification:
"If the value is 3, then host software wants to selectively suspend the
device connected to this port. The hub shall transition the link to U3
from any of the other U states using allowed link state transitions.
If the port is not already in the U0 state, then it shall transition the
port to the U0 state and then initiate the transition to U3."

The phrase "then it shall transition the port to the U0 state" what does "it" here refer to?
Is it the hub or the software?
If it's the former then it's indeed a buggy hub, but if it's the latter I think reverting the commit is the right thing to do.

Kai-Heng

> 
> Alan Stern
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Revert "usb: Avoid unnecessary LPM enabling and disabling during suspend and resume"
  2019-10-03 11:55   ` Kai-Heng Feng
@ 2019-10-03 14:26     ` Alan Stern
  2019-10-03 14:57       ` Kai-Heng Feng
  0 siblings, 1 reply; 13+ messages in thread
From: Alan Stern @ 2019-10-03 14:26 UTC (permalink / raw)
  To: Kai-Heng Feng; +Cc: mathias.nyman, gregkh, linux-usb, linux-kernel

On Thu, 3 Oct 2019, Kai-Heng Feng wrote:

> > On Oct 2, 2019, at 23:47, Alan Stern <stern@rowland.harvard.edu> wrote:
> > 
> > On Wed, 2 Oct 2019, Kai-Heng Feng wrote:
> > 
> >> This reverts commit d590c23111505635e1beb01006612971e5ede8aa.
> >> 
> >> Dell WD15 dock has a topology like this:
> >> /:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 10000M
> >>    |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/7p, 5000M
> >>            |__ Port 2: Dev 3, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M
> >> 
> >> Their IDs:
> >> Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> >> Bus 004 Device 002: ID 0424:5537 Standard Microsystems Corp.
> >> Bus 004 Device 004: ID 0bda:8153 Realtek Semiconductor Corp.
> >> 
> >> Ethernet cannot be detected after plugging ethernet cable to the dock,
> >> the hub and roothub get runtime resumed and runtime suspended
> >> immediately:
> >> ...
> > 
> >> After some trial and errors, the issue goes away if LPM on the SMSC hub
> >> is disabled. Digging further, enabling and disabling LPM during runtime
> >> resume and runtime suspend respectively can solve the issue.
> >> 
> >> So bring back the old LPM behavior, which the SMSC hub inside Dell WD15
> >> depends on.
> >> 
> >> Fixes: d590c2311150 ("usb: Avoid unnecessary LPM enabling and disabling during suspend and resume")
> >> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > 
> > Maybe it would be better to have a VID/PID-specific quirk for this?
> 
> Re-reading the spec, I think we need some clarification:
> "If the value is 3, then host software wants to selectively suspend the
> device connected to this port. The hub shall transition the link to U3
> from any of the other U states using allowed link state transitions.
> If the port is not already in the U0 state, then it shall transition the
> port to the U0 state and then initiate the transition to U3."
> 
> The phrase "then it shall transition the port to the U0 state" what does "it" here refer to?
> Is it the hub or the software?
> If it's the former then it's indeed a buggy hub, but if it's the latter I think reverting the commit is the right thing to do.

In my opinion, "it" here refers to the hub.  This is because of the 
parallel construction with the preceding sentence ("... shall 
transition the link/port"), which indicates that the subjects should be 
the same.

Alan Stern


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Revert "usb: Avoid unnecessary LPM enabling and disabling during suspend and resume"
  2019-10-03 14:26     ` Alan Stern
@ 2019-10-03 14:57       ` Kai-Heng Feng
  0 siblings, 0 replies; 13+ messages in thread
From: Kai-Heng Feng @ 2019-10-03 14:57 UTC (permalink / raw)
  To: Alan Stern; +Cc: mathias.nyman, gregkh, linux-usb, linux-kernel



> On Oct 3, 2019, at 22:26, Alan Stern <stern@rowland.harvard.edu> wrote:
> 
> On Thu, 3 Oct 2019, Kai-Heng Feng wrote:
> 
>>> On Oct 2, 2019, at 23:47, Alan Stern <stern@rowland.harvard.edu> wrote:
>>> 
>>> On Wed, 2 Oct 2019, Kai-Heng Feng wrote:
>>> 
>>>> This reverts commit d590c23111505635e1beb01006612971e5ede8aa.
>>>> 
>>>> Dell WD15 dock has a topology like this:
>>>> /:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 10000M
>>>>   |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/7p, 5000M
>>>>           |__ Port 2: Dev 3, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M
>>>> 
>>>> Their IDs:
>>>> Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
>>>> Bus 004 Device 002: ID 0424:5537 Standard Microsystems Corp.
>>>> Bus 004 Device 004: ID 0bda:8153 Realtek Semiconductor Corp.
>>>> 
>>>> Ethernet cannot be detected after plugging ethernet cable to the dock,
>>>> the hub and roothub get runtime resumed and runtime suspended
>>>> immediately:
>>>> ...
>>> 
>>>> After some trial and errors, the issue goes away if LPM on the SMSC hub
>>>> is disabled. Digging further, enabling and disabling LPM during runtime
>>>> resume and runtime suspend respectively can solve the issue.
>>>> 
>>>> So bring back the old LPM behavior, which the SMSC hub inside Dell WD15
>>>> depends on.
>>>> 
>>>> Fixes: d590c2311150 ("usb: Avoid unnecessary LPM enabling and disabling during suspend and resume")
>>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>>> 
>>> Maybe it would be better to have a VID/PID-specific quirk for this?
>> 
>> Re-reading the spec, I think we need some clarification:
>> "If the value is 3, then host software wants to selectively suspend the
>> device connected to this port. The hub shall transition the link to U3
>> from any of the other U states using allowed link state transitions.
>> If the port is not already in the U0 state, then it shall transition the
>> port to the U0 state and then initiate the transition to U3."
>> 
>> The phrase "then it shall transition the port to the U0 state" what does "it" here refer to?
>> Is it the hub or the software?
>> If it's the former then it's indeed a buggy hub, but if it's the latter I think reverting the commit is the right thing to do.
> 
> In my opinion, "it" here refers to the hub.  This is because of the 
> parallel construction with the preceding sentence ("... shall 
> transition the link/port"), which indicates that the subjects should be 
> the same.

Hmm, okay, this is ambiguous to a non-native speaker like me.
I'll use a quirk instead.

Kai-Heng

> 
> Alan Stern
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2] usb: Add a new quirk to let buggy hub enable and disable LPM during suspend and resume
  2019-10-02 15:15 [PATCH] Revert "usb: Avoid unnecessary LPM enabling and disabling during suspend and resume" Kai-Heng Feng
  2019-10-02 15:47 ` Alan Stern
@ 2019-10-03 15:56 ` Kai-Heng Feng
  2019-10-03 16:02   ` Greg KH
  2019-10-03 17:24   ` [PATCH v3] " Kai-Heng Feng
  1 sibling, 2 replies; 13+ messages in thread
From: Kai-Heng Feng @ 2019-10-03 15:56 UTC (permalink / raw)
  To: gregkh; +Cc: stern, mathias.nyman, linux-usb, linux-kernel, Kai-Heng Feng

Dell WD15 dock has a topology like this:
/:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 10000M
    |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/7p, 5000M
            |__ Port 2: Dev 3, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M

Their IDs:
Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 004 Device 002: ID 0424:5537 Standard Microsystems Corp.
Bus 004 Device 004: ID 0bda:8153 Realtek Semiconductor Corp.

Ethernet cannot be detected after plugging ethernet cable to the dock,
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

At this point the SMSC hub (usb 4-1) enters into compliance mode
(USB_SS_PORT_LS_COMP_MOD), and USB core tries to warm-reset it,

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

However the warm-reset never success, the asserted PCI PME keeps the
runtime-resume, warm-reset and runtime-suspend loop which never bring it back
and causing spurious interrupts floods.

After some trial and errors, the issue goes away if LPM on the SMSC hub
is disabled. Digging further, enabling and disabling LPM during runtime
resume and runtime suspend respectively can solve the issue.

So bring back the old LPM behavior as a quirk and use it for the SMSC
hub to solve the issue.

Fixes: d590c2311150 ("usb: Avoid unnecessary LPM enabling and disabling during suspend and resume")
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  3 +++
 drivers/usb/core/hub.c                          | 15 +++++++++++++++
 drivers/usb/core/quirks.c                       |  6 ++++++
 include/linux/usb/quirks.h                      |  3 +++
 4 files changed, 27 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index c7ac2f3ac99f..0b5ba2545373 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4974,6 +4974,9 @@
 					pause after every control message);
 				o = USB_QUIRK_HUB_SLOW_RESET (Hub needs extra
 					delay after resetting its port);
+				p = USB_QUIRK_PM_SET_LPM (Device needs to
+					disable LPM on suspend and enable LPM
+					on resume);
 			Example: quirks=0781:5580:bk,0a5c:5834:gij
 
 	usbhid.mousepoll=
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 236313f41f4a..5e07407c8204 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3269,6 +3269,13 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
 		if (PMSG_IS_AUTO(msg))
 			goto err_ltm;
 	}
+	if (udev->quirks & USB_QUIRK_PM_SET_LPM &&
+	    usb_unlocked_disable_lpm(udev)) {
+		dev_err(&udev->dev, "Failed to disable LPM before suspend\n.");
+		status = -ENOMEM;
+		if (PMSG_IS_AUTO(msg))
+			goto err_lpm3;
+	}
 
 	/* see 7.1.7.6 */
 	if (hub_is_superspeed(hub->hdev))
@@ -3295,6 +3302,10 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
 	if (status) {
 		dev_dbg(&port_dev->dev, "can't suspend, status %d\n", status);
 
+		/* Try to enable USB3 LPM again */
+		if (udev->quirks & USB_QUIRK_PM_SET_LPM)
+			usb_unlocked_enable_lpm(udev);
+ err_lpm3:
 		/* Try to enable USB3 LTM again */
 		usb_enable_ltm(udev);
  err_ltm:
@@ -3586,6 +3597,10 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
 
 		/* Try to enable USB3 LTM */
 		usb_enable_ltm(udev);
+
+		/* Try to enable USB3 LPM */
+		if (udev->quirks & USB_QUIRK_PM_SET_LPM)
+			usb_unlocked_enable_lpm(udev);
 	}
 
 	usb_unlock_port(port_dev);
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index 6b6413073584..75bbc9faddb4 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -131,6 +131,9 @@ static int quirks_param_set(const char *val, const struct kernel_param *kp)
 			case 'o':
 				flags |= USB_QUIRK_HUB_SLOW_RESET;
 				break;
+			case 'p':
+				flags |= USB_QUIRK_PM_SET_LPM;
+				break;
 			/* Ignore unrecognized flag characters */
 			}
 		}
@@ -203,6 +206,9 @@ static const struct usb_device_id usb_quirk_list[] = {
 	/* USB3503 */
 	{ USB_DEVICE(0x0424, 0x3503), .driver_info = USB_QUIRK_RESET_RESUME },
 
+	/* SMSC USB5537B USB 3.0 Hub (Dell WD15) */
+	{ USB_DEVICE(0x0424, 0x5537), .driver_info = USB_QUIRK_PM_SET_LPM },
+
 	/* Microsoft Wireless Laser Mouse 6000 Receiver */
 	{ USB_DEVICE(0x045e, 0x00e1), .driver_info = USB_QUIRK_RESET_RESUME },
 
diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h
index a1be64c9940f..3f2d97eff369 100644
--- a/include/linux/usb/quirks.h
+++ b/include/linux/usb/quirks.h
@@ -69,4 +69,7 @@
 /* Hub needs extra delay after resetting its port. */
 #define USB_QUIRK_HUB_SLOW_RESET		BIT(14)
 
+/* Device needs to disable LPM on suspend and enable LPM on resume */
+#define USB_QUIRK_PM_SET_LPM			BIT(15)
+
 #endif /* __LINUX_USB_QUIRKS_H */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] usb: Add a new quirk to let buggy hub enable and disable LPM during suspend and resume
  2019-10-03 15:56 ` [PATCH v2] usb: Add a new quirk to let buggy hub enable and disable LPM during suspend and resume Kai-Heng Feng
@ 2019-10-03 16:02   ` Greg KH
  2019-10-03 17:24   ` [PATCH v3] " Kai-Heng Feng
  1 sibling, 0 replies; 13+ messages in thread
From: Greg KH @ 2019-10-03 16:02 UTC (permalink / raw)
  To: Kai-Heng Feng; +Cc: stern, mathias.nyman, linux-usb, linux-kernel

On Thu, Oct 03, 2019 at 11:56:40PM +0800, Kai-Heng Feng wrote:
> Dell WD15 dock has a topology like this:
> /:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 10000M
>     |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/7p, 5000M
>             |__ Port 2: Dev 3, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M
> 
> Their IDs:
> Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> Bus 004 Device 002: ID 0424:5537 Standard Microsystems Corp.
> Bus 004 Device 004: ID 0bda:8153 Realtek Semiconductor Corp.
> 
> Ethernet cannot be detected after plugging ethernet cable to the dock,
> 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
> 
> At this point the SMSC hub (usb 4-1) enters into compliance mode
> (USB_SS_PORT_LS_COMP_MOD), and USB core tries to warm-reset it,
> 
> [  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
> ...
> 
> However the warm-reset never success, the asserted PCI PME keeps the
> runtime-resume, warm-reset and runtime-suspend loop which never bring it back
> and causing spurious interrupts floods.
> 
> After some trial and errors, the issue goes away if LPM on the SMSC hub
> is disabled. Digging further, enabling and disabling LPM during runtime
> resume and runtime suspend respectively can solve the issue.
> 
> So bring back the old LPM behavior as a quirk and use it for the SMSC
> hub to solve the issue.
> 
> Fixes: d590c2311150 ("usb: Avoid unnecessary LPM enabling and disabling during suspend and resume")
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  3 +++
>  drivers/usb/core/hub.c                          | 15 +++++++++++++++
>  drivers/usb/core/quirks.c                       |  6 ++++++
>  include/linux/usb/quirks.h                      |  3 +++
>  4 files changed, 27 insertions(+)

What changed from v1?  That should go below the --- line.

Can you send v3 please?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3] usb: Add a new quirk to let buggy hub enable and disable LPM during suspend and resume
  2019-10-03 15:56 ` [PATCH v2] usb: Add a new quirk to let buggy hub enable and disable LPM during suspend and resume Kai-Heng Feng
  2019-10-03 16:02   ` Greg KH
@ 2019-10-03 17:24   ` Kai-Heng Feng
  2019-10-03 19:04     ` Alan Stern
  1 sibling, 1 reply; 13+ messages in thread
From: Kai-Heng Feng @ 2019-10-03 17:24 UTC (permalink / raw)
  To: gregkh; +Cc: stern, mathias.nyman, linux-usb, linux-kernel, Kai-Heng Feng

Dell WD15 dock has a topology like this:
/:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 10000M
    |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/7p, 5000M
            |__ Port 2: Dev 3, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M

Their IDs:
Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 004 Device 002: ID 0424:5537 Standard Microsystems Corp.
Bus 004 Device 004: ID 0bda:8153 Realtek Semiconductor Corp.

Ethernet cannot be detected after plugging ethernet cable to the dock,
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

At this point the SMSC hub (usb 4-1) enters into compliance mode
(USB_SS_PORT_LS_COMP_MOD), and USB core tries to warm-reset it,

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

However the warm-reset never success, the asserted PCI PME keeps the
runtime-resume, warm-reset and runtime-suspend loop which never bring it back
and causing spurious interrupts floods.

After some trial and errors, the issue goes away if LPM on the SMSC hub
is disabled. Digging further, enabling and disabling LPM during runtime
resume and runtime suspend respectively can solve the issue.

So bring back the old LPM behavior as a quirk and use it for the SMSC
hub to solve the issue.

Fixes: d590c2311150 ("usb: Avoid unnecessary LPM enabling and disabling during suspend and resume")
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v3:
- Add forgotten patch revision changelog.

v2:
- Explained by Alan, the hub should properly handle U3 -> U0 transition.
  So use a quirk to target this buggy device only.

 Documentation/admin-guide/kernel-parameters.txt |  3 +++
 drivers/usb/core/hub.c                          | 15 +++++++++++++++
 drivers/usb/core/quirks.c                       |  6 ++++++
 include/linux/usb/quirks.h                      |  3 +++
 4 files changed, 27 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index c7ac2f3ac99f..0b5ba2545373 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4974,6 +4974,9 @@
 					pause after every control message);
 				o = USB_QUIRK_HUB_SLOW_RESET (Hub needs extra
 					delay after resetting its port);
+				p = USB_QUIRK_PM_SET_LPM (Device needs to
+					disable LPM on suspend and enable LPM
+					on resume);
 			Example: quirks=0781:5580:bk,0a5c:5834:gij
 
 	usbhid.mousepoll=
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 236313f41f4a..5e07407c8204 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3269,6 +3269,13 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
 		if (PMSG_IS_AUTO(msg))
 			goto err_ltm;
 	}
+	if (udev->quirks & USB_QUIRK_PM_SET_LPM &&
+	    usb_unlocked_disable_lpm(udev)) {
+		dev_err(&udev->dev, "Failed to disable LPM before suspend\n.");
+		status = -ENOMEM;
+		if (PMSG_IS_AUTO(msg))
+			goto err_lpm3;
+	}
 
 	/* see 7.1.7.6 */
 	if (hub_is_superspeed(hub->hdev))
@@ -3295,6 +3302,10 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
 	if (status) {
 		dev_dbg(&port_dev->dev, "can't suspend, status %d\n", status);
 
+		/* Try to enable USB3 LPM again */
+		if (udev->quirks & USB_QUIRK_PM_SET_LPM)
+			usb_unlocked_enable_lpm(udev);
+ err_lpm3:
 		/* Try to enable USB3 LTM again */
 		usb_enable_ltm(udev);
  err_ltm:
@@ -3586,6 +3597,10 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
 
 		/* Try to enable USB3 LTM */
 		usb_enable_ltm(udev);
+
+		/* Try to enable USB3 LPM */
+		if (udev->quirks & USB_QUIRK_PM_SET_LPM)
+			usb_unlocked_enable_lpm(udev);
 	}
 
 	usb_unlock_port(port_dev);
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index 6b6413073584..75bbc9faddb4 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -131,6 +131,9 @@ static int quirks_param_set(const char *val, const struct kernel_param *kp)
 			case 'o':
 				flags |= USB_QUIRK_HUB_SLOW_RESET;
 				break;
+			case 'p':
+				flags |= USB_QUIRK_PM_SET_LPM;
+				break;
 			/* Ignore unrecognized flag characters */
 			}
 		}
@@ -203,6 +206,9 @@ static const struct usb_device_id usb_quirk_list[] = {
 	/* USB3503 */
 	{ USB_DEVICE(0x0424, 0x3503), .driver_info = USB_QUIRK_RESET_RESUME },
 
+	/* SMSC USB5537B USB 3.0 Hub (Dell WD15) */
+	{ USB_DEVICE(0x0424, 0x5537), .driver_info = USB_QUIRK_PM_SET_LPM },
+
 	/* Microsoft Wireless Laser Mouse 6000 Receiver */
 	{ USB_DEVICE(0x045e, 0x00e1), .driver_info = USB_QUIRK_RESET_RESUME },
 
diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h
index a1be64c9940f..3f2d97eff369 100644
--- a/include/linux/usb/quirks.h
+++ b/include/linux/usb/quirks.h
@@ -69,4 +69,7 @@
 /* Hub needs extra delay after resetting its port. */
 #define USB_QUIRK_HUB_SLOW_RESET		BIT(14)
 
+/* Device needs to disable LPM on suspend and enable LPM on resume */
+#define USB_QUIRK_PM_SET_LPM			BIT(15)
+
 #endif /* __LINUX_USB_QUIRKS_H */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] usb: Add a new quirk to let buggy hub enable and disable LPM during suspend and resume
  2019-10-03 17:24   ` [PATCH v3] " Kai-Heng Feng
@ 2019-10-03 19:04     ` Alan Stern
  2019-10-17  6:33       ` Kai-Heng Feng
  0 siblings, 1 reply; 13+ messages in thread
From: Alan Stern @ 2019-10-03 19:04 UTC (permalink / raw)
  To: Kai-Heng Feng; +Cc: gregkh, mathias.nyman, USB list, Kernel development list

On Fri, 4 Oct 2019, Kai-Heng Feng wrote:

> Dell WD15 dock has a topology like this:
> /:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 10000M
>     |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/7p, 5000M
>             |__ Port 2: Dev 3, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M
> 
> Their IDs:
> Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> Bus 004 Device 002: ID 0424:5537 Standard Microsystems Corp.
> Bus 004 Device 004: ID 0bda:8153 Realtek Semiconductor Corp.
> 
> Ethernet cannot be detected after plugging ethernet cable to the dock,
> 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
> 
> At this point the SMSC hub (usb 4-1) enters into compliance mode
> (USB_SS_PORT_LS_COMP_MOD), and USB core tries to warm-reset it,
> 
> [  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
> ...
> 
> However the warm-reset never success, the asserted PCI PME keeps the
> runtime-resume, warm-reset and runtime-suspend loop which never bring it back
> and causing spurious interrupts floods.
> 
> After some trial and errors, the issue goes away if LPM on the SMSC hub
> is disabled. Digging further, enabling and disabling LPM during runtime
> resume and runtime suspend respectively can solve the issue.
> 
> So bring back the old LPM behavior as a quirk and use it for the SMSC
> hub to solve the issue.
> 
> Fixes: d590c2311150 ("usb: Avoid unnecessary LPM enabling and disabling during suspend and resume")
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> v3:
> - Add forgotten patch revision changelog.
> 
> v2:
> - Explained by Alan, the hub should properly handle U3 -> U0 transition.
>   So use a quirk to target this buggy device only.
> 
>  Documentation/admin-guide/kernel-parameters.txt |  3 +++
>  drivers/usb/core/hub.c                          | 15 +++++++++++++++
>  drivers/usb/core/quirks.c                       |  6 ++++++
>  include/linux/usb/quirks.h                      |  3 +++
>  4 files changed, 27 insertions(+)

Mathias may want to try something different to fix this problem.  But
if he doesn't, this patch is okay with me.

Acked-by: Alan Stern <stern@rowland.harvard.edu>

Alan Stern


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] usb: Add a new quirk to let buggy hub enable and disable LPM during suspend and resume
  2019-10-03 19:04     ` Alan Stern
@ 2019-10-17  6:33       ` Kai-Heng Feng
  2019-10-18 18:59         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 13+ messages in thread
From: Kai-Heng Feng @ 2019-10-17  6:33 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Alan Stern, Mathias Nyman, USB list, Kernel development list



> On Oct 4, 2019, at 03:04, Alan Stern <stern@rowland.harvard.edu> wrote:
> 
> On Fri, 4 Oct 2019, Kai-Heng Feng wrote:
> 
>> Dell WD15 dock has a topology like this:
>> /:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 10000M
>>    |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/7p, 5000M
>>            |__ Port 2: Dev 3, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M
>> 
>> Their IDs:
>> Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
>> Bus 004 Device 002: ID 0424:5537 Standard Microsystems Corp.
>> Bus 004 Device 004: ID 0bda:8153 Realtek Semiconductor Corp.
>> 
>> Ethernet cannot be detected after plugging ethernet cable to the dock,
>> 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
>> 
>> At this point the SMSC hub (usb 4-1) enters into compliance mode
>> (USB_SS_PORT_LS_COMP_MOD), and USB core tries to warm-reset it,
>> 
>> [  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
>> ...
>> 
>> However the warm-reset never success, the asserted PCI PME keeps the
>> runtime-resume, warm-reset and runtime-suspend loop which never bring it back
>> and causing spurious interrupts floods.
>> 
>> After some trial and errors, the issue goes away if LPM on the SMSC hub
>> is disabled. Digging further, enabling and disabling LPM during runtime
>> resume and runtime suspend respectively can solve the issue.
>> 
>> So bring back the old LPM behavior as a quirk and use it for the SMSC
>> hub to solve the issue.
>> 
>> Fixes: d590c2311150 ("usb: Avoid unnecessary LPM enabling and disabling during suspend and resume")
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> ---
>> v3:
>> - Add forgotten patch revision changelog.
>> 
>> v2:
>> - Explained by Alan, the hub should properly handle U3 -> U0 transition.
>>  So use a quirk to target this buggy device only.
>> 
>> Documentation/admin-guide/kernel-parameters.txt |  3 +++
>> drivers/usb/core/hub.c                          | 15 +++++++++++++++
>> drivers/usb/core/quirks.c                       |  6 ++++++
>> include/linux/usb/quirks.h                      |  3 +++
>> 4 files changed, 27 insertions(+)
> 
> Mathias may want to try something different to fix this problem.  But
> if he doesn't, this patch is okay with me.
> 
> Acked-by: Alan Stern <stern@rowland.harvard.edu>

If there's no objection, can we merge this patch?

Kai-Heng. 

> 
> Alan Stern


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] usb: Add a new quirk to let buggy hub enable and disable LPM during suspend and resume
  2019-10-17  6:33       ` Kai-Heng Feng
@ 2019-10-18 18:59         ` Greg Kroah-Hartman
  2019-10-21 13:59           ` Mathias Nyman
  0 siblings, 1 reply; 13+ messages in thread
From: Greg Kroah-Hartman @ 2019-10-18 18:59 UTC (permalink / raw)
  To: Kai-Heng Feng
  Cc: Alan Stern, Mathias Nyman, USB list, Kernel development list

On Thu, Oct 17, 2019 at 02:33:00PM +0800, Kai-Heng Feng wrote:
> 
> 
> > On Oct 4, 2019, at 03:04, Alan Stern <stern@rowland.harvard.edu> wrote:
> > 
> > On Fri, 4 Oct 2019, Kai-Heng Feng wrote:
> > 
> >> Dell WD15 dock has a topology like this:
> >> /:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 10000M
> >>    |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/7p, 5000M
> >>            |__ Port 2: Dev 3, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M
> >> 
> >> Their IDs:
> >> Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> >> Bus 004 Device 002: ID 0424:5537 Standard Microsystems Corp.
> >> Bus 004 Device 004: ID 0bda:8153 Realtek Semiconductor Corp.
> >> 
> >> Ethernet cannot be detected after plugging ethernet cable to the dock,
> >> 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
> >> 
> >> At this point the SMSC hub (usb 4-1) enters into compliance mode
> >> (USB_SS_PORT_LS_COMP_MOD), and USB core tries to warm-reset it,
> >> 
> >> [  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
> >> ...
> >> 
> >> However the warm-reset never success, the asserted PCI PME keeps the
> >> runtime-resume, warm-reset and runtime-suspend loop which never bring it back
> >> and causing spurious interrupts floods.
> >> 
> >> After some trial and errors, the issue goes away if LPM on the SMSC hub
> >> is disabled. Digging further, enabling and disabling LPM during runtime
> >> resume and runtime suspend respectively can solve the issue.
> >> 
> >> So bring back the old LPM behavior as a quirk and use it for the SMSC
> >> hub to solve the issue.
> >> 
> >> Fixes: d590c2311150 ("usb: Avoid unnecessary LPM enabling and disabling during suspend and resume")
> >> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> >> ---
> >> v3:
> >> - Add forgotten patch revision changelog.
> >> 
> >> v2:
> >> - Explained by Alan, the hub should properly handle U3 -> U0 transition.
> >>  So use a quirk to target this buggy device only.
> >> 
> >> Documentation/admin-guide/kernel-parameters.txt |  3 +++
> >> drivers/usb/core/hub.c                          | 15 +++++++++++++++
> >> drivers/usb/core/quirks.c                       |  6 ++++++
> >> include/linux/usb/quirks.h                      |  3 +++
> >> 4 files changed, 27 insertions(+)
> > 
> > Mathias may want to try something different to fix this problem.  But
> > if he doesn't, this patch is okay with me.
> > 
> > Acked-by: Alan Stern <stern@rowland.harvard.edu>
> 
> If there's no objection, can we merge this patch?

I wanted to have Mathias weigh in on this before merging it...

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] usb: Add a new quirk to let buggy hub enable and disable LPM during suspend and resume
  2019-10-18 18:59         ` Greg Kroah-Hartman
@ 2019-10-21 13:59           ` Mathias Nyman
  2019-10-21 18:40             ` Kai-Heng Feng
  0 siblings, 1 reply; 13+ messages in thread
From: Mathias Nyman @ 2019-10-21 13:59 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Kai-Heng Feng
  Cc: Alan Stern, Mathias Nyman, USB list, Kernel development list

On 18.10.2019 21.59, Greg Kroah-Hartman wrote:
> On Thu, Oct 17, 2019 at 02:33:00PM +0800, Kai-Heng Feng wrote:
>>
>>
>>> On Oct 4, 2019, at 03:04, Alan Stern <stern@rowland.harvard.edu> wrote:
>>>
>>> On Fri, 4 Oct 2019, Kai-Heng Feng wrote:
>>>
>>>> Dell WD15 dock has a topology like this:
>>>> /:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 10000M
>>>>     |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/7p, 5000M
>>>>             |__ Port 2: Dev 3, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M
>>>>
>>>> Their IDs:
>>>> Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
>>>> Bus 004 Device 002: ID 0424:5537 Standard Microsystems Corp.
>>>> Bus 004 Device 004: ID 0bda:8153 Realtek Semiconductor Corp.
>>>>
>>>> Ethernet cannot be detected after plugging ethernet cable to the dock,
>>>> 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
>>>>
>>>> At this point the SMSC hub (usb 4-1) enters into compliance mode
>>>> (USB_SS_PORT_LS_COMP_MOD), and USB core tries to warm-reset it,
>>>>
>>>> [  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
>>>> ...
>>>>
>>>> However the warm-reset never success, the asserted PCI PME keeps the
>>>> runtime-resume, warm-reset and runtime-suspend loop which never bring it back
>>>> and causing spurious interrupts floods.
>>>>
>>>> After some trial and errors, the issue goes away if LPM on the SMSC hub
>>>> is disabled. Digging further, enabling and disabling LPM during runtime
>>>> resume and runtime suspend respectively can solve the issue.
>>>>
>>>> So bring back the old LPM behavior as a quirk and use it for the SMSC
>>>> hub to solve the issue.
>>>>
>>>> Fixes: d590c2311150 ("usb: Avoid unnecessary LPM enabling and disabling during suspend and resume")
>>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>>>> ---
>>>> v3:
>>>> - Add forgotten patch revision changelog.
>>>>
>>>> v2:
>>>> - Explained by Alan, the hub should properly handle U3 -> U0 transition.
>>>>   So use a quirk to target this buggy device only.
>>>>
>>>> Documentation/admin-guide/kernel-parameters.txt |  3 +++
>>>> drivers/usb/core/hub.c                          | 15 +++++++++++++++
>>>> drivers/usb/core/quirks.c                       |  6 ++++++
>>>> include/linux/usb/quirks.h                      |  3 +++
>>>> 4 files changed, 27 insertions(+)
>>>
>>> Mathias may want to try something different to fix this problem.  But
>>> if he doesn't, this patch is okay with me.
>>>
>>> Acked-by: Alan Stern <stern@rowland.harvard.edu>
>>
>> If there's no objection, can we merge this patch?
> 
> I wanted to have Mathias weigh in on this before merging it...
> 

This might need some closer inspection still.

The "Get port status 4-1 read: 0x10202e" means port is not really in compliance mode,
instead port has CAS (Cold Attach Status) bit set, meaning parts of xHC needed for
link training were probably still powered off when device was plugged in, so device failed
to reach a connected, enabled, U0: link state. I needs to be warm reset.

there is no CAS link state in USB3 spec, so xhci driver reports a compliance mode link state
to usb core instead. Both states are resolved by a warm reset.

But looks like warm reset is refused as usb device state is still "suspended" in software:
"usb 4-1: device reset not allowed in state 8"

-Mathias

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] usb: Add a new quirk to let buggy hub enable and disable LPM during suspend and resume
  2019-10-21 13:59           ` Mathias Nyman
@ 2019-10-21 18:40             ` Kai-Heng Feng
  0 siblings, 0 replies; 13+ messages in thread
From: Kai-Heng Feng @ 2019-10-21 18:40 UTC (permalink / raw)
  To: Mathias Nyman
  Cc: Greg Kroah-Hartman, Alan Stern, Mathias Nyman, USB list,
	Kernel development list



> On Oct 21, 2019, at 21:59, Mathias Nyman <mathias.nyman@linux.intel.com> wrote:
> 
> On 18.10.2019 21.59, Greg Kroah-Hartman wrote:
>> On Thu, Oct 17, 2019 at 02:33:00PM +0800, Kai-Heng Feng wrote:
>>> 
>>> 
>>>> On Oct 4, 2019, at 03:04, Alan Stern <stern@rowland.harvard.edu> wrote:
>>>> 
>>>> On Fri, 4 Oct 2019, Kai-Heng Feng wrote:
>>>> 
>>>>> Dell WD15 dock has a topology like this:
>>>>> /:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 10000M
>>>>>    |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/7p, 5000M
>>>>>            |__ Port 2: Dev 3, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M
>>>>> 
>>>>> Their IDs:
>>>>> Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
>>>>> Bus 004 Device 002: ID 0424:5537 Standard Microsystems Corp.
>>>>> Bus 004 Device 004: ID 0bda:8153 Realtek Semiconductor Corp.
>>>>> 
>>>>> Ethernet cannot be detected after plugging ethernet cable to the dock,
>>>>> 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
>>>>> 
>>>>> At this point the SMSC hub (usb 4-1) enters into compliance mode
>>>>> (USB_SS_PORT_LS_COMP_MOD), and USB core tries to warm-reset it,
>>>>> 
>>>>> [  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
>>>>> ...
>>>>> 
>>>>> However the warm-reset never success, the asserted PCI PME keeps the
>>>>> runtime-resume, warm-reset and runtime-suspend loop which never bring it back
>>>>> and causing spurious interrupts floods.
>>>>> 
>>>>> After some trial and errors, the issue goes away if LPM on the SMSC hub
>>>>> is disabled. Digging further, enabling and disabling LPM during runtime
>>>>> resume and runtime suspend respectively can solve the issue.
>>>>> 
>>>>> So bring back the old LPM behavior as a quirk and use it for the SMSC
>>>>> hub to solve the issue.
>>>>> 
>>>>> Fixes: d590c2311150 ("usb: Avoid unnecessary LPM enabling and disabling during suspend and resume")
>>>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>>>>> ---
>>>>> v3:
>>>>> - Add forgotten patch revision changelog.
>>>>> 
>>>>> v2:
>>>>> - Explained by Alan, the hub should properly handle U3 -> U0 transition.
>>>>>  So use a quirk to target this buggy device only.
>>>>> 
>>>>> Documentation/admin-guide/kernel-parameters.txt |  3 +++
>>>>> drivers/usb/core/hub.c                          | 15 +++++++++++++++
>>>>> drivers/usb/core/quirks.c                       |  6 ++++++
>>>>> include/linux/usb/quirks.h                      |  3 +++
>>>>> 4 files changed, 27 insertions(+)
>>>> 
>>>> Mathias may want to try something different to fix this problem.  But
>>>> if he doesn't, this patch is okay with me.
>>>> 
>>>> Acked-by: Alan Stern <stern@rowland.harvard.edu>
>>> 
>>> If there's no objection, can we merge this patch?
>> I wanted to have Mathias weigh in on this before merging it...
> 
> This might need some closer inspection still.
> 
> The "Get port status 4-1 read: 0x10202e" means port is not really in compliance mode,
> instead port has CAS (Cold Attach Status) bit set, meaning parts of xHC needed for
> link training were probably still powered off when device was plugged in, so device failed
> to reach a connected, enabled, U0: link state. I needs to be warm reset.

[  433.315239] xhci_hcd 0000:3a:00.0: Get port status 4-1 read: 0x10202e2, return 0x10343
Ok, so we should check 0x10202e2 from xHC here, instead of 0x10343.

> 
> there is no CAS link state in USB3 spec, so xhci driver reports a compliance mode link state
> to usb core instead. Both states are resolved by a warm reset.
> 
> But looks like warm reset is refused as usb device state is still "suspended" in software:
> "usb 4-1: device reset not allowed in state 8"

Thanks for pointing this out. I'll see what's really going on here.

Kai-Heng

> 
> -Mathias


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2019-10-21 18:40 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-02 15:15 [PATCH] Revert "usb: Avoid unnecessary LPM enabling and disabling during suspend and resume" Kai-Heng Feng
2019-10-02 15:47 ` Alan Stern
2019-10-03 11:55   ` Kai-Heng Feng
2019-10-03 14:26     ` Alan Stern
2019-10-03 14:57       ` Kai-Heng Feng
2019-10-03 15:56 ` [PATCH v2] usb: Add a new quirk to let buggy hub enable and disable LPM during suspend and resume Kai-Heng Feng
2019-10-03 16:02   ` Greg KH
2019-10-03 17:24   ` [PATCH v3] " Kai-Heng Feng
2019-10-03 19:04     ` Alan Stern
2019-10-17  6:33       ` Kai-Heng Feng
2019-10-18 18:59         ` Greg Kroah-Hartman
2019-10-21 13:59           ` Mathias Nyman
2019-10-21 18:40             ` Kai-Heng Feng

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).