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

[Marco Felsch]

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@xxxxxxxxxxxxxx>
> > ---
> >  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    |