[PATCH v4 0/2] usb: misc: onboard_usb_dev: don't defer probing after the device is configured

[PATCH v4 0/2] usb: misc: onboard_usb_dev: don't defer probing after the device is configured

[James Clark]

On a BeagleY-AI, kernels since commit f918e3804498 ("arm64: enable
HAVE_SHARED_GPIOS for all ARM64 platforms") no longer boot: the on-board
USB hub is re-probed in a loop, because onboard_usb_dev defers probing
after the hub has been configured.

v3 added a pre_probe callback to the USB core, which turned out to add
significant complexity. v4 follows Johan Hovold's other suggestion [1]
instead. Patch 1 exports the generic driver's operations. Patch 2
changes onboard_usb_dev to call them itself instead of setting
generic_subclass.

Tested on the BeagleY-AI on top of v7.4-rc1: the board boots normally,
and unbinding and rebinding the onboard-usb-dev platform device works.

I used an LLM coding assistant to help prepare this series.

[1] https://lore.kernel.org/all/auSdA8MfolA8OhdM@hovoldconsulting.com/

v3: https://lore.kernel.org/all/cover.1793669297.git.jjc@jclark.com/
v2: https://lore.kernel.org/all/20261030151919.3511106-1-jjc@jclark.com/
v1: https://lore.kernel.org/all/20261030114705.3159093-1-jjc@jclark.com/

James Clark (2):
  usb: core: export the generic device driver's operations
  usb: misc: onboard_usb_dev: don't defer probing after the device is
    configured

 drivers/usb/core/generic.c         |  4 ++++
 drivers/usb/core/usb.h             |  6 ------
 drivers/usb/misc/onboard_usb_dev.c | 14 +++++++++++---
 include/linux/usb.h                | 10 +++++++++-
 4 files changed, 24 insertions(+), 10 deletions(-)


base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.56.0

[PATCH v4 1/2] usb: core: export the generic device driver's operations

[James Clark]

Export usb_generic_driver_probe(), usb_generic_driver_disconnect(),
usb_generic_driver_suspend() and usb_generic_driver_resume(), and move
their declarations to include/linux/usb.h. This lets a driver whose
probe may return -EPROBE_DEFER leave generic_subclass unset and call
them itself, so that it can defer before the device is configured.

Say in the generic_subclass kerneldoc not to set it in that case.

Signed-off-by: James Clark <jjc@jclark.com>
---
 drivers/usb/core/generic.c |  4 ++++
 drivers/usb/core/usb.h     |  6 ------
 include/linux/usb.h        | 10 +++++++++-
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c
index a48994e11ef3..53675d9fdb03 100644
--- a/drivers/usb/core/generic.c
+++ b/drivers/usb/core/generic.c
@@ -261,6 +261,7 @@ int usb_generic_driver_probe(struct usb_device *udev)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(usb_generic_driver_probe);
 
 void usb_generic_driver_disconnect(struct usb_device *udev)
 {
@@ -271,6 +272,7 @@ void usb_generic_driver_disconnect(struct usb_device *udev)
 	if (udev->actconfig)
 		usb_set_configuration(udev, -1);
 }
+EXPORT_SYMBOL_GPL(usb_generic_driver_disconnect);
 
 #ifdef	CONFIG_PM
 
@@ -301,6 +303,7 @@ int usb_generic_driver_suspend(struct usb_device *udev, pm_message_t msg)
 		usbfs_notify_suspend(udev);
 	return rc;
 }
+EXPORT_SYMBOL_GPL(usb_generic_driver_suspend);
 
 int usb_generic_driver_resume(struct usb_device *udev, pm_message_t msg)
 {
@@ -320,6 +323,7 @@ int usb_generic_driver_resume(struct usb_device *udev, pm_message_t msg)
 		usbfs_notify_resume(udev);
 	return rc;
 }
+EXPORT_SYMBOL_GPL(usb_generic_driver_resume);
 
 #endif	/* CONFIG_PM */
 
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index a9b37aeb515b..9f1e4d197695 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -50,12 +50,6 @@ extern int usb_get_bos_descriptor(struct usb_device *dev);
 extern void usb_release_bos_descriptor(struct usb_device *dev);
 extern int usb_set_configuration(struct usb_device *dev, int configuration);
 extern int usb_choose_configuration(struct usb_device *udev);
-extern int usb_generic_driver_probe(struct usb_device *udev);
-extern void usb_generic_driver_disconnect(struct usb_device *udev);
-extern int usb_generic_driver_suspend(struct usb_device *udev,
-		pm_message_t msg);
-extern int usb_generic_driver_resume(struct usb_device *udev,
-		pm_message_t msg);
 
 static inline unsigned usb_get_max_power(struct usb_device *udev,
 		struct usb_host_config *c)
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 825f03b29067..4844f53e2fa8 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1301,7 +1301,8 @@ struct usb_driver {
  *	for devices bound to this driver.
  * @generic_subclass: if set to 1, the generic USB driver's probe, disconnect,
  *	resume and suspend functions will be called in addition to the driver's
- *	own, so this part of the setup does not need to be replicated.
+ *	own, so this part of the setup does not need to be replicated. Do not set
+ *	this if @probe may return -EPROBE_DEFER.
  *
  * USB device drivers must provide a name, other driver fields are optional.
  */
@@ -1375,6 +1376,13 @@ extern int __usb_register_device_driver(struct usb_device_driver *,
 			struct module *, const char *);
 extern void usb_deregister_device_driver(struct usb_device_driver *);
 
+extern int usb_generic_driver_probe(struct usb_device *udev);
+extern void usb_generic_driver_disconnect(struct usb_device *udev);
+extern int usb_generic_driver_suspend(struct usb_device *udev,
+		pm_message_t msg);
+extern int usb_generic_driver_resume(struct usb_device *udev,
+		pm_message_t msg);
+
 extern int usb_register_dev(struct usb_interface *intf,
 			    struct usb_class_driver *class_driver);
 extern void usb_deregister_dev(struct usb_interface *intf,
-- 
2.56.0

Re: [PATCH v4 1/2] usb: core: export the generic device driver's operations

[sashiko-bot]

> Export usb_generic_driver_probe(), usb_generic_driver_disconnect(),
> usb_generic_driver_suspend() and usb_generic_driver_resume(), and move
> their declarations to include/linux/usb.h. This lets a driver whose
> probe may return -EPROBE_DEFER leave generic_subclass unset and call
> them itself, so that it can defer before the device is configured.
> 
> Say in the generic_subclass kerneldoc not to set it in that case.
> 
> Signed-off-by: James Clark <jjc@jclark.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793941292.git.jjc@jclark.com?part=1

[PATCH v4 2/2] usb: misc: onboard_usb_dev: don't defer probing after the device is configured

[James Clark]

onboard_usb_dev breaks the rule that a driver must not return
-EPROBE_DEFER from its probe after it has registered child devices,
because that causes a probe loop (see commit fbc35b45f9f6 ("Add
documentation on meaning of -EPROBE_DEFER")). Its USB device driver
sets generic_subclass, so the generic probe runs first and registers
the device's interfaces. onboard_dev_usbdev_probe() then returns
-EPROBE_DEFER until the platform device for the onboard device has
probed. On a BeagleY-AI, the resulting loop has broken booting since
commit f918e3804498 ("arm64: enable HAVE_SHARED_GPIOS for all ARM64
platforms").

Call the generic driver's operations from the driver itself instead of
relying on generic_subclass.

When the device's node has neither a platform device nor a peer hub,
return -ENODEV instead of -EINVAL, so that the device is still
configured. The device now stays unconfigured while the probe is
deferred.

Fixes: 8bc063641ceb ("usb: misc: Add onboard_usb_hub driver")
Suggested-by: Johan Hovold <johan@kernel.org>
Signed-off-by: James Clark <jjc@jclark.com>
---
 drivers/usb/misc/onboard_usb_dev.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c
index c60406240416..51985e3473ad 100644
--- a/drivers/usb/misc/onboard_usb_dev.c
+++ b/drivers/usb/misc/onboard_usb_dev.c
@@ -591,7 +591,7 @@ static struct onboard_dev *_find_onboard_dev(struct device *dev)
 		np = of_parse_phandle(dev->of_node, "peer-hub", 0);
 		if (!np) {
 			dev_err(dev, "failed to find device node for peer hub\n");
-			return ERR_PTR(-EINVAL);
+			return ERR_PTR(-ENODEV);
 		}
 
 		pdev = of_find_device_by_node(np);
@@ -639,7 +639,11 @@ static int onboard_dev_usbdev_probe(struct usb_device *udev)
 	if (err)
 		return err;
 
-	return 0;
+	err = usb_generic_driver_probe(udev);
+	if (err)
+		onboard_dev_remove_usbdev(onboard_dev, udev);
+
+	return err;
 }
 
 static void onboard_dev_usbdev_disconnect(struct usb_device *udev)
@@ -647,6 +651,7 @@ static void onboard_dev_usbdev_disconnect(struct usb_device *udev)
 	struct onboard_dev *onboard_dev = dev_get_drvdata(&udev->dev);
 
 	onboard_dev_remove_usbdev(onboard_dev, udev);
+	usb_generic_driver_disconnect(udev);
 }
 
 static const struct usb_device_id onboard_dev_id_table[] = {
@@ -697,7 +702,10 @@ static struct usb_device_driver onboard_dev_usbdev_driver = {
 	.match = onboard_dev_usbdev_match,
 	.probe = onboard_dev_usbdev_probe,
 	.disconnect = onboard_dev_usbdev_disconnect,
-	.generic_subclass = 1,
+#ifdef CONFIG_PM
+	.suspend = usb_generic_driver_suspend,
+	.resume = usb_generic_driver_resume,
+#endif
 	.supports_autosuspend =	1,
 	.id_table = onboard_dev_id_table,
 };
-- 
2.56.0

Re: [PATCH v4 2/2] usb: misc: onboard_usb_dev: don't defer probing after the device is configured

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] usb: onboard_usb_dev: -ENODEV fallback leaves device on
  deferred_probe_pending_list
--

commit 8a540d2b1c8688c36c28e760d6d5507f9a82ef03
Author: James Clark <jjc@jclark.com>

usb: misc: onboard_usb_dev: don't defer probing after the device is configured

This commit removes generic_subclass from onboard_dev_usbdev_driver and calls
the generic USB driver operations directly so the USB device is not
configured before probe deferral.  It also returns -ENODEV instead of -EINVAL
when no peer-hub node is found so the device falls back to the generic
driver.

> diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c
> index c604062404162..51985e3473ad8 100644
> --- a/drivers/usb/misc/onboard_usb_dev.c
> +++ b/drivers/usb/misc/onboard_usb_dev.c
> @@ -591,7 +591,7 @@ static struct onboard_dev *_find_onboard_dev(struct device *dev)
>  		np = of_parse_phandle(dev->of_node, "peer-hub", 0);
>  		if (!np) {
>  			dev_err(dev, "failed to find device node for peer hub\n");
> -			return ERR_PTR(-EINVAL);
> +			return ERR_PTR(-ENODEV);
>  		}
>  
>  		pdev = of_find_device_by_node(np);
> @@ -639,7 +639,11 @@ static int onboard_dev_usbdev_probe(struct usb_device *udev)
>  	if (err)
>  		return err;
>  
> -	return 0;
> +	err = usb_generic_driver_probe(udev);
> +	if (err)
> +		onboard_dev_remove_usbdev(onboard_dev, udev);
> +
> +	return err;
>  }
>  
>  static void onboard_dev_usbdev_disconnect(struct usb_device *udev)
> @@ -647,6 +651,7 @@ static void onboard_dev_usbdev_disconnect(struct usb_device *udev)
>  	struct onboard_dev *onboard_dev = dev_get_drvdata(&udev->dev);
>  
>  	onboard_dev_remove_usbdev(onboard_dev, udev);
> +	usb_generic_driver_disconnect(udev);
>  }
>  
>  static const struct usb_device_id onboard_dev_id_table[] = {
> @@ -697,7 +702,10 @@ static struct usb_device_driver onboard_dev_usbdev_driver = {
>  	.match = onboard_dev_usbdev_match,
>  	.probe = onboard_dev_usbdev_probe,
>  	.disconnect = onboard_dev_usbdev_disconnect,
> -	.generic_subclass = 1,
> +#ifdef CONFIG_PM
> +	.suspend = usb_generic_driver_suspend,
> +	.resume = usb_generic_driver_resume,
> +#endif
>  	.supports_autosuspend =	1,
>  	.id_table = onboard_dev_id_table,
>  };

[Severity: Medium]
Can returning -ENODEV from _find_onboard_dev() leave udev unconfigured on
deferred_probe_pending_list instead of falling back to usb_generic_driver?

During __device_attach(), bus_for_each_drv() visits usb_generic_driver
before onboard_dev_usbdev_driver, where usb_generic_driver_match() returns
false because udev->use_generic_driver is still 0.  When
onboard_dev_usbdev_probe() then fails with -ENODEV, usb_probe_device() sets
udev->use_generic_driver = 1 and converts the error to -EPROBE_DEFER:

drivers/usb/core/driver.c:usb_probe_device() {
    ...
	if (error == -ENODEV && udriver != &usb_generic_driver &&
	    (udriver->id_table || udriver->match)) {
		udev->use_generic_driver = 1;
		return -EPROBE_DEFER;
	}
    ...
}

Previously, generic_subclass caused usb_generic_driver_probe() to run first
and bind child interfaces, which incremented deferred_trigger_count so
driver_probe_device() immediately re-triggered deferred probing.  With
generic_subclass removed, no device binds during probe, so
driver_probe_device() adds udev to deferred_probe_pending_list without
calling driver_deferred_probe_trigger().  Unless another device happens to
bind later, deferred_probe_work is not queued and udev stays unconfigured.

Also, if driver_attach() runs for onboard_dev_usbdev_driver, neither
usb_driver_applicable() nor onboard_dev_usbdev_match() checks
udev->use_generic_driver, so it matches onboard_dev_usbdev_driver again and
repeats the -ENODEV -> -EPROBE_DEFER return without triggering deferred
probing.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793941292.git.jjc@jclark.com?part=2