From: Chris Chiu <chris.chiu@canonical.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Greg KH <gregkh@linuxfoundation.org>,
m.v.b@runbox.com, hadess@hadess.net, linux-usb@vger.kernel.org,
Linux Kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 1/2] USB: reset-resume the device when PORT_SUSPEND is set but timeout
Date: Tue, 11 May 2021 13:04:36 +0800 [thread overview]
Message-ID: <CABTNMG24BOS_+uGEYFLJrmEW10eTGjrOowwS7Zb_U+AvkZWTyQ@mail.gmail.com> (raw)
In-Reply-To: <20210510150203.GD863718@rowland.harvard.edu>
On Mon, May 10, 2021 at 11:02 PM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Mon, May 10, 2021 at 10:50:29PM +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.
> >
> > This commit force reset-resume the device connected to the timeout
> > but suspended port so that the hub will have chance to clear the
> > PORT_SUSPEND feature during resume.
>
> Are you certain that the reset-resume is needed? What happens if you
> leave out the line that sets udev->reset_resume? The rest of the patch
> will cause the kernel to realize that the port really is suspended, so
> maybe the suspend feature will get cleared properly during resume.
>
> It's worthwhile to try the experiement and see what happens.
>
> Alan Stern
>
If I leave out the udev->reset_resume set, the resume will fail. Please refer
to the following kernel log. The usb 1-1 is the hub which has wakeup enabled
descendants.
[ 57.210472] usb 1-1: kworker/u32:7 timed out on ep0out len=0/0
[ 57.211022] usb 1-1-port3: suspend timeout, status 0507
[ 57.211130] hub 1-1:1.0: hub_suspend
[ 57.230500] usb 1-1: usb suspend, wakeup 0
The timeout happens at 57.210472 and you can see the PORT_SUSPEND
bit is actually set in the "status 0507". The following shows the resume log.
[ 58.046556] usb 1-1: usb resume
[ 58.114515] usb 1-1: Waited 0ms for CONNECT
[ 58.114524] usb 1-1: finish resume
[ 58.114928] hub 1-1:1.0: hub_resume
[ 58.116035] usb 1-1-port3: status 0507 change 0000
[ 58.116720] usb 1-1-port5: status 0503 change 0000
[ 58.116778] hub 1-1.3:1.0: hub_resume
[ 58.116908] hub 1-1.3:1.0: hub_ext_port_status failed (err = -71)
[ 58.116952] usb 1-1.5: Waited 0ms for CONNECT
[ 58.116955] usb 1-1.5: finish resume
[ 58.117157] hub 1-1.3:1.0: hub_ext_port_status failed (err = -71)
[ 58.117397] usb 1-1.3-port5: can't resume, status -71
[ 58.117782] hub 1-1.3:1.0: hub_ext_port_status failed (err = -71)
[ 58.118147] usb 1-1.3-port2: can't resume, status -71
[ 58.118149] usb 1-1.3.2: Waited 0ms for CONNECT
[ 58.118151] usb 1-1.3-port2: status 07eb.906e after resume, -19
[ 58.118153] usb 1-1.3.2: can't resume, status -19
[ 58.118154] usb 1-1.3-port2: logical disconnect
[ 58.118526] usb 1-1.3-port2: cannot disable (err = -71)
As you see in the 58.116035, the hub_resume and activate is OK for the
usb 1-1. The "usb 1-1.3: finish resume" is not in the log because it's not
considered suspended and no chance to ClearPortFeature. Then it fails
the subsequent hub 1-1.3 resume and active because the usb 1-1.3 happens
to be a downstream hub. So this is why we need at least udev->reset_resume
to give this kind of timeout port/device a chance to clear port feature and
come back from an unknown state.
Chris
> > Signed-off-by: Chris Chiu <chris.chiu@canonical.com>
> > ---
> >
> > Changelog:
> > v2:
> > - create a new variable to keep the result of hub_port_status
> > when suspend timeout.
> >
> > drivers/usb/core/hub.c | 15 +++++++++++++++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index b2bc4b7c4289..3c823544e425 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)) {
> > + udev->reset_resume = 1;
> > + goto err_wakeup;
> > + }
> > + }
> > +
> > dev_dbg(&port_dev->dev, "can't suspend, status %d\n", status);
> >
> > /* Try to enable USB3 LTM again */
> > --
> > 2.20.1
> >
next prev parent reply other threads:[~2021-05-11 5:04 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-10 14:50 [PATCH v2 0/2] USB: propose a generic fix for PORT_SUSPEND set feature timeout chris.chiu
2021-05-10 14:50 ` [PATCH v2 1/2] USB: reset-resume the device when PORT_SUSPEND is set but timeout chris.chiu
2021-05-10 15:02 ` Alan Stern
2021-05-11 5:04 ` Chris Chiu [this message]
2021-05-11 16:30 ` Alan Stern
2021-05-12 4:17 ` Chris Chiu
2021-05-12 15:04 ` Alan Stern
2021-05-13 4:21 ` Chris Chiu
2021-05-13 14:41 ` Alan Stern
2021-05-10 14:50 ` [PATCH v2 2/2] Revert "USB: Add reset-resume quirk for WD19's Realtek Hub" chris.chiu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CABTNMG24BOS_+uGEYFLJrmEW10eTGjrOowwS7Zb_U+AvkZWTyQ@mail.gmail.com \
--to=chris.chiu@canonical.com \
--cc=gregkh@linuxfoundation.org \
--cc=hadess@hadess.net \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=m.v.b@runbox.com \
--cc=stern@rowland.harvard.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).