Bug 207015
| Summary: | Do not configure two power buttons with buggy firmware | ||
|---|---|---|---|
| Product: | ACPI | Reporter: | Paul Menzel (pmenzel+bugzilla.kernel.org) |
| Component: | ACPICA-Core | Assignee: | acpi_acpica-core (acpi_acpica-core) |
| Status: | CLOSED DOCUMENTED | ||
| Severity: | normal | CC: | rui.zhang |
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | 5.6-rc7 | Subsystem: | |
| Regression: | No | Bisected commit-id: | |
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. 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. |
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