2005-12-03 17:24:21

by Kay Sievers

[permalink] [raw]
Subject: ide: MODALIAS support for autoloading of ide-cd, ide-disk, ...

This applies on top of the changes currently in the -mm tree, which
rename ".hotplug" to ".uevent". I don't have any IDE hardware left
around here, so I've tested it only with a PCMCIA CF adapter. :)

Thanks,
Kay



From: Kay Sievers <[email protected]>

IDE: MODALIAS support for autoloading of ide-cd, ide-disk, ...

Add MODULE_ALIAS to IDE midlayer modules to autoload them depending
on the probed media type.

$ modinfo ide-disk
filename: /lib/modules/2.6.15-rc4-g1b0997f5/kernel/drivers/ide/ide-disk.ko
description: ATA DISK Driver
alias: ide:*m-disk*
license: GPL
...

$ modprobe -vn ide:m-disk
insmod /lib/modules/2.6.15-rc4-g1b0997f5/kernel/drivers/ide/ide-disk.ko

$ cat /sys/bus/ide/devices/0.0/modalias
ide:m-disk

It also adds attributes to the IDE device:
$ tree /sys/bus/ide/devices/0.0/
/sys/bus/ide/devices/0.0/
|-- bus -> ../../../../../../../bus/ide
|-- drivename
|-- media
|-- modalias
|-- power
| |-- state
| `-- wakeup
`-- uevent

$ cat /sys/bus/ide/devices/0.0/{modalias,drivename,media}
ide:m-disk
hda
disk

Signed-off-by: Kay Sievers <[email protected]>
---
diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 9455e42..24e3e64 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -3516,6 +3516,7 @@ static int __init ide_cdrom_init(void)
return driver_register(&ide_cdrom_driver.gen_driver);
}

+MODULE_ALIAS("ide:*m-cdrom*");
module_init(ide_cdrom_init);
module_exit(ide_cdrom_exit);
MODULE_LICENSE("GPL");
diff --git a/drivers/ide/ide-disk.c b/drivers/ide/ide-disk.c
index f4e3d35..06fb261 100644
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -1271,6 +1271,7 @@ static int __init idedisk_init(void)
return driver_register(&idedisk_driver.gen_driver);
}

+MODULE_ALIAS("ide:*m-disk*");
module_init(idedisk_init);
module_exit(idedisk_exit);
MODULE_LICENSE("GPL");
diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 9e293c8..fba3fff 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -2197,6 +2197,7 @@ static int __init idefloppy_init(void)
return driver_register(&idefloppy_driver.gen_driver);
}

+MODULE_ALIAS("ide:*m-floppy*");
module_init(idefloppy_init);
module_exit(idefloppy_exit);
MODULE_LICENSE("GPL");
diff --git a/drivers/ide/ide-generic.c b/drivers/ide/ide-generic.c
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 7d7944e..a621548 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -4947,6 +4947,7 @@ out:
return error;
}

+MODULE_ALIAS("ide:*m-tape*");
module_init(idetape_init);
module_exit(idetape_exit);
MODULE_ALIAS_CHARDEV_MAJOR(IDETAPE_MAJOR);
diff --git a/drivers/ide/ide.c b/drivers/ide/ide.c
index 8af179b..96d39a1 100644
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -1904,9 +1904,73 @@ static int ide_bus_match(struct device *
return 1;
}

+static char *media_string(ide_drive_t *drive)
+{
+ switch (drive->media) {
+ case ide_disk:
+ return "disk";
+ case ide_cdrom:
+ return "cdrom";
+ case ide_tape:
+ return "tape";
+ case ide_floppy:
+ return "floppy";
+ default:
+ return "UNKNOWN";
+ }
+}
+
+static ssize_t media_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ ide_drive_t *drive = dev->driver_data;
+ return sprintf(buf, "%s\n", media_string(drive));
+}
+
+static ssize_t drivename_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ ide_drive_t *drive = dev->driver_data;
+ return sprintf(buf, "%s\n", drive->name);
+}
+
+static ssize_t modalias_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ ide_drive_t *drive = dev->driver_data;
+ return sprintf(buf, "ide:m-%s\n", media_string(drive));
+}
+
+static struct device_attribute ide_dev_attrs[] = {
+ __ATTR_RO(media),
+ __ATTR_RO(drivename),
+ __ATTR_RO(modalias),
+ __ATTR_NULL
+};
+
+static int ide_uevent(struct device *dev, char **envp, int num_envp,
+ char *buffer, int buffer_size)
+{
+ ide_drive_t *drive = dev->driver_data;
+ int i = 0;
+ int length = 0;
+
+ add_uevent_var(envp, num_envp, &i, buffer, buffer_size, &length,
+ "MEDIA=%s", media_string(drive));
+
+ add_uevent_var(envp, num_envp, &i, buffer, buffer_size, &length,
+ "DRIVENAME=%s", drive->name);
+
+ add_uevent_var(envp, num_envp, &i, buffer, buffer_size, &length,
+ "MODALIAS=ide:m-%s",
+ media_string(drive));
+
+ envp[i] = NULL;
+ return 0;
+}
+
struct bus_type ide_bus_type = {
.name = "ide",
.match = ide_bus_match,
+ .uevent = ide_uevent,
+ .dev_attrs = ide_dev_attrs,
.suspend = generic_ide_suspend,
.resume = generic_ide_resume,
};



Subject: Re: ide: MODALIAS support for autoloading of ide-cd, ide-disk, ...

Hi,

Looks fine but what about ide-scsi driver and ide_optical media?

Bartlomiej

On 12/3/05, Kay Sievers <[email protected]> wrote:
> This applies on top of the changes currently in the -mm tree, which
> rename ".hotplug" to ".uevent". I don't have any IDE hardware left
> around here, so I've tested it only with a PCMCIA CF adapter. :)
>
> Thanks,
> Kay
>
>
>
> From: Kay Sievers <[email protected]>
>
> IDE: MODALIAS support for autoloading of ide-cd, ide-disk, ...
>
> Add MODULE_ALIAS to IDE midlayer modules to autoload them depending
> on the probed media type.
>
> $ modinfo ide-disk
> filename: /lib/modules/2.6.15-rc4-g1b0997f5/kernel/drivers/ide/ide-disk.ko
> description: ATA DISK Driver
> alias: ide:*m-disk*
> license: GPL
> ...
>
> $ modprobe -vn ide:m-disk
> insmod /lib/modules/2.6.15-rc4-g1b0997f5/kernel/drivers/ide/ide-disk.ko
>
> $ cat /sys/bus/ide/devices/0.0/modalias
> ide:m-disk
>
> It also adds attributes to the IDE device:
> $ tree /sys/bus/ide/devices/0.0/
> /sys/bus/ide/devices/0.0/
> |-- bus -> ../../../../../../../bus/ide
> |-- drivename
> |-- media
> |-- modalias
> |-- power
> | |-- state
> | `-- wakeup
> `-- uevent
>
> $ cat /sys/bus/ide/devices/0.0/{modalias,drivename,media}
> ide:m-disk
> hda
> disk
>
> Signed-off-by: Kay Sievers <[email protected]>
> ---
> diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
> index 9455e42..24e3e64 100644
> --- a/drivers/ide/ide-cd.c
> +++ b/drivers/ide/ide-cd.c
> @@ -3516,6 +3516,7 @@ static int __init ide_cdrom_init(void)
> return driver_register(&ide_cdrom_driver.gen_driver);
> }
>
> +MODULE_ALIAS("ide:*m-cdrom*");
> module_init(ide_cdrom_init);
> module_exit(ide_cdrom_exit);
> MODULE_LICENSE("GPL");
> diff --git a/drivers/ide/ide-disk.c b/drivers/ide/ide-disk.c
> index f4e3d35..06fb261 100644
> --- a/drivers/ide/ide-disk.c
> +++ b/drivers/ide/ide-disk.c
> @@ -1271,6 +1271,7 @@ static int __init idedisk_init(void)
> return driver_register(&idedisk_driver.gen_driver);
> }
>
> +MODULE_ALIAS("ide:*m-disk*");
> module_init(idedisk_init);
> module_exit(idedisk_exit);
> MODULE_LICENSE("GPL");
> diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
> index 9e293c8..fba3fff 100644
> --- a/drivers/ide/ide-floppy.c
> +++ b/drivers/ide/ide-floppy.c
> @@ -2197,6 +2197,7 @@ static int __init idefloppy_init(void)
> return driver_register(&idefloppy_driver.gen_driver);
> }
>
> +MODULE_ALIAS("ide:*m-floppy*");
> module_init(idefloppy_init);
> module_exit(idefloppy_exit);
> MODULE_LICENSE("GPL");
> diff --git a/drivers/ide/ide-generic.c b/drivers/ide/ide-generic.c
> diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
> index 7d7944e..a621548 100644
> --- a/drivers/ide/ide-tape.c
> +++ b/drivers/ide/ide-tape.c
> @@ -4947,6 +4947,7 @@ out:
> return error;
> }
>
> +MODULE_ALIAS("ide:*m-tape*");
> module_init(idetape_init);
> module_exit(idetape_exit);
> MODULE_ALIAS_CHARDEV_MAJOR(IDETAPE_MAJOR);
> diff --git a/drivers/ide/ide.c b/drivers/ide/ide.c
> index 8af179b..96d39a1 100644
> --- a/drivers/ide/ide.c
> +++ b/drivers/ide/ide.c
> @@ -1904,9 +1904,73 @@ static int ide_bus_match(struct device *
> return 1;
> }
>
> +static char *media_string(ide_drive_t *drive)
> +{
> + switch (drive->media) {
> + case ide_disk:
> + return "disk";
> + case ide_cdrom:
> + return "cdrom";
> + case ide_tape:
> + return "tape";
> + case ide_floppy:
> + return "floppy";
> + default:
> + return "UNKNOWN";
> + }
> +}
> +
> +static ssize_t media_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + ide_drive_t *drive = dev->driver_data;
> + return sprintf(buf, "%s\n", media_string(drive));
> +}
> +
> +static ssize_t drivename_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + ide_drive_t *drive = dev->driver_data;
> + return sprintf(buf, "%s\n", drive->name);
> +}
> +
> +static ssize_t modalias_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + ide_drive_t *drive = dev->driver_data;
> + return sprintf(buf, "ide:m-%s\n", media_string(drive));
> +}
> +
> +static struct device_attribute ide_dev_attrs[] = {
> + __ATTR_RO(media),
> + __ATTR_RO(drivename),
> + __ATTR_RO(modalias),
> + __ATTR_NULL
> +};
> +
> +static int ide_uevent(struct device *dev, char **envp, int num_envp,
> + char *buffer, int buffer_size)
> +{
> + ide_drive_t *drive = dev->driver_data;
> + int i = 0;
> + int length = 0;
> +
> + add_uevent_var(envp, num_envp, &i, buffer, buffer_size, &length,
> + "MEDIA=%s", media_string(drive));
> +
> + add_uevent_var(envp, num_envp, &i, buffer, buffer_size, &length,
> + "DRIVENAME=%s", drive->name);
> +
> + add_uevent_var(envp, num_envp, &i, buffer, buffer_size, &length,
> + "MODALIAS=ide:m-%s",
> + media_string(drive));
> +
> + envp[i] = NULL;
> + return 0;
> +}
> +
> struct bus_type ide_bus_type = {
> .name = "ide",
> .match = ide_bus_match,
> + .uevent = ide_uevent,
> + .dev_attrs = ide_dev_attrs,
> .suspend = generic_ide_suspend,
> .resume = generic_ide_resume,
> };

2005-12-05 19:31:43

by Marco d'Itri

[permalink] [raw]
Subject: Re: ide: MODALIAS support for autoloading of ide-cd, ide-disk, ...

On Dec 05, Bartlomiej Zolnierkiewicz <[email protected]> wrote:

> Looks fine but what about ide-scsi driver and ide_optical media?
The Debian hotplug script, which does the same thing, does not know
about these modules and nobody ever complained, so if some support is
needed it can be added later.

(And ide-scsi is dead anyway...)

--
ciao,
Marco

Subject: Re: ide: MODALIAS support for autoloading of ide-cd, ide-disk, ...

On 12/5/05, Marco d'Itri <[email protected]> wrote:
> On Dec 05, Bartlomiej Zolnierkiewicz <[email protected]> wrote:
>
> > Looks fine but what about ide-scsi driver and ide_optical media?
> The Debian hotplug script, which does the same thing, does not know
> about these modules and nobody ever complained, so if some support is

Does the hotplug script export kernel sysfs attributes?

> needed it can be added later.

The problem is that you get wrong data for "modalias" sysfs attribute
for ide-scsi module (because "modalias" is based on the media type).

Until this is issue solved I can't accept this patch.

> (And ide-scsi is dead anyway...)

ide-scsi is the only way to support some devices currently

Bartlomiej

2005-12-05 23:33:21

by Kay Sievers

[permalink] [raw]
Subject: Re: ide: MODALIAS support for autoloading of ide-cd, ide-disk, ...

On Tue, Dec 06, 2005 at 12:18:03AM +0100, Bartlomiej Zolnierkiewicz wrote:
> On 12/5/05, Marco d'Itri <[email protected]> wrote:
> > On Dec 05, Bartlomiej Zolnierkiewicz <[email protected]> wrote:
> >
> > > Looks fine but what about ide-scsi driver and ide_optical media?
> > The Debian hotplug script, which does the same thing, does not know
> > about these modules and nobody ever complained, so if some support is
>
> Does the hotplug script export kernel sysfs attributes?

You mean if the hotplug script uses sysfs? It looks up the
data in /proc (which is ugly, cause you have to loop until
the /proc file arrives).

> > needed it can be added later.
>
> The problem is that you get wrong data for "modalias" sysfs attribute
> for ide-scsi module (because "modalias" is based on the media type).
>
> Until this is issue solved I can't accept this patch.

The correct way is probably to have the same MODULE_ALIAS() in
ide-scsi and we just control the loaded module with the module-init-tools
blacklist entry. Does that make sense to you?

Thanks,
Kay

Subject: Re: ide: MODALIAS support for autoloading of ide-cd, ide-disk, ...

And after more careful inspection....

$ tree /sys/bus/ide/devices/0.0/
/sys/bus/ide/devices/0.0/
|-- block -> ../../../../../block/hda
|-- bus -> ../../../../../bus/ide
|-- driver -> ../../../../../bus/ide/drivers/ide-disk
|-- power
| |-- state
| `-- wakeup
`-- uevent

this is with the standard kernel without this patch.

Key, could you please explain what this patch actually *does*
(besides adding MODALIAS support) to such sysfs dummy as me?

Thanks,
Bartlomiej

On 12/6/05, Bartlomiej Zolnierkiewicz <[email protected]> wrote:
> On 12/5/05, Marco d'Itri <[email protected]> wrote:
> > On Dec 05, Bartlomiej Zolnierkiewicz <[email protected]> wrote:
> >
> > > Looks fine but what about ide-scsi driver and ide_optical media?
> > The Debian hotplug script, which does the same thing, does not know
> > about these modules and nobody ever complained, so if some support is
>
> Does the hotplug script export kernel sysfs attributes?
>
> > needed it can be added later.
>
> The problem is that you get wrong data for "modalias" sysfs attribute
> for ide-scsi module (because "modalias" is based on the media type).
>
> Until this is issue solved I can't accept this patch.
>
> > (And ide-scsi is dead anyway...)
>
> ide-scsi is the only way to support some devices currently
>
> Bartlomiej
>