[PATCH] usb: port: add delay after usb_hub_set_port_power()

[PATCH] usb: port: add delay after usb_hub_set_port_power()

[Xu Yang]

When disable the root hub port, somehow the device is disconnected and
re-connected again. This happens because usb_clear_port_feature() does not
clear a truly happened port change. That says, in fact, port change event
may happen after usb_clear_port_feature() is called. Then the subsequent
port change event will have impact on usb device suspend routine.

Below log shows what is happening:

$ echo 1 > usb1-port1/disable
[   37.958239] usb 1-1: USB disconnect, device number 2
[   37.964101] usb 1-1: unregistering device
[   37.970070] hub 1-0:1.0: hub_suspend
[   37.971305] hub 1-0:1.0: state 7 ports 1 chg 0000 evt 0002
[   37.974412] usb usb1: bus auto-suspend, wakeup 1
[   37.988175] usb usb1: suspend raced with wakeup event         <---
[   37.993947] usb usb1: usb auto-resume
[   37.998401] hub 1-0:1.0: hub_resume
[   38.105688] usb usb1-port1: status 0000, change 0000, 12 Mb/s
[   38.112399] hub 1-0:1.0: state 7 ports 1 chg 0000 evt 0000
[   38.118645] hub 1-0:1.0: hub_suspend
[   38.122963] usb usb1: bus auto-suspend, wakeup 1
[   38.200368] usb usb1: usb wakeup-resume
[   38.204982] usb usb1: usb auto-resume
[   38.209376] hub 1-0:1.0: hub_resume
[   38.213676] usb usb1-port1: status 0101 change 0001
[   38.321552] hub 1-0:1.0: state 7 ports 1 chg 0002 evt 0000
[   38.327978] usb usb1-port1: status 0101, change 0000, 12 Mb/s
[   38.457429] usb 1-1: new high-speed USB device number 3 using ci_hdrc

To fix the issue, add delay after usb_hub_set_port_power(). So port change
bit will be fixed to the final state and usb_clear_port_feature() can
correctly clear it after this period. This will also avoid usb runtime
suspend routine to run because usb_autopm_put_interface() not run yet.

Fixes: f061f43d7418 ("usb: hub: port: add sysfs entry to switch port power")
Cc: stable@kernel.org
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 drivers/usb/core/port.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index f54198171b6a..a47df5d32f7c 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -141,6 +141,7 @@ static ssize_t disable_store(struct device *dev, struct device_attribute *attr,
 		usb_disconnect(&port_dev->child);
 
 	rc = usb_hub_set_port_power(hdev, hub, port1, !disabled);
+	msleep(2 * hub_power_on_good_delay(hub));
 
 	if (disabled) {
 		usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_CONNECTION);
-- 
2.34.1

Re: [PATCH] usb: port: add delay after usb_hub_set_port_power()

[Greg KH]

On Wed, Feb 11, 2026 at 06:36:28PM +0800, Xu Yang wrote:
> When disable the root hub port, somehow the device is disconnected and
> re-connected again. This happens because usb_clear_port_feature() does not
> clear a truly happened port change. That says, in fact, port change event
> may happen after usb_clear_port_feature() is called. Then the subsequent
> port change event will have impact on usb device suspend routine.
> 
> Below log shows what is happening:
> 
> $ echo 1 > usb1-port1/disable
> [   37.958239] usb 1-1: USB disconnect, device number 2
> [   37.964101] usb 1-1: unregistering device
> [   37.970070] hub 1-0:1.0: hub_suspend
> [   37.971305] hub 1-0:1.0: state 7 ports 1 chg 0000 evt 0002
> [   37.974412] usb usb1: bus auto-suspend, wakeup 1
> [   37.988175] usb usb1: suspend raced with wakeup event         <---
> [   37.993947] usb usb1: usb auto-resume
> [   37.998401] hub 1-0:1.0: hub_resume
> [   38.105688] usb usb1-port1: status 0000, change 0000, 12 Mb/s
> [   38.112399] hub 1-0:1.0: state 7 ports 1 chg 0000 evt 0000
> [   38.118645] hub 1-0:1.0: hub_suspend
> [   38.122963] usb usb1: bus auto-suspend, wakeup 1
> [   38.200368] usb usb1: usb wakeup-resume
> [   38.204982] usb usb1: usb auto-resume
> [   38.209376] hub 1-0:1.0: hub_resume
> [   38.213676] usb usb1-port1: status 0101 change 0001
> [   38.321552] hub 1-0:1.0: state 7 ports 1 chg 0002 evt 0000
> [   38.327978] usb usb1-port1: status 0101, change 0000, 12 Mb/s
> [   38.457429] usb 1-1: new high-speed USB device number 3 using ci_hdrc
> 
> To fix the issue, add delay after usb_hub_set_port_power(). So port change
> bit will be fixed to the final state and usb_clear_port_feature() can
> correctly clear it after this period. This will also avoid usb runtime
> suspend routine to run because usb_autopm_put_interface() not run yet.
> 
> Fixes: f061f43d7418 ("usb: hub: port: add sysfs entry to switch port power")
> Cc: stable@kernel.org
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  drivers/usb/core/port.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> index f54198171b6a..a47df5d32f7c 100644
> --- a/drivers/usb/core/port.c
> +++ b/drivers/usb/core/port.c
> @@ -141,6 +141,7 @@ static ssize_t disable_store(struct device *dev, struct device_attribute *attr,
>  		usb_disconnect(&port_dev->child);
>  
>  	rc = usb_hub_set_port_power(hdev, hub, port1, !disabled);
> +	msleep(2 * hub_power_on_good_delay(hub));

This feels like a hack, and you are just getting lucky.  Why this
specific amount of time, what guarantees that this is ok?

And why are you disabling a root hub port at all?  How is that even
guaranteed to work?

thanks,

greg k-h

Re: [PATCH] usb: port: add delay after usb_hub_set_port_power()

[Alan Stern]

On Wed, Feb 11, 2026 at 06:36:28PM +0800, Xu Yang wrote:
> When disable the root hub port, somehow the device is disconnected and
> re-connected again. This happens because usb_clear_port_feature() does not
> clear a truly happened port change. That says, in fact, port change event
> may happen after usb_clear_port_feature() is called. Then the subsequent
> port change event will have impact on usb device suspend routine.

This is not a very good description of the problem.  Here's a better 
one:

When a port is disabled, an attached device will be disconnected.  This 
causes a port-status-change event, which will race with hub autosuspend 
(if the disabled port was the only connected port on its hub), causing 
an immediate resume and a second autosuspend.  Both of these can be 
avoided by adding a short delay after the call to 
usb_hub_set_port_power().

Alan Stern

> Below log shows what is happening:
> 
> $ echo 1 > usb1-port1/disable
> [   37.958239] usb 1-1: USB disconnect, device number 2
> [   37.964101] usb 1-1: unregistering device
> [   37.970070] hub 1-0:1.0: hub_suspend
> [   37.971305] hub 1-0:1.0: state 7 ports 1 chg 0000 evt 0002
> [   37.974412] usb usb1: bus auto-suspend, wakeup 1
> [   37.988175] usb usb1: suspend raced with wakeup event         <---
> [   37.993947] usb usb1: usb auto-resume
> [   37.998401] hub 1-0:1.0: hub_resume
> [   38.105688] usb usb1-port1: status 0000, change 0000, 12 Mb/s
> [   38.112399] hub 1-0:1.0: state 7 ports 1 chg 0000 evt 0000
> [   38.118645] hub 1-0:1.0: hub_suspend
> [   38.122963] usb usb1: bus auto-suspend, wakeup 1
> [   38.200368] usb usb1: usb wakeup-resume
> [   38.204982] usb usb1: usb auto-resume
> [   38.209376] hub 1-0:1.0: hub_resume
> [   38.213676] usb usb1-port1: status 0101 change 0001
> [   38.321552] hub 1-0:1.0: state 7 ports 1 chg 0002 evt 0000
> [   38.327978] usb usb1-port1: status 0101, change 0000, 12 Mb/s
> [   38.457429] usb 1-1: new high-speed USB device number 3 using ci_hdrc
> 
> To fix the issue, add delay after usb_hub_set_port_power(). So port change
> bit will be fixed to the final state and usb_clear_port_feature() can
> correctly clear it after this period. This will also avoid usb runtime
> suspend routine to run because usb_autopm_put_interface() not run yet.
> 
> Fixes: f061f43d7418 ("usb: hub: port: add sysfs entry to switch port power")
> Cc: stable@kernel.org
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  drivers/usb/core/port.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> index f54198171b6a..a47df5d32f7c 100644
> --- a/drivers/usb/core/port.c
> +++ b/drivers/usb/core/port.c
> @@ -141,6 +141,7 @@ static ssize_t disable_store(struct device *dev, struct device_attribute *attr,
>  		usb_disconnect(&port_dev->child);
>  
>  	rc = usb_hub_set_port_power(hdev, hub, port1, !disabled);
> +	msleep(2 * hub_power_on_good_delay(hub));
>  
>  	if (disabled) {
>  		usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_CONNECTION);
> -- 
> 2.34.1
> 

Re: [PATCH] usb: port: add delay after usb_hub_set_port_power()

[Greg KH]

On Wed, Feb 11, 2026 at 10:04:32AM -0500, Alan Stern wrote:
> On Wed, Feb 11, 2026 at 06:36:28PM +0800, Xu Yang wrote:
> > When disable the root hub port, somehow the device is disconnected and
> > re-connected again. This happens because usb_clear_port_feature() does not
> > clear a truly happened port change. That says, in fact, port change event
> > may happen after usb_clear_port_feature() is called. Then the subsequent
> > port change event will have impact on usb device suspend routine.
> 
> This is not a very good description of the problem.  Here's a better 
> one:
> 
> When a port is disabled, an attached device will be disconnected.  This 
> causes a port-status-change event, which will race with hub autosuspend 
> (if the disabled port was the only connected port on its hub), causing 
> an immediate resume and a second autosuspend.  Both of these can be 
> avoided by adding a short delay after the call to 
> usb_hub_set_port_power().

What guarantees that a "short delay" will solve this?  And how long of a
delay?  What guarantees that sleeping will not just reduce the race
window?

thanks,

greg k-h

Re: [PATCH] usb: port: add delay after usb_hub_set_port_power()

[Alan Stern]

On Wed, Feb 11, 2026 at 04:47:42PM +0100, Greg KH wrote:
> On Wed, Feb 11, 2026 at 10:04:32AM -0500, Alan Stern wrote:
> > On Wed, Feb 11, 2026 at 06:36:28PM +0800, Xu Yang wrote:
> > > When disable the root hub port, somehow the device is disconnected and
> > > re-connected again. This happens because usb_clear_port_feature() does not
> > > clear a truly happened port change. That says, in fact, port change event
> > > may happen after usb_clear_port_feature() is called. Then the subsequent
> > > port change event will have impact on usb device suspend routine.
> > 
> > This is not a very good description of the problem.  Here's a better 
> > one:
> > 
> > When a port is disabled, an attached device will be disconnected.  This 
> > causes a port-status-change event, which will race with hub autosuspend 
> > (if the disabled port was the only connected port on its hub), causing 
> > an immediate resume and a second autosuspend.  Both of these can be 
> > avoided by adding a short delay after the call to 
> > usb_hub_set_port_power().
> 
> What guarantees that a "short delay" will solve this?  And how long of a
> delay?  What guarantees that sleeping will not just reduce the race
> window?

There is no guarantee, but the probability is high.  The delay merely 
needs to be long enough for the hub driver's kernel thread to handle the 
port-status-change event.  For root hubs, this is interrupt-driven so it 
should happen pretty quickly (unless the thread is busy handling another 
device or root hub).  For external hubs, the delay won't be long enough.

And even if the delay doesn't work, we're no worse off than we are now 
-- you end up with an autosuspend-autoresume-autosuspend sequence in 
quick succession, where the first two aren't necessary but do no real 
harm.

Another way to avoid the problem would be to set the root hub's 
autosuspend delay to a few milliseconds.  But we set the delay to 0 in 
commit 596d789a211d ("USB: set hub's default autosuspend delay as 0") 
for what seemed to be good reasons.

Alan Stern

Re: [PATCH] usb: port: add delay after usb_hub_set_port_power()

[Xu Yang]

On Wed, Feb 11, 2026 at 12:19:38PM +0100, Greg KH wrote:
> On Wed, Feb 11, 2026 at 06:36:28PM +0800, Xu Yang wrote:
> > When disable the root hub port, somehow the device is disconnected and
> > re-connected again. This happens because usb_clear_port_feature() does not
> > clear a truly happened port change. That says, in fact, port change event
> > may happen after usb_clear_port_feature() is called. Then the subsequent
> > port change event will have impact on usb device suspend routine.
> > 
> > Below log shows what is happening:
> > 
> > $ echo 1 > usb1-port1/disable
> > [   37.958239] usb 1-1: USB disconnect, device number 2
> > [   37.964101] usb 1-1: unregistering device
> > [   37.970070] hub 1-0:1.0: hub_suspend
> > [   37.971305] hub 1-0:1.0: state 7 ports 1 chg 0000 evt 0002
> > [   37.974412] usb usb1: bus auto-suspend, wakeup 1
> > [   37.988175] usb usb1: suspend raced with wakeup event         <---
> > [   37.993947] usb usb1: usb auto-resume
> > [   37.998401] hub 1-0:1.0: hub_resume
> > [   38.105688] usb usb1-port1: status 0000, change 0000, 12 Mb/s
> > [   38.112399] hub 1-0:1.0: state 7 ports 1 chg 0000 evt 0000
> > [   38.118645] hub 1-0:1.0: hub_suspend
> > [   38.122963] usb usb1: bus auto-suspend, wakeup 1
> > [   38.200368] usb usb1: usb wakeup-resume
> > [   38.204982] usb usb1: usb auto-resume
> > [   38.209376] hub 1-0:1.0: hub_resume
> > [   38.213676] usb usb1-port1: status 0101 change 0001
> > [   38.321552] hub 1-0:1.0: state 7 ports 1 chg 0002 evt 0000
> > [   38.327978] usb usb1-port1: status 0101, change 0000, 12 Mb/s
> > [   38.457429] usb 1-1: new high-speed USB device number 3 using ci_hdrc
> > 
> > To fix the issue, add delay after usb_hub_set_port_power(). So port change
> > bit will be fixed to the final state and usb_clear_port_feature() can
> > correctly clear it after this period. This will also avoid usb runtime
> > suspend routine to run because usb_autopm_put_interface() not run yet.
> > 
> > Fixes: f061f43d7418 ("usb: hub: port: add sysfs entry to switch port power")
> > Cc: stable@kernel.org
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
> >  drivers/usb/core/port.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> > index f54198171b6a..a47df5d32f7c 100644
> > --- a/drivers/usb/core/port.c
> > +++ b/drivers/usb/core/port.c
> > @@ -141,6 +141,7 @@ static ssize_t disable_store(struct device *dev, struct device_attribute *attr,
> >  		usb_disconnect(&port_dev->child);
> >  
> >  	rc = usb_hub_set_port_power(hdev, hub, port1, !disabled);
> > +	msleep(2 * hub_power_on_good_delay(hub));
> 
> This feels like a hack, and you are just getting lucky.  Why this
> specific amount of time, what guarantees that this is ok?

The max time should be the period that VBUS from on to off. The port
change event will happen during this period. I refer to hub.c#n5608
using msleep(2 * hub_power_on_good_delay(hub)) for this case too. But
for root hub port, I agree with Alan the delay won't need too long since
the host controller will trigger an interrupt soon.

> 
> And why are you disabling a root hub port at all?  How is that even
> guaranteed to work?

Like a common hub port, when the "disable" interface is exposed to the
user, we can't prevent the user use it. The reason may be to power off
the VBUS, to avoid devices be recognized, to debug, etc.

Thanks,
Xu Yang

Re: [PATCH] usb: port: add delay after usb_hub_set_port_power()

[Xu Yang]

On Wed, Feb 11, 2026 at 10:04:32AM -0500, Alan Stern wrote:
> On Wed, Feb 11, 2026 at 06:36:28PM +0800, Xu Yang wrote:
> > When disable the root hub port, somehow the device is disconnected and
> > re-connected again. This happens because usb_clear_port_feature() does not
> > clear a truly happened port change. That says, in fact, port change event
> > may happen after usb_clear_port_feature() is called. Then the subsequent
> > port change event will have impact on usb device suspend routine.
> 
> This is not a very good description of the problem.  Here's a better 
> one:
> 
> When a port is disabled, an attached device will be disconnected.  This 
> causes a port-status-change event, which will race with hub autosuspend 
> (if the disabled port was the only connected port on its hub), causing 
> an immediate resume and a second autosuspend.  Both of these can be 
> avoided by adding a short delay after the call to 
> usb_hub_set_port_power().

Thank you for providing a better one!

Thanks,
Xu Yang