[PATCH v4] USB: Fix device driver race

[PATCH v4] USB: Fix device driver race

[Bastien Nocera]

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
Signed-off-by: Bastien Nocera <hadess@hadess.net>
---

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 | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index f81606c6a35b..6731a8e104bc 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -905,6 +905,27 @@ static int usb_uevent(struct device *dev, struct kobj_uevent_env *env)
 	return 0;
 }
 
+static int __usb_bus_reprobe_drivers(struct device *dev, void *data)
+{
+	struct usb_device_driver *new_udriver = data;
+	struct usb_device_driver *udriver;
+	struct usb_device *udev;
+
+	if (!dev->driver)
+		return 0;
+
+	udriver = to_usb_device_driver(dev->driver);
+	if (udriver != &usb_generic_driver)
+		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))
+		device_reprobe(dev);
+
+	return 0;
+}
+
 /**
  * usb_register_device_driver - register a USB device (not interface) driver
  * @new_udriver: USB operations for the device driver
@@ -934,13 +955,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
+		 */
+		if (new_udriver->match || new_udriver->id_table)
+			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;
 }

Re: [PATCH v4] USB: Fix device driver race

[Sergei Shtylyov]

Hello!

On 24.07.2020 11:59, 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

    It's strange that nobody has noticed... you need to cite the commit 
summary here as well, enclosed into ("").

> Signed-off-by: Bastien Nocera <hadess@hadess.net>
[...]
> diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
> index f81606c6a35b..6731a8e104bc 100644
> --- a/drivers/usb/core/driver.c
> +++ b/drivers/usb/core/driver.c
[...]

MBR, Sergei

Re: [PATCH v4] USB: Fix device driver race

[Bastien Nocera]

On Fri, 2020-07-24 at 13:18 +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 24.07.2020 11:59, 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
> 
>     It's strange that nobody has noticed... you need to cite the
> commit 
> summary here as well, enclosed into ("").

Changed locally, thanks!

> > Signed-off-by: Bastien Nocera <hadess@hadess.net>
> [...]
> > diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
> > index f81606c6a35b..6731a8e104bc 100644
> > --- a/drivers/usb/core/driver.c
> > +++ b/drivers/usb/core/driver.c
> [...]
> 
> MBR, Sergei

Re: [PATCH v4] USB: Fix device driver race

[Andy Shevchenko]

On Fri, Jul 24, 2020 at 12:03 PM Bastien Nocera <hadess@hadess.net> 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.

...

> +static int __usb_bus_reprobe_drivers(struct device *dev, void *data)
> +{
> +       struct usb_device_driver *new_udriver = data;
> +       struct usb_device_driver *udriver;
> +       struct usb_device *udev;
> +
> +       if (!dev->driver)
> +               return 0;
> +
> +       udriver = to_usb_device_driver(dev->driver);
> +       if (udriver != &usb_generic_driver)
> +               return 0;

What about

static bool is_dev_usb_generic_driver(dev)
{
      struct usb_device_driver *udd = dev->driver ?
to_usb_device_driver(dev->driver) : NULL;

      return udd == &usb_generic_driver;
}

  if (!is_dev_usb_generic_driver)
    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))
> +               device_reprobe(dev);
> +
> +       return 0;

What about

     udev = to_usb_device(dev);
     if (usb_device_match_id(udev, new_udriver->id_table) == NULL)
       return 0;
?

  if (new_udriver->match && new_udriver->match(udev) == 0))
               device_reprobe(dev);
    return 0;

> +}

...

> +               /* Check whether any device could be better served with
> +                * this new driver
> +                */

Comment style?

...

> +               if (new_udriver->match || new_udriver->id_table)

But match check is incorporated in the loop function.

> +                       bus_for_each_dev(&usb_bus_type, NULL, new_udriver,
> +                                        __usb_bus_reprobe_drivers);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4] USB: Fix device driver race

[Andy Shevchenko]

On Fri, Jul 24, 2020 at 1:32 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, Jul 24, 2020 at 12:03 PM Bastien Nocera <hadess@hadess.net> wrote:

...

> > +static int __usb_bus_reprobe_drivers(struct device *dev, void *data)
> > +{
> > +       struct usb_device_driver *new_udriver = data;
> > +       struct usb_device_driver *udriver;
> > +       struct usb_device *udev;
> > +
> > +       if (!dev->driver)
> > +               return 0;
> > +
> > +       udriver = to_usb_device_driver(dev->driver);
> > +       if (udriver != &usb_generic_driver)
> > +               return 0;
>
> What about
>
> static bool is_dev_usb_generic_driver(dev)
> {
>       struct usb_device_driver *udd = dev->driver ?
> to_usb_device_driver(dev->driver) : NULL;
>
>       return udd == &usb_generic_driver;
> }
>
>   if (!is_dev_usb_generic_driver)
>     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))
> > +               device_reprobe(dev);
> > +
> > +       return 0;
>
> What about
>
>      udev = to_usb_device(dev);
>      if (usb_device_match_id(udev, new_udriver->id_table) == NULL)
>        return 0;
> ?
>
>   if (new_udriver->match && new_udriver->match(udev) == 0))
>                device_reprobe(dev);
>     return 0;
>
> > +}

It actually sparks a lot of similarities with
__check_usb_generic(). Perhaps you may unify them?


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4] USB: Fix device driver race

[Bastien Nocera]

On Fri, 2020-07-24 at 13:39 +0300, Andy Shevchenko wrote:
> On Fri, Jul 24, 2020 at 1:32 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Fri, Jul 24, 2020 at 12:03 PM Bastien Nocera <hadess@hadess.net>
> > wrote:
> 
> ...
> 
> > > +static int __usb_bus_reprobe_drivers(struct device *dev, void
> > > *data)
> > > +{
> > > +       struct usb_device_driver *new_udriver = data;
> > > +       struct usb_device_driver *udriver;
> > > +       struct usb_device *udev;
> > > +
> > > +       if (!dev->driver)
> > > +               return 0;
> > > +
> > > +       udriver = to_usb_device_driver(dev->driver);
> > > +       if (udriver != &usb_generic_driver)
> > > +               return 0;
> > 
> > What about
> > 
> > static bool is_dev_usb_generic_driver(dev)
> > {
> >       struct usb_device_driver *udd = dev->driver ?
> > to_usb_device_driver(dev->driver) : NULL;
> > 
> >       return udd == &usb_generic_driver;
> > }
> > 
> >   if (!is_dev_usb_generic_driver)
> >     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))
> > > +               device_reprobe(dev);
> > > +
> > > +       return 0;
> > 
> > What about
> > 
> >      udev = to_usb_device(dev);
> >      if (usb_device_match_id(udev, new_udriver->id_table) == NULL)
> >        return 0;
> > ?
> > 
> >   if (new_udriver->match && new_udriver->match(udev) == 0))
> >                device_reprobe(dev);
> >     return 0;
> > 
> > > +}
> 
> It actually sparks a lot of similarities with
> __check_usb_generic(). Perhaps you may unify them?

They might look alike, but apart from that last check (id_table and
match), they're really not. It's a different device driver that's
getting checked against the generic driver.

I tried to merge those, and it ended up being either completely wrong,
or just as long as the code that used to be there before.

Re: [PATCH v4] USB: Fix device driver race

[Bastien Nocera]

On Fri, 2020-07-24 at 13:32 +0300, Andy Shevchenko wrote:
> On Fri, Jul 24, 2020 at 12:03 PM Bastien Nocera <hadess@hadess.net>
> 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.
> 
> ...
> 
> > +static int __usb_bus_reprobe_drivers(struct device *dev, void
> > *data)
> > +{
> > +       struct usb_device_driver *new_udriver = data;
> > +       struct usb_device_driver *udriver;
> > +       struct usb_device *udev;
> > +
> > +       if (!dev->driver)
> > +               return 0;
> > +
> > +       udriver = to_usb_device_driver(dev->driver);
> > +       if (udriver != &usb_generic_driver)
> > +               return 0;
> 
> What about
> 
> static bool is_dev_usb_generic_driver(dev)
> {
>       struct usb_device_driver *udd = dev->driver ?
> to_usb_device_driver(dev->driver) : NULL;
> 
>       return udd == &usb_generic_driver;
> }
> 
>   if (!is_dev_usb_generic_driver)
>     return 0;
> 
> ?

Great, done locally.

> > +       udev = to_usb_device(dev);
> > +       if (usb_device_match_id(udev, new_udriver->id_table) !=
> > NULL ||
> > +           (new_udriver->match && new_udriver->match(udev) == 0))
> > +               device_reprobe(dev);
> > +
> > +       return 0;
> 
> What about
> 
>      udev = to_usb_device(dev);
>      if (usb_device_match_id(udev, new_udriver->id_table) == NULL)
>        return 0;
> ?
> 
>   if (new_udriver->match && new_udriver->match(udev) == 0))
>                device_reprobe(dev);
>     return 0;

That's not the same conditionals. If there's ->match but no matching
->id_table, you're not going to reprobe, when we'd want to.

> > +}
> 
> ...
> 
> > +               /* Check whether any device could be better served
> > with
> > +                * this new driver
> > +                */
> 
> Comment style?

Fixed locally.

> ...
> 
> > +               if (new_udriver->match || new_udriver->id_table)
> 
> But match check is incorporated in the loop function.

It's ->match || ->id_table, not simply ->match. Drivers can have
either, and we need to reprobe if either of those are present (and
we're also checking in the function to avoid calling a NULL pointer ;)

> > +                       bus_for_each_dev(&usb_bus_type, NULL,
> > new_udriver,
> > +                                        __usb_bus_reprobe_drivers)
> > ;

Re: [PATCH v4] USB: Fix device driver race

[Alan Stern]

On Fri, Jul 24, 2020 at 01:32:40PM +0300, Andy Shevchenko wrote:
> On Fri, Jul 24, 2020 at 12:03 PM Bastien Nocera <hadess@hadess.net> 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.
> 
> ...
> 
> > +static int __usb_bus_reprobe_drivers(struct device *dev, void *data)
> > +{
> > +       struct usb_device_driver *new_udriver = data;
> > +       struct usb_device_driver *udriver;
> > +       struct usb_device *udev;
> > +
> > +       if (!dev->driver)
> > +               return 0;
> > +
> > +       udriver = to_usb_device_driver(dev->driver);
> > +       if (udriver != &usb_generic_driver)
> > +               return 0;
> 
> What about
> 
> static bool is_dev_usb_generic_driver(dev)
> {
>       struct usb_device_driver *udd = dev->driver ?
> to_usb_device_driver(dev->driver) : NULL;
> 
>       return udd == &usb_generic_driver;
> }
> 
>   if (!is_dev_usb_generic_driver)
>     return 0;
> 
> ?

Why would you want to add more lines of code to do the same thing?  
Abstraction is fine up to point, but excessive abstraction only makes 
things more difficult.

> 
> > +       udev = to_usb_device(dev);
> > +       if (usb_device_match_id(udev, new_udriver->id_table) != NULL ||
> > +           (new_udriver->match && new_udriver->match(udev) == 0))
> > +               device_reprobe(dev);
> > +
> > +       return 0;
> 
> What about
> 
>      udev = to_usb_device(dev);
>      if (usb_device_match_id(udev, new_udriver->id_table) == NULL)
>        return 0;
> ?

The logic is wrong.  If usb_device_match_id() returns NULL then we want 
to go ahead with the second test, not give up right away.  But this 
could be written as:

	if (usb_device_match_id(udev, new_udriver->id_table) == NULL &&
			(!new_udriver->match || new_udriver->match(udev) != 0))
		return 0;

	device_reprobe(dev);

This would make the overall flow of the routine more uniform.

> > +               /* Check whether any device could be better served with
> > +                * this new driver
> > +                */
> 
> Comment style?

Right:

	/*
	 * Multiline comments
	 * should look like this.
	 */

> ...
> 
> > +               if (new_udriver->match || new_udriver->id_table)
> 
> But match check is incorporated in the loop function.

Agreed, this test is redundant.  However, we should test that 
new_udriver != &usb_generic_driver.

Alan Stern

> 
> > +                       bus_for_each_dev(&usb_bus_type, NULL, new_udriver,
> > +                                        __usb_bus_reprobe_drivers);
> 
> -- 
> With Best Regards,
> Andy Shevchenko

Re: [PATCH v4] USB: Fix device driver race

[Bastien Nocera]

On Fri, 2020-07-24 at 11:27 -0400, Alan Stern wrote:
> On Fri, Jul 24, 2020 at 01:32:40PM +0300, Andy Shevchenko wrote:
> > On Fri, Jul 24, 2020 at 12:03 PM Bastien Nocera <hadess@hadess.net>
> > 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.
> > 
> > ...
> > 
> > > +static int __usb_bus_reprobe_drivers(struct device *dev, void
> > > *data)
> > > +{
> > > +       struct usb_device_driver *new_udriver = data;
> > > +       struct usb_device_driver *udriver;
> > > +       struct usb_device *udev;
> > > +
> > > +       if (!dev->driver)
> > > +               return 0;
> > > +
> > > +       udriver = to_usb_device_driver(dev->driver);
> > > +       if (udriver != &usb_generic_driver)
> > > +               return 0;
> > 
> > What about
> > 
> > static bool is_dev_usb_generic_driver(dev)
> > {
> >       struct usb_device_driver *udd = dev->driver ?
> > to_usb_device_driver(dev->driver) : NULL;
> > 
> >       return udd == &usb_generic_driver;
> > }
> > 
> >   if (!is_dev_usb_generic_driver)
> >     return 0;
> > 
> > ?
> 
> Why would you want to add more lines of code to do the same thing?  
> Abstraction is fine up to point, but excessive abstraction only
> makes 
> things more difficult.

I actually like that one, it made the code clearer IMO.

> > > +       udev = to_usb_device(dev);
> > > +       if (usb_device_match_id(udev, new_udriver->id_table) !=
> > > NULL ||
> > > +           (new_udriver->match && new_udriver->match(udev) ==
> > > 0))
> > > +               device_reprobe(dev);
> > > +
> > > +       return 0;
> > 
> > What about
> > 
> >      udev = to_usb_device(dev);
> >      if (usb_device_match_id(udev, new_udriver->id_table) == NULL)
> >        return 0;
> > ?
> 
> The logic is wrong.  If usb_device_match_id() returns NULL then we
> want 
> to go ahead with the second test, not give up right away.  But this 
> could be written as:
> 
> 	if (usb_device_match_id(udev, new_udriver->id_table) == NULL &&
> 			(!new_udriver->match || new_udriver-
> >match(udev) != 0))
> 		return 0;
> 
> 	device_reprobe(dev);
> 
> This would make the overall flow of the routine more uniform.

Adopted.

> > > +               /* Check whether any device could be better
> > > served with
> > > +                * this new driver
> > > +                */
> > 
> > Comment style?
> 
> Right:
> 
> 	/*
> 	 * Multiline comments
> 	 * should look like this.
> 	 */
> 
> > ...
> > 
> > > +               if (new_udriver->match || new_udriver->id_table)
> > 
> > But match check is incorporated in the loop function.
> 
> Agreed, this test is redundant.  However, we should test that 
> new_udriver != &usb_generic_driver.

Do you really want to loop over every USB device when you know for a
fact that not a single one will match?

I guess it's unlikely, the generic driver would be loaded before any
device, and the specialised drivers need to be able to selected, so
I've done that locally.

> Alan Stern
> 
> > > +                       bus_for_each_dev(&usb_bus_type, NULL,
> > > new_udriver,
> > > +                                        __usb_bus_reprobe_driver
> > > s);
> > 
> > -- 
> > With Best Regards,
> > Andy Shevchenko

Re: [PATCH v4] USB: Fix device driver race

[Alan Stern]

On Fri, Jul 24, 2020 at 06:52:31PM +0200, Bastien Nocera wrote:
> On Fri, 2020-07-24 at 11:27 -0400, Alan Stern wrote:

> > > > +               if (new_udriver->match || new_udriver->id_table)
> > > 
> > > But match check is incorporated in the loop function.
> > 
> > Agreed, this test is redundant.  However, we should test that 
> > new_udriver != &usb_generic_driver.
> 
> Do you really want to loop over every USB device when you know for a
> fact that not a single one will match?

Think of it the other way around: How often will anybody load a 
specialized USB device driver that doesn't have a match function or ID 
table?  It wouldn't match any devices!

> I guess it's unlikely, the generic driver would be loaded before any
> device,

Since it's built into usbcore, I guess that's true.

>  and the specialised drivers need to be able to selected, so
> I've done that locally.

Okay, you're ready to submit the next version?

Alan Stern

Re: [PATCH v4] USB: Fix device driver race

[Bastien Nocera]

On Fri, 2020-07-24 at 13:08 -0400, Alan Stern wrote:
> On Fri, Jul 24, 2020 at 06:52:31PM +0200, Bastien Nocera wrote:
> > On Fri, 2020-07-24 at 11:27 -0400, Alan Stern wrote:
> > > > > +               if (new_udriver->match || new_udriver-
> > > > > >id_table)
> > > > 
> > > > But match check is incorporated in the loop function.
> > > 
> > > Agreed, this test is redundant.  However, we should test that 
> > > new_udriver != &usb_generic_driver.
> > 
> > Do you really want to loop over every USB device when you know for
> > a
> > fact that not a single one will match?
> 
> Think of it the other way around: How often will anybody load a 
> specialized USB device driver that doesn't have a match function or
> ID 
> table?  It wouldn't match any devices!
> 
> > I guess it's unlikely, the generic driver would be loaded before
> > any
> > device,
> 
> Since it's built into usbcore, I guess that's true.
> 
> >  and the specialised drivers need to be able to selected, so
> > I've done that locally.
> 
> Okay, you're ready to submit the next version?

It's compiling. Then I need to test it. I've also added a couple of
fixes/cleanups I ran into while doing this.

Re: [PATCH v4] USB: Fix device driver race

[Bastien Nocera]

On Fri, 2020-07-24 at 19:14 +0200, Bastien Nocera wrote:
> 
<snip>
> It's compiling. Then I need to test it. I've also added a couple of
> fixes/cleanups I ran into while doing this.

And sent

Re: [PATCH v4] USB: Fix device driver race

[Andy Shevchenko]

On Fri, Jul 24, 2020 at 6:27 PM Alan Stern <stern@rowland.harvard.edu> wrote:
> On Fri, Jul 24, 2020 at 01:32:40PM +0300, Andy Shevchenko wrote:
> > On Fri, Jul 24, 2020 at 12:03 PM Bastien Nocera <hadess@hadess.net> wrote:

...

> > What about
> >
> > static bool is_dev_usb_generic_driver(dev)
> > {
> >       struct usb_device_driver *udd = dev->driver ?
> > to_usb_device_driver(dev->driver) : NULL;
> >
> >       return udd == &usb_generic_driver;
> > }
> >
> >   if (!is_dev_usb_generic_driver)
> >     return 0;
> >
> > ?
>
> Why would you want to add more lines of code to do the same thing?
> Abstraction is fine up to point, but excessive abstraction only makes
> things more difficult.

At least to put into ternary will actually reduce LOCs in the original
code (if we drop helper).

The idea of a helper comes to mind that somebody else might reuse
(__check_usb_generic()?).

...

> The logic is wrong.  If usb_device_match_id() returns NULL then we want
> to go ahead with the second test, not give up right away.  But this
> could be written as:

Yes, sorry I didn't think well here.

>
>         if (usb_device_match_id(udev, new_udriver->id_table) == NULL &&
>                         (!new_udriver->match || new_udriver->match(udev) != 0))
>                 return 0;
>
>         device_reprobe(dev);
>
> This would make the overall flow of the routine more uniform.

Yes, my intention was to make all error cases go first.

-- 
With Best Regards,
Andy Shevchenko