2005-03-10 00:55:04

by Greg KH

[permalink] [raw]
Subject: [BK PATCH] Driver core and kobject updates for 2.6.11

Hi,

Here are some driver core and kobject/kref updates for 2.6.11. They
have all been in the -mm releases for some time now. There is also a
documentation update for the schedule removal feature list.

Please pull from: bk://kernel.bkbits.net/gregkh/linux/2.6.11/driver

Individual patches will follow, sent to the linux-kernel list.

thanks,

greg k-h

Documentation/feature-removal-schedule.txt | 23 +++++-
drivers/base/bus.c | 4 -
drivers/base/class.c | 98 +++++++++++++----------------
drivers/base/class_simple.c | 21 ------
drivers/base/driver.c | 13 +--
drivers/base/map.c | 21 ++----
drivers/base/platform.c | 2
drivers/base/sys.c | 39 +++++------
drivers/block/floppy.c | 19 +----
drivers/block/genhd.c | 55 ++++++++++------
drivers/i2c/i2c-dev.c | 9 --
drivers/media/video/videodev.c | 11 ---
drivers/usb/core/file.c | 64 +++++-------------
fs/char_dev.c | 26 ++-----
include/linux/device.h | 5 -
include/linux/kobj_map.h | 2
include/linux/kobject.h | 2
include/linux/kref.h | 2
lib/kobject.c | 15 ++--
lib/kref.c | 11 ++-
20 files changed, 202 insertions(+), 240 deletions(-)
-----


Arjan van de Ven:
o Kobject: remove some unneeded exports

Dominik Brodowski:
o cpufreq 2.4 interface removal schedule

Greg Kroah-Hartman:
o class: add a semaphore to struct class, and use that instead of the subsystem rwsem
o sysdev: remove the rwsem usage from this subsystem
o sysdev: fix the name of the list of drivers to be a sane name
o kmap: remove usage of rwsem from kobj_map
o USB: move usb core to use class_simple instead of it's own class functions
o kref: make kref_put return if this was the last put call
o sysdev: make system_subsys static as no one else needs access to it
o kset: make ksets have a spinlock, and use that to lock their lists

Kay Sievers:
o floppy.c: pass physical device to device registration
o Driver core: add "bus" symlink to class/block devices
o videodev: pass dev_t to the class core
o i2c: class driver pass dev_t to the class core
o usb: class driver pass dev_t to the class core
o class_simple: pass dev_t to the class core
o block core: export MAJOR/MINOR to the hotplug env
o class core: export MAJOR/MINOR to the hotplug env

Mike Waychison:
o driver core: clean driver unload

Randy Dunlap:
o Add 2.4.x cpufreq /proc and sysctl interface removal feature-removal-schedule

Russell King:
o driver core: Separate platform device name from platform device number


2005-03-10 00:49:52

by Greg KH

[permalink] [raw]
Subject: [PATCH] sysdev: fix the name of the list of drivers to be a sane name

ChangeSet 1.2053, 2005/03/09 15:35:31-08:00, [email protected]

[PATCH] sysdev: fix the name of the list of drivers to be a sane name

Heh, "global_drivers" as a static...

Signed-off-by: Greg Kroah-Hartman <[email protected]>


drivers/base/sys.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)


diff -Nru a/drivers/base/sys.c b/drivers/base/sys.c
--- a/drivers/base/sys.c 2005-03-09 16:28:17 -08:00
+++ b/drivers/base/sys.c 2005-03-09 16:28:17 -08:00
@@ -102,7 +102,7 @@
EXPORT_SYMBOL_GPL(sysdev_class_unregister);


-static LIST_HEAD(global_drivers);
+static LIST_HEAD(sysdev_drivers);

/**
* sysdev_driver_register - Register auxillary driver
@@ -112,7 +112,7 @@
* If @cls is valid, then @drv is inserted into @cls->drivers to be
* called on each operation on devices of that class. The refcount
* of @cls is incremented.
- * Otherwise, @drv is inserted into global_drivers, and called for
+ * Otherwise, @drv is inserted into sysdev_drivers, and called for
* each device.
*/

@@ -130,7 +130,7 @@
drv->add(dev);
}
} else
- list_add_tail(&drv->entry, &global_drivers);
+ list_add_tail(&drv->entry, &sysdev_drivers);
up_write(&system_subsys.rwsem);
return 0;
}
@@ -199,7 +199,7 @@
*/

/* Notify global drivers */
- list_for_each_entry(drv, &global_drivers, entry) {
+ list_for_each_entry(drv, &sysdev_drivers, entry) {
if (drv->add)
drv->add(sysdev);
}
@@ -219,7 +219,7 @@
struct sysdev_driver * drv;

down_write(&system_subsys.rwsem);
- list_for_each_entry(drv, &global_drivers, entry) {
+ list_for_each_entry(drv, &sysdev_drivers, entry) {
if (drv->remove)
drv->remove(sysdev);
}
@@ -268,7 +268,7 @@
pr_debug(" %s\n", kobject_name(&sysdev->kobj));

/* Call global drivers first. */
- list_for_each_entry(drv, &global_drivers, entry) {
+ list_for_each_entry(drv, &sysdev_drivers, entry) {
if (drv->shutdown)
drv->shutdown(sysdev);
}
@@ -319,7 +319,7 @@
pr_debug(" %s\n", kobject_name(&sysdev->kobj));

/* Call global drivers first. */
- list_for_each_entry(drv, &global_drivers, entry) {
+ list_for_each_entry(drv, &sysdev_drivers, entry) {
if (drv->suspend)
drv->suspend(sysdev, state);
}
@@ -375,7 +375,7 @@
}

/* Call global drivers. */
- list_for_each_entry(drv, &global_drivers, entry) {
+ list_for_each_entry(drv, &sysdev_drivers, entry) {
if (drv->resume)
drv->resume(sysdev);
}

2005-03-10 00:48:40

by Greg KH

[permalink] [raw]
Subject: [PATCH] kref: make kref_put return if this was the last put call.

ChangeSet 1.2050, 2005/03/09 10:00:20-08:00, [email protected]

[PATCH] kref: make kref_put return if this was the last put call.

This is needed for the upcoming klist code from Pat.

Signed-off-by: Greg Kroah-Hartman <[email protected]>


include/linux/kref.h | 2 +-
lib/kref.c | 11 +++++++++--
2 files changed, 10 insertions(+), 3 deletions(-)


diff -Nru a/include/linux/kref.h b/include/linux/kref.h
--- a/include/linux/kref.h 2005-03-09 16:28:38 -08:00
+++ b/include/linux/kref.h 2005-03-09 16:28:38 -08:00
@@ -26,7 +26,7 @@

void kref_init(struct kref *kref);
void kref_get(struct kref *kref);
-void kref_put(struct kref *kref, void (*release) (struct kref *kref));
+int kref_put(struct kref *kref, void (*release) (struct kref *kref));

#endif /* __KERNEL__ */
#endif /* _KREF_H_ */
diff -Nru a/lib/kref.c b/lib/kref.c
--- a/lib/kref.c 2005-03-09 16:28:38 -08:00
+++ b/lib/kref.c 2005-03-09 16:28:38 -08:00
@@ -42,14 +42,21 @@
* in as this function.
*
* Decrement the refcount, and if 0, call release().
+ * Return 1 if the object was removed, otherwise return 0. Beware, if this
+ * function returns 0, you still can not count on the kref from remaining in
+ * memory. Only use the return value if you want to see if the kref is now
+ * gone, not present.
*/
-void kref_put(struct kref *kref, void (*release) (struct kref *kref))
+int kref_put(struct kref *kref, void (*release)(struct kref *kref))
{
WARN_ON(release == NULL);
WARN_ON(release == (void (*)(struct kref *))kfree);

- if (atomic_dec_and_test(&kref->refcount))
+ if (atomic_dec_and_test(&kref->refcount)) {
release(kref);
+ return 1;
+ }
+ return 0;
}

EXPORT_SYMBOL(kref_init);

2005-03-10 00:55:08

by Greg KH

[permalink] [raw]
Subject: [PATCH] usb: class driver pass dev_t to the class core

ChangeSet 1.2042, 2005/03/09 09:51:30-08:00, [email protected]

[PATCH] usb: class driver pass dev_t to the class core

Signed-off-by: Kay Sievers <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>


drivers/usb/core/file.c | 9 +--------
1 files changed, 1 insertion(+), 8 deletions(-)


diff -Nru a/drivers/usb/core/file.c b/drivers/usb/core/file.c
--- a/drivers/usb/core/file.c 2005-03-09 16:29:34 -08:00
+++ b/drivers/usb/core/file.c 2005-03-09 16:29:34 -08:00
@@ -107,13 +107,6 @@
unregister_chrdev(USB_MAJOR, "usb");
}

-static ssize_t show_dev(struct class_device *class_dev, char *buf)
-{
- int minor = (int)(long)class_get_devdata(class_dev);
- return print_dev_t(buf, MKDEV(USB_MAJOR, minor));
-}
-static CLASS_DEVICE_ATTR(dev, S_IRUGO, show_dev, NULL);
-
/**
* usb_register_dev - register a USB device, and ask for a minor number
* @intf: pointer to the usb_interface that is being registered
@@ -184,6 +177,7 @@
class_dev = kmalloc(sizeof(*class_dev), GFP_KERNEL);
if (class_dev) {
memset(class_dev, 0x00, sizeof(struct class_device));
+ class_dev->devt = MKDEV(USB_MAJOR, minor);
class_dev->class = &usb_class;
class_dev->dev = &intf->dev;

@@ -195,7 +189,6 @@
snprintf(class_dev->class_id, BUS_ID_SIZE, "%s", temp);
class_set_devdata(class_dev, (void *)(long)intf->minor);
class_device_register(class_dev);
- class_device_create_file(class_dev, &class_device_attr_dev);
intf->class_dev = class_dev;
}
exit:

2005-03-10 00:55:05

by Greg KH

[permalink] [raw]
Subject: [PATCH] kmap: remove usage of rwsem from kobj_map.

ChangeSet 1.2052, 2005/03/09 15:06:02-08:00, [email protected]

[PATCH] kmap: remove usage of rwsem from kobj_map.

This forces the caller to provide the lock, but as they all already had one, it's not a big change.
It also removes the now-unneeded cdev_subsys. Thanks to Jon Corbet for reminding me about that.

Signed-off-by: Greg Kroah-Hartman <[email protected]>


drivers/base/map.c | 21 ++++++++++-----------
drivers/block/genhd.c | 2 +-
fs/char_dev.c | 26 +++++++++-----------------
include/linux/kobj_map.h | 2 +-
4 files changed, 21 insertions(+), 30 deletions(-)


diff -Nru a/drivers/base/map.c b/drivers/base/map.c
--- a/drivers/base/map.c 2005-03-09 16:28:24 -08:00
+++ b/drivers/base/map.c 2005-03-09 16:28:24 -08:00
@@ -25,7 +25,7 @@
int (*lock)(dev_t, void *);
void *data;
} *probes[255];
- struct rw_semaphore *sem;
+ struct semaphore *sem;
};

int kobj_map(struct kobj_map *domain, dev_t dev, unsigned long range,
@@ -53,7 +53,7 @@
p->range = range;
p->data = data;
}
- down_write(domain->sem);
+ down(domain->sem);
for (i = 0, p -= n; i < n; i++, p++, index++) {
struct probe **s = &domain->probes[index % 255];
while (*s && (*s)->range < range)
@@ -61,7 +61,7 @@
p->next = *s;
*s = p;
}
- up_write(domain->sem);
+ up(domain->sem);
return 0;
}

@@ -75,7 +75,7 @@
if (n > 255)
n = 255;

- down_write(domain->sem);
+ down(domain->sem);
for (i = 0; i < n; i++, index++) {
struct probe **s;
for (s = &domain->probes[index % 255]; *s; s = &(*s)->next) {
@@ -88,7 +88,7 @@
}
}
}
- up_write(domain->sem);
+ up(domain->sem);
kfree(found);
}

@@ -99,7 +99,7 @@
unsigned long best = ~0UL;

retry:
- down_read(domain->sem);
+ down(domain->sem);
for (p = domain->probes[MAJOR(dev) % 255]; p; p = p->next) {
struct kobject *(*probe)(dev_t, int *, void *);
struct module *owner;
@@ -120,7 +120,7 @@
module_put(owner);
continue;
}
- up_read(domain->sem);
+ up(domain->sem);
kobj = probe(dev, index, data);
/* Currently ->owner protects _only_ ->probe() itself. */
module_put(owner);
@@ -128,12 +128,11 @@
return kobj;
goto retry;
}
- up_read(domain->sem);
+ up(domain->sem);
return NULL;
}

-struct kobj_map *kobj_map_init(kobj_probe_t *base_probe,
- struct subsystem *s)
+struct kobj_map *kobj_map_init(kobj_probe_t *base_probe, struct semaphore *sem)
{
struct kobj_map *p = kmalloc(sizeof(struct kobj_map), GFP_KERNEL);
struct probe *base = kmalloc(sizeof(struct probe), GFP_KERNEL);
@@ -151,6 +150,6 @@
base->get = base_probe;
for (i = 0; i < 255; i++)
p->probes[i] = base;
- p->sem = &s->rwsem;
+ p->sem = sem;
return p;
}
diff -Nru a/drivers/block/genhd.c b/drivers/block/genhd.c
--- a/drivers/block/genhd.c 2005-03-09 16:28:24 -08:00
+++ b/drivers/block/genhd.c 2005-03-09 16:28:24 -08:00
@@ -302,7 +302,7 @@

static int __init genhd_device_init(void)
{
- bdev_map = kobj_map_init(base_probe, &block_subsys);
+ bdev_map = kobj_map_init(base_probe, &block_subsys_sem);
blk_dev_init();
subsystem_register(&block_subsys);
return 0;
diff -Nru a/fs/char_dev.c b/fs/char_dev.c
--- a/fs/char_dev.c 2005-03-09 16:28:24 -08:00
+++ b/fs/char_dev.c 2005-03-09 16:28:24 -08:00
@@ -29,7 +29,7 @@
/* degrade to linked list for small systems */
#define MAX_PROBE_HASH (CONFIG_BASE_SMALL ? 1 : 255)

-static DEFINE_RWLOCK(chrdevs_lock);
+static DECLARE_MUTEX(chrdevs_lock);

static struct char_device_struct {
struct char_device_struct *next;
@@ -55,13 +55,13 @@

len = sprintf(page, "Character devices:\n");

- read_lock(&chrdevs_lock);
+ down(&chrdevs_lock);
for (i = 0; i < ARRAY_SIZE(chrdevs) ; i++) {
for (cd = chrdevs[i]; cd; cd = cd->next)
len += sprintf(page+len, "%3d %s\n",
cd->major, cd->name);
}
- read_unlock(&chrdevs_lock);
+ up(&chrdevs_lock);

return len;
}
@@ -91,7 +91,7 @@

memset(cd, 0, sizeof(struct char_device_struct));

- write_lock_irq(&chrdevs_lock);
+ down(&chrdevs_lock);

/* temporary */
if (major == 0) {
@@ -126,10 +126,10 @@
}
cd->next = *cp;
*cp = cd;
- write_unlock_irq(&chrdevs_lock);
+ up(&chrdevs_lock);
return cd;
out:
- write_unlock_irq(&chrdevs_lock);
+ up(&chrdevs_lock);
kfree(cd);
return ERR_PTR(ret);
}
@@ -140,7 +140,7 @@
struct char_device_struct *cd = NULL, **cp;
int i = major_to_index(major);

- write_lock_irq(&chrdevs_lock);
+ up(&chrdevs_lock);
for (cp = &chrdevs[i]; *cp; cp = &(*cp)->next)
if ((*cp)->major == major &&
(*cp)->baseminor == baseminor &&
@@ -150,7 +150,7 @@
cd = *cp;
*cp = cd->next;
}
- write_unlock_irq(&chrdevs_lock);
+ up(&chrdevs_lock);
return cd;
}

@@ -381,8 +381,6 @@
}


-static decl_subsys(cdev, NULL, NULL);
-
static void cdev_default_release(struct kobject *kobj)
{
struct cdev *p = container_of(kobj, struct cdev, kobj);
@@ -435,13 +433,7 @@

void __init chrdev_init(void)
{
-/*
- * Keep cdev_subsys around because (and only because) the kobj_map code
- * depends on the rwsem it contains. We don't make it public in sysfs,
- * however.
- */
- subsystem_init(&cdev_subsys);
- cdev_map = kobj_map_init(base_probe, &cdev_subsys);
+ cdev_map = kobj_map_init(base_probe, &chrdevs_lock);
}


diff -Nru a/include/linux/kobj_map.h b/include/linux/kobj_map.h
--- a/include/linux/kobj_map.h 2005-03-09 16:28:24 -08:00
+++ b/include/linux/kobj_map.h 2005-03-09 16:28:24 -08:00
@@ -7,6 +7,6 @@
kobj_probe_t *, int (*)(dev_t, void *), void *);
void kobj_unmap(struct kobj_map *, dev_t, unsigned long);
struct kobject *kobj_lookup(struct kobj_map *, dev_t, int *);
-struct kobj_map *kobj_map_init(kobj_probe_t *, struct subsystem *);
+struct kobj_map *kobj_map_init(kobj_probe_t *, struct semaphore *);

#endif

2005-03-10 01:10:48

by Greg KH

[permalink] [raw]
Subject: [PATCH] driver core: clean driver unload

ChangeSet 1.2045, 2005/03/09 09:52:29-08:00, [email protected]

[PATCH] driver core: clean driver unload

Get rid of semaphore abuse by converting device_driver->unload_sem
semaphore to device_driver->unloaded completion.

This should get rid of any confusion as well as save a few bytes in the
process.

Signed-off-by: Mike Waychison <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>


drivers/base/bus.c | 2 +-
drivers/base/driver.c | 13 ++++++-------
include/linux/device.h | 2 +-
3 files changed, 8 insertions(+), 9 deletions(-)


diff -Nru a/drivers/base/bus.c b/drivers/base/bus.c
--- a/drivers/base/bus.c 2005-03-09 16:29:14 -08:00
+++ b/drivers/base/bus.c 2005-03-09 16:29:14 -08:00
@@ -65,7 +65,7 @@
static void driver_release(struct kobject * kobj)
{
struct device_driver * drv = to_driver(kobj);
- up(&drv->unload_sem);
+ complete(&drv->unloaded);
}

static struct kobj_type ktype_driver = {
diff -Nru a/drivers/base/driver.c b/drivers/base/driver.c
--- a/drivers/base/driver.c 2005-03-09 16:29:14 -08:00
+++ b/drivers/base/driver.c 2005-03-09 16:29:14 -08:00
@@ -79,14 +79,14 @@
* since most of the things we have to do deal with the bus
* structures.
*
- * The one interesting aspect is that we initialize @drv->unload_sem
- * to a locked state here. It will be unlocked when the driver
- * reference count reaches 0.
+ * The one interesting aspect is that we setup @drv->unloaded
+ * as a completion that gets complete when the driver reference
+ * count reaches 0.
*/
int driver_register(struct device_driver * drv)
{
INIT_LIST_HEAD(&drv->devices);
- init_MUTEX_LOCKED(&drv->unload_sem);
+ init_completion(&drv->unloaded);
return bus_add_driver(drv);
}

@@ -97,7 +97,7 @@
*
* Again, we pass off most of the work to the bus-level call.
*
- * Though, once that is done, we attempt to take @drv->unload_sem.
+ * Though, once that is done, we wait until @drv->unloaded is completed.
* This will block until the driver refcount reaches 0, and it is
* released. Only modular drivers will call this function, and we
* have to guarantee that it won't complete, letting the driver
@@ -107,8 +107,7 @@
void driver_unregister(struct device_driver * drv)
{
bus_remove_driver(drv);
- down(&drv->unload_sem);
- up(&drv->unload_sem);
+ wait_for_completion(&drv->unloaded);
}

/**
diff -Nru a/include/linux/device.h b/include/linux/device.h
--- a/include/linux/device.h 2005-03-09 16:29:14 -08:00
+++ b/include/linux/device.h 2005-03-09 16:29:14 -08:00
@@ -102,7 +102,7 @@
char * name;
struct bus_type * bus;

- struct semaphore unload_sem;
+ struct completion unloaded;
struct kobject kobj;
struct list_head devices;


2005-03-10 01:16:00

by Greg KH

[permalink] [raw]
Subject: [PATCH] kset: make ksets have a spinlock, and use that to lock their lists

ChangeSet 1.2048, 2005/03/09 09:53:28-08:00, [email protected]

[PATCH] kset: make ksets have a spinlock, and use that to lock their lists

Do this instead of using the rwsem of a subsys.
Smaller, faster, and I'm trying to get rid of the rwsem in the subsys.

Signed-off-by: Greg Kroah-Hartman <[email protected]>


include/linux/kobject.h | 2 ++
lib/kobject.c | 13 +++++++------
2 files changed, 9 insertions(+), 6 deletions(-)


diff -Nru a/include/linux/kobject.h b/include/linux/kobject.h
--- a/include/linux/kobject.h 2005-03-09 16:28:52 -08:00
+++ b/include/linux/kobject.h 2005-03-09 16:28:52 -08:00
@@ -20,6 +20,7 @@
#include <linux/types.h>
#include <linux/list.h>
#include <linux/sysfs.h>
+#include <linux/spinlock.h>
#include <linux/rwsem.h>
#include <linux/kref.h>
#include <linux/kobject_uevent.h>
@@ -102,6 +103,7 @@
struct subsystem * subsys;
struct kobj_type * ktype;
struct list_head list;
+ spinlock_t list_lock;
struct kobject kobj;
struct kset_hotplug_ops * hotplug_ops;
};
diff -Nru a/lib/kobject.c b/lib/kobject.c
--- a/lib/kobject.c 2005-03-09 16:28:52 -08:00
+++ b/lib/kobject.c 2005-03-09 16:28:52 -08:00
@@ -140,9 +140,9 @@
static void unlink(struct kobject * kobj)
{
if (kobj->kset) {
- down_write(&kobj->kset->subsys->rwsem);
+ spin_lock(&kobj->kset->list_lock);
list_del_init(&kobj->entry);
- up_write(&kobj->kset->subsys->rwsem);
+ spin_unlock(&kobj->kset->list_lock);
}
kobject_put(kobj);
}
@@ -168,13 +168,13 @@
kobj->kset ? kobj->kset->kobj.name : "<NULL>" );

if (kobj->kset) {
- down_write(&kobj->kset->subsys->rwsem);
+ spin_lock(&kobj->kset->list_lock);

if (!parent)
parent = kobject_get(&kobj->kset->kobj);

list_add_tail(&kobj->entry,&kobj->kset->list);
- up_write(&kobj->kset->subsys->rwsem);
+ spin_unlock(&kobj->kset->list_lock);
}
kobj->parent = parent;

@@ -380,6 +380,7 @@
{
kobject_init(&k->kobj);
INIT_LIST_HEAD(&k->list);
+ spin_lock_init(&k->list_lock);
}


@@ -444,7 +445,7 @@
struct list_head * entry;
struct kobject * ret = NULL;

- down_read(&kset->subsys->rwsem);
+ spin_lock(&kset->list_lock);
list_for_each(entry,&kset->list) {
struct kobject * k = to_kobj(entry);
if (kobject_name(k) && !strcmp(kobject_name(k),name)) {
@@ -452,7 +453,7 @@
break;
}
}
- up_read(&kset->subsys->rwsem);
+ spin_unlock(&kset->list_lock);
return ret;
}


2005-03-10 01:19:17

by Greg KH

[permalink] [raw]
Subject: [PATCH] class_simple: pass dev_t to the class core

ChangeSet 1.2041, 2005/03/09 09:33:17-08:00, [email protected]

[PATCH] class_simple: pass dev_t to the class core

Signed-off-by: Kay Sievers <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>


drivers/base/class_simple.c | 21 ++-------------------
1 files changed, 2 insertions(+), 19 deletions(-)


diff -Nru a/drivers/base/class_simple.c b/drivers/base/class_simple.c
--- a/drivers/base/class_simple.c 2005-03-09 16:29:41 -08:00
+++ b/drivers/base/class_simple.c 2005-03-09 16:29:41 -08:00
@@ -10,18 +10,15 @@

#include <linux/config.h>
#include <linux/device.h>
-#include <linux/kdev_t.h>
#include <linux/err.h>

struct class_simple {
- struct class_device_attribute attr;
struct class class;
};
#define to_class_simple(d) container_of(d, struct class_simple, class)

struct simple_dev {
struct list_head node;
- dev_t dev;
struct class_device class_dev;
};
#define to_simple_dev(d) container_of(d, struct simple_dev, class_dev)
@@ -35,12 +32,6 @@
kfree(s_dev);
}

-static ssize_t show_dev(struct class_device *class_dev, char *buf)
-{
- struct simple_dev *s_dev = to_simple_dev(class_dev);
- return print_dev_t(buf, s_dev->dev);
-}
-
static void class_simple_release(struct class *class)
{
struct class_simple *cs = to_class_simple(class);
@@ -75,12 +66,6 @@
cs->class.class_release = class_simple_release;
cs->class.release = release_simple_dev;

- cs->attr.attr.name = "dev";
- cs->attr.attr.mode = S_IRUGO;
- cs->attr.attr.owner = owner;
- cs->attr.show = show_dev;
- cs->attr.store = NULL;
-
retval = class_register(&cs->class);
if (retval)
goto error;
@@ -143,7 +128,7 @@
}
memset(s_dev, 0x00, sizeof(*s_dev));

- s_dev->dev = dev;
+ s_dev->class_dev.devt = dev;
s_dev->class_dev.dev = device;
s_dev->class_dev.class = &cs->class;

@@ -154,8 +139,6 @@
if (retval)
goto error;

- class_device_create_file(&s_dev->class_dev, &cs->attr);
-
spin_lock(&simple_dev_list_lock);
list_add(&s_dev->node, &simple_dev_list);
spin_unlock(&simple_dev_list_lock);
@@ -200,7 +183,7 @@

spin_lock(&simple_dev_list_lock);
list_for_each_entry(s_dev, &simple_dev_list, node) {
- if (s_dev->dev == dev) {
+ if (s_dev->class_dev.devt == dev) {
found = 1;
break;
}

2005-03-10 01:34:31

by Greg KH

[permalink] [raw]
Subject: [PATCH] Add 2.4.x cpufreq /proc and sysctl interface removal feature-removal-schedule

ChangeSet 1.2036, 2005/03/09 09:31:40-08:00, [email protected]

[PATCH] Add 2.4.x cpufreq /proc and sysctl interface removal feature-removal-schedule

Add 2.4.x cpufreq /proc and sysctl interface removal
to the feature-removal-schedule.

Signed-off-by: Randy Dunlap <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>


Documentation/feature-removal-schedule.txt | 9 +++++++++
1 files changed, 9 insertions(+)


diff -Nru a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
--- a/Documentation/feature-removal-schedule.txt 2005-03-09 16:30:16 -08:00
+++ b/Documentation/feature-removal-schedule.txt 2005-03-09 16:30:16 -08:00
@@ -15,3 +15,12 @@
against the LSB, and can be replaced by using udev.
Who: Greg Kroah-Hartman <[email protected]>

+---------------------------
+
+What: /proc/sys/cpu and the sysctl interface to cpufreq (2.4.x interfaces)
+When: January 2005
+Files: drivers/cpufreq/: cpufreq_userspace.c, proc_intf.c
+ function calls throughout the kernel tree
+Why: Deprecated, has been replaced/superseded by (what?)....
+Who: Dominik Brodowski <[email protected]>
+

2005-03-10 01:55:10

by Greg KH

[permalink] [raw]
Subject: [PATCH] i2c: class driver pass dev_t to the class core

ChangeSet 1.2043, 2005/03/09 09:51:50-08:00, [email protected]

[PATCH] i2c: class driver pass dev_t to the class core

Signed-off-by: Kay Sievers <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>


drivers/i2c/i2c-dev.c | 9 +--------
1 files changed, 1 insertion(+), 8 deletions(-)


diff -Nru a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
--- a/drivers/i2c/i2c-dev.c 2005-03-09 16:29:27 -08:00
+++ b/drivers/i2c/i2c-dev.c 2005-03-09 16:29:27 -08:00
@@ -108,13 +108,6 @@
spin_unlock(&i2c_dev_array_lock);
}

-static ssize_t show_dev(struct class_device *class_dev, char *buf)
-{
- struct i2c_dev *i2c_dev = to_i2c_dev(class_dev);
- return print_dev_t(buf, MKDEV(I2C_MAJOR, i2c_dev->minor));
-}
-static CLASS_DEVICE_ATTR(dev, S_IRUGO, show_dev, NULL);
-
static ssize_t show_adapter_name(struct class_device *class_dev, char *buf)
{
struct i2c_dev *i2c_dev = to_i2c_dev(class_dev);
@@ -451,11 +444,11 @@
else
i2c_dev->class_dev.dev = adap->dev.parent;
i2c_dev->class_dev.class = &i2c_dev_class;
+ i2c_dev->class_dev.devt = MKDEV(I2C_MAJOR, i2c_dev->minor);
snprintf(i2c_dev->class_dev.class_id, BUS_ID_SIZE, "i2c-%d", i2c_dev->minor);
retval = class_device_register(&i2c_dev->class_dev);
if (retval)
goto error;
- class_device_create_file(&i2c_dev->class_dev, &class_device_attr_dev);
class_device_create_file(&i2c_dev->class_dev, &class_device_attr_name);
return 0;
error:

2005-03-10 02:27:07

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH] Add 2.4.x cpufreq /proc and sysctl interface removal feature-removal-schedule

On Wed, Mar 09, 2005 at 04:34:38PM -0800, Greg KH wrote:
> ChangeSet 1.2036, 2005/03/09 09:31:40-08:00, [email protected]
>
> [PATCH] Add 2.4.x cpufreq /proc and sysctl interface removal feature-removal-schedule
>
> Add 2.4.x cpufreq /proc and sysctl interface removal
> to the feature-removal-schedule.
>
> Signed-off-by: Randy Dunlap <[email protected]>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>
>
> Documentation/feature-removal-schedule.txt | 9 +++++++++
> 1 files changed, 9 insertions(+)
>
>
> diff -Nru a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
> --- a/Documentation/feature-removal-schedule.txt 2005-03-09 16:30:16 -08:00
> +++ b/Documentation/feature-removal-schedule.txt 2005-03-09 16:30:16 -08:00
> @@ -15,3 +15,12 @@
> against the LSB, and can be replaced by using udev.
> Who: Greg Kroah-Hartman <[email protected]>
>
> +---------------------------
> +
> +What: /proc/sys/cpu and the sysctl interface to cpufreq (2.4.x interfaces)
> +When: January 2005

You're about 2 months too late 8-)

Dave

2005-03-10 01:15:43

by Greg KH

[permalink] [raw]
Subject: [PATCH] Kobject: remove some unneeded exports

ChangeSet 1.2035, 2005/03/09 09:31:21-08:00, [email protected]

[PATCH] Kobject: remove some unneeded exports

kobject_get_path and kobject_rename are only used by the sysfs core code
and not aren't really driver-ish code. Remove the unused exports

Signed-off-by: Arjan van de Ven <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>


lib/kobject.c | 2 --
1 files changed, 2 deletions(-)


diff -Nru a/lib/kobject.c b/lib/kobject.c
--- a/lib/kobject.c 2005-03-09 16:30:23 -08:00
+++ b/lib/kobject.c 2005-03-09 16:30:23 -08:00
@@ -524,7 +524,6 @@
}
}

-EXPORT_SYMBOL(kobject_get_path);
EXPORT_SYMBOL(kobject_init);
EXPORT_SYMBOL(kobject_register);
EXPORT_SYMBOL(kobject_unregister);
@@ -532,7 +531,6 @@
EXPORT_SYMBOL(kobject_put);
EXPORT_SYMBOL(kobject_add);
EXPORT_SYMBOL(kobject_del);
-EXPORT_SYMBOL(kobject_rename);

EXPORT_SYMBOL(kset_register);
EXPORT_SYMBOL(kset_unregister);

2005-03-10 01:10:47

by Greg KH

[permalink] [raw]
Subject: [PATCH] driver core: Separate platform device name from platform device number

ChangeSet 1.2038, 2005/03/09 09:32:19-08:00, [email protected]

[PATCH] driver core: Separate platform device name from platform device number

Separate platform device name from platform device number such that
names ending with numbers aren't confusing.

Signed-off-by: Russell King <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>


drivers/base/platform.c | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)


diff -Nru a/drivers/base/platform.c b/drivers/base/platform.c
--- a/drivers/base/platform.c 2005-03-09 16:30:02 -08:00
+++ b/drivers/base/platform.c 2005-03-09 16:30:02 -08:00
@@ -131,7 +131,7 @@
pdev->dev.bus = &platform_bus_type;

if (pdev->id != -1)
- snprintf(pdev->dev.bus_id, BUS_ID_SIZE, "%s%u", pdev->name, pdev->id);
+ snprintf(pdev->dev.bus_id, BUS_ID_SIZE, "%s.%u", pdev->name, pdev->id);
else
strlcpy(pdev->dev.bus_id, pdev->name, BUS_ID_SIZE);


2005-03-10 02:40:33

by Greg KH

[permalink] [raw]
Subject: [PATCH] Driver core: add "bus" symlink to class/block devices

ChangeSet 1.2046, 2005/03/09 09:52:48-08:00, [email protected]

[PATCH] Driver core: add "bus" symlink to class/block devices

On Tue, Feb 15, 2005 at 09:53:44PM +0100, Kay Sievers wrote:
> Add a "bus" symlink to the class and block devices, just like the "driver"
> and "device" links. This may be a huge speed gain for e.g. udev to determine
> the bus value of a device, as we currently need to do a brute-force scan in
> /sys/bus/* to find this value.

Hmm, while playing around with it, I think we should create the "bus"
link on the physical device on not on the class device.

Also the current "driver" link at the class device should be removed,
cause class devices don't have a driver. Block devices never had this
misleading symlink.

From the class device we point with the "device" link to the physical
device, and only the physical device should have the "driver" and the
"bus" link, as it represents the real relationship.

Signed-off-by: Kay Sievers <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>


drivers/base/bus.c | 2 ++
drivers/base/class.c | 36 +++++-------------------------------
2 files changed, 7 insertions(+), 31 deletions(-)


diff -Nru a/drivers/base/bus.c b/drivers/base/bus.c
--- a/drivers/base/bus.c 2005-03-09 16:29:06 -08:00
+++ b/drivers/base/bus.c 2005-03-09 16:29:06 -08:00
@@ -465,6 +465,7 @@
up_write(&dev->bus->subsys.rwsem);
device_add_attrs(bus, dev);
sysfs_create_link(&bus->devices.kobj, &dev->kobj, dev->bus_id);
+ sysfs_create_link(&dev->kobj, &dev->bus->subsys.kset.kobj, "bus");
}
return error;
}
@@ -481,6 +482,7 @@
void bus_remove_device(struct device * dev)
{
if (dev->bus) {
+ sysfs_remove_link(&dev->kobj, "bus");
sysfs_remove_link(&dev->bus->devices.kobj, dev->bus_id);
device_remove_attrs(dev->bus, dev);
down_write(&dev->bus->subsys.rwsem);
diff -Nru a/drivers/base/class.c b/drivers/base/class.c
--- a/drivers/base/class.c 2005-03-09 16:29:06 -08:00
+++ b/drivers/base/class.c 2005-03-09 16:29:06 -08:00
@@ -196,33 +196,6 @@
sysfs_remove_bin_file(&class_dev->kobj, attr);
}

-static int class_device_dev_link(struct class_device * class_dev)
-{
- if (class_dev->dev)
- return sysfs_create_link(&class_dev->kobj,
- &class_dev->dev->kobj, "device");
- return 0;
-}
-
-static void class_device_dev_unlink(struct class_device * class_dev)
-{
- sysfs_remove_link(&class_dev->kobj, "device");
-}
-
-static int class_device_driver_link(struct class_device * class_dev)
-{
- if ((class_dev->dev) && (class_dev->dev->driver))
- return sysfs_create_link(&class_dev->kobj,
- &class_dev->dev->driver->kobj, "driver");
- return 0;
-}
-
-static void class_device_driver_unlink(struct class_device * class_dev)
-{
- sysfs_remove_link(&class_dev->kobj, "driver");
-}
-
-
static ssize_t
class_device_attr_show(struct kobject * kobj, struct attribute * attr,
char * buf)
@@ -452,8 +425,9 @@
class_device_create_file(class_dev, &class_device_attr_dev);

class_device_add_attrs(class_dev);
- class_device_dev_link(class_dev);
- class_device_driver_link(class_dev);
+ if (class_dev->dev)
+ sysfs_create_link(&class_dev->kobj,
+ &class_dev->dev->kobj, "device");

register_done:
if (error && parent)
@@ -482,8 +456,8 @@
up_write(&parent->subsys.rwsem);
}

- class_device_dev_unlink(class_dev);
- class_device_driver_unlink(class_dev);
+ if (class_dev->dev)
+ sysfs_remove_link(&class_dev->kobj, "device");
class_device_remove_attrs(class_dev);

kobject_del(&class_dev->kobj);

2005-03-10 02:40:32

by Greg KH

[permalink] [raw]
Subject: [PATCH] class core: export MAJOR/MINOR to the hotplug env

ChangeSet 1.2039, 2005/03/09 09:32:38-08:00, [email protected]

[PATCH] class core: export MAJOR/MINOR to the hotplug env

Move the creation of the sysfs "dev" file of a class device into the
driver core. The struct class_device contains a dev_t value now. If set,
the driver core will create the "dev" file containing the major/minor
numbers automatically.

Signed-off-by: Kay Sievers <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>


drivers/base/class.c | 39 +++++++++++++++++++++++++++++----------
include/linux/device.h | 1 +
2 files changed, 30 insertions(+), 10 deletions(-)


diff -Nru a/drivers/base/class.c b/drivers/base/class.c
--- a/drivers/base/class.c 2005-03-09 16:29:55 -08:00
+++ b/drivers/base/class.c 2005-03-09 16:29:55 -08:00
@@ -15,6 +15,7 @@
#include <linux/module.h>
#include <linux/init.h>
#include <linux/string.h>
+#include <linux/kdev_t.h>
#include "base.h"

#define to_class_attr(_attr) container_of(_attr, struct class_attribute, attr)
@@ -298,9 +299,9 @@
int num_envp, char *buffer, int buffer_size)
{
struct class_device *class_dev = to_class_dev(kobj);
- int retval = 0;
int i = 0;
int length = 0;
+ int retval = 0;

pr_debug("%s - name = %s\n", __FUNCTION__, class_dev->class_id);

@@ -313,26 +314,34 @@
&length, "PHYSDEVPATH=%s", path);
kfree(path);

- /* add bus name of physical device */
if (dev->bus)
add_hotplug_env_var(envp, num_envp, &i,
buffer, buffer_size, &length,
"PHYSDEVBUS=%s", dev->bus->name);

- /* add driver name of physical device */
if (dev->driver)
add_hotplug_env_var(envp, num_envp, &i,
buffer, buffer_size, &length,
"PHYSDEVDRIVER=%s", dev->driver->name);
-
- /* terminate, set to next free slot, shrink available space */
- envp[i] = NULL;
- envp = &envp[i];
- num_envp -= i;
- buffer = &buffer[length];
- buffer_size -= length;
}

+ if (MAJOR(class_dev->devt)) {
+ add_hotplug_env_var(envp, num_envp, &i,
+ buffer, buffer_size, &length,
+ "MAJOR=%u", MAJOR(class_dev->devt));
+
+ add_hotplug_env_var(envp, num_envp, &i,
+ buffer, buffer_size, &length,
+ "MINOR=%u", MINOR(class_dev->devt));
+ }
+
+ /* terminate, set to next free slot, shrink available space */
+ envp[i] = NULL;
+ envp = &envp[i];
+ num_envp -= i;
+ buffer = &buffer[length];
+ buffer_size -= length;
+
if (class_dev->class->hotplug) {
/* have the bus specific function add its stuff */
retval = class_dev->class->hotplug (class_dev, envp, num_envp,
@@ -388,6 +397,12 @@
}
}

+static ssize_t show_dev(struct class_device *class_dev, char *buf)
+{
+ return print_dev_t(buf, class_dev->devt);
+}
+static CLASS_DEVICE_ATTR(dev, S_IRUGO, show_dev, NULL);
+
void class_device_initialize(struct class_device *class_dev)
{
kobj_set_kset_s(class_dev, class_obj_subsys);
@@ -432,6 +447,10 @@
class_intf->add(class_dev);
up_write(&parent->subsys.rwsem);
}
+
+ if (MAJOR(class_dev->devt))
+ class_device_create_file(class_dev, &class_device_attr_dev);
+
class_device_add_attrs(class_dev);
class_device_dev_link(class_dev);
class_device_driver_link(class_dev);
diff -Nru a/include/linux/device.h b/include/linux/device.h
--- a/include/linux/device.h 2005-03-09 16:29:55 -08:00
+++ b/include/linux/device.h 2005-03-09 16:29:55 -08:00
@@ -184,6 +184,7 @@

struct kobject kobj;
struct class * class; /* required */
+ dev_t devt; /* dev_t, creates the sysfs "dev" */
struct device * dev; /* not necessary, but nice to have */
void * class_data; /* class-specific data */


2005-03-10 02:40:31

by Greg KH

[permalink] [raw]
Subject: [PATCH] sysdev: make system_subsys static as no one else needs access to it.

ChangeSet 1.2049, 2005/03/09 09:59:49-08:00, [email protected]

[PATCH] sysdev: make system_subsys static as no one else needs access to it.

Signed-off-by: Greg Kroah-Hartman <[email protected]>


drivers/base/sys.c | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)


diff -Nru a/drivers/base/sys.c b/drivers/base/sys.c
--- a/drivers/base/sys.c 2005-03-09 16:28:45 -08:00
+++ b/drivers/base/sys.c 2005-03-09 16:28:45 -08:00
@@ -79,7 +79,7 @@
/*
* declare system_subsys
*/
-decl_subsys(system, &ktype_sysdev, NULL);
+static decl_subsys(system, &ktype_sysdev, NULL);

int sysdev_class_register(struct sysdev_class * cls)
{

2005-03-10 02:40:30

by Greg KH

[permalink] [raw]
Subject: [PATCH] sysdev: remove the rwsem usage from this subsystem.

ChangeSet 1.2054, 2005/03/09 15:39:09-08:00, [email protected]

[PATCH] sysdev: remove the rwsem usage from this subsystem.

If further finer grained locking is needed, we can add a lock to the sysdev_class to
lock the class drivers list. But if you do that, remember the global list also is still
there and needs to be protected. That's why I went with a simple lock for everything.

Signed-off-by: Greg Kroah-Hartman <[email protected]>


drivers/base/sys.c | 21 +++++++++++----------
1 files changed, 11 insertions(+), 10 deletions(-)


diff -Nru a/drivers/base/sys.c b/drivers/base/sys.c
--- a/drivers/base/sys.c 2005-03-09 16:28:10 -08:00
+++ b/drivers/base/sys.c 2005-03-09 16:28:10 -08:00
@@ -103,6 +103,7 @@


static LIST_HEAD(sysdev_drivers);
+static DECLARE_MUTEX(sysdev_drivers_lock);

/**
* sysdev_driver_register - Register auxillary driver
@@ -119,7 +120,7 @@
int sysdev_driver_register(struct sysdev_class * cls,
struct sysdev_driver * drv)
{
- down_write(&system_subsys.rwsem);
+ down(&sysdev_drivers_lock);
if (cls && kset_get(&cls->kset)) {
list_add_tail(&drv->entry, &cls->drivers);

@@ -131,7 +132,7 @@
}
} else
list_add_tail(&drv->entry, &sysdev_drivers);
- up_write(&system_subsys.rwsem);
+ up(&sysdev_drivers_lock);
return 0;
}

@@ -144,7 +145,7 @@
void sysdev_driver_unregister(struct sysdev_class * cls,
struct sysdev_driver * drv)
{
- down_write(&system_subsys.rwsem);
+ down(&sysdev_drivers_lock);
list_del_init(&drv->entry);
if (cls) {
if (drv->remove) {
@@ -154,7 +155,7 @@
}
kset_put(&cls->kset);
}
- up_write(&system_subsys.rwsem);
+ up(&sysdev_drivers_lock);
}

EXPORT_SYMBOL_GPL(sysdev_driver_register);
@@ -193,7 +194,7 @@
if (!error) {
struct sysdev_driver * drv;

- down_write(&system_subsys.rwsem);
+ down(&sysdev_drivers_lock);
/* Generic notification is implicit, because it's that
* code that should have called us.
*/
@@ -209,7 +210,7 @@
if (drv->add)
drv->add(sysdev);
}
- up_write(&system_subsys.rwsem);
+ up(&sysdev_drivers_lock);
}
return error;
}
@@ -218,7 +219,7 @@
{
struct sysdev_driver * drv;

- down_write(&system_subsys.rwsem);
+ down(&sysdev_drivers_lock);
list_for_each_entry(drv, &sysdev_drivers, entry) {
if (drv->remove)
drv->remove(sysdev);
@@ -228,7 +229,7 @@
if (drv->remove)
drv->remove(sysdev);
}
- up_write(&system_subsys.rwsem);
+ up(&sysdev_drivers_lock);

kobject_unregister(&sysdev->kobj);
}
@@ -255,7 +256,7 @@

pr_debug("Shutting Down System Devices\n");

- down_write(&system_subsys.rwsem);
+ down(&sysdev_drivers_lock);
list_for_each_entry_reverse(cls, &system_subsys.kset.list,
kset.kobj.entry) {
struct sys_device * sysdev;
@@ -284,7 +285,7 @@
cls->shutdown(sysdev);
}
}
- up_write(&system_subsys.rwsem);
+ up(&sysdev_drivers_lock);
}



2005-03-10 02:40:29

by Greg KH

[permalink] [raw]
Subject: [PATCH] cpufreq 2.4 interface removal schedule

ChangeSet 1.2037, 2005/03/09 09:32:00-08:00, [email protected]

[PATCH] cpufreq 2.4 interface removal schedule

Even though these 2.4. interfaces are already gone in Dave Jones' cpufreq
bitkeeper tree, here's a patch which properly announces it in
Documentation/feature-removal-schedule.txt:


Add meaningful content concerning the removal of deprecated interfaces to
the cpufreq core.

Signed-off-by: Dominik Brodowski <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>


Documentation/feature-removal-schedule.txt | 14 +++++++++++---
1 files changed, 11 insertions(+), 3 deletions(-)


diff -Nru a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
--- a/Documentation/feature-removal-schedule.txt 2005-03-09 16:30:09 -08:00
+++ b/Documentation/feature-removal-schedule.txt 2005-03-09 16:30:09 -08:00
@@ -17,10 +17,18 @@

---------------------------

-What: /proc/sys/cpu and the sysctl interface to cpufreq (2.4.x interfaces)
+What: /proc/sys/cpu/*, sysctl and /proc/cpufreq interfaces to cpufreq (2.4.x interfaces)
When: January 2005
Files: drivers/cpufreq/: cpufreq_userspace.c, proc_intf.c
- function calls throughout the kernel tree
-Why: Deprecated, has been replaced/superseded by (what?)....
+Why: /proc/sys/cpu/* has been deprecated since inclusion of cpufreq into
+ the main kernel tree. It bloats /proc/ unnecessarily and doesn't work
+ well with the "governor"-based design of cpufreq.
+ /proc/cpufreq/* has also been deprecated for a long time and was only
+ meant for usage during 2.5. until the new sysfs-based interface became
+ ready. It has an inconsistent interface which doesn't work well with
+ userspace setting the frequency. The output from /proc/cpufreq/* can
+ be emulated using "cpufreq-info --proc" (cpufrequtils).
+ Both interfaces are superseded by the cpufreq interface in
+ /sys/devices/system/cpu/cpu%n/cpufreq/.
Who: Dominik Brodowski <[email protected]>


2005-03-10 01:10:44

by Greg KH

[permalink] [raw]
Subject: [PATCH] floppy.c: pass physical device to device registration

ChangeSet 1.2047, 2005/03/09 09:53:08-08:00, [email protected]

[PATCH] floppy.c: pass physical device to device registration

With this patch the floppy driver creates the usual symlink in sysfs to
the physical device backing the block device:

$tree /sys/block/
/sys/block/
|-- fd0
| |-- dev
| |-- device -> ../../devices/platform/floppy0
...

Signed-off-by: Kay Sievers <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>


drivers/block/floppy.c | 19 ++++++-------------
1 files changed, 6 insertions(+), 13 deletions(-)


diff -Nru a/drivers/block/floppy.c b/drivers/block/floppy.c
--- a/drivers/block/floppy.c 2005-03-09 16:28:59 -08:00
+++ b/drivers/block/floppy.c 2005-03-09 16:28:59 -08:00
@@ -4370,6 +4370,10 @@
goto out_flush_work;
}

+ err = platform_device_register(&floppy_device);
+ if (err)
+ goto out_flush_work;
+
for (drive = 0; drive < N_DRIVE; drive++) {
if (!(allowed_drive_mask & (1 << drive)))
continue;
@@ -4379,23 +4383,12 @@
disks[drive]->private_data = (void *)(long)drive;
disks[drive]->queue = floppy_queue;
disks[drive]->flags |= GENHD_FL_REMOVABLE;
+ disks[drive]->driverfs_dev = &floppy_device.dev;
add_disk(disks[drive]);
}

- err = platform_device_register(&floppy_device);
- if (err)
- goto out_del_disk;
-
return 0;

-out_del_disk:
- for (drive = 0; drive < N_DRIVE; drive++) {
- if (!(allowed_drive_mask & (1 << drive)))
- continue;
- if (fdc_state[FDC(drive)].version == FDC_NONE)
- continue;
- del_gendisk(disks[drive]);
- }
out_flush_work:
flush_scheduled_work();
if (usage_count)
@@ -4600,7 +4593,6 @@
int drive;

init_completion(&device_release);
- platform_device_unregister(&floppy_device);
blk_unregister_region(MKDEV(FLOPPY_MAJOR, 0), 256);
unregister_blkdev(FLOPPY_MAJOR, "fd");

@@ -4614,6 +4606,7 @@
}
put_disk(disks[drive]);
}
+ platform_device_unregister(&floppy_device);
devfs_remove("floppy");

del_timer_sync(&fd_timeout);

2005-03-10 00:55:03

by Greg KH

[permalink] [raw]
Subject: [PATCH] class: add a semaphore to struct class, and use that instead of the subsystem rwsem.

ChangeSet 1.2055, 2005/03/09 15:41:29-08:00, [email protected]

[PATCH] class: add a semaphore to struct class, and use that instead of the subsystem rwsem.

This moves us away from using the rwsem, although recursive adds and removes of class devices
is not yet possible (nor is it really known if it even is needed.) So this simple change is
done instead.

Signed-off-by: Greg Kroah-Hartman <[email protected]>


drivers/base/class.c | 23 +++++++++++------------
include/linux/device.h | 2 +-
2 files changed, 12 insertions(+), 13 deletions(-)


diff -Nru a/drivers/base/class.c b/drivers/base/class.c
--- a/drivers/base/class.c 2005-03-09 16:28:04 -08:00
+++ b/drivers/base/class.c 2005-03-09 16:28:04 -08:00
@@ -140,6 +140,7 @@

INIT_LIST_HEAD(&cls->children);
INIT_LIST_HEAD(&cls->interfaces);
+ init_MUTEX(&cls->sem);
error = kobject_set_name(&cls->subsys.kset.kobj, "%s", cls->name);
if (error)
return error;
@@ -413,12 +414,12 @@

/* now take care of our own registration */
if (parent) {
- down_write(&parent->subsys.rwsem);
+ down(&parent->sem);
list_add_tail(&class_dev->node, &parent->children);
list_for_each_entry(class_intf, &parent->interfaces, node)
if (class_intf->add)
class_intf->add(class_dev);
- up_write(&parent->subsys.rwsem);
+ up(&parent->sem);
}

if (MAJOR(class_dev->devt))
@@ -448,12 +449,12 @@
struct class_interface * class_intf;

if (parent) {
- down_write(&parent->subsys.rwsem);
+ down(&parent->sem);
list_del_init(&class_dev->node);
list_for_each_entry(class_intf, &parent->interfaces, node)
if (class_intf->remove)
class_intf->remove(class_dev);
- up_write(&parent->subsys.rwsem);
+ up(&parent->sem);
}

if (class_dev->dev)
@@ -509,8 +510,8 @@

int class_interface_register(struct class_interface *class_intf)
{
- struct class * parent;
- struct class_device * class_dev;
+ struct class *parent;
+ struct class_device *class_dev;

if (!class_intf || !class_intf->class)
return -ENODEV;
@@ -519,14 +520,13 @@
if (!parent)
return -EINVAL;

- down_write(&parent->subsys.rwsem);
+ down(&parent->sem);
list_add_tail(&class_intf->node, &parent->interfaces);
-
if (class_intf->add) {
list_for_each_entry(class_dev, &parent->children, node)
class_intf->add(class_dev);
}
- up_write(&parent->subsys.rwsem);
+ up(&parent->sem);

return 0;
}
@@ -539,14 +539,13 @@
if (!parent)
return;

- down_write(&parent->subsys.rwsem);
+ down(&parent->sem);
list_del_init(&class_intf->node);
-
if (class_intf->remove) {
list_for_each_entry(class_dev, &parent->children, node)
class_intf->remove(class_dev);
}
- up_write(&parent->subsys.rwsem);
+ up(&parent->sem);

class_put(parent);
}
diff -Nru a/include/linux/device.h b/include/linux/device.h
--- a/include/linux/device.h 2005-03-09 16:28:04 -08:00
+++ b/include/linux/device.h 2005-03-09 16:28:04 -08:00
@@ -15,7 +15,6 @@
#include <linux/ioport.h>
#include <linux/kobject.h>
#include <linux/list.h>
-#include <linux/spinlock.h>
#include <linux/types.h>
#include <linux/module.h>
#include <linux/pm.h>
@@ -148,6 +147,7 @@
struct subsystem subsys;
struct list_head children;
struct list_head interfaces;
+ struct semaphore sem; /* locks both the children and interfaces lists */

struct class_attribute * class_attrs;
struct class_device_attribute * class_dev_attrs;

2005-03-10 03:24:43

by Greg KH

[permalink] [raw]
Subject: [PATCH] block core: export MAJOR/MINOR to the hotplug env

ChangeSet 1.2040, 2005/03/09 09:32:58-08:00, [email protected]

[PATCH] block core: export MAJOR/MINOR to the hotplug env

Signed-off-by: Kay Sievers <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>


drivers/block/genhd.c | 53 ++++++++++++++++++++++++++++++++------------------
1 files changed, 34 insertions(+), 19 deletions(-)


diff -Nru a/drivers/block/genhd.c b/drivers/block/genhd.c
--- a/drivers/block/genhd.c 2005-03-09 16:29:48 -08:00
+++ b/drivers/block/genhd.c 2005-03-09 16:29:48 -08:00
@@ -430,42 +430,57 @@
static int block_hotplug(struct kset *kset, struct kobject *kobj, char **envp,
int num_envp, char *buffer, int buffer_size)
{
- struct device *dev = NULL;
struct kobj_type *ktype = get_ktype(kobj);
+ struct device *physdev;
+ struct gendisk *disk;
+ struct hd_struct *part;
int length = 0;
int i = 0;

- /* get physical device backing disk or partition */
if (ktype == &ktype_block) {
- struct gendisk *disk = container_of(kobj, struct gendisk, kobj);
- dev = disk->driverfs_dev;
+ disk = container_of(kobj, struct gendisk, kobj);
+ add_hotplug_env_var(envp, num_envp, &i, buffer, buffer_size,
+ &length, "MINOR=%u", disk->first_minor);
} else if (ktype == &ktype_part) {
- struct gendisk *disk = container_of(kobj->parent, struct gendisk, kobj);
- dev = disk->driverfs_dev;
- }
-
- if (dev) {
- /* add physical device, backing this device */
- char *path = kobject_get_path(&dev->kobj, GFP_KERNEL);
+ disk = container_of(kobj->parent, struct gendisk, kobj);
+ part = container_of(kobj, struct hd_struct, kobj);
+ add_hotplug_env_var(envp, num_envp, &i, buffer, buffer_size,
+ &length, "MINOR=%u",
+ disk->first_minor + part->partno);
+ } else
+ return 0;
+
+ add_hotplug_env_var(envp, num_envp, &i, buffer, buffer_size, &length,
+ "MAJOR=%u", disk->major);
+
+ /* add physical device, backing this device */
+ physdev = disk->driverfs_dev;
+ if (physdev) {
+ char *path = kobject_get_path(&physdev->kobj, GFP_KERNEL);

add_hotplug_env_var(envp, num_envp, &i, buffer, buffer_size,
&length, "PHYSDEVPATH=%s", path);
kfree(path);

- /* add bus name of physical device */
- if (dev->bus)
+ if (physdev->bus)
add_hotplug_env_var(envp, num_envp, &i,
buffer, buffer_size, &length,
- "PHYSDEVBUS=%s", dev->bus->name);
+ "PHYSDEVBUS=%s",
+ physdev->bus->name);

- /* add driver name of physical device */
- if (dev->driver)
+ if (physdev->driver)
add_hotplug_env_var(envp, num_envp, &i,
buffer, buffer_size, &length,
- "PHYSDEVDRIVER=%s", dev->driver->name);
-
- envp[i] = NULL;
+ "PHYSDEVDRIVER=%s",
+ physdev->driver->name);
}
+
+ /* terminate, set to next free slot, shrink available space */
+ envp[i] = NULL;
+ envp = &envp[i];
+ num_envp -= i;
+ buffer = &buffer[length];
+ buffer_size -= length;

return 0;
}

2005-03-10 03:24:41

by Greg KH

[permalink] [raw]
Subject: [PATCH] videodev: pass dev_t to the class core

ChangeSet 1.2044, 2005/03/09 09:52:10-08:00, [email protected]

[PATCH] videodev: pass dev_t to the class core

Signed-off-by: Kay Sievers <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>


drivers/media/video/videodev.c | 11 +----------
1 files changed, 1 insertion(+), 10 deletions(-)


diff -Nru a/drivers/media/video/videodev.c b/drivers/media/video/videodev.c
--- a/drivers/media/video/videodev.c 2005-03-09 16:29:20 -08:00
+++ b/drivers/media/video/videodev.c 2005-03-09 16:29:20 -08:00
@@ -46,15 +46,7 @@
return sprintf(buf,"%.*s\n",(int)sizeof(vfd->name),vfd->name);
}

-static ssize_t show_dev(struct class_device *cd, char *buf)
-{
- struct video_device *vfd = container_of(cd, struct video_device, class_dev);
- dev_t dev = MKDEV(VIDEO_MAJOR, vfd->minor);
- return print_dev_t(buf,dev);
-}
-
static CLASS_DEVICE_ATTR(name, S_IRUGO, show_name, NULL);
-static CLASS_DEVICE_ATTR(dev, S_IRUGO, show_dev, NULL);

struct video_device *video_device_alloc(void)
{
@@ -347,12 +339,11 @@
if (vfd->dev)
vfd->class_dev.dev = vfd->dev;
vfd->class_dev.class = &video_class;
+ vfd->class_dev.devt = MKDEV(VIDEO_MAJOR, vfd->minor);
strlcpy(vfd->class_dev.class_id, vfd->devfs_name + 4, BUS_ID_SIZE);
class_device_register(&vfd->class_dev);
class_device_create_file(&vfd->class_dev,
&class_device_attr_name);
- class_device_create_file(&vfd->class_dev,
- &class_device_attr_dev);

#if 1 /* needed until all drivers are fixed */
if (!vfd->release)

2005-03-10 04:00:41

by Greg KH

[permalink] [raw]
Subject: [PATCH] USB: move usb core to use class_simple instead of it's own class functions.

ChangeSet 1.2051, 2005/03/09 12:17:18-08:00, [email protected]

[PATCH] USB: move usb core to use class_simple instead of it's own class functions.

This is needed if the class code is going to be made easier to use, and it makes the code
smaller and easier to understand.

Signed-off-by: Greg Kroah-Hartman <[email protected]>


drivers/usb/core/file.c | 55 ++++++++++++++++--------------------------------
1 files changed, 19 insertions(+), 36 deletions(-)


diff -Nru a/drivers/usb/core/file.c b/drivers/usb/core/file.c
--- a/drivers/usb/core/file.c 2005-03-09 16:28:31 -08:00
+++ b/drivers/usb/core/file.c 2005-03-09 16:28:31 -08:00
@@ -66,16 +66,7 @@
.open = usb_open,
};

-static void release_usb_class_dev(struct class_device *class_dev)
-{
- dbg("%s - %s", __FUNCTION__, class_dev->class_id);
- kfree(class_dev);
-}
-
-static struct class usb_class = {
- .name = "usb",
- .release = &release_usb_class_dev,
-};
+static struct class_simple *usb_class;

int usb_major_init(void)
{
@@ -87,9 +78,9 @@
goto out;
}

- error = class_register(&usb_class);
- if (error) {
- err("class_register failed for usb devices");
+ usb_class = class_simple_create(THIS_MODULE, "usb");
+ if (IS_ERR(usb_class)) {
+ err("class_simple_create failed for usb devices");
unregister_chrdev(USB_MAJOR, "usb");
goto out;
}
@@ -102,7 +93,7 @@

void usb_major_cleanup(void)
{
- class_unregister(&usb_class);
+ class_simple_destroy(usb_class);
devfs_remove("usb");
unregister_chrdev(USB_MAJOR, "usb");
}
@@ -134,7 +125,6 @@
int minor_base = class_driver->minor_base;
int minor = 0;
char name[BUS_ID_SIZE];
- struct class_device *class_dev;
char *temp;

#ifdef CONFIG_USB_DYNAMIC_MINORS
@@ -174,22 +164,18 @@
devfs_mk_cdev(MKDEV(USB_MAJOR, minor), class_driver->mode, name);

/* create a usb class device for this usb interface */
- class_dev = kmalloc(sizeof(*class_dev), GFP_KERNEL);
- if (class_dev) {
- memset(class_dev, 0x00, sizeof(struct class_device));
- class_dev->devt = MKDEV(USB_MAJOR, minor);
- class_dev->class = &usb_class;
- class_dev->dev = &intf->dev;
-
- temp = strrchr(name, '/');
- if (temp && (temp[1] != 0x00))
- ++temp;
- else
- temp = name;
- snprintf(class_dev->class_id, BUS_ID_SIZE, "%s", temp);
- class_set_devdata(class_dev, (void *)(long)intf->minor);
- class_device_register(class_dev);
- intf->class_dev = class_dev;
+ temp = strrchr(name, '/');
+ if (temp && (temp[1] != 0x00))
+ ++temp;
+ else
+ temp = name;
+ intf->class_dev = class_simple_device_add(usb_class, MKDEV(USB_MAJOR, minor), &intf->dev, "%s", temp);
+ if (IS_ERR(intf->class_dev)) {
+ spin_lock (&minor_lock);
+ usb_minors[intf->minor] = NULL;
+ spin_unlock (&minor_lock);
+ devfs_remove (name);
+ retval = PTR_ERR(intf->class_dev);
}
exit:
return retval;
@@ -232,11 +218,8 @@

snprintf(name, BUS_ID_SIZE, class_driver->name, intf->minor - minor_base);
devfs_remove (name);
-
- if (intf->class_dev) {
- class_device_unregister(intf->class_dev);
- intf->class_dev = NULL;
- }
+ class_simple_device_remove(MKDEV(USB_MAJOR, intf->minor));
+ intf->class_dev = NULL;
intf->minor = -1;
}
EXPORT_SYMBOL(usb_deregister_dev);

2005-03-10 04:59:53

by Dominik Brodowski

[permalink] [raw]
Subject: Re: [PATCH] Add 2.4.x cpufreq /proc and sysctl interface removal feature-removal-schedule

On Wed, Mar 09, 2005 at 04:34:38PM -0800, Greg KH wrote:
> ChangeSet 1.2036, 2005/03/09 09:31:40-08:00, [email protected]
>
> [PATCH] Add 2.4.x cpufreq /proc and sysctl interface removal feature-removal-schedule
>
> Add 2.4.x cpufreq /proc and sysctl interface removal
> to the feature-removal-schedule.
>
> [PATCH] cpufreq 2.4 interface removal schedule
>
> Even though these 2.4. interfaces are already gone in Dave Jones' cpufreq
> bitkeeper tree, here's a patch which properly announces it in
> Documentation/feature-removal-schedule.txt:


Both already _were_ in Linus' tree; the entry got removed along with the
cpufreq 2.4. interface. So please do not re-add this entry.

Dominik

2005-03-11 00:01:21

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH] cpufreq 2.4 interface removal schedule

On Wed, Mar 09, 2005 at 05:32:00PM +0000, Linux Kernel wrote:
> ChangeSet 1.1994.11.3, 2005/03/09 09:32:00-08:00, [email protected]
>
> [PATCH] cpufreq 2.4 interface removal schedule
>

> ChangeSet 1.1994.11.2, 2005/03/09 09:31:40-08:00, [email protected]
>
> [PATCH] Add 2.4.x cpufreq /proc and sysctl interface removal feature-removal-schedule

please bk cset -x these. They shouldn't have ended up in your tree since
this stuff got ripped out two months ago.

Dave

2005-03-25 18:01:42

by Paul Mundt

[permalink] [raw]
Subject: Re: [PATCH] driver core: Separate platform device name from platform device number

On Wed, Mar 09, 2005 at 04:34:39PM -0800, Greg KH wrote:
> [PATCH] driver core: Separate platform device name from platform device number
>
> Separate platform device name from platform device number such that
> names ending with numbers aren't confusing.
>
This might make sense for devices that end in numbers, but does it really
make sense for devices that don't? I don't really see how having
something like randomfb.0 is intuitive, this may make sense for things
like serial8250 where another 0 would be misleading without some form of
delimiter, but those are the corner cases and should be treated as such.

It's a bit irritating to have to constantly update userspace code that is
acting under the false pretense that there is some sort of consistent
naming scheme in place that won't change every time some new corner case
crops up.

(And yes, I should have brought this up when the patch was posted, but I
didn't see it until _after_ being bit by this change).


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

2005-03-25 18:10:27

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] driver core: Separate platform device name from platform device number

On Fri, Mar 25, 2005 at 08:01:36PM +0200, Paul Mundt wrote:
> On Wed, Mar 09, 2005 at 04:34:39PM -0800, Greg KH wrote:
> > [PATCH] driver core: Separate platform device name from platform device number
> >
> > Separate platform device name from platform device number such that
> > names ending with numbers aren't confusing.
> >
> This might make sense for devices that end in numbers, but does it really
> make sense for devices that don't?

Then fix those drivers to not put the number in there if they don't have
one :)

> I don't really see how having something like randomfb.0 is intuitive,
> this may make sense for things like serial8250 where another 0 would
> be misleading without some form of delimiter, but those are the corner
> cases and should be treated as such.

I don't see the serial8250 driver adding that .0 to it on my machines,
does this happen on yours?

> It's a bit irritating to have to constantly update userspace code that is
> acting under the false pretense that there is some sort of consistent
> naming scheme in place that won't change every time some new corner case
> crops up.

What userspace code are you referring to?

thanks,

greg k-h

2005-03-25 18:35:58

by Paul Mundt

[permalink] [raw]
Subject: Re: [PATCH] driver core: Separate platform device name from platform device number

On Fri, Mar 25, 2005 at 10:10:14AM -0800, Greg KH wrote:
> > This might make sense for devices that end in numbers, but does it really
> > make sense for devices that don't?
>
> Then fix those drivers to not put the number in there if they don't have
> one :)
>
But they do have non -1 ids, the device name itself just doesn't end in a
number. In this event, the delimiter makes no sense.

These drivers are expecting that you will have <devicename><id>, and
application code is expecting the same. /dev follows this convention too,
I don't see this as being an unreasonable expectation.

If anything, serial8250 is broken and should rename itself to something
not ending in a number. It's not nice when one driver exhibits a corner
case and decides to change the semantics for everyone else.

> I don't see the serial8250 driver adding that .0 to it on my machines,
> does this happen on yours?
>
Yes, we end up having /sys/devices/platform/serial8250 and serial8250.0.
Where serial8250.0 ends up as:

drwxr-xr-x 3 0 0 0 Jan 1 00:00 .
drwxr-xr-x 18 0 0 0 Jan 1 00:00 ..
lrwxrwxrwx 1 0 0 0 Jan 1 00:00 bus -> ../../../bus/platform
-rw-r--r-- 1 0 0 4096 Jan 1 00:00 detach_state
lrwxrwxrwx 1 0 0 0 Jan 1 00:00 driver -> ../../../bus/platform/drivers/serial8250
drwxr-xr-x 2 0 0 0 Jan 1 00:00 power

That doesn't really bother me, having serial8250.0 is more sensible then
serial82500. For this type of corner case the delimiter makes sense, but
not in a blanket sense.

> What userspace code are you referring to?
>
Anything that expects that it can open a /sys/devices/platform/<device><id>
path. I have a few applications like this, I have no reason to doubt that
others do too. I don't see any reason to go out of the way to break this
convention if the end of the device name is not a number.


Attachments:
(No filename) (1.90 kB)
(No filename) (189.00 B)
Download all attachments

2005-03-25 19:38:39

by Kyle Moffett

[permalink] [raw]
Subject: Re: [PATCH] driver core: Separate platform device name from platform device number

On Mar 25, 2005, at 13:35, Paul Mundt wrote:
> Anything that expects that it can open a
> /sys/devices/platform/<device><id>
> path. I have a few applications like this, I have no reason to doubt
> that
> others do too. I don't see any reason to go out of the way to break
> this
> convention if the end of the device name is not a number.

So how would you tell the difference between the following?
device = "foobar0"
id = -1
path = "/sys/devices/platform/foobar0"
versus
device = "foobar"
id = 0
path = "/sys/devices/platform/foobar0"

I'll agree that having two drivers named like this is bad, but how is a
userspace application given a path like "/sys/devices/platform/foobar0"
supposed to figure out which one it is. It's not as nice to add the
extra period, but otherwise you end up with a lot of _extra_ special
cases in both the kernel _and_ applications, which helps nobody.

Cheers,
Kyle Moffett

-----BEGIN GEEK CODE BLOCK-----
Version: 3.12
GCM/CS/IT/U d- s++: a18 C++++>$ UB/L/X/*++++(+)>$ P+++(++++)>$
L++++(+++) E W++(+) N+++(++) o? K? w--- O? M++ V? PS+() PE+(-) Y+
PGP+++ t+(+++) 5 X R? tv-(--) b++++(++) DI+ D+ G e->++++$ h!*()>++$ r
!y?(-)
------END GEEK CODE BLOCK------


2005-03-25 19:58:34

by Paul Mundt

[permalink] [raw]
Subject: Re: [PATCH] driver core: Separate platform device name from platform device number

On Fri, Mar 25, 2005 at 02:38:22PM -0500, Kyle Moffett wrote:
> So how would you tell the difference between the following?
> device = "foobar0"
> id = -1
> path = "/sys/devices/platform/foobar0"
> versus
> device = "foobar"
> id = 0
> path = "/sys/devices/platform/foobar0"
>
Easy, we use the delimiter on anything ending with a number at the end of
the device name.. so for device = "foobar0", this would end up as
/sys/devices/platform/foobar0.0, whereas in the latter case this would
end up as /sys/devices/platform/foobar0.

The first case is a corner case, and really shouldn't happen that much in
practice outside of broken drivers.

> It's not as nice to add the extra period, but otherwise you end up with
> a lot of _extra_ special cases in both the kernel _and_ applications,
> which helps nobody.
>
No you don't, it's pretty easy to figure out that if the end of the
device name is a number that there will be a delimiter between that and
the id. This should be the exception, not the rule.

We don't go around changing /dev semantics everytime someone decides to
call their device something silly, I don't see why platform devices
should be treated differently, better to just fix the broken drivers..


Attachments:
(No filename) (1.19 kB)
(No filename) (189.00 B)
Download all attachments

2005-03-25 20:17:27

by Kyle Moffett

[permalink] [raw]
Subject: Re: [PATCH] driver core: Separate platform device name from platform device number

On Mar 25, 2005, at 14:58, Paul Mundt wrote:
> On Fri, Mar 25, 2005 at 02:38:22PM -0500, Kyle Moffett wrote:
>> So how would you tell the difference between the following?
>> device = "foobar0"
>> id = -1
>> path = "/sys/devices/platform/foobar0"
>> versus
>> device = "foobar"
>> id = 0
>> path = "/sys/devices/platform/foobar0"
>>
> Easy, we use the delimiter on anything ending with a number at the end
> of
> the device name.. so for device = "foobar0", this would end up as
> /sys/devices/platform/foobar0.0, whereas in the latter case this would
> end up as /sys/devices/platform/foobar0.

But then you've just created yet another special case that will clutter
the
interface in both the kernel _and_ all the applications. Besides, the
ID
on the first entry isn't 0, it's -1, which indicates it's a singleton.
You'd break existing applications anyways, because you're renaming a
device
that existed and worked fine until it got renamed. We should try to fix
the interface properly where it's broken so that we don't have to live
with
the consequences for the next 3 years.

> The first case is a corner case, and really shouldn't happen that much
> in
> practice outside of broken drivers.

But a driver ending in a number _isn't_ really a corner case. Most
devices
have model numbers, and so we shouldn't twist that case to do something
funny when we shouldn't have to.

> No you don't, it's pretty easy to figure out that if the end of the
> device name is a number that there will be a delimiter between that and
> the id. This should be the exception, not the rule.

But on the first example, there _isn't_ an ID, it's -1, it's a
singleton.

> We don't go around changing /dev semantics everytime someone decides to
> call their device something silly, I don't see why platform devices
> should be treated differently, better to just fix the broken drivers..

Fix the broken interface to have a unique naming scheme that doesn't
need
special cases and this problem will be less likely to occur again in the
future.

We all agree that if we were just creating this interface now, we would
use something like this, right?

if (id == -1) {
snprintf( path, path_len, "/sys/devices/platform/%s", name);
} else {
snprintf( path, path_len, "/sys/devices/platform/%s.%lu", name, id );
}

So why not do this now? It's a lot simpler and easier to get right,
with
no special cases other than the already existing singleton case. It
also
only requires a 1-character change to all existing code, the extra ".".

Cheers,
Kyle Moffett

-----BEGIN GEEK CODE BLOCK-----
Version: 3.12
GCM/CS/IT/U d- s++: a18 C++++>$ UB/L/X/*++++(+)>$ P+++(++++)>$
L++++(+++) E W++(+) N+++(++) o? K? w--- O? M++ V? PS+() PE+(-) Y+
PGP+++ t+(+++) 5 X R? tv-(--) b++++(++) DI+ D+ G e->++++$ h!*()>++$ r
!y?(-)
------END GEEK CODE BLOCK------


2005-03-25 20:26:55

by Russell King

[permalink] [raw]
Subject: Re: [PATCH] driver core: Separate platform device name from platform device number

On Fri, Mar 25, 2005 at 09:58:26PM +0200, Paul Mundt wrote:
> On Fri, Mar 25, 2005 at 02:38:22PM -0500, Kyle Moffett wrote:
> > So how would you tell the difference between the following?
> > device = "foobar0"
> > id = -1
> > path = "/sys/devices/platform/foobar0"
> > versus
> > device = "foobar"
> > id = 0
> > path = "/sys/devices/platform/foobar0"
> >
> Easy, we use the delimiter on anything ending with a number at the end of
> the device name.. so for device = "foobar0", this would end up as
> /sys/devices/platform/foobar0.0, whereas in the latter case this would
> end up as /sys/devices/platform/foobar0.

Eh? How do you end up with "/sys/devices/platform/foobar0.0" for the
former case? It has an ID of "-1", and not zero. Your idea doesn't
make any sense.

> The first case is a corner case, and really shouldn't happen that much in
> practice outside of broken drivers.

It does happen today. Firstly, the 8250 driver registers a device of
"serial8250" with id = -1 for the backwards-compatible devices.
Platforms can then register a platform device called "serial8250"
with zero or positive id numbers.

> > It's not as nice to add the extra period, but otherwise you end up with
> > a lot of _extra_ special cases in both the kernel _and_ applications,
> > which helps nobody.
> >
> No you don't, it's pretty easy to figure out that if the end of the
> device name is a number that there will be a delimiter between that and
> the id. This should be the exception, not the rule.

Note that id = -1 means _no id_. So, Kyle is quite correct to ask about
that case.

device = "serial8250"
id = -1
=> /sys/devices/platform/serial8250

The "-1" means "do not add the ID".

but, under the old naming scenario, the following comes out to the same
sysfs path:

device = "serial825"
id = 0
=> /sys/devices/platform/serial8250

and

device = "serial8250"
id = 0
=> /sys/devices/platform/serial82500

is just too confusing. Same problem with i82365 platform devices, etc.

> We don't go around changing /dev semantics everytime someone decides to
> call their device something silly, I don't see why platform devices
> should be treated differently, better to just fix the broken drivers..

It's not about something being called something silly. It's about
the original concept of how to generate the path being down right
stupid.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core

2005-03-25 20:56:19

by Paul Mundt

[permalink] [raw]
Subject: Re: [PATCH] driver core: Separate platform device name from platform device number

On Fri, Mar 25, 2005 at 08:25:08PM +0000, Russell King wrote:
> Eh? How do you end up with "/sys/devices/platform/foobar0.0" for the
> former case? It has an ID of "-1", and not zero. Your idea doesn't
> make any sense.
>
Yes, I missed the -1 part, so Kyle is correct.

It would be trivial to treat them both as foobar0 and have the
registration succeed for whoever gets it first, but I could see that this
would be problematic in the serial8250 case. On the other hand, this is
then serial8250's problem.

> > The first case is a corner case, and really shouldn't happen that much in
> > practice outside of broken drivers.
>
> It does happen today. Firstly, the 8250 driver registers a device of
> "serial8250" with id = -1 for the backwards-compatible devices.
> Platforms can then register a platform device called "serial8250"
> with zero or positive id numbers.
>
That's fine, but that still doesn't make it any less of a corner case.

> > We don't go around changing /dev semantics everytime someone decides to
> > call their device something silly, I don't see why platform devices
> > should be treated differently, better to just fix the broken drivers..
>
> It's not about something being called something silly. It's about
> the original concept of how to generate the path being down right
> stupid.
>
<dev><id> is a fairly common thing, if you have a problem with this,
maybe you would like to audit /dev while you are at it. I don't disagree
with you that this is useful for the devices that do end with numbers in
their names, but breaking everything else as a result of this makes no
sense either.

What would you do if you needed to register a character device using the
name of the device (which may end in a number, and there was a range of
them)? This likely doesn't happen enough in practice for anyone to
actually care, but you would have the same problem there otherwise.

This should arguably be the problem of the corner case driver, it
certainly shouldn't change convention for everyone else. While the
original concept of how to generate the path may have been "down right
stupid", it works for /dev, and I don't see how adding a superfluous . in
the paths of devices that just don't care and subsequently breaking
existing expectations of behaviour is any more inspired..


Attachments:
(No filename) (2.26 kB)
(No filename) (189.00 B)
Download all attachments

2005-03-25 21:04:23

by Russell King

[permalink] [raw]
Subject: Re: [PATCH] driver core: Separate platform device name from platform device number

On Fri, Mar 25, 2005 at 10:56:03PM +0200, Paul Mundt wrote:
> On Fri, Mar 25, 2005 at 08:25:08PM +0000, Russell King wrote:
> > Eh? How do you end up with "/sys/devices/platform/foobar0.0" for the
> > former case? It has an ID of "-1", and not zero. Your idea doesn't
> > make any sense.
> >
> Yes, I missed the -1 part, so Kyle is correct.
>
> It would be trivial to treat them both as foobar0 and have the
> registration succeed for whoever gets it first, but I could see that this
> would be problematic in the serial8250 case. On the other hand, this is
> then serial8250's problem.

Thank you for ignoring the other case of i82385 to justify your point
of view of it being just a single driver problem.

Maybe you can work out a patch to fix up this mess?

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core

2005-03-25 22:16:41

by Paul Mundt

[permalink] [raw]
Subject: Re: [PATCH] driver core: Separate platform device name from platform device number

On Fri, Mar 25, 2005 at 09:03:57PM +0000, Russell King wrote:
> > It would be trivial to treat them both as foobar0 and have the
> > registration succeed for whoever gets it first, but I could see that this
> > would be problematic in the serial8250 case. On the other hand, this is
> > then serial8250's problem.
>
> Thank you for ignoring the other case of i82385 to justify your point
> of view of it being just a single driver problem.
>
I didn't ignore it, I said that this was useful for anything that had
device names ending in numbers. The above was just in reply to what you
had pointed out about the serial8250 behaviour. Thank you for missing the
point though.

> Maybe you can work out a patch to fix up this mess?
>
Yes, I'll hack something together in the morning.


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