* [PATCH v3 0/2] USB: propose a generic fix for PORT_SUSPEND set feature timeout @ 2021-05-13 4:14 chris.chiu 2021-05-13 4:14 ` [PATCH v3 1/2] USB: Verify the port status when timeout happens during port suspend chris.chiu 2021-05-13 4:14 ` [PATCH v3 2/2] Revert "USB: Add reset-resume quirk for WD19's Realtek Hub" chris.chiu 0 siblings, 2 replies; 5+ messages in thread From: chris.chiu @ 2021-05-13 4:14 UTC (permalink / raw) To: stern, gregkh, m.v.b, hadess; +Cc: linux-usb, linux-kernel, Chris Chiu From: Chris Chiu <chris.chiu@canonical.com> For the Realtek Hub which fails to resume the port which has wakeup enable descendants, trying to propose a more generic and better fix to have the runtime suspend/resume work instead of a reset-resume quirk. Chris Chiu (2): USB: Verify the port status when timeout happens during port suspend Revert "USB: Add reset-resume quirk for WD19's Realtek Hub" drivers/usb/core/hub.c | 16 ++++++++++++++++ drivers/usb/core/quirks.c | 1 - 2 files changed, 16 insertions(+), 1 deletion(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 1/2] USB: Verify the port status when timeout happens during port suspend 2021-05-13 4:14 [PATCH v3 0/2] USB: propose a generic fix for PORT_SUSPEND set feature timeout chris.chiu @ 2021-05-13 4:14 ` chris.chiu 2021-05-13 14:35 ` Alan Stern 2021-05-13 4:14 ` [PATCH v3 2/2] Revert "USB: Add reset-resume quirk for WD19's Realtek Hub" chris.chiu 1 sibling, 1 reply; 5+ messages in thread From: chris.chiu @ 2021-05-13 4:14 UTC (permalink / raw) To: stern, gregkh, m.v.b, hadess; +Cc: linux-usb, linux-kernel, Chris Chiu From: Chris Chiu <chris.chiu@canonical.com> On the Realtek high-speed Hub(0bda:5487), the port which has wakeup enabled_descendants will sometimes timeout when setting PORT_SUSPEND feature. After checking the PORT_SUSPEND bit in wPortStatus, it is already set. However, the hub will fail to activate because the PORT_SUSPEND feature of that port is not cleared during resume. All connected devices are lost after resume. Check the port status to verify whether it's really suspended when timeout happpens. If yes, mark it as suspended so the device can be resumed correctly. Signed-off-by: Chris Chiu <chris.chiu@canonical.com> --- Changelog: v3: - create a new goto target for the timeout case instead of reset_resume - Revise the commit title/message because reset_resume is not required. v2: - create a new variable to keep the result of hub_port_status when suspend timeout. drivers/usb/core/hub.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index b2bc4b7c4289..c5d64175eaa9 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -3385,6 +3385,21 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg) status = 0; } if (status) { + if (status == -ETIMEDOUT) { + u16 portstatus, portchange; + + int ret = hub_port_status(hub, port1, &portstatus, + &portchange); + + dev_dbg(&port_dev->dev, + "suspend timeout, status %04x\n", portstatus); + + if (ret == 0 && port_is_suspended(hub, portstatus)) { + status = 0; + goto suspend_done; + } + } + dev_dbg(&port_dev->dev, "can't suspend, status %d\n", status); /* Try to enable USB3 LTM again */ @@ -3401,6 +3416,7 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg) if (!PMSG_IS_AUTO(msg)) status = 0; } else { + suspend_done: dev_dbg(&udev->dev, "usb %ssuspend, wakeup %d\n", (PMSG_IS_AUTO(msg) ? "auto-" : ""), udev->do_remote_wakeup); -- 2.20.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/2] USB: Verify the port status when timeout happens during port suspend 2021-05-13 4:14 ` [PATCH v3 1/2] USB: Verify the port status when timeout happens during port suspend chris.chiu @ 2021-05-13 14:35 ` Alan Stern 0 siblings, 0 replies; 5+ messages in thread From: Alan Stern @ 2021-05-13 14:35 UTC (permalink / raw) To: chris.chiu; +Cc: gregkh, m.v.b, hadess, linux-usb, linux-kernel On Thu, May 13, 2021 at 12:14:45PM +0800, chris.chiu@canonical.com wrote: > From: Chris Chiu <chris.chiu@canonical.com> > > On the Realtek high-speed Hub(0bda:5487), the port which has wakeup > enabled_descendants will sometimes timeout when setting PORT_SUSPEND > feature. After checking the PORT_SUSPEND bit in wPortStatus, it is > already set. However, the hub will fail to activate because the > PORT_SUSPEND feature of that port is not cleared during resume. All > connected devices are lost after resume. The last two sentences of this paragraph are now inaccurate. Please fix them up to match the current patch version. > Check the port status to verify whether it's really suspended when > timeout happpens. If yes, mark it as suspended so the device can > be resumed correctly. > > Signed-off-by: Chris Chiu <chris.chiu@canonical.com> > --- > > Changelog: > v3: > - create a new goto target for the timeout case instead of > reset_resume > - Revise the commit title/message because reset_resume is not > required. > v2: > - create a new variable to keep the result of hub_port_status > when suspend timeout. > > drivers/usb/core/hub.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > index b2bc4b7c4289..c5d64175eaa9 100644 > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -3385,6 +3385,21 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg) > status = 0; > } > if (status) { > + if (status == -ETIMEDOUT) { > + u16 portstatus, portchange; > + Extra blank line. > + int ret = hub_port_status(hub, port1, &portstatus, > + &portchange); > + > + dev_dbg(&port_dev->dev, > + "suspend timeout, status %04x\n", portstatus); The portstatus value shouldn't be printed if ret < 0, because it won't be initialized. If you want, initialize portstatus to 0 in the declaration. Also, there should be a comment here explaining why this code is needed. It should say pretty the same thing as the patch description, but more briefly (two sentences should be sufficient). Alan Stern > + > + if (ret == 0 && port_is_suspended(hub, portstatus)) { > + status = 0; > + goto suspend_done; > + } > + } > + > dev_dbg(&port_dev->dev, "can't suspend, status %d\n", status); > > /* Try to enable USB3 LTM again */ > @@ -3401,6 +3416,7 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg) > if (!PMSG_IS_AUTO(msg)) > status = 0; > } else { > + suspend_done: > dev_dbg(&udev->dev, "usb %ssuspend, wakeup %d\n", > (PMSG_IS_AUTO(msg) ? "auto-" : ""), > udev->do_remote_wakeup); > -- > 2.20.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 2/2] Revert "USB: Add reset-resume quirk for WD19's Realtek Hub" 2021-05-13 4:14 [PATCH v3 0/2] USB: propose a generic fix for PORT_SUSPEND set feature timeout chris.chiu 2021-05-13 4:14 ` [PATCH v3 1/2] USB: Verify the port status when timeout happens during port suspend chris.chiu @ 2021-05-13 4:14 ` chris.chiu 2021-05-13 14:40 ` Alan Stern 1 sibling, 1 reply; 5+ messages in thread From: chris.chiu @ 2021-05-13 4:14 UTC (permalink / raw) To: stern, gregkh, m.v.b, hadess; +Cc: linux-usb, linux-kernel, Chris Chiu From: Chris Chiu <chris.chiu@canonical.com> This reverts commit ca91fd8c7643d93bfc18a6fec1a0d3972a46a18a. The problematic hub should be taken care for each setting PORT_SUSPEND feature timeout instead of reset-resume all the time. Signed-off-by: Chris Chiu <chris.chiu@canonical.com> --- drivers/usb/core/quirks.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c index 21e7522655ac..6114cf83bb44 100644 --- a/drivers/usb/core/quirks.c +++ b/drivers/usb/core/quirks.c @@ -406,7 +406,6 @@ static const struct usb_device_id usb_quirk_list[] = { /* Realtek hub in Dell WD19 (Type-C) */ { USB_DEVICE(0x0bda, 0x0487), .driver_info = USB_QUIRK_NO_LPM }, - { USB_DEVICE(0x0bda, 0x5487), .driver_info = USB_QUIRK_RESET_RESUME }, /* Generic RTL8153 based ethernet adapters */ { USB_DEVICE(0x0bda, 0x8153), .driver_info = USB_QUIRK_NO_LPM }, -- 2.20.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 2/2] Revert "USB: Add reset-resume quirk for WD19's Realtek Hub" 2021-05-13 4:14 ` [PATCH v3 2/2] Revert "USB: Add reset-resume quirk for WD19's Realtek Hub" chris.chiu @ 2021-05-13 14:40 ` Alan Stern 0 siblings, 0 replies; 5+ messages in thread From: Alan Stern @ 2021-05-13 14:40 UTC (permalink / raw) To: chris.chiu; +Cc: gregkh, m.v.b, hadess, linux-usb, linux-kernel On Thu, May 13, 2021 at 12:14:46PM +0800, chris.chiu@canonical.com wrote: > From: Chris Chiu <chris.chiu@canonical.com> > > This reverts commit ca91fd8c7643d93bfc18a6fec1a0d3972a46a18a. The Use the proper format for referring to commits: just the first 12 hex digits of the commit ID, followed by the commit's name in parentheses and quotation marks. See other patch submissions in the mailing list archives for examples. > problematic hub should be taken care for each setting PORT_SUSPEND > feature timeout instead of reset-resume all the time. The last sentence is not grammatical. It should say something more like: The previous patch in this series now handles problematic hubs by checking the port status when a PORT_SUSPEND timeout occurs, so we don't need to use reset-resume all the time. Alan Stern > > Signed-off-by: Chris Chiu <chris.chiu@canonical.com> > --- > drivers/usb/core/quirks.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c > index 21e7522655ac..6114cf83bb44 100644 > --- a/drivers/usb/core/quirks.c > +++ b/drivers/usb/core/quirks.c > @@ -406,7 +406,6 @@ static const struct usb_device_id usb_quirk_list[] = { > > /* Realtek hub in Dell WD19 (Type-C) */ > { USB_DEVICE(0x0bda, 0x0487), .driver_info = USB_QUIRK_NO_LPM }, > - { USB_DEVICE(0x0bda, 0x5487), .driver_info = USB_QUIRK_RESET_RESUME }, > > /* Generic RTL8153 based ethernet adapters */ > { USB_DEVICE(0x0bda, 0x8153), .driver_info = USB_QUIRK_NO_LPM }, > -- > 2.20.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-05-13 14:40 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-13 4:14 [PATCH v3 0/2] USB: propose a generic fix for PORT_SUSPEND set feature timeout chris.chiu 2021-05-13 4:14 ` [PATCH v3 1/2] USB: Verify the port status when timeout happens during port suspend chris.chiu 2021-05-13 14:35 ` Alan Stern 2021-05-13 4:14 ` [PATCH v3 2/2] Revert "USB: Add reset-resume quirk for WD19's Realtek Hub" chris.chiu 2021-05-13 14:40 ` Alan Stern
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).