Hi,
Here's some driver core changes that do the following things:
- remove struct device.name field and fix up remaining
subsystems
- add warnings for programmers who implement driver core code
incorrectly.
Pat has approved these changes.
Please pull from:
bk://kernel.bkbits.net/gregkh/linux/driver-2.6
thanks,
greg k-h
p.s. I'll send these as patches in response to this email to lkml for
those who want to see them.
drivers/base/power.c | 140 ------------------------------------------
drivers/base/class.c | 14 +++-
drivers/base/core.c | 17 +++--
drivers/base/interface.c | 96 +++++++---------------------
drivers/base/platform.c | 1
drivers/base/power.c | 2
drivers/base/power/shutdown.c | 68 ++++++++++++++++++++
drivers/block/floppy.c | 6 -
drivers/ide/ide-probe.c | 3
drivers/media/video/bttv-if.c | 2
drivers/pcmcia/i82365.c | 3
drivers/pcmcia/tcic.c | 3
drivers/pcmcia/yenta_socket.c | 2
drivers/scsi/scsi_debug.c | 2
drivers/scsi/scsi_scan.c | 20 ------
drivers/scsi/scsi_sysfs.c | 2
include/linux/device.h | 26 +++----
17 files changed, 131 insertions(+), 276 deletions(-)
-----
Greg Kroah-Hartman:
o Driver Core: add warnings if .release functions are not set for objects
o Remove usage of struct device.name from scsi core
o Remove usage of struct device.name from bttv driver
o Remove usage of struct device.name from pcmcia layer
o Remove usage of struct device.name from ide core
o Remove .name usage from floppy driver
o Driver Core: remove struct device.name as it is not needed
ChangeSet 1.1152.2.5, 2003/08/14 16:55:36-07:00, [email protected]
Remove usage of struct device.name from bttv driver
I missed this on the i2c series of patches.
drivers/media/video/bttv-if.c | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
diff -Nru a/drivers/media/video/bttv-if.c b/drivers/media/video/bttv-if.c
--- a/drivers/media/video/bttv-if.c Fri Aug 15 11:15:56 2003
+++ b/drivers/media/video/bttv-if.c Fri Aug 15 11:15:56 2003
@@ -315,7 +315,7 @@
memcpy(&btv->i2c_client, &bttv_i2c_client_template,
sizeof(struct i2c_client));
- sprintf(btv->i2c_adap.dev.name, "bt848 #%d", btv->nr);
+ sprintf(btv->i2c_adap.name, "bt848 #%d", btv->nr);
btv->i2c_adap.dev.parent = &btv->dev->dev;
btv->i2c_algo.data = btv;
ChangeSet 1.1152.2.6, 2003/08/15 10:08:00-07:00, [email protected]
Remove usage of struct device.name from scsi core
drivers/scsi/scsi_debug.c | 2 --
drivers/scsi/scsi_scan.c | 20 --------------------
drivers/scsi/scsi_sysfs.c | 2 --
3 files changed, 24 deletions(-)
diff -Nru a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
--- a/drivers/scsi/scsi_debug.c Fri Aug 15 11:15:52 2003
+++ b/drivers/scsi/scsi_debug.c Fri Aug 15 11:15:52 2003
@@ -1566,7 +1566,6 @@
module_exit(scsi_debug_exit);
static struct device pseudo_primary = {
- .name = "Host/Pseudo Bridge",
.bus_id = "pseudo_0",
};
@@ -1630,7 +1629,6 @@
sdbg_host->dev.bus = &pseudo_lld_bus;
sdbg_host->dev.parent = &pseudo_primary;
sdbg_host->dev.release = &sdebug_release_adapter;
- sprintf(sdbg_host->dev.name, "scsi debug adapter");
sprintf(sdbg_host->dev.bus_id, "adapter%d", scsi_debug_add_host);
error = device_register(&sdbg_host->dev);
diff -Nru a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
--- a/drivers/scsi/scsi_scan.c Fri Aug 15 11:15:52 2003
+++ b/drivers/scsi/scsi_scan.c Fri Aug 15 11:15:52 2003
@@ -447,24 +447,6 @@
return;
}
-static void scsi_set_name(struct scsi_device *sdev, char *inq_result)
-{
- int i;
- char type[72];
-
- i = inq_result[0] & 0x1f;
- if (i < MAX_SCSI_DEVICE_CODE)
- strcpy(type, scsi_device_types[i]);
- else
- strcpy(type, "Unknown");
-
- i = strlen(type) - 1;
- while (i >= 0 && type[i] == ' ')
- type[i--] = '\0';
-
- snprintf(sdev->sdev_gendev.name, DEVICE_NAME_SIZE, "SCSI %s", type);
-}
-
/**
* scsi_add_lun - allocate and fully initialze a Scsi_Device
* @sdevscan: holds information to be stored in the new Scsi_Device
@@ -538,8 +520,6 @@
default:
printk(KERN_INFO "scsi: unknown device type %d\n", sdev->type);
}
-
- scsi_set_name(sdev, inq_result);
print_inquiry(inq_result);
diff -Nru a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
--- a/drivers/scsi/scsi_sysfs.c Fri Aug 15 11:15:52 2003
+++ b/drivers/scsi/scsi_sysfs.c Fri Aug 15 11:15:52 2003
@@ -408,8 +408,6 @@
device_initialize(&shost->shost_gendev);
snprintf(shost->shost_gendev.bus_id, BUS_ID_SIZE, "host%d",
shost->host_no);
- snprintf(shost->shost_gendev.name, DEVICE_NAME_SIZE, "%s",
- shost->hostt->proc_name);
shost->shost_gendev.release = scsi_host_dev_release;
class_device_initialize(&shost->shost_classdev);
ChangeSet 1.1152.2.2, 2003/08/14 16:51:20-07:00, [email protected]
Remove .name usage from floppy driver.
drivers/block/floppy.c | 3 ---
1 files changed, 3 deletions(-)
diff -Nru a/drivers/block/floppy.c b/drivers/block/floppy.c
--- a/drivers/block/floppy.c Fri Aug 15 11:16:09 2003
+++ b/drivers/block/floppy.c Fri Aug 15 11:16:09 2003
@@ -4228,9 +4228,6 @@
static struct platform_device floppy_device = {
.name = "floppy",
.id = 0,
- .dev = {
- .name = "Floppy Drive",
- },
};
static struct kobject *floppy_find(dev_t dev, int *part, void *data)
ChangeSet 1.1152.2.3, 2003/08/14 16:52:13-07:00, [email protected]
Remove usage of struct device.name from ide core
drivers/ide/ide-probe.c | 3 ---
1 files changed, 3 deletions(-)
diff -Nru a/drivers/ide/ide-probe.c b/drivers/ide/ide-probe.c
--- a/drivers/ide/ide-probe.c Fri Aug 15 11:16:05 2003
+++ b/drivers/ide/ide-probe.c Fri Aug 15 11:16:05 2003
@@ -648,7 +648,6 @@
{
/* register with global device tree */
strlcpy(hwif->gendev.bus_id,hwif->name,BUS_ID_SIZE);
- snprintf(hwif->gendev.name,DEVICE_NAME_SIZE,"IDE Controller");
hwif->gendev.driver_data = hwif;
if (hwif->pci_dev)
hwif->gendev.parent = &hwif->pci_dev->dev;
@@ -1217,8 +1216,6 @@
ide_add_generic_settings(drive);
snprintf(drive->gendev.bus_id,BUS_ID_SIZE,"%u.%u",
hwif->index,unit);
- snprintf(drive->gendev.name,DEVICE_NAME_SIZE,
- "%s","IDE Drive");
drive->gendev.parent = &hwif->gendev;
drive->gendev.bus = &ide_bus_type;
drive->gendev.driver_data = drive;
ChangeSet 1.1152.2.1, 2003/08/14 16:31:09-07:00, [email protected]
Driver Core: remove struct device.name as it is not needed
If a specific driver subsystem needs a name field, they should implement it
for just that subsystem.
drivers/base/core.c | 6 ++----
drivers/base/interface.c | 8 --------
drivers/base/platform.c | 1 -
drivers/base/power.c | 2 +-
include/linux/device.h | 1 -
5 files changed, 3 insertions(+), 15 deletions(-)
diff -Nru a/drivers/base/core.c b/drivers/base/core.c
--- a/drivers/base/core.c Fri Aug 15 11:16:13 2003
+++ b/drivers/base/core.c Fri Aug 15 11:16:13 2003
@@ -210,8 +210,7 @@
parent = get_device(dev->parent);
- pr_debug("DEV: registering device: ID = '%s', name = %s\n",
- dev->bus_id, dev->name);
+ pr_debug("DEV: registering device: ID = '%s'\n", dev->bus_id);
/* first, register with generic layer. */
strlcpy(dev->kobj.name,dev->bus_id,KOBJ_NAME_LEN);
@@ -337,8 +336,7 @@
*/
void device_unregister(struct device * dev)
{
- pr_debug("DEV: Unregistering device. ID = '%s', name = '%s'\n",
- dev->bus_id,dev->name);
+ pr_debug("DEV: Unregistering device. ID = '%s'\n", dev->bus_id);
device_del(dev);
put_device(dev);
}
diff -Nru a/drivers/base/interface.c b/drivers/base/interface.c
--- a/drivers/base/interface.c Fri Aug 15 11:16:13 2003
+++ b/drivers/base/interface.c Fri Aug 15 11:16:13 2003
@@ -14,13 +14,6 @@
#include <linux/stat.h>
#include <linux/string.h>
-static ssize_t device_read_name(struct device * dev, char * buf)
-{
- return sprintf(buf,"%s\n",dev->name);
-}
-
-static DEVICE_ATTR(name,S_IRUGO,device_read_name,NULL);
-
static ssize_t
device_read_power(struct device * dev, char * page)
{
@@ -91,7 +84,6 @@
device_read_power,device_write_power);
struct attribute * dev_default_attrs[] = {
- &dev_attr_name.attr,
&dev_attr_power.attr,
NULL,
};
diff -Nru a/drivers/base/platform.c b/drivers/base/platform.c
--- a/drivers/base/platform.c Fri Aug 15 11:16:13 2003
+++ b/drivers/base/platform.c Fri Aug 15 11:16:13 2003
@@ -15,7 +15,6 @@
#include <linux/init.h>
struct device legacy_bus = {
- .name = "legacy bus",
.bus_id = "legacy",
};
diff -Nru a/drivers/base/power.c b/drivers/base/power.c
--- a/drivers/base/power.c Fri Aug 15 11:16:13 2003
+++ b/drivers/base/power.c Fri Aug 15 11:16:13 2003
@@ -59,7 +59,7 @@
error = dev->driver->suspend(dev,state,level);
if (error)
printk(KERN_ERR "%s: suspend returned %d\n",
- dev->name,error);
+ dev->bus_id, error);
}
}
up_write(&devices_subsys.rwsem);
diff -Nru a/include/linux/device.h b/include/linux/device.h
--- a/include/linux/device.h Fri Aug 15 11:16:13 2003
+++ b/include/linux/device.h Fri Aug 15 11:16:13 2003
@@ -258,7 +258,6 @@
struct device * parent;
struct kobject kobj;
- char name[DEVICE_NAME_SIZE]; /* descriptive ascii string */
char bus_id[BUS_ID_SIZE]; /* position on parent bus */
struct bus_type * bus; /* type of bus device is on */
ChangeSet 1.1152.2.7, 2003/08/15 10:12:59-07:00, [email protected]
Driver Core: add warnings if .release functions are not set for objects.
This has been in the -mm tree for a while and has helped a lot in finding
lots of improper users of the driver core. It also moves the driver core
code up quite a few levels on the "Rusty's guide to kernel APIs".
drivers/base/class.c | 6 ++++++
drivers/base/core.c | 6 ++++++
2 files changed, 12 insertions(+)
diff -Nru a/drivers/base/class.c b/drivers/base/class.c
--- a/drivers/base/class.c Fri Aug 15 11:15:48 2003
+++ b/drivers/base/class.c Fri Aug 15 11:15:48 2003
@@ -194,6 +194,12 @@
if (cls->release)
cls->release(cd);
+ else {
+ printk(KERN_ERR "Device class '%s' does not have a release() function, "
+ "it is broken and must be fixed.\n",
+ cd->class_id);
+ WARN_ON(1);
+ }
}
static struct kobj_type ktype_class_device = {
diff -Nru a/drivers/base/core.c b/drivers/base/core.c
--- a/drivers/base/core.c Fri Aug 15 11:15:48 2003
+++ b/drivers/base/core.c Fri Aug 15 11:15:48 2003
@@ -77,6 +77,12 @@
struct device * dev = to_dev(kobj);
if (dev->release)
dev->release(dev);
+ else {
+ printk(KERN_ERR "Device '%s' does not have a release() function, "
+ "it is broken and must be fixed.\n",
+ dev->bus_id);
+ WARN_ON(1);
+ }
}
static struct kobj_type ktype_device = {
ChangeSet 1.1152.2.4, 2003/08/14 16:53:55-07:00, [email protected]
Remove usage of struct device.name from pcmcia layer
drivers/pcmcia/i82365.c | 3 ---
drivers/pcmcia/tcic.c | 3 ---
drivers/pcmcia/yenta_socket.c | 2 +-
3 files changed, 1 insertion(+), 7 deletions(-)
diff -Nru a/drivers/pcmcia/i82365.c b/drivers/pcmcia/i82365.c
--- a/drivers/pcmcia/i82365.c Fri Aug 15 11:16:00 2003
+++ b/drivers/pcmcia/i82365.c Fri Aug 15 11:16:00 2003
@@ -1361,9 +1361,6 @@
static struct platform_device i82365_device = {
.name = "i82365",
.id = 0,
- .dev = {
- .name = "i82365",
- },
};
static int __init init_i82365(void)
diff -Nru a/drivers/pcmcia/tcic.c b/drivers/pcmcia/tcic.c
--- a/drivers/pcmcia/tcic.c Fri Aug 15 11:16:00 2003
+++ b/drivers/pcmcia/tcic.c Fri Aug 15 11:16:00 2003
@@ -372,9 +372,6 @@
static struct platform_device tcic_device = {
.name = "tcic-pcmcia",
.id = 0,
- .dev = {
- .name = "tcic-pcmcia",
- },
};
diff -Nru a/drivers/pcmcia/yenta_socket.c b/drivers/pcmcia/yenta_socket.c
--- a/drivers/pcmcia/yenta_socket.c Fri Aug 15 11:16:00 2003
+++ b/drivers/pcmcia/yenta_socket.c Fri Aug 15 11:16:00 2003
@@ -899,7 +899,7 @@
/* We must finish initialization here */
- if (!socket->cb_irq || request_irq(socket->cb_irq, yenta_interrupt, SA_SHIRQ, socket->dev->dev.name, socket)) {
+ if (!socket->cb_irq || request_irq(socket->cb_irq, yenta_interrupt, SA_SHIRQ, pci_name(socket->dev), socket)) {
/* No IRQ or request_irq failed. Poll */
socket->cb_irq = 0; /* But zero is a valid IRQ number. */
init_timer(&socket->poll_timer);
On Fri, Aug 15, 2003 at 09:54:59PM +0200, Ingo Oeser wrote:
> Hi Greg,
Hi, I've brought this back to lkml as I'm getting tired of private email
threads about this topic. Hope you don't mind.
> On Fri, Aug 15, 2003 at 11:25:00AM -0700, Greg KH wrote:
> > Here's some driver core changes that do the following things:
> > - remove struct device.name field and fix up remaining
> > subsystems
>
> Could you point me to the rationale about this?
>
> I for one considered "everything should have a name" policy very
> useful and extendible.
The main problem is that we don't want to be putting name databases in
the kernel, like PCI, PnP, and USB were starting to do. People were
starting to complain that the PCI and USB names were not the "proper"
name, as we didn't have enough room for the "full" name.
Naming databases belong in userspace. For PCI, PnP, and USB we can
determine the name ourselves from userspace using lspci, lspnp, and
lsusb. Getting rid of the name field prevents us from relying on kernel
code when we shouldn't be.
> Not that I would like to change that back, just like to know why
> this is done, why so late and why after introducing it into all
> drivers in core?
We messed up, and we're now fixing that before people start to rely on
it :)
Now some subsystems still want to export a "name" as there is no other
way to determine the type of the device (no vendor or product ids.) For
them, a name field is just fine to have. For example, I moved the name
field back into the i2c_client and i2c_adapter structures for this very
reason.
Hey, we're saving kernel memory, and this is a problem? :)
Hope this helps explain things.
greg k-h
Hi,
On Fri, Aug 15, 2003 at 08:28:33PM -0700, Greg KH wrote:
> On Fri, Aug 15, 2003 at 09:54:59PM +0200, Ingo Oeser wrote:
[...]
> Hi, I've brought this back to lkml as I'm getting tired of private email
> threads about this topic. Hope you don't mind.
I don't.
> > On Fri, Aug 15, 2003 at 11:25:00AM -0700, Greg KH wrote:
> > > Here's some driver core changes that do the following things:
> > > - remove struct device.name field and fix up remaining
> > > subsystems
> >
> > Could you point me to the rationale about this?
> >
> > I for one considered "everything should have a name" policy very
> > useful and extendible.
> Naming databases belong in userspace. For PCI, PnP, and USB we can
> determine the name ourselves from userspace using lspci, lspnp, and
> lsusb. Getting rid of the name field prevents us from relying on kernel
> code when we shouldn't be.
lspci at least shows only name OR number, but never both
together.
So that this tool is not very useful for name resolving in case
of problems, because you have no simple way to match your input
with its output. But don't worry, M$ does the same UI mistake and
this can be easily fixed.
But this shifting is a good reason. This also helps the "product
and company changing names" disease ;-)
> Hey, we're saving kernel memory, and this is a problem? :)
;-)
> Hope this helps explain things.
Explains exactly what I liked to know.
Many thanks and regards
Ingo Oeser