Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[RFC] subclasses in sysfs to solve world peace

16 views
Skip to first unread message

Greg KH

unread,
Sep 15, 2005, 8:21:30 PM9/15/05
to Dmitry Torokhov, linux-...@vger.kernel.org, Kay Sievers, Vojtech Pavlik, Hannes Reinecke, Patrick Mochel, air...@linux.ie
Ok, first off, I hate talking about architecture changes without showing
the code for what I am talking about first, but as this is an issue that
needs to be agreed upon by a wide range of developers, I'll just write
up what I am thinking about doing before actually doing it...

The problem: We need a way to show complex class and class device
structures properly in sysfs. Examples of these "complex" views are the
input layer's use of different input drivers for devices (usually the
same device), the video subsystem view of frame buffer devices and
monitors, and even the block layer with it's disks and partitions.

Proposed solutions in the past have been either add the ability to nest
classes themselves (as shown in Dmitry's recent proposal for fixing the
input layer), or add the ability to nest class_device structures (I
think others have tried to do this in the past, sorry for not
remembering the specifics.) Both of these proposals don't really solve
the real problem, that of the fact that we need to come up with a
solution that all of the different subsystems can use properly.

So, here's my take on it. Feel free to tell me what I messed up :)

I would like to add something called "subclasses" for lack of a better
term. These subclasses would have both drivers, and devices associated
with them. This would show up as the following tree of directories:

/sys/class/input/
|-- input0
| |-- event0
| `-- mouse0
|-- input1
| |-- event1
| `-- ts0
|-- mice
`-- drivers
|-- event
|-- mouse
`-- ts

Here we have 3 struct class_devices like today, input0, input1, and
mice. We also have struct subclass_drivers of event, mouse, and ts.
These will create the struct subclass_devices event0, mouse0, event1,
and ts0. The "dev" node files will show up in the directories mice,
event0, mouse0, event1, and ts0, like you would expect them too.

So, this lets us create a solution for the input subsystem, but will it
also work for block and video?

For block, yes, I think so. There is no requirement for a
subclass_driver to create the subclass devices. Any code can create and
register with the class core a subclass device, just set up the parent
pointer and away you go. So, in the following instance:
/sys/class/block/
`-- sda
|-- sda1
|-- sda2
`-- sda3

"block" would represent the struct class.
"sda" would represent the struct class_device.
"sda1", "sda2", and "sda3" would represent the different subclass
devices that are bound to the class device "sda".

And yes, before you all point out the class_interface logic, yes, the
subclass_driver stuff looks a lot like this idea. Possibly we could
just get rid of the interface stuff and use the subclass_driver logic,
but I'd have to look at how people are using the interface logic before
I can give a confident answer about that.

Hotplug events would be simple with this scheme, the class stuff would
remain the same, and the devpath would just point to the subdir (hotplug
events would happen for both the class devices and the subclass
devices.) And yes, udev and libsysfs would have to be changed to
support this, so we better get this right before pushing it out to the
world...

But, what about video devices? David and Pat, we talked about this at
OLS, but Pat kept the paper we drew on, and the beer we were drinking at
the time has made my memory a bit fuzzy as to all of your requirements
for the video subsystem. I remember things about frame buffers and
monitors and other things like that, but nothing specific, sorry. Could
you outline your needs and I'll see if this proposed structure would
solve your issues?

Well, that was a very brief summary, of which I know there are lots of
questions for the areas I didn't explain well enough. Comments?
Criticisms? Want to see the code before commenting?

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Dmitry Torokhov

unread,
Sep 15, 2005, 8:59:47 PM9/15/05
to Greg KH, linux-...@vger.kernel.org, Kay Sievers, Vojtech Pavlik, Hannes Reinecke, Patrick Mochel, air...@linux.ie

This proposed scheme does not answer the question: "what input interfaces
present in my system?". Input interfaces are objects in their own right
and deserve their own spot. Compare your picture with the one below:

[dtor@core ~]$ tree /sys/class/input/
/sys/class/input/
|-- devices
| |-- input0
| | |-- device -> ../../../../devices/platform/i8042/serio1
| | `-- event0 -> ../../../../class/input/interfaces/event0
| |-- input1
| | |-- device -> ../../../../devices/platform/i8042/serio0
| | |-- event1 -> ../../../../class/input/interfaces/event1
| | |-- mouse0 -> ../../../../class/input/interfaces/mouse0
| | `-- ts0 -> ../../../../class/input/interfaces/ts0
| `-- input2
| |-- device -> ../../../../devices/platform/i8042/serio0/serio2
| |-- event2 -> ../../../../class/input/interfaces/event2
| |-- mouse1 -> ../../../../class/input/interfaces/mouse1
| `-- ts1 -> ../../../../class/input/interfaces/ts1
`-- interfaces
|-- event0
| |-- dev
| `-- device -> ../../../../class/input/devices/input0
|-- event1
| |-- dev
| `-- device -> ../../../../class/input/devices/input1
|-- event2
| |-- dev
| `-- device -> ../../../../class/input/devices/input2
|-- mice
| `-- dev
|-- mouse0
| |-- dev
| `-- device -> ../../../../class/input/devices/input1
|-- mouse1
| |-- dev
| `-- device -> ../../../../class/input/devices/input2
|-- ts0
| |-- dev
| `-- device -> ../../../../class/input/devices/input1
`-- ts1
|-- dev
`-- device -> ../../../../class/input/devices/input2

Here you have exactly the same information as in your picture plus you can
see the other class (input interfaces) as well.

The only issue is that links between those class devices are created in
input core but we can solve it. We just need to allow class devices be
children of other class devices.

> So, this lets us create a solution for the input subsystem, but will it
> also work for block and video?
>
> For block, yes, I think so. There is no requirement for a
> subclass_driver to create the subclass devices. Any code can create and
> register with the class core a subclass device, just set up the parent
> pointer and away you go. So, in the following instance:
> /sys/class/block/
> `-- sda
> |-- sda1
> |-- sda2
> `-- sda3
>

Here is a different puzzle. Instead of adding interfaces to a device you
partition it. I don't think we need to mix those 2 tasks together.


--
Dmitry

Kay Sievers

unread,
Sep 15, 2005, 9:05:07 PM9/15/05
to Greg KH, Dmitry Torokhov, linux-...@vger.kernel.org, Vojtech Pavlik, Hannes Reinecke, Patrick Mochel, air...@linux.ie

I like that the child devices are actually below the parent device
and represent the logical structure. I prefer that compared to the
symlink-representation between the classes at the same directory
level which the input patches propose.

> Here we have 3 struct class_devices like today, input0, input1, and
> mice. We also have struct subclass_drivers of event, mouse, and ts.
> These will create the struct subclass_devices event0, mouse0, event1,
> and ts0. The "dev" node files will show up in the directories mice,
> event0, mouse0, event1, and ts0, like you would expect them too.
>
> So, this lets us create a solution for the input subsystem, but will it
> also work for block and video?
>
> For block, yes, I think so. There is no requirement for a
> subclass_driver to create the subclass devices. Any code can create and
> register with the class core a subclass device, just set up the parent
> pointer and away you go. So, in the following instance:
> /sys/class/block/
> `-- sda
> |-- sda1
> |-- sda2
> `-- sda3
>
> "block" would represent the struct class.
> "sda" would represent the struct class_device.
> "sda1", "sda2", and "sda3" would represent the different subclass
> devices that are bound to the class device "sda".

How will the SUBSYSTEM (kset name) value look like for a "subclass"?
Will it have it's own value or will all class devices and subclass
devices share the same SUBSYSTEM?

What are the "subclass drivers"? Similar to the current "bus drivers"?

Will it be possible to have subclasses of subclasses? :)

Thanks,
Kay

Dmitry Torokhov

unread,
Sep 15, 2005, 9:27:37 PM9/15/05
to Kay Sievers, Greg KH, linux-...@vger.kernel.org, Vojtech Pavlik, Hannes Reinecke, Patrick Mochel, air...@linux.ie
On Thursday 15 September 2005 20:04, Kay Sievers wrote:
> I like that the child devices are actually below the parent device
> and represent the logical structure. I prefer that compared to the
> symlink-representation between the classes at the same directory
> level which the input patches propose.

Why don't we take it a step further and abandon classes altogether?
This way everything will grow from their respective hardware devices.

Class represent a set of objects with similar characteristics. In
this regard event0 is no "lesser" than input0. Although they are
linked they are objects of the same importance. I do want to see
all input interfaces without scanning bunch of directories.

--
Dmitry

Kay Sievers

unread,
Sep 15, 2005, 9:47:36 PM9/15/05
to Dmitry Torokhov, Greg KH, linux-...@vger.kernel.org, Vojtech Pavlik, Hannes Reinecke, Patrick Mochel, air...@linux.ie
On Thu, Sep 15, 2005 at 07:58:44PM -0500, Dmitry Torokhov wrote:
> On Thursday 15 September 2005 19:20, Greg KH wrote:
> > I would like to add something called "subclasses" for lack of a better
> > term. These subclasses would have both drivers, and devices associated
> > with them. This would show up as the following tree of directories:
> >
> > /sys/class/input/
> > |-- input0
> > | |-- event0
> > | `-- mouse0
> > |-- input1
> > | |-- event1
> > | `-- ts0
> > |-- mice
> > `-- drivers
> > |-- event
> > |-- mouse
> > `-- ts
> >
> > Here we have 3 struct class_devices like today, input0, input1, and
> > mice. We also have struct subclass_drivers of event, mouse, and ts.
> > These will create the struct subclass_devices event0, mouse0, event1,
> > and ts0. The "dev" node files will show up in the directories mice,
> > event0, mouse0, event1, and ts0, like you would expect them too.
>
> This proposed scheme does not answer the question: "what input interfaces
> present in my system?".

If this is really needed, we can just create a "interfaces" directory at
every "class" top-level and put symlinks pointing to all interfaces of that
class into it. How does that sound?

> Input interfaces are objects in their own right
> and deserve their own spot. Compare your picture with the one below:
>
> [dtor@core ~]$ tree /sys/class/input/
> /sys/class/input/
> |-- devices
> | |-- input0
> | | |-- device -> ../../../../devices/platform/i8042/serio1
> | | `-- event0 -> ../../../../class/input/interfaces/event0

> `-- interfaces


> |-- event0
> | |-- dev
> | `-- device -> ../../../../class/input/devices/input0
>

> Here you have exactly the same information as in your picture plus you can
> see the other class (input interfaces) as well.

Well, this is just putting two different classes into one subdirectory. :)
We would better just add "/sys/class/input_device" and don't need to change
any userspace software then.
Sure there is the cosmetical difference that the two classes are just named
input* and not live in the same directory, but that's not enough reason
to start a complete different model in sysfs, I think.

I would like to have the option to move "block" into "class" some day
and therefore prefer the "stacking class devices", compared to the "grouping
and symlinking" classes model.

Kay

David Lang

unread,
Sep 15, 2005, 9:47:34 PM9/15/05
to linux-...@vger.kernel.org
Lee Revell wrote

> BTW the rate of kernel development is just insane these days, even
> compared to a year ago. It's quite encouraging. At this rate we're
> going to leave the "competition" in the dust.

no kidding, it wasn't that long ago that complete rc patches were just a
couple meg, now the rc patch _changelog_ is 1.4MB

I know, part of this is due to the 'merge it all in the first two weeks'
approach, but even so WOW!!

David Lang
--
There are two ways of constructing a software design. One way is to make it so simple that there are obviously no deficiencies. And the other way is to make it so complicated that there are no obvious deficiencies.
-- C.A.R. Hoare

Kay Sievers

unread,
Sep 15, 2005, 9:54:41 PM9/15/05
to Dmitry Torokhov, Greg KH, linux-...@vger.kernel.org, Vojtech Pavlik, Hannes Reinecke, Patrick Mochel, air...@linux.ie
On Thu, Sep 15, 2005 at 08:23:43PM -0500, Dmitry Torokhov wrote:
> On Thursday 15 September 2005 20:04, Kay Sievers wrote:
> > I like that the child devices are actually below the parent device
> > and represent the logical structure. I prefer that compared to the
> > symlink-representation between the classes at the same directory
> > level which the input patches propose.
>
> Why don't we take it a step further and abandon classes altogether?
> This way everything will grow from their respective hardware devices.

Not everything is hardware. :)

> Class represent a set of objects with similar characteristics. In
> this regard event0 is no "lesser" than input0. Although they are
> linked they are objects of the same importance. I do want to see
> all input interfaces without scanning bunch of directories.

No problem, how about this:


/sys/class/input/
|-- input0
| |-- event0

| | `-- dev
| `-- mouse0
| | `-- dev

|-- input1
| |-- event1
| | `-- dev
| `-- ts0
| | `-- dev
|-- mice
| `-- dev
`-- interfaces
|-- event0 ->·../input0/event0
|-- event1 ->·../input1/event1
|-- mouse0 ->·../input0/mouse0
|-- mice -> ../mice
`-- ts0 -> ../input1/ts0

Kay

Dmitry Torokhov

unread,
Sep 15, 2005, 9:58:52 PM9/15/05
to Kay Sievers, Greg KH, linux-...@vger.kernel.org, Vojtech Pavlik, Hannes Reinecke, Patrick Mochel, air...@linux.ie
On Thursday 15 September 2005 20:46, Kay Sievers wrote:
>
> I would like to have the option to move "block" into "class" some day
> and therefore prefer the "stacking class devices", compared to the "grouping
> and symlinking" classes model.
>

The question is why can't we have both models? I agree that for block
you want what Greg is proposing, for input and some other subsystems
- not so much.

--
Dmitry

Dmitry Torokhov

unread,
Sep 15, 2005, 10:04:14 PM9/15/05
to Kay Sievers, Greg KH, linux-...@vger.kernel.org, Vojtech Pavlik, Hannes Reinecke, Patrick Mochel, air...@linux.ie

I am thinking... the rule would be - when adding a class device if it
has a class_device parent then it gets added to parent's directory and
symlinked into class. Otherwise it gets added into class directory.

I do not want to have a separate subclass_device structure...

--
Dmitry

Kay Sievers

unread,
Sep 15, 2005, 10:15:17 PM9/15/05
to Dmitry Torokhov, Greg KH, linux-...@vger.kernel.org, Vojtech Pavlik, Hannes Reinecke, Patrick Mochel, air...@linux.ie

Like this?

/sys/class/input/
|-- input0
| |-- event0
| | `-- dev
| `-- mouse0
| | `-- dev
|-- input1
| |-- event1
| | `-- dev
| `-- ts0
| | `-- dev
|-- mice
| `-- dev

|-- event0 ->·input0/event0
|-- event1 ->·input1/event1
|-- mouse0 ->·input0/mouse0
`-- ts0 -> input1/ts0

Kay

Dmitry Torokhov

unread,
Sep 15, 2005, 10:40:12 PM9/15/05
to Kay Sievers, Greg KH, linux-...@vger.kernel.org, Vojtech Pavlik, Hannes Reinecke, Patrick Mochel, air...@linux.ie

No, like your first picture, except 'interfaces/mice' will be a directo
ry,
not a symlink, since it does not have class_device parent. I should hav
e
said "Otherwise it gets added into _its_ class directory".

--
Dmitry

Kay Sievers

unread,
Sep 15, 2005, 10:45:20 PM9/15/05
to Dmitry Torokhov, Greg KH, linux-...@vger.kernel.org, Vojtech Pavlik, Hannes Reinecke, Patrick Mochel, air...@linux.ie
On Thu, Sep 15, 2005 at 09:36:08PM -0500, Dmitry Torokhov wrote:
> On Thursday 15 September 2005 21:14, Kay Sievers wrote:
> > On Thu, Sep 15, 2005 at 09:03:41PM -0500, Dmitry Torokhov wrote:
> > > On Thursday 15 September 2005 20:54, Kay Sievers wrote:
> > > > On Thu, Sep 15, 2005 at 08:23:43PM -0500, Dmitry Torokhov wrote
:
> > > > > On Thursday 15 September 2005 20:04, Kay Sievers wrote:
> > > > > > I like that the child devices are actually below the parent
device
> > > > > > and represent the logical structure. I prefer that compared
to the

> > > > > > symlink-representation between the classes at the same dire
ctory
> > > > > > level which the input patches propose.
> > > > >
> > > > > Why don't we take it a step further and abandon classes altog
ether?
> > > > > This way everything will grow from their respective hardware
devices.
> > > >
> > > > Not everything is hardware. :)
> > > >
> > > > > Class represent a set of objects with similar characteristics

Ah, I see. But the second model would work without any changes to
existing software. :)

Kay

Dmitry Torokhov

unread,
Sep 15, 2005, 11:11:29 PM9/15/05
to Kay Sievers, Greg KH, linux-...@vger.kernel.org, Vojtech Pavlik, Hannes Reinecke, Patrick Mochel, air...@linux.ie
On Thursday 15 September 2005 21:43, Kay Sievers wrote:
>
> Ah, I see. But the second model would work without any changes to
> existing software. :)
>

Even for block? Besides, I don't think the result is "clean" even for
input devices. Interfaces would have to check whetehr the device is
input or interface device, etc, etc. I think it is time for change ;)

--
Dmitry

Dmitry Torokhov

unread,
Sep 16, 2005, 3:22:39 AM9/16/05
to Kay Sievers, Greg KH, linux-...@vger.kernel.org, Vojtech Pavlik, Hannes Reinecke, Patrick Mochel, air...@linux.ie
> >
> > No, like your first picture, except 'interfaces/mice' will be a directory,
> > not a symlink, since it does not have class_device parent. I should have
> > said "Otherwise it gets added into _its_ class directory".
>

Ok, this is _very_ raw and I am creating double symlinks somehow, but still
it shows it can be done:

[dtor@core ~]$ tree /sys/class/input/
/sys/class/input/
|-- devices
| |-- input0

| | |-- capabilities
| | | |-- abs
| | | |-- ev
| | | |-- ff
| | | |-- key
| | | |-- led
| | | |-- msc
| | | |-- rel
| | | |-- snd
| | | `-- sw
| | |-- device -> ../../../../devices/platform/i8042/serio1
| | |-- event0
| | | |-- dev
| | | `-- device -> ../../../../../class/input/devices/input0
| | |-- event0
| | | |-- dev
| | | `-- device -> ../../../../../class/input/devices/input0
| | |-- id
| | | |-- bustype
| | | |-- product
| | | |-- vendor
| | | `-- version
| | |-- name
| | |-- phys
| | `-- uniq
| |-- input1
| | |-- capabilities
| | | |-- abs
| | | |-- ev
| | | |-- ff
| | | |-- key
| | | |-- led
| | | |-- msc
| | | |-- rel
| | | |-- snd
| | | `-- sw
| | |-- device -> ../../../../devices/platform/i8042/serio0
| | |-- event1
| | | |-- dev
| | | `-- device -> ../../../../../class/input/devices/input1
| | |-- event1
| | | |-- dev
| | | `-- device -> ../../../../../class/input/devices/input1
| | |-- id
| | | |-- bustype
| | | |-- product
| | | |-- vendor
| | | `-- version
| | |-- mouse0
| | | |-- dev
| | | `-- device -> ../../../../../class/input/devices/input1
| | |-- mouse0
| | | |-- dev
| | | `-- device -> ../../../../../class/input/devices/input1
| | |-- name
| | |-- phys
| | |-- ts0
| | | |-- dev
| | | `-- device -> ../../../../../class/input/devices/input1
| | |-- ts0
| | | |-- dev
| | | `-- device -> ../../../../../class/input/devices/input1
| | `-- uniq
| `-- input2
| |-- capabilities
| | |-- abs
| | |-- ev
| | |-- ff
| | |-- key
| | |-- led
| | |-- msc
| | |-- rel
| | |-- snd
| | `-- sw


| |-- device -> ../../../../devices/platform/i8042/serio0/serio2
| |-- event2

| | |-- dev
| | `-- device -> ../../../../../class/input/devices/input2
| |-- event2
| | |-- dev
| | `-- device -> ../../../../../class/input/devices/input2
| |-- id
| | |-- bustype
| | |-- product
| | |-- vendor
| | `-- version
| |-- mouse1
| | |-- dev
| | `-- device -> ../../../../../class/input/devices/input2
| |-- mouse1
| | |-- dev
| | `-- device -> ../../../../../class/input/devices/input2
| |-- name
| |-- phys
| |-- ts1
| | |-- dev
| | `-- device -> ../../../../../class/input/devices/input2
| |-- ts1
| | |-- dev
| | `-- device -> ../../../../../class/input/devices/input2
| `-- uniq
`-- interfaces
|-- event0 -> ../../../class/input/devices/input0/event0
|-- event1 -> ../../../class/input/devices/input1/event1
|-- event2 -> ../../../class/input/devices/input2/event2
|-- mice
| `-- dev
|-- mouse0 -> ../../../class/input/devices/input1/mouse0
|-- mouse1 -> ../../../class/input/devices/input2/mouse1
|-- ts0 -> ../../../class/input/devices/input1/ts0
`-- ts1 -> ../../../class/input/devices/input2/ts1

--
Dmitry

Subject: XXX
Signed-off-by: Dmitry Torokhov <dt...@mail.ru>
---

drivers/base/class.c | 81 +++++++++++++++++++++++++++++++++++--------------
drivers/input/input.c | 6 +++
include/linux/device.h | 5 ++-
3 files changed, 67 insertions(+), 25 deletions(-)

Index: work/drivers/base/class.c
===================================================================
--- work.orig/drivers/base/class.c
+++ work/drivers/base/class.c
@@ -485,7 +485,7 @@ static char *make_class_name(struct clas

int class_device_add(struct class_device *class_dev)
{
- struct class * parent = NULL;
+ struct class * class = NULL;
struct class_interface * class_intf;
char *class_name = NULL;
int error;
@@ -499,15 +499,18 @@ int class_device_add(struct class_device
goto register_done;
}

- parent = class_get(class_dev->class);
+ class = class_get(class_dev->class);

pr_debug("CLASS: registering class device: ID = '%s'\n",
class_dev->class_id);

/* first, register with generic layer. */
kobject_set_name(&class_dev->kobj, "%s", class_dev->class_id);
- if (parent)
- class_dev->kobj.parent = &parent->subsys.kset.kobj;
+
+ if (is_a_class_device(class_dev->parent))
+ class_dev->kobj.parent = class_dev->parent;
+ else if (class)
+ class_dev->kobj.parent = &class->subsys.kset.kobj;

if ((error = kobject_add(&class_dev->kobj)))
goto register_done;
@@ -524,7 +527,7 @@ int class_device_add(struct class_device

attr->attr.name = "dev";
attr->attr.mode = S_IRUGO;
- attr->attr.owner = parent->owner;
+ attr->attr.owner = class ? class->owner : NULL;
attr->show = show_dev;
attr->store = NULL;
class_device_create_file(class_dev, attr);
@@ -532,31 +535,45 @@ int class_device_add(struct class_device
}

class_device_add_attrs(class_dev);
+
+ /* this will go away */
if (class_dev->dev) {
class_name = make_class_name(class_dev);
- sysfs_create_link(&class_dev->kobj,
- &class_dev->dev->kobj, "device");
+ sysfs_create_link(&class_dev->kobj, &class_dev->dev->kobj,
+ "device");
sysfs_create_link(&class_dev->dev->kobj, &class_dev->kobj,
class_name);
+ kfree(class_name);
+ } else if (class_dev->parent) {
+ class_name = make_class_name(class_dev);
+ sysfs_create_link(&class_dev->kobj, class_dev->parent,
+ "device");
+ sysfs_create_link(class_dev->parent, &class_dev->kobj,
+ is_a_class_device(class_dev->parent) ?
+ class_dev->class_id : class_name);
+ kfree(class_name);
}

+ if (class && is_a_class_device(class_dev->parent))
+ sysfs_create_link(&class->subsys.kset.kobj, &class_dev->kobj,
+ class_dev->class_id);
+
kobject_hotplug(&class_dev->kobj, KOBJ_ADD);

/* notify any interfaces this device is now here */
- if (parent) {
- down(&parent->sem);
- list_add_tail(&class_dev->node, &parent->children);
- list_for_each_entry(class_intf, &parent->interfaces, node)
+ if (class) {
+ down(&class->sem);
+ list_add_tail(&class_dev->node, &class->children);
+ list_for_each_entry(class_intf, &class->interfaces, node)
if (class_intf->add)
class_intf->add(class_dev, class_intf);
- up(&parent->sem);
+ up(&class->sem);
}

register_done:
- if (error && parent)
- class_put(parent);
+ if (error && class)
+ class_put(class);
class_device_put(class_dev);
- kfree(class_name);
return error;
}

@@ -619,34 +636,48 @@ error:

void class_device_del(struct class_device *class_dev)
{
- struct class * parent = class_dev->class;
+ struct class * class = class_dev->class;
struct class_interface * class_intf;
char *class_name = NULL;

- if (parent) {
- down(&parent->sem);
+ if (class) {
+ down(&class->sem);
list_del_init(&class_dev->node);
- list_for_each_entry(class_intf, &parent->interfaces, node)
+ list_for_each_entry(class_intf, &class->interfaces, node)
if (class_intf->remove)
class_intf->remove(class_dev, class_intf);
- up(&parent->sem);
+ up(&class->sem);
+ }
+
+ if (class && is_a_class_device(class_dev->parent))
+ sysfs_remove_link(&class->subsys.kset.kobj, class_dev->class_id);
+
+ if (class_dev->parent) {
+ class_name = make_class_name(class_dev);
+ sysfs_remove_link(&class_dev->kobj, "device");
+ sysfs_remove_link(class_dev->parent,
+ is_a_class_device(class_dev->parent) ?
+ class_dev->class_id : class_name);
+ kfree(class_name);
}

if (class_dev->dev) {
class_name = make_class_name(class_dev);
sysfs_remove_link(&class_dev->kobj, "device");
sysfs_remove_link(&class_dev->dev->kobj, class_name);
+ kfree(class_name);
}
+
if (class_dev->devt_attr)
class_device_remove_file(class_dev, class_dev->devt_attr);
+
class_device_remove_attrs(class_dev);

kobject_hotplug(&class_dev->kobj, KOBJ_REMOVE);
kobject_del(&class_dev->kobj);

- if (parent)
- class_put(parent);
- kfree(class_name);
+ if (class)
+ class_put(class);
}

void class_device_unregister(struct class_device *class_dev)
@@ -715,6 +746,10 @@ void class_device_put(struct class_devic
kobject_put(&class_dev->kobj);
}

+int is_a_class_device(struct kobject *kobj)
+{
+ return kobj && get_ktype(kobj) == &ktype_class_device;
+}

int class_interface_register(struct class_interface *class_intf)
{
Index: work/include/linux/device.h
===================================================================
--- work.orig/include/linux/device.h
+++ work/include/linux/device.h
@@ -199,7 +199,8 @@ struct class_device {
struct class * class; /* required */
dev_t devt; /* dev_t, creates the sysfs "dev" */
struct class_device_attribute *devt_attr;
- struct device * dev; /* not necessary, but nice to have */
+ struct device * dev; /* not necessary, but nice to have (going away) */
+ struct kobject * parent; /* either device or class_device */
void * class_data; /* class-specific data */

char class_id[BUS_ID_SIZE]; /* unique to this class */
@@ -229,6 +230,8 @@ extern int class_device_rename(struct cl
extern struct class_device * class_device_get(struct class_device *);
extern void class_device_put(struct class_device *);

+extern int is_a_class_device(struct kobject *);
+
struct class_device_attribute {
struct attribute attr;
ssize_t (*show)(struct class_device *, char * buf);
Index: work/drivers/input/input.c
===================================================================
--- work.orig/drivers/input/input.c
+++ work/drivers/input/input.c
@@ -948,18 +948,22 @@ int input_create_interface_device(struct

dev->devt = devt;
dev->class = &input_interface_class;
+ if (handle->dev)
+ dev->parent = &handle->dev->cdev.kobj;
strlcpy(dev->class_id, handle->name, BUS_ID_SIZE);

+
error = class_device_register(dev);
if (error)
return error;

+/*
if (handle->dev) {
sysfs_create_link(&dev->kobj, &handle->dev->cdev.kobj, "device");
sysfs_create_link(&handle->dev->cdev.kobj, &dev->kobj,
kobject_name(&dev->kobj));
}
-
+*/
handle->intf_dev = dev;

__module_get(THIS_MODULE);

Vojtech Pavlik

unread,
Sep 16, 2005, 4:03:59 AM9/16/05
to Kay Sievers, Greg KH, Dmitry Torokhov, linux-...@vger.kernel.org, Hannes Reinecke, Patrick Mochel, air...@linux.ie

I like this one better, too. It's much simpler, and does the job just as
well.

--
Vojtech Pavlik
SuSE Labs, SuSE CR

Vojtech Pavlik

unread,
Sep 16, 2005, 4:04:00 AM9/16/05
to Dmitry Torokhov, Kay Sievers, Greg KH, linux-...@vger.kernel.org, Hannes Reinecke, Patrick Mochel, air...@linux.ie
On Thu, Sep 15, 2005 at 08:23:43PM -0500, Dmitry Torokhov wrote:
> On Thursday 15 September 2005 20:04, Kay Sievers wrote:
> > I like that the child devices are actually below the parent device
> > and represent the logical structure. I prefer that compared to the
> > symlink-representation between the classes at the same directory
> > level which the input patches propose.
>
> Why don't we take it a step further and abandon classes altogether?
> This way everything will grow from their respective hardware devices.

That'd seem like a quite a good idea to me. ;)

> Class represent a set of objects with similar characteristics. In
> this regard event0 is no "lesser" than input0.

Well, input0 itself can't be accessed from userspace, so it's different.

> Although they are
> linked they are objects of the same importance. I do want to see
> all input interfaces without scanning bunch of directories.

A directory with symlinks to all the interfaces of the class might make
sense.

--
Vojtech Pavlik
SuSE Labs, SuSE CR

Dmitry Torokhov

unread,
Sep 16, 2005, 11:45:01 AM9/16/05
to Vojtech Pavlik, Kay Sievers, Greg KH, linux-...@vger.kernel.org, Hannes Reinecke, Patrick Mochel, air...@linux.ie
On 9/16/05, Vojtech Pavlik <voj...@suse.cz> wrote:
> On Thu, Sep 15, 2005 at 08:23:43PM -0500, Dmitry Torokhov wrote:
> > On Thursday 15 September 2005 20:04, Kay Sievers wrote:
> > > I like that the child devices are actually below the parent device
> > > and represent the logical structure. I prefer that compared to the
> > > symlink-representation between the classes at the same directory
> > > level which the input patches propose.
> >
> > Why don't we take it a step further and abandon classes altogether?
> > This way everything will grow from their respective hardware devices.
>
> That'd seem like a quite a good idea to me. ;)
>

You just saying that ;)

Look at I2C/sensors people. They are moving from having every sensor
crampled into I2C bus to hwmon class so all the sensors can be easily
located (by some program or what's not).

> > Class represent a set of objects with similar characteristics. In
> > this regard event0 is no "lesser" than input0.
>
> Well, input0 itself can't be accessed from userspace, so it's different.
>

Why is this a factor? We are not talking about /dev here. We have a
lot of things in sysfs that are not directly accessible from
userspace.

> > Although they are
> > linked they are objects of the same importance. I do want to see
> > all input interfaces without scanning bunch of directories.
>
> A directory with symlinks to all the interfaces of the class might make
> sense.
>

I'll try fix the patch I posted last night (that implements the above,
or at least what Kay described with sub-devices residing under their
parent devices and symlinked into their classes), I believe it could
also be used for block, so it will be like:

../block/
|-- devices
| |-- sda
| | |-- device -> ../../../../
| | |-- sda1
| | | |-- dev
| | | `-- device -> ../../../../../block/partitions/sda1
| | |-- sda2
| | | |-- dev
| | | `-- device -> ../../../../../block/partitions/sda2
..
`-- partitions
|-- sda1 -> ../../../class/block/devices/sda/sda1
|-- sda2 -> ../../../class/block/devices/sda/sda2

--
Dmitry

Greg KH

unread,
Sep 16, 2005, 5:58:23 PM9/16/05
to Dmitry Torokhov, linux-...@vger.kernel.org, Kay Sievers, Vojtech Pavlik, Hannes Reinecke, Patrick Mochel, air...@linux.ie
On Thu, Sep 15, 2005 at 07:58:44PM -0500, Dmitry Torokhov wrote:
> On Thursday 15 September 2005 19:20, Greg KH wrote:
> >
> > /sys/class/input/
> > |-- input0
> > | |-- event0
> > | `-- mouse0
> > |-- input1
> > | |-- event1
> > | `-- ts0
> > |-- mice
> > `-- drivers
> > |-- event
> > |-- mouse
> > `-- ts
> >
> > Here we have 3 struct class_devices like today, input0, input1, and
> > mice. We also have struct subclass_drivers of event, mouse, and ts.
> > These will create the struct subclass_devices event0, mouse0, event1,
> > and ts0. The "dev" node files will show up in the directories mice,
> > event0, mouse0, event1, and ts0, like you would expect them too.
> >
>
> This proposed scheme does not answer the question: "what input interfaces
> present in my system?". Input interfaces are objects in their own right
> and deserve their own spot. Compare your picture with the one below:

Yes it does. They are the leaf nodes in the tree. They will also have
a "dev" file in them, which is the only real way to determine this for
stuff like udev and HAL.

I didn't show all the symlinks and files as I was trying to make the
directory structure the main point. I can fill them in if you are
curious.

Yeah, you can. But you don't have a place for the "drivers" of those
interfaces, which is something you will probably need (I think video
needs them)

> The only issue is that links between those class devices are created in
> input core but we can solve it. We just need to allow class devices be
> children of other class devices.

No, I still don't think we need that.

> > So, this lets us create a solution for the input subsystem, but will it
> > also work for block and video?
> >
> > For block, yes, I think so. There is no requirement for a
> > subclass_driver to create the subclass devices. Any code can create and
> > register with the class core a subclass device, just set up the parent
> > pointer and away you go. So, in the following instance:
> > /sys/class/block/
> > `-- sda
> > |-- sda1
> > |-- sda2
> > `-- sda3
> >
>
> Here is a different puzzle. Instead of adding interfaces to a device you
> partition it. I don't think we need to mix those 2 tasks together.

Well, whatever solution we come up with, has to work for block, input,
and video at the least, or it's not acceptable.

Ok, I'll go off and try to code this all up to see if it's even
possible, on my plane ride. More from me next Tuesday...

thanks,

greg k-h

Greg KH

unread,
Sep 16, 2005, 5:58:24 PM9/16/05
to Dmitry Torokhov, Kay Sievers, linux-...@vger.kernel.org, Vojtech Pavlik, Hannes Reinecke, Patrick Mochel, air...@linux.ie
On Thu, Sep 15, 2005 at 08:23:43PM -0500, Dmitry Torokhov wrote:
> On Thursday 15 September 2005 20:04, Kay Sievers wrote:
> > I like that the child devices are actually below the parent device
> > and represent the logical structure. I prefer that compared to the
> > symlink-representation between the classes at the same directory
> > level which the input patches propose.
>
> Why don't we take it a step further and abandon classes altogether?

Not going to happen, sorry :)

greg k-h

Greg KH

unread,
Sep 16, 2005, 5:59:17 PM9/16/05
to dtor...@ameritech.net, Vojtech Pavlik, Kay Sievers, linux-...@vger.kernel.org, Hannes Reinecke, Patrick Mochel, air...@linux.ie
On Fri, Sep 16, 2005 at 10:44:36AM -0500, Dmitry Torokhov wrote:
> I'll try fix the patch I posted last night (that implements the above,
> or at least what Kay described with sub-devices residing under their
> parent devices and symlinked into their classes), I believe it could
> also be used for block, so it will be like:
>
> .../block/

> |-- devices
> | |-- sda
> | | |-- device -> ../../../../
> | | |-- sda1
> | | | |-- dev
> | | | `-- device -> ../../../../../block/partitions/sda1
> | | |-- sda2
> | | | |-- dev
> | | | `-- device -> ../../../../../block/partitions/sda2
> ...

> `-- partitions
> |-- sda1 -> ../../../class/block/devices/sda/sda1
> |-- sda2 -> ../../../class/block/devices/sda/sda2

Nah, that's a mess. I think the proposal I had would work for both
input and block with a minimum of disruption. Still don't know about
video though, David said he would take some time this weekend to get me
some feedback, which is good, as I have to get on a 14 hour plane ride
soon...

thanks,

greg k-h

Greg KH

unread,
Sep 16, 2005, 5:59:30 PM9/16/05
to Kay Sievers, Dmitry Torokhov, linux-...@vger.kernel.org, Vojtech Pavlik, Hannes Reinecke, Patrick Mochel, air...@linux.ie
On Fri, Sep 16, 2005 at 03:04:38AM +0200, Kay Sievers wrote:
> How will the SUBSYSTEM (kset name) value look like for a "subclass"?

It would be the CLASS value, to make it easier for userspace.

> Will it have it's own value or will all class devices and subclass
> devices share the same SUBSYSTEM?

Yes, they will all share the same.

> What are the "subclass drivers"? Similar to the current "bus drivers"?

No, kinda like what the mouse driver is today. It's a type of driver
that creates subclass devices for class devices.

> Will it be possible to have subclasses of subclasses? :)

Heh, no, the tree stops here :)

thanks,

greg k-h

Greg KH

unread,
Sep 16, 2005, 5:59:45 PM9/16/05
to Dmitry Torokhov, Kay Sievers, linux-...@vger.kernel.org, Vojtech Pavlik, Hannes Reinecke, Patrick Mochel, air...@linux.ie
On Fri, Sep 16, 2005 at 02:21:53AM -0500, Dmitry Torokhov wrote:
> > >
> > > No, like your first picture, except 'interfaces/mice' will be a directory,
> > > not a symlink, since it does not have class_device parent. I should have
> > > said "Otherwise it gets added into _its_ class directory".
> >
>
> Ok, this is _very_ raw and I am creating double symlinks somehow, but still
> it shows it can be done:
>
> [dtor@core ~]$ tree /sys/class/input/
> /sys/class/input/
> |-- devices
> | |-- input0

Close, just drop the "devices" subdir, and you have my proposal, I'm
glad we agree :)

> `-- interfaces
> |-- event0 -> ../../../class/input/devices/input0/event0

I don't see why we need the interfaces subdir at all.

thanks,

greg k-h

Dmitry Torokhov

unread,
Sep 16, 2005, 6:46:05 PM9/16/05
to Greg KH, Kay Sievers, linux-...@vger.kernel.org, Vojtech Pavlik, Hannes Reinecke, Patrick Mochel, air...@linux.ie
On 9/16/05, Greg KH <gre...@suse.de> wrote:
> On Fri, Sep 16, 2005 at 03:04:38AM +0200, Kay Sievers wrote:
> > How will the SUBSYSTEM (kset name) value look like for a "subclass"?
>
> It would be the CLASS value, to make it easier for userspace.
>
> > Will it have it's own value or will all class devices and subclass
> > devices share the same SUBSYSTEM?
>
> Yes, they will all share the same.
>
> > What are the "subclass drivers"? Similar to the current "bus drivers"?
>
> No, kinda like what the mouse driver is today. It's a type of driver
> that creates subclass devices for class devices.
>
> > Will it be possible to have subclasses of subclasses? :)
>
> Heh, no, the tree stops here :)
>

I think you are potentially shooting yourself in the foot. There is no
technical reasons for limiting depth of the tree.

--
Dmitry

Dmitry Torokhov

unread,
Sep 16, 2005, 6:56:06 PM9/16/05
to Greg KH, Kay Sievers, linux-...@vger.kernel.org, Vojtech Pavlik, Hannes Reinecke, Patrick Mochel, air...@linux.ie
On 9/16/05, Greg KH <gre...@suse.de> wrote:
> On Fri, Sep 16, 2005 at 02:21:53AM -0500, Dmitry Torokhov wrote:
> > > >
> > > > No, like your first picture, except 'interfaces/mice' will be a directory,
> > > > not a symlink, since it does not have class_device parent. I should have
> > > > said "Otherwise it gets added into _its_ class directory".
> > >
> >
> > Ok, this is _very_ raw and I am creating double symlinks somehow, but still
> > it shows it can be done:
> >
> > [dtor@core ~]$ tree /sys/class/input/
> > /sys/class/input/
> > |-- devices
> > | |-- input0
>
> Close, just drop the "devices" subdir, and you have my proposal, I'm
> glad we agree :)
>

No, not entirely agree. I _want_ devices subdir to separate objects of
different [sub]classes. We don;'t mix SCSI class_devices with USB, why
messing it here?

> > `-- interfaces
> > |-- event0 -> ../../../class/input/devices/input0/event0
>
> I don't see why we need the interfaces subdir at all.
>

For the same reasons you have hwmon stuff - one shoudl not hunt
through maze of subdirectories and filter unwanted data to get list of
objects of given class.

--
Dmitry

Dmitry Torokhov

unread,
Sep 16, 2005, 6:57:14 PM9/16/05
to Greg KH, Vojtech Pavlik, Kay Sievers, linux-...@vger.kernel.org, Hannes Reinecke, Patrick Mochel, air...@linux.ie
On 9/16/05, Greg KH <gre...@suse.de> wrote:

You have to adjust udev to accept mouseX being not on /sys/class/input
level anyway so disruption is still here.

--
Dmitry

Antonino A. Daplas

unread,
Sep 16, 2005, 8:21:00 PM9/16/05
to Greg KH, Dmitry Torokhov, linux-...@vger.kernel.org, Kay Sievers, Vojtech Pavlik, Hannes Reinecke, Patrick Mochel, air...@linux.ie
Greg KH wrote:

>
> But, what about video devices? David and Pat, we talked about this at
> OLS, but Pat kept the paper we drew on, and the beer we were drinking at
> the time has made my memory a bit fuzzy as to all of your requirements
> for the video subsystem. I remember things about frame buffers and
> monitors and other things like that, but nothing specific, sorry. Could
> you outline your needs and I'll see if this proposed structure would
> solve your issues?
>

I'm still not very familiar with sysfs, but this is a possible simplistic
view for the graphics class:

/sys/class/graphics/
|-- fb0
| |-- framebuffer0
| `-- display0
|-- fb1
| |-- framebuffer1
| |-- display1
| '-- display2
|-- fb2
| |-- framebuffer2
| '-- display3
'-- fb3
|-- framebuffer3
'-- display3

graphics is the class
fb0, fb1, etc is the class_device
framebuffer and display are subclasses?

- fb0 is a simple device, one framebuffer attached to one display
- fb1 is one framebuffer with 2 displays (mirrored)
- perhaps, fb2 and fb3 are multi-head, different framebuffers, different displays
but same device

display does not have a driver as they are created by the framebuffer themselves,
is that okay?

How about backlight/lcd drivers? They can stand on their own, but if a framebuffer
driver is loaded, a backlight/lcd driver can be bound to fb.

How about i2c? Under display?

Offtopic:

Main limitation of sysfs concerning video devices is that the one value,
one attribute may not be appropriate. For example, setting xres and yres also
necessitates simultaneous changes in other attributes of the display (pixelclock,
margins, etc). Jon Smirl somewhat made it work by making mode attribute a string
and accept only modes that are present in the driver's private mode database.
Custom timings are not accepted unless the user updates the private mode database
of the driver (has to use an ioctl to do that).

Similarly, pixelformats are problematic. Again, Jon Smirl made this work somehow
by accepting strings. Custom pixelformats are again problematic, and one has
to use the ioctl to set that.

Tony

Dave Airlie

unread,
Sep 16, 2005, 8:49:12 PM9/16/05
to Greg KH, dtor...@ameritech.net, Vojtech Pavlik, Kay Sievers, linux-...@vger.kernel.org, Hannes Reinecke, Patrick Mochel

> Nah, that's a mess. I think the proposal I had would work for both
> input and block with a minimum of disruption. Still don't know about
> video though, David said he would take some time this weekend to get me
> some feedback, which is good, as I have to get on a 14 hour plane ride
> soon...
>

Okay from my hazy memories at KS and previous attempts at implementing
this (Patrick might be able to find the piece of paper with the pretty
pictures :-):

For video we need to be able to bind a number of sub-drivers to a toplevel
driver but have them participate in the whole driver architecture
correctly.

I think we need to end up with something like:
/sys/class/video_adapter
- /radeon0
/radeondrm0
/radeonfb0
/radeonsomethingIhaventthoughtabout0
- /radeon1
/radeondrm1
/radeonfb1
/radeonsomethingIhaven'tthoughtabout1
- /mga0
/mgadrm0
/mgafb0
/mgasomethingIhaventthoughtabout0

The lowlevel driver will be responsible for PCI handling for the card and
will have the list of PCI IDs to bind to, the higher level drivers need to
be dynamically inserted/removed and should be able to have instances of
themselves appear/disappear with hotplugging of the adapters.

My previous attempts at this led me to try to use a bus driver at the
radeon0 level and have my subdrivers appear on the "radeon" bus, also some
work done by Alan Cox on a vga class driver went the same direction, but
this got very unwieldly when I started to actually try and get it working.

To get the above scheme, something like a) start machine with one radeon
adapter, insert radeon_lowlevel module, it brings up
/sys/class/video_adapter/radeon0, insert radeondrm and radeonfb module
above it and you get the radeondrm0 and radeonfb0 subdrivers.
b) hotplug a second radeon, radeon1 appears with drm and fb drivers bound
to it as
well.
c) hotplug mga card, load the subdrivers and have it appear.

Hopefully there is enough info here to figure out if we can fit into your
propsed scheme, there may be an extra level in the sysfs hierarchy above
I'm not really sure, I'm not sure if it would end up as
/sys/class/video_adapter/video0/radeon0/radeondrm0 or
/sys/class/video_adapter/radeon/radeon0/radeondrm0

This is quite different that /sys/class/graphics stuff, it is more for the
lowlevel card drivers than the highlevel interfaces.

Regards,
Dave.

--
David Airlie, Software Engineer
http://www.skynet.ie/~airlied / airlied at skynet.ie
Linux kernel - DRI, VAX / pam_smb / ILUG

0 new messages