MODULE_DEVICE_TABLE(isapnp, ...) does nothing

MODULE_DEVICE_TABLE(isapnp, ...) does nothing

[Ondrej Zary]

Hello,
just found that Linux does not automatically load "ne" module for ISA PnP 
NE2000 cards (RTL8019AS). ne.c contains this:

static struct isapnp_device_id isapnp_clone_list[] __initdata = {
        {       ISAPNP_CARD_ID('A','X','E',0x2011),
                ISAPNP_VENDOR('A','X','E'), ISAPNP_FUNCTION(0x2011),
                (long) "NetGear EA201" },
        {       ISAPNP_ANY_ID, ISAPNP_ANY_ID,
                ISAPNP_VENDOR('E','D','I'), ISAPNP_FUNCTION(0x0216),
                (long) "NN NE2000" },
        {       ISAPNP_ANY_ID, ISAPNP_ANY_ID,
                ISAPNP_VENDOR('P','N','P'), ISAPNP_FUNCTION(0x80d6),
                (long) "Generic PNP" },
        { }     /* terminate list */
};

MODULE_DEVICE_TABLE(isapnp, isapnp_clone_list);

but "modinfo ne" does not show any aliases. The ne.mod.c file produced during 
build does not contain aliases too. The problem is that 
scripts/mod/file2alias.c simply ignores isapnp.

ne.c is not the only file using MODULE_DEVICE_TABLE(isapnp, ...):
drivers/pcmcia/i82365.c
drivers/net/3c515.c
drivers/net/ne.c
drivers/net/smc-ultra.c
drivers/isdn/hisax/hisax_fspcipnp.c
drivers/media/radio/radio-sf16fmi.c
drivers/scsi/aha1542.c
drivers/scsi/aha152x.c
drivers/scsi/sym53c416.c
drivers/scsi/g_NCR5380.c
sound/oss/ad1848.c

How is (was) this supposed to work?

-- 
Ondrej Zary

Re: MODULE_DEVICE_TABLE(isapnp, ...) does nothing

[Rusty Russell]

On Mon, 23 Nov 2009 07:31:57 am Ondrej Zary wrote:
> The problem is that 
> scripts/mod/file2alias.c simply ignores isapnp.

AFAICT it always has, and noone has complained until now.  Perhaps something
was still reading /lib/modules/`uname -r`/modules.isapnpmap?

Two things are required:
1) Modify file2alias to add aliases for isapnp.  This is pretty easy.
2) Expose the isapnp devices in sysfs where udev will match them up
   (/sys/bus/isa/devices/<dev>/modalias I guess?)

The first patches would look something like these.  We could mangle the
PNP names a-la ISAPNP_VENDOR/ISAPNP_DEVICE, but as long as the published
modalias matches the file2alias result, it doesn't matter.

Jaroslav?  Greg?  Patches for sysfs welcome...

Thanks,
Rusty.
PS.  Jon, perhaps the new decade is a good time to kill modules.*map?

isapnp: move definitions to mod_devicetable.h so file2alias can reach them.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/include/linux/isapnp.h b/include/linux/isapnp.h
--- a/include/linux/isapnp.h
+++ b/include/linux/isapnp.h
@@ -43,10 +43,10 @@
  */
 
 #ifdef __KERNEL__
+#include <linux/mod_devicetable.h>
 
 #define DEVICE_COUNT_COMPATIBLE 4
 
-#define ISAPNP_ANY_ID		0xffff
 #define ISAPNP_CARD_DEVS	8
 
 #define ISAPNP_CARD_ID(_va, _vb, _vc, _device) \
@@ -74,12 +74,6 @@ struct isapnp_card_id {
 #define ISAPNP_DEVICE_SINGLE_END \
 		.card_vendor = 0, .card_device = 0
 
-struct isapnp_device_id {
-	unsigned short card_vendor, card_device;
-	unsigned short vendor, function;
-	unsigned long driver_data;	/* data private to the driver */
-};
-
 #if defined(CONFIG_ISAPNP) || (defined(CONFIG_ISAPNP_MODULE) && defined(MODULE))
 
 #define __ISAPNP__
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -474,4 +474,11 @@ struct platform_device_id {
 			__attribute__((aligned(sizeof(kernel_ulong_t))));
 };
 
+#define ISAPNP_ANY_ID		0xffff
+struct isapnp_device_id {
+	unsigned short card_vendor, card_device;
+	unsigned short vendor, function;
+	kernel_ulong_t driver_data;	/* data private to the driver */
+};
+
 #endif /* LINUX_MOD_DEVICETABLE_H */


isapnp: put aliases in .modinfo so modinfo can find them.

Once isa devices publish a modalias field, I think udev should "Just Work".

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -727,6 +727,19 @@ static int do_platform_entry(const char 
 	return 1;
 }
 
+/* Format is: isa:cvNcdNvNfN */
+static int do_isapnp_entry(const char *filename,
+			   struct isapnp_device_id *id, char *alias)
+{
+	strcpy(alias, "isa:");
+	ADD(alias, "cv", id->card_vendor != ISAPNP_ANY_ID, id->card_vendor);
+	ADD(alias, "cd", id->card_device != ISAPNP_ANY_ID, id->card_device);
+	ADD(alias, "v", id->vendor != ISAPNP_ANY_ID, id->vendor);
+	ADD(alias, "f", id->function, id->function);
+	add_wildcard(alias);
+	return 1;
+}
+
 /* Ignore any prefix, eg. some architectures prepend _ */
 static inline int sym_is(const char *symbol, const char *name)
 {
@@ -874,6 +887,10 @@ void handle_moddevtable(struct module *m
 		do_table(symval, sym->st_size,
 			 sizeof(struct platform_device_id), "platform",
 			 do_platform_entry, mod);
+	else if (sym_is(symname, "__mod_isapnp_device_table"))
+		do_table(symval, sym->st_size,
+			 sizeof(struct isapnp_device_id), "isa",
+			 do_isapnp_entry, mod);
 	free(zeros);
 }
 

Re: MODULE_DEVICE_TABLE(isapnp, ...) does nothing

[Takashi Iwai]

At Mon, 23 Nov 2009 13:59:53 +1030,
Rusty Russell wrote:
> 
> On Mon, 23 Nov 2009 07:31:57 am Ondrej Zary wrote:
> > The problem is that 
> > scripts/mod/file2alias.c simply ignores isapnp.
> 
> AFAICT it always has, and noone has complained until now.  Perhaps something
> was still reading /lib/modules/`uname -r`/modules.isapnpmap?

I don't think modules.isapnpmap is needed again.
The question came from the fact that the isapnp device wasn't loaded
automatically.

But, I thought Kay already added isapnp support sometime ago, but it
didn't seem to get in...  Kay?


> Two things are required:
> 1) Modify file2alias to add aliases for isapnp.  This is pretty easy.
> 2) Expose the isapnp devices in sysfs where udev will match them up
>    (/sys/bus/isa/devices/<dev>/modalias I guess?)

There is non-pnp ISA bus, so I'm afraid "isa" may conflict.
I suppose "isapnp" would be a safer choice.


thanks,

Takashi

> The first patches would look something like these.  We could mangle the
> PNP names a-la ISAPNP_VENDOR/ISAPNP_DEVICE, but as long as the published
> modalias matches the file2alias result, it doesn't matter.
> 
> Jaroslav?  Greg?  Patches for sysfs welcome...
> 
> Thanks,
> Rusty.
> PS.  Jon, perhaps the new decade is a good time to kill modules.*map?
> 
> isapnp: move definitions to mod_devicetable.h so file2alias can reach them.
> 
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> 
> diff --git a/include/linux/isapnp.h b/include/linux/isapnp.h
> --- a/include/linux/isapnp.h
> +++ b/include/linux/isapnp.h
> @@ -43,10 +43,10 @@
>   */
>  
>  #ifdef __KERNEL__
> +#include <linux/mod_devicetable.h>
>  
>  #define DEVICE_COUNT_COMPATIBLE 4
>  
> -#define ISAPNP_ANY_ID		0xffff
>  #define ISAPNP_CARD_DEVS	8
>  
>  #define ISAPNP_CARD_ID(_va, _vb, _vc, _device) \
> @@ -74,12 +74,6 @@ struct isapnp_card_id {
>  #define ISAPNP_DEVICE_SINGLE_END \
>  		.card_vendor = 0, .card_device = 0
>  
> -struct isapnp_device_id {
> -	unsigned short card_vendor, card_device;
> -	unsigned short vendor, function;
> -	unsigned long driver_data;	/* data private to the driver */
> -};
> -
>  #if defined(CONFIG_ISAPNP) || (defined(CONFIG_ISAPNP_MODULE) && defined(MODULE))
>  
>  #define __ISAPNP__
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -474,4 +474,11 @@ struct platform_device_id {
>  			__attribute__((aligned(sizeof(kernel_ulong_t))));
>  };
>  
> +#define ISAPNP_ANY_ID		0xffff
> +struct isapnp_device_id {
> +	unsigned short card_vendor, card_device;
> +	unsigned short vendor, function;
> +	kernel_ulong_t driver_data;	/* data private to the driver */
> +};
> +
>  #endif /* LINUX_MOD_DEVICETABLE_H */
> 
> 
> isapnp: put aliases in .modinfo so modinfo can find them.
> 
> Once isa devices publish a modalias field, I think udev should "Just Work".
> 
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> 
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -727,6 +727,19 @@ static int do_platform_entry(const char 
>  	return 1;
>  }
>  
> +/* Format is: isa:cvNcdNvNfN */
> +static int do_isapnp_entry(const char *filename,
> +			   struct isapnp_device_id *id, char *alias)
> +{
> +	strcpy(alias, "isa:");
> +	ADD(alias, "cv", id->card_vendor != ISAPNP_ANY_ID, id->card_vendor);
> +	ADD(alias, "cd", id->card_device != ISAPNP_ANY_ID, id->card_device);
> +	ADD(alias, "v", id->vendor != ISAPNP_ANY_ID, id->vendor);
> +	ADD(alias, "f", id->function, id->function);
> +	add_wildcard(alias);
> +	return 1;
> +}
> +
>  /* Ignore any prefix, eg. some architectures prepend _ */
>  static inline int sym_is(const char *symbol, const char *name)
>  {
> @@ -874,6 +887,10 @@ void handle_moddevtable(struct module *m
>  		do_table(symval, sym->st_size,
>  			 sizeof(struct platform_device_id), "platform",
>  			 do_platform_entry, mod);
> +	else if (sym_is(symname, "__mod_isapnp_device_table"))
> +		do_table(symval, sym->st_size,
> +			 sizeof(struct isapnp_device_id), "isa",
> +			 do_isapnp_entry, mod);
>  	free(zeros);
>  }
>  
> 

Re: MODULE_DEVICE_TABLE(isapnp, ...) does nothing

[Kay Sievers]

On Mon, Nov 23, 2009 at 09:40, Takashi Iwai <tiwai@suse.de> wrote:
> At Mon, 23 Nov 2009 13:59:53 +1030,
> Rusty Russell wrote:
>>
>> On Mon, 23 Nov 2009 07:31:57 am Ondrej Zary wrote:
>> > The problem is that
>> > scripts/mod/file2alias.c simply ignores isapnp.
>>
>> AFAICT it always has, and noone has complained until now.  Perhaps something
>> was still reading /lib/modules/`uname -r`/modules.isapnpmap?
>
> I don't think modules.isapnpmap is needed again.
> The question came from the fact that the isapnp device wasn't loaded
> automatically.

All the map files are not uses for anything these days. Ideally,
depmod should just stop creating these dead files.

> But, I thought Kay already added isapnp support sometime ago, but it
> didn't seem to get in...  Kay?

No, we don't even have proper modaliases for the pnp bus
(/sys/bus/pnp) and the alias string for pnp in the modules have a
broken and unfixable format. PNP autoloading is all handled by the
acpi modaliases these days. This is the first time in years I hear
anybody asking for hotplug setups of plain old pnp devices. :)

Thanks,
Kay

Re: MODULE_DEVICE_TABLE(isapnp, ...) does nothing

[Ondrej Zary]

On Monday 23 November 2009, Kay Sievers wrote:
> On Mon, Nov 23, 2009 at 09:40, Takashi Iwai <tiwai@suse.de> wrote:
> > At Mon, 23 Nov 2009 13:59:53 +1030,
> >
> > Rusty Russell wrote:
> >> On Mon, 23 Nov 2009 07:31:57 am Ondrej Zary wrote:
> >> > The problem is that
> >> > scripts/mod/file2alias.c simply ignores isapnp.
> >>
> >> AFAICT it always has, and noone has complained until now.  Perhaps
> >> something was still reading /lib/modules/`uname -r`/modules.isapnpmap?
> >
> > I don't think modules.isapnpmap is needed again.
> > The question came from the fact that the isapnp device wasn't loaded
> > automatically.
>
> All the map files are not uses for anything these days. Ideally,
> depmod should just stop creating these dead files.
>
> > But, I thought Kay already added isapnp support sometime ago, but it
> > didn't seem to get in...  Kay?
>
> No, we don't even have proper modaliases for the pnp bus
> (/sys/bus/pnp) and the alias string for pnp in the modules have a
> broken and unfixable format. PNP autoloading is all handled by the
> acpi modaliases these days. This is the first time in years I hear
> anybody asking for hotplug setups of plain old pnp devices. :)


Added these two lines to modules.alias (and deleted modules.alias.bin):
alias acpi*:RTL8019:* ne
alias pnp:dRTL8019* ne

and it works. This works too:
alias acpi*:PNP80d6:* ne
alias pnp:dPNP80d6* ne

The card has two IDs (as some PnP devices do) and both work.

-- 
Ondrej Zary

Re: MODULE_DEVICE_TABLE(isapnp, ...) does nothing

[Kay Sievers]

On Tue, Nov 24, 2009 at 08:29, Ondrej Zary <linux@rainbow-software.org> wrote:
> On Monday 23 November 2009, Kay Sievers wrote:
>> On Mon, Nov 23, 2009 at 09:40, Takashi Iwai <tiwai@suse.de> wrote:
>> > At Mon, 23 Nov 2009 13:59:53 +1030,
>> >
>> > Rusty Russell wrote:
>> >> On Mon, 23 Nov 2009 07:31:57 am Ondrej Zary wrote:
>> >> > The problem is that
>> >> > scripts/mod/file2alias.c simply ignores isapnp.
>> >>
>> >> AFAICT it always has, and noone has complained until now.  Perhaps
>> >> something was still reading /lib/modules/`uname -r`/modules.isapnpmap?
>> >
>> > I don't think modules.isapnpmap is needed again.
>> > The question came from the fact that the isapnp device wasn't loaded
>> > automatically.
>>
>> All the map files are not uses for anything these days. Ideally,
>> depmod should just stop creating these dead files.
>>
>> > But, I thought Kay already added isapnp support sometime ago, but it
>> > didn't seem to get in...  Kay?
>>
>> No, we don't even have proper modaliases for the pnp bus
>> (/sys/bus/pnp) and the alias string for pnp in the modules have a
>> broken and unfixable format. PNP autoloading is all handled by the
>> acpi modaliases these days. This is the first time in years I hear
>> anybody asking for hotplug setups of plain old pnp devices. :)
>
>
> Added these two lines to modules.alias (and deleted modules.alias.bin):
> alias acpi*:RTL8019:* ne
> alias pnp:dRTL8019* ne
>
> and it works. This works too:
> alias acpi*:PNP80d6:* ne
> alias pnp:dPNP80d6* ne

Oh, your box has acpi mapped pnp aliases for this device? You can see
your ID string somewhere in this list?
  grep . /sys/bus/acpi/devices/*/modalias

If not, is it in the pnp bus list?
  grep . /sys/bus/pnp/devices/*/id

Thanks,
Kay

Re: MODULE_DEVICE_TABLE(isapnp, ...) does nothing

[Ondrej Zary]

On Tuesday 24 November 2009 09:57:28 Kay Sievers wrote:
> On Tue, Nov 24, 2009 at 08:29, Ondrej Zary <linux@rainbow-software.org> 
wrote:
> > On Monday 23 November 2009, Kay Sievers wrote:
> >> On Mon, Nov 23, 2009 at 09:40, Takashi Iwai <tiwai@suse.de> wrote:
> >> > At Mon, 23 Nov 2009 13:59:53 +1030,
> >> >
> >> > Rusty Russell wrote:
> >> >> On Mon, 23 Nov 2009 07:31:57 am Ondrej Zary wrote:
> >> >> > The problem is that
> >> >> > scripts/mod/file2alias.c simply ignores isapnp.
> >> >>
> >> >> AFAICT it always has, and noone has complained until now.  Perhaps
> >> >> something was still reading /lib/modules/`uname
> >> >> -r`/modules.isapnpmap?
> >> >
> >> > I don't think modules.isapnpmap is needed again.
> >> > The question came from the fact that the isapnp device wasn't loaded
> >> > automatically.
> >>
> >> All the map files are not uses for anything these days. Ideally,
> >> depmod should just stop creating these dead files.
> >>
> >> > But, I thought Kay already added isapnp support sometime ago, but it
> >> > didn't seem to get in...  Kay?
> >>
> >> No, we don't even have proper modaliases for the pnp bus
> >> (/sys/bus/pnp) and the alias string for pnp in the modules have a
> >> broken and unfixable format. PNP autoloading is all handled by the
> >> acpi modaliases these days. This is the first time in years I hear
> >> anybody asking for hotplug setups of plain old pnp devices. :)
> >
> > Added these two lines to modules.alias (and deleted modules.alias.bin):
> > alias acpi*:RTL8019:* ne
> > alias pnp:dRTL8019* ne
> >
> > and it works. This works too:
> > alias acpi*:PNP80d6:* ne
> > alias pnp:dPNP80d6* ne
>
> Oh, your box has acpi mapped pnp aliases for this device? You can see
> your ID string somewhere in this list?
>   grep . /sys/bus/acpi/devices/*/modalias
>
> If not, is it in the pnp bus list?
>   grep . /sys/bus/pnp/devices/*/id

It's listen in the pnp bus only:

$ grep -i 80d6 /sys/bus/acpi/devices/*/modalias
$ grep -i 80d6 /sys/bus/pnp/devices/*/id
/sys/bus/pnp/devices/01:01.00/id:PNP80d6
$ cat /sys/bus/pnp/devices/01\:01.00/id
RTL8019
PNP80d6

So the acpi alias was useless (I just copied another entry). This alone works:
alias pnp:dPNP80d6* ne

-- 
Ondrej Zary

Re: MODULE_DEVICE_TABLE(isapnp, ...) does nothing

[Kay Sievers]

On Tue, Nov 24, 2009 at 22:23, Ondrej Zary <linux@rainbow-software.org> wrote:
> On Tuesday 24 November 2009 09:57:28 Kay Sievers wrote:

>> Oh, your box has acpi mapped pnp aliases for this device? You can see
>> your ID string somewhere in this list?
>>   grep . /sys/bus/acpi/devices/*/modalias
>>
>> If not, is it in the pnp bus list?
>>   grep . /sys/bus/pnp/devices/*/id
>
> It's listen in the pnp bus only:
>
> $ grep -i 80d6 /sys/bus/acpi/devices/*/modalias
> $ grep -i 80d6 /sys/bus/pnp/devices/*/id
> /sys/bus/pnp/devices/01:01.00/id:PNP80d6
> $ cat /sys/bus/pnp/devices/01\:01.00/id
> RTL8019
> PNP80d6
>
> So the acpi alias was useless (I just copied another entry). This alone works:
> alias pnp:dPNP80d6* ne

I see. Plain upstream udev would not load anything here, as the pnp
bus has no modalias support because of the non-working aliases. We
rely on acpi mapping all the pnp entries. Your system probably runs
the shell script which iterates over the "id" file and calls modprobe
for all it finds in there.

Kay

Re: MODULE_DEVICE_TABLE(isapnp, ...) does nothing

[Ondrej Zary]

On Wednesday 25 November 2009, Kay Sievers wrote:
> On Tue, Nov 24, 2009 at 22:23, Ondrej Zary <linux@rainbow-software.org> 
wrote:
> > On Tuesday 24 November 2009 09:57:28 Kay Sievers wrote:
> >> Oh, your box has acpi mapped pnp aliases for this device? You can see
> >> your ID string somewhere in this list?
> >>   grep . /sys/bus/acpi/devices/*/modalias
> >>
> >> If not, is it in the pnp bus list?
> >>   grep . /sys/bus/pnp/devices/*/id
> >
> > It's listen in the pnp bus only:
> >
> > $ grep -i 80d6 /sys/bus/acpi/devices/*/modalias
> > $ grep -i 80d6 /sys/bus/pnp/devices/*/id
> > /sys/bus/pnp/devices/01:01.00/id:PNP80d6
> > $ cat /sys/bus/pnp/devices/01\:01.00/id
> > RTL8019
> > PNP80d6
> >
> > So the acpi alias was useless (I just copied another entry). This alone
> > works: alias pnp:dPNP80d6* ne
>
> I see. Plain upstream udev would not load anything here, as the pnp
> bus has no modalias support because of the non-working aliases. We
> rely on acpi mapping all the pnp entries. Your system probably runs
> the shell script which iterates over the "id" file and calls modprobe
> for all it finds in there.

It's Debian Squeeze.

-- 
Ondrej Zary

Re: MODULE_DEVICE_TABLE(isapnp, ...) does nothing

[Matthieu CASTET]

Hi,

Kay Sievers <kay.sievers <at> vrfy.org> writes:
> 
> No, we don't even have proper modaliases for the pnp bus
> (/sys/bus/pnp) and the alias string for pnp in the modules have a
> broken and unfixable format. PNP autoloading is all handled by the
> acpi modaliases these days. This is the first time in years I hear
> anybody asking for hotplug setups of plain old pnp devices. :)
> 
The naming is really bad. This as nothing to do with acpi : pnp and pnp_cards
entries are duplicated to another naming. And this should work on system 
without
acpi.
This should have been named to pnp2 or something like that.

Now I don't see why isapnp_device_id and pnp_card_device_id can't be merged
together.

For example most of alsa isa card use pnp_card_device_id and not 
isapnp_device_id.

Matthieu

Re: MODULE_DEVICE_TABLE(isapnp, ...) does nothing

[Kay Sievers]

On Wed, Nov 25, 2009 at 15:42, Matthieu CASTET <castet.matthieu@free.fr> wrote:
> Kay Sievers <kay.sievers <at> vrfy.org> writes:
>>
>> No, we don't even have proper modaliases for the pnp bus
>> (/sys/bus/pnp) and the alias string for pnp in the modules have a
>> broken and unfixable format. PNP autoloading is all handled by the
>> acpi modaliases these days. This is the first time in years I hear
>> anybody asking for hotplug setups of plain old pnp devices. :)
>>
> The naming is really bad. This as nothing to do with acpi : pnp and pnp_cards
> entries are duplicated to another naming. And this should work on system
> without
> acpi.
> This should have been named to pnp2 or something like that.

I don' think there is anything like a second version, it's just that
acpi handles the same pnp ids natively, so we added (non-broken)
matches for it, which works properly on almost all current boxes. If
someone wants to finally fix pnp and get proper modalias working, the
acpi alias generation in the pnp modules can be reverted without
trouble.

> Now I don't see why isapnp_device_id and pnp_card_device_id can't be merged
> together.
>
> For example most of alsa isa card use pnp_card_device_id and not
> isapnp_device_id.

It's just that nobody really cared, I guess. I wouldn't be surprised,
if this is just another round of "discussion" like we had so many
about this topic the past years, and nothing will happen to
pnp/isapnp. :)

Kay

Re: MODULE_DEVICE_TABLE(isapnp, ...) does nothing

[Rusty Russell]

On Mon, 23 Nov 2009 07:10:22 pm Takashi Iwai wrote:
> At Mon, 23 Nov 2009 13:59:53 +1030, Rusty Russell wrote:
> > Two things are required:
> > 1) Modify file2alias to add aliases for isapnp.  This is pretty easy.
> > 2) Expose the isapnp devices in sysfs where udev will match them up
> >    (/sys/bus/isa/devices/<dev>/modalias I guess?)
> 
> There is non-pnp ISA bus, so I'm afraid "isa" may conflict.
> I suppose "isapnp" would be a safer choice.

Without pnp, I don't think you can enumerate the ISA bus. Hence I chose
"isa:" in the hope that udev would "just work" if isapnp created the
modalias under /sys/bus/isa...

But Kay says there's a pnp bus (who knew?).  I don't know what he means
by unfixable aliases tho, since it's all in the kernel source so we can
change it at any time?

Thanks,
Rusty.

Re: MODULE_DEVICE_TABLE(isapnp, ...) does nothing

[Kay Sievers]

On Mon, Nov 23, 2009 at 22:51, Rusty Russell <rusty@rustcorp.com.au> wrote:
> On Mon, 23 Nov 2009 07:10:22 pm Takashi Iwai wrote:
>> At Mon, 23 Nov 2009 13:59:53 +1030, Rusty Russell wrote:
>> > Two things are required:
>> > 1) Modify file2alias to add aliases for isapnp.  This is pretty easy.
>> > 2) Expose the isapnp devices in sysfs where udev will match them up
>> >    (/sys/bus/isa/devices/<dev>/modalias I guess?)
>>
>> There is non-pnp ISA bus, so I'm afraid "isa" may conflict.
>> I suppose "isapnp" would be a safer choice.
>
> Without pnp, I don't think you can enumerate the ISA bus. Hence I chose
> "isa:" in the hope that udev would "just work" if isapnp created the
> modalias under /sys/bus/isa...
>
> But Kay says there's a pnp bus (who knew?).  I don't know what he means
> by unfixable aliases tho, since it's all in the kernel source so we can
> change it at any time?

Here's a bit of the background:

The aliases in the modules can only match a single value, but PNP
devices often have vendor specific IDs and usual IDs describing the
function.

Like here, where the interesting ID is only the second one of the two
for a single device:
  $ grep . /sys/bus/pnp/devices/*/id
  /sys/bus/pnp/devices/00:09/id:LEN0006
  /sys/bus/pnp/devices/00:09/id:PNP0f13

So the kernel device would need to compose a "modalias" string with
all IDs in a single line. But the in-module aliases can unfortunately
match only on a single value, like:
  alias pnp:dPNP0700* floppy

For that reason, a while ago udev switched to acpi aliases entirely,
and dropped all pnp: usage. The acpi aliases can handle multi-values
just fine with single strings like:
  $ cat /sys/bus/acpi/devices/LEN0006:00/modalias
  acpi:LEN0006:PNP0F13:

  alias acpi*:PNP0700:* floppy

Udev does not use any pnp: or /sys/bus/pnp/ values since a while. But
there might be still distros who use the (broken) pnp: aliases and
ship the (also broken) shell scripts to iterate over the sysfs devices
and call modprobe for all they find.

Thanks,
Kay

Re: MODULE_DEVICE_TABLE(isapnp, ...) does nothing

[Rusty Russell]

On Tue, 24 Nov 2009 07:23:51 pm Kay Sievers wrote:
> On Mon, Nov 23, 2009 at 22:51, Rusty Russell <rusty@rustcorp.com.au> wrote:
> > On Mon, 23 Nov 2009 07:10:22 pm Takashi Iwai wrote:
> >> At Mon, 23 Nov 2009 13:59:53 +1030, Rusty Russell wrote:
> >> > Two things are required:
> >> > 1) Modify file2alias to add aliases for isapnp.  This is pretty easy.
> >> > 2) Expose the isapnp devices in sysfs where udev will match them up
> >> >    (/sys/bus/isa/devices/<dev>/modalias I guess?)
> >>
> >> There is non-pnp ISA bus, so I'm afraid "isa" may conflict.
> >> I suppose "isapnp" would be a safer choice.
> >
> > Without pnp, I don't think you can enumerate the ISA bus. Hence I chose
> > "isa:" in the hope that udev would "just work" if isapnp created the
> > modalias under /sys/bus/isa...
> >
> > But Kay says there's a pnp bus (who knew?).  I don't know what he means
> > by unfixable aliases tho, since it's all in the kernel source so we can
> > change it at any time?
> 
> Here's a bit of the background:
> 
> The aliases in the modules can only match a single value, but PNP
> devices often have vendor specific IDs and usual IDs describing the
> function.

I'm confused.  You can have multiple aliases in a module.  Many do.

> Like here, where the interesting ID is only the second one of the two
> for a single device:
>   $ grep . /sys/bus/pnp/devices/*/id
>   /sys/bus/pnp/devices/00:09/id:LEN0006
>   /sys/bus/pnp/devices/00:09/id:PNP0f13
> 
> So the kernel device would need to compose a "modalias" string with
> all IDs in a single line. But the in-module aliases can unfortunately
> match only on a single value, like:
>   alias pnp:dPNP0700* floppy
> 
> For that reason, a while ago udev switched to acpi aliases entirely,
> and dropped all pnp: usage. The acpi aliases can handle multi-values
> just fine with single strings like:
>   $ cat /sys/bus/acpi/devices/LEN0006:00/modalias
>   acpi:LEN0006:PNP0F13:
> 
>   alias acpi*:PNP0700:* floppy

Then why didn't you change the pnp modalias format to be more like the ACPI
one?

(I know nothing about ACPI or PNP, so maybe I'm missing something fundamental).

Thanks,
Rusty.

Re: MODULE_DEVICE_TABLE(isapnp, ...) does nothing

[Kay Sievers]

On Wed, Nov 25, 2009 at 00:51, Rusty Russell <rusty@rustcorp.com.au> wrote:
> On Tue, 24 Nov 2009 07:23:51 pm Kay Sievers wrote:
>> On Mon, Nov 23, 2009 at 22:51, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> > On Mon, 23 Nov 2009 07:10:22 pm Takashi Iwai wrote:
>> >> At Mon, 23 Nov 2009 13:59:53 +1030, Rusty Russell wrote:
>> >> > Two things are required:
>> >> > 1) Modify file2alias to add aliases for isapnp.  This is pretty easy.
>> >> > 2) Expose the isapnp devices in sysfs where udev will match them up
>> >> >    (/sys/bus/isa/devices/<dev>/modalias I guess?)
>> >>
>> >> There is non-pnp ISA bus, so I'm afraid "isa" may conflict.
>> >> I suppose "isapnp" would be a safer choice.
>> >
>> > Without pnp, I don't think you can enumerate the ISA bus. Hence I chose
>> > "isa:" in the hope that udev would "just work" if isapnp created the
>> > modalias under /sys/bus/isa...
>> >
>> > But Kay says there's a pnp bus (who knew?).  I don't know what he means
>> > by unfixable aliases tho, since it's all in the kernel source so we can
>> > change it at any time?
>>
>> Here's a bit of the background:
>>
>> The aliases in the modules can only match a single value, but PNP
>> devices often have vendor specific IDs and usual IDs describing the
>> function.
>
> I'm confused.  You can have multiple aliases in a module.  Many do.

Sure, but we have only a single modalias string to match against. The
aliases need to be in a form where they can match the modalias string
containing more than a single entry. The current ones can't do that,
but they were known to be used by some systems with hacky shell
scripts or custom modprobe configs to load pnp device modules.

>> Like here, where the interesting ID is only the second one of the two
>> for a single device:
>>   $ grep . /sys/bus/pnp/devices/*/id
>>   /sys/bus/pnp/devices/00:09/id:LEN0006
>>   /sys/bus/pnp/devices/00:09/id:PNP0f13
>>
>> So the kernel device would need to compose a "modalias" string with
>> all IDs in a single line. But the in-module aliases can unfortunately
>> match only on a single value, like:
>>   alias pnp:dPNP0700* floppy
>>
>> For that reason, a while ago udev switched to acpi aliases entirely,
>> and dropped all pnp: usage. The acpi aliases can handle multi-values
>> just fine with single strings like:
>>   $ cat /sys/bus/acpi/devices/LEN0006:00/modalias
>>   acpi:LEN0006:PNP0F13:
>>
>>   alias acpi*:PNP0700:* floppy
>
> Then why didn't you change the pnp modalias format to be more like the ACPI
> one?

I'm all for doing that, but the current ones are used in old shell
script hacks and custom modprobe configs. Not sure, if that is still
something we need to care about. As mentioned, for that reason,
upstream udev dropped all that a while ago and supports only acpi
aliases today. At the moment we decide not to care about the possible
old users anymore, we could change the pnp aliases to a sane format.

> (I know nothing about ACPI or PNP, so maybe I'm missing something fundamental).

I think you don't miss anything, it's just a bit hard sometimes to get
rid of old users, even when they use the stuff in a wrong way. :)

Thanks,
Kay

Re: MODULE_DEVICE_TABLE(isapnp, ...) does nothing

[Ondrej Zary]

On Tuesday 24 November 2009 09:53:51 Kay Sievers wrote:
> On Mon, Nov 23, 2009 at 22:51, Rusty Russell <rusty@rustcorp.com.au> wrote:
> > On Mon, 23 Nov 2009 07:10:22 pm Takashi Iwai wrote:
> >> At Mon, 23 Nov 2009 13:59:53 +1030, Rusty Russell wrote:
> >> > Two things are required:
> >> > 1) Modify file2alias to add aliases for isapnp.  This is pretty easy.
> >> > 2) Expose the isapnp devices in sysfs where udev will match them up
> >> >    (/sys/bus/isa/devices/<dev>/modalias I guess?)
> >>
> >> There is non-pnp ISA bus, so I'm afraid "isa" may conflict.
> >> I suppose "isapnp" would be a safer choice.
> >
> > Without pnp, I don't think you can enumerate the ISA bus. Hence I chose
> > "isa:" in the hope that udev would "just work" if isapnp created the
> > modalias under /sys/bus/isa...
> >
> > But Kay says there's a pnp bus (who knew?).  I don't know what he means
> > by unfixable aliases tho, since it's all in the kernel source so we can
> > change it at any time?
>
> Here's a bit of the background:
>
> The aliases in the modules can only match a single value, but PNP
> devices often have vendor specific IDs and usual IDs describing the
> function.
>
> Like here, where the interesting ID is only the second one of the two
> for a single device:
>   $ grep . /sys/bus/pnp/devices/*/id
>   /sys/bus/pnp/devices/00:09/id:LEN0006
>   /sys/bus/pnp/devices/00:09/id:PNP0f13

It's the same for my RTL8019 card:
$ grep . /sys/bus/pnp/devices/*/id
/sys/bus/pnp/devices/01:01.00/id:RTL8019
/sys/bus/pnp/devices/01:01.00/id:PNP80d6

and it works with this (which could be easily generated from the current ne 
module, I think):
alias pnp:dPNP80d6* ne

This works fine too:
alias pnp:dRTL8019* ne
alias pnp:dPNP80d6* ne

> So the kernel device would need to compose a "modalias" string with
> all IDs in a single line. But the in-module aliases can unfortunately
> match only on a single value, like:
>   alias pnp:dPNP0700* floppy
>
> For that reason, a while ago udev switched to acpi aliases entirely,
> and dropped all pnp: usage. The acpi aliases can handle multi-values
> just fine with single strings like:
>   $ cat /sys/bus/acpi/devices/LEN0006:00/modalias
>   acpi:LEN0006:PNP0F13:
>
>   alias acpi*:PNP0700:* floppy
>
> Udev does not use any pnp: or /sys/bus/pnp/ values since a while. But
> there might be still distros who use the (broken) pnp: aliases and
> ship the (also broken) shell scripts to iterate over the sysfs devices
> and call modprobe for all they find.
>
> Thanks,
> Kay



-- 
Ondrej Zary

Re: MODULE_DEVICE_TABLE(isapnp, ...) does nothing

[Ondrej Zary]

On Monday 23 November 2009 04:29:53 Rusty Russell wrote:
> On Mon, 23 Nov 2009 07:31:57 am Ondrej Zary wrote:
> > The problem is that
> > scripts/mod/file2alias.c simply ignores isapnp.
>
> AFAICT it always has, and noone has complained until now.  Perhaps
> something was still reading /lib/modules/`uname -r`/modules.isapnpmap?
>
> Two things are required:
> 1) Modify file2alias to add aliases for isapnp.  This is pretty easy.
> 2) Expose the isapnp devices in sysfs where udev will match them up
>    (/sys/bus/isa/devices/<dev>/modalias I guess?)
>
> The first patches would look something like these.  We could mangle the
> PNP names a-la ISAPNP_VENDOR/ISAPNP_DEVICE, but as long as the published
> modalias matches the file2alias result, it doesn't matter.
>
> Jaroslav?  Greg?  Patches for sysfs welcome...
>
> Thanks,
> Rusty.
> PS.  Jon, perhaps the new decade is a good time to kill modules.*map?
>
> isapnp: move definitions to mod_devicetable.h so file2alias can reach them.
>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>
> diff --git a/include/linux/isapnp.h b/include/linux/isapnp.h
> --- a/include/linux/isapnp.h
> +++ b/include/linux/isapnp.h
> @@ -43,10 +43,10 @@
>   */
>
>  #ifdef __KERNEL__
> +#include <linux/mod_devicetable.h>
>
>  #define DEVICE_COUNT_COMPATIBLE 4
>
> -#define ISAPNP_ANY_ID		0xffff
>  #define ISAPNP_CARD_DEVS	8
>
>  #define ISAPNP_CARD_ID(_va, _vb, _vc, _device) \
> @@ -74,12 +74,6 @@ struct isapnp_card_id {
>  #define ISAPNP_DEVICE_SINGLE_END \
>  		.card_vendor = 0, .card_device = 0
>
> -struct isapnp_device_id {
> -	unsigned short card_vendor, card_device;
> -	unsigned short vendor, function;
> -	unsigned long driver_data;	/* data private to the driver */
> -};
> -
>  #if defined(CONFIG_ISAPNP) || (defined(CONFIG_ISAPNP_MODULE) &&
> defined(MODULE))
>
>  #define __ISAPNP__
> diff --git a/include/linux/mod_devicetable.h
> b/include/linux/mod_devicetable.h --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -474,4 +474,11 @@ struct platform_device_id {
>  			__attribute__((aligned(sizeof(kernel_ulong_t))));
>  };
>
> +#define ISAPNP_ANY_ID		0xffff
> +struct isapnp_device_id {
> +	unsigned short card_vendor, card_device;
> +	unsigned short vendor, function;
> +	kernel_ulong_t driver_data;	/* data private to the driver */
> +};
> +
>  #endif /* LINUX_MOD_DEVICETABLE_H */
>
>
> isapnp: put aliases in .modinfo so modinfo can find them.
>
> Once isa devices publish a modalias field, I think udev should "Just Work".
>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -727,6 +727,19 @@ static int do_platform_entry(const char
>  	return 1;
>  }
>
> +/* Format is: isa:cvNcdNvNfN */
> +static int do_isapnp_entry(const char *filename,
> +			   struct isapnp_device_id *id, char *alias)
> +{
> +	strcpy(alias, "isa:");
> +	ADD(alias, "cv", id->card_vendor != ISAPNP_ANY_ID, id->card_vendor);
> +	ADD(alias, "cd", id->card_device != ISAPNP_ANY_ID, id->card_device);
> +	ADD(alias, "v", id->vendor != ISAPNP_ANY_ID, id->vendor);
> +	ADD(alias, "f", id->function, id->function);
> +	add_wildcard(alias);
> +	return 1;
> +}
> +
>  /* Ignore any prefix, eg. some architectures prepend _ */
>  static inline int sym_is(const char *symbol, const char *name)
>  {
> @@ -874,6 +887,10 @@ void handle_moddevtable(struct module *m
>  		do_table(symval, sym->st_size,
>  			 sizeof(struct platform_device_id), "platform",
>  			 do_platform_entry, mod);
> +	else if (sym_is(symname, "__mod_isapnp_device_table"))
> +		do_table(symval, sym->st_size,
> +			 sizeof(struct isapnp_device_id), "isa",
> +			 do_isapnp_entry, mod);
>  	free(zeros);
>  }

The above patch did not work. However, the patch below works fine (at least 
with Debian). It needs your first patch that moves the definitions to 
mod_devicetable.h. Verified that aliases for these modules are generated 
correctly:
drivers/media/radio/radio-sf16fmi.c
drivers/net/ne.c
drivers/net/3c515.c
drivers/net/smc-ultra.c
drivers/pcmcia/i82365.c
drivers/scsi/aha1542.c
drivers/scsi/aha152x.c
drivers/scsi/sym53c416.c
drivers/scsi/g_NCR5380.c

Tested with RTL8019AS (ne), AVA-1505AE (aha152x) and dtc436e (g_NCR5380) 
cards - they now work automatically.

Found that drivers/isdn/hisax/hisax_fcpcipnp.c has broken pnp device table - 
wrong type (isapnp instead of pnp) and also ending record missing.


Generate pnp:d aliases for isapnp_device_tables. This allows udev to load
these modules automatically.

Signed-off-by: Ondrej Zary <linux@rainbow-software.org>

--- linux-source-2.6.32-orig/scripts/mod/file2alias.c	2009-12-03 04:51:21.000000000 +0100
+++ linux-source-2.6.32/scripts/mod/file2alias.c	2009-12-18 20:20:57.000000000 +0100
@@ -727,6 +727,19 @@ static int do_platform_entry(const char 
 	return 1;
 }
 
+/* looks like: "pnp:dD" */
+static int do_isapnp_entry(const char *filename,
+			   struct isapnp_device_id *id, char *alias)
+{
+	sprintf(alias, "pnp:d%c%c%c%x%x%x%x*",
+		'A' + ((id->vendor >> 2) & 0x3f) - 1,
+		'A' + (((id->vendor & 3) << 3) | ((id->vendor >> 13) & 7)) - 1,
+		'A' + ((id->vendor >> 8) & 0x1f) - 1,
+		(id->function >> 4) & 0x0f, id->function & 0x0f,
+		(id->function >> 12) & 0x0f, (id->function >> 8) & 0x0f);
+	return 1;
+}
+
 /* Ignore any prefix, eg. some architectures prepend _ */
 static inline int sym_is(const char *symbol, const char *name)
 {
@@ -874,6 +887,10 @@ void handle_moddevtable(struct module *m
 		do_table(symval, sym->st_size,
 			 sizeof(struct platform_device_id), "platform",
 			 do_platform_entry, mod);
+	else if (sym_is(symname, "__mod_isapnp_device_table"))
+		do_table(symval, sym->st_size,
+			sizeof(struct isapnp_device_id), "isa",
+			do_isapnp_entry, mod);
 	free(zeros);
 }
 


hisax_fcpcipnp: fix broken isapnp device table.

--- linux-source-2.6.32-orig/drivers/isdn/hisax/hisax_fcpcipnp.c	2009-12-03 04:51:21.000000000 +0100
+++ linux-source-2.6.32/drivers/isdn/hisax/hisax_fcpcipnp.c	2009-12-17 23:55:08.000000000 +0100
@@ -74,9 +74,10 @@ static struct pnp_device_id fcpnp_ids[] 
 		.id		= "AVM0900",
 		.driver_data	= (unsigned long) "Fritz!Card PnP",
 	},
+	{ .id = "" }
 };
 
-MODULE_DEVICE_TABLE(isapnp, fcpnp_ids);
+MODULE_DEVICE_TABLE(pnp, fcpnp_ids);
 #endif
 
 static int protocol = 2;       /* EURO-ISDN Default */


-- 
Ondrej Zary

Re: MODULE_DEVICE_TABLE(isapnp, ...) does nothing

[Rusty Russell]

On Sat, 19 Dec 2009 06:22:39 am Ondrej Zary wrote:
> Generate pnp:d aliases for isapnp_device_tables. This allows udev to load
> these modules automatically.
> 
> Signed-off-by: Ondrej Zary <linux@rainbow-software.org>

I'm happy with this, so I've separated and taken them.  Too late for the
merge window I'm afraid, but it'll be in next one.

Thanks,
Rusty.

Re: MODULE_DEVICE_TABLE(isapnp, ...) does nothing

[Ondrej Zary]

On Monday 21 December 2009, Rusty Russell wrote:
> On Sat, 19 Dec 2009 06:22:39 am Ondrej Zary wrote:
> > Generate pnp:d aliases for isapnp_device_tables. This allows udev to load
> > these modules automatically.
> >
> > Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
>
> I'm happy with this, so I've separated and taken them.  Too late for the
> merge window I'm afraid, but it'll be in next one.

When will this go in?

-- 
Ondrej Zary

Re: MODULE_DEVICE_TABLE(isapnp, ...) does nothing

[Rusty Russell]

On Tue, 23 Mar 2010 12:14:38 am Ondrej Zary wrote:
> On Monday 21 December 2009, Rusty Russell wrote:
> > On Sat, 19 Dec 2009 06:22:39 am Ondrej Zary wrote:
> > > Generate pnp:d aliases for isapnp_device_tables. This allows udev to load
> > > these modules automatically.
> > >
> > > Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
> >
> > I'm happy with this, so I've separated and taken them.  Too late for the
> > merge window I'm afraid, but it'll be in next one.
> 
> When will this go in?

Sorry, my bad.  It was at the back of my queue and got lost.

It'd be nice for it to go in, but it needs at least a few days in linux-next,
simple as it is.  Linus is unlikely to believe me if I tell him it's suddenly
urgent.

Sorry,
Rusty.



> 
>