[PATCH 2.6.10-rc1 0/4] driver-model: manual device attach

[PATCH 2.6.10-rc1 0/4] driver-model: manual device attach

[Tejun Heo]

 Hello, again. :-)

 These are the manual device attach patches I was talking about in the
previous posting.  These patches need devparam patches to be applied
first.  It's composed of two parts.

 1. sysctl node dev.autoattach

 dev.autoattach is read/write integer sysctl node which controls
driver-model's behavior regarding device - driver association.

 0: autoattach disabled.  devices are not associated with drivers
    automatically.  i.e. insmod'ing e100.ko won't cause it to attach to the
    actual e100 devices.
 1: autoattach enabled.  The default value.  This is the same as the
    current driver model behavior.  Driver model automatically associates
    devices to drivers.
 2: rescan command.  If this value is written, bus_rescan_devices() is
    invoked for all the registered bus types; thus attaching all
    devices to available drivers.  After rescan is complete, the
     autoattach value is set to 1.

 2. per-device attach and detach sysfs node.

 Two files named attach and detach are created under each device's
sysfs directory.  Reading attach node shows the name of applicable
drivers.  Writing a driver name attaches the device to the driver.
Also, per-device parameters can be specified when writing to an attach
node.  Writing anything to the write-only detach node detaches the
driver from the currently associated driver.

========= So, for example, on my machine which has two e100's...

# pwd
/sys/bus/pci/devices/0000:00:04.0
# sysctl dev.autoattach
dev.autoattach = 1
# modprobe e100
e100: Intel(R) PRO/100 Network Driver, 3.2.3-k2-NAPI
e100: Copyright(c) 1999-2004 Intel Corporation
e100: eth0: e100_probe: addr 0xfeafe000, irq 20, MAC addr 00:E0:81:01:7F:F3
e100: eth1: e100_probe: addr 0xfeafd000, irq 21, MAC addr 00:E0:81:01:7F:F4
# modprobe eepro100
eepro100.c:v1.09j-t 9/29/99 Donald Becker http://www.scyld.com/network...
eepro100.c: $Revision: 1.36 $ 2000/11/17 Modified by Andrey V. Savochkin...
# ls -l driver
lrwxrwxrwx  1 root root 0 Nov  4 16:26 driver -> ../../../bus/pci/drivers/e100
# rmmod e100
# rmmod eepro100
# sysctl -w dev.autoattach=0
dev.autoattach = 0
# modprobe e100
e100: Intel(R) PRO/100 Network Driver, 3.2.3-k2-NAPI
e100: Copyright(c) 1999-2004 Intel Corporation
# modprobe eepro100
eepro100.c:v1.09j-t 9/29/99 Donald Becker http://www.scyld.com/network...
eepro100.c: $Revision: 1.36 $ 2000/11/17 Modified by Andrey V. Savochkin...
# ls -l driver
ls: driver: No such file or directory
# cat attach
e100
eepro100
# echo eepro100 > attach
eth0: OEM i82557/i82558 10/100 Ethernet, 00:E0:81:01:7F:F3, IRQ 20.
  Receiver lock-up bug exists -- enabling work-around.
  Board assembly 123456-120, Physical connectors present: RJ45
  ...
# ls -l driver
lrwxrwxrwx  1 root root 0 Nov  4 16:27 driver -> ../../../bus/pci/drivers/eepro100
# sysctl -w dev.autoattach=2
e100: eth1: e100_probe: addr 0xfeafd000, irq 21, MAC addr 00:E0:81:01:7F:F4
dev.autoattach = 2

======== And, drivers can accept per-device parameters like the following.

# pwd
/sys/bus/dp/devices/dp_dev1
# ls -l
total 0
-rw-r--r--  1 root root 4096 Nov  4 16:34 attach
--w-------  1 root root 4096 Nov  4 16:34 detach
-rw-r--r--  1 root root 4096 Nov  4 16:34 detach_state
# cat attach
babo
# echo babo n=15 opts=0,9,8 dp_class.integer=12 > attach
dp_bus: 0 1 2 1,2,3,0,0,0 cnt=3
dp_drv: 15 0,9,8,0 cnt=3
dp_class: 12 120 "dp_class"
# ls -l driver
lrwxrwxrwx  1 root root 0 Nov  4 16:35 driver -> ../../bus/dp/drivers/babo
# ls parameters/
a  ar  b  byte  c  charp  integer  n  opts

===========

 We'll need to expand user-space hotplug facility to make full use of
manualattach feature.  I think with a bit more information exported to
userland and some standardized parameters (i.e. `name' parameter for
all class devices), we'll be able to give high-granuality control over
driver-model to users.

 What do you guys think?  I think this can be quite useful with right
user-space tools.

 Thanks.

--
tejun

Re: [PATCH 2.6.10-rc1 1/4] driver-model: sysctl node dev.autoattach

[Tejun Heo]

 ma_01_sysctl_dev_autoattach.patch

 This patch implements sysctl node dev.autoattach.


Signed-off-by: Tejun Heo <tj@home-tj.org>


Index: linux-export/drivers/base/bus.c
===================================================================
--- linux-export.orig/drivers/base/bus.c	2004-11-04 11:04:13.000000000 +0900
+++ linux-export/drivers/base/bus.c	2004-11-04 11:04:14.000000000 +0900
@@ -17,6 +17,12 @@
 #include "base.h"
 #include "power/power.h"
 
+int dev_autoattach = 1, dev_autoattach_min = 0, dev_autoattach_max = 2;
+
+static LIST_HEAD(all_buses);
+static spinlock_t all_buses_lock = SPIN_LOCK_UNLOCKED;
+static DECLARE_MUTEX(all_buses_traverse_mutex);
+
 #define to_dev(node) container_of(node, struct device, bus_list)
 #define to_drv(node) container_of(node, struct device_driver, kobj.entry)
 
@@ -329,7 +335,7 @@ int device_attach(struct device * dev)
 		return 1;
 	}
 
-	if (bus->match) {
+	if (dev_autoattach && bus->match) {
 		list_for_each(entry, &bus->drivers.list) {
 			struct device_driver * drv = to_drv(entry);
 			error = driver_probe_device(drv, dev);
@@ -366,7 +372,7 @@ void driver_attach(struct device_driver 
 	struct list_head * entry;
 	int error;
 
-	if (!bus->match)
+	if (!dev_autoattach || !bus->match)
 		return;
 
 	list_for_each(entry, &bus->devices.list) {
@@ -723,6 +729,10 @@ int bus_register(struct bus_type * bus)
 		goto bus_drivers_fail;
 	bus_add_attrs(bus);
 
+	spin_lock(&all_buses_lock);
+	list_add_tail(&bus->node, &all_buses);
+	spin_unlock(&all_buses_lock);
+
 	pr_debug("bus type '%s' registered\n", bus->name);
 	return 0;
 
@@ -745,12 +755,61 @@ out:
 void bus_unregister(struct bus_type * bus)
 {
 	pr_debug("bus %s: unregistering\n", bus->name);
+
+	spin_lock(&all_buses_lock);
+	list_del(&bus->node);
+	spin_unlock(&all_buses_lock);
+
 	bus_remove_attrs(bus);
 	kset_unregister(&bus->drivers);
 	kset_unregister(&bus->devices);
 	subsystem_unregister(&bus->subsys);
 }
 
+
+/**
+ *	dev_autoattach_handler - proc_handler for sysctl node dev.autoattach
+ */
+int dev_autoattach_handler(ctl_table *table, int write, struct file *filp,
+			   void __user *buffer, size_t *lenp, loff_t *ppos)
+{
+	int ret;
+
+	ret = proc_dointvec_minmax(table, write, filp, buffer, lenp, ppos);
+
+	down(&all_buses_traverse_mutex);
+
+	if (dev_autoattach == 2) {
+		struct list_head marker;
+
+		spin_lock(&all_buses_lock);
+		list_add(&marker, &all_buses);
+		while (marker.next != &all_buses) {
+			struct bus_type *bus;
+			bus = container_of(marker.next, struct bus_type, node);
+			if (!(bus = get_bus(bus)))
+				continue;
+			/* Okay, we have the next bus, move it ahead
+			   of the marker and perform rescan. */
+			list_del(&bus->node);
+			list_add_tail(&bus->node, &marker);
+			spin_unlock(&all_buses_lock);
+
+			bus_rescan_devices(bus);
+
+			put_bus(bus);
+			spin_lock(&all_buses_lock);
+		}
+		list_del(&marker);
+		spin_unlock(&all_buses_lock);
+		dev_autoattach = 1;
+	}
+
+	up(&all_buses_traverse_mutex);
+
+	return ret;
+}
+
 int __init buses_init(void)
 {
 	return subsystem_register(&bus_subsys);
Index: linux-export/include/linux/device.h
===================================================================
--- linux-export.orig/include/linux/device.h	2004-11-04 11:04:12.000000000 +0900
+++ linux-export/include/linux/device.h	2004-11-04 11:04:14.000000000 +0900
@@ -20,6 +20,7 @@
 #include <linux/module.h>
 #include <linux/pm.h>
 #include <linux/deviceparam.h>
+#include <linux/sysctl.h>
 #include <asm/semaphore.h>
 #include <asm/atomic.h>
 
@@ -54,6 +55,7 @@ struct bus_type {
 	struct subsystem	subsys;
 	struct kset		drivers;
 	struct kset		devices;
+	struct list_head	node;
 
 	struct bus_attribute	* bus_attrs;
 	struct device_attribute	* dev_attrs;
@@ -414,6 +416,12 @@ extern void device_shutdown(void);
 extern int firmware_register(struct subsystem *);
 extern void firmware_unregister(struct subsystem *);
 
+/* dev.autoattach sysctl node */
+extern int dev_autoattach, dev_autoattach_min, dev_autoattach_max;
+extern int dev_autoattach_handler(ctl_table *table, int write,
+				  struct file *filp, void __user *buffer,
+				  size_t *lenp, loff_t *ppos);
+
 /* debugging and troubleshooting/diagnostic helpers. */
 #define dev_printk(level, dev, format, arg...)	\
 	printk(level "%s %s: " format , (dev)->driver ? (dev)->driver->name : "" , (dev)->bus_id , ## arg)
Index: linux-export/include/linux/sysctl.h
===================================================================
--- linux-export.orig/include/linux/sysctl.h	2004-11-04 10:25:58.000000000 +0900
+++ linux-export/include/linux/sysctl.h	2004-11-04 11:04:14.000000000 +0900
@@ -691,6 +691,7 @@ enum {
 	DEV_RAID=4,
 	DEV_MAC_HID=5,
 	DEV_SCSI=6,
+	DEV_AUTOATTACH=7,
 };
 
 /* /proc/sys/dev/cdrom */
Index: linux-export/kernel/sysctl.c
===================================================================
--- linux-export.orig/kernel/sysctl.c	2004-11-04 10:25:58.000000000 +0900
+++ linux-export/kernel/sysctl.c	2004-11-04 11:04:14.000000000 +0900
@@ -41,6 +41,7 @@
 #include <linux/limits.h>
 #include <linux/dcache.h>
 #include <linux/syscalls.h>
+#include <linux/device.h>
 
 #include <asm/uaccess.h>
 #include <asm/processor.h>
@@ -936,6 +937,16 @@ static ctl_table debug_table[] = {
 };
 
 static ctl_table dev_table[] = {
+	{
+		.ctl_name	= DEV_AUTOATTACH,
+		.procname	= "autoattach",
+		.data		= &dev_autoattach,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= &dev_autoattach_handler,
+		.extra1		= &dev_autoattach_min,
+		.extra2		= &dev_autoattach_max,
+	},
 	{ .ctl_name = 0 }
 };  
 

Re: [PATCH 2.6.10-rc1 2/4] driver-model: devparam expanded to accept direct per-device parameters via @args argument

[Tejun Heo]

 ma_02_devparam_set_by_args.patch

 This patch implements set_devparams_by_args() and expands related
functions to accept an const char * @args argument.  This enables
setting per-device parameters directly from argument string rather
than from parameters stored while booting or loading modules.


Signed-off-by: Tejun Heo <tj@home-tj.org>


Index: linux-export/drivers/base/bus.c
===================================================================
--- linux-export.orig/drivers/base/bus.c	2004-11-04 14:40:01.000000000 +0900
+++ linux-export/drivers/base/bus.c	2004-11-04 14:40:01.000000000 +0900
@@ -287,7 +287,8 @@ void device_bind_driver(struct device * 
  *	If we find a match, we call @drv->probe(@dev) if it exists, and
  *	call device_bind_driver() above.
  */
-int driver_probe_device(struct device_driver * drv, struct device * dev)
+int driver_probe_device(struct device_driver * drv, struct device * dev,
+			const char *args)
 {
 	int error;
 
@@ -296,7 +297,7 @@ int driver_probe_device(struct device_dr
 
 	dev->driver = drv;
 
-	if ((error = devparam_set_params(dev)) != 0)
+	if ((error = devparam_set_params(dev, args)) != 0)
 		goto devparam_fail;
 
 	if (drv->probe) {
@@ -338,7 +339,7 @@ int device_attach(struct device * dev)
 	if (dev_autoattach && bus->match) {
 		list_for_each(entry, &bus->drivers.list) {
 			struct device_driver * drv = to_drv(entry);
-			error = driver_probe_device(drv, dev);
+			error = driver_probe_device(drv, dev, NULL);
 			if (!error)
 				/* success, driver matched */
 				return 1;
@@ -378,7 +379,7 @@ void driver_attach(struct device_driver 
 	list_for_each(entry, &bus->devices.list) {
 		struct device * dev = container_of(entry, struct device, bus_list);
 		if (!dev->driver) {
-			error = driver_probe_device(drv, dev);
+			error = driver_probe_device(drv, dev, NULL);
 			if (error && (error != -ENODEV))
 				/* driver matched but the probe failed */
 				printk(KERN_WARNING
Index: linux-export/drivers/base/deviceparam.c
===================================================================
--- linux-export.orig/drivers/base/deviceparam.c	2004-11-04 14:39:59.000000000 +0900
+++ linux-export/drivers/base/deviceparam.c	2004-11-04 14:40:01.000000000 +0900
@@ -9,6 +9,7 @@
 #include <linux/deviceparam.h>
 #include <linux/device.h>
 #include <linux/module.h>
+#include <linux/ctype.h>
 
 #if 0
 #define pdebug(fmt, args...) printk("[%-25s] " fmt, __FUNCTION__ , ##args)
@@ -800,27 +801,113 @@ static int set_devparams_by_storedparams
 	return 0;
 }
 
-int devparam_set_params(struct device *dev)
+static int set_devparams_by_args(struct device *dev, char *args)
 {
 	struct device_driver *drv = dev->driver;
+	int which, i, err;
+	long **bitmaps;
 	size_t size;
-	int which, ret;
 	struct device_paramset_def *setdef;
+	char *param, *val;
+
+	/* Allocate bitmaps */
+	err = -ENOMEM;
+
+	size = drv->nr_paramsets * sizeof(bitmaps[0]);
+	if (!(bitmaps = kmalloc(size, GFP_KERNEL)))
+		goto out;
+	memset(bitmaps, 0, size);
+
+	for_each_setdef(which, setdef, drv) {
+		size = ALIGN(setdef->nr_defs, 8) / 8;
+		if (!(bitmaps[which] = kmalloc(size, GFP_KERNEL)))
+			goto out;
+		memset(bitmaps[which], 0, size);
+	}
+
+	/* Okay, parse */
+	while (*args) {
+		int ret;
+		args = param_next_arg(args, &param, &val);
+		pdebug("param=\"%s\" val=\"%s\"\n", param, val);
+		ret = parse_one_devparam(param, param, val,
+					 drv, NULL, dev, bitmaps);
+		if (ret == -ENOENT)
+			printk(KERN_ERR "Device params: Unknown parameter "
+			       "`%s'\n", param);
+	}
+
+	/* Finalize */
+	for_each_setdef(which, setdef, drv) {
+		for (i = 0; i < setdef->nr_defs; i++) {
+			struct device_param_def *def = &setdef->defs[i];
+
+			if (test_bit(i, bitmaps[which]) || def->dfl == NULL)
+				continue;
+
+			err = call_set(def, NULL, dev->paramsets[which]);
+			if (err < 0)
+				goto out;
+		}
+	}
 
-	pdebug("invoked for %s\n", drv->name);
+	err = 0;
+
+ out:
+	if (bitmaps) {
+		for (i = 0; i < drv->nr_paramsets; i++)
+			kfree(bitmaps[i]);
+		kfree(bitmaps);
+	}
+	return err;
+}
+
+int devparam_set_params(struct device *dev, const char *args)
+{
+	struct device_driver *drv = dev->driver;
+	char *buf;
+	size_t size, args_len;
+	int which, ret;
+	struct device_paramset_def *setdef;
 
+	pdebug("invoked for %s args=\"%s\"\n", drv->name, args);
 	dev->paramsets = NULL;
 	dev->bus_paramset = NULL;
 	dev->paramset_idx = -1;
 
-	if (drv->nr_paramsets == 0)
+	/* Trim leading white spaces */
+	if (args) {
+		while (isspace(*args))
+			args++;
+	}
+
+	if (drv->nr_paramsets == 0) {
+		if (args && *args)
+			printk(KERN_ERR "Device params: %s does not take any "
+			       "device parameters\n", drv->name);
 		return 0;
+	}
 
 	size = drv->nr_paramsets * sizeof(void *);
-	if ((dev->paramsets = kmalloc(size, GFP_KERNEL)) == NULL)
+	args_len = args ? strlen(args) + 1 : 0;
+
+	dev->paramsets = kmalloc(size + args_len, GFP_KERNEL);
+	if (dev->paramsets == NULL)
 		return -ENOMEM;
+
 	memset(dev->paramsets, 0, size);
 
+	buf = NULL;
+	if (args) {
+		char *p;
+		buf = (char *)dev->paramsets + size;
+		memcpy(buf, args, args_len);
+		/* Trim tailing white spaces */
+		p = buf + args_len - 2;
+		while (p >= buf && isspace(*p))
+			*p-- = '\0';
+	}
+
 	for_each_setdef(which, setdef, drv) {
 		void *ps;
 		if ((ps = kmalloc(setdef->size, GFP_KERNEL)) == NULL) {
@@ -831,7 +918,12 @@ int devparam_set_params(struct device *d
 		dev->paramsets[which] = ps;
 	}
 
-	if ((ret = set_devparams_by_storedparams(dev)) < 0)
+	if (buf)
+		ret = set_devparams_by_args(dev, buf);
+	else
+		ret = set_devparams_by_storedparams(dev);
+
+	if (ret < 0)
 		goto free_out;
 	
 	if (drv->bus && drv->bus->paramset_def)
Index: linux-export/include/linux/device.h
===================================================================
--- linux-export.orig/include/linux/device.h	2004-11-04 14:40:01.000000000 +0900
+++ linux-export/include/linux/device.h	2004-11-04 14:40:01.000000000 +0900
@@ -342,7 +342,8 @@ extern int device_for_each_child(struct 
  * Manual binding of a device to driver. See drivers/base/bus.c
  * for information on use.
  */
-extern int  driver_probe_device(struct device_driver * drv, struct device * dev);
+extern int  driver_probe_device(struct device_driver * drv, struct device * dev,
+				const char *args);
 extern void device_bind_driver(struct device * dev);
 extern void device_release_driver(struct device * dev);
 extern int  device_attach(struct device * dev);
Index: linux-export/include/linux/deviceparam.h
===================================================================
--- linux-export.orig/include/linux/deviceparam.h	2004-11-04 14:39:59.000000000 +0900
+++ linux-export/include/linux/deviceparam.h	2004-11-04 14:40:01.000000000 +0900
@@ -220,7 +220,7 @@ int devparam_module_init(struct module *
 int devparam_unknown_modparam(char *name, char *val, void *arg);
 void devparam_module_done(struct module *module);
 
-int devparam_set_params(struct device *dev);
+int devparam_set_params(struct device *dev, const char *args);
 void devparam_release_params(struct device *dev);
 
 #endif /*__LINUX_DEVICEPARAM_H*/

Re: [PATCH 2.6.10-rc1 3/4] driver-model: detach_state functions renamed

[Tejun Heo]

 ma_03_detach_state_fn_rename.patch

 This patch renames detach_{show,store}() functions which is used for
detach_state device interface node to detach_state_{show,store}().


Signed-off-by: Tejun Heo <tj@home-tj.org>


Index: linux-export/drivers/base/interface.c
===================================================================
--- linux-export.orig/drivers/base/interface.c	2004-11-04 10:25:59.000000000 +0900
+++ linux-export/drivers/base/interface.c	2004-11-04 11:04:15.000000000 +0900
@@ -27,12 +27,13 @@
  *	driver's suspend method.
  */
 
-static ssize_t detach_show(struct device * dev, char * buf)
+static ssize_t detach_state_show(struct device * dev, char * buf)
 {
 	return sprintf(buf, "%u\n", dev->detach_state);
 }
 
-static ssize_t detach_store(struct device * dev, const char * buf, size_t n)
+static ssize_t detach_state_store(struct device * dev,
+				  const char * buf, size_t n)
 {
 	u32 state;
 	state = simple_strtoul(buf, NULL, 10);
@@ -42,7 +43,7 @@ static ssize_t detach_store(struct devic
 	return n;
 }
 
-static DEVICE_ATTR(detach_state, 0644, detach_show, detach_store);
+static DEVICE_ATTR(detach_state, 0644, detach_state_show, detach_state_store);
 
 
 struct attribute * dev_default_attrs[] = {

Re: [PATCH 2.6.10-rc1 4/4] driver-model: attach/detach sysfs node implemented

[Tejun Heo]

 ma_04_manual_attach.patch

 This patch implements device interface nodes attach and detach.
Reading attach node shows the name of applicable drivers.  Writing a
driver name attaches the device to the driver.  Writing anything to
the write-only detach node detaches the driver from the currently
associated driver.


Signed-off-by: Tejun Heo <tj@home-tj.org>


Index: linux-export/drivers/base/interface.c
===================================================================
--- linux-export.orig/drivers/base/interface.c	2004-11-04 11:04:15.000000000 +0900
+++ linux-export/drivers/base/interface.c	2004-11-04 11:04:15.000000000 +0900
@@ -13,6 +13,7 @@
 #include <linux/err.h>
 #include <linux/stat.h>
 #include <linux/string.h>
+#include <linux/ctype.h>
 
 /**
  *	detach_state - control the default power state for the device.
@@ -46,7 +47,113 @@ static ssize_t detach_state_store(struct
 static DEVICE_ATTR(detach_state, 0644, detach_state_show, detach_state_store);
 
 
+/**
+ *	attach - manually attaches the device to the specified driver
+ *
+ *	When read, this node shows the list of the attachable drivers.
+ *	Writing the name of a driver attaches the device to the
+ *	driver.
+ */
+
+struct attach_show_arg {
+	struct device * dev;
+	char * buf;
+	size_t left;
+};
+
+static int attach_show_helper(struct device_driver * drv, void * void_arg)
+{
+	struct attach_show_arg * arg = void_arg;
+	int ret;
+
+	if (drv->bus->match(arg->dev, drv)) {
+		ret = snprintf(arg->buf, arg->left, "%s\n", drv->name);
+		if (ret >= arg->left)
+			return -ENOSPC;
+		arg->buf += ret;
+		arg->left -= ret;
+	}
+
+	return 0;
+}
+
+static ssize_t attach_show(struct device * dev, char * buf)
+{
+	struct attach_show_arg arg = { dev, buf, PAGE_SIZE };
+	int ret = 0;
+
+	if (dev->bus->match)
+		ret = bus_for_each_drv(dev->bus, NULL, &arg, attach_show_helper);
+
+	return ret ?: PAGE_SIZE - arg.left;
+}
+
+static int attach_store_helper(struct device_driver * drv, void * arg)
+{
+	const char * p = *(void **)arg;
+	int len;
+
+	len = strlen(drv->name);
+	if (!strncmp(drv->name, p, len) &&
+	    (p[len] == '\0' || isspace(p[len]))) {
+		*(void **)(arg) = get_driver(drv);
+		return 1;
+	}
+
+	return 0;
+}
+
+static ssize_t attach_store(struct device * dev, const char * buf, size_t n)
+{
+	void * arg = (void *)buf;
+	struct device_driver *drv;
+	int error;
+
+	if (bus_for_each_drv(dev->bus, NULL, &arg, attach_store_helper) == 0)
+		return -ENOENT;
+	drv = arg;
+
+	/* Skip driver name */
+	while (*buf != '\0' && !isspace(*buf))
+		buf++;
+
+	/* Attach */
+	error = -EBUSY;
+	down_write(&dev->bus->subsys.rwsem);
+	if (dev->driver == NULL)
+		error = driver_probe_device(drv, dev, buf);
+	up_write(&dev->bus->subsys.rwsem);
+
+	if (error)
+		printk(KERN_WARNING "%s: probe of %s failed with error %d\n",
+		       drv->name, dev->bus_id, error);
+
+	return error ?: n;
+}
+
+static DEVICE_ATTR(attach, 0644, attach_show, attach_store);
+
+
+/**
+ *	detach - manually detaches the device from its associated driver.
+ *
+ *	This is a write-only node.  When any value is written, it detaches
+ *	the device from its associated driver.
+ */
+static ssize_t detach_store(struct device * dev, const char * buf, size_t n)
+{
+	down_write(&dev->bus->subsys.rwsem);
+	device_release_driver(dev);
+	up_write(&dev->bus->subsys.rwsem);
+	return n;
+}
+
+static DEVICE_ATTR(detach, 0200, NULL, detach_store);
+
+
 struct attribute * dev_default_attrs[] = {
 	&dev_attr_detach_state.attr,
+	&dev_attr_attach.attr,
+	&dev_attr_detach.attr,
 	NULL,
 };

Re: [PATCH 2.6.10-rc1 4/4] driver-model: attach/detach sysfs node implemented

[Dmitry Torokhov]

On Thursday 04 November 2004 02:46 am, Tejun Heo wrote:
>  ma_04_manual_attach.patch
> 
>  This patch implements device interface nodes attach and detach.
> Reading attach node shows the name of applicable drivers.  Writing a
> driver name attaches the device to the driver.  Writing anything to
> the write-only detach node detaches the driver from the currently
> associated driver.
> 
...
> +/**
> + *   detach - manually detaches the device from its associated driver.
> + *
> + *   This is a write-only node.  When any value is written, it detaches
> + *   the device from its associated driver.
> + */
> +static ssize_t detach_store(struct device * dev, const char * buf, size_t
> n)
> +{
> +     down_write(&dev->bus->subsys.rwsem);
> +     device_release_driver(dev);
> +     up_write(&dev->bus->subsys.rwsem);
> +     return n;
> +}

This will not work for pretty much any bus but PCI because only PCI
allows to detach a driver leaving children devices on the bus. The
rest of buses remove children devices when disconnecting parent.

Also, there usually much more going on with regard to locking and
other bus-specific actions besides taking bus's rwsem when binding
devices. Serio bus will definitely get upset if you try to disconnect
even a leaf device in the manner presented above and I think USB
will get upset as well.

I have tried the naïve approach as well but in the end we need bus
-specific helper to do manual connect/disconnect. Please take a look
at these:

http://marc.theaimsgroup.com/?l=linux-kernel&m=109908274124446&w=2
http://marc.theaimsgroup.com/?l=linux-kernel&m=109912528510337&w=2
http://marc.theaimsgroup.com/?l=linux-kernel&m=109912553831130&w=2
http://marc.theaimsgroup.com/?l=linux-kernel&m=109912553827412&w=2

-- 
Dmitry

Re: [PATCH 2.6.10-rc1 4/4] driver-model: attach/detach sysfs node implemented

[Greg KH]

On Thu, Nov 04, 2004 at 12:05:31PM -0500, Dmitry Torokhov wrote:
> On Thursday 04 November 2004 02:46 am, Tejun Heo wrote:
> > ?ma_04_manual_attach.patch
> > 
> > ?This patch implements device interface nodes attach and detach.
> > Reading attach node shows the name of applicable drivers. ?Writing a
> > driver name attaches the device to the driver. ?Writing anything to
> > the write-only detach node detaches the driver from the currently
> > associated driver.
> > 
> ...
> > +/**
> > + *???detach - manually detaches the device from its associated driver.
> > + *
> > + *???This is a write-only node. ?When any value is written, it detaches
> > + *???the device from its associated driver.
> > + */
> > +static ssize_t detach_store(struct device * dev, const char * buf, size_t
> > n)
> > +{
> > +?????down_write(&dev->bus->subsys.rwsem);
> > +?????device_release_driver(dev);
> > +?????up_write(&dev->bus->subsys.rwsem);
> > +?????return n;
> > +}
> 
> This will not work for pretty much any bus but PCI because only PCI
> allows to detach a driver leaving children devices on the bus. The
> rest of buses remove children devices when disconnecting parent.

Yeah, I was glad you stepped in.  Both of you are trying to work on the
same problem, in different ways.  It would be great if you both could
work out a common method together.

> Also, there usually much more going on with regard to locking and
> other bus-specific actions besides taking bus's rwsem when binding
> devices. Serio bus will definitely get upset if you try to disconnect
> even a leaf device in the manner presented above and I think USB
> will get upset as well.

No, we can disconnect a driver from a device just fine for USB with no
problems (as long as it's not the hub driver from a hub device, we need
to never be able to disconnect those.)

thanks,

greg k-h

Re: [PATCH 2.6.10-rc1 0/4] driver-model: manual device attach

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 936 bytes --]

hoi :)

On Thu, Nov 04, 2004 at 04:43:30PM +0900, Tejun Heo wrote:
>  Two files named attach and detach are created under each device's
> sysfs directory.  Reading attach node shows the name of applicable
> drivers.  Writing a driver name attaches the device to the driver.
> Also, per-device parameters can be specified when writing to an attach
> node.  Writing anything to the write-only detach node detaches the
> driver from the currently associated driver.

perhaps it'll be simpler with only the attach file and using a special
magic value ("", "none", "detach", whatever) to manually detach a device
from the driver.

Is it possible (and worthwhile) to reattach a manually detached device
to the default driver? Perhaps using a magic value "auto" for attach.
Something like your dev.autoattach=2 rescan method, but for one
device only. (What is the use case to rescan all busses, anyway?)

-- 
Martin Waitz

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 2.6.10-rc1 0/4] driver-model: manual device attach

[Greg KH]

On Thu, Nov 04, 2004 at 04:43:30PM +0900, Tejun Heo wrote:
>  Hello, again. :-)
> 
>  These are the manual device attach patches I was talking about in the
> previous posting.  These patches need devparam patches to be applied
> first.  It's composed of two parts.
> 
>  1. sysctl node dev.autoattach
> 
>  dev.autoattach is read/write integer sysctl node which controls
> driver-model's behavior regarding device - driver association.

Ick, no new sysctls please.  Make this a per-bus attribute that gets
written to in sysfs.  Much nicer and much finer control then.

>  0: autoattach disabled.  devices are not associated with drivers
>     automatically.  i.e. insmod'ing e100.ko won't cause it to attach to the
>     actual e100 devices.
>  1: autoattach enabled.  The default value.  This is the same as the
>     current driver model behavior.  Driver model automatically associates
>     devices to drivers.
>  2: rescan command.  If this value is written, bus_rescan_devices() is
>     invoked for all the registered bus types; thus attaching all
>     devices to available drivers.  After rescan is complete, the
>      autoattach value is set to 1.

Make this a different sysfs file.  "rescan" would be good.

Look at how pci can handle adding new devices to their drivers from
sysfs.  If we can move that kind of functionality to the driver core, so
that all busses get it (it will require a new per-bus callback though,
se the other patches recently posted to lkml for an example of this),
that would be what I would like to see happen.

>  2. per-device attach and detach sysfs node.
> 
>  Two files named attach and detach are created under each device's
> sysfs directory.  Reading attach node shows the name of applicable
> drivers.

How does a device know what drivers could be bound to it?  It's the
other way around, drivers know what kind of devices they can bind to.
Let's add the ability to add more devices to a driver through sysfs,
again, like PCI does.

> Writing a driver name attaches the device to the driver.

No, do it the other way, attach a driver to a device.

thanks,

greg k-h

Re: [PATCH 2.6.10-rc1 0/4] driver-model: manual device attach

[Tejun Heo]

 Hello, Greg,

On Thu, Nov 04, 2004 at 09:53:19AM -0800, Greg KH wrote:
> How does a device know what drivers could be bound to it?  It's the
> other way around, drivers know what kind of devices they can bind to.
> Let's add the ability to add more devices to a driver through sysfs,
> again, like PCI does.
> 
> > Writing a driver name attaches the device to the driver.
> 
> No, do it the other way, attach a driver to a device.

 Well, I just happen to think that devices attach to drivers as in
"attaching pci device 0000:00:08.0 to e1000 driver".  Maybe I'm just
weird. :-P

-- 
tejun

Re: [PATCH 2.6.10-rc1 0/4] driver-model: manual device attach

[Dmitry Torokhov]

On Thursday 04 November 2004 12:53 pm, Greg KH wrote:
> On Thu, Nov 04, 2004 at 04:43:30PM +0900, Tejun Heo wrote:
> >  Hello, again. :-)
> > 
> >  These are the manual device attach patches I was talking about in the
> > previous posting.  These patches need devparam patches to be applied
> > first.  It's composed of two parts.
> > 
> >  1. sysctl node dev.autoattach
> > 
> >  dev.autoattach is read/write integer sysctl node which controls
> > driver-model's behavior regarding device - driver association.
> 
> Ick, no new sysctls please.  Make this a per-bus attribute that gets
> written to in sysfs.  Much nicer and much finer control then.
>

I think that my bind)mode patches which allow to control binding through
sysfs on pre-device and per-driver base should suffice here. I really doubt
that anybody would want to keep autoattach disabled and do all matching
manually ;). Besides having per-driver attribute allows drivers authors
control binding.
 
> >  0: autoattach disabled.  devices are not associated with drivers
> >     automatically.  i.e. insmod'ing e100.ko won't cause it to attach to the
> >     actual e100 devices.
> >  1: autoattach enabled.  The default value.  This is the same as the
> >     current driver model behavior.  Driver model automatically associates
> >     devices to drivers.
> >  2: rescan command.  If this value is written, bus_rescan_devices() is
> >     invoked for all the registered bus types; thus attaching all
> >     devices to available drivers.  After rescan is complete, the
> >      autoattach value is set to 1.
> 
> Make this a different sysfs file.  "rescan" would be good.
> 
> Look at how pci can handle adding new devices to their drivers from
> sysfs.  If we can move that kind of functionality to the driver core, so
> that all busses get it (it will require a new per-bus callback though,
> se the other patches recently posted to lkml for an example of this),
> that would be what I would like to see happen.
> 
> >  2. per-device attach and detach sysfs node.
> > 
> >  Two files named attach and detach are created under each device's
> > sysfs directory.  Reading attach node shows the name of applicable
> > drivers.
>

Do we really need 2 or even 3 files ("attach", "detach" and "rescan")?
Given that you really can't (at least not yet) do all there operations
for all buses from the core that woudl require 3 per-bus callbacks.
I think reserving special values such as "none" or "detach" and "rescan"
shoudl work just fine and also willallow extending supported operations
on per-bus basis. For example serio bus supports "reconnect" option which
tries to re-initialize device if something happened to it. It does not
want to do rescan as that would generate new input devices while it is
much more convenient to re-use old ones. 
 
> How does a device know what drivers could be bound to it?  It's the
> other way around, drivers know what kind of devices they can bind to.

But when 2+ drivers can be bound to a device then particular _device_
gets to decide which driver is best suited for it, like in cases of
e100/eepro100 or psmouse/serio_raw.

> Let's add the ability to add more devices to a driver through sysfs,
> again, like PCI does.
>

Well, PCI does add a new ID to a driver allowing it to bind to a whole
new set of devices. I agree that this is a "driver" operation. 
 
> > Writing a driver name attaches the device to the driver.
> 
> No, do it the other way, attach a driver to a device.
>

I disagree. Here you working with particular device. You are not saying
"from now on I want e100 to bind all my 5 new network cards that happen
to have id XXXX:YYYY". Instead you are saying "I want to bind e100 driver
to this card residing at /sys/bus/pci/0000.....". In other word it is
operation on particular device and should be done by manipulating device
attribute.

-- 
Dmitry

Re: [PATCH 2.6.10-rc1 0/4] driver-model: manual device attach

[Tejun Heo]

On Fri, Nov 05, 2004 at 12:02:57AM -0500, Dmitry Torokhov wrote:
> I think that my bind)mode patches which allow to control binding through
> sysfs on pre-device and per-driver base should suffice here. I really doubt
> that anybody would want to keep autoattach disabled and do all matching
> manually ;). Besides having per-driver attribute allows drivers authors
> control binding.

 Think about extending hotplug to cover all device bindings.  It will
kick in very early in the booting process (maybe in the initrd image)
and names/binds every device on the device with appropriate arguments
as user requested.  I was thinking about usages like that when I was
making the sysctl node.  Maybe I was going too far. :-)

> Do we really need 2 or even 3 files ("attach", "detach" and "rescan")?
> Given that you really can't (at least not yet) do all there operations
> for all buses from the core that woudl require 3 per-bus callbacks.
> I think reserving special values such as "none" or "detach" and "rescan"
> shoudl work just fine and also willallow extending supported operations
> on per-bus basis. For example serio bus supports "reconnect" option which
> tries to re-initialize device if something happened to it. It does not
> want to do rescan as that would generate new input devices while it is
> much more convenient to re-use old ones. 

 How about making the command format "CMD ARGS" rather than
"{CMD|DRIVERNAME}"  i.e.

 not

 # echo e100 > drvctl
 # echo detach > drvctl

 but

 # echo attach e100 > drvctl
 # echo detach > drvctl

 But, I don't know.  It now just seems too much like a proc node.

> I disagree. Here you working with particular device. You are not saying
> "from now on I want e100 to bind all my 5 new network cards that happen
> to have id XXXX:YYYY". Instead you are saying "I want to bind e100 driver
> to this card residing at /sys/bus/pci/0000.....". In other word it is
> operation on particular device and should be done by manipulating device
> attribute.

 I kind of agree with you but I think either way is fine.

 Thanks.

-- 
tejun

Re: [PATCH 2.6.10-rc1 0/4] driver-model: manual device attach

[Dmitry Torokhov]

On Fri, 5 Nov 2004 15:32:37 +0900, Tejun Heo <tj@home-tj.org> wrote:
> On Fri, Nov 05, 2004 at 12:02:57AM -0500, Dmitry Torokhov wrote:
> 
> > Do we really need 2 or even 3 files ("attach", "detach" and "rescan")?
> > Given that you really can't (at least not yet) do all there operations
> > for all buses from the core that woudl require 3 per-bus callbacks.
> > I think reserving special values such as "none" or "detach" and "rescan"
> > shoudl work just fine and also willallow extending supported operations
> > on per-bus basis. For example serio bus supports "reconnect" option which
> > tries to re-initialize device if something happened to it. It does not
> > want to do rescan as that would generate new input devices while it is
> > much more convenient to re-use old ones.
> 
> How about making the command format "CMD ARGS" rather than
> "{CMD|DRIVERNAME}"  i.e.
> 
> not
> 
> # echo e100 > drvctl
> # echo detach > drvctl
> 
> but
> 
> # echo attach e100 > drvctl
> # echo detach > drvctl
> 
> But, I don't know.  It now just seems too much like a proc node.
>

Well, I was lazy and did not want to do any parsing at all, but I do
not have anything against "CMD ARG ARG ARG" form, especially
if integrate drvparm. 

-- 
Dmitry

Re: [PATCH 2.6.10-rc1 0/4] driver-model: manual device attach

[Dmitry Torokhov]

On Friday 05 November 2004 01:32 am, Tejun Heo wrote:
> > Do we really need 2 or even 3 files ("attach", "detach" and "rescan")?
> > Given that you really can't (at least not yet) do all there operations
> > for all buses from the core that woudl require 3 per-bus callbacks.
> > I think reserving special values such as "none" or "detach" and "rescan"
> > shoudl work just fine and also willallow extending supported operations
> > on per-bus basis. For example serio bus supports "reconnect" option which
> > tries to re-initialize device if something happened to it. It does not
> > want to do rescan as that would generate new input devices while it is
> > much more convenient to re-use old ones. 
> 
>  How about making the command format "CMD ARGS" rather than
> "{CMD|DRIVERNAME}"  i.e.
> 

Hi guys,

What do you think about following set of patches. It adds drvctl default
device attribute (write only) that controls device/driver binding. It
expects commands in form of "CMD [DRIVER_NAME] [ARG ARG...]" so I think
it will be very easy to adapt it to Tejun's per-device parameters.

I adjusted serio bus to the new form of commands so now it accepts:
"detach", "attach <driver>", "rescan", "reconnect"

And I added PCI bus drvctl that understands "detach", "attach <driver>",
and "rescan".

As we add drvctl methods to the rest of the buses we can review what
can be pulled into the core and what has to stay bus-specific.

Plus there is the bind_mode patch that allows switching between automatic
and manual binding on per-device/per-driver base.

-- 
Dmitry

[PATCH 1/3] Add drvctl default device attribute

[Dmitry Torokhov]

===================================================================


ChangeSet@1.1961, 2004-11-08 02:03:59-05:00, dtor_core@ameritech.net
  Driver core: add "drvctl" default device attribute that allows
               userspace request execution of bus-specific actions
               for devices. Used to manually control driver and
               device binding/unbinding/rebinding, causes execution
               of bus->drvctl() helper. Expects commands in form of
               "CMD [DRIVER NAME] [ARG ARG...]]".
  
               Serio bus's drvctl rearranged to accept the following
               commands: "detach", "attach <driver>", "rescan", and
               "reconnect".
  
  Signed-off-by: Dmitry Torokhov <dtor@mail.ru>


 drivers/base/interface.c    |   74 +++++++++++++++++++++++++++++++++++++++++++-
 drivers/input/serio/serio.c |   61 +++++++++++++++++-------------------
 include/linux/device.h      |    4 +-
 3 files changed, 106 insertions(+), 33 deletions(-)


===================================================================



diff -Nru a/drivers/base/interface.c b/drivers/base/interface.c
--- a/drivers/base/interface.c	2004-11-08 02:10:34 -05:00
+++ b/drivers/base/interface.c	2004-11-08 02:10:34 -05:00
@@ -13,6 +13,7 @@
 #include <linux/err.h>
 #include <linux/stat.h>
 #include <linux/string.h>
+#include <linux/ctype.h>
 
 /**
  *	detach_state - control the default power state for the device.
@@ -42,10 +43,81 @@
 	return n;
 }
 
-static DEVICE_ATTR(detach_state, 0644, detach_show, detach_store);
+/**
+ *	drvctl - controls device and driver binding.
+ *
+ *	Writing to the attribute causes execution of bus-specific drvctl()
+ *	helper that is used to disconnect device from its driver or rebind
+ *	device to a specific driver.
+ *	Commands are expected in the following format:
+ *	CMD [DRIVER_NAME] [ARG ARG ARG]
+ *	drvctl handler is free to mangle the args string.
+ */
+
+#define GET_WORD(x)					\
+do {							\
+	while (isspace(*args)) args++;			\
+	(x) = args;					\
+	while (*args && !isspace(*args)) args++;	\
+	if (isspace(*args)) {				\
+		save_chr = *args;			\
+		*args++ = '\0';				\
+	}						\
+} while (0)						\
 
+static ssize_t drvctl_store(struct device * dev, const char * buf, size_t count)
+{
+	int retval = -ENOSYS;
+	struct device_driver *drv = NULL;
+	char *str, *action, *drv_name, *args;
+	char save_chr = 0;
+
+	if (!dev->bus->drvctl)
+		return -ENOSYS;
+
+	/* copy buffer so we can mangle it */
+	if (!(args = str = kmalloc(count + 1, GFP_KERNEL)))
+		return -ENOMEM;
+
+	memcpy(str, buf, count);
+	str[count] = '\0';
+
+	GET_WORD(action);
+	GET_WORD(drv_name);
+
+	if (*drv_name) {
+		if (!(drv = driver_find(drv_name, dev->bus))) {
+			/*
+			 * if this is not a name of existing driver
+			 * merge it with thye rest of the string and
+			 * pass to drvctl as 'args'
+			 */
+			if (*args)
+				*--args = save_chr;
+			args = drv_name;
+		}
+	}
+
+	while (*args == ' ') args++;
+
+	pr_debug("drvctl -  action: '%s', driver: '%s', args: '%s'\n",
+		 action, drv ? drv->name : "", args);
+
+	retval = dev->bus->drvctl(dev, action, drv, args);
+
+	if (drv)
+		put_driver(drv);
+	kfree(str);
+
+	return (retval < 0) ? retval : count;
+}
+#undef GET_WORD
+
+static DEVICE_ATTR(detach_state, 0644, detach_show, detach_store);
+static DEVICE_ATTR(drvctl, 0200, NULL, drvctl_store);
 
 struct attribute * dev_default_attrs[] = {
 	&dev_attr_detach_state.attr,
+	&dev_attr_drvctl.attr,
 	NULL,
 };
diff -Nru a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
--- a/drivers/input/serio/serio.c	2004-11-08 02:10:34 -05:00
+++ b/drivers/input/serio/serio.c	2004-11-08 02:10:34 -05:00
@@ -246,36 +246,6 @@
 	return sprintf(buf, "%s\n", serio->name);
 }
 
-static ssize_t serio_rebind_driver(struct device *dev, const char *buf, size_t count)
-{
-	struct serio *serio = to_serio_port(dev);
-	struct device_driver *drv;
-	int retval;
-
-	retval = down_interruptible(&serio_sem);
-	if (retval)
-		return retval;
-
-	retval = count;
-	if (!strncmp(buf, "none", count)) {
-		serio_disconnect_port(serio);
-	} else if (!strncmp(buf, "reconnect", count)) {
-		serio_reconnect_port(serio);
-	} else if (!strncmp(buf, "rescan", count)) {
-		serio_disconnect_port(serio);
-		serio_connect_port(serio, NULL);
-	} else if ((drv = driver_find(buf, &serio_bus)) != NULL) {
-		serio_disconnect_port(serio);
-		serio_connect_port(serio, to_serio_driver(drv));
-		put_driver(drv);
-	} else {
-		retval = -EINVAL;
-	}
-
-	up(&serio_sem);
-
-	return retval;
-}
 
 static ssize_t serio_show_bind_mode(struct device *dev, char *buf)
 {
@@ -302,7 +272,6 @@
 
 static struct device_attribute serio_device_attrs[] = {
 	__ATTR(description, S_IRUGO, serio_show_description, NULL),
-	__ATTR(drvctl, S_IWUSR, NULL, serio_rebind_driver),
 	__ATTR(bind_mode, S_IWUSR | S_IRUGO, serio_show_bind_mode, serio_set_bind_mode),
 	__ATTR_NULL
 };
@@ -597,6 +566,35 @@
 	up(&serio_sem);
 }
 
+static int serio_rebind_driver(struct device *dev, const char *action,
+			       struct device_driver *drv, char *args)
+{
+	struct serio *serio = to_serio_port(dev);
+	int retval;
+
+	retval = down_interruptible(&serio_sem);
+	if (retval)
+		return retval;
+
+	if (!strcmp(action, "detach")) {
+		serio_disconnect_port(serio);
+	} else if (!strcmp(action, "reconnect")) {
+		serio_reconnect_port(serio);
+	} else if (!strcmp(action, "rescan")) {
+		serio_disconnect_port(serio);
+		serio_connect_port(serio, NULL);
+	} else if (!strcmp(action, "attach") && drv) {
+		serio_disconnect_port(serio);
+		serio_connect_port(serio, to_serio_driver(drv));
+	} else {
+		retval = -EINVAL;
+	}
+
+	up(&serio_sem);
+
+	return retval;
+}
+
 static void serio_set_drv(struct serio *serio, struct serio_driver *drv)
 {
 	down(&serio->drv_sem);
@@ -661,6 +659,7 @@
 
 	serio_bus.dev_attrs = serio_device_attrs;
 	serio_bus.drv_attrs = serio_driver_attrs;
+	serio_bus.drvctl = serio_rebind_driver;
 	bus_register(&serio_bus);
 
 	return 0;
diff -Nru a/include/linux/device.h b/include/linux/device.h
--- a/include/linux/device.h	2004-11-08 02:10:34 -05:00
+++ b/include/linux/device.h	2004-11-08 02:10:34 -05:00
@@ -59,7 +59,9 @@
 	struct driver_attribute	* drv_attrs;
 
 	int		(*match)(struct device * dev, struct device_driver * drv);
-	int		(*hotplug) (struct device *dev, char **envp, 
+	int		(*drvctl)(struct device * dev, const char * action,
+				  struct device_driver * drv, char * args);
+	int		(*hotplug) (struct device *dev, char **envp,
 				    int num_envp, char *buffer, int buffer_size);
 	int		(*suspend)(struct device * dev, u32 state);
 	int		(*resume)(struct device * dev);

[PATCH 2/3] Add drvctl handler to PCI bus

[Dmitry Torokhov]

===================================================================


ChangeSet@1.1962, 2004-11-08 02:06:19-05:00, dtor_core@ameritech.net
  PCI: Add devctl method to PCI bus. The following commands are
       available: "detach", "attach <driver>", and "rescan".
  
  Signed-off-by: Dmitry Torokhov <dtor@mail.ru>


 pci-driver.c |   58 ++++++++++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 44 insertions(+), 14 deletions(-)


===================================================================



diff -Nru a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
--- a/drivers/pci/pci-driver.c	2004-11-08 02:20:22 -05:00
+++ b/drivers/pci/pci-driver.c	2004-11-08 02:20:22 -05:00
@@ -186,7 +186,7 @@
  *                    PCI device id structure
  * @ids: array of PCI device id structures to search in
  * @dev: the PCI device structure to match against
- * 
+ *
  * Used by a driver to check whether a PCI device present in the
  * system is in its list of supported devices.Returns the matching
  * pci_device_id structure or %NULL if there is no match.
@@ -204,12 +204,12 @@
 
 /**
  * pci_device_probe_static()
- * 
+ *
  * returns 0 and sets pci_dev->driver when drv claims pci_dev, else error.
  */
 static int
 pci_device_probe_static(struct pci_driver *drv, struct pci_dev *pci_dev)
-{		   
+{
 	int error = -ENODEV;
 	const struct pci_device_id *id;
 
@@ -227,13 +227,13 @@
 
 /**
  * __pci_device_probe()
- * 
+ *
  * returns 0  on success, else error.
  * side-effect: pci_dev->driver is set to drv when drv claims pci_dev.
  */
 static int
 __pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev)
-{		   
+{
 	int error = 0;
 
 	if (!pci_dev->driver && drv->probe) {
@@ -314,7 +314,7 @@
 }
 
 
-/* 
+/*
  * Default resume method for devices that have no driver provided resume,
  * or not even a driver at all.
  */
@@ -394,10 +394,10 @@
 /**
  * pci_register_driver - register a new pci driver
  * @drv: the driver structure to register
- * 
+ *
  * Adds the driver structure to the list of registered drivers.
- * Returns a negative value on error, otherwise 0. 
- * If no error occured, the driver remains registered even if 
+ * Returns a negative value on error, otherwise 0.
+ * If no error occured, the driver remains registered even if
  * no device was claimed during registration.
  */
 int pci_register_driver(struct pci_driver *drv)
@@ -425,7 +425,7 @@
 /**
  * pci_unregister_driver - unregister a pci driver
  * @drv: the driver structure to unregister
- * 
+ *
  * Deletes the driver structure from the list of registered PCI drivers,
  * gives it a chance to clean up by calling its remove() function for
  * each device it was responsible for, and marks those devices as
@@ -447,7 +447,7 @@
  * pci_dev_driver - get the pci_driver of a device
  * @dev: the device to query
  *
- * Returns the appropriate pci_driver structure or %NULL if there is no 
+ * Returns the appropriate pci_driver structure or %NULL if there is no
  * registered driver for the device.
  */
 struct pci_driver *
@@ -457,7 +457,7 @@
 		return dev->driver;
 	else {
 		int i;
-		for(i=0; i<=PCI_ROM_RESOURCE; i++)
+		for(i = 0; i <= PCI_ROM_RESOURCE; i++)
 			if (dev->resource[i].flags & IORESOURCE_BUSY)
 				return &pci_compat_driver;
 	}
@@ -468,12 +468,12 @@
  * pci_bus_match - Tell if a PCI device structure has a matching PCI device id structure
  * @ids: array of PCI device id structures to search in
  * @dev: the PCI device structure to match against
- * 
+ *
  * Used by a driver to check whether a PCI device present in the
  * system is in its list of supported devices.Returns the matching
  * pci_device_id structure or %NULL if there is no match.
  */
-static int pci_bus_match(struct device * dev, struct device_driver * drv) 
+static int pci_bus_match(struct device * dev, struct device_driver * drv)
 {
 	const struct pci_dev * pci_dev = to_pci_dev(dev);
 	struct pci_driver * pci_drv = to_pci_driver(drv);
@@ -490,6 +490,35 @@
 	return pci_bus_match_dynids(pci_dev, pci_drv);
 }
 
+/*
+ * This is PCI bus's drvctl method that handles manual device binding.
+ */
+static int pci_rebind_driver(struct device *dev, const char *action,
+			     struct device_driver *drv, char *args)
+{
+	int retval = 0;
+
+	if (!strcmp(action, "detach")) {
+		down_write(&dev->bus->subsys.rwsem);
+		device_release_driver(dev);
+		up_write(&dev->bus->subsys.rwsem);
+	} else if (!strcmp(action, "rescan")) {
+		down_write(&dev->bus->subsys.rwsem);
+		device_release_driver(dev);
+		device_attach(dev);
+		up_write(&dev->bus->subsys.rwsem);
+	} else if (!strcmp(action, "attach") && drv) {
+		down_write(&dev->bus->subsys.rwsem);
+		device_release_driver(dev);
+		driver_probe_device(drv, dev);
+		up_write(&dev->bus->subsys.rwsem);
+	} else {
+		retval = -EINVAL;
+	}
+
+	return retval;
+}
+
 /**
  * pci_dev_get - increments the reference count of the pci device structure
  * @dev: the device being referenced
@@ -534,6 +563,7 @@
 	.name		= "pci",
 	.match		= pci_bus_match,
 	.hotplug	= pci_hotplug,
+	.drvctl		= pci_rebind_driver,
 	.suspend	= pci_device_suspend,
 	.resume		= pci_device_resume,
 	.dev_attrs	= pci_dev_attrs,

[PATCH 3/3] Add bind_mode default device/driver attributes

[Dmitry Torokhov]

===================================================================


ChangeSet@1.1963, 2004-11-08 02:07:15-05:00, dtor_core@ameritech.net
  Driver core: add "bind_mode" default device and driver attribute.
               Calls to device_attach() and driver_attach() will not
               bind device and driver if either one of them is in
               "manual" bind mode. Manual binding is expected to be
               handled by bus's drvctl()
  
           echo -n "manual" > /sus/bus/serio/devices/serio2/bind_mode
           echo -n "auto" > /sys/bus/serio/drivers/serio_raw/bind_mode
  
  Signed-off-by: Dmitry Torokhov <dtor@mail.ru>


 drivers/base/bus.c              |   20 ++++++----
 drivers/base/interface.c        |   79 ++++++++++++++++++++++++++++++++++++----
 drivers/input/serio/serio.c     |   59 ++---------------------------
 drivers/input/serio/serio_raw.c |    4 +-
 include/linux/device.h          |    5 ++
 include/linux/serio.h           |    4 --
 6 files changed, 97 insertions(+), 74 deletions(-)


===================================================================



diff -Nru a/drivers/base/bus.c b/drivers/base/bus.c
--- a/drivers/base/bus.c	2004-11-08 02:20:53 -05:00
+++ b/drivers/base/bus.c	2004-11-08 02:20:53 -05:00
@@ -68,9 +68,12 @@
 	up(&drv->unload_sem);
 }
 
+extern struct attribute * drv_default_attrs[];
+
 static struct kobj_type ktype_driver = {
 	.sysfs_ops	= &driver_sysfs_ops,
 	.release	= driver_release,
+	.default_attrs	= drv_default_attrs,
 };
 
 
@@ -319,9 +322,12 @@
 		return 1;
 	}
 
-	if (bus->match) {
-		list_for_each(entry, &bus->drivers.list) {
-			struct device_driver * drv = to_drv(entry);
+	if (dev->manual_bind || !bus->match)
+		return 0;
+
+	list_for_each(entry, &bus->drivers.list) {
+		struct device_driver * drv = to_drv(entry);
+		if (!drv->manual_bind) {
 			error = driver_probe_device(drv, dev);
 			if (!error)
 				/* success, driver matched */
@@ -329,8 +335,8 @@
 			if (error != -ENODEV)
 				/* driver matched but the probe failed */
 				printk(KERN_WARNING
-				    "%s: probe of %s failed with error %d\n",
-				    drv->name, dev->bus_id, error);
+					"%s: probe of %s failed with error %d\n",
+					drv->name, dev->bus_id, error);
 		}
 	}
 
@@ -356,12 +362,12 @@
 	struct list_head * entry;
 	int error;
 
-	if (!bus->match)
+	if (drv->manual_bind || !bus->match)
 		return;
 
 	list_for_each(entry, &bus->devices.list) {
 		struct device * dev = container_of(entry, struct device, bus_list);
-		if (!dev->driver) {
+		if (!dev->driver && !dev->manual_bind) {
 			error = driver_probe_device(drv, dev);
 			if (error && (error != -ENODEV))
 				/* driver matched but the probe failed */
diff -Nru a/drivers/base/interface.c b/drivers/base/interface.c
--- a/drivers/base/interface.c	2004-11-08 02:20:53 -05:00
+++ b/drivers/base/interface.c	2004-11-08 02:20:53 -05:00
@@ -1,6 +1,6 @@
 /*
  * drivers/base/interface.c - common driverfs interface that's exported to
- * 	the world for all devices.
+ * 	the world for all devices and drivers.
  *
  * Copyright (c) 2002-3 Patrick Mochel
  * Copyright (c) 2002-3 Open Source Development Labs
@@ -16,6 +16,35 @@
 #include <linux/ctype.h>
 
 /**
+ *	bind_mode - control the binding mode for the device.
+ *
+ *	When set to "auto" driver core will try to automatically bind the
+ *	device once appropriate driver becomes available. When bind mode
+ *	is "manual" intervention from userspace is required.
+ */
+
+static ssize_t dev_bind_mode_show(struct device * dev, char * buf)
+{
+	return sprintf(buf, "%s\n", dev->manual_bind ? "manual" : "auto");
+}
+
+static ssize_t dev_bind_mode_store(struct device * dev, const char * buf, size_t count)
+{
+	int retval = count;
+
+	if (!strncmp(buf, "manual", count)) {
+		dev->manual_bind = 1;
+	} else if (!strncmp(buf, "auto", count)) {
+		dev->manual_bind = 0;
+	} else {
+		retval = -EINVAL;
+	}
+
+	return retval;
+}
+
+
+/**
  *	detach_state - control the default power state for the device.
  *
  *	This is the state the device enters when it's driver module is
@@ -28,12 +57,12 @@
  *	driver's suspend method.
  */
 
-static ssize_t detach_show(struct device * dev, char * buf)
+static ssize_t dev_detach_show(struct device * dev, char * buf)
 {
 	return sprintf(buf, "%u\n", dev->detach_state);
 }
 
-static ssize_t detach_store(struct device * dev, const char * buf, size_t n)
+static ssize_t dev_detach_store(struct device * dev, const char * buf, size_t n)
 {
 	u32 state;
 	state = simple_strtoul(buf, NULL, 10);
@@ -65,7 +94,7 @@
 	}						\
 } while (0)						\
 
-static ssize_t drvctl_store(struct device * dev, const char * buf, size_t count)
+static ssize_t dev_drvctl_store(struct device * dev, const char * buf, size_t count)
 {
 	int retval = -ENOSYS;
 	struct device_driver *drv = NULL;
@@ -113,11 +142,49 @@
 }
 #undef GET_WORD
 
-static DEVICE_ATTR(detach_state, 0644, detach_show, detach_store);
-static DEVICE_ATTR(drvctl, 0200, NULL, drvctl_store);
+static DEVICE_ATTR(bind_mode, 0644, dev_bind_mode_show, dev_bind_mode_store);
+static DEVICE_ATTR(detach_state, 0644, dev_detach_show, dev_detach_store);
+static DEVICE_ATTR(drvctl, 0200, NULL, dev_drvctl_store);
 
 struct attribute * dev_default_attrs[] = {
+	&dev_attr_bind_mode.attr,
 	&dev_attr_detach_state.attr,
 	&dev_attr_drvctl.attr,
 	NULL,
 };
+
+/**
+ *	bind_mode - control the binding mode for the driver.
+ *
+ *	When set to "auto" driver core will try to automatically bind the
+ *	driver once appropriate device becomes available. When bind mode
+ *	is "manual" intervention from userspace is required.
+ */
+
+static ssize_t drv_bind_mode_show(struct device_driver * drv, char * buf)
+{
+	return sprintf(buf, "%s\n", drv->manual_bind ? "manual" : "auto");
+}
+
+static ssize_t drv_bind_mode_store(struct device_driver * drv, const char * buf, size_t count)
+{
+	int retval = count;
+
+	if (!strncmp(buf, "manual", count)) {
+		drv->manual_bind = 1;
+	} else if (!strncmp(buf, "auto", count)) {
+		drv->manual_bind = 0;
+	} else {
+		retval = -EINVAL;
+	}
+
+	return retval;
+}
+
+static DRIVER_ATTR(bind_mode, 0644, drv_bind_mode_show, drv_bind_mode_store);
+
+struct attribute * drv_default_attrs[] = {
+	&driver_attr_bind_mode.attr,
+	NULL,
+};
+
diff -Nru a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
--- a/drivers/input/serio/serio.c	2004-11-08 02:20:53 -05:00
+++ b/drivers/input/serio/serio.c	2004-11-08 02:20:53 -05:00
@@ -92,7 +92,7 @@
 	struct serio_driver *drv;
 
 	list_for_each_entry(drv, &serio_driver_list, node)
-		if (!drv->manual_bind)
+		if (!drv->driver.manual_bind)
 			if (serio_bind_driver(serio, drv))
 				break;
 }
@@ -246,33 +246,8 @@
 	return sprintf(buf, "%s\n", serio->name);
 }
 
-
-static ssize_t serio_show_bind_mode(struct device *dev, char *buf)
-{
-	struct serio *serio = to_serio_port(dev);
-	return sprintf(buf, "%s\n", serio->manual_bind ? "manual" : "auto");
-}
-
-static ssize_t serio_set_bind_mode(struct device *dev, const char *buf, size_t count)
-{
-	struct serio *serio = to_serio_port(dev);
-	int retval;
-
-	retval = count;
-	if (!strncmp(buf, "manual", count)) {
-		serio->manual_bind = 1;
-	} else if (!strncmp(buf, "auto", count)) {
-		serio->manual_bind = 0;
-	} else {
-		retval = -EINVAL;
-	}
-
-	return retval;
-}
-
 static struct device_attribute serio_device_attrs[] = {
 	__ATTR(description, S_IRUGO, serio_show_description, NULL),
-	__ATTR(bind_mode, S_IWUSR | S_IRUGO, serio_show_bind_mode, serio_set_bind_mode),
 	__ATTR_NULL
 };
 
@@ -347,7 +322,7 @@
 
 	if (drv)
 		serio_bind_driver(serio, drv);
-	else if (!serio->manual_bind)
+	else if (!serio->dev.manual_bind)
 		serio_find_driver(serio);
 
 	/* Ok, now bind children, if any */
@@ -359,7 +334,7 @@
 
 		serio_create_port(serio);
 
-		if (!serio->manual_bind) {
+		if (!serio->dev.manual_bind) {
 			/*
 			 * With children we just _prefer_ passed in driver,
 			 * but we will try other options in case preferred
@@ -481,34 +456,8 @@
 	return sprintf(buf, "%s\n", driver->description ? driver->description : "(none)");
 }
 
-static ssize_t serio_driver_show_bind_mode(struct device_driver *drv, char *buf)
-{
-	struct serio_driver *serio_drv = to_serio_driver(drv);
-	return sprintf(buf, "%s\n", serio_drv->manual_bind ? "manual" : "auto");
-}
-
-static ssize_t serio_driver_set_bind_mode(struct device_driver *drv, const char *buf, size_t count)
-{
-	struct serio_driver *serio_drv = to_serio_driver(drv);
-	int retval;
-
-	retval = count;
-	if (!strncmp(buf, "manual", count)) {
-		serio_drv->manual_bind = 1;
-	} else if (!strncmp(buf, "auto", count)) {
-		serio_drv->manual_bind = 0;
-	} else {
-		retval = -EINVAL;
-	}
-
-	return retval;
-}
-
-
 static struct driver_attribute serio_driver_attrs[] = {
 	__ATTR(description, S_IRUGO, serio_driver_show_description, NULL),
-	__ATTR(bind_mode, S_IWUSR | S_IRUGO,
-		serio_driver_show_bind_mode, serio_driver_set_bind_mode),
 	__ATTR_NULL
 };
 
@@ -523,7 +472,7 @@
 	drv->driver.bus = &serio_bus;
 	driver_register(&drv->driver);
 
-	if (drv->manual_bind)
+	if (drv->driver.manual_bind)
 		goto out;
 
 start_over:
diff -Nru a/drivers/input/serio/serio_raw.c b/drivers/input/serio/serio_raw.c
--- a/drivers/input/serio/serio_raw.c	2004-11-08 02:20:53 -05:00
+++ b/drivers/input/serio/serio_raw.c	2004-11-08 02:20:53 -05:00
@@ -365,14 +365,14 @@
 
 static struct serio_driver serio_raw_drv = {
 	.driver		= {
-		.name	= "serio_raw",
+		.name		= "serio_raw",
+		.manual_bind	= 1,
 	},
 	.description	= DRIVER_DESC,
 	.interrupt	= serio_raw_interrupt,
 	.connect	= serio_raw_connect,
 	.reconnect	= serio_raw_reconnect,
 	.disconnect	= serio_raw_disconnect,
-	.manual_bind	= 1,
 };
 
 int __init serio_raw_init(void)
diff -Nru a/include/linux/device.h b/include/linux/device.h
--- a/include/linux/device.h	2004-11-08 02:20:53 -05:00
+++ b/include/linux/device.h	2004-11-08 02:20:53 -05:00
@@ -104,6 +104,8 @@
 	char			* name;
 	struct bus_type		* bus;
 
+	unsigned int		manual_bind;
+
 	struct semaphore	unload_sem;
 	struct kobject		kobj;
 	struct list_head	devices;
@@ -266,6 +268,9 @@
 	struct bus_type	* bus;		/* type of bus device is on */
 	struct device_driver *driver;	/* which driver has allocated this
 					   device */
+	unsigned int manual_bind;	/* indicates whether the core will
+					   try to find a driver for the
+					   device automatically */
 	void		*driver_data;	/* data private to the driver */
 	void		*platform_data;	/* Platform specific data (e.g. ACPI,
 					   BIOS data relevant to device) */
diff -Nru a/include/linux/serio.h b/include/linux/serio.h
--- a/include/linux/serio.h	2004-11-08 02:20:53 -05:00
+++ b/include/linux/serio.h	2004-11-08 02:20:53 -05:00
@@ -27,8 +27,6 @@
 	char name[32];
 	char phys[32];
 
-	unsigned int manual_bind;
-
 	unsigned short idbus;
 	unsigned short idvendor;
 	unsigned short idproduct;
@@ -59,8 +57,6 @@
 struct serio_driver {
 	void *private;
 	char *description;
-
-	unsigned int manual_bind;
 
 	void (*write_wakeup)(struct serio *);
 	irqreturn_t (*interrupt)(struct serio *, unsigned char,

Re: [PATCH 2.6.10-rc1 4/4] driver-model: attach/detach sysfs node implemented

[Dmitry Torokhov]

> Greg KH wrote:
> On Thu, Nov 04, 2004 at 12:05:31PM -0500, Dmitry Torokhov wrote:
> > Also, there usually much more going on with regard to locking and
> > other bus-specific actions besides taking bus's rwsem when binding
> > devices. Serio bus will definitely get upset if you try to disconnect
> > even a leaf device in the manner presented above and I think USB
> > will get upset as well.
> 
> No, we can disconnect a driver from a device just fine for USB with no

What about connecting? I am pretty ignorant of USB inner workings
but when I took a glance there seems to be a lot of preparations
before device is ready to be probed...

> problems (as long as it's not the hub driver from a hub device, we need
> to never be able to disconnect those.)

Never say never ;) That was the first thing I did after playing with
PCI devices when I tried doing what Tejun did.

If kernel advertises an userspace interface it will be used. I can see
myself wanting to disconnect my hub and all its devices so my wireless
explorer does not use batteries and I do not want to figure out what
port it is connected to... Someone else will find another reason,
I don't know.

I also think that even PCI should kill children devices behind a bridge
if bridge driver is disconnected to manage resources in more strict way.
But I think that would require notion of generic/specialized driver and
require automatic rebinding of specialized driver over generic one so
every PCI device has a driver attached to it.

-- 
Dmitry

Re: [PATCH 2.6.10-rc1 4/4] driver-model: attach/detach sysfs node implemented

[Tejun Heo]

 Hello Dmitry.  Hello Greg.

On Thu, Nov 04, 2004 at 11:06:19AM -0800, Dmitry Torokhov wrote:
> What about connecting? I am pretty ignorant of USB inner workings
> but when I took a glance there seems to be a lot of preparations
> before device is ready to be probed...

 IMHO, it would be better to coerce whatever bus to follow common
driver-model synchronization/attach/detach rules and be able to do
straight-forward implementation of features in the core driver-model.
If the current driver-model isn't enough, the core code should be
expanded rather than doing bus-specific dances in individual buses.
But I don't really know about any bus other than PCI, so maybe I'm
being too naive.

> > problems (as long as it's not the hub driver from a hub device, we need
> > to never be able to disconnect those.)
> 
> Never say never ;) That was the first thing I did after playing with
> PCI devices when I tried doing what Tejun did.
> 
> If kernel advertises an userspace interface it will be used. I can see
> myself wanting to disconnect my hub and all its devices so my wireless
> explorer does not use batteries and I do not want to figure out what
> port it is connected to... Someone else will find another reason,
> I don't know.
> 
> I also think that even PCI should kill children devices behind a bridge
> if bridge driver is disconnected to manage resources in more strict way.
> But I think that would require notion of generic/specialized driver and
> require automatic rebinding of specialized driver over generic one so
> every PCI device has a driver attached to it.

 I think above can be cleanly solved by enforcing that no device can
be attached to a driver unless all its ancestors are attached to a
driver.  The check can be made easiliy inside the driver-model, and,
if needed, making dummy drivers for internal node devices which
orignally didn't need one shouldn't be difficult.  We can just return
-EBUSY for any attempts to detach an internal device which has
driver-attached children.  This way, recursing and all other chores
can be dumped to user-space where they belong.

 And regarding the duplicate works, my work on manual-attach was
primarily to show how dynamic device-driver binding can work with
devparam; also, Dmitry seems to understand the problem better than me.
So, I think I should back off on manualattach.  Dmitry, what do you
think about integrating devparam with your work?

 Thanks.

-- 
tejun

Re: [PATCH 2.6.10-rc1 4/4] driver-model: attach/detach sysfs node implemented

[Dmitry Torokhov]

On Thursday 04 November 2004 11:45 pm, Tejun Heo wrote:
>  Hello Dmitry.  Hello Greg.
> 
> On Thu, Nov 04, 2004 at 11:06:19AM -0800, Dmitry Torokhov wrote:
> > What about connecting? I am pretty ignorant of USB inner workings
> > but when I took a glance there seems to be a lot of preparations
> > before device is ready to be probed...
> 
>  IMHO, it would be better to coerce whatever bus to follow common
> driver-model synchronization/attach/detach rules and be able to do
> straight-forward implementation of features in the core driver-model.
> If the current driver-model isn't enough, the core code should be
> expanded rather than doing bus-specific dances in individual buses.
> But I don't really know about any bus other than PCI, so maybe I'm
> being too naive.
>

The main problem I guess is that driver model does not allow registering
or removal of child devices during driver->probe() on the same bus. So
every bus does it its own way. Plus most buses were around way before
driver model so their historical locking rules affect this as well.
Down the road, as the stuff gets cleaned we can have a shot of pulling
bus code into drivre core, but not yet I think.
 
> > > problems (as long as it's not the hub driver from a hub device, we need
> > > to never be able to disconnect those.)
> > 
> > Never say never ;) That was the first thing I did after playing with
> > PCI devices when I tried doing what Tejun did.
> > 
> > If kernel advertises an userspace interface it will be used. I can see
> > myself wanting to disconnect my hub and all its devices so my wireless
> > explorer does not use batteries and I do not want to figure out what
> > port it is connected to... Someone else will find another reason,
> > I don't know.
> > 
> > I also think that even PCI should kill children devices behind a bridge
> > if bridge driver is disconnected to manage resources in more strict way.
> > But I think that would require notion of generic/specialized driver and
> > require automatic rebinding of specialized driver over generic one so
> > every PCI device has a driver attached to it.
> 
>  I think above can be cleanly solved by enforcing that no device can
> be attached to a driver unless all its ancestors are attached to a
> driver.  The check can be made easiliy inside the driver-model, and,
> if needed, making dummy drivers for internal node devices which
> orignally didn't need one shouldn't be difficult.

Right now not all parent PCI devices have drivers. IIRC there were talks
about implementing generic bridge driver but then some bridges need special
handling and the concern was that generic driver would prevent binding of
a special driver.

> We can just return 
> -EBUSY for any attempts to detach an internal device which has
> driver-attached children.  This way, recursing and all other chores
> can be dumped to user-space where they belong.
> 

The system needs to handle situation when you physically yanking part of a
device tree and get rid of all devices, like in cases when you pull an USB
cable connected to a HUB with a mouse and a scanner and a pinter and whatever
else out there. We need to get rid of all these devices and I am not sure
that offloading this task to userspace is good idea. Especially if removal
happens when the box is suspended and at wakeup we have completely different
set of devices...

>  And regarding the duplicate works, my work on manual-attach was
> primarily to show how dynamic device-driver binding can work with
> devparam; also, Dmitry seems to understand the problem better than me.
> So, I think I should back off on manualattach.  Dmitry, what do you
> think about integrating devparam with your work?
>

I do not see why we shoudln't but first I need to persuade Greg to apply
my patches ;)
 
-- 
Dmitry