2002-07-29 00:57:55

by Adam Belay

[permalink] [raw]
Subject: [PATCH] integrate driverfs and devfs (2.5.28)

This patch integrates driverfs and devfs. A summary is as follows:
- create new interface directory and move interface.c
* I intend to add more to this directory later
- add devfs entry list
- add devfs related functions
- create devfs interface
This patch is intended to be as non intrusive as possible. Therefore
it doesn't modify devfs directly but instead creates a layer above it.
This is due to the fact that if devfs was modified it would break
every driver. Eventually we have to decide when and how to
integrate it directly, This patch will provide the necessary
infrastructure. Please Apply.
cheers,
Adam Belay

diff -ur --new-file a/drivers/base/Makefile b/drivers/base/Makefile
--- a/drivers/base/Makefile Sun Jul 28 20:28:33 2002
+++ b/drivers/base/Makefile Sat Jul 27 21:09:41 2002
@@ -1,7 +1,7 @@
# Makefile for the Linux device tree

-obj-y := core.o sys.o interface.o fs.o power.o bus.o \
- driver.o
+obj-y := core.o sys.o fs.o power.o bus.o \
+ driver.o interfaces/

export-objs := core.o fs.o power.o sys.o bus.o driver.o

diff -ur --new-file a/drivers/base/core.c b/drivers/base/core.c
--- a/drivers/base/core.c Fri Jun 21 09:15:52 2002
+++ b/drivers/base/core.c Sun Jul 28 16:06:39 2002
@@ -199,6 +199,7 @@
INIT_LIST_HEAD(&dev->node);
INIT_LIST_HEAD(&dev->children);
INIT_LIST_HEAD(&dev->g_list);
+ INIT_LIST_HEAD(&dev->devfs_handles);
spin_lock_init(&dev->lock);
atomic_set(&dev->refcount,2);

diff -ur --new-file a/drivers/base/interface.c b/drivers/base/interface.c
--- a/drivers/base/interface.c Tue Jun 4 00:07:28 2002
+++ b/drivers/base/interface.c Thu Jan 1 00:00:00 1970
@@ -1,103 +0,0 @@
-/*
- * drivers/base/interface.c - common driverfs interface that's exported to
- * the world for all devices.
- * Copyright (c) 2002 Patrick Mochel
- * 2002 Open Source Development Lab
- */
-
-#include <linux/device.h>
-#include <linux/err.h>
-#include <linux/stat.h>
-
-static ssize_t device_read_name(struct device * dev, char * buf, size_t
count, loff_t off)
-{
- return off ? 0 : sprintf(buf,"%s\n",dev->name);
-}
-
-static struct driver_file_entry device_name_entry = {
- name: "name",
- mode: S_IRUGO,
- show: device_read_name,
-};
-
-static ssize_t
-device_read_power(struct device * dev, char * page, size_t count,
loff_t off)
-{
- return off ? 0 : sprintf(page,"%d\n",dev->current_state);
-}
-
-static ssize_t
-device_write_power(struct device * dev, const char * buf, size_t count,
loff_t off)
-{
- char str_command[20];
- char str_level[20];
- int num_args;
- u32 state;
- u32 int_level;
- int error = 0;
-
- if (off)
- return 0;
-
- if (!dev->driver)
- goto done;
-
- num_args = sscanf(buf,"%10s %10s %u",str_command,str_level,&state);
-
- error = -EINVAL;
-
- if (!num_args)
- goto done;
-
- if (!strnicmp(str_command,"suspend",7)) {
- if (num_args != 3)
- goto done;
- if (!strnicmp(str_level,"notify",6))
- int_level = SUSPEND_NOTIFY;
- else if (!strnicmp(str_level,"save",4))
- int_level = SUSPEND_SAVE_STATE;
- else if (!strnicmp(str_level,"disable",7))
- int_level = SUSPEND_DISABLE;
- else if (!strnicmp(str_level,"powerdown",8))
- int_level = SUSPEND_POWER_DOWN;
- else
- goto done;
-
- if (dev->driver->suspend)
- error = dev->driver->suspend(dev,state,int_level);
- else
- error = 0;
- } else if (!strnicmp(str_command,"resume",6)) {
- if (num_args != 2)
- goto done;
-
- if (!strnicmp(str_level,"poweron",7))
- int_level = RESUME_POWER_ON;
- else if (!strnicmp(str_level,"restore",7))
- int_level = RESUME_RESTORE_STATE;
- else if (!strnicmp(str_level,"enable",6))
- int_level = RESUME_ENABLE;
- else
- goto done;
-
- if (dev->driver->resume)
- error = dev->driver->resume(dev,int_level);
- else
- error = 0;
- }
- done:
- return error < 0 ? error : count;
-}
-
-static struct driver_file_entry device_power_entry = {
- name: "power",
- mode: S_IWUSR | S_IRUGO,
- show: device_read_power,
- store: device_write_power,
-};
-
-struct driver_file_entry * device_default_files[] = {
- &device_name_entry,
- &device_power_entry,
- NULL,
-};
diff -ur --new-file a/drivers/base/interfaces/Makefile
b/drivers/base/interfaces/Makefile
--- a/drivers/base/interfaces/Makefile Thu Jan 1 00:00:00 1970
+++ b/drivers/base/interfaces/Makefile Sat Jul 27 12:56:29 2002
@@ -0,0 +1,7 @@
+# Makefile for the Linux device tree
+
+obj-y := interface.o devfs.o
+
+export-objs := devfs.o
+
+include $(TOPDIR)/Rules.make
diff -ur --new-file a/drivers/base/interfaces/devfs.c
b/drivers/base/interfaces/devfs.c
--- a/drivers/base/interfaces/devfs.c Thu Jan 1 00:00:00 1970
+++ b/drivers/base/interfaces/devfs.c Sun Jul 28 19:52:30 2002
@@ -0,0 +1,65 @@
+/*
+ * drivers/base/interfaces/dev.c - integrates devfs
+ * into the Driver Model and exports an
+ * interface to all devices.
+ * Copyright (c) 2002 Adam Belay ([email protected])
+ *
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+ *
+ *
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/stat.h>
+#include <linux/module.h>
+#include <linux/devfs_fs_kernel.h>
+#include <linux/slab.h>
+
+/**
+ * register_devfs_handle - registers a device handle into the driverfs
+ * @dhandle the devfs to register
+ * @dev the dev to register the handle under
+ */
+
+ldm_devfs_t register_devfs_handle(devfs_handle_t dhandle, struct device
* dev)
+{
+ struct dev_handle * entry;
+ entry = kmalloc(sizeof(*entry),GFP_KERNEL);
+ entry->handle = dhandle;
+ list_add_tail(&entry->device_list,&dev->devfs_handles);
+ return entry;
+}
+
+/**
+ * register_devfs_handle - an all in one package, this is identical to
+ * devfs_register except it includes dev
+ * @dev the dev to register the handle under
+ */
+
+ldm_devfs_t ldm_devfs_register (devfs_handle_t dir, const char *name,
+ unsigned int flags,
+ unsigned int major, unsigned int minor,
+ umode_t mode, void *ops, void *info,
+ struct device * dev)
+{
+ return register_devfs_handle(devfs_register(dir,name,flags,major,minor,
+ mode,ops,info),dev);
+}
+
+EXPORT_SYMBOL(register_devfs_handle);
+EXPORT_SYMBOL(ldm_devfs_register);
diff -ur --new-file a/drivers/base/interfaces/interface.c
b/drivers/base/interfaces/interface.c
--- a/drivers/base/interfaces/interface.c Thu Jan 1 00:00:00 1970
+++ b/drivers/base/interfaces/interface.c Sun Jul 28 19:54:24 2002
@@ -0,0 +1,137 @@
+/*
+ * drivers/base/interfaces/interface.c - common driverfs interface
that's exported to
+ * the world for all devices.
+ * Copyright (c) 2002 Patrick Mochel
+ * 2002 Open Source Development Lab
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/stat.h>
+
+static ssize_t device_read_name(struct device * dev, char * buf, size_t
count, loff_t off)
+{
+ return off ? 0 : sprintf(buf,"%s\n",dev->name);
+}
+
+static struct driver_file_entry device_name_entry = {
+ name: "name",
+ mode: S_IRUGO,
+ show: device_read_name,
+};
+
+static ssize_t
+device_read_power(struct device * dev, char * page, size_t count,
loff_t off)
+{
+ return off ? 0 : sprintf(page,"%d\n",dev->current_state);
+}
+
+static ssize_t
+device_write_power(struct device * dev, const char * buf, size_t count,
loff_t off)
+{
+ char str_command[20];
+ char str_level[20];
+ int num_args;
+ u32 state;
+ u32 int_level;
+ int error = 0;
+
+ if (off)
+ return 0;
+
+ if (!dev->driver)
+ goto done;
+
+ num_args = sscanf(buf,"%10s %10s %u",str_command,str_level,&state);
+
+ error = -EINVAL;
+
+ if (!num_args)
+ goto done;
+
+ if (!strnicmp(str_command,"suspend",7)) {
+ if (num_args != 3)
+ goto done;
+ if (!strnicmp(str_level,"notify",6))
+ int_level = SUSPEND_NOTIFY;
+ else if (!strnicmp(str_level,"save",4))
+ int_level = SUSPEND_SAVE_STATE;
+ else if (!strnicmp(str_level,"disable",7))
+ int_level = SUSPEND_DISABLE;
+ else if (!strnicmp(str_level,"powerdown",8))
+ int_level = SUSPEND_POWER_DOWN;
+ else
+ goto done;
+
+ if (dev->driver->suspend)
+ error = dev->driver->suspend(dev,state,int_level);
+ else
+ error = 0;
+ } else if (!strnicmp(str_command,"resume",6)) {
+ if (num_args != 2)
+ goto done;
+
+ if (!strnicmp(str_level,"poweron",7))
+ int_level = RESUME_POWER_ON;
+ else if (!strnicmp(str_level,"restore",7))
+ int_level = RESUME_RESTORE_STATE;
+ else if (!strnicmp(str_level,"enable",6))
+ int_level = RESUME_ENABLE;
+ else
+ goto done;
+
+ if (dev->driver->resume)
+ error = dev->driver->resume(dev,int_level);
+ else
+ error = 0;
+ }
+ done:
+ return error < 0 ? error : count;
+}
+
+static struct driver_file_entry device_power_entry = {
+ name: "power",
+ mode: S_IWUSR | S_IRUGO,
+ show: device_read_power,
+ store: device_write_power,
+};
+
+/* device_read_devfs - Adam Belay ([email protected])*/
+static ssize_t device_read_devfs(struct device * dev, char * buf,
size_t count,
+ loff_t off)
+{
+ int offset, major, minor;
+ char * str = buf;
+ char pathbuf[64];
+ struct list_head * pos;
+ struct dev_handle * devfs;
+ if (off)
+ return 0;
+ list_for_each_prev(pos, &dev->devfs_handles)
+ {
+ devfs = list_entry(pos,struct dev_handle,device_list);
+ offset = devfs_generate_path (devfs->handle, pathbuf, sizeof
pathbuf);
+ if (offset >= 0 && (0 ==
devfs_get_maj_min(devfs->handle,&major,&minor)))
+ {
+ str += sprintf(str,"%d,%d %s\n",major,minor,
+ (pathbuf+offset));
+ }
+ else
+ return 0;
+ }
+ return (str - buf);
+}
+
+
+static struct driver_file_entry device_devfs_entry = {
+ name: "dev",
+ mode: S_IRUGO,
+ show: device_read_devfs,
+};
+
+struct driver_file_entry * device_default_files[] = {
+ &device_name_entry,
+ &device_power_entry,
+ &device_devfs_entry,
+ NULL,
+};
diff -ur --new-file a/drivers/block/floppy.c b/drivers/block/floppy.c
--- a/drivers/block/floppy.c Wed Jul 24 18:37:14 2002
+++ b/drivers/block/floppy.c Sat Jul 27 20:14:27 2002
@@ -3969,6 +3969,11 @@
revalidate: floppy_revalidate,
};

+static struct device device_floppy = {
+ name: "floppy",
+ bus_id: "03?0",
+};
+
static void __init register_devfs_entries (int drive)
{
int base_minor, i;
@@ -3992,10 +3997,10 @@
char name[16];

sprintf (name, "%d%s", drive, table[table_sup[UDP->cmos][i]]);
- devfs_register (devfs_handle, name, DEVFS_FL_DEFAULT, MAJOR_NR,
+ ldm_devfs_register (devfs_handle, name, DEVFS_FL_DEFAULT, MAJOR_NR,
base_minor + (table_sup[UDP->cmos][i] << 2),
S_IFBLK | S_IRUSR | S_IWUSR | S_IRGRP |S_IWGRP,
- &floppy_fops, NULL);
+ &floppy_fops, NULL, &device_floppy);
} while (table_sup[UDP->cmos][i++]);
}
}
@@ -4219,10 +4224,6 @@

static int have_no_fdc= -ENODEV;

-static struct device device_floppy = {
- name: "floppy",
- bus_id: "03?0",
-};

int __init floppy_init(void)
{
diff -ur --new-file a/include/linux/device.h b/include/linux/device.h
--- a/include/linux/device.h Wed Jul 24 18:37:32 2002
+++ b/include/linux/device.h Sat Jul 27 20:58:25 2002
@@ -29,12 +29,12 @@
#include <linux/list.h>
#include <linux/sched.h>
#include <linux/driverfs_fs.h>
+#include <linux/devfs_fs_kernel.h>

#define DEVICE_NAME_SIZE 80
#define DEVICE_ID_SIZE 32
#define BUS_ID_SIZE 16

-
enum {
SUSPEND_NOTIFY,
SUSPEND_SAVE_STATE,
@@ -120,7 +120,7 @@
extern void put_driver(struct device_driver * drv);
extern void remove_driver(struct device_driver * drv);

-extern int driver_for_each_dev(struct device_driver * drv, void * data,
+extern int driver_for_each_dev(struct device_driver * drv, void * data,
int (*callback)(struct device * dev, void * data));


@@ -130,6 +130,7 @@
struct list_head bus_list; /* node in bus's list */
struct list_head driver_list;
struct list_head children;
+ struct list_head devfs_handles; /* stores devfs entries */
struct device * parent;

char name[DEVICE_NAME_SIZE]; /* descriptive ascii string */
@@ -159,6 +160,21 @@

void (*release)(struct device * dev);
};
+
+struct dev_handle {
+ devfs_handle_t handle;
+ struct list_head device_list; /* node in device's list */
+};
+
+typedef struct dev_handle * ldm_devfs_t;
+
+extern ldm_devfs_t register_devfs_handle(devfs_handle_t dhandle, struct
device * dev);
+extern ldm_devfs_t ldm_devfs_register (devfs_handle_t dir, const char
*name,
+ unsigned int flags,
+ unsigned int major, unsigned int minor,
+ umode_t mode, void *ops, void *info,
+ struct device * dev);
+

#define to_device(d) container_of(d, struct device, dir)




2002-07-29 17:42:31

by Adam Belay

[permalink] [raw]
Subject: Re: [PATCH] integrate driverfs and devfs (2.5.28)

Forgot to mention:
- the floppy driver was converted as an example
- adds a new file "dev" to each driverfs entry
- output is in the following format
MAJOR,MINOR PATH
- one line per each devfs entry during output
- to test try: #cat (driverfs)/root/sys/03?0/dev
assuming you have a floppy drive
- Applies cleanly to 2.5.29 as well
- If there's a demand for it I'll add the ability to unregister
individual devfs entries from driverfs
Cheers,
ambx1

[email protected] wrote:

> This patch integrates driverfs and devfs. A summary is as follows:
> - create new interface directory and move interface.c
> * I intend to add more to this directory later
> - add devfs entry list
> - add devfs related functions
> - create devfs interface
> This patch is intended to be as non intrusive as possible. Therefore
> it doesn't modify devfs directly but instead creates a layer above it.
> This is due to the fact that if devfs was modified it would break
> every driver. Eventually we have to decide when and how to
> integrate it directly, This patch will provide the necessary
> infrastructure. Please Apply.
> cheers,
> Adam Belay


2002-07-29 19:30:27

by Patrick Mochel

[permalink] [raw]
Subject: Re: [PATCH] integrate driverfs and devfs (2.5.28)


Wow. I don't know whether to laugh, or to cry.

On Sun, 28 Jul 2002, Adam Belay wrote:

> This patch integrates driverfs and devfs. A summary is as follows:
> - create new interface directory and move interface.c
> * I intend to add more to this directory later
> - add devfs entry list
> - add devfs related functions
> - create devfs interface
> This patch is intended to be as non intrusive as possible. Therefore
> it doesn't modify devfs directly but instead creates a layer above it.
> This is due to the fact that if devfs was modified it would break
> every driver. Eventually we have to decide when and how to
> integrate it directly, This patch will provide the necessary
> infrastructure. Please Apply.

Not a chance. I refuse to apply any devfs interface layers to the device
model core, or any devfs-specific members to the device model data
structures. And, that's just on principle.

As for technical reasons, why on earth would you want to do this in the
first place?

We've collectively decided that enforcing device naming policy in the
kernel is the Wrong Thing To Do. devfs does this; and your patch furthers
the pain.

Besides, look at the interace. Tell me devfs_register is not horrid. 8
parameters is rediculous, and you want to add another one? And, change
every driver?

I won't touch devfs, as it's unmaintainable. I won't even look at it,
because it's unreadable. But, if you really want to build a bridge between
the two, I would suggest looking at a way to collapse some of the data
structures and the calls. devfs_register (or the like) should take 1
structure with all the information it needs in it. Most of that data you
can derive from other places (like a higher level). In fact, IMO, it
should be the higher level that is doing the registration of the devices,
not the individual drivers themselves.

I appreciate the effort, but I don't support it. We should soon have a
working device class model, which is where you'll start to see devfs-like
concepts handled in a sane way. Please look there for ideas, rather than
devfs.

Thanks,

-pat

2002-07-29 20:41:39

by Adam Belay

[permalink] [raw]
Subject: Re: [PATCH] integrate driverfs and devfs (2.5.28)



[email protected] wrote:

>Wow. I don't know whether to laugh, or to cry.
>
Nor do I.

>
>Not a chance. I refuse to apply any devfs interface layers to the device
>model core, or any devfs-specific members to the device model data
>structures. And, that's just on principle.
>
This patch merely adds one new list to the device struct. The
information it provides is not devfs specific and it only extracts data
from the devfs structures that already exist.

>
>
>As for technical reasons, why on earth would you want to do this in the
>first place?
>
Actually doing this is extremely useful. Without such an interface it's
impossible to tell which device interface in the driver model
corresponds to which dev entry both with and without the use of devfs.
Imagine a user trying to configure a serial port through the driverfs.
Without this interface how can the user determine which serial port he
is configuring. With my patch it is possible to determine the !major!
and !minor! number of the device as well as a path to devfs in the event
that it is used. It provides information so that the user can determine
which device the driver model is talking about. With the floppy driver
that I converted it is especially nice because it lists for the user all
the many devices that correspond with that particular floppy drive.
Many user level scripts and programs "will" need this kind of
information when the driver model is complete.

>
>
>We've collectively decided that enforcing device naming policy in the
>kernel is the Wrong Thing To Do. devfs does this; and your patch furthers
>the pain.
>
My patch does not enforce any policies, it merely extracts information
from the already existing devfs. The major and minor numbers provided
are useful to even those who don't use devfs.

>
>
>Besides, look at the interace. Tell me devfs_register is not horrid. 8
>parameters is rediculous, and you want to add another one? And, change
>every driver?
>
You're already changing every driver with this new driver model. Yes, I
agree, it is horrid. Like I said earlier this is only stage one of this
integration. It currently does not break anything and merely provides
the !option! for a driver to register its devfs handle. I wanted to get
this feature in before the freeze. If you read my comments you'd
realize that my function with 9 parameters is an all in one function
that is only optional. Remove it or change it if you like.

>
>
>I won't touch devfs, as it's unmaintainable. I won't even look at it,
>because it's unreadable. But, if you really want to build a bridge between
>the two, I would suggest looking at a way to collapse some of the data
>structures and the calls. devfs_register (or the like) should take 1
>structure with all the information it needs in it. Most of that data you
>can derive from other places (like a higher level). In fact, IMO, it
>should be the higher level that is doing the registration of the devices,
>not the individual drivers themselves.
>
I agree that should definitely be done, maybe I'll even do it (I'm not
the maintainer though). What do you mean by the higher level should
register the devices?

>
>
>I appreciate the effort, but I don't support it. We should soon have a
>working device class model, which is where you'll start to see devfs-like
>concepts handled in a sane way. Please look there for ideas, rather than
>devfs.
>
Device Class Model, hmm, I can't wait to see that.
My next driver model patch will be less controversial:-;.

>
>
>Thanks,
>
> -pat
>
Please have a more open mind about this. It may need a bit of
adaptation but is still very useful. If you have any questions or
comments feel free to contact me.
Thanks,
Adam

2002-07-29 22:18:14

by Patrick Mochel

[permalink] [raw]
Subject: Re: [PATCH] integrate driverfs and devfs (2.5.28)


> >Not a chance. I refuse to apply any devfs interface layers to the device
> >model core, or any devfs-specific members to the device model data
> >structures. And, that's just on principle.
> >
> This patch merely adds one new list to the device struct. The
> information it provides is not devfs specific and it only extracts data
> from the devfs structures that already exist.

Look at what you named it:

+ struct list_head devfs_handles; /* stores devfs entries */

If that's not devfs specific, why does it carry such a name?

> >As for technical reasons, why on earth would you want to do this in the
> >first place?
> >
> Actually doing this is extremely useful. Without such an interface it's
> impossible to tell which device interface in the driver model
> corresponds to which dev entry both with and without the use of devfs.

I agree that such information can be useful, but you're going about it the
wrong way (for several reasons now that I look a little closer at the
patch).

1) devfs imposes a default naming policy. That is bad, wrong and unjust.
There shalt not be a default naming policy in the kernel. Period.

2) You're not exploiting the interface; you're abusing the infrastructure.
You want to modify every driver to call this wrapper function that resides
in the device model core, just so you can get one extra file in the
device's driverfs directory?

devfs was already implemented in the wrong way in the first place. Instead
of requiring modification to every driver, the devfs registration should
have taken place in the subsystem for which the driver belongs to. In most
cases (I won't say all), the driver already registers the devices it
attaches to with _something_. The proper thing to do is not to create a
parallel API, but one the subsystems can use. The subsystems already know
most of the information about the device, they should use it.

If you want this information exposed, the really unintrusive thing to do
would be to just call device_create_file in the drivers you want to expose
this information. Or, in the subsystems the driver registers with. Which
brings me to my next point:

3) You're munging three values in the driverfs file.

Thou shalt not have more than one value per file.

Exceptions are available, but only in extreme cases (like arrays of
information; we're still not quite sure what to do with that).

4) You're doing this:

+struct dev_handle {
+ devfs_handle_t handle;
+ struct list_head device_list; /* node in device's list */
+};
+
+typedef struct dev_handle * ldm_devfs_t;

Thou shalt not use typedef.

5) You're adding a function with _9_ parameters, two of which are void *.

6) You're adding infrastructure in the core for an interface that is
optional and rarely used.

> Imagine a user trying to configure a serial port through the driverfs.
> Without this interface how can the user determine which serial port he
> is configuring. With my patch it is possible to determine the !major!
> and !minor! number of the device as well as a path to devfs in the event
> that it is used. It provides information so that the user can determine
> which device the driver model is talking about. With the floppy driver
> that I converted it is especially nice because it lists for the user all
> the many devices that correspond with that particular floppy drive.

With device classes, you will have a directory like this:

/devices/class/floppy/

With symlinks back to the device's directory in the physical hierarchy
layout. The user can see what devices of what type they have, and have
access to their configuration items.

It is that mapping (from logical to physical) that is really useful, not
the other way around. Users probably aren't going to be poking around
in the physical layout as much as they will be in the logical layout (but
we keep all the attributes in one place with symlinks between them).

> Many user level scripts and programs "will" need this kind of
> information when the driver model is complete.

That's not an argument. You, nor I, nor anyone else can accurately say
exactly what user level scripts will need when it is done. Furthermore,
the point of the device model is not, has never been, nor ever will be, to
satisfy the mythical requirements of userspace. It is meant to consolidate
and centralize internal interfaces. The fact that they're exported to
userspace is because we can easily.

> >We've collectively decided that enforcing device naming policy in the
> >kernel is the Wrong Thing To Do. devfs does this; and your patch
> >furthers the pain.
> >
> My patch does not enforce any policies, it merely extracts information
> from the already existing devfs. The major and minor numbers provided
> are useful to even those who don't use devfs.

So why call include the devfs information at all? The SCSI people have
been doing something similar for a couple of versions now. They export a
driverfs file with the kdev_t value in it. Granted, they export it as one
value, which is bad, but it's only the kdev_t number.

If you want the path to the device node, write a script that looks it up
(in userspace).

> Please have a more open mind about this. It may need a bit of
> adaptation but is still very useful. If you have any questions or
> comments feel free to contact me.

I do have an open mind. This is not a rash decision. I began looking at
this a year ago. After many long days of meditation, chant, and heavy
drug use, I came to the conclusion that devfs does not have Buddha in it.
And, if it doesn't have Buddha, I can't hang with it. Therefore, I will
refuse any patch that does anything but remove devfs-isms.

-pat

2002-07-29 23:23:42

by Oliver Neukum

[permalink] [raw]
Subject: Re: [PATCH] integrate driverfs and devfs (2.5.28)

Am Dienstag, 30. Juli 2002 00:21 schrieb Patrick Mochel:

> 1) devfs imposes a default naming policy. That is bad, wrong and unjust.
> There shalt not be a default naming policy in the kernel. Period.

Why not? Who really needs the ability to name anything in /dev ?
You can always use a symlink if you realy, realy want.

[..]
> devfs was already implemented in the wrong way in the first place. Instead
> of requiring modification to every driver, the devfs registration should
> have taken place in the subsystem for which the driver belongs to. In most
> cases (I won't say all), the driver already registers the devices it
> attaches to with _something_. The proper thing to do is not to create a
> parallel API, but one the subsystems can use. The subsystems already know
> most of the information about the device, they should use it.

I am afraid I have to disagree violently here.
A device on this level is a logical thing. It must not matter which subsystem it
is attached to. Furthermore, the subsystem cannot know what the physical
device actually does. That's what a driver does.

<snip comments on code quality>

> With symlinks back to the device's directory in the physical hierarchy
> layout. The user can see what devices of what type they have, and have
> access to their configuration items.
>
> It is that mapping (from logical to physical) that is really useful, not
> the other way around. Users probably aren't going to be poking around
> in the physical layout as much as they will be in the logical layout (but
> we keep all the attributes in one place with symlinks between them).

The problem is that a device without a mapping to a driver is a valid
state. In fact, this is how hotplugging scripts have to work.

> So why call include the devfs information at all? The SCSI people have
> been doing something similar for a couple of versions now. They export a
> driverfs file with the kdev_t value in it. Granted, they export it as one
> value, which is bad, but it's only the kdev_t number.

They export a kdev_t ???

Regards
Oliver

2002-07-30 16:29:27

by Patrick Mochel

[permalink] [raw]
Subject: Re: [PATCH] integrate driverfs and devfs (2.5.28)


> > 1) devfs imposes a default naming policy. That is bad, wrong and unjust.
> > There shalt not be a default naming policy in the kernel. Period.
>
> Why not? Who really needs the ability to name anything in /dev ?
> You can always use a symlink if you realy, realy want.

You can use a symlink, but it was an explicit design decision to not go
that direction. If we create device nodes in the kernel, we're giving
userspace something to use, even if we really, really want them to use
symlinks. We're implementing a default naming policy. And, if it is there,
people will use it.

We don't want people to use that naming scheme. We don't want people to
rely on the naming scheme. It gives us a little more freedom and room to
move around in the future, should we ever decide that some part of it was
a bad idea. That's not to say that the interface is going to be volatile
forever. But, it will happen that some part of it will suck and need to be
redone.

But, why should the kernel even deal with it? It's a problem that is so
much better done in userspace. It's configurable, scalable, and easily
managable. Forcing userspace to have and use this freedom makes things so
much simpler for both us and them.

> [..]
> > devfs was already implemented in the wrong way in the first place. Instead
> > of requiring modification to every driver, the devfs registration should
> > have taken place in the subsystem for which the driver belongs to. In most
> > cases (I won't say all), the driver already registers the devices it
> > attaches to with _something_. The proper thing to do is not to create a
> > parallel API, but one the subsystems can use. The subsystems already know
> > most of the information about the device, they should use it.
>
> I am afraid I have to disagree violently here.
> A device on this level is a logical thing. It must not matter which subsystem it
> is attached to. Furthermore, the subsystem cannot know what the physical
> device actually does. That's what a driver does.

A little clarification about the term subsystem: I use the term to talk
about both class drivers and bus drivers. I don't have an exact definition
of the word, but I basically consider it to be a collection of programming
interfaces and components that register with it.

Ok, it's a shoddy definition. But, in the context, I was talking about the
class subsystem that the driver registers the device with. Does that help
at all?

> > With symlinks back to the device's directory in the physical hierarchy
> > layout. The user can see what devices of what type they have, and have
> > access to their configuration items.
> >
> > It is that mapping (from logical to physical) that is really useful, not
> > the other way around. Users probably aren't going to be poking around
> > in the physical layout as much as they will be in the logical layout (but
> > we keep all the attributes in one place with symlinks between them).
>
> The problem is that a device without a mapping to a driver is a valid
> state. In fact, this is how hotplugging scripts have to work.

Yes, and the idea is to pass the physical device path to /sbin/hotplug
when the device is inserted, so it can get at the device attributes.

> > So why call include the devfs information at all? The SCSI people have
> > been doing something similar for a couple of versions now. They export a
> > driverfs file with the kdev_t value in it. Granted, they export it as one
> > value, which is bad, but it's only the kdev_t number.
>
> They export a kdev_t ???

Yep.

-pat

2002-07-30 17:56:47

by Adam Belay

[permalink] [raw]
Subject: Re: [PATCH] integrate driverfs and devfs (2.5.28)

The problem is that a device without a mapping to a driver is a valid
> state.

Ok here's a new less devfs dependent idea. Its sole goal is to
acomplish the above. Why not add a list to device like I did before
only instead of using devfs handles to discover major and minor numbers
I can store the major and minor numbers directly. I could even use
kdev_t. This way it is not dependent on devfs and does not enforce any
of its policies. Then I'll create a quick interface for driverfs as
well as a simple register and unregister function. I'll name it dev
like before and it can display information in the following format:
MAJOR, MINOR. It will print one line per dev. Finally I can use one
of devfs's find functions to generate the path although I may just
forget devfs entirely. This interface is necessary because user level
programs can determine which driverfs entries correspond to which
entries in their dev directory. I think this will be useful for hotplug
as well as some of my own projects. If you support this I'll get to
work on a patch. Also I had a driver model related question. Should a
driver be able to belong to more than one bus? Also are you going to
create an interface that exports drivers directly instead of only
through bus and device class?
Thanks,
Adam

2002-08-05 12:21:33

by Rob Landley

[permalink] [raw]
Subject: Re: [PATCH] integrate driverfs and devfs (2.5.28)

On Monday 29 July 2002 07:25 pm, Oliver Neukum wrote:
> Am Dienstag, 30. Juli 2002 00:21 schrieb Patrick Mochel:
> > 1) devfs imposes a default naming policy. That is bad, wrong and unjust.
> > There shalt not be a default naming policy in the kernel. Period.
>
> Why not? Who really needs the ability to name anything in /dev ?
> You can always use a symlink if you realy, realy want.

Okay, I'll bite.

So what's root_dev_names in init/do_mounts.c? If a default naming policy is
so unacceptably evil, is that being removed in 2.5 and everybody being told
to use major/minor for the root device?

By the way, why doesn't imposing consistent predefined major/minor numbers
(0x0301 instead of "hda1") count as "policy"? I'm honestly curious...

Rob

2002-08-06 06:10:40

by Rob Landley

[permalink] [raw]
Subject: Policy vs API (was Re: [PATCH] integrate driverfs and devfs (2.5.28))

On Monday 05 August 2002 07:19 pm, Greg KH wrote:

> > By the way, why doesn't imposing consistent predefined major/minor
> > numbers (0x0301 instead of "hda1") count as "policy"? I'm honestly
> > curious...
>
> It does. I want to get rid of it too :) But that's still a ways away...

The user needs some kind of API. Some way for a program to say "Could I
please talk to the slave drive on the second IDE controller". What I'm
wondering is where's the line between "policy" and "API"? You can't have a
standard API without SOME level of "policy".

Major/minor device numbers suck because the namespace is too small (for
historical reasons), it's randomly organized (again for historical reasons),
it's almost exhausted (due to the first two), making it bigger will just help
the clutter breed, and it's almost impossible to recycle old numbers as long
as three people out there are still using the floppy tape controller or
whatever. Plus humans really do think in words (or at least text strings),
hence the phone book and DNS to access numbers via strings. (And the /dev
directory.)

There already ARE standardized text based APIs, and internationalization
people object to them for political correctness reasons and that's just
stupid. If somebody is REALLY stupid enough to translate the standard C
library function names into chinese unicode, and then try to actually link a
program against it, they deserve what they get. Same with the standard unix
commands: awk, grep, and sed aren't english. (They're posix. :)
Internationalizing the data going through an API is one thing, trying to
internationalize the API itself is just silly. Might as well encourage
non-english posts on linux-kernel...

I've noticed a bias against actually using strings to interface the kernel
with userspace (the module autoloader being one relatively obvious example),
but it's still done in a bunch of places anyway ("/sbin/init", "linuxrc",
"hotplug", "modprobe", the text command line for kernel booting with "init=",
"root=", "initrd="...). The kernel talks to other parts of the kernel by
exporting text symbols, userspace talks to userspace with strings, why is
kernel to userspace fundamentally different? The problem with the horror
that is /proc is that it's not ORGANIZED, not that it's text. People put so
much stuff into /proc because they WANT a text API, and for a long time that
was their only real outlet.

The problems with devfs are, well, numerous, but the fundamental idea of
automatically mounting a /dev directory with the available devices into it
rather than a MAKEDEV script to create 8 zillion nodes for devices you might
conceivably want to borrow from a museum someday... Implementation issues
aside, what was wrong with the idea itself? The rebellion against using the
existing names there makes devfs a lot less interesting to me, I know that
much. It doesn't provide the same API the /dev directory does, and half the
software I use won't compile against it without a chainsaw and a bullwhip, or
the daemon kludge. Code quality aside, the new devfs api never struck me as
an improvement over the old one. (I STILL don't know why a change of
implementation mandated a change of API.)

I'm under the vague impression that devicefs may somehow become the new
driver API at some point after Buck Rogers returns from his frozen orbit.
The whole POINT of devicefs seems to be to expose new data through a fresh
API that's designed to grow without getting disorganized. This implies (to
me) a move towards a device API defined in the context of a text namespace.

If there's some move underway to avoid doing so (because any standard API is
"policy"), what exactly is the better alternative?

Rob

2002-08-05 23:18:09

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] integrate driverfs and devfs (2.5.28)

On Mon, Aug 05, 2002 at 02:26:35AM -0400, Rob Landley wrote:
>
> So what's root_dev_names in init/do_mounts.c? If a default naming policy is
> so unacceptably evil, is that being removed in 2.5 and everybody being told
> to use major/minor for the root device?

Yes. Well no. We can't break backwards compatibility with userspace
like this. So things like root device naming will have to live on for a
while.

> By the way, why doesn't imposing consistent predefined major/minor numbers
> (0x0301 instead of "hda1") count as "policy"? I'm honestly curious...

It does. I want to get rid of it too :) But that's still a ways away...

thanks,

greg k-h

2002-08-06 16:33:49

by Greg KH

[permalink] [raw]
Subject: Re: Policy vs API (was Re: [PATCH] integrate driverfs and devfs (2.5.28))

On Mon, Aug 05, 2002 at 07:51:21PM -0400, Rob Landley wrote:
>
> I'm under the vague impression that devicefs may somehow become the new
> driver API at some point after Buck Rogers returns from his frozen orbit.

Sorry, but it is the new driver API right now :)
PCI is pretty much already done in the latest 2.5 kernel, and I'm trying
to finish up the USB conversion within the next few days.

Also, to address all of the other comments that I snipped out of your
email, please go read the documentation and paper at:
http://www.kernel.org/pub/linux/kernel/people/mochel/doc/
to get an idea of what is happening, and what it looks like.

You can also mount driverfs right now to see what it looks like for your
system.

If you have any further questions or comments after reading the above
documentation, and playing around with the existing driverfs filesystem,
please let us know.

thanks,

greg k-h

2002-08-06 17:38:21

by Patrick Mochel

[permalink] [raw]
Subject: Re: Policy vs API (was Re: [PATCH] integrate driverfs and devfs (2.5.28))


> The user needs some kind of API. Some way for a program to say "Could I
> please talk to the slave drive on the second IDE controller". What I'm
> wondering is where's the line between "policy" and "API"? You can't have a
> standard API without SOME level of "policy".

There is a difference between policy and mechanism. The mechanism provides
the infrastructure to develop policy.

It is definitely difficult to _not_ build in some sort of default policy,
because we want to make assumptions about what we know is true. We need
default policy, but the argument is that it should reside in the kernel.

Note that in order to boot, we need default policy in either the kernel or
initramfs. We also need compatibility for some time, so it's not like the
in-kernel policy is going away right immediately.

> I've noticed a bias against actually using strings to interface the kernel
> with userspace (the module autoloader being one relatively obvious example),
> but it's still done in a bunch of places anyway ("/sbin/init", "linuxrc",
> "hotplug", "modprobe", the text command line for kernel booting with "init=",
> "root=", "initrd="...). The kernel talks to other parts of the kernel by
> exporting text symbols, userspace talks to userspace with strings, why is
> kernel to userspace fundamentally different? The problem with the horror
> that is /proc is that it's not ORGANIZED, not that it's text. People put so
> much stuff into /proc because they WANT a text API, and for a long time that
> was their only real outlet.

ASCII is good. We want ASCII interfaces. Period. If there are any doubts
or objections, please read the archives.

> The problems with devfs are, well, numerous, but the fundamental idea of
> automatically mounting a /dev directory with the available devices into it
> rather than a MAKEDEV script to create 8 zillion nodes for devices you might
> conceivably want to borrow from a museum someday... Implementation issues
> aside, what was wrong with the idea itself?

It depends on what your idea of the idea is. The idea of exposing only the
hardware that is present is good. But, that's about all the praise about
it you'll get from me.

> I'm under the vague impression that devicefs may somehow become the new
> driver API at some point after Buck Rogers returns from his frozen orbit.
> The whole POINT of devicefs seems to be to expose new data through a fresh
> API that's designed to grow without getting disorganized. This implies (to
> me) a move towards a device API defined in the context of a text namespace.

Remember that there are two pieces to this movement: the device model and
driverfs. Everyone calls it driverfs because it's cute, catchy, and what
is actually visible to people. 'The device model' is a crappy name, but I
don't exactly have a better name for it (besides 'Rita', but that doesn't
really make much sense).

The device model is a consolidation of the device and driver
infrastructure. It's purely internal and invisible to the user (excluding
the problem of device naming and /sbin/hotplug).

driverfs is simply the window into the organization of these internal
structures, the relationships between them, and their attributes (via an
ASCII interface).

The distinction is important, and I encourage you to read the
documentation at the link Greg posted (and continue asking questions).



-pat