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@xxxxxxxxxx
> > Signed-off-by: Xu Yang <xu.yang_2@xxxxxxx>
> > ---
> >  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