From: Alan Stern <stern@rowland.harvard.edu>
To: Keiya Nobuta <nobuta.keiya@fujitsu.com>
Cc: gregkh@linuxfoundation.org, <linux-usb@vger.kernel.org>
Subject: Re: [PATCH] usb: core: hub: Improved device recognition on remote wakeup
Date: Thu, 9 Jan 2020 10:22:00 -0500 (EST) [thread overview]
Message-ID: <Pine.LNX.4.44L0.2001091019520.1614-100000@iolanthe.rowland.org> (raw)
In-Reply-To: <20200109051448.28150-1-nobuta.keiya@fujitsu.com>
On Thu, 9 Jan 2020, Keiya Nobuta wrote:
> If hub_activate() is called before D+ has stabilized after remote
> wakeup, the following situation might occur:
>
> __ ___________________
> / \ /
> D+ __/ \__/
>
> Hub _______________________________
> | ^ ^ ^
> | | | |
> Host _____v__|___|___________|______
> | | | |
> | | | \-- Interrupt Transfer (*3)
> | | \-- ClearPortFeature (*2)
> | \-- GetPortStatus (*1)
> \-- Host detects remote wakeup
>
> - D+ goes high, Host starts running by remote wakeup
> - D+ is not stable, goes low
> - Host requests GetPortStatus at (*1) and gets the following hub status:
> - Current Connect Status bit is 0
> - Connect Status Change bit is 1
> - D+ stabilizes, goes high
> - Host requests ClearPortFeature and thus Connect Status Change bit is
> cleared at (*2)
> - After waiting 100 ms, Host starts the Interrupt Transfer at (*3)
> - Since the Connect Status Change bit is 0, Hub returns NAK.
>
> In this case, port_event() is not called in hub_event() and Host cannot
> recognize device. To solve this issue, flag change_bits even if only
> Connect Status Change bit is 1 when got in the first GetPortStatus.
>
> This issue occurs rarely because it only if D+ changes during a very
> short time between GetPortStatus and ClearPortFeature. However, it is
> fatal if it occurs in embedded system.
>
> Signed-off-by: Keiya Nobuta <nobuta.keiya@fujitsu.com>
> ---
> drivers/usb/core/hub.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index f229ad6..77f8eb1 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -1192,6 +1192,7 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
> * PORT_OVER_CURRENT is not. So check for any of them.
> */
> if (udev || (portstatus & USB_PORT_STAT_CONNECTION) ||
> + (portchange & USB_PORT_STAT_C_CONNECTION) ||
> (portstatus & USB_PORT_STAT_OVERCURRENT) ||
> (portchange & USB_PORT_STAT_C_OVERCURRENT))
> set_bit(port1, hub->change_bits);
I would have added the new test one line lower, so that all the
portstatus checks were adjacent and all the portchange checks were
adjacent. But that's a very small thing...
Acked-by: Alan Stern <stern@rowland.harvard.edu>
next prev parent reply other threads:[~2020-01-09 15:22 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-09 5:14 [PATCH] usb: core: hub: Improved device recognition on remote wakeup Keiya Nobuta
2020-01-09 15:22 ` Alan Stern [this message]
2020-01-09 15:46 ` Greg KH
2020-01-10 0:48 ` nobuta.keiya
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=Pine.LNX.4.44L0.2001091019520.1614-100000@iolanthe.rowland.org \
--to=stern@rowland.harvard.edu \
--cc=gregkh@linuxfoundation.org \
--cc=linux-usb@vger.kernel.org \
--cc=nobuta.keiya@fujitsu.com \
/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).