usb: misc: onboard_usb_dev: don't defer probing after the device is configured
Subsystems (4)
v5 cover usb: misc: onboard_usb_dev: don't defer probing after the device is configured
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.
Following Johan Hovold's suggestion [1], patch 1 exports the generic driver's operations, and patch 2 changes onboard_usb_dev to call them itself instead of setting generic_subclass, so that it can defer probing before the device is configured.
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. With the peer-hub property removed from one hub's node, that hub binds to the generic driver.
I used an LLM coding assistant to help prepare this series.
Changes in v5:
- Patch 1: added Alan's Acked-by
- Patch 2: stopped matching a device that has no platform device, so that the generic driver binds it (Sashiko)
- Link to v4: https://lore.kernel.org/all/[email protected]/
Changes in v4:
- Switched to calling the generic driver's operations from onboard_usb_dev, instead of adding pre_probe to the USB core
- Link to v3: https://lore.kernel.org/all/[email protected]/
Changes in v3:
- Added a patch fixing existing problems in usb_probe_device() (Alan)
- Link to v2: https://lore.kernel.org/all/[email protected]/
Changes in v2:
- Switched to adding a pre_probe callback to the USB core (Johan)
- Link to v1: https://lore.kernel.org/all/[email protected]/
[1] https://lore.kernel.org/all/[email protected]/
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 | 67 +++++++++++++++++++++--------- include/linux/usb.h | 10 ++++- 4 files changed, 61 insertions(+), 26 deletions(-)
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
Signature · 1 line
2.56.0
PATCH v5 1/2 usb: core: export the generic device driver's operations
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.
---
Changes in v5:
- Added Alan's Acked-by
Changes in v4:
- New patch
drivers/usb/core/generic.c | 4 ++++ drivers/usb/core/usb.h | 6 ------ include/linux/usb.h | 10 +++++++++- 3 files changed, 13 insertions(+), 7 deletions(-)
drivers/usb/core/generic.c +4 -0
@@ -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 */
drivers/usb/core/usb.h +0 -6
@@ -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)
include/linux/usb.h +9 -1
@@ -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,
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v5 1/2] usb: core: export the generic device driver's operations
Sashiko has reviewed this patch and found no issues.
Quoted 9 lines
Export
usb_generic_driver_probe(),usb_generic_driver_disconnect(),usb_generic_driver_suspend()andusb_generic_driver_resume(), and move their declarations toinclude/linux/usb.h. This lets a driver whose probe may return -EPROBE_DEFER leavegeneric_subclassunset and call them itself, so that it can defer before the device is configured.Say in the
generic_subclasskerneldoc not to set it in that case.Signed-off-by James Clark <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
PATCH v5 2/2 usb: misc: onboard_usb_dev: don't defer probing after the device is configured
In reply to James Clarkonboard_usb_dev breaks the rule that a driver must not return -EPROBE_DEFER from its probe after it has registered child devices (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 the BeagleY-AI, this results in a probe loop, which 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, so that the probe is deferred before the device is configured. The device now stays unconfigured while the probe is deferred, including when the platform device never probes. Before, it was configured and unconfigured repeatedly until the platform device probed.
Make onboard_dev_usbdev_match() not match a device when neither its node nor its peer hub's node has a platform device, so that the generic driver binds it instead. If the platform device goes away between match and probe, return -ENODEV instead of -EINVAL, so that usb_probe_device() falls back to the generic driver.
---
Changes in v5:
- Stopped matching a device when neither its node nor its peer hub's node has a platform device, so that the generic driver binds it (Sashiko)
- Split the platform device lookup out of _find_onboard_dev() into onboard_dev_find_pdev(), and dropped the "failed to find device node for peer hub" error
- Commit message: explained the -ENODEV
Changes in v4:
- Switched to calling the generic driver's operations instead of setting generic_subclass, replacing pre_probe in the USB core
drivers/usb/misc/onboard_usb_dev.c | 67 +++++++++++++++++++++--------- 1 file changed, 48 insertions(+), 19 deletions(-)
drivers/usb/misc/onboard_usb_dev.c +48 -19
@@ -576,6 +576,25 @@ static struct platform_driver onboard_dev_driver = {#define VENDOR_ID_VIA 0x2109#define VENDOR_ID_XMOS 0x20B1 +static struct platform_device *onboard_dev_find_pdev (struct device *dev) +{ +struct platform_device *pdev; +struct device_node *np; + + pdev =of_find_device_by_node (dev->of_node); +if (pdev) +return pdev; + + np =of_parse_phandle (dev->of_node,"peer-hub" ,0 ); +if (!np) +return NULL ; + + pdev =of_find_device_by_node (np); +of_node_put (np); + +return pdev; +} +/* * Returns the onboard_dev platform device that is associated with the USB * device passed as parameter.
@@ -583,23 +602,11 @@ static struct platform_driver onboard_dev_driver = {static struct onboard_dev *_find_onboard_dev (struct device *dev) {struct platform_device *pdev; -struct device_node *np;struct onboard_dev *onboard_dev; - pdev =of_find_device_by_node (dev->of_node); -if (!pdev) { - 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 ); - } - - pdev =of_find_device_by_node (np); -of_node_put (np); - -if (!pdev) -return ERR_PTR (-ENODEV ); - } + pdev =onboard_dev_find_pdev (dev); +if (!pdev) +return ERR_PTR (-ENODEV ); onboard_dev =dev_get_drvdata (&pdev->dev);put_device (&pdev->dev);
@@ -619,8 +626,22 @@ static struct onboard_dev *_find_onboard_dev(struct device *dev)static bool onboard_dev_usbdev_match (struct usb_device *udev) { -/* Onboard devices using this driver must have a device tree node */ -return !!udev->dev.of_node; +struct platform_device *pdev; + +/* +* Onboard devices using this driver must have a device tree node with +* a platform device, its own or its peer hub's. Leave any other device +* to the generic driver. +*/ +if (!udev->dev.of_node) +return false; + + pdev =onboard_dev_find_pdev (&udev->dev); +if (!pdev) +return false; + +put_device (&pdev->dev); +return true; }static int onboard_dev_usbdev_probe (struct usb_device *udev)
@@ -639,7 +660,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 +672,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 +723,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, };
Signature · 1 line
2.56.0
[email protected]
AI reviewSashiko has reviewed this patch and found no issues.
Quoted 23 lines
onboard_usb_devbreaks the rule that a driver must not return -EPROBE_DEFER from its probe after it has registered child devices (see commitfbc35b45f9f6("Add documentation on meaning of -EPROBE_DEFER")). Its USB device driver setsgeneric_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 the BeagleY-AI, this results in a probe loop, which has broken booting since commitf918e3804498("arm64: enableHAVE_SHARED_GPIOSfor all ARM64 platforms").Call the generic driver's operations from the driver itself instead of relying on
generic_subclass, so that the probe is deferred before the device is configured. The device now stays unconfigured while the probe is deferred, including when the platform device never probes. Before, it was configured and unconfigured repeatedly until the platform device probed.Make
onboard_dev_usbdev_match()not match a device when neither its node nor its peer hub's node has a platform device, so that the generic driver binds it instead. If the platform device goes away between match and probe, return-ENODEVinstead of-EINVAL, so thatusb_probe_device()falls back to the generic driver.
Quoted 3 lines
Fixes 8bc063641ceb ("usb: misc: Add onboard_usb_hub driver")Suggested-by Johan Hovold <[email protected]>Signed-off-by James Clark <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2