| Message ID | 20200818110445.509668-3-hadess@hadess.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v8,1/3] USB: Also match device drivers using the ->match vfunc | expand |
On 8/18/20 2:04 PM, Bastien Nocera wrote: > When a new device with a specialised device driver is plugged in, the > new driver will be modprobe()'d but the driver core will attach the > "generic" driver to the device. > > After that, nothing will trigger a reprobe when the modprobe()'d device > driver has finished initialising, as the device has the "generic" > driver attached to it. > > Trigger a reprobe ourselves when new specialised drivers get registered. > > Fixes: 88b7381a939d ("USB: Select better matching USB drivers when available") > Signed-off-by: Bastien Nocera <hadess@hadess.net> [...] > drivers/usb/core/driver.c | 40 +++++++++++++++++++++++++++++++++++++-- > 1 file changed, 38 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c > index f81606c6a35b..7e73e989645b 100644 > --- a/drivers/usb/core/driver.c > +++ b/drivers/usb/core/driver.c [...] > @@ -934,13 +963,20 @@ int usb_register_device_driver(struct usb_device_driver *new_udriver, > > retval = driver_register(&new_udriver->drvwrap.driver); > > - if (!retval) > + if (!retval) { > pr_info("%s: registered new device driver %s\n", > usbcore_name, new_udriver->name); > - else > + /* > + * Check whether any device could be better served with > + * this new driver > + */ > + bus_for_each_dev(&usb_bus_type, NULL, new_udriver, > + __usb_bus_reprobe_drivers); > + } else { > printk(KERN_ERR "%s: error %d registering device " > " driver %s\n", Unrelated but... hm, this string literal seems weird. GregKH, would it be OK if we fix it? > usbcore_name, retval, new_udriver->name); > + } > > return retval; > } > MBR, Sergei
On Tue, Aug 18, 2020 at 08:06:20PM +0300, Sergei Shtylyov wrote: > On 8/18/20 2:04 PM, Bastien Nocera wrote: > > > When a new device with a specialised device driver is plugged in, the > > new driver will be modprobe()'d but the driver core will attach the > > "generic" driver to the device. > > > > After that, nothing will trigger a reprobe when the modprobe()'d device > > driver has finished initialising, as the device has the "generic" > > driver attached to it. > > > > Trigger a reprobe ourselves when new specialised drivers get registered. > > > > Fixes: 88b7381a939d ("USB: Select better matching USB drivers when available") > > Signed-off-by: Bastien Nocera <hadess@hadess.net> > [...] > > drivers/usb/core/driver.c | 40 +++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 38 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c > > index f81606c6a35b..7e73e989645b 100644 > > --- a/drivers/usb/core/driver.c > > +++ b/drivers/usb/core/driver.c > [...] > > @@ -934,13 +963,20 @@ int usb_register_device_driver(struct usb_device_driver *new_udriver, > > > > retval = driver_register(&new_udriver->drvwrap.driver); > > > > - if (!retval) > > + if (!retval) { > > pr_info("%s: registered new device driver %s\n", > > usbcore_name, new_udriver->name); > > - else > > + /* > > + * Check whether any device could be better served with > > + * this new driver > > + */ > > + bus_for_each_dev(&usb_bus_type, NULL, new_udriver, > > + __usb_bus_reprobe_drivers); > > + } else { > > printk(KERN_ERR "%s: error %d registering device " > > " driver %s\n", > > Unrelated but... hm, this string literal seems weird. GregKH, would it be OK if we fix it? Please do.
On Tue, Aug 18, 2020 at 08:06:20PM +0300, Sergei Shtylyov wrote: > On 8/18/20 2:04 PM, Bastien Nocera wrote: > > + } else { > > printk(KERN_ERR "%s: error %d registering device " > > " driver %s\n", > > Unrelated but... hm, this string literal seems weird. GregKH, would it be OK if we fix it? > > > usbcore_name, retval, new_udriver->name); Indeed, an extra tab character snuck in there by mistake. It has been present ever since 2006, when the routine was originally added by commit 8bb54ab573ec ("usbcore: add usb_device_driver definition"). It's perfectly okay with me if someone wants to remove the extra tab. In fact, nowadays we'd remove the line break entirely. Alan Stern
On 8/18/20 8:29 PM, Alan Stern wrote: >>> + } else { >>> printk(KERN_ERR "%s: error %d registering device " >>> " driver %s\n", >> >> Unrelated but... hm, this string literal seems weird. GregKH, would it be OK if we fix it? >> >>> usbcore_name, retval, new_udriver->name); > > Indeed, an extra tab character snuck in there by mistake. It has been > present ever since 2006, when the routine was originally added by commit > 8bb54ab573ec ("usbcore: add usb_device_driver definition"). And meanwhile it got copied to another function, usb_register_driver(). I guess it's OK to fix both w/one patch? > It's perfectly okay with me if someone wants to remove the extra tab. > In fact, nowadays we'd remove the line break entirely. It seems this wasn't needed even in the 80-column era... > Alan Stern MBR, Sergei
On Tue, Aug 18, 2020 at 11:49:43PM +0300, Sergei Shtylyov wrote: > On 8/18/20 8:29 PM, Alan Stern wrote: > > >>> + } else { > >>> printk(KERN_ERR "%s: error %d registering device " > >>> " driver %s\n", > >> > >> Unrelated but... hm, this string literal seems weird. GregKH, would it be OK if we fix it? > >> > >>> usbcore_name, retval, new_udriver->name); > > > > Indeed, an extra tab character snuck in there by mistake. It has been > > present ever since 2006, when the routine was originally added by commit > > 8bb54ab573ec ("usbcore: add usb_device_driver definition"). > > And meanwhile it got copied to another function, usb_register_driver(). > I guess it's OK to fix both w/one patch? Yes.
diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index f81606c6a35b..7e73e989645b 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -905,6 +905,35 @@ static int usb_uevent(struct device *dev, struct kobj_uevent_env *env) return 0; } +static bool is_dev_usb_generic_driver(struct device *dev) +{ + struct usb_device_driver *udd = dev->driver ? + to_usb_device_driver(dev->driver) : NULL; + + return udd == &usb_generic_driver; +} + +static int __usb_bus_reprobe_drivers(struct device *dev, void *data) +{ + struct usb_device_driver *new_udriver = data; + struct usb_device *udev; + int ret; + + if (!is_dev_usb_generic_driver(dev)) + return 0; + + udev = to_usb_device(dev); + if (usb_device_match_id(udev, new_udriver->id_table) == NULL && + (!new_udriver->match || new_udriver->match(udev) != 0)) + return 0; + + ret = device_reprobe(dev); + if (ret && ret != -EPROBE_DEFER) + dev_err(dev, "Failed to reprobe device (error %d)\n", ret); + + return 0; +} + /** * usb_register_device_driver - register a USB device (not interface) driver * @new_udriver: USB operations for the device driver @@ -934,13 +963,20 @@ int usb_register_device_driver(struct usb_device_driver *new_udriver, retval = driver_register(&new_udriver->drvwrap.driver); - if (!retval) + if (!retval) { pr_info("%s: registered new device driver %s\n", usbcore_name, new_udriver->name); - else + /* + * Check whether any device could be better served with + * this new driver + */ + bus_for_each_dev(&usb_bus_type, NULL, new_udriver, + __usb_bus_reprobe_drivers); + } else { printk(KERN_ERR "%s: error %d registering device " " driver %s\n", usbcore_name, retval, new_udriver->name); + } return retval; }
When a new device with a specialised device driver is plugged in, the new driver will be modprobe()'d but the driver core will attach the "generic" driver to the device. After that, nothing will trigger a reprobe when the modprobe()'d device driver has finished initialising, as the device has the "generic" driver attached to it. Trigger a reprobe ourselves when new specialised drivers get registered. Fixes: 88b7381a939d ("USB: Select better matching USB drivers when available") Signed-off-by: Bastien Nocera <hadess@hadess.net> Acked-by: Alan Stern <stern@rowland.harvard.edu> --- Changes since v7: - None Changes since v6: - Added Alan's ack Changes since v5: - Throw error when device_reprobe() fails Changes since v4: - Add commit subject to "fixes" section - Clarify conditional that checks for generic driver - Remove check duplicated inside the loop Changes since v3: - Only reprobe devices that could use the new driver - Many code fixes Changes since v2: - Fix formatting Changes since v1: - Simplified after Alan Stern's comments and some clarifications from Benjamin Tissoires. drivers/usb/core/driver.c | 40 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-)