driver core: generalize driver_override infrastructure
Subsystems (13)
Danilo Krummrich
maintainerv2 cover driver core: generalize driver_override infrastructure
Currently, there are 12 busses (including platform and PCI) that duplicate the driver_override logic for their individual devices.
All of them seem to be prone to the bug described in [1].
While this could be solved for every bus individually using a separate lock, solving this in the driver-core generically results in less (and cleaner) changes overall.
Thus, move driver_override to struct device, provide corresponding accessors for busses and handle locking with a separate lock internally.
In particular, add device_set_driver_override(), device_has_driver_override(), device_match_driver_override() and a helper, DEVICE_ATTR_DRIVER_OVERRIDE(), to declare the corresponding sysfs store() and show() callbacks.
Until all busses have migrated, keep driver_set_override() in place.
Note that we can't use the device lock for the reasons described in [2].
This patch series includes the migration of the platform bus; patches for all other affected busses still need to be extracted as a follow-up of the WIP treewide patch in [3].
[1] https://bugzilla.kernel.org/show_bug.cgi?id=220789
[2] https://lore.kernel.org/driver-core/[email protected]/
[3] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=driver_override
Changes in v2:
- Drop DEVICE_ATTR_DRIVER_OVERRIDE() and make driver_override a struct bus_type feature instead.
- Add driver_override documentation in .../driver-model/binding.rst.
- Move kfree(dev->driver_override.name) before release().
- hwmon: axi-fan: Use NULL instead of dev_name() in devm_request_threaded_irq().
Danilo Krummrich (4):
driver core: generalize driver_override in struct device
docs: driver-model: document driver_override
hwmon: axi-fan: don't use driver_override as IRQ name driver core: platform: use generic driver_override infrastructure
.../driver-api/driver-model/binding.rst | 48 +++++++++++++++ arch/sh/drivers/platform_early.c | 6 +- drivers/base/bus.c | 43 ++++++++++++- drivers/base/core.c | 2 + drivers/base/dd.c | 60 +++++++++++++++++++ drivers/base/platform.c | 37 ++---------- drivers/bus/simple-pm-bus.c | 4 +- drivers/clk/imx/clk-scu.c | 3 +- drivers/hwmon/axi-fan-control.c | 2 +- drivers/slimbus/qcom-ngd-ctrl.c | 6 +- include/linux/device.h | 54 +++++++++++++++++ include/linux/device/bus.h | 4 ++ include/linux/platform_device.h | 5 -- sound/soc/samsung/i2s.c | 6 +- 14 files changed, 228 insertions(+), 52 deletions(-)
base-commit: 11439c4635edd669ae435eec308f4ab8a0804808
Signature · 1 line
2.53.0
Danilo Krummrich
maintainerPATCH v2 1/4 driver core: generalize driver_override in struct device
Currently, there are 12 busses (including platform and PCI) that duplicate the driver_override logic for their individual devices.
All of them seem to be prone to the bug described in [1].
While this could be solved for every bus individually using a separate lock, solving this in the driver-core generically results in less (and cleaner) changes overall.
Thus, move driver_override to struct device, provide corresponding accessors for busses and handle locking with a separate lock internally.
In particular, add device_set_driver_override(), device_has_driver_override(), device_match_driver_override() and a helper, DEVICE_ATTR_DRIVER_OVERRIDE(), to declare the corresponding sysfs store() and show() callbacks.
Until all busses have migrated, keep driver_set_override() in place.
Note that we can't use the device lock for the reasons described in [2].
--- drivers/base/bus.c | 43 ++++++++++++++++++++++++++- drivers/base/core.c | 2 ++ drivers/base/dd.c | 60 ++++++++++++++++++++++++++++++++++++++ include/linux/device.h | 54 ++++++++++++++++++++++++++++++++++ include/linux/device/bus.h | 4 +++ 5 files changed, 162 insertions(+), 1 deletion(-)
drivers/base/bus.c +42 -1
@@ -504,6 +504,36 @@ int bus_for_each_drv(const struct bus_type *bus, struct device_driver *start, }EXPORT_SYMBOL_GPL (bus_for_each_drv); +static ssize_t driver_override_store (struct device *dev, +struct device_attribute *attr, +const char *buf,size_t count) +{ +int ret; + + ret =__device_set_driver_override (dev, buf, count); +if (ret) +return ret; + +return count; +} + +static ssize_t driver_override_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +guard (spinlock)(&dev->driver_override.lock); +return sysfs_emit (buf,"%s\n" , dev->driver_override.name); +} +static DEVICE_ATTR_RW (driver_override); + +static struct attribute *driver_override_dev_attrs[] = { + &dev_attr_driver_override.attr, +NULL , +}; + +static const struct attribute_group driver_override_dev_group = { + .attrs = driver_override_dev_attrs, +}; +/** * bus_add_device - add device to bus * @dev: device being added
@@ -537,9 +567,15 @@ int bus_add_device(struct device *dev)if (error) goto out_put; +if (sp->bus->driver_override) { + error =device_add_group (dev, &driver_override_dev_group); +if (error) + goto out_groups; + } + error =sysfs_create_link (&sp->devices_kset->kobj, &dev->kobj,dev_name (dev));if (error) - goto out_groups; + goto out_override; error =sysfs_create_link (&dev->kobj, &sp->subsys.kobj,"subsystem" );if (error)
@@ -550,6 +586,9 @@ int bus_add_device(struct device *dev) out_subsys:sysfs_remove_link (&sp->devices_kset->kobj,dev_name (dev)); +out_override: +if (dev->bus->driver_override) +device_remove_group (dev, &driver_override_dev_group); out_groups:device_remove_groups (dev, sp->bus->dev_groups); out_put:
@@ -607,6 +646,8 @@ void bus_remove_device(struct device *dev)sysfs_remove_link (&dev->kobj,"subsystem" );sysfs_remove_link (&sp->devices_kset->kobj,dev_name (dev)); +if (dev->bus->driver_override) +device_remove_group (dev, &driver_override_dev_group);device_remove_groups (dev, dev->bus->dev_groups);if (klist_node_attached (&dev->p->knode_bus))klist_del (&dev->p->knode_bus);
drivers/base/core.c +2 -0
@@ -2556,6 +2556,7 @@ static void device_release(struct kobject *kobj)devres_release_all (dev);kfree (dev->dma_range_map); +kfree (dev->driver_override.name);if (dev->release) dev->release (dev);
@@ -3159,6 +3160,7 @@ void device_initialize(struct device *dev)kobject_init (&dev->kobj, &device_ktype);INIT_LIST_HEAD (&dev->dma_pools);mutex_init (&dev->mutex); +spin_lock_init (&dev->driver_override.lock);lockdep_set_novalidate_class (&dev->mutex);spin_lock_init (&dev->devres_lock);INIT_LIST_HEAD (&dev->devres_head);
drivers/base/dd.c +60 -0
66 lines not on this page · raw message
include/linux/device.h +54 -0
72 lines not on this page · raw message
include/linux/device/bus.h +4 -0
@@ -63,6 +63,9 @@ struct fwnode_handle;* this bus. * @pm: Power management operations of this bus, callback the specific * device driver's pm-ops. +* @driver_override: Set to true if this bus supports the driver_override +* mechanism, which allows userspace to force a specific +* driver to bind to a device via a sysfs attribute. * @need_parent_lock: When probing or removing a device on this bus, the * device core should lock the device's parent. *
@@ -104,6 +107,7 @@ struct bus_type {const struct dev_pm_ops *pm; +bool driver_override;bool need_parent_lock; };
Signature · 1 line
2.53.0
Frank Li
maintainerRe: [PATCH v2 1/4] driver core: generalize driver_override in struct device
From: Frank Li (AI-BOT) <[email protected]>
AI bot review and may be useless.
drivers/base/bus.c int bus_for_each_drv(const struct bus_type *bus, struct device_driver *start, · line 518
Quoted 12 lines
drivers/base/bus.c +12
-0+static ssize_t driver_override_store (struct device *dev, +struct device_attribute *attr, +const char *buf,size_t count) +{ +int ret; + + ret =__device_set_driver_override (dev, buf, count); +if (ret) +return ret; + +return count; +}
Potential issue: if __device_set_driver_override() succeeds but modifies state, returning 'count' may not reflect actual bytes consumed if buf contains embedded nulls. Consider documenting expected behavior or validating input length upfront.
drivers/base/bus.c int bus_for_each_drv(const struct bus_type *bus, struct device_driver *start, · line 525
Quoted 6 lines
drivers/base/bus.c +6
-0+static ssize_t driver_override_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +guard (spinlock)(&dev->driver_override.lock); +return sysfs_emit (buf,"%s\n" , dev->driver_override.name); +}
Potential NULL dereference: if dev->driver_override.name is NULL, sysfs_emit() will print "(null)". Add explicit check or document that NULL is acceptable output.
drivers/base/bus.c int bus_add_device(struct device *dev) · line 574
Quoted 5 lines
drivers/base/bus.c +5
-0+if (sp->bus->driver_override) { + error =device_add_group (dev, &driver_override_dev_group); +if (error) + goto out_groups; + }
Error path bug: on device_add_group() failure, code jumps to out_groups but should jump to out_override (which doesn't exist yet). This leaks the sysfs link created below. Reorder or add intermediate label.
drivers/base/bus.c int bus_add_device(struct device *dev) · line 578
Quoted 4 lines
drivers/base/bus.c +3
-1+ error =sysfs_create_link (&sp->devices_kset->kobj, &dev->kobj,dev_name (dev)); +if (error) - goto out_groups; + goto out_override;
Good: label renamed to match new cleanup order.
drivers/base/bus.c int bus_add_device(struct device *dev) · line 591
Quoted 3 lines
drivers/base/bus.c +3
-0+out_override: +if (dev->bus->driver_override) +device_remove_group (dev, &driver_override_dev_group);
Potential issue: dev->bus may be NULL or different from sp->bus at this point. Use sp->bus for consistency with the add path above.
drivers/base/dd.c static void __exit deferred_probe_exit(void) · line 390
Quoted 7 lines
drivers/base/dd.c +7
-0+int __device_set_driver_override (struct device *dev,const char *s,size_t len) +{ +const char *new, *old; +char *cp; + +if (!s) +return -EINVAL ;
Inconsistency: len parameter is passed but then recalculated via strlen(s) below. If len is meant to be trusted, don't recalculate. If not, remove the parameter or document why both exist.
drivers/base/dd.c static void __exit deferred_probe_exit(void) · line 405
Quoted 1 line
drivers/base/dd.c +1
-0+ len =strlen (s);
This overwrites the len parameter, making the initial bounds check at PAGE_SIZE potentially useless. Clarify intent: is len from userspace or always recomputed?
Quoted 7 lines
drivers/base/dd.c +7
-0+ new =kstrndup (s, len,GFP_KERNEL ); +if (!new) +return -ENOMEM ; + +spin_lock (&dev->driver_override.lock); + old = dev->driver_override.name; +if (cp != s) {
drivers/base/dd.c static void __exit deferred_probe_exit(void) · line 438
Quoted 10 lines
drivers/base/dd.c +10
-0+ dev->driver_override.name = new; +spin_unlock (&dev->driver_override.lock); + }else { +/* "\n" passed - clear override */ + dev->driver_override.name =NULL ; +spin_unlock (&dev->driver_override.lock); + +kfree (new); + } +kfree (old);
Logic is correct but confusing: cp is set only if '\n' is found, so the condition if (cp != s) is checking "was newline NOT at start". Add a comment explaining the two paths (newline-at-start clears, otherwise
Re: [PATCH v2 1/4] driver core: generalize driver_override in struct device
In reply to Danilo KrummrichQuoted 20 lines · Danilo Krummrich
Currently, there are 12 busses (including platform and PCI) that duplicate the
driver_overridelogic for their individual devices.All of them seem to be prone to the bug described in [1].
While this could be solved for every bus individually using a separate lock, solving this in the driver-core generically results in less (and cleaner) changes overall.
Thus, move
driver_overrideto struct device, provide corresponding accessors for busses and handle locking with a separate lock internally.In particular, add
device_set_driver_override(),device_has_driver_override(),device_match_driver_override()and a helper,DEVICE_ATTR_DRIVER_OVERRIDE(), to declare the corresponding sysfsstore()andshow()callbacks.Until all busses have migrated, keep
driver_set_override()in place.
Quoted 7 lines
Note that we can't use the device lock for the reasons described in [2].
Co-developed-by Gui-Dong Han <[email protected]>Signed-off-by Gui-Dong Han <[email protected]>Signed-off-by Danilo Krummrich <[email protected]>
Tested this on QEMU PCI with various debug options enabled. Everything looks good so far, no issues found.
Will keep testing.
Quoted 280 lines
--- drivers/base/bus.c | 43 ++++++++++++++++++++++++++- drivers/base/core.c | 2 ++ drivers/base/dd.c | 60 ++++++++++++++++++++++++++++++++++++++ include/linux/device.h | 54 ++++++++++++++++++++++++++++++++++ include/linux/device/bus.h | 4 +++ 5 files changed, 162 insertions(+), 1 deletion(-)drivers/base/bus.c +42
-1@@ -504,6 +504,36 @@ int bus_for_each_drv(const struct bus_type *bus, struct device_driver *start, }EXPORT_SYMBOL_GPL (bus_for_each_drv); +static ssize_t driver_override_store (struct device *dev, +struct device_attribute *attr, +const char *buf,size_t count) +{ +int ret; + + ret =__device_set_driver_override (dev, buf, count); +if (ret) +return ret; + +return count; +} + +static ssize_t driver_override_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +guard (spinlock)(&dev->driver_override.lock); +return sysfs_emit (buf,"%s\n" , dev->driver_override.name); +} +static DEVICE_ATTR_RW (driver_override); + +static struct attribute *driver_override_dev_attrs[] = { + &dev_attr_driver_override.attr, +NULL , +}; + +static const struct attribute_group driver_override_dev_group = { + .attrs = driver_override_dev_attrs, +}; +/** * bus_add_device - add device to bus * @dev: device being added @@ -537,9 +567,15 @@ int bus_add_device(struct device *dev)if (error) goto out_put; +if (sp->bus->driver_override) { + error =device_add_group (dev, &driver_override_dev_group); +if (error) + goto out_groups; + } + error =sysfs_create_link (&sp->devices_kset->kobj, &dev->kobj,dev_name (dev));if (error) - goto out_groups; + goto out_override; error =sysfs_create_link (&dev->kobj, &sp->subsys.kobj,"subsystem" );if (error)@@ -550,6 +586,9 @@ int bus_add_device(struct device *dev) out_subsys:sysfs_remove_link (&sp->devices_kset->kobj,dev_name (dev)); +out_override: +if (dev->bus->driver_override) +device_remove_group (dev, &driver_override_dev_group); out_groups:device_remove_groups (dev, sp->bus->dev_groups); out_put:@@ -607,6 +646,8 @@ void bus_remove_device(struct device *dev)sysfs_remove_link (&dev->kobj,"subsystem" );sysfs_remove_link (&sp->devices_kset->kobj,dev_name (dev)); +if (dev->bus->driver_override) +device_remove_group (dev, &driver_override_dev_group);device_remove_groups (dev, dev->bus->dev_groups);if (klist_node_attached (&dev->p->knode_bus))klist_del (&dev->p->knode_bus);drivers/base/core.c +2
-0@@ -2556,6 +2556,7 @@ static void device_release(struct kobject *kobj)devres_release_all (dev);kfree (dev->dma_range_map); +kfree (dev->driver_override.name);if (dev->release) dev->release (dev);@@ -3159,6 +3160,7 @@ void device_initialize(struct device *dev)kobject_init (&dev->kobj, &device_ktype);INIT_LIST_HEAD (&dev->dma_pools);mutex_init (&dev->mutex); +spin_lock_init (&dev->driver_override.lock);lockdep_set_novalidate_class (&dev->mutex);spin_lock_init (&dev->devres_lock);INIT_LIST_HEAD (&dev->devres_head);drivers/base/dd.c +60
-0@@ -381,6 +381,66 @@ static void __exit deferred_probe_exit(void) }__exitcall (deferred_probe_exit); +int __device_set_driver_override (struct device *dev,const char *s,size_t len) +{ +const char *new, *old; +char *cp; + +if (!s) +return -EINVAL ; + +/* +* The stored value will be used in sysfs show callback (sysfs_emit()), +* which has a length limit of PAGE_SIZE and adds a trailing newline. +* Thus we can store one character less to avoid truncation during sysfs +* show. +*/ +if (len >= (PAGE_SIZE -1 )) +return -EINVAL ; + +/* +* Compute the real length of the string in case userspace sends us a +* bunch of \0 characters like python likes to do. +*/ + len =strlen (s); + +if (!len) { +/* Empty string passed - clear override */ +spin_lock (&dev->driver_override.lock); + old = dev->driver_override.name; + dev->driver_override.name =NULL ; +spin_unlock (&dev->driver_override.lock); +kfree (old); + +return 0 ; + } + + cp =strnchr (s, len,'\n' ); +if (cp) + len = cp - s; + + new =kstrndup (s, len,GFP_KERNEL ); +if (!new) +return -ENOMEM ; + +spin_lock (&dev->driver_override.lock); + old = dev->driver_override.name; +if (cp != s) { + dev->driver_override.name = new; +spin_unlock (&dev->driver_override.lock); + }else { +/* "\n" passed - clear override */ + dev->driver_override.name =NULL ; +spin_unlock (&dev->driver_override.lock); + +kfree (new); + } +kfree (old); + +return 0 ; +} +EXPORT_SYMBOL_GPL (__device_set_driver_override); +/** * device_is_bound() - Check if device is bound to a driver * @dev: device to check include/linux/device.h +54
-0@@ -483,6 +483,8 @@ struct device_physical_location {* on. This shrinks the "Board Support Packages" (BSPs) and * minimizes board-specific #ifdefs in drivers. * @driver_data: Private pointer for driver specific info. +* @driver_override: Driver name to force a match. Do not touch directly; use +* device_set_driver_override() instead. * @links: Links to suppliers and consumers of this device. * @power: For device power management. * See Documentation/driver-api/pm/devices.rst for details. @@ -576,6 +578,10 @@ struct device {core doesn't touch it */ void *driver_data;/* Driver data, set and get with dev_set_drvdata/dev_get_drvdata */ +struct { +const char *name; +spinlock_t lock; + } driver_override;struct mutex mutex;/* mutex to synchronize calls to * its driver. */ @@ -701,6 +707,54 @@ struct device_link {#define kobj_to_dev (__kobj) container_of_const(__kobj, struct device, kobj) +int __device_set_driver_override (struct device *dev,const char *s,size_t len); + +/** +* device_set_driver_override() - Helper to set or clear driver override. +* @dev: Device to change +* @s: NUL-terminated string, new driver name to force a match, pass empty +* string to clear it ("" or "\n", where the latter is only for sysfs +* interface). +* +* Helper to set or clear driver override of a device. +* +* Returns: 0 on success or a negative error code on failure. +*/ +static inline int device_set_driver_override (struct device *dev,const char *s) +{ +return __device_set_driver_override (dev, s, s ?strlen (s) :0 ); +} + +/** +* device_has_driver_override() - Check if a driver override has been set. +* @dev: device to check +* +* Returns true if a driver override has been set for this device. +*/ +static inline bool device_has_driver_override (struct device *dev) +{ +guard (spinlock)(&dev->driver_override.lock); +return !!dev->driver_override.name; +} + +/** +* device_match_driver_override() - Match a driver against the device's driver_override. +* @dev: device to check +* @drv: driver to match against +* +* Returns > 0 if a driver override is set and matches the given driver, 0 if a +* driver override is set but does not match, or < 0 if a driver override is not +* set at all. +*/ +static inline int device_match_driver_override (struct device *dev, +const struct device_driver *drv) +{ +guard (spinlock)(&dev->driver_override.lock); +if (dev->driver_override.name) +return !strcmp (dev->driver_override.name, drv->name); +return -1 ; +} +/** * device_iommu_mapped - Returns true when the device DMA is translated * by an IOMMU include/linux/device/bus.h +4
-1@@ -63,6 +63,9 @@ struct fwnode_handle;* this bus. * @pm: Power management operations of this bus, callback the specific * device driver's pm-ops. +* @driver_override: Set to true if this bus supports the driver_override +* mechanism, which allows userspace to force a specific +* driver to bind to a device via a sysfs attribute. * @need_parent_lock: When probing or removing a device on this bus, the * device core should lock the device's parent. * @@ -104,6 +107,7 @@ struct bus_type {const struct dev_pm_ops *pm; +bool driver_override;bool need_parent_lock; }; --2.53.0
Danilo Krummrich
maintainerPATCH v2 2/4 docs: driver-model: document driver_override
In reply to Danilo KrummrichNow that we support driver_override as a driver-core feature through struct device and struct bus_type, add some documentation in the context of how a device / driver binding is established.
--- .../driver-api/driver-model/binding.rst | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+)
Documentation/driver-api/driver-model/binding.rst +48 -0
@@ -99,3 +99,51 @@ of the driver is decremented. All symlinks between the two are removed. When a driver is removed, the list of devices that it supports is iterated over, and the driver's remove callback is called for each one. The device is removed from that list and the symlinks removed. + + +Driver Override +~~~~~~~~~~~~~~~ + +Userspace may override the standard matching by writing a driver name to +a device's ``driver_override`` sysfs attribute. When set, only a driver +whose name matches the override will be considered during binding. This +bypasses all bus-specific matching (OF, ACPI, ID tables, etc.). + +The override may be cleared by writing an empty string, which returns +the device to standard matching rules. Writing to ``driver_override`` +does not automatically unbind the device from its current driver or +make any attempt to load the specified driver. + +Buses opt into this mechanism by setting the ``driver_override`` flag in +their ``struct bus_type``:: + + const struct bus_type example_bus_type = { + ... + .driver_override = true, + }; + +When the flag is set, the driver core automatically creates the +``driver_override`` sysfs attribute for every device on that bus. + +The bus's ``match()`` callback should check the override before performing +its own matching, using ``device_match_driver_override()``:: + + static int example_match(struct device *dev, const struct device_driver *drv) + { + int ret; + + ret = device_match_driver_override(dev, drv); + if (ret >= 0) + return ret; + + /* Fall through to bus-specific matching... */ + } + +``device_match_driver_override()`` returns > 0 if the override matches +the given driver, 0 if the override is set but does not match, or < 0 if +no override is set at all. + +Additional helpers are available: + +- ``device_set_driver_override()`` - set or clear the override from kernel code. +- ``device_has_driver_override()`` - check whether an override is set.
Signature · 1 line
2.53.0
Frank Li
maintainerRe: [PATCH v2 2/4] docs: driver-model: document driver_override
From: Frank Li (AI-BOT) <[email protected]>
AI bot review and may be useless.
Quoted 36 lines
Documentation/driver-api/driver-model/binding.rst +36
-0+Driver Override +~~~~~~~~~~~~~~~ + +Userspace may override the standard matching by writing a driver name to +a device's ``driver_override`` sysfs attribute. When set, only a driver +whose name matches the override will be considered during binding. This +bypasses all bus-specific matching (OF, ACPI, ID tables, etc.). + +The override may be cleared by writing an empty string, which returns +the device to standard matching rules. Writing to ``driver_override`` +does not automatically unbind the device from its current driver or +make any attempt to load the specified driver. + +Buses opt into this mechanism by setting the ``driver_override`` flag in +their ``struct bus_type``:: + + const struct bus_type example_bus_type = { + ... + .driver_override = true, + }; + +When the flag is set, the driver core automatically creates the +``driver_override`` sysfs attribute for every device on that bus. + +The bus's ``match()`` callback should check the override before performing +its own matching, using ``device_match_driver_override()``:: + + static int example_match(struct device *dev, const struct device_driver *drv) + { + int ret; + + ret = device_match_driver_override(dev, drv); + if (ret >= 0) + return ret; + + /* Fall through to bus-specific matching... */
Documentation/driver-api/driver-model/binding.rst of the driver is decremented. All symlinks between the two are removed. · line 149
Quoted 10 lines
Documentation/driver-api/driver-model/binding.rst +10
-0+ } + +``device_match_driver_override()`` returns > 0 if the override matches +the given driver, 0 if the override is set but does not match, or < 0 if +no override is set at all. + +Additional helpers are available: + +- ``device_set_driver_override()`` - set or clear the override from kernel code. +- ``device_has_driver_override()`` - check whether an override is set.
Minor: Consider adding a note that the override persists across unbind/rebind cycles, or clarify the lifetime semantics. The current text doesn't explicitly state whether the override survives device removal or driver unload.
Also, the example code comment "Fall through to bus-specific matching..." is slightly misleading—there's no actual fallthrough; the function returns. Consider rewording to "Continue with bus-specific matching logic" for clarity.
Danilo Krummrich
maintainerPATCH v2 3/4 hwmon: axi-fan: don't use driver_override as IRQ name
In reply to Danilo KrummrichDo not use driver_override as IRQ name, as it is not guaranteed to point to a valid string; use NULL instead (which makes the devm IRQ helpers use dev_name()).
--- drivers/hwmon/axi-fan-control.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/hwmon/axi-fan-control.c +1 -1
@@ -507,7 +507,7 @@ static int axi_fan_control_probe(struct platform_device *pdev) ret =devm_request_threaded_irq (&pdev->dev, ctl->irq,NULL , axi_fan_control_irq_handler,IRQF_ONESHOT |IRQF_TRIGGER_HIGH , - pdev->driver_override, ctl); +NULL , ctl);if (ret)return dev_err_probe (&pdev->dev, ret,"failed to request an irq\n" );
Signature · 1 line
2.53.0
Re: [PATCH v2 3/4] hwmon: axi-fan: don't use driver_override as IRQ name
Quoted 7 lines · Danilo Krummrich
Do not use
driver_overrideas IRQ name, as it is not guaranteed to point to a valid string; use NULL instead (which makes the devm IRQ helpers usedev_name()).Fixes 8412b410fa5e ("hwmon: Support ADI Fan Control IP")Signed-off-by Danilo Krummrich <[email protected]>---
Quoted 16 lines
drivers/hwmon/axi-fan-control.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)drivers/hwmon/axi-fan-control.c +1
-1@@ -507,7 +507,7 @@ static int axi_fan_control_probe(struct platform_device *pdev) ret = devm_request_threaded_irq(&pdev->dev, ctl->irq, NULL, axi_fan_control_irq_handler, IRQF_ONESHOT | IRQF_TRIGGER_HIGH, - pdev->driver_override, ctl); +NULL , ctl); if (ret) return dev_err_probe(&pdev->dev, ret, "failed to request an irq\n");
Re: [PATCH v2 3/4] hwmon: axi-fan: don't use driver_override as IRQ name
In reply to Danilo KrummrichQuoted 7 lines · Danilo Krummrich
Do not use
driver_overrideas IRQ name, as it is not guaranteed to point to a valid string; use NULL instead (which makes the devm IRQ helpers usedev_name()).Fixes 8412b410fa5e ("hwmon: Support ADI Fan Control IP")Signed-off-by Danilo Krummrich <[email protected]>Reviewed-by Nuno Sá <[email protected]>
Applied.
Thanks,
Guenter
Danilo Krummrich
maintainerRe: [PATCH v2 3/4] hwmon: axi-fan: don't use driver_override as IRQ name
Quoted 10 lines · Guenter Roeck
Quoted 7 lines · Danilo Krummrich
Do not use
driver_overrideas IRQ name, as it is not guaranteed to point to a valid string; use NULL instead (which makes the devm IRQ helpers usedev_name()).Fixes 8412b410fa5e ("hwmon: Support ADI Fan Control IP")Signed-off-by Danilo Krummrich <[email protected]>Reviewed-by Nuno Sá <[email protected]>Applied.
I need this patch in the driver-core tree, otherwise I can't land patch 4 of this series [1]. :(
Any chance you can drop it from your tree?
Re: [PATCH v2 3/4] hwmon: axi-fan: don't use driver_override as IRQ name
Quoted 10 lines · Danilo Krummrich
Quoted 9 lines · Guenter Roeck
Quoted 7 lines · Danilo Krummrich
Do not use
driver_overrideas IRQ name, as it is not guaranteed to point to a valid string; use NULL instead (which makes the devm IRQ helpers usedev_name()).Fixes 8412b410fa5e ("hwmon: Support ADI Fan Control IP")Signed-off-by Danilo Krummrich <[email protected]>Reviewed-by Nuno Sá <[email protected]>
Quoted 9 lines
Quoted 1 line
Applied.
I need this patch in the driver-core tree, otherwise I can't land patch 4 of this series [1]. :(
Any chance you can drop it from your tree?
Sure, I'll drop it. Please add
Thanks,
Guenter
Danilo Krummrich
maintainerRe: [PATCH v2 3/4] hwmon: axi-fan: don't use driver_override as IRQ name
Quoted 16 lines · Guenter Roeck
Quoted 15 lines · Danilo Krummrich
Quoted 10 lines · Guenter Roeck
Quoted 7 lines · Danilo Krummrich
Do not use
driver_overrideas IRQ name, as it is not guaranteed to point to a valid string; use NULL instead (which makes the devm IRQ helpers usedev_name()).Fixes 8412b410fa5e ("hwmon: Support ADI Fan Control IP")Signed-off-by Danilo Krummrich <[email protected]>Reviewed-by Nuno Sá <[email protected]>Applied.
I need this patch in the driver-core tree, otherwise I can't land patch 4 of this series [1]. :(
Quoted 8 lines
Quoted 4 lines
Any chance you can drop it from your tree?
Sure, I'll drop it. Please add
Acked-by Guenter Roeck <[email protected]>
Thanks!
Frank Li
maintainerRe: [PATCH v2 3/4] hwmon: axi-fan: don't use driver_override as IRQ name
In reply to Danilo KrummrichFrom: Frank Li (AI-BOT) <[email protected]>
drivers/hwmon/axi-fan-control.c static int axi_fan_control_probe(struct platform_device *pdev) · line 510
Quoted 5 lines
ret = devm_request_threaded_irq(&pdev->dev, ctl->irq, NULL, axi_fan_control_irq_handler, IRQF_ONESHOT | IRQF_TRIGGER_HIGH,drivers/hwmon/axi-fan-control.c +1
-1- pdev->driver_override, ctl); + NULL, ctl);
Good catch. Using driver_override as IRQ name is indeed unsafe since it's not guaranteed to be a valid string. Passing NULL is correct here; the devm IRQ helpers will fall back to dev_name() which is always valid.
The fix is minimal and addresses the root cause. The error handling path below looks fine—dev_err_probe() will still work correctly with the NULL name.
Looks good to me.
AI bot review and may be useless.
Danilo Krummrich
maintainerPATCH v2 4/4 driver core: platform: use generic driver_override infrastructure
In reply to Danilo KrummrichWhen a driver is probed through __driver_attach(), the bus' match() callback is called without the device lock held, thus accessing the driver_override field without a lock, which can cause a UAF.
Fix this by using the driver-core driver_override infrastructure taking care of proper locking internally.
Note that calling match() from __driver_attach() without the device lock held is intentional. [1]
--- arch/sh/drivers/platform_early.c | 6 ++++-- drivers/base/platform.c | 37 +++++--------------------------- drivers/bus/simple-pm-bus.c | 4 ++-- drivers/clk/imx/clk-scu.c | 3 +-- drivers/slimbus/qcom-ngd-ctrl.c | 6 ++---- include/linux/platform_device.h | 5 ----- sound/soc/samsung/i2s.c | 6 +++--- 7 files changed, 17 insertions(+), 50 deletions(-)
arch/sh/drivers/platform_early.c +4 -2
@@ -25,10 +25,12 @@ static int platform_match(struct device *dev, struct device_driver *drv) {struct platform_device *pdev =to_platform_device (dev);struct platform_driver *pdrv =to_platform_driver (drv); +int ret;/* When driver_override is set, only bind to the matching driver */ -if (pdev->driver_override) -return !strcmp (pdev->driver_override, drv->name); + ret =device_match_driver_override (dev, drv); +if (ret >=0 ) +return ret;/* Then try to match against the id table */ if (pdrv->id_table)
drivers/base/platform.c +5 -32
@@ -603,7 +603,6 @@ static void platform_device_release(struct device *dev)kfree (pa->pdev.dev.platform_data);kfree (pa->pdev.mfd_cell);kfree (pa->pdev.resource); -kfree (pa->pdev.driver_override);kfree (pa); }
@@ -1306,38 +1305,9 @@ static ssize_t numa_node_show(struct device *dev, }static DEVICE_ATTR_RO (numa_node); -static ssize_t driver_override_show (struct device *dev, -struct device_attribute *attr,char *buf) -{ -struct platform_device *pdev =to_platform_device (dev); -ssize_t len; - -device_lock (dev); - len =sysfs_emit (buf,"%s\n" , pdev->driver_override); -device_unlock (dev); - -return len; -} - -static ssize_t driver_override_store (struct device *dev, -struct device_attribute *attr, -const char *buf,size_t count) -{ -struct platform_device *pdev =to_platform_device (dev); -int ret; - - ret =driver_set_override (dev, &pdev->driver_override, buf, count); -if (ret) -return ret; - -return count; -} -static DEVICE_ATTR_RW (driver_override); -static struct attribute *platform_dev_attrs[] = { &dev_attr_modalias.attr, &dev_attr_numa_node.attr, - &dev_attr_driver_override.attr,NULL , };
@@ -1377,10 +1347,12 @@ static int platform_match(struct device *dev, const struct device_driver *drv) {struct platform_device *pdev =to_platform_device (dev);struct platform_driver *pdrv =to_platform_driver (drv); +int ret;/* When driver_override is set, only bind to the matching driver */ -if (pdev->driver_override) -return !strcmp (pdev->driver_override, drv->name); + ret =device_match_driver_override (dev, drv); +if (ret >=0 ) +return ret;/* Attempt an OF style match first */ if (of_driver_match_device (dev, drv))
@@ -1516,6 +1488,7 @@ static const struct dev_pm_ops platform_dev_pm_ops = {const struct bus_type platform_bus_type = { .name ="platform" , .dev_groups = platform_dev_groups, + .driver_override = true, .match = platform_match, .uevent = platform_uevent, .probe = platform_probe,
drivers/bus/simple-pm-bus.c +2 -2
@@ -36,7 +36,7 @@ static int simple_pm_bus_probe(struct platform_device *pdev)* that's not listed in simple_pm_bus_of_match. We don't want to do any * of the simple-pm-bus tasks for these devices, so return early. */ -if (pdev->driver_override) +if (device_has_driver_override (&pdev->dev))return 0 ; match =of_match_device (dev->driver->of_match_table, dev);
@@ -78,7 +78,7 @@ static void simple_pm_bus_remove(struct platform_device *pdev) {const void *data =of_device_get_match_data (&pdev->dev); -if (pdev->driver_override || data) +if (device_has_driver_override (&pdev->dev) || data)return ;dev_dbg (&pdev->dev,"%s\n" , __func__);
drivers/clk/imx/clk-scu.c +1 -2
@@ -706,8 +706,7 @@ struct clk_hw *imx_clk_scu_alloc_dev(const char *name,if (ret) goto put_device; - ret =driver_set_override (&pdev->dev, &pdev->driver_override, -"imx-scu-clk" ,strlen ("imx-scu-clk" )); + ret =device_set_driver_override (&pdev->dev,"imx-scu-clk" );if (ret) goto put_device;
drivers/slimbus/qcom-ngd-ctrl.c +2 -4
@@ -1535,10 +1535,8 @@ static int of_qcom_slim_ngd_register(struct device *parent, ngd->id = id; ngd->pdev->dev.parent = parent; - ret =driver_set_override (&ngd->pdev->dev, - &ngd->pdev->driver_override, -QCOM_SLIM_NGD_DRV_NAME , -strlen (QCOM_SLIM_NGD_DRV_NAME )); + ret =device_set_driver_override (&ngd->pdev->dev, +QCOM_SLIM_NGD_DRV_NAME );if (ret) {platform_device_put (ngd->pdev);kfree (ngd);
include/linux/platform_device.h +0 -5
@@ -31,11 +31,6 @@ struct platform_device {struct resource *resource;const struct platform_device_id *id_entry; -/* -* Driver name to force a match. Do not set directly, because core -* frees it. Use driver_set_override() to set or clear it. -*/ -const char *driver_override;/* MFD cell pointer */ struct mfd_cell *mfd_cell;
sound/soc/samsung/i2s.c +3 -3
@@ -1360,10 +1360,10 @@ static int i2s_create_secondary_device(struct samsung_i2s_priv *priv)if (!pdev_sec)return -ENOMEM ; - pdev_sec->driver_override =kstrdup ("samsung-i2s" ,GFP_KERNEL ); -if (!pdev_sec->driver_override) { + ret =device_set_driver_override (&pdev_sec->dev,"samsung-i2s" ); +if (ret) {platform_device_put (pdev_sec); -return -ENOMEM ; +return ret; } ret =platform_device_add (pdev_sec);
Signature · 1 line
2.53.0
Frank Li
maintainerRe: [PATCH v2 4/4] driver core: platform: use generic driver_override infrastructure
From: Frank Li (AI-BOT) <[email protected]>
AI bot review and may be useless.
arch/sh/drivers/platform_early.c static int platform_match(struct device *dev, struct device_driver *drv) · line 33
Quoted 12 lines
arch/sh/drivers/platform_early.c +4
-2@@ -25,10 +25,12 @@ static int platform_match(struct device *dev, struct device_driver *drv) {struct platform_device *pdev =to_platform_device (dev);struct platform_driver *pdrv =to_platform_driver (drv); +int ret;/* When driver_override is set, only bind to the matching driver */ -if (pdev->driver_override) -return !strcmp (pdev->driver_override, drv->name); + ret =device_match_driver_override (dev, drv); +if (ret >=0 ) +return ret;
Good: centralizes locking logic. Verify device_match_driver_override() handles -1 return for "not set" case consistently across all callers.
drivers/base/platform.c static void platform_device_release(struct device *dev) · line 606
Quoted 5 lines
drivers/base/platform.c +0
-1@@ -603,7 +603,6 @@ static void platform_device_release(struct device *dev)kfree (pa->pdev.dev.platform_data);kfree (pa->pdev.mfd_cell);kfree (pa->pdev.resource); -kfree (pa->pdev.driver_override);
Correct: driver_override now managed by generic infrastructure, not platform_device. Ensure device_release() in driver-core handles cleanup.
drivers/base/platform.c static ssize_t numa_node_show(struct device *dev, · line 1320
Quoted 12 lines
drivers/base/platform.c +0
-12-static ssize_t driver_override_show (struct device *dev, -struct device_attribute *attr,char *buf) -{ -struct platform_device *pdev =to_platform_device (dev); -ssize_t len; - -device_lock (dev); - len =sysfs_emit (buf,"%s\n" , pdev->driver_override); -device_unlock (dev); - -return len; -}
Good: removes duplicate sysfs attribute. Verify generic implementation in device-core provides equivalent locking and sysfs_emit() behavior.
arch/sh/drivers/platform_early.c static int platform_match(struct device *dev, const struct device_driver *drv) · line 1355
Quoted 12 lines
arch/sh/drivers/platform_early.c +4
-2@@ -1377,10 +1347,12 @@ static int platform_match(struct device *dev, const struct device_driver *drv) {struct platform_device *pdev =to_platform_device (dev);struct platform_driver *pdrv =to_platform_driver (drv); +int ret;/* When driver_override is set, only bind to the matching driver */ -if (pdev->driver_override) -return !strcmp (pdev->driver_override, drv->name); + ret =device_match_driver_override (dev, drv); +if (ret >=0 ) +return ret;
Same pattern as arch/sh. Consistent refactoring across platform code.
drivers/base/platform.c static const struct dev_pm_ops platform_dev_pm_ops = { · line 1491
Quoted 1 line
drivers/base/platform.c +1
-0+ .driver_override = true,
Verify this flag is documented in struct bus_type definition and that all bus_type users expecting driver_override support have this set.
drivers/bus/simple-pm-bus.c static int simple_pm_bus_probe(struct platform_device *pdev) · line 39
Quoted 2 lines
drivers/bus/simple-pm-bus.c +1
-1-if (pdev->driver_override) +if (device_has_driver_override (&pdev->dev))
Good: accessor function prevents direct field access. Confirm device_has_driver_override() is properly exported and inlined where needed for performance.
drivers/clk/imx/clk-scu.c struct clk_hw *imx_clk_scu_alloc_dev(const char *name, · line 709
Quoted 3 lines
drivers/clk/imx/clk-scu.c +1
-2- ret =driver_set_override (&pdev->dev, &pdev->driver_override, -"imx-scu-clk" ,strlen ("imx-scu-clk" )); + ret =device_set_driver_override (&pdev->dev,"imx-scu-clk" );
Good: removes strlen() call; generic function should handle length internally. Verify no callers relied on old signature.
Quoted 3 lines
sound/soc/samsung/i2s.c +1
-2- pdev_sec->driver_override =kstrdup ("samsung-i2s" ,GFP_KERNEL ); -if (!pdev_sec->driver_override) { + ret = device_set_driver_override(&pdev_sec->dev, "samsung
Danilo Krummrich
maintainerRe: [PATCH v2 4/4] driver core: platform: use generic driver_override infrastructure
In reply to Danilo KrummrichQuoted 4 lines · Danilo Krummrich
arch/sh/drivers/platform_early.c | 6 ++++-- drivers/clk/imx/clk-scu.c | 3 +-- drivers/slimbus/qcom-ngd-ctrl.c | 6 ++---- sound/soc/samsung/i2s.c | 6 +++---
Some ACKs would be appreciated. :)
Danilo Krummrich
maintainerRe: [PATCH v2 4/4] driver core: platform: use generic driver_override infrastructure
Quoted 7 lines · Danilo Krummrich
Quoted 4 lines · Danilo Krummrich
arch/sh/drivers/platform_early.c | 6 ++++-- drivers/clk/imx/clk-scu.c | 3 +-- drivers/slimbus/qcom-ngd-ctrl.c | 6 ++---- sound/soc/samsung/i2s.c | 6 +++---Some ACKs would be appreciated. :)
Looking forward to pick this up soon, please let me know if there are any concerns.
Thanks,
Danilo
Danilo Krummrich
maintainerRe: [PATCH v2 4/4] driver core: platform: use generic driver_override infrastructure
In reply to Danilo Krummrich(Cc: Bartosz)
Quoted 9 lines · Danilo Krummrich
arch/sh/drivers/platform_early.c +1
-0@@ -25,10 +25,12 @@ static int platform_match(struct device *dev, struct device_driver *drv) {struct platform_device *pdev =to_platform_device (dev);struct platform_driver *pdrv =to_platform_driver (drv); +int ret;
arch/sh/drivers/platform_early.c static int platform_match(struct device *dev, struct device_driver *drv) · line 36
Quoted 10 lines
arch/sh/drivers/platform_early.c +3
-2/* When driver_override is set, only bind to the matching driver */ -if (pdev->driver_override) -return !strcmp (pdev->driver_override, drv->name); + ret =device_match_driver_override (dev, drv); +if (ret >=0 ) +return ret;/* Then try to match against the id table */ if (pdrv->id_table)
I was just about to pick up this series, but then noticed that checking for driver_override in the platform_early case doesn't make sense in the first place and was accidentally added when the platform_match() callback was copied over in commit 507fd01d5333 ("drivers: move the early platform device support to arch/sh").
Thus, I'm going to drop this hunk and add in the following patch; please let me know if there are any concerns.
commit 39cae4095efda4b00b436c0fc46f21de84128969
Author: Danilo Krummrich <[email protected]> Date: Tue Mar 17 00:37:15 2026 +0100
sh: platform_early: remove pdev->driver_override check
In commit 507fd01d5333 ("drivers: move the early platform device support to arch/sh") platform_match() was copied over to the sh platform_early code, accidentally including the driver_override check.
This check does not make sense for platform_early, as sysfs is not even available in first place at this point in the boot process, hence remove the check.
arch/sh/drivers/platform_early.c +0 -4
@@ -26,10 +26,6 @@ static int platform_match(struct device *dev, struct device_driver *drv)struct platform_device *pdev =to_platform_device (dev);struct platform_driver *pdrv =to_platform_driver (drv); -/* When driver_override is set, only bind to the matching driver */ -if (pdev->driver_override) -return !strcmp (pdev->driver_override, drv->name); -/* Then try to match against the id table */ if (pdrv->id_table)return platform_match_id (pdrv->id_table, pdev) !=NULL ;
Greg KH
maintainerRe: [PATCH v2 4/4] driver core: platform: use generic driver_override infrastructure
Quoted 56 lines · Danilo Krummrich
(Cc: Bartosz)
Quoted 9 lines · Danilo Krummrich
arch/sh/drivers/platform_early.c +1
-0@@ -25,10 +25,12 @@ static int platform_match(struct device *dev, struct device_driver *drv) {struct platform_device *pdev =to_platform_device (dev);struct platform_driver *pdrv =to_platform_driver (drv); +int ret;arch/sh/drivers/platform_early.c static int platform_match(struct device *dev, struct device_driver *drv) · line 36
Quoted 10 lines
arch/sh/drivers/platform_early.c +3
-2/* When driver_override is set, only bind to the matching driver */ -if (pdev->driver_override) -return !strcmp (pdev->driver_override, drv->name); + ret =device_match_driver_override (dev, drv); +if (ret >=0 ) +return ret;/* Then try to match against the id table */ if (pdrv->id_table)I was just about to pick up this series, but then noticed that checking for
driver_overridein theplatform_earlycase doesn't make sense in the first place and was accidentally added when theplatform_match()callback was copied over in commit507fd01d5333("drivers: move the early platform device support toarch/sh").Thus, I'm going to drop this hunk and add in the following patch; please let me know if there are any concerns.
commit
39cae4095efda4b00b436c0fc46f21de84128969Author: Danilo Krummrich <[email protected]> Date: Tue Mar 17 00:37:15 2026 +0100sh:
platform_early: removepdev->driver_overridecheckIn commit
507fd01d5333("drivers: move the early platform device support toarch/sh")platform_match()was copied over to the shplatform_earlycode, accidentally including thedriver_overridecheck.This check does not make sense for
platform_early, as sysfs is not even available in first place at this point in the boot process, hence remove the check.Fixes 507fd01d5333 ("drivers: move the early platform device support to arch/sh")Signed-off-by Danilo Krummrich <[email protected]>arch/sh/drivers/platform_early.c +0
-0
arch/sh/drivers/platform_early.c static int platform_match(struct device *dev, struct device_driver *drv) · line 31
Quoted 10 lines
arch/sh/drivers/platform_early.c +0
-4@@ -26,10 +26,6 @@ static int platform_match(struct device *dev, struct device_driver *drv)struct platform_device *pdev =to_platform_device (dev);struct platform_driver *pdrv =to_platform_driver (drv); -/* When driver_override is set, only bind to the matching driver */ -if (pdev->driver_override) -return !strcmp (pdev->driver_override, drv->name); -/* Then try to match against the id table */ if (pdrv->id_table)return platform_match_id (pdrv->id_table, pdev) !=NULL ;
Makes sense to me, good catch!
Geert Uytterhoeven
Reviewed-byRe: [PATCH v2 4/4] driver core: platform: use generic driver_override infrastructure
In reply to Danilo KrummrichHi Danilo,
Quoted 52 lines · Danilo Krummrich
Quoted 9 lines · Danilo Krummrich
arch/sh/drivers/platform_early.c +1
-0@@ -25,10 +25,12 @@ static int platform_match(struct device *dev, struct device_driver *drv) {struct platform_device *pdev =to_platform_device (dev);struct platform_driver *pdrv =to_platform_driver (drv); +int ret;arch/sh/drivers/platform_early.c static int platform_match(struct device *dev, struct device_driver *drv) · line 36
Quoted 10 lines
arch/sh/drivers/platform_early.c +3
-2/* When driver_override is set, only bind to the matching driver */ -if (pdev->driver_override) -return !strcmp (pdev->driver_override, drv->name); + ret =device_match_driver_override (dev, drv); +if (ret >=0 ) +return ret;/* Then try to match against the id table */ if (pdrv->id_table)I was just about to pick up this series, but then noticed that checking for
driver_overridein theplatform_earlycase doesn't make sense in the first place and was accidentally added when theplatform_match()callback was copied over in commit507fd01d5333("drivers: move the early platform device support toarch/sh").Thus, I'm going to drop this hunk and add in the following patch; please let me know if there are any concerns.
commit
39cae4095efda4b00b436c0fc46f21de84128969Author: Danilo Krummrich <[email protected]> Date: Tue Mar 17 00:37:15 2026 +0100sh:
platform_early: removepdev->driver_overridecheckIn commit
507fd01d5333("drivers: move the early platform device support toarch/sh")platform_match()was copied over to the shplatform_earlycode, accidentally including thedriver_overridecheck.This check does not make sense for
platform_early, as sysfs is not even available in first place at this point in the boot process, hence remove the check.Fixes 507fd01d5333 ("drivers: move the early platform device support to arch/sh")Signed-off-by Danilo Krummrich <[email protected]>arch/sh/drivers/platform_early.c +0
-0
arch/sh/drivers/platform_early.c static int platform_match(struct device *dev, struct device_driver *drv) · line 31
Quoted 10 lines
arch/sh/drivers/platform_early.c +0
-4@@ -26,10 +26,6 @@ static int platform_match(struct device *dev, struct device_driver *drv)struct platform_device *pdev =to_platform_device (dev);struct platform_driver *pdrv =to_platform_driver (drv); -/* When driver_override is set, only bind to the matching driver */ -if (pdev->driver_override) -return !strcmp (pdev->driver_override, drv->name); -/* Then try to match against the id table */ if (pdrv->id_table)return platform_match_id (pdrv->id_table, pdev) !=NULL ;
Nice catch!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Quoted 10 lines · Danilo Krummrich
Currently, there are 12 busses (including platform and PCI) that duplicate the
driver_overridelogic for their individual devices.All of them seem to be prone to the bug described in [1].
While this could be solved for every bus individually using a separate lock, solving this in the driver-core generically results in less (and cleaner) changes overall.
Quoted 6 lines
Thus, move
driver_overrideto struct device, provide corresponding accessors for busses and handle locking with a separate lock internally.In particular, add
device_set_driver_override(),device_has_driver_override(),device_match_driver_override()and a helper,DEVICE_ATTR_DRIVER_OVERRIDE(), to declare the corresponding sysfsstore()andshow()callbacks.
Hi Danilo,
Thanks for the v2. The code looks good and the bus_type approach is much cleaner.
Just a minor note on the commit messages for the cover letter and patch 1: they still mention adding the DEVICE_ATTR_DRIVER_OVERRIDE() helper, even though it was correctly dropped in this version.
Thanks.
Quoted 48 lines
Until all busses have migrated, keep
driver_set_override()in place.Note that we can't use the device lock for the reasons described in [2].
This patch series includes the migration of the platform bus; patches for all other affected busses still need to be extracted as a follow-up of the WIP treewide patch in [3].
[1] https://bugzilla.kernel.org/show_bug.cgi?id=220789
[2] https://lore.kernel.org/driver-core/[email protected]/
[3] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=driver_overrideChanges in v2:
- DropDEVICE_ATTR_DRIVER_OVERRIDE()and makedriver_overridea structbus_typefeature instead.
- Adddriver_overridedocumentation in.../driver-model/binding.rst.
- Movekfree(dev->driver_override.name)beforerelease().
- hwmon: axi-fan: Use NULL instead ofdev_name()indevm_request_threaded_irq().Danilo Krummrich (4):
driver core: generalizedriver_overridein struct device
docs: driver-model: documentdriver_override
hwmon: axi-fan: don't usedriver_overrideas IRQ name driver core: platform: use genericdriver_overrideinfrastructure.../driver-api/driver-model/binding.rst | 48 +++++++++++++++ arch/sh/drivers/platform_early.c | 6 +- drivers/base/bus.c | 43 ++++++++++++- drivers/base/core.c | 2 + drivers/base/dd.c | 60 +++++++++++++++++++ drivers/base/platform.c | 37 ++---------- drivers/bus/simple-pm-bus.c | 4 +- drivers/clk/imx/clk-scu.c | 3 +- drivers/hwmon/axi-fan-control.c | 2 +- drivers/slimbus/qcom-ngd-ctrl.c | 6 +- include/linux/device.h | 54 +++++++++++++++++ include/linux/device/bus.h | 4 ++ include/linux/platform_device.h | 5 -- sound/soc/samsung/i2s.c | 6 +- 14 files changed, 228 insertions(+), 52 deletions(-)base-commit:
11439c4635edd669ae435eec308f4ab8a0804808--
2.53.0
Quoted 20 lines · Danilo Krummrich
Currently, there are 12 busses (including platform and PCI) that duplicate the
driver_overridelogic for their individual devices.All of them seem to be prone to the bug described in [1].
While this could be solved for every bus individually using a separate lock, solving this in the driver-core generically results in less (and cleaner) changes overall.
Thus, move
driver_overrideto struct device, provide corresponding accessors for busses and handle locking with a separate lock internally.In particular, add
device_set_driver_override(),device_has_driver_override(),device_match_driver_override()and a helper,DEVICE_ATTR_DRIVER_OVERRIDE(), to declare the corresponding sysfsstore()andshow()callbacks.Until all busses have migrated, keep
driver_set_override()in place.Note that we can't use the device lock for the reasons described in [2].
Quoted 7 lines
This patch series includes the migration of the platform bus; patches for all other affected busses still need to be extracted as a follow-up of the WIP treewide patch in [3].
[1] https://bugzilla.kernel.org/show_bug.cgi?id=220789
[2] https://lore.kernel.org/driver-core/[email protected]/
[3] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=driver_override
On Tue Mar 3, 2026 at 12:53 PM CET, Danilo Krummrich wrote:
Applied to driver-core-linus, thanks!
Quoted 1 line
driver core: generalize
driver_overridein struct device
[ Use dev->bus instead of sp->bus for consistency; fix commit message to
refer to the struct bus_type's driver_override feature flag. - Danilo ]
Quoted 2 lines
docs: driver-model: document
driver_override
hwmon: axi-fan: don't usedriver_overrideas IRQ name
sh: platform_early: remove pdev->driver_override check
Quoted 1 line
driver core: platform: use generic
driver_overrideinfrastructure