[PATCH 1/3] driver core: generalize driver_override in struct device
Danilo Krummrich <[email protected]>
| Newsgroups | gmane.linux.ports.arm.msm,gmane.linux.kernel,gmane.linux.kernel.hwmon,gmane.linux.sound,gmane.linux.ports.sh.devel |
|---|---|
| Message-ID | <[email protected]> |
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]. Link: https://bugzilla.kernel.org/show_bug.cgi?id=220789 [1] Link: https://lore.kernel.org/driver-core/[email protected]/ [2] Signed-off-by: Danilo Krummrich <[email protected]> --- drivers/base/core.c | 2 ++ drivers/base/dd.c | 60 +++++++++++++++++++++++++++++++ include/linux/device.h | 81 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 143 insertions(+) diff --git a/drivers/base/core.c b/drivers/base/core.c index 791f9e444df8..a8cb90577d10 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -2566,6 +2566,7 @@ static void device_release(struct kobject *kobj) else WARN(1, KERN_ERR "Device '%s' does not have a release() function, it is broken and must be fixed. See Documentation/core-api/kobject.rst.\n", dev_name(dev)); + kfree(dev->driver_override.name); kfree(p); } @@ -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); diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 0354f209529c..697e36e63cab 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -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 diff --git a/include/linux/device.h b/include/linux/device.h index 0be95294b6e6..4599156d5cbd 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -266,6 +266,33 @@ ssize_t device_show_string(struct device *dev, struct device_attribute *attr, struct dev_ext_attribute dev_attr_##_name = \ { __ATTR(_name, (_mode) & ~0222, device_show_string, NULL), (_var) } +/** + * DEVICE_ATTR_DRIVER_OVERRIDE - Define sysfs driver_override attribute callbacks + * + * Generates the standard driver_override_show() and driver_override_store() + * sysfs callbacks and the static DEVICE_ATTR_RW(driver_override) declaration. + */ +#define DEVICE_ATTR_DRIVER_OVERRIDE() \ +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) + #define DEVICE_ATTR_IGNORE_LOCKDEP(_name, _mode, _show, _store) \ struct device_attribute dev_attr_##_name = \ __ATTR_IGNORE_LOCKDEP(_name, _mode, _show, _store) @@ -483,6 +510,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 +605,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 +734,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, strlen(s)); +} + +/** + * 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 -- 2.53.0