[PATCH RFC] Specifying default-disabled devices

[Zev Weiss]

Hi all,

I'd like to hear people's thoughts on how to approach specifying that a device
is present, but should be left alone by default.  The specific example I have
at hand is that of the host firmware (BIOS) SPI flash -- it's attached to a SPI
controller on my BMC, but actually accessing it via that interface requires a
bit of a dance involving three GPIOs and exchanging some IPMI commands with the
ME, so if I have it defined in my dts, the mtd driver's probe routine during
the BMC's boot sequence just spews errors into dmesg and fails (I can then
manually bind it later via sysfs after fiddling with GPIOs and IPMI from
userspace).  This "works" in its clumsy way in this specific case, but I'd like
to try to upstream a cleaner (and hopefully reasonably general) solution.

What I've had patched into my local fork for a while is a 'noautobind' kernel
command-line argument that lists devices to skip automatic probing on, and then
just a simple check in driver_probe_device() against that list (bypassed if
called from a sysfs 'bind' write).  (Patch for dev-5.10 inline below, though it
doesn't currently apply to mainline.)

The other alternative I've considered (though not actually implemented thus
far) is to start using the "reserved" status value defined in the device-tree
spec (section 2.3.4, [0]) to mean this -- from the wording in the spec it seems
like a not-terribly-unreasonable interpretation, and the existing kernel &
u-boot fdt code don't seem to make any use of it as far as I can see (though I
don't know what might be out there in device-tree implementations floating
around in other projects).

Anyone have any particular thoughts?  The 'noautobind' approach is a pretty
small, simple patch, and I'd guess a 'status = "reserved"' patch probably
wouldn't be much bigger, but I figured it might be worth it to solicit opinions
from OpenBMC people before potentially ruffling driver-core/device-tree
feathers on upstream lists.


Thanks,
Zev

[0] https://github.com/devicetree-org/devicetree-specification/releases/download/v0.3/devicetree-specification-v0.3.pdf

---
 drivers/base/base.h |  2 +-
 drivers/base/bus.c  |  2 +-
 drivers/base/dd.c   | 33 +++++++++++++++++++++++++++------
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/base/base.h b/drivers/base/base.h
index 52b3d7b75c27..98bd131dacfc 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -152,7 +152,7 @@ extern int driver_add_groups(struct device_driver *drv,
 			     const struct attribute_group **groups);
 extern void driver_remove_groups(struct device_driver *drv,
 				 const struct attribute_group **groups);
-int device_driver_attach(struct device_driver *drv, struct device *dev);
+int device_driver_attach(struct device_driver *drv, struct device *dev, bool explicit);
 void device_driver_detach(struct device *dev);
 
 extern char *make_class_name(const char *name, struct kobject *kobj);
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 36d0c654ea61..4a672a7051e5 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -211,7 +211,7 @@ static ssize_t bind_store(struct device_driver *drv, const char *buf,
 
 	dev = bus_find_device_by_name(bus, NULL, buf);
 	if (dev && dev->driver == NULL && driver_match_device(drv, dev)) {
-		err = device_driver_attach(drv, dev);
+		err = device_driver_attach(drv, dev, true);
 
 		if (err > 0) {
 			/* success */
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index e2cf3b29123e..bf723a30bbaf 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -62,6 +62,10 @@ static bool initcalls_done;
 #define ASYNC_DRV_NAMES_MAX_LEN	256
 static char async_probe_drv_names[ASYNC_DRV_NAMES_MAX_LEN];
 
+/* Saved copy of noautobind kernel cmdline arg */
+#define NOAUTOBIND_DEVS_MAX_LEN 256
+static char noautobind_dev_names[NOAUTOBIND_DEVS_MAX_LEN];
+
 /*
  * In some cases, like suspend to RAM or hibernation, It might be reasonable
  * to prohibit probing of devices as it could be unsafe.
@@ -713,6 +717,7 @@ EXPORT_SYMBOL_GPL(wait_for_device_probe);
  * driver_probe_device - attempt to bind device & driver together
  * @drv: driver to bind a device to
  * @dev: device to try to bind to the driver
+ * @explicit: whether or not this is an explicit bind request from userspace
  *
  * This function returns -ENODEV if the device is not registered,
  * 1 if the device is bound successfully and 0 otherwise.
@@ -722,13 +727,18 @@ EXPORT_SYMBOL_GPL(wait_for_device_probe);
  *
  * If the device has a parent, runtime-resume the parent before driver probing.
  */
-static int driver_probe_device(struct device_driver *drv, struct device *dev)
+static int driver_probe_device(struct device_driver *drv, struct device *dev, bool explicit)
 {
 	int ret = 0;
 
 	if (!device_is_registered(dev))
 		return -ENODEV;
 
+	if (!explicit && parse_option_str(noautobind_dev_names, dev_name(dev))) {
+		dev_info(dev, "skipping %s probe of noautobind device\n", drv->name);
+		return -EPROBE_DEFER;
+	}
+
 	pr_debug("bus: '%s': %s: matched device %s with driver %s\n",
 		 drv->bus->name, __func__, dev_name(dev), drv->name);
 
@@ -766,6 +776,16 @@ static int __init save_async_options(char *buf)
 }
 __setup("driver_async_probe=", save_async_options);
 
+static int __init save_noautobind_options(char *buf)
+{
+	if (strlen(buf) >= NOAUTOBIND_DEVS_MAX_LEN)
+		pr_warn("'noautobind' device name list too long!\n");
+
+	strlcpy(noautobind_dev_names, buf, NOAUTOBIND_DEVS_MAX_LEN);
+	return 0;
+}
+__setup("noautobind=", save_noautobind_options);
+
 bool driver_allows_async_probing(struct device_driver *drv)
 {
 	switch (drv->probe_type) {
@@ -846,7 +866,7 @@ static int __device_attach_driver(struct device_driver *drv, void *_data)
 	if (data->check_async && async_allowed != data->want_async)
 		return 0;
 
-	return driver_probe_device(drv, dev);
+	return driver_probe_device(drv, dev, false);
 }
 
 static void __device_attach_async_helper(void *_dev, async_cookie_t cookie)
@@ -1000,11 +1020,12 @@ static void __device_driver_unlock(struct device *dev, struct device *parent)
  * device_driver_attach - attach a specific driver to a specific device
  * @drv: Driver to attach
  * @dev: Device to attach it to
+ * @explicit: whether or not this is an explicit bind request from userspace
  *
  * Manually attach driver to a device. Will acquire both @dev lock and
  * @dev->parent lock if needed.
  */
-int device_driver_attach(struct device_driver *drv, struct device *dev)
+int device_driver_attach(struct device_driver *drv, struct device *dev, bool explicit)
 {
 	int ret = 0;
 
@@ -1015,7 +1036,7 @@ int device_driver_attach(struct device_driver *drv, struct device *dev)
 	 * bound a driver before us just skip the driver probe call.
 	 */
 	if (!dev->p->dead && !dev->driver)
-		ret = driver_probe_device(drv, dev);
+		ret = driver_probe_device(drv, dev, explicit);
 
 	__device_driver_unlock(dev, dev->parent);
 
@@ -1037,7 +1058,7 @@ static void __driver_attach_async_helper(void *_dev, async_cookie_t cookie)
 	 * bound a driver before us just skip the driver probe call.
 	 */
 	if (!dev->p->dead && !dev->driver)
-		ret = driver_probe_device(drv, dev);
+		ret = driver_probe_device(drv, dev, false);
 
 	__device_driver_unlock(dev, dev->parent);
 
@@ -1092,7 +1113,7 @@ static int __driver_attach(struct device *dev, void *data)
 		return 0;
 	}
 
-	device_driver_attach(drv, dev);
+	device_driver_attach(drv, dev, false);
 
 	return 0;
 }
-- 
2.33.0