Re: [PATCH v3 1/2] usb: core: clean up before handing a device to the generic driver

[Alan Stern]

On Tue, Nov 03, 2026 at 10:34:12AM +0700, James Clark wrote:
> When a USB device driver's probe returns -ENODEV, usb_probe_device()
> hands the device to the generic driver. It does not first undo what it
> did before calling the probe. This causes two problems.
> 
> First, if the driver sets generic_subclass, the generic probe has
> already run. It configured the device and sent a USB_DEVICE_ADD
> notification. The generic driver then does both again. Notifier users
> such as the usbport LED trigger see two USB_DEVICE_ADDs with no
> USB_DEVICE_REMOVE in between.
> 
> Second, if the driver does not set supports_autosuspend, a runtime PM
> reference was taken before the probe. Nothing drops it, so the device
> can never be runtime suspended.
> 
> Clean up before handing the device over, in the same way as
> usb_unbind_device() does when a driver is unbound: first, for a driver
> that sets generic_subclass, call usb_generic_driver_disconnect();
> second, for a driver that does not set supports_autosuspend, drop the
> runtime PM reference with usb_autosuspend_device().
> 
> This leaves one limitation. Until the generic driver binds, the device
> has no driver. If it is runtime suspended in that time, it can't be
> resumed. This is transient: the generic driver resumes the device when
> it binds.
> 
> This change does not alter what happens when the probe fails with any
> other error. The device stays configured, so its interfaces keep
> working. It keeps the runtime PM reference, because a device without a
> driver can't be resumed. Its USB_DEVICE_ADD stays unpaired, because
> sending USB_DEVICE_REMOVE would mean unconfiguring the device.
> 
> Fixes: 88b7381a939d ("USB: Select better matching USB drivers when available")
> Signed-off-by: James Clark <jjc@xxxxxxxxxx>
> ---
>  drivers/usb/core/driver.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
> index d3a61ebc6afb..9b8cd0e4219e 100644
> --- a/drivers/usb/core/driver.c
> +++ b/drivers/usb/core/driver.c
> @@ -295,6 +295,10 @@ static int usb_probe_device(struct device *dev)
>  		error = -EINVAL;
>  	if (error == -ENODEV && udriver != &usb_generic_driver &&
>  	    (udriver->id_table || udriver->match)) {
> +		if (udriver->generic_subclass)
> +			usb_generic_driver_disconnect(udev);
> +		if (!udriver->supports_autosuspend)
> +			usb_autosuspend_device(udev);
>  		udev->use_generic_driver = 1;
>  		return -EPROBE_DEFER;
>  	}

I was talking about more than just this part.  What happens if 
usb_generic_driver_probe() (near the start of the function) returns an 
error?  (And shouldn't the lines that test the return value and do the 
early return be part of the same conditional block as the function 
call?)

Alan Stern