Hello Linus,
Looks like this one liner patch never made it... Hopefully, it'll go in
before you release rc9 or 2.6.29.
Best,
Mathieu
----- Forwarded message from Henrique de Moraes Holschuh <[email protected]> -----
Delivered-To: [email protected]
X-Sasl-enc: J0pR0vt90JACc2icrP/7acPg7Vn8fC4jw2hSyELrb8PJ 1234704353
X-Virus-Scanned: Debian amavisd-new at khazad-dum.debian.net
From: Henrique de Moraes Holschuh <[email protected]>
To: Len Brown <[email protected]>
Cc: [email protected], [email protected],
Mathieu Chouquet-Stringer <[email protected]>, [email protected]
Subject: [PATCH] thinkpad-acpi: fix module autoloading for older models
Date: Sun, 15 Feb 2009 10:25:51 -0300
X-Mailer: git-send-email 1.5.6.5
X-CRM114-Version: 20070301-BlameBaltar ( TRE 0.7.5 (LGPL) ) MF-F8A122EB [pR: -2.3502]
X-CRM114-CacheID: sfid-20090215_142656_606015_D7CAAB9F
X-CRM114-Status: UNSURE (-2.3502) This message is 'unsure'; please train it!
From: Mathieu Chouquet-Stringer <[email protected]>
Looking at the source, there seems to be a missing * to match my DMI
string. I mean for newer IBM and Lenovo's laptops you match either one
of the following:
MODULE_ALIAS("dmi:bvnIBM:*:svnIBM:*:pvrThinkPad*:rvnIBM:*");
MODULE_ALIAS("dmi:bvnLENOVO:*:svnLENOVO:*:pvrThinkPad*:rvnLENOVO:*");
While for older Thinkpads, you do this (for instance):
IBM_BIOS_MODULE_ALIAS("1[0,3,6,8,A-G,I,K,M-P,S,T]");
with IBM_BIOS_MODULE_ALIAS being MODULE_ALIAS("dmi:bvnIBM:bvr" __type "ET??WW")
Note there's no * terminating the string. As result, udev doesn't load
anything because modprobe cannot find anything matching this (my
machine actually):
udevtest: run: '/sbin/modprobe dmi:bvnIBM:bvr1IET71WW(2.10):bd06/16/2006:svnIBM:pn236621U:pvrNotAvailable:rvnIBM:rn236621U:rvrNotAvailable:cvnIBM:ct10:cvrNotAvailable:'
Signed-off-by: Mathieu Chouquet-Stringer <[email protected]>
Acked-by: Henrique de Moraes Holschuh <[email protected]>
Cc: [email protected]
---
drivers/platform/x86/thinkpad_acpi.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
Len, please consider this for 2.6.29. It is an obvious one-liner.
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 91ae159..d218ecb 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -7524,7 +7524,7 @@ MODULE_ALIAS(TPACPI_DRVR_SHORTNAME);
* if it is not there yet.
*/
#define IBM_BIOS_MODULE_ALIAS(__type) \
- MODULE_ALIAS("dmi:bvnIBM:bvr" __type "ET??WW")
+ MODULE_ALIAS("dmi:bvnIBM:bvr" __type "ET??WW*")
/* Non-ancient thinkpads */
MODULE_ALIAS("dmi:bvnIBM:*:svnIBM:*:pvrThinkPad*:rvnIBM:*");
--
1.5.6.5
On Sat, Mar 14, 2009 at 11:42, Mathieu Chouquet-Stringer
<[email protected]> wrote:
> While for older Thinkpads, you do this (for instance):
> IBM_BIOS_MODULE_ALIAS("1[0,3,6,8,A-G,I,K,M-P,S,T]");
Aliases are shell-style globs. Any idea what the ',' are doing in a
character class? Confused ...
Thanks,
Kay
On Sat, Mar 14, 2009 at 04:14:19PM +0100, Kay Sievers wrote:
> On Sat, Mar 14, 2009 at 11:42, Mathieu Chouquet-Stringer
> <[email protected]> wrote:
>
> > While for older Thinkpads, you do this (for instance):
> > IBM_BIOS_MODULE_ALIAS("1[0,3,6,8,A-G,I,K,M-P,S,T]");
>
> Aliases are shell-style globs. Any idea what the ',' are doing in a
> character class? Confused ...
Good point... I guess it's a typo: perhaps the original authors thought
of it as a brace expansion? Something like 1{0,3,6,...}?
A proper patch would then be:
Signed-off-by: Mathieu Chouquet-Stringer <[email protected]>
drivers/platform/x86/thinkpad_acpi.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index bcbc051..d243320 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -7532,7 +7532,7 @@ MODULE_ALIAS(TPACPI_DRVR_SHORTNAME);
* if it is not there yet.
*/
#define IBM_BIOS_MODULE_ALIAS(__type) \
- MODULE_ALIAS("dmi:bvnIBM:bvr" __type "ET??WW")
+ MODULE_ALIAS("dmi:bvnIBM:bvr" __type "ET??WW*")
/* Non-ancient thinkpads */
MODULE_ALIAS("dmi:bvnIBM:*:svnIBM:*:pvrThinkPad*:rvnIBM:*");
@@ -7541,9 +7541,9 @@ MODULE_ALIAS("dmi:bvnLENOVO:*:svnLENOVO:*:pvrThinkPad*:rvnLENOVO:*");
/* Ancient thinkpad BIOSes have to be identified by
* BIOS type or model number, and there are far less
* BIOS types than model numbers... */
-IBM_BIOS_MODULE_ALIAS("I[B,D,H,I,M,N,O,T,W,V,Y,Z]");
-IBM_BIOS_MODULE_ALIAS("1[0,3,6,8,A-G,I,K,M-P,S,T]");
-IBM_BIOS_MODULE_ALIAS("K[U,X-Z]");
+IBM_BIOS_MODULE_ALIAS("I[BDHIMNOTWVYZ]");
+IBM_BIOS_MODULE_ALIAS("1[0368A-GIKM-PST]");
+IBM_BIOS_MODULE_ALIAS("K[UX-Z]");
MODULE_AUTHOR("Borislav Deianov, Henrique de Moraes Holschuh");
MODULE_DESCRIPTION(TPACPI_DESC);
On Sat, 14 Mar 2009, Mathieu Chouquet-Stringer wrote:
> On Sat, Mar 14, 2009 at 04:14:19PM +0100, Kay Sievers wrote:
> > On Sat, Mar 14, 2009 at 11:42, Mathieu Chouquet-Stringer
> > <[email protected]> wrote:
> >
> > > While for older Thinkpads, you do this (for instance):
> > > IBM_BIOS_MODULE_ALIAS("1[0,3,6,8,A-G,I,K,M-P,S,T]");
> >
> > Aliases are shell-style globs. Any idea what the ',' are doing in a
> > character class? Confused ...
>
> Good point... I guess it's a typo: perhaps the original authors thought
> of it as a brace expansion? Something like 1{0,3,6,...}?
I bet that's exactly what happened... it was a long time ago, so I don't
recall exactly what I was (not) thinking about when I did it.
Thanks for noticing this, and the revised patch looks good.
> A proper patch would then be:
>
> Signed-off-by: Mathieu Chouquet-Stringer <[email protected]>
> drivers/platform/x86/thinkpad_acpi.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index bcbc051..d243320 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -7532,7 +7532,7 @@ MODULE_ALIAS(TPACPI_DRVR_SHORTNAME);
> * if it is not there yet.
> */
> #define IBM_BIOS_MODULE_ALIAS(__type) \
> - MODULE_ALIAS("dmi:bvnIBM:bvr" __type "ET??WW")
> + MODULE_ALIAS("dmi:bvnIBM:bvr" __type "ET??WW*")
>
> /* Non-ancient thinkpads */
> MODULE_ALIAS("dmi:bvnIBM:*:svnIBM:*:pvrThinkPad*:rvnIBM:*");
> @@ -7541,9 +7541,9 @@ MODULE_ALIAS("dmi:bvnLENOVO:*:svnLENOVO:*:pvrThinkPad*:rvnLENOVO:*");
> /* Ancient thinkpad BIOSes have to be identified by
> * BIOS type or model number, and there are far less
> * BIOS types than model numbers... */
> -IBM_BIOS_MODULE_ALIAS("I[B,D,H,I,M,N,O,T,W,V,Y,Z]");
> -IBM_BIOS_MODULE_ALIAS("1[0,3,6,8,A-G,I,K,M-P,S,T]");
> -IBM_BIOS_MODULE_ALIAS("K[U,X-Z]");
> +IBM_BIOS_MODULE_ALIAS("I[BDHIMNOTWVYZ]");
> +IBM_BIOS_MODULE_ALIAS("1[0368A-GIKM-PST]");
> +IBM_BIOS_MODULE_ALIAS("K[UX-Z]");
>
> MODULE_AUTHOR("Borislav Deianov, Henrique de Moraes Holschuh");
> MODULE_DESCRIPTION(TPACPI_DESC);
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
On Sat, 14 Mar 2009, Henrique de Moraes Holschuh wrote:
> On Sat, 14 Mar 2009, Mathieu Chouquet-Stringer wrote:
> > On Sat, Mar 14, 2009 at 04:14:19PM +0100, Kay Sievers wrote:
> > > On Sat, Mar 14, 2009 at 11:42, Mathieu Chouquet-Stringer
> > > <[email protected]> wrote:
> > >
> > > > While for older Thinkpads, you do this (for instance):
> > > > IBM_BIOS_MODULE_ALIAS("1[0,3,6,8,A-G,I,K,M-P,S,T]");
> > >
> > > Aliases are shell-style globs. Any idea what the ',' are doing in a
> > > character class? Confused ...
> >
> > Good point... I guess it's a typo: perhaps the original authors thought
> > of it as a brace expansion? Something like 1{0,3,6,...}?
>
> I bet that's exactly what happened... it was a long time ago, so I don't
> recall exactly what I was (not) thinking about when I did it.
>
> Thanks for noticing this, and the revised patch looks good.
revised patch (with commit log from 1st patch) applied.
thanks,
Len Brown, Intel Open Source Technology Center