Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

ACPI / EC: Fix regression related to PM ops support in ECDT device

On platforms (ASUS X550ZE and possibly all ASUS X series) with valid ECDT
EC but invalid DSDT EC, EC PM ops won't be invoked as ECDT EC is not an
ACPI device. Thus the following commit actually removed post-resume
acpi_ec_enable_event() invocation for such platforms, and triggered a
regression on them that after being resumed, EC (actually should be ECDT)
driver stops handling EC events:

Commit: c2b46d679b30c5c0d7eb47a21085943242bdd8dc
Subject: ACPI / EC: Add PM operations to improve event handling for resume process

Notice that the root cause actually is "ECDT is not an ACPI device" rather
than "the timing of acpi_ec_enable_event() invocation", this patch fixes
this issue by enumerating ECDT EC as an ACPI device. Due to the existence
of the noirq stage, the ability of tuning the timing of
acpi_ec_enable_event() invocation is still meaningful.

This patch is a little bit different from the posted fix by moving
acpi_config_boot_ec() from acpi_ec_ecdt_start() to acpi_ec_add() to make
sure that EC event handling won't be stopped as long as the ACPI EC driver
is bound. Thus the following sequence shouldn't disable EC event handling:
unbind,suspend,resume,bind.

Fixes: c2b46d679b30 (ACPI / EC: Add PM operations to improve event handling for resume process)
Link: https://bugzilla.kernel.org/show_bug.cgi?id=196847
Reported-by: Luya Tshimbalanga <luya@fedoraproject.org>
Tested-by: Luya Tshimbalanga <luya@fedoraproject.org>
Cc: 4.9+ <stable@vger.kernel.org> # 4.9+
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

authored by

Lv Zheng and committed by
Rafael J. Wysocki
a64a62ce 53c5eaab

+69 -24
+45 -24
drivers/acpi/ec.c
··· 1597 1597 { 1598 1598 struct acpi_ec *ec = NULL; 1599 1599 int ret; 1600 + bool is_ecdt = false; 1601 + acpi_status status; 1600 1602 1601 1603 strcpy(acpi_device_name(device), ACPI_EC_DEVICE_NAME); 1602 1604 strcpy(acpi_device_class(device), ACPI_EC_CLASS); 1603 1605 1604 - ec = acpi_ec_alloc(); 1605 - if (!ec) 1606 - return -ENOMEM; 1607 - if (ec_parse_device(device->handle, 0, ec, NULL) != 1608 - AE_CTRL_TERMINATE) { 1606 + if (!strcmp(acpi_device_hid(device), ACPI_ECDT_HID)) { 1607 + is_ecdt = true; 1608 + ec = boot_ec; 1609 + } else { 1610 + ec = acpi_ec_alloc(); 1611 + if (!ec) 1612 + return -ENOMEM; 1613 + status = ec_parse_device(device->handle, 0, ec, NULL); 1614 + if (status != AE_CTRL_TERMINATE) { 1609 1615 ret = -EINVAL; 1610 1616 goto err_alloc; 1617 + } 1611 1618 } 1612 1619 1613 1620 if (acpi_is_boot_ec(ec)) { 1614 - boot_ec_is_ecdt = false; 1615 - /* 1616 - * Trust PNP0C09 namespace location rather than ECDT ID. 1617 - * 1618 - * But trust ECDT GPE rather than _GPE because of ASUS quirks, 1619 - * so do not change boot_ec->gpe to ec->gpe. 1620 - */ 1621 - boot_ec->handle = ec->handle; 1622 - acpi_handle_debug(ec->handle, "duplicated.\n"); 1623 - acpi_ec_free(ec); 1624 - ec = boot_ec; 1625 - ret = acpi_config_boot_ec(ec, ec->handle, true, false); 1621 + boot_ec_is_ecdt = is_ecdt; 1622 + if (!is_ecdt) { 1623 + /* 1624 + * Trust PNP0C09 namespace location rather than 1625 + * ECDT ID. But trust ECDT GPE rather than _GPE 1626 + * because of ASUS quirks, so do not change 1627 + * boot_ec->gpe to ec->gpe. 1628 + */ 1629 + boot_ec->handle = ec->handle; 1630 + acpi_handle_debug(ec->handle, "duplicated.\n"); 1631 + acpi_ec_free(ec); 1632 + ec = boot_ec; 1633 + } 1634 + ret = acpi_config_boot_ec(ec, ec->handle, true, is_ecdt); 1626 1635 } else 1627 1636 ret = acpi_ec_setup(ec, true); 1628 1637 if (ret) ··· 1644 1635 ret = !!request_region(ec->command_addr, 1, "EC cmd"); 1645 1636 WARN(!ret, "Could not request EC cmd io port 0x%lx", ec->command_addr); 1646 1637 1647 - /* Reprobe devices depending on the EC */ 1648 - acpi_walk_dep_device_list(ec->handle); 1638 + if (!is_ecdt) { 1639 + /* Reprobe devices depending on the EC */ 1640 + acpi_walk_dep_device_list(ec->handle); 1641 + } 1649 1642 acpi_handle_debug(ec->handle, "enumerated.\n"); 1650 1643 return 0; 1651 1644 ··· 1703 1692 1704 1693 static const struct acpi_device_id ec_device_ids[] = { 1705 1694 {"PNP0C09", 0}, 1695 + {ACPI_ECDT_HID, 0}, 1706 1696 {"", 0}, 1707 1697 }; 1708 1698 ··· 1776 1764 * Note: ec->handle can be valid if this function is called after 1777 1765 * acpi_ec_add(), hence the fast path. 1778 1766 */ 1779 - if (boot_ec->handle != ACPI_ROOT_OBJECT) 1780 - handle = boot_ec->handle; 1781 - else if (!acpi_ec_ecdt_get_handle(&handle)) 1782 - return -ENODEV; 1783 - return acpi_config_boot_ec(boot_ec, handle, true, true); 1767 + if (boot_ec->handle == ACPI_ROOT_OBJECT) { 1768 + if (!acpi_ec_ecdt_get_handle(&handle)) 1769 + return -ENODEV; 1770 + boot_ec->handle = handle; 1771 + } 1772 + 1773 + /* Register to ACPI bus with PM ops attached */ 1774 + return acpi_bus_register_early_device(ACPI_BUS_TYPE_ECDT_EC); 1784 1775 } 1785 1776 1786 1777 #if 0 ··· 2035 2020 2036 2021 /* Drivers must be started after acpi_ec_query_init() */ 2037 2022 dsdt_fail = acpi_bus_register_driver(&acpi_ec_driver); 2023 + /* 2024 + * Register ECDT to ACPI bus only when PNP0C09 probe fails. This is 2025 + * useful for platforms (confirmed on ASUS X550ZE) with valid ECDT 2026 + * settings but invalid DSDT settings. 2027 + * https://bugzilla.kernel.org/show_bug.cgi?id=196847 2028 + */ 2038 2029 ecdt_fail = acpi_ec_ecdt_start(); 2039 2030 return ecdt_fail && dsdt_fail ? -ENODEV : 0; 2040 2031 }
+1
drivers/acpi/internal.h
··· 115 115 bool acpi_device_is_battery(struct acpi_device *adev); 116 116 bool acpi_device_is_first_physical_node(struct acpi_device *adev, 117 117 const struct device *dev); 118 + int acpi_bus_register_early_device(int type); 118 119 119 120 /* -------------------------------------------------------------------------- 120 121 Device Matching and Notification
+21
drivers/acpi/scan.c
··· 1024 1024 case ACPI_BUS_TYPE_SLEEP_BUTTON: 1025 1025 strcpy(device->pnp.bus_id, "SLPF"); 1026 1026 break; 1027 + case ACPI_BUS_TYPE_ECDT_EC: 1028 + strcpy(device->pnp.bus_id, "ECDT"); 1029 + break; 1027 1030 default: 1028 1031 acpi_get_name(device->handle, ACPI_SINGLE_NAME, &buffer); 1029 1032 /* Clean up trailing underscores (if any) */ ··· 1306 1303 break; 1307 1304 case ACPI_BUS_TYPE_SLEEP_BUTTON: 1308 1305 acpi_add_id(pnp, ACPI_BUTTON_HID_SLEEPF); 1306 + break; 1307 + case ACPI_BUS_TYPE_ECDT_EC: 1308 + acpi_add_id(pnp, ACPI_ECDT_HID); 1309 1309 break; 1310 1310 } 1311 1311 } ··· 2054 2048 acpi_device_clear_enumerated(adev); 2055 2049 } 2056 2050 EXPORT_SYMBOL_GPL(acpi_bus_trim); 2051 + 2052 + int acpi_bus_register_early_device(int type) 2053 + { 2054 + struct acpi_device *device = NULL; 2055 + int result; 2056 + 2057 + result = acpi_add_single_object(&device, NULL, 2058 + type, ACPI_STA_DEFAULT); 2059 + if (result) 2060 + return result; 2061 + 2062 + device->flags.match_driver = true; 2063 + return device_attach(&device->dev); 2064 + } 2065 + EXPORT_SYMBOL_GPL(acpi_bus_register_early_device); 2057 2066 2058 2067 static int acpi_bus_scan_fixed(void) 2059 2068 {
+1
include/acpi/acpi_bus.h
··· 105 105 ACPI_BUS_TYPE_THERMAL, 106 106 ACPI_BUS_TYPE_POWER_BUTTON, 107 107 ACPI_BUS_TYPE_SLEEP_BUTTON, 108 + ACPI_BUS_TYPE_ECDT_EC, 108 109 ACPI_BUS_DEVICE_TYPE_COUNT 109 110 }; 110 111
+1
include/acpi/acpi_drivers.h
··· 58 58 #define ACPI_VIDEO_HID "LNXVIDEO" 59 59 #define ACPI_BAY_HID "LNXIOBAY" 60 60 #define ACPI_DOCK_HID "LNXDOCK" 61 + #define ACPI_ECDT_HID "LNXEC" 61 62 /* Quirk for broken IBM BIOSes */ 62 63 #define ACPI_SMBUS_IBM_HID "SMBUSIBM" 63 64