char/tpm: tpm_infineon no longer loaded for HP 2510p laptop

char/tpm: tpm_infineon no longer loaded for HP 2510p laptop

[Frans Pop]

While comparing the loaded modules for 2.6.26 and 2.6.27-rc3 for my HP 
2510p, I noticed that the tpm_infineon module and related modules no 
longer get loaded automatically.

The difference seems to be that 2.6.26 listed:
/lib/modules/2.6.26.2/modules.alias:alias pnp:dIFX0102* tpm_infineon
/lib/modules/2.6.26.2/modules.alias:alias pnp:dIFX0101* tpm_infineon

while 2.6.27 has:
/lib/modules/2.6.27-rc3/modules.alias:alias acpi*:IFX0101:* tpm_infineon
/lib/modules/2.6.27-rc3/modules.alias:alias pnp:dIFX0101* tpm_infineon

My system has:
$ grep IFX /sys/bus/pnp/devices/*/id
/sys/bus/pnp/devices/00:02/id:IFX0102

And if I modprobe the module manually the device really does seem to be 
supported:
pnp: the driver 'tpm_inf_pnp' has been registered
tpm_inf_pnp 00:02: Found C284 with ID IFX0102
tpm_inf_pnp 00:02: TPM found: config base 0x560, data base 0x570, chip 
version 0x000b, vendor id 0x15d1 (Infineon), product id 0x000b (SLB 9635 
TT 1.2)
tpm_inf_pnp 00:02: driver attached

Relevant bit of the kernel config:
CONFIG_TCG_TPM=m
CONFIG_TCG_TIS=m
# CONFIG_TCG_NSC is not set
# CONFIG_TCG_ATMEL is not set
CONFIG_TCG_INFINEON=m

I've taken a quick look at changes from 2.6.26 in drivers/char/tpm/ but 
did not see really anything there that could explain this difference in 
behavior.

Cheers,
FJP

Re: char/tpm: tpm_infineon no longer loaded for HP 2510p laptop

[Bjorn Helgaas]

On Monday 18 August 2008 07:40:24 am Frans Pop wrote:
> While comparing the loaded modules for 2.6.26 and 2.6.27-rc3 for my HP 
> 2510p, I noticed that the tpm_infineon module and related modules no 
> longer get loaded automatically.
> 
> The difference seems to be that 2.6.26 listed:
> /lib/modules/2.6.26.2/modules.alias:alias pnp:dIFX0102* tpm_infineon
> /lib/modules/2.6.26.2/modules.alias:alias pnp:dIFX0101* tpm_infineon
> 
> while 2.6.27 has:
> /lib/modules/2.6.27-rc3/modules.alias:alias acpi*:IFX0101:* tpm_infineon
> /lib/modules/2.6.27-rc3/modules.alias:alias pnp:dIFX0101* tpm_infineon
> 
> My system has:
> $ grep IFX /sys/bus/pnp/devices/*/id
> /sys/bus/pnp/devices/00:02/id:IFX0102

drivers/char/tpm/tpm_infineon.c hasn't changed since v2.6.26.  I think
the problem is more likely related to commit 22454cb99fc39f2629a:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=22454cb99fc39f2629ad06a7eccb3df312f8830e

I don't know enough about modaliases to understand the point of this
patch, but I am suspicious of this part of it:

-               do_table(symval, sym->st_size,
-                        sizeof(struct pnp_device_id), "pnp",
-                        do_pnp_entry, mod);
+               do_pnp_device_entry(symval, sym->st_size, mod);

That suggests to me that where we used to generate an alias for every
PNP ID in the table:

    static const struct pnp_device_id tpm_pnp_tbl[] = {
        {"IFX0101", 0},
        {"IFX0102", 0},
        {"", 0}
    };

possibly the new code only does it for the first entry (IFX0101).

Apart from problem Frans points out, I'd like to understand the reason
for generating the "acpi*" aliases for PNP drivers.  I'd like to move
away from ACPI being visible to userland, and this file2alias.c change
makes it *more* visible.

Bjorn

Re: char/tpm: tpm_infineon no longer loaded for HP 2510p laptop

[Rafael J. Wysocki]

On Wednesday, 20 of August 2008, Bjorn Helgaas wrote:
> On Monday 18 August 2008 07:40:24 am Frans Pop wrote:
> > While comparing the loaded modules for 2.6.26 and 2.6.27-rc3 for my HP 
> > 2510p, I noticed that the tpm_infineon module and related modules no 
> > longer get loaded automatically.
> > 
> > The difference seems to be that 2.6.26 listed:
> > /lib/modules/2.6.26.2/modules.alias:alias pnp:dIFX0102* tpm_infineon
> > /lib/modules/2.6.26.2/modules.alias:alias pnp:dIFX0101* tpm_infineon
> > 
> > while 2.6.27 has:
> > /lib/modules/2.6.27-rc3/modules.alias:alias acpi*:IFX0101:* tpm_infineon
> > /lib/modules/2.6.27-rc3/modules.alias:alias pnp:dIFX0101* tpm_infineon
> > 
> > My system has:
> > $ grep IFX /sys/bus/pnp/devices/*/id
> > /sys/bus/pnp/devices/00:02/id:IFX0102
> 
> drivers/char/tpm/tpm_infineon.c hasn't changed since v2.6.26.  I think
> the problem is more likely related to commit 22454cb99fc39f2629a:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=22454cb99fc39f2629ad06a7eccb3df312f8830e

Frans, could you verify if reverting commit 22454cb99fc39f2629a fixes the
problem for you?

Thanks,
Rafael

Re: char/tpm: tpm_infineon no longer loaded for HP 2510p laptop

[Kay Sievers]

On Thu, 2008-08-21 at 14:40 +0200, Rafael J. Wysocki wrote:
> On Wednesday, 20 of August 2008, Bjorn Helgaas wrote:
> > On Monday 18 August 2008 07:40:24 am Frans Pop wrote:
> > > While comparing the loaded modules for 2.6.26 and 2.6.27-rc3 for my HP 
> > > 2510p, I noticed that the tpm_infineon module and related modules no 
> > > longer get loaded automatically.
> > > 
> > > The difference seems to be that 2.6.26 listed:
> > > /lib/modules/2.6.26.2/modules.alias:alias pnp:dIFX0102* tpm_infineon
> > > /lib/modules/2.6.26.2/modules.alias:alias pnp:dIFX0101* tpm_infineon
> > > 
> > > while 2.6.27 has:
> > > /lib/modules/2.6.27-rc3/modules.alias:alias acpi*:IFX0101:* tpm_infineon
> > > /lib/modules/2.6.27-rc3/modules.alias:alias pnp:dIFX0101* tpm_infineon
> > > 
> > > My system has:
> > > $ grep IFX /sys/bus/pnp/devices/*/id
> > > /sys/bus/pnp/devices/00:02/id:IFX0102
> > 
> > drivers/char/tpm/tpm_infineon.c hasn't changed since v2.6.26.  I think
> > the problem is more likely related to commit 22454cb99fc39f2629a:
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=22454cb99fc39f2629ad06a7eccb3df312f8830e
> 
> Frans, could you verify if reverting commit 22454cb99fc39f2629a fixes the
> problem for you?

Yeah, we miss to loop over the list. This should fix it. Thanks!


From: Kay Sievers <kay.sievers@vrfy.org>
Subject: pnp: fix "add acpi:* modalias entries"

With 22454cb99fc39f2629ad06a7eccb3df312f8830e we added only the
first entry of the device table. We need to loop over the whole
device list.

Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
---

 file2alias.c |   16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -344,14 +344,20 @@ static void do_pnp_device_entry(void *symval, unsigned long size,
 				struct module *mod)
 {
 	const unsigned long id_size = sizeof(struct pnp_device_id);
-	const struct pnp_device_id *id = symval;
+	const unsigned int count = (size / id_size)-1;
+	const struct pnp_device_id *devs = symval;
+	unsigned int i;
 
 	device_id_check(mod->name, "pnp", size, id_size, symval);
 
-	buf_printf(&mod->dev_table_buf,
-		   "MODULE_ALIAS(\"pnp:d%s*\");\n", id->id);
-	buf_printf(&mod->dev_table_buf,
-		   "MODULE_ALIAS(\"acpi*:%s:*\");\n", id->id);
+	for (i = 0; i < count; i++) {
+		const char *id = (char *)devs[i].id;
+
+		buf_printf(&mod->dev_table_buf,
+			   "MODULE_ALIAS(\"pnp:d%s*\");\n", id);
+		buf_printf(&mod->dev_table_buf,
+			   "MODULE_ALIAS(\"acpi*:%s:*\");\n", id);
+	}
 }
 
 /* looks like: "pnp:dD" for every device of the card */

Re: char/tpm: tpm_infineon no longer loaded for HP 2510p laptop

[Bjorn Helgaas]

On Thursday 21 August 2008 07:28:56 am Kay Sievers wrote:
> Yeah, we miss to loop over the list. This should fix it. Thanks!

Thanks for the patch.

Can somebody elaborate on why we need to add "acpi*" aliases for all
PNP devices?  That broadens the kernel/user interface, so I'd like
to understand why we need it.

Bjorn

Re: char/tpm: tpm_infineon no longer loaded for HP 2510p laptop

[Kay Sievers]

On Thu, 2008-08-21 at 09:14 -0600, Bjorn Helgaas wrote:
> On Thursday 21 August 2008 07:28:56 am Kay Sievers wrote:
> > Yeah, we miss to loop over the list. This should fix it. Thanks!
> 
> Thanks for the patch.
> 
> Can somebody elaborate on why we need to add "acpi*" aliases for all
> PNP devices?  That broadens the kernel/user interface, so I'd like
> to understand why we need it.

We already do ACPI module autoloading by MODALIAS for other things than
pnp. ACPI exports the pnp devices with modalias, but the modules do not
have a matching alias, this add them.

PNP has no MODALIAS support at all, and the current pnp-aliases would
not work for the standard modalias method, they would need to change
their format.

The plan is to replace the current pnp modprobe shell script hack in
udev, when ACPI devices load the right modules without any special
userspace mangling.

Thanks,
Kay

Re: char/tpm: tpm_infineon no longer loaded for HP 2510p laptop

[Bjorn Helgaas]

On Thursday 21 August 2008 09:38:25 am Kay Sievers wrote:
> On Thu, 2008-08-21 at 09:14 -0600, Bjorn Helgaas wrote:
> > Can somebody elaborate on why we need to add "acpi*" aliases for all
> > PNP devices?  That broadens the kernel/user interface, so I'd like
> > to understand why we need it.

Sorry, I should have prefaced my question with "I'm completely ignorant
of this modalias stuff."  Is there a "complete idiot's guide to modules
and udev"?  There's precious little in Documentation/ other than a bunch
of sample rules for various subsystems.

> We already do ACPI module autoloading by MODALIAS for other things than
> pnp. ACPI exports the pnp devices with modalias, but the modules do not
> have a matching alias, this add them.

I'm guessing this has something to do with acpi_device_uevent().

> PNP has no MODALIAS support at all, and the current pnp-aliases would
> not work for the standard modalias method, they would need to change
> their format.

pnp_bus_type has no .uevent method.  What if I added one?  Would that
help this situation?

It seems wrong for file2alias.c to take every PNP device (even if it's
an ISAPNP or PNPBIOS device) and add "acpi*" aliases for it.

> The plan is to replace the current pnp modprobe shell script hack in
> udev, when ACPI devices load the right modules without any special
> userspace mangling.

Is this the shell hack you mean (from etc/udev/rules.d/80-drivers.rules
in udev-117)?

  SUBSYSTEM=="pnp", DRIVER!="?*", ENV{MODALIAS}!="?*", \
    RUN{ignore_error}+="/bin/sh -c '/sbin/modprobe -a $$(while read id; do echo pnp:d$$id; done < /sys$devpath/id)'"

I agree that's gross.  Could I fix this by implementing pnp_device_uevent()?

Bjorn

Re: char/tpm: tpm_infineon no longer loaded for HP 2510p laptop

[Frans Pop]

On Thursday 21 August 2008, Kay Sievers wrote:
> From: Kay Sievers <kay.sievers@vrfy.org>
> Subject: pnp: fix "add acpi:* modalias entries"
>
> With 22454cb99fc39f2629ad06a7eccb3df312f8830e we added only the
> first entry of the device table. We need to loop over the whole
> device list.
>
> Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>

This patch works for me (tested on top of -rc4).

$ grep IFX /lib/modules/2.6.27-rc4/modules.alias
alias acpi*:IFX0102:* tpm_infineon
alias pnp:dIFX0102* tpm_infineon
alias acpi*:IFX0101:* tpm_infineon
alias pnp:dIFX0101* tpm_infineon

And the tpm modules are automatically loaded again.

Cheers,
FJP

Re: char/tpm: tpm_infineon no longer loaded for HP 2510p laptop

[Kay Sievers]

Bjorn Helgaas wrote:
>On Thursday 21 August 2008 09:38:25 am Kay Sievers wrote:
>> On Thu, 2008-08-21 at 09:14 -0600, Bjorn Helgaas wrote:
>> > Can somebody elaborate on why we need to add "acpi*" aliases for all
>> > PNP devices?  That broadens the kernel/user interface, so I'd like
>> > to understand why we need it.
>
>Sorry, I should have prefaced my question with "I'm completely ignorant
>of this modalias stuff."

:)

>Is there a "complete idiot's guide to modules
>and udev"?  There's precious little in Documentation/ other than a bunch
>of sample rules for various subsystems.

I don't know of any specific documentation, but it's pretty easy:
Devices, any kind of device, can export a match, based on specific
properties of the subsystem it belongs to. In most cases its the same
propery/id that is used inside the kernel, to find a (already loaded)
driver which will bind this device. 

Any unique string, hardware ID's, whatever, are stuffed into a modalias,
prefixed by "<subsystem>:" to be unique.

PCI and USB are pretty obvious, they just stuff all their hardware ID'S
into a string, in a defined order, and let every device export that value
to userspace by MODALIAS environment key and the "modalias" sysfs file.
Other subsystem may have simple strings to match, they define
themselves.

Now, the drivers contain "match id tables" which are used by the core
to bind devices to drivers. These tables area made available to
file2alias.c in the module postprocessing, and will end up in the
module file itself. The string is mangled to contain wildcards, so
they can just be fnmatch()'d against the modalias value, which the
devices export.

The string embedded in the modules are extracted by depmod, and put
into the modules.aliases file in /lib/modules/. Every time modprobe
is called with an alias, it searches through this file and loads all
modules which contained a matching alias string.

Udev does nothing but stupidly running modprobe for every device which
contains MODALIAS in the event environment, and passes $MODALIAS as
an argument to modprobe.

That's basically the whole module autoloading today. :)

>> We already do ACPI module autoloading by MODALIAS for other things than
>> pnp. ACPI exports the pnp devices with modalias, but the modules do not
>> have a matching alias, this add them.
>
>I'm guessing this has something to do with acpi_device_uevent().

That's where the MODALIAS environment value is composed, yes.

>> PNP has no MODALIAS support at all, and the current pnp-aliases would
>> not work for the standard modalias method, they would need to change
>> their format.
>
>pnp_bus_type has no .uevent method.  What if I added one?  Would that
>help this situation?

Only if we would change the format of the aliases.

>It seems wrong for file2alias.c to take every PNP device (even if it's
>an ISAPNP or PNPBIOS device) and add "acpi*" aliases for it.

Yeah, they are all exported by acpi too.

>> The plan is to replace the current pnp modprobe shell script hack in
>> udev, when ACPI devices load the right modules without any special
>> userspace mangling.
>
>Is this the shell hack you mean (from etc/udev/rules.d/80-drivers.rules
>in udev-117)?
>
>  SUBSYSTEM=="pnp", DRIVER!="?*", ENV{MODALIAS}!="?*", \
>    RUN{ignore_error}+="/bin/sh -c '/sbin/modprobe -a $$(while read id; do 
>echo pnp:d$$id; done < /sys$devpath/id)'"
>
>I agree that's gross.

Yeah, that's what we want to kill. :)

>Could I fix this by implementing pnp_device_uevent()?

Only if we change the format of the current pnp device aliases
to something like:
  pnp*:XYZ2324:*
  pnp*:ABC1234:*

and create a "modalias" file at every pnp device, and add MODALIAS to
the uevent. The modalias must contains all ID's which belong to that
device in one single string, separated and terminated by a special
character, something like:
  pnp:ABC1234:XYZ2324:RST3445:

That's how acpi should work with this patch now.

Kay

Re: char/tpm: tpm_infineon no longer loaded for HP 2510p laptop

[Bjorn Helgaas]

On Thursday 21 August 2008 03:18:33 pm Kay Sievers wrote:
> Bjorn Helgaas wrote:
> >Is there a "complete idiot's guide to modules
> >and udev"?  There's precious little in Documentation/ other than a bunch
> >of sample rules for various subsystems.
> 
> I don't know of any specific documentation, but it's pretty easy:
> ...

Thanks for the tutorial.

> >Could I fix this by implementing pnp_device_uevent()?
> 
> Only if we change the format of the current pnp device aliases
> to something like:
>   pnp*:XYZ2324:*
>   pnp*:ABC1234:*
> 
> and create a "modalias" file at every pnp device, and add MODALIAS to
> the uevent. The modalias must contains all ID's which belong to that
> device in one single string, separated and terminated by a special
> character, something like:
>   pnp:ABC1234:XYZ2324:RST3445:

This all sounds like good stuff that I'd like PNP to have.  Is there
any reason I shouldn't implement pnp_device_uevent()?  Any backwards-
compatibility issues?

I think that sounds like a better solution than doing this PNP ID
mangling.

Bjorn

Re: char/tpm: tpm_infineon no longer loaded for HP 2510p laptop

[Kay Sievers]

On Thu, 2008-08-21 at 15:58 -0600, Bjorn Helgaas wrote:
> On Thursday 21 August 2008 03:18:33 pm Kay Sievers wrote:
> > Bjorn Helgaas wrote:
> > >Is there a "complete idiot's guide to modules
> > >and udev"?  There's precious little in Documentation/ other than a bunch
> > >of sample rules for various subsystems.
> > 
> > I don't know of any specific documentation, but it's pretty easy:
> > ...
> 
> Thanks for the tutorial.
> 
> > >Could I fix this by implementing pnp_device_uevent()?
> > 
> > Only if we change the format of the current pnp device aliases
> > to something like:
> >   pnp*:XYZ2324:*
> >   pnp*:ABC1234:*
> > 
> > and create a "modalias" file at every pnp device, and add MODALIAS to
> > the uevent. The modalias must contains all ID's which belong to that
> > device in one single string, separated and terminated by a special
> > character, something like:
> >   pnp:ABC1234:XYZ2324:RST3445:
> 
> This all sounds like good stuff that I'd like PNP to have.  Is there
> any reason I shouldn't implement pnp_device_uevent()?  Any backwards-
> compatibility issues?

Yeah, people probably use the current aliases in modprobe.conf files,
I'm not sure, if we can change them.

> I think that sounds like a better solution than doing this PNP ID
> mangling.

Sure, that will go with the acpi aliases + acpi modalias, or if you
change pnp to have proper pnp aliases + pnp modalias, then with them,
yes.

Thanks,
Kay

Re: char/tpm: tpm_infineon no longer loaded for HP 2510p laptop

[Bjorn Helgaas]

On Friday 22 August 2008 2:40:46 am Kay Sievers wrote:
> On Thu, 2008-08-21 at 15:58 -0600, Bjorn Helgaas wrote:
> > On Thursday 21 August 2008 03:18:33 pm Kay Sievers wrote:
> > > Bjorn Helgaas wrote:
> > > >Is there a "complete idiot's guide to modules
> > > >and udev"?  There's precious little in Documentation/ other than a
> > > > bunch of sample rules for various subsystems.
> > >
> > > I don't know of any specific documentation, but it's pretty easy:
> > > ...
> >
> > Thanks for the tutorial.
> >
> > > >Could I fix this by implementing pnp_device_uevent()?
> > >
> > > Only if we change the format of the current pnp device aliases
> > > to something like:
> > >   pnp*:XYZ2324:*
> > >   pnp*:ABC1234:*
> > >
> > > and create a "modalias" file at every pnp device, and add MODALIAS to
> > > the uevent. The modalias must contains all ID's which belong to that
> > > device in one single string, separated and terminated by a special
> > > character, something like:
> > >   pnp:ABC1234:XYZ2324:RST3445:
> >
> > This all sounds like good stuff that I'd like PNP to have.  Is there
> > any reason I shouldn't implement pnp_device_uevent()?  Any backwards-
> > compatibility issues?
>
> Yeah, people probably use the current aliases in modprobe.conf files,
> I'm not sure, if we can change them.
>
> > I think that sounds like a better solution than doing this PNP ID
> > mangling.
>
> Sure, that will go with the acpi aliases + acpi modalias, or if you
> change pnp to have proper pnp aliases + pnp modalias, then with them,
> yes.

Since PNP currently doesn't generate any uevents or modalias files,
I expect that a non-ACPI system will be unable to autoload modules
for ISAPNP or PNPBIOS devices.  Right?

Bjorn

Re: char/tpm: tpm_infineon no longer loaded for HP 2510p laptop

[Kay Sievers]

On Fri, 2008-08-22 at 06:06 -0600, Bjorn Helgaas wrote:
> On Friday 22 August 2008 2:40:46 am Kay Sievers wrote:
> > On Thu, 2008-08-21 at 15:58 -0600, Bjorn Helgaas wrote:
> > > On Thursday 21 August 2008 03:18:33 pm Kay Sievers wrote:
> > > > Bjorn Helgaas wrote:
> > > > >Is there a "complete idiot's guide to modules
> > > > >and udev"?  There's precious little in Documentation/ other than a
> > > > > bunch of sample rules for various subsystems.
> > > >
> > > > I don't know of any specific documentation, but it's pretty easy:
> > > > ...
> > >
> > > Thanks for the tutorial.
> > >
> > > > >Could I fix this by implementing pnp_device_uevent()?
> > > >
> > > > Only if we change the format of the current pnp device aliases
> > > > to something like:
> > > >   pnp*:XYZ2324:*
> > > >   pnp*:ABC1234:*
> > > >
> > > > and create a "modalias" file at every pnp device, and add MODALIAS to
> > > > the uevent. The modalias must contains all ID's which belong to that
> > > > device in one single string, separated and terminated by a special
> > > > character, something like:
> > > >   pnp:ABC1234:XYZ2324:RST3445:
> > >
> > > This all sounds like good stuff that I'd like PNP to have.  Is there
> > > any reason I shouldn't implement pnp_device_uevent()?  Any backwards-
> > > compatibility issues?
> >
> > Yeah, people probably use the current aliases in modprobe.conf files,
> > I'm not sure, if we can change them.
> >
> > > I think that sounds like a better solution than doing this PNP ID
> > > mangling.
> >
> > Sure, that will go with the acpi aliases + acpi modalias, or if you
> > change pnp to have proper pnp aliases + pnp modalias, then with them,
> > yes.
> 
> Since PNP currently doesn't generate any uevents or modalias files,
> I expect that a non-ACPI system will be unable to autoload modules
> for ISAPNP or PNPBIOS devices.  Right?

They do create events, but without modalias. The shell script hack,
which udev runs, will make the event behave like it contained one.

Kay

Re: char/tpm: tpm_infineon no longer loaded for HP 2510p laptop

[Bjorn Helgaas]

On Friday 22 August 2008 06:43:05 am Kay Sievers wrote:
> On Fri, 2008-08-22 at 06:06 -0600, Bjorn Helgaas wrote:
> > Since PNP currently doesn't generate any uevents or modalias files,
> > I expect that a non-ACPI system will be unable to autoload modules
> > for ISAPNP or PNPBIOS devices.  Right?
> 
> They do create events, but without modalias. The shell script hack,
> which udev runs, will make the event behave like it contained one.

I'm finally looking at this again; sorry for the long hiatus.  I'm
working on a patch to add PNP uevent support, modalias sysfs files
for PNP, and file2alias.c changes to match, and I just want to
make sure I'm understanding this correctly.

Before your file2alias.c changes[1], I think we generated this:
    alias pnp:dPNP0500* 8250_pnp

We relied on the udev shell hack to run "modprobe -a pnp:dPNP0500"
based on the contents of /sys/bus/pnp/devices/00:05/id.

With your file2alias.c changes, we now generate this:
    alias acpi*:PNP0500:* 8250_pnp
    alias pnp:dPNP0500* 8250_pnp

On ACPI systems, this works fine because "acpi*:PNP0500:*" matches
the ACPI-generated uevents like:
    MODALIAS=acpi:PNP0501:PNP0500:

We can load 8250_pnp without relying on the udev shell hack
*on ACPI systems*.

I thought the object of your file2alias.c changes was to remove the
need for the udev shell hack, but don't we still require it on
non-ACPI systems because they won't emit the ACPI uevents?

Bjorn

[1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=22454cb99fc39f2629ad06a7eccb3df312f8830e
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=5e4c6564c95ce127beeefe75e15cd11c93487436

Re: char/tpm: tpm_infineon no longer loaded for HP 2510p laptop

[Kay Sievers]

On Sat, Oct 4, 2008 at 12:01 AM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> On Friday 22 August 2008 06:43:05 am Kay Sievers wrote:
>> On Fri, 2008-08-22 at 06:06 -0600, Bjorn Helgaas wrote:
>> > Since PNP currently doesn't generate any uevents or modalias files,
>> > I expect that a non-ACPI system will be unable to autoload modules
>> > for ISAPNP or PNPBIOS devices.  Right?
>>
>> They do create events, but without modalias. The shell script hack,
>> which udev runs, will make the event behave like it contained one.
>
> I'm finally looking at this again; sorry for the long hiatus.  I'm
> working on a patch to add PNP uevent support, modalias sysfs files
> for PNP, and file2alias.c changes to match, and I just want to
> make sure I'm understanding this correctly.

Sounds good, just in mind, that there are custom modprobe configs out
there, that rely on the current pnp alias format, like:
  alias pnp:dPNP0510 irtty-sir
  alias pnp:dPNP0511 irtty-sir
  alias pnp:dPNP0700 floppy
  alias pnp:dPNP0303 atkbd
  alias pnp:dPNP0f13 psmouse
  ...
We should make sure, that this still works, which wouldn't if we just
do the acpi style aliases.

> Before your file2alias.c changes[1], I think we generated this:
>    alias pnp:dPNP0500* 8250_pnp
>
> We relied on the udev shell hack to run "modprobe -a pnp:dPNP0500"
> based on the contents of /sys/bus/pnp/devices/00:05/id.
>
> With your file2alias.c changes, we now generate this:
>    alias acpi*:PNP0500:* 8250_pnp
>    alias pnp:dPNP0500* 8250_pnp
>
> On ACPI systems, this works fine because "acpi*:PNP0500:*" matches
> the ACPI-generated uevents like:
>    MODALIAS=acpi:PNP0501:PNP0500:
>
> We can load 8250_pnp without relying on the udev shell hack
> *on ACPI systems*.
>
> I thought the object of your file2alias.c changes was to remove the
> need for the udev shell hack, but don't we still require it on
> non-ACPI systems because they won't emit the ACPI uevents?

Yes, the plan is to move that rule from the default udev rule set to
the isapnp package.

Thanks,
Kay

Re: char/tpm: tpm_infineon no longer loaded for HP 2510p laptop

[Bjorn Helgaas]

On Saturday 04 October 2008 6:09:38 am Kay Sievers wrote:
> On Sat, Oct 4, 2008 at 12:01 AM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > On Friday 22 August 2008 06:43:05 am Kay Sievers wrote:
> >> On Fri, 2008-08-22 at 06:06 -0600, Bjorn Helgaas wrote:
> >> > Since PNP currently doesn't generate any uevents or modalias files,
> >> > I expect that a non-ACPI system will be unable to autoload modules
> >> > for ISAPNP or PNPBIOS devices.  Right?
> >>
> >> They do create events, but without modalias. The shell script hack,
> >> which udev runs, will make the event behave like it contained one.
> >
> > I'm finally looking at this again; sorry for the long hiatus.  I'm
> > working on a patch to add PNP uevent support, modalias sysfs files
> > for PNP, and file2alias.c changes to match, and I just want to
> > make sure I'm understanding this correctly.
>
> Sounds good, just in mind, that there are custom modprobe configs out
> there, that rely on the current pnp alias format, like:
>   alias pnp:dPNP0510 irtty-sir
>   alias pnp:dPNP0511 irtty-sir
>   alias pnp:dPNP0700 floppy
>   alias pnp:dPNP0303 atkbd
>   alias pnp:dPNP0f13 psmouse
>   ...
> We should make sure, that this still works, which wouldn't if we just
> do the acpi style aliases.

My thought is to make PNP emit uevents with modaliases like
"MODALIAS=pnp:PNP0501:PNP0500:" and make file2alias generate
aliases like "alias pnp*:PNP0500:* 8250_pnp".

Modprobe configs like "alias pnp:dPNP0510 irtty-sir" would
still work but would continue to depend on the udev shell hack.

If we just move the udev shell hack to the isapnp package, those
"alias pnp:dPNP0510 irtty-sir" configs would then depend on isapnp,
which doesn't seem like quite what we want.  We can certainly have
PNP0510 ACPI devices that don't depend on isapnp.

All I want to do now is *enable* more conventional configs like
"alias pnp*:PNP0510:* irtty-sir" to work without extra hacks.
I don't have any plan for actually removing the hacks or doing
any user-space transition.

> > Before your file2alias.c changes[1], I think we generated this:
> >    alias pnp:dPNP0500* 8250_pnp
> >
> > We relied on the udev shell hack to run "modprobe -a pnp:dPNP0500"
> > based on the contents of /sys/bus/pnp/devices/00:05/id.
> >
> > With your file2alias.c changes, we now generate this:
> >    alias acpi*:PNP0500:* 8250_pnp
> >    alias pnp:dPNP0500* 8250_pnp
> >
> > On ACPI systems, this works fine because "acpi*:PNP0500:*" matches
> > the ACPI-generated uevents like:
> >    MODALIAS=acpi:PNP0501:PNP0500:
> >
> > We can load 8250_pnp without relying on the udev shell hack
> > *on ACPI systems*.
> >
> > I thought the object of your file2alias.c changes was to remove the
> > need for the udev shell hack, but don't we still require it on
> > non-ACPI systems because they won't emit the ACPI uevents?
>
> Yes, the plan is to move that rule from the default udev rule set to
> the isapnp package.

Re: char/tpm: tpm_infineon no longer loaded for HP 2510p laptop

[Kay Sievers]

On Sat, Oct 4, 2008 at 5:31 PM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> On Saturday 04 October 2008 6:09:38 am Kay Sievers wrote:
>> On Sat, Oct 4, 2008 at 12:01 AM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
>> > On Friday 22 August 2008 06:43:05 am Kay Sievers wrote:
>> >> On Fri, 2008-08-22 at 06:06 -0600, Bjorn Helgaas wrote:
>> >> > Since PNP currently doesn't generate any uevents or modalias files,
>> >> > I expect that a non-ACPI system will be unable to autoload modules
>> >> > for ISAPNP or PNPBIOS devices.  Right?
>> >>
>> >> They do create events, but without modalias. The shell script hack,
>> >> which udev runs, will make the event behave like it contained one.
>> >
>> > I'm finally looking at this again; sorry for the long hiatus.  I'm
>> > working on a patch to add PNP uevent support, modalias sysfs files
>> > for PNP, and file2alias.c changes to match, and I just want to
>> > make sure I'm understanding this correctly.
>>
>> Sounds good, just in mind, that there are custom modprobe configs out
>> there, that rely on the current pnp alias format, like:
>>   alias pnp:dPNP0510 irtty-sir
>>   alias pnp:dPNP0511 irtty-sir
>>   alias pnp:dPNP0700 floppy
>>   alias pnp:dPNP0303 atkbd
>>   alias pnp:dPNP0f13 psmouse
>>   ...
>> We should make sure, that this still works, which wouldn't if we just
>> do the acpi style aliases.
>
> My thought is to make PNP emit uevents with modaliases like
> "MODALIAS=pnp:PNP0501:PNP0500:" and make file2alias generate
> aliases like "alias pnp*:PNP0500:* 8250_pnp".

Looks good, and it is how they should look like. The current pnp
aliases are totally broken and useless for any usual modalias
matching.

> Modprobe configs like "alias pnp:dPNP0510 irtty-sir" would
> still work but would continue to depend on the udev shell hack.

The way the current shell hack is done, is that it gets disabled
automatically at the moment pnp events carry $MODALIAS.

> I don't have any plan for actually removing the hacks or doing
> any user-space transition.

Hmm, I guess there is no way around that. :)

Thanks,
Kay