driver core: subclasses
RFC PATCH 1/1 driver core: subclasses
Greg,
The below patch is a first pass at implementing subclasses, for review and comment.
As a test, I modified drivers/input/input.c to allocate a new class, and register it as a subclass.
Before:
/sys/class/input//sys/class/input_device/
After:/sys/class/input/input_device/
In this pass I'm not exporting the subclass_register/unregister functions, but am expecting callers to use the new class_subclass_create() call, and older callers to continue using class_create() which is now static inline.
Callers to class_create() are expected to call class_destroy() on the subclasses before the parent class.
Feedback appreciated.
Thanks,
Matt
Signature · 135 lines
Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com
--- linux-2.6/drivers/base/class.c 2005-07-06 14:19:37.000000000 -0400
+++ linux-2.6/drivers/base/class.c 2005-07-08 18:41:19.000000000 -0400
@@ -133,6 +133,26 @@ static void remove_class_attrs(struct cl
}
}
+static int class_subclass_register(struct class *subclass)
+{
+ struct class *parent;
+
+ if (!subclass)
+ return -ENODEV;
+
+ parent = class_get(subclass->parent);
+ if (!parent)
+ return -EINVAL;
+
+ subclass->subsys.kset.kobj.parent = &parent->subsys.kset.kobj;
+
+ down(&parent->sem);
+ list_add_tail(&parent->subclasses, &subclass->subclass_node);
+ up(&parent->sem);
+
+ return 0;
+}
+
int class_register(struct class * cls)
{
int error;
@@ -141,6 +161,8 @@ int class_register(struct class * cls)
INIT_LIST_HEAD(&cls->children);
INIT_LIST_HEAD(&cls->interfaces);
+ INIT_LIST_HEAD(&cls->subclasses);
+ INIT_LIST_HEAD(&cls->subclass_node);
init_MUTEX(&cls->sem);
error = kobject_set_name(&cls->subsys.kset.kobj, "%s", cls->name);
if (error)
@@ -148,6 +170,7 @@ int class_register(struct class * cls)
subsys_set_kset(cls, class_subsys);
+ class_subclass_register(cls);
error = subsystem_register(&cls->subsys);
if (!error) {
error = add_class_attrs(class_get(cls));
@@ -156,10 +179,25 @@ int class_register(struct class * cls)
return error;
}
+static void class_subclass_unregister(struct class *subclass)
+{
+ struct class *parent = subclass->parent;
+
+ if (!parent)
+ return;
+
+ down(&parent->sem);
+ list_del_init(&subclass->subclass_node);
+ subclass->parent = NULL;
+ up(&parent->sem);
+ class_put(parent);
+}
+
void class_unregister(struct class * cls)
{
pr_debug("device class '%s': unregistering\n", cls->name);
remove_class_attrs(cls);
+ class_subclass_unregister(cls);
subsystem_unregister(&cls->subsys);
}
@@ -184,7 +222,7 @@ static void class_device_create_release(
* Note, the pointer created here is to be destroyed when finished by
* making a call to class_destroy().
*/
-struct class *class_create(struct module *owner, char *name)
+struct class *class_subclass_create(struct module *owner, char *name, struct class *parent)
{
struct class *cls;
int retval;
@@ -200,6 +238,7 @@ struct class *class_create(struct module
cls->owner = owner;
cls->class_release = class_create_release;
cls->release = class_device_create_release;
+ cls->parent = parent;
retval = class_register(cls);
if (retval)
@@ -738,8 +777,8 @@ EXPORT_SYMBOL_GPL(class_register);
EXPORT_SYMBOL_GPL(class_unregister);
EXPORT_SYMBOL_GPL(class_get);
EXPORT_SYMBOL_GPL(class_put);
-EXPORT_SYMBOL_GPL(class_create);
+EXPORT_SYMBOL_GPL(class_subclass_create);
EXPORT_SYMBOL_GPL(class_destroy);
EXPORT_SYMBOL_GPL(class_device_register);
--- linux-2.6/include/linux/device.h 2005-07-06 14:21:20.000000000 -0400
+++ linux-2.6-input/include/linux/device.h 2005-07-08 18:39:16.000000000 -0400
@@ -155,11 +155,14 @@ struct device * driver_find_device(struc
struct class {
const char * name;
struct module * owner;
+ struct class * parent; /* for subclasses */
struct subsystem subsys;
- struct list_head children;
+ struct list_head children; /* for class_devices */
struct list_head interfaces;
- struct semaphore sem; /* locks both the children and interfaces lists */
+ struct list_head subclasses;
+ struct list_head subclass_node; /* parent->sem held when accessing */
+ struct semaphore sem; /* locks children, subclasses, interfaces lists */
struct class_attribute * class_attrs;
struct class_device_attribute * class_dev_attrs;
@@ -258,7 +261,11 @@ struct class_interface {
extern int class_interface_register(struct class_interface *);
extern void class_interface_unregister(struct class_interface *);
-extern struct class *class_create(struct module *owner, char *name);
+extern struct class *class_subclass_create(struct module *owner, char *name, struct class *parent);
+static inline struct class *class_create(struct module *owner, char *name)
+{
+ return class_subclass_create(owner, name, NULL);
+}
extern void class_destroy(struct class *cls);
extern struct class_device *class_device_create(struct class *cls, dev_t devt,
struct device *device, char *fmt, ...)Quoted 8 lines · Matt Domsch
Greg,
The below patch is a first pass at implementing subclasses, for review and comment.
As a test, I modified
drivers/input/input.cto allocate a new class, and register it as a subclass.
Quoted 7 lines
Before:
/sys/class/input//sys/class/input_device/After:
/sys/class/input/input_device/
Oops, when you mentioned this on irc, I thought you were referring to class_devices not classes. I don't want classes to be able to be nested, only class devices.
I don't see a need for nested classes, as I thought the input thread had resolved itself so that it didn't need it (but I stopped paying attention, sorry, so I might be wrong here...)
Why can't you just use class_device's that can have children? That way the /sys/block stuff could be converted to also use this.
thanks,
greg k-h
Quoted 11 lines · Greg KH
Quoted 2 lines · Matt Domsch
The below patch is a first pass at implementing subclasses, for review and comment.
Oops, when you mentioned this on irc, I thought you were referring to
class_devicesnot classes. I don't want classes to be able to be nested, only class devices.I don't see a need for nested classes, as I thought the input thread had resolved itself so that it didn't need it (but I stopped paying attention, sorry, so I might be wrong here...)
The tailing part of the thread is here:
http://marc.theaimsgroup.com/?l=linux-kernel&m=111873628324649&w=2
http://marc.theaimsgroup.com/?l=linux-hotplug-devel&m=111877313202313&w=2
where Hannes and Dmitry seemed to indicate (to my reading) that they were looking for subclasses. Perhaps it's not necessary.
Quoted 2 lines
Why can't you just use class_device's that can have children? That way the
/sys/blockstuff could be converted to also use this.
I will investigate.
Thanks,
Matt
Signature · 4 lines
Matt Domsch Software Architect Dell Linux Solutions linux.dell.com & www.dell.com/linux Linux on Dell mailing lists @ http://lists.us.dell.com
Quoted 10 lines · Matt Domsch
Quoted 9 lines · Greg KH
Quoted 2 lines · Matt Domsch
The below patch is a first pass at implementing subclasses, for review and comment.
Oops, when you mentioned this on irc, I thought you were referring to
class_devicesnot classes. I don't want classes to be able to be nested, only class devices.I don't see a need for nested classes,
[dtor@core ~]$ ls -1d /sys/class/ieee1394*/sys/class/ieee1394/sys/class/ieee1394_host/sys/class/ieee1394_node/sys/class/ieee1394_protocol
This one is worst. But USB, SCSI, I2C all could use subclasses too.
Quoted 4 lines
Quoted 3 lines
as I thought the input thread had
resolved itself so that it didn't need it (but I stopped paying attention, sorry, so I might be wrong here...)
As far as input subsystem goes we do need nested classes if we were to keep compatibility with hotplug/udev installations. Please take a look at the patch below. It complements my version of nested classes patch (I am attaching it too since I think Matt's version is too complex). Basically top class will define subsystem while nested classes will define individual components of subsystem. This will allow keeping old input hotplug handlers for both input devices and interface devices.
I want the tree look somethng like this:
/sys/class/input/input_dev/input_dev0
| input_dev1
| input_dev2
|
/input_intf/mouse0
mice
js1
js2 Or we can go even deeper makeing input_intf a subtree with a subclass for every interface type (evdev, mousedev, etc). But I think that would be overkill.
Quoted 9 lines
The tailing part of the thread is here:
http://marc.theaimsgroup.com/?l=linux-kernel&m=111873628324649&w=2
http://marc.theaimsgroup.com/?l=linux-hotplug-devel&m=111877313202313&w=2
where Hannes and Dmitry seemed to indicate (to my reading) that they were looking for subclasses. Perhaps it's not necessary.Quoted 2 lines
Why can't you just use class_device's that can have children? That way the
/sys/blockstuff could be converted to also use this.
We need both I think.
Signature · 57 lines
Dmitry =================================================================== Driver core: make parent class define subsystem for its children When there is a hierarchy of classes make parent class define subsystem as reported in hotplug notifications. The full class path is reported in a new CLASS environment variable. Signed-off-by: Dmitry Torokhov <[email protected]> --- drivers/base/class.c | 14 ++++++++++++-- 1 files changed, 12 insertions(+), 2 deletions(-) Index: work/drivers/base/class.c =================================================================== --- work.orig/drivers/base/class.c +++ work/drivers/base/class.c @@ -332,25 +332,35 @@ static int class_hotplug_filter(struct k static const char *class_hotplug_name(struct kset *kset, struct kobject *kobj) { struct class_device *class_dev = to_class_dev(kobj); + struct class *class = class_dev->class; - return class_dev->class->name; + while (class->parent) + class = class->parent; + + return class->name; } static int class_hotplug(struct kset *kset, struct kobject *kobj, char **envp, int num_envp, char *buffer, int buffer_size) { struct class_device *class_dev = to_class_dev(kobj); + const char *path; int i = 0; int length = 0; int retval = 0; pr_debug("%s - name = %s\n", __FUNCTION__, class_dev->class_id); + path = kobject_get_path(&class_dev->class->subsys.kset.kobj, GFP_KERNEL); + add_hotplug_env_var(envp, num_envp, &i, buffer, buffer_size, + &length, "CLASS=%s", path); + kfree(path); + if (class_dev->dev) { /* add physical device, backing this device */ struct device *dev = class_dev->dev; - char *path = kobject_get_path(&dev->kobj, GFP_KERNEL); + path = kobject_get_path(&dev->kobj, GFP_KERNEL); add_hotplug_env_var(envp, num_envp, &i, buffer, buffer_size, &length, "PHYSDEVPATH=%s", path); kfree(path);
Subject: Allow nesting classes
Driver core: allow classes have children.
---
drivers/base/class.c | 5 ++++- include/linux/device.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-)
drivers/base/class.c +4 -1
@@ -146,7 +146,10 @@ int class_register(struct class * cls)if (error)return error; -subsys_set_kset (cls, class_subsys); +if (cls->parent) +subsys_set_kset (cls, cls->parent->subsys); +else +subsys_set_kset (cls, class_subsys); error =subsystem_register (&cls->subsys);if (!error) {
include/linux/device.h +1 -0
@@ -157,6 +157,7 @@ struct class {struct module * owner;struct subsystem subsys; +struct class * parent;struct list_head children;struct list_head interfaces;struct semaphore sem;/* locks both the children and interfaces lists */