[PATCH v5 2/4] usb: hub: add infrastructure to pass onboard_dev port features

Marco Felsch posted 4 patches 1 month, 1 week ago
[PATCH v5 2/4] usb: hub: add infrastructure to pass onboard_dev port features
Posted by Marco Felsch 1 month, 1 week ago
On board devices may require special handling for en-/disable port
features due to PCB design decisions e.g. enable/disable the VBUS power
on the port via a host controlled regulator or GPIO.

This commit adds the necessary infrastructure to prepare the common code
base for such use-cases.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/usb/core/hub.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++--
 drivers/usb/core/hub.h |  2 ++
 include/linux/usb.h    |  3 +++
 3 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 24960ba9caa915f12a4f5582269808fdebd1ee11..9fdfd2f0aacc9b1994cd3761330968e052167c67 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -453,9 +453,19 @@ static int clear_hub_feature(struct usb_device *hdev, int feature)
  */
 int usb_clear_port_feature(struct usb_device *hdev, int port1, int feature)
 {
-	return usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
+	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
+	int ret;
+
+	ret = usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
 		USB_REQ_CLEAR_FEATURE, USB_RT_PORT, feature, port1,
 		NULL, 0, 1000);
+	if (ret)
+		return ret;
+
+	if (hub->onboard_hub_clear_port_feature)
+		ret = hub->onboard_hub_clear_port_feature(hdev, feature, port1);
+
+	return ret;
 }
 
 /*
@@ -463,9 +473,19 @@ int usb_clear_port_feature(struct usb_device *hdev, int port1, int feature)
  */
 static int set_port_feature(struct usb_device *hdev, int port1, int feature)
 {
-	return usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
+	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
+	int ret;
+
+	ret = usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
 		USB_REQ_SET_FEATURE, USB_RT_PORT, feature, port1,
 		NULL, 0, 1000);
+	if (ret)
+		return ret;
+
+	if (hub->onboard_hub_set_port_feature)
+		ret = hub->onboard_hub_set_port_feature(hdev, feature, port1);
+
+	return ret;
 }
 
 static char *to_led_name(int selector)
@@ -6545,6 +6565,37 @@ void usb_hub_adjust_deviceremovable(struct usb_device *hdev,
 	}
 }
 
+/**
+ * usb_hub_register_port_feature_hooks - Register port set/get feature hooks
+ * @hdev: USB device belonging to the usb hub
+ * @set_port_feature: set_feature hook which gets called by the hub core
+ * @clear_port_feature: clear_feature hook which gets called by the hub core
+ *
+ * Register set/get_port_feature hooks for a onboard_dev hub.
+ */
+void usb_hub_register_port_feature_hooks(struct usb_device *hdev,
+		int (*set_port_feature)(struct usb_device *, int, int),
+		int (*clear_port_feature)(struct usb_device *, int, int))
+{
+	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
+
+	if (WARN_ON_ONCE(is_root_hub(hdev) || !hub))
+		return;
+
+	if (set_port_feature)
+		hub->onboard_hub_set_port_feature = set_port_feature;
+	if (clear_port_feature)
+		hub->onboard_hub_clear_port_feature = clear_port_feature;
+
+	/*
+	 * Keep it simple for now. Just check the power state and re-sync it
+	 * after adding the hooks since the onboard-dev may do some additional
+	 * logic e.g. controlling regulators.
+	 */
+	hub_power_on(hub, false);
+}
+EXPORT_SYMBOL_GPL(usb_hub_register_port_feature_hooks);
+
 #ifdef CONFIG_ACPI
 /**
  * usb_get_hub_port_acpi_handle - Get the usb port's acpi handle
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 297adf2c6078809ca582104f228e5222c464f999..31800b5922ba896dc7cac5f9e3ed1a77e7c5a801 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -76,6 +76,8 @@ struct usb_hub {
 	struct timer_list	irq_urb_retry;
 	struct usb_port		**ports;
 	struct list_head        onboard_devs;
+	int (*onboard_hub_set_port_feature)(struct usb_device *udev, int feature, int port1);
+	int (*onboard_hub_clear_port_feature)(struct usb_device *udev, int feature, int port1);
 };
 
 /**
diff --git a/include/linux/usb.h b/include/linux/usb.h
index fbfcc70b07fbe5dbaa1ca1be55af127c62294cf7..5f68a066029ea0508fad461daf7ebe6f534778d2 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -946,6 +946,9 @@ int usb_hub_claim_port(struct usb_device *hdev, unsigned port1,
 		struct usb_dev_state *owner);
 int usb_hub_release_port(struct usb_device *hdev, unsigned port1,
 		struct usb_dev_state *owner);
+void usb_hub_register_port_feature_hooks(struct usb_device *hdev,
+		int (*set_port_feature)(struct usb_device *, int, int),
+		int (*clear_port_feature)(struct usb_device *, int, int));
 
 /**
  * usb_make_path - returns stable device path in the usb tree

-- 
2.47.3
Re: [PATCH v5 2/4] usb: hub: add infrastructure to pass onboard_dev port features
Posted by Greg Kroah-Hartman 3 weeks ago
On Mon, Feb 23, 2026 at 12:27:35PM +0100, Marco Felsch wrote:
> On board devices may require special handling for en-/disable port
> features due to PCB design decisions e.g. enable/disable the VBUS power
> on the port via a host controlled regulator or GPIO.
> 
> This commit adds the necessary infrastructure to prepare the common code
> base for such use-cases.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  drivers/usb/core/hub.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  drivers/usb/core/hub.h |  2 ++
>  include/linux/usb.h    |  3 +++
>  3 files changed, 58 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 24960ba9caa915f12a4f5582269808fdebd1ee11..9fdfd2f0aacc9b1994cd3761330968e052167c67 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -453,9 +453,19 @@ static int clear_hub_feature(struct usb_device *hdev, int feature)
>   */
>  int usb_clear_port_feature(struct usb_device *hdev, int port1, int feature)
>  {
> -	return usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
> +	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
> +	int ret;
> +
> +	ret = usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
>  		USB_REQ_CLEAR_FEATURE, USB_RT_PORT, feature, port1,
>  		NULL, 0, 1000);
> +	if (ret)
> +		return ret;
> +
> +	if (hub->onboard_hub_clear_port_feature)
> +		ret = hub->onboard_hub_clear_port_feature(hdev, feature, port1);
> +
> +	return ret;
>  }
>  
>  /*
> @@ -463,9 +473,19 @@ int usb_clear_port_feature(struct usb_device *hdev, int port1, int feature)
>   */
>  static int set_port_feature(struct usb_device *hdev, int port1, int feature)
>  {
> -	return usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
> +	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
> +	int ret;
> +
> +	ret = usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
>  		USB_REQ_SET_FEATURE, USB_RT_PORT, feature, port1,
>  		NULL, 0, 1000);
> +	if (ret)
> +		return ret;
> +
> +	if (hub->onboard_hub_set_port_feature)
> +		ret = hub->onboard_hub_set_port_feature(hdev, feature, port1);
> +
> +	return ret;
>  }
>  
>  static char *to_led_name(int selector)
> @@ -6545,6 +6565,37 @@ void usb_hub_adjust_deviceremovable(struct usb_device *hdev,
>  	}
>  }
>  
> +/**
> + * usb_hub_register_port_feature_hooks - Register port set/get feature hooks
> + * @hdev: USB device belonging to the usb hub
> + * @set_port_feature: set_feature hook which gets called by the hub core
> + * @clear_port_feature: clear_feature hook which gets called by the hub core
> + *
> + * Register set/get_port_feature hooks for a onboard_dev hub.
> + */
> +void usb_hub_register_port_feature_hooks(struct usb_device *hdev,
> +		int (*set_port_feature)(struct usb_device *, int, int),
> +		int (*clear_port_feature)(struct usb_device *, int, int))

This should be a structure, don't force function pointers to be passed
in a function, that way lies madness :)

thanks,

greg k-h
Re: [PATCH v5 2/4] usb: hub: add infrastructure to pass onboard_dev port features
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:35PM +0100, Marco Felsch wrote:
> > On board devices may require special handling for en-/disable port
> > features due to PCB design decisions e.g. enable/disable the VBUS power
> > on the port via a host controlled regulator or GPIO.
> > 
> > This commit adds the necessary infrastructure to prepare the common code
> > base for such use-cases.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  drivers/usb/core/hub.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++--
> >  drivers/usb/core/hub.h |  2 ++
> >  include/linux/usb.h    |  3 +++
> >  3 files changed, 58 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index 24960ba9caa915f12a4f5582269808fdebd1ee11..9fdfd2f0aacc9b1994cd3761330968e052167c67 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -453,9 +453,19 @@ static int clear_hub_feature(struct usb_device *hdev, int feature)
> >   */
> >  int usb_clear_port_feature(struct usb_device *hdev, int port1, int feature)
> >  {
> > -	return usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
> > +	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
> > +	int ret;
> > +
> > +	ret = usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
> >  		USB_REQ_CLEAR_FEATURE, USB_RT_PORT, feature, port1,
> >  		NULL, 0, 1000);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (hub->onboard_hub_clear_port_feature)
> > +		ret = hub->onboard_hub_clear_port_feature(hdev, feature, port1);
> > +
> > +	return ret;
> >  }
> >  
> >  /*
> > @@ -463,9 +473,19 @@ int usb_clear_port_feature(struct usb_device *hdev, int port1, int feature)
> >   */
> >  static int set_port_feature(struct usb_device *hdev, int port1, int feature)
> >  {
> > -	return usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
> > +	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
> > +	int ret;
> > +
> > +	ret = usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
> >  		USB_REQ_SET_FEATURE, USB_RT_PORT, feature, port1,
> >  		NULL, 0, 1000);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (hub->onboard_hub_set_port_feature)
> > +		ret = hub->onboard_hub_set_port_feature(hdev, feature, port1);
> > +
> > +	return ret;
> >  }
> >  
> >  static char *to_led_name(int selector)
> > @@ -6545,6 +6565,37 @@ void usb_hub_adjust_deviceremovable(struct usb_device *hdev,
> >  	}
> >  }
> >  
> > +/**
> > + * usb_hub_register_port_feature_hooks - Register port set/get feature hooks
> > + * @hdev: USB device belonging to the usb hub
> > + * @set_port_feature: set_feature hook which gets called by the hub core
> > + * @clear_port_feature: clear_feature hook which gets called by the hub core
> > + *
> > + * Register set/get_port_feature hooks for a onboard_dev hub.
> > + */
> > +void usb_hub_register_port_feature_hooks(struct usb_device *hdev,
> > +		int (*set_port_feature)(struct usb_device *, int, int),
> > +		int (*clear_port_feature)(struct usb_device *, int, int))
> 
> This should be a structure, don't force function pointers to be passed
> in a function, that way lies madness :)

Sure, do you have a proper struct name in mind already? How about:
 - usb_hub_hooks, or
 - usb_onboard_hub_hooks

Regards,
  Marco


> 
> thanks,
> 
> greg k-h
> 

-- 
#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    |