Bug 207015 - Do not configure two power buttons with buggy firmware
Summary: Do not configure two power buttons with buggy firmware
Status: CLOSED DOCUMENTED
Alias: None
Product: ACPI
Classification: Unclassified
Component: ACPICA-Core (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: acpi_acpica-core@kernel-bugs.osdl.org
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-03-29 13:02 UTC by Paul Menzel
Modified: 2021-01-03 15:00 UTC (History)
1 user (show)

See Also:
Kernel Version: 5.6-rc7
Subsystem:
Regression: No
Bisected commit-id:


Attachments

Description Paul Menzel 2020-03-29 13:02:33 UTC
Most firmware seems to violate the ACPI spec (Table 4-13 Power Button Support) by configuring two power buttons.

1.  On a desktop board:

```
[…]
[    0.000000] DMI: Micro-Star International Co., Ltd. MS-7A37/B350M MORTAR (MS-7A37), BIOS 1.MR 12/02/2019
[…]
[    7.272188] input: Power Button as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0C:00/input/input2
[    7.272247] ACPI: Power Button [PWRB]
[    7.272294] input: Power Button as /devices/LNXSYSTM:00/LNXPWRBN:00/input/input3
[    7.272342] ACPI: Power Button [PWRF]
```

2.  On a laptop (no idea where the two buttons come from here):

```
[…]
[    0.000000] DMI: Dell Inc. Latitude E7250/0TVD2T, BIOS A19 01/23/2018
[…]
[    6.638110] calling  acpi_button_driver_init+0x0/0x1000 [button] @ 323
[    6.638232] input: Lid Switch as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0D:00/input/input3
[    6.639672] ACPI: Lid Switch [LID0]
[    6.639684] probe of PNP0C0D:00 returned 1 after 1502 usecs
[    6.639730] input: Power Button as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0C:00/input/input4
[    6.639799] ACPI: Power Button [PBTN]
[    6.639807] probe of PNP0C0C:00 returned 1 after 114 usecs
[    6.639841] input: Sleep Button as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0E:00/input/input5
[    6.639888] ACPI: Sleep Button [SBTN]
[    6.639897] probe of PNP0C0E:00 returned 1 after 87 usecs
[    6.639938] input: Power Button as /devices/LNXSYSTM:00/LNXPWRBN:00/input/input6
[    6.639990] ACPI: Power Button [PWRF]
[    6.640003] probe of LNXPWRBN:00 returned 1 after 95 usecs
[    6.640012] initcall acpi_button_driver_init+0x0/0x1000 [button] returned 0 after 510 usecs
```

If an ACPI PNP0C0C device, the PWR_Button field in FACP should not be zero.

Furquan noticed this for coreboot [1], and fwts also added a check for this.

The Linux kernel should also warn about this firmware bug, and only set up the fixed hardware power button, if the field in FACP is zero.

The Linux code below would need to be adapted, but I wonder how to read the FACP state.

```
        if (!strcmp(hid, ACPI_BUTTON_HID_POWER) ||
            !strcmp(hid, ACPI_BUTTON_HID_POWERF)) {
                button->type = ACPI_BUTTON_TYPE_POWER;
                strcpy(name, ACPI_BUTTON_DEVICE_NAME_POWER);
                sprintf(class, "%s/%s",
                        ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_POWER);
```

This way udev and other user space programs also would not have to configure the incorrect button device.

[1]: https://review.coreboot.org/c/coreboot/+/27272
[2]: https://git.launchpad.net/fwts/commit/?id=8a93b82ce64d1e23bfd8943707e8429a693e2a97
Comment 1 Zhang Rui 2020-11-19 11:27:48 UTC
I think kernel does not enumerate the Fixed power button device when PWR_BUTTON is set to 1, so we're okay in this case.

But according to the ACPI spec about "PWR_BUTTON",
"A zero indicates the power button is handled as a fixed feature
programming model; a one indicates the power button is
handled as a control method device. If the system does not have
a power button, this value would be “1” and no power button
device would be present.
Independent of the value of this field, the presence of a power
button device in the namespace indicates to OSPM that the
power button is handled as a control method device."

So when PWR_BUTTON is 0, and there is control method power button device in ACPI namespace, we should enumerate and probe both, right?

I have the impression that in some cases, control method power button is needed, because the power button event is SCI -> EC GPE -> ACPI Notify(PWRB), but I'm not sure if FADT PWR_BUTTON flag is set to 1 or not in this case.
Comment 2 Zhang Rui 2021-01-03 15:00:19 UTC
Hi, Paul,

based on the ACPI spec, it seems that Linux is doing the right thing, aka, always registering the control method power button devices, even if  PWR_BUTTON is 0.
So this bug is closed. Please feel free to reopen it if you still have any questions.

Note You need to log in before you can comment on or make changes to this bug.