2004-11-04 07:44:10

by Tejun Heo

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

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


2004-11-04 07:45:18

by Tejun Heo

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

ma_01_sysctl_dev_autoattach.patch

This patch implements sysctl node dev.autoattach.


Signed-off-by: Tejun Heo <[email protected]>


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 }
};

2004-11-04 07:46:53

by Tejun Heo

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

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 <[email protected]>


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[] = {

2004-11-04 07:48:48

by Tejun Heo

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

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 <[email protected]>


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*/

2004-11-04 07:50:50

by Tejun Heo

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

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 <[email protected]>


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,
};

2004-11-04 10:28:59

by Martin Waitz

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

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


Attachments:
(No filename) (915.00 B)
(No filename) (189.00 B)
Download all attachments

2004-11-04 17:19:40

by Dmitry Torokhov

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

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

2004-11-04 17:57:27

by Greg KH

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

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

2004-11-04 17:57:27

by Greg KH

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

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

2004-11-05 04:50:25

by Tejun Heo

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

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

2004-11-05 05:03:13

by Dmitry Torokhov

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

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

2004-11-05 06:32:44

by Tejun Heo

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

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

2004-11-05 14:54:35

by Dmitry Torokhov

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

On Fri, 5 Nov 2004 15:32:37 +0900, Tejun Heo <[email protected]> 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

2004-11-08 07:26:59

by Dmitry Torokhov

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

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

2004-11-08 07:27:32

by Dmitry Torokhov

[permalink] [raw]
Subject: [PATCH 1/3] Add drvctl default device attribute


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


[email protected], 2004-11-08 02:03:59-05:00, [email protected]
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 <[email protected]>


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);

2004-11-08 07:28:53

by Dmitry Torokhov

[permalink] [raw]
Subject: [PATCH 2/3] Add drvctl handler to PCI bus


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


[email protected], 2004-11-08 02:06:19-05:00, [email protected]
PCI: Add devctl method to PCI bus. The following commands are
available: "detach", "attach <driver>", and "rescan".

Signed-off-by: Dmitry Torokhov <[email protected]>


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,

2004-11-08 07:30:27

by Dmitry Torokhov

[permalink] [raw]
Subject: [PATCH 3/3] Add bind_mode default device/driver attributes


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


[email protected], 2004-11-08 02:07:15-05:00, [email protected]
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 <[email protected]>


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,