Re: [coreboot] IBM/Lenovo X60t: LED_: Excess arguments - needs 1, found 2 (was: IBM x60t test - DSDT is in fact incomplete)

[Paul Menzel]

Dear Charles, dear David,

Am Montag, den 25.08.2014, 11:21 -0600 schrieb David Hubbard:
> I'm focusing in on this error message first:
>
> ACPI Warning: For _SB_.PCI0.LPCB.EC__.LED_: Excess arguments - needs 1,
> found 2
>
> Can you take a look at the .asl files in your coreboot build? I believe you
> will find something like "Method (LED, 2, NotSerialized)" which means the
> LED method wants 2 arguments. Then look for calls to "LED ()" and if you
> find one only passing 1 argument, that is the problem.

I also noticed that message and quickly looked through the coreboot ASL
files, but could not find anything calling that message. Looking at the
Linux kernel sources I was also unable to find a call site. But I’d say
the OS call this method incorrectly.

[…]

Thanks,

Paul

IBM/Lenovo X60t: LED_: Excess arguments - needs 1, found 2 (was: IBM x60t test - DSDT is in fact incomplete)

[Charles Devereaux]

Hello

Not sure!

Apparently, after doing echo "0 blink" >/proc/acpi/ibm/led, this is handled
led_set_status in drivers/platform/x86/thinkpad_acpi.c : the first argument
is the led, the second the status :
acpi_evalf(led_handle, NULL, NULL, "vdd", led, led_led_arg1[ledstatus]))

For the LED string, see in led_init_detect_mode :
status = acpi_get_handle(ec_handle, "LED", &led_handle);

My understanding is that the OS does the call that correctly, but that
coreboot ASL tables only expect one argument.

In coreboot/build/mainboard/lenovo/x60/dsdt.ramstage.asl, you will see
Method (LED, 1, NotSerialized) which seems to confirm that, since in
acpi-3.dsl I see instead  Method (LED, 2, NotSerialized)

On Tue, Aug 26, 2014 at 2:50 AM, Paul Menzel <
[e-mail]> wrote:
> Dear Charles, dear David,
>
> Am Montag, den 25.08.2014, 11:21 -0600 schrieb David Hubbard:
> > I'm focusing in on this error message first:
> >
> > ACPI Warning: For _SB_.PCI0.LPCB.EC__.LED_: Excess arguments - needs 1,
> > found 2
> >
> > Can you take a look at the .asl files in your coreboot build? I believe
>
> you
> > will find something like "Method (LED, 2, NotSerialized)" which means the
> > LED method wants 2 arguments. Then look for calls to "LED ()" and if you
> > find one only passing 1 argument, that is the problem.
>
> I also noticed that message and quickly looked through the coreboot ASL
> files, but could not find anything calling that message. Looking at the
> Linux kernel sources I was also unable to find a call site. But I’d say
> the OS call this method incorrectly.
>
> […]
>
> Thanks,
>
> Paul

IBM/Lenovo X60t: LED_: Excess arguments - needs 1, found 2

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 26.08.2014 08:50, Paul Menzel wrote:
> Dear Charles, dear David,
>
> Am Montag, den 25.08.2014, 11:21 -0600 schrieb David Hubbard:
> > I'm focusing in on this error message first:
> >
> > ACPI Warning: For _SB_.PCI0.LPCB.EC__.LED_: Excess arguments - needs 1,
> > found 2
> >
> > Can you take a look at the .asl files in your coreboot build? I believe you
> > will find something like "Method (LED, 2, NotSerialized)" which means the
> > LED method wants 2 arguments. Then look for calls to "LED ()" and if you
> > find one only passing 1 argument, that is the problem.
>
> I also noticed that message and quickly looked through the coreboot ASL
> files, but could not find anything calling that message. Looking at the
> Linux kernel sources I was also unable to find a call site. But I’d say
> the OS call this method incorrectly.

It's called by acpi_thinkpad module:
        status = acpi_get_handle(ec_handle, "LED", &led_handle);

if (!acpi_evalf(led_handle, NULL, NULL, "vdd",
                                led, led_led_arg1[ledstatus]))
        static const unsigned int led_led_arg1[] = { 0, 0x80, 0xc0 };

So first argument is 0-based LED number and the second is 0/0x80/0xc0
for state which matches EC bits pretty closely (up to some shifts).
We probably should rename out LED method to sth else and provide a
compatible LED method.
Technically whole acpi_thinkpad is outside of ACPI spec but it's needed
to use some features like Lock hotkey which is outside of ACPI spec as well.
> […]
>
> Thanks,
>
> Paul