On Fri, Dec 01, 2023 at 10:29:51AM -0800, Douglas Anderson wrote: > For some USB devices we might want to do something different for > usb_choose_configuration(). One example here is the r8152 driver where > we want to end up using the vendor driver with the preferred > interface. > > The r8152 driver tried to make things work by implementing a USB > generic_subclass driver and then overriding the normal config > selection after it happened. This is less than ideal and also caused > breakage if someone deauthorized and re-authorized the USB device > because the USB core ended up going back to it's default logic for > choosing the best config. I made an attempt to fix this [1] but it was > a bit ugly. > > Let's do this better and allow USB generic_subclass drivers to > override usb_choose_configuration(). > > [1] https://lore.kernel.org/r/20231130154337.1.Ie00e07f07f87149c9ce0b27ae4e26991d307e14b@changeid > > Suggested-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> > Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx> > --- Reviewed-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> > Changes in v2: > - ("Allow subclassed USB drivers to override ...") new for v2. > > drivers/usb/core/generic.c | 7 +++++++ > include/linux/usb.h | 6 ++++++ > 2 files changed, 13 insertions(+) > > diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c > index 740342a2812a..dcb897158228 100644 > --- a/drivers/usb/core/generic.c > +++ b/drivers/usb/core/generic.c > @@ -59,10 +59,17 @@ int usb_choose_configuration(struct usb_device *udev) > int num_configs; > int insufficient_power = 0; > struct usb_host_config *c, *best; > + struct usb_device_driver *udriver = to_usb_device_driver(udev->dev.driver); > > if (usb_device_is_owned(udev)) > return 0; > > + if (udriver->choose_configuration) { > + i = udriver->choose_configuration(udev); > + if (i >= 0) > + return i; > + } > + > best = NULL; > c = udev->config; > num_configs = udev->descriptor.bNumConfigurations; > diff --git a/include/linux/usb.h b/include/linux/usb.h > index 8c61643acd49..618e5a0b1a22 100644 > --- a/include/linux/usb.h > +++ b/include/linux/usb.h > @@ -1264,6 +1264,9 @@ struct usb_driver { > * module is being unloaded. > * @suspend: Called when the device is going to be suspended by the system. > * @resume: Called when the device is being resumed by the system. > + * @choose_configuration: If non-NULL, called instead of the default > + * usb_choose_configuration(). If this returns an error then we'll go > + * on to call the normal usb_choose_configuration(). > * @dev_groups: Attributes attached to the device that will be created once it > * is bound to the driver. > * @drvwrap: Driver-model core structure wrapper. > @@ -1287,6 +1290,9 @@ struct usb_device_driver { > > int (*suspend) (struct usb_device *udev, pm_message_t message); > int (*resume) (struct usb_device *udev, pm_message_t message); > + > + int (*choose_configuration) (struct usb_device *udev); > + > const struct attribute_group **dev_groups; > struct usbdrv_wrap drvwrap; > const struct usb_device_id *id_table; > -- > 2.43.0.rc2.451.g8631bc7472-goog >
