[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20110704140755.GB2967@nautilus.holoscopio.com>
Date: Mon, 4 Jul 2011 11:07:55 -0300
From: Thadeu Lima de Souza Cascardo <cascardo@...oscopio.com>
To: Carlos Alberto Lopez Perez <clopez@...lia.com>
Cc: Daniel Oliveira Nascimento <don@...t.com.br>,
Matthew Garrett <mjg@...hat.com>,
platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-acpi@...r.kernel.org, lenb@...nel.org
Subject: Re: [PATCH 1/1] x86/platform: classmate-laptop: Add cmpc_keys
support for new classmate laptops
On Wed, Jun 22, 2011 at 08:43:25PM +0200, Carlos Alberto Lopez Perez wrote:
> On 06/21/2011 10:50 PM, Thadeu Lima de Souza Cascardo wrote:
> > On Tue, Jun 21, 2011 at 10:24:57PM +0200, Carlos Alberto Lopez Perez wrote:
> >> On 06/21/2011 09:50 PM, Thadeu Lima de Souza Cascardo wrote:
> >>> On Tue, Jun 21, 2011 at 09:40:01PM +0200, Carlos Alberto Lopez Perez wrote:
> >>>> The cmpc_keys ACPI driver from the classmate-laptop module fails to
> >>>> recognize the device on the 4th generation of classmate laptops (Atom N450)
> >>>> because the device ID is named "FNBT0000" (uppercase N) on this laptops.
> >>>>
> >>>> MODALIAS=acpi:FNBT0000:
> >>>>
> >>>> This patch makes the driver cmpc_keys recognize it:
> >>>>
> >>>> input: cmpc_keys as /devices/LNXSYSTM:00/device:00/FNBT0000:00/input/input18
> >>>>
> >>>> Signed-off-by: Carlos Alberto Lopez Perez <clopez@...lia.com>
> >>>> ---
> >>>> drivers/platform/x86/classmate-laptop.c | 3 +++
> >>>> 1 files changed, 3 insertions(+), 0 deletions(-)
> >>>>
> >>>> diff --git a/drivers/platform/x86/classmate-laptop.c b/drivers/platform/x86/classmate-laptop.c
> >>>> index 94f93b6..03896cd 100644
> >>>> --- a/drivers/platform/x86/classmate-laptop.c
> >>>> +++ b/drivers/platform/x86/classmate-laptop.c
> >>>> @@ -40,6 +40,7 @@ struct cmpc_accel {
> >>>> #define CMPC_TABLET_HID "TBLT0000"
> >>>> #define CMPC_IPML_HID "IPML200"
> >>>> #define CMPC_KEYS_HID "FnBT0000"
> >>>> +#define CMPC_KEYS_4GEN_HID "FNBT0000"
> >>>>
> >>>> /*
> >>>> * Generic input device code.
> >>>> @@ -683,6 +684,7 @@ static int cmpc_keys_remove(struct acpi_device *acpi, int type)
> >>>>
> >>>> static const struct acpi_device_id cmpc_keys_device_ids[] = {
> >>>> {CMPC_KEYS_HID, 0},
> >>>> + {CMPC_KEYS_4GEN_HID, 0},
> >>>> {"", 0}
> >>>> };
> >>>>
> >>>> @@ -754,6 +756,7 @@ static const struct acpi_device_id cmpc_device_ids[] = {
> >>>> {CMPC_TABLET_HID, 0},
> >>>> {CMPC_IPML_HID, 0},
> >>>> {CMPC_KEYS_HID, 0},
> >>>> + {CMPC_KEYS_4GEN_HID, 0},
> >>>> {"", 0}
> >>>> };
> >>>>
> >>>> --
> >>>> 1.7.5.3
> >>>>
> >>>
> >>> Not-Acked-By: Thadeu Lima de Souza Cascardo <cascardo@...oscopio.com>
> >>>
> >>> Recent Linux versions have "upper-cased" the device names, because ACPI
> >>> spec says devices must have upper-case letters. This is probably related
> >>> to this change, not because the device name has changed in recent
> >>> versions of the device. So the correct fix would be to either revert
> >>> this change in Linux or use only a single uppercase name for the device.
> >>>
> >>> Thanks anyway for this report. I have received one report in private a
> >>> while back, but was too lazy to discuss this on the list. Any pointers
> >>> on that? Easist way to go is to simply replace FnBT for FNBT.
> >>>
> >>> Regards,
> >>> Cascardo.
> >>
> >> The ACPI specification [1] says the following:
> >>
> >> """
> >> A valid PNP ID must be of the form “AAA####” where A is an uppercase letter
> >> and # is a hex digit.
> >> A valid ACPI ID must be of the form “ACPI####” where # is a hex digit.
> >> """
> >>
> >> It don't says that the ACPI ID must be uppercase.
> >>
> >> I don't have an older generation of the classmate laptop to test if the
> >> ACPI ID is FnBT or FNBT, but I guess that if it was working until now, is
> >> because the older generations have FnBT instead of FNBT.
> >>
> >> However I just found a previous discussion about this issue on LKML [2] and
> >> seems that the patch is included (commit: 72638f5).
> >>
> >> So... this means that the cmpc_keys module was not working for any
> >> classmate laptop since ~2.6.29 ?
> >>
> >> If that is the case then replacing FnBT with FNBT will be fine.
> >>
> >> Thanks!
> >>
> >> Best regards!
> >>
> >
> > Commit 77b23f712bc40a65160e7d02b045f1562bb43ff1 has changed this
> > behaviour between 2.6.36 and 2.6.37. Could you try using 2.6.36 and see
> > if the device appears as FnBT? Also, you could try using acpidump and
> > look at the decompiled DSDT so we can confirm whether it is FnBT or
> > FNBT. Attached is a patch that, when comparing device and driver names
> > for ACPI, will do it case-insensitivily.
> >
>
> You are right. The real ACPI ID of the device is FnBT0000 but the kernel
> is uppercasing it.
>
> # acpidump | grep -i -C1 FNBT0000
> 0b30: 00 0a 00 0a 14 5b 82 34 46 4e 42 54 08 5f 48 49 .....[.4FNBT._HI
> 0b40: 44 0d 46 6e 42 54 30 30 30 30 00 08 5f 55 49 44 D.FnBT0000.._UID
> 0b50: 0a 00 08 46 4e 4c 4b 0a 01 14 11 46 4e 43 54 01 ...FNLK....FNCT.
>
>
> [*] In kernels <= 2.6.36 (Tested in 2.6.32 and 2.6.36) the ACPI ID is
> registered as FnBT0000
>
> # cat /sys/bus/acpi/devices/FnBT0000\:00/modalias
> acpi:FnBT0000:
>
> And the cmpc_keys driver of the classmate-laptop module recognized the
> device as expected:
>
> input: cmpc_keys as /devices/LNXSYSTM:00/LNXSYBUS:00/FnBT0000:00/input/input5
>
>
>
>
> [*] In kernels => 2.6.37 (Tested in 2.6.37 and 2.6.39) the ACPI ID is
> registered uppercased as FNBT0000 and therefore the cmpc_keys driver fails
> to recognize it.
>
> # cat /sys/bus/acpi/devices/FNBT0000\:00/modalias
> acpi:FNBT0000:
>
>
>
> [*] With the kernel 2.6.39.1 and your patch for drivers/acpi/scan.c the
> ACPI ID is registered uppercased as FNBT0000 *but* the cmpc_keys driver
> recognizes it.
>
> # cat /sys/bus/acpi/devices/FNBT0000\:00/modalias
> acpi:FNBT0000:
>
> input: cmpc_keys as /devices/LNXSYSTM:00/device:00/FNBT0000:00/input/input5
>
>
> [*] With the kernel 2.6.39.1 and the commit
> 77b23f712bc40a65160e7d02b045f1562bb43ff1 reverted happens the same that
> with kernels <= 2.6.36 (the ACPI ID is registered as FnBT0000)
>
>
> > So, we have three options here. Revert an ACPICA commit, apply a
> > different matching code for ACPI bus in Linux or tell all drivers they
> > must use an uppper-case name instead of the device name as it appears in
> > the ACPI tables.
> >
>
> Yes, we have three options here to solve the problem but I'm not sure which
> one is the correct.
>
> At least I think it has been demonstrated that the ACPI ID is *not
> required* to be uppercase by the ACPI specification as you can see in the
> acpidump of this device (FnBT0000).
>
> So probably the "orthodox" way of solving this problem is reverting the
> commit 77b23f712bc40a65160e7d02b045f1562bb43ff1 which is doing a wrong
> assumption (its not true that the ACPI ID _must_ be uppercase).
>
>
> Regards!
>
I am copying the acpi list and Len Brown to see if they can tell us
which one should be the right solution:
1) reverting the patch in acpica that puts all names in uppercase
2) using a case-insensitive match in acpi bus driver
3) using uppercase name in classmate-laptop driver
Regards,
Cascardo.
Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)
Powered by blists - more mailing lists