Re: [coreboot] IBM x60t test - DSDT is in fact incomplete

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 25.08.2014 04:39, Charles Devereaux wrote:
> Hello
>
> Previously
> (http://www.coreboot.org/pipermail/coreboot/2014-July/078320.html) I
> mentioned that 3 bugs seemed to be related to the DSDT:
>  - missing ACPI events when the stylus is inserted/removed
> How is the OS supposed to react to them?
>  - errors when trying to make the leds blink with tpacpi
> details? usecase?
>  - errors during TPM initialization
> Some people here would call it a feature.
> Ideally, the DSDT should be fixed within coreboot, but this goes beyond
> my present abilities.
> Not true. Just do the same changes to the corresponding *.asl files in
> coreboot repo and send the patch to gerrit. Other than a layer of
> preprocessing, it's exactly the same code as you got from disassembly.

IBM x60t test - DSDT is in fact incomplete

[Charles Devereaux]

Hello


On Mon, Aug 25, 2014 at 2:42 AM, Vladimir 'φ-coder/phcoder' Serbinenko <
[e-mail]> wrote:
> > - missing ACPI events when the stylus is inserted/removed
> > How is the OS supposed to react to them?

Issue stylus insert and remove messages which can then be mapped to scripts

[ 1778.225577] acpid[440]: completed netlink event "ibm/hotkey IBM0068:00
00000080 0000500c"

[ 1778.734936] acpid[440]: received netlink event "ibm/hotkey IBM0068:00
00000080 0000500b"
> > - errors when trying to make the leds blink with tpacpi
> > details? usecase?

echo "0 blink" > /proc/acpi/ibm/led

Currently fails with :

[  104.727409] ACPI Warning: For \_SB_.PCI0.LPCB.EC__.LED_: Excess
arguments - needs 1, found 2 (20130328/nspredef-272)
Blinking leds are a simple way to do many things (ex: indicate successful
system boot on a screenless computer)
> - errors during TPM initialization
> Some people here would call it a feature.

I do not call non working hardware a feature, especially if linux drivers
exist.
Letting users be able to use or not use their hardware in anyway they chose
is a feature.

TPM is just a glorified way to store hashes, but it could have some uses
say.
> Ideally, the DSDT should be fixed within coreboot, but this goes beyond
> > my present abilities.
> > Not true. Just do the same changes to the corresponding *.asl files in
> > coreboot repo and send the patch to gerrit. Other than a layer of
> > preprocessing, it's exactly the same code as you got from disassembly.

I did not realize that and thought it interacted with static defines - and
so I would have to create a coreboot version-specific DSDT. Thanks. I will
see if I can have that work.

For the ACPI Warning about SystemIO, I have no idea though.

Charles

IBM x60t test - DSDT is in fact incomplete

[Vladimir 'φ-coder/phcoder' Serbinenko]

> > Ideally, the DSDT should be fixed within coreboot, but this goes beyond
> > my present abilities.
> > Not true. Just do the same changes to the corresponding *.asl files in
> > coreboot repo and send the patch to gerrit. Other than a layer of
> > preprocessing, it's exactly the same code as you got from disassembly.

Sorry, I misread you. I thought that you extracted coreboot DSDT from
running system then patched it and used as custom DSDT. I'm going to
make few experiments on my x220t.

IBM x60t test - DSDT is in fact incomplete

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 25.08.2014 22:53, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> > > Ideally, the DSDT should be fixed within coreboot, but this goes beyond
> > > my present abilities.
> > > Not true. Just do the same changes to the corresponding *.asl files in
> > > coreboot repo and send the patch to gerrit. Other than a layer of
> > > preprocessing, it's exactly the same code as you got from disassembly.
>
> Sorry, I misread you. I thought that you extracted coreboot DSDT from
> running system then patched it and used as custom DSDT. I'm going to
> make few experiments on my x220t.

This may interest you:
On X220t
stylus removal:
[17424.931729] ACPI : EC: ===== TASK =====
[17424.931747] ACPI : EC: ---> status = 0x28
[17424.931755] ACPI : EC: <--- command = 0x84
[17424.931852] ACPI : EC: ===== IRQ =====
[17424.931865] ACPI : EC: ---> status = 0x09
[17424.931874] ACPI : EC: ---> data = 0x5d
[17424.931885] ACPI : EC: ---> status = 0x08

So it's _Q5D

Stylus reinsert:
[17493.249126] ACPI : EC: push gpe query to the queue
[17493.249198] ACPI : EC: ===== TASK =====
[17493.249207] ACPI : EC: ---> status = 0x28
[17493.249213] ACPI : EC: <--- command = 0x84
[17493.249293] ACPI : EC: ===== IRQ =====
[17493.249306] ACPI : EC: ---> status = 0x09
[17493.249316] ACPI : EC: ---> data = 0x5c
[17493.249329] ACPI : EC: ---> status = 0x08

So it's _Q5C

Turning LID around:
[17582.701907] ACPI : EC: push gpe query to the queue
[17582.701979] ACPI : EC: ===== TASK =====
[17582.701987] ACPI : EC: ---> status = 0x28
[17582.701994] ACPI : EC: <--- command = 0x84
[17582.702075] ACPI : EC: ===== IRQ =====
[17582.702092] ACPI : EC: ---> status = 0x09
[17582.702096] ACPI : EC: ---> data = 0x5e
[17582.702104] ACPI : EC: ---> status = 0x08
So it's _Q5E

Back to laptop layout:
[17590.610440] ACPI : EC: push gpe query to the queue
[17590.610513] ACPI : EC: ===== TASK =====
[17590.610521] ACPI : EC: ---> status = 0x28
[17590.610527] ACPI : EC: <--- command = 0x84
[17590.610610] ACPI : EC: ===== IRQ =====
[17590.610620] ACPI : EC: ---> status = 0x09
[17590.610628] ACPI : EC: ---> data = 0x5f
[17590.610641] ACPI : EC: ---> status = 0x08
so it's _Q5F

Do you get the same events on X60t?
> From thinkpad-acpi.c:
>         TP_HKEY_EV_TABLET_TABLET        = 0x5009, /* tablet swivel up */
>         TP_HKEY_EV_TABLET_NOTEBOOK      = 0x500a, /* tablet swivel down */
>         TP_HKEY_EV_PEN_INSERTED         = 0x500b, /* tablet pen inserted */
>         TP_HKEY_EV_PEN_REMOVED          = 0x500c, /* tablet pen removed */

So those are the values MHKP has to return.

http://review.coreboot.org/6765 implements it. Please test.

IBM x60t test - DSDT is in fact incomplete

[Charles Devereaux]

Hello

For your information, this patch does not gives me any of these ACPI events
on the x60 tablet, so the patch may be specific to the X220t

To correct the events value, I tried to replicate your events output with :
enable CONFIG_DYNAMIC_DEBUG
echo -n 'file ec.c +p' | sudo tee /sys/kernel/debug/dynamic_debug/control

I still can't get logs similar to yours.

I would love to make that work. Please let me know how I can report the
correct values.

Thanks
Charles

On Mon, Aug 25, 2014 at 5:02 PM, Vladimir 'φ-coder/phcoder' Serbinenko <
[e-mail]> wrote:
> On 25.08.2014 22:53, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> > > > Ideally, the DSDT should be fixed within coreboot, but this goes beyond
> > > > my present abilities.
> > > > Not true. Just do the same changes to the corresponding *.asl files in
> > > > coreboot repo and send the patch to gerrit. Other than a layer of
> > > > preprocessing, it's exactly the same code as you got from disassembly.
> >
> > Sorry, I misread you. I thought that you extracted coreboot DSDT from
> > running system then patched it and used as custom DSDT. I'm going to
> > make few experiments on my x220t.
>
> This may interest you:
> On X220t
> stylus removal:
> [17424.931729] ACPI : EC: ===== TASK =====
> [17424.931747] ACPI : EC: ---> status = 0x28
> [17424.931755] ACPI : EC: <--- command = 0x84
> [17424.931852] ACPI : EC: ===== IRQ =====
> [17424.931865] ACPI : EC: ---> status = 0x09
> [17424.931874] ACPI : EC: ---> data = 0x5d
> [17424.931885] ACPI : EC: ---> status = 0x08
>
> So it's _Q5D
>
> Stylus reinsert:
> [17493.249126] ACPI : EC: push gpe query to the queue
> [17493.249198] ACPI : EC: ===== TASK =====
> [17493.249207] ACPI : EC: ---> status = 0x28
> [17493.249213] ACPI : EC: <--- command = 0x84
> [17493.249293] ACPI : EC: ===== IRQ =====
> [17493.249306] ACPI : EC: ---> status = 0x09
> [17493.249316] ACPI : EC: ---> data = 0x5c
> [17493.249329] ACPI : EC: ---> status = 0x08
>
> So it's _Q5C
>
> Turning LID around:
> [17582.701907] ACPI : EC: push gpe query to the queue
> [17582.701979] ACPI : EC: ===== TASK =====
> [17582.701987] ACPI : EC: ---> status = 0x28
> [17582.701994] ACPI : EC: <--- command = 0x84
> [17582.702075] ACPI : EC: ===== IRQ =====
> [17582.702092] ACPI : EC: ---> status = 0x09
> [17582.702096] ACPI : EC: ---> data = 0x5e
> [17582.702104] ACPI : EC: ---> status = 0x08
> So it's _Q5E
>
> Back to laptop layout:
> [17590.610440] ACPI : EC: push gpe query to the queue
> [17590.610513] ACPI : EC: ===== TASK =====
> [17590.610521] ACPI : EC: ---> status = 0x28
> [17590.610527] ACPI : EC: <--- command = 0x84
> [17590.610610] ACPI : EC: ===== IRQ =====
> [17590.610620] ACPI : EC: ---> status = 0x09
> [17590.610628] ACPI : EC: ---> data = 0x5f
> [17590.610641] ACPI : EC: ---> status = 0x08
> so it's _Q5F
>
> Do you get the same events on X60t?
>
> From thinkpad-acpi.c:
>         TP_HKEY_EV_TABLET_TABLET        = 0x5009, /* tablet swivel up */
>         TP_HKEY_EV_TABLET_NOTEBOOK      = 0x500a, /* tablet swivel down */
>         TP_HKEY_EV_PEN_INSERTED         = 0x500b, /* tablet pen inserted */
>         TP_HKEY_EV_PEN_REMOVED          = 0x500c, /* tablet pen removed */
>
> So those are the values MHKP has to return.
>
> http://review.coreboot.org/6765 implements it. Please test.