[PATCH v5 1/4] usb: port: track the disabled state

Marco Felsch posted 4 patches 1 month, 1 week ago
[PATCH v5 1/4] usb: port: track the disabled state
Posted by Marco Felsch 1 month, 1 week ago
The disable state isn't tracked at the moment, instead the state is
directly passed to the hub driver. Change this behavior to only trigger
the hub if a state change happened. Exit early in case of no state
changes but don't return an error.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/usb/core/hub.h  | 2 ++
 drivers/usb/core/port.c | 6 ++++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 9ebc5ef54a325d63e01b0deb59a1853d2b13c8d5..297adf2c6078809ca582104f228e5222c464f999 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -97,6 +97,7 @@ struct usb_hub {
  * @usb3_lpm_u2_permit: whether USB3 U2 LPM is permitted.
  * @early_stop: whether port initialization will be stopped earlier.
  * @ignore_event: whether events of the port are ignored.
+ * @disabled: whether the port is disabled
  */
 struct usb_port {
 	struct usb_device *child;
@@ -118,6 +119,7 @@ struct usb_port {
 	unsigned int is_superspeed:1;
 	unsigned int usb3_lpm_u1_permit:1;
 	unsigned int usb3_lpm_u2_permit:1;
+	unsigned int disabled:1;
 };
 
 #define to_usb_port(_dev) \
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 44e38f922bc553adee64b35c536dfd4154a42d8a..86e9d6d0c0f505782569565fde8e4a46b06b8b4d 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -117,6 +117,10 @@ static ssize_t disable_store(struct device *dev, struct device_attribute *attr,
 	if (rc)
 		return rc;
 
+	/* Early quit if no change was detected */
+	if (port_dev->disabled == disabled)
+		return count;
+
 	hub_get(hub);
 	rc = usb_autopm_get_interface(intf);
 	if (rc < 0)
@@ -148,6 +152,8 @@ static ssize_t disable_store(struct device *dev, struct device_attribute *attr,
 			usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE);
 	}
 
+	port_dev->disabled = disabled;
+
 	if (!rc)
 		rc = count;
 

-- 
2.47.3
Re: [PATCH v5 1/4] usb: port: track the disabled state
Posted by Greg Kroah-Hartman 3 weeks ago
On Mon, Feb 23, 2026 at 12:27:34PM +0100, Marco Felsch wrote:
> The disable state isn't tracked at the moment, instead the state is
> directly passed to the hub driver. Change this behavior to only trigger
> the hub if a state change happened. Exit early in case of no state
> changes but don't return an error.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  drivers/usb/core/hub.h  | 2 ++
>  drivers/usb/core/port.c | 6 ++++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
> index 9ebc5ef54a325d63e01b0deb59a1853d2b13c8d5..297adf2c6078809ca582104f228e5222c464f999 100644
> --- a/drivers/usb/core/hub.h
> +++ b/drivers/usb/core/hub.h
> @@ -97,6 +97,7 @@ struct usb_hub {
>   * @usb3_lpm_u2_permit: whether USB3 U2 LPM is permitted.
>   * @early_stop: whether port initialization will be stopped earlier.
>   * @ignore_event: whether events of the port are ignored.
> + * @disabled: whether the port is disabled
>   */
>  struct usb_port {
>  	struct usb_device *child;
> @@ -118,6 +119,7 @@ struct usb_port {
>  	unsigned int is_superspeed:1;
>  	unsigned int usb3_lpm_u1_permit:1;
>  	unsigned int usb3_lpm_u2_permit:1;
> +	unsigned int disabled:1;
>  };
>  
>  #define to_usb_port(_dev) \
> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> index 44e38f922bc553adee64b35c536dfd4154a42d8a..86e9d6d0c0f505782569565fde8e4a46b06b8b4d 100644
> --- a/drivers/usb/core/port.c
> +++ b/drivers/usb/core/port.c
> @@ -117,6 +117,10 @@ static ssize_t disable_store(struct device *dev, struct device_attribute *attr,
>  	if (rc)
>  		return rc;
>  
> +	/* Early quit if no change was detected */
> +	if (port_dev->disabled == disabled)
> +		return count;
> +

This will change behavior where someone tells the port to be enabled
again, when it already is.  Is that ok?

thanks,

greg k-h
Re: [PATCH v5 1/4] usb: port: track the disabled state
Posted by Marco Felsch 1 week, 5 days ago
On 26-03-11, Greg Kroah-Hartman wrote:
> On Mon, Feb 23, 2026 at 12:27:34PM +0100, Marco Felsch wrote:
> > The disable state isn't tracked at the moment, instead the state is
> > directly passed to the hub driver. Change this behavior to only trigger
> > the hub if a state change happened. Exit early in case of no state
> > changes but don't return an error.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

...

> >  #define to_usb_port(_dev) \
> > diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> > index 44e38f922bc553adee64b35c536dfd4154a42d8a..86e9d6d0c0f505782569565fde8e4a46b06b8b4d 100644
> > --- a/drivers/usb/core/port.c
> > +++ b/drivers/usb/core/port.c
> > @@ -117,6 +117,10 @@ static ssize_t disable_store(struct device *dev, struct device_attribute *attr,
> >  	if (rc)
> >  		return rc;
> >  
> > +	/* Early quit if no change was detected */
> > +	if (port_dev->disabled == disabled)
> > +		return count;
> > +
> 
> This will change behavior where someone tells the port to be enabled
> again, when it already is.  Is that ok?

That's the whole purpose of this patch. Can you please elaborate why
someone wants to enable or disbale a port more than once in a row?

Regards,
  Marco

-- 
#gernperDu 
#CallMeByMyFirstName

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |
Re: [PATCH v5 1/4] usb: port: track the disabled state
Posted by Greg Kroah-Hartman 2 days, 20 hours ago
On Fri, Mar 20, 2026 at 11:16:43PM +0100, Marco Felsch wrote:
> On 26-03-11, Greg Kroah-Hartman wrote:
> > On Mon, Feb 23, 2026 at 12:27:34PM +0100, Marco Felsch wrote:
> > > The disable state isn't tracked at the moment, instead the state is
> > > directly passed to the hub driver. Change this behavior to only trigger
> > > the hub if a state change happened. Exit early in case of no state
> > > changes but don't return an error.
> > > 
> > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> 
> ...
> 
> > >  #define to_usb_port(_dev) \
> > > diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> > > index 44e38f922bc553adee64b35c536dfd4154a42d8a..86e9d6d0c0f505782569565fde8e4a46b06b8b4d 100644
> > > --- a/drivers/usb/core/port.c
> > > +++ b/drivers/usb/core/port.c
> > > @@ -117,6 +117,10 @@ static ssize_t disable_store(struct device *dev, struct device_attribute *attr,
> > >  	if (rc)
> > >  		return rc;
> > >  
> > > +	/* Early quit if no change was detected */
> > > +	if (port_dev->disabled == disabled)
> > > +		return count;
> > > +
> > 
> > This will change behavior where someone tells the port to be enabled
> > again, when it already is.  Is that ok?
> 
> That's the whole purpose of this patch. Can you please elaborate why
> someone wants to enable or disbale a port more than once in a row?

I have given up trying to understand why users do what users do :)