From 065bd4d35b3fb4484c61fc40a51eeffd5abe52e8 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 26 Oct 2019 22:24:31 +0200 Subject: ACPI: button: Refactor lid_init_state module parsing code Replace the weird strncmp() calls in param_set_lid_init_state(), which look to me like they will also accept things like "opennnn" to use sysfs_match_string instead. Also rewrite param_get_lid_init_state() using the new lid_init_state_str array. Instead of doing a straightforward one line replacement, e.g. : return sprintf(buffer, lid_init_state_str[lid_init_state]); print all possible values, putting [] around the selected value, so that users can easily find out what the possible values are. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Rafael J. Wysocki --- drivers/acpi/button.c | 62 ++++++++++++++++++++++++++------------------------- 1 file changed, 32 insertions(+), 30 deletions(-) (limited to 'drivers/acpi/button.c') diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c index 4a2cde2c536a..121d747a840c 100644 --- a/drivers/acpi/button.c +++ b/drivers/acpi/button.c @@ -44,9 +44,17 @@ #define ACPI_BUTTON_DEVICE_NAME_LID "Lid Switch" #define ACPI_BUTTON_TYPE_LID 0x05 -#define ACPI_BUTTON_LID_INIT_IGNORE 0x00 -#define ACPI_BUTTON_LID_INIT_OPEN 0x01 -#define ACPI_BUTTON_LID_INIT_METHOD 0x02 +enum { + ACPI_BUTTON_LID_INIT_IGNORE, + ACPI_BUTTON_LID_INIT_OPEN, + ACPI_BUTTON_LID_INIT_METHOD, +}; + +static const char * const lid_init_state_str[] = { + [ACPI_BUTTON_LID_INIT_IGNORE] = "ignore", + [ACPI_BUTTON_LID_INIT_OPEN] = "open", + [ACPI_BUTTON_LID_INIT_METHOD] = "method", +}; #define _COMPONENT ACPI_BUTTON_COMPONENT ACPI_MODULE_NAME("button"); @@ -578,36 +586,30 @@ static int acpi_button_remove(struct acpi_device *device) static int param_set_lid_init_state(const char *val, const struct kernel_param *kp) { - int result = 0; - - if (!strncmp(val, "open", sizeof("open") - 1)) { - lid_init_state = ACPI_BUTTON_LID_INIT_OPEN; - pr_info("Notify initial lid state as open\n"); - } else if (!strncmp(val, "method", sizeof("method") - 1)) { - lid_init_state = ACPI_BUTTON_LID_INIT_METHOD; - pr_info("Notify initial lid state with _LID return value\n"); - } else if (!strncmp(val, "ignore", sizeof("ignore") - 1)) { - lid_init_state = ACPI_BUTTON_LID_INIT_IGNORE; - pr_info("Do not notify initial lid state\n"); - } else - result = -EINVAL; - return result; + int i; + + i = sysfs_match_string(lid_init_state_str, val); + if (i < 0) + return i; + + lid_init_state = i; + pr_info("Initial lid state set to '%s'\n", lid_init_state_str[i]); + return 0; } -static int param_get_lid_init_state(char *buffer, - const struct kernel_param *kp) +static int param_get_lid_init_state(char *buf, const struct kernel_param *kp) { - switch (lid_init_state) { - case ACPI_BUTTON_LID_INIT_OPEN: - return sprintf(buffer, "open"); - case ACPI_BUTTON_LID_INIT_METHOD: - return sprintf(buffer, "method"); - case ACPI_BUTTON_LID_INIT_IGNORE: - return sprintf(buffer, "ignore"); - default: - return sprintf(buffer, "invalid"); - } - return 0; + int i, c = 0; + + for (i = 0; i < ARRAY_SIZE(lid_init_state_str); i++) + if (i == lid_init_state) + c += sprintf(buf + c, "[%s] ", lid_init_state_str[i]); + else + c += sprintf(buf + c, "%s ", lid_init_state_str[i]); + + buf[c - 1] = '\n'; /* Replace the final space with a newline */ + + return c; } module_param_call(lid_init_state, -- cgit v1.2.3 From 593681e2c75f59f23cf6f6cefc4f00cae2a4522b Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 26 Oct 2019 22:24:32 +0200 Subject: ACPI: button: Allow disabling LID support with the lid_init_state module option Add a new "disabled" value for the lid_init_state module option, which can be used to disable LID support on devices where it is completely broken. Sometimes devices seem to spontaneously suspend and the cause for this is not clear. The LID switch is known to be one possible cause for this, this commit allows easily disabling the LID switch for testing if it is the cause. For example some devices which do not even have a lid, still have a LID device in their ACPI tables, pointing to a floating GPIO. This is not really related to the initial LID state, but re-using the existing option keeps things simple and it will make it much easier to add DMI quirks which can either disable the LID completely or set another non-default lid_init_state value, both of which are necessary on some devices. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Rafael J. Wysocki --- drivers/acpi/button.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'drivers/acpi/button.c') diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c index 121d747a840c..7f69d8d1905b 100644 --- a/drivers/acpi/button.c +++ b/drivers/acpi/button.c @@ -48,12 +48,14 @@ enum { ACPI_BUTTON_LID_INIT_IGNORE, ACPI_BUTTON_LID_INIT_OPEN, ACPI_BUTTON_LID_INIT_METHOD, + ACPI_BUTTON_LID_INIT_DISABLED, }; static const char * const lid_init_state_str[] = { [ACPI_BUTTON_LID_INIT_IGNORE] = "ignore", [ACPI_BUTTON_LID_INIT_OPEN] = "open", [ACPI_BUTTON_LID_INIT_METHOD] = "method", + [ACPI_BUTTON_LID_INIT_DISABLED] = "disabled", }; #define _COMPONENT ACPI_BUTTON_COMPONENT @@ -480,7 +482,9 @@ static int acpi_button_add(struct acpi_device *device) char *name, *class; int error; - if (!strcmp(hid, ACPI_BUTTON_HID_LID) && dmi_check_system(lid_blacklst)) + if (!strcmp(hid, ACPI_BUTTON_HID_LID) && + (dmi_check_system(lid_blacklst) || + lid_init_state == ACPI_BUTTON_LID_INIT_DISABLED)) return -ENODEV; button = kzalloc(sizeof(struct acpi_button), GFP_KERNEL); -- cgit v1.2.3 From d7cd08231a7fafb0d81786515527651d3242a7f4 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 26 Oct 2019 22:24:33 +0200 Subject: ACPI: button: Turn lid_blacklst DMI table into a generic quirk table Commit 3540c32a9ae4 ("ACPI / button: Add quirks for initial lid state notification") added 3 different modes to the LID handling code to deal with various buggy implementations. Until now users which need one of the 2 non-default modes to get their HW to work have to pass a kernel commandline option for this. E.g. https://bugzilla.kernel.org/show_bug.cgi?id=106151 was closed with a note that the user has to add "button.lid_init_state=open" to the kernel commandline to get the LID code to not cause undesirable suspends on his Samsung N210 Plus. This commit modifies the existing lid_blacklst DMI table so that it can be used not only to completely disable the LID code on devices where the ACPI tables are broken beyond repair, but also to select one of the 2 non default LID handling modes on devices where this is necessary. This will allow us to add quirks to make the LID work OOTB on broken devices. Getting this working OOTB is esp. important because the typical breakage is false LID closed reporting, causing undesirable suspends which basically make the system unusable. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Rafael J. Wysocki --- drivers/acpi/button.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) (limited to 'drivers/acpi/button.c') diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c index 7f69d8d1905b..d83b15bae515 100644 --- a/drivers/acpi/button.c +++ b/drivers/acpi/button.c @@ -75,18 +75,16 @@ static const struct acpi_device_id button_device_ids[] = { }; MODULE_DEVICE_TABLE(acpi, button_device_ids); -/* - * Some devices which don't even have a lid in anyway have a broken _LID - * method (e.g. pointing to a floating gpio pin) causing spurious LID events. - */ -static const struct dmi_system_id lid_blacklst[] = { +/* Please keep this list sorted alphabetically by vendor and model */ +static const struct dmi_system_id dmi_lid_quirks[] = { { - /* GP-electronic T701 */ + /* GP-electronic T701, _LID method points to a floating GPIO */ .matches = { DMI_MATCH(DMI_SYS_VENDOR, "Insyde"), DMI_MATCH(DMI_PRODUCT_NAME, "T701"), DMI_MATCH(DMI_BIOS_VERSION, "BYT70A.YNCHENG.WIN.007"), }, + .driver_data = (void *)(long)ACPI_BUTTON_LID_INIT_DISABLED, }, {} }; @@ -128,7 +126,7 @@ struct acpi_button { static BLOCKING_NOTIFIER_HEAD(acpi_lid_notifier); static struct acpi_device *lid_device; -static u8 lid_init_state = ACPI_BUTTON_LID_INIT_METHOD; +static long lid_init_state = -1; static unsigned long lid_report_interval __read_mostly = 500; module_param(lid_report_interval, ulong, 0644); @@ -483,8 +481,7 @@ static int acpi_button_add(struct acpi_device *device) int error; if (!strcmp(hid, ACPI_BUTTON_HID_LID) && - (dmi_check_system(lid_blacklst) || - lid_init_state == ACPI_BUTTON_LID_INIT_DISABLED)) + lid_init_state == ACPI_BUTTON_LID_INIT_DISABLED) return -ENODEV; button = kzalloc(sizeof(struct acpi_button), GFP_KERNEL); @@ -623,6 +620,16 @@ MODULE_PARM_DESC(lid_init_state, "Behavior for reporting LID initial state"); static int acpi_button_register_driver(struct acpi_driver *driver) { + const struct dmi_system_id *dmi_id; + + if (lid_init_state == -1) { + dmi_id = dmi_first_match(dmi_lid_quirks); + if (dmi_id) + lid_init_state = (long)dmi_id->driver_data; + else + lid_init_state = ACPI_BUTTON_LID_INIT_METHOD; + } + /* * Modules such as nouveau.ko and i915.ko have a link time dependency * on acpi_lid_open(), and would therefore not be loadable on ACPI -- cgit v1.2.3 From 932e1ba486117de2fcea3df27ad8218ad6c11470 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 26 Oct 2019 22:24:34 +0200 Subject: ACPI: button: Add DMI quirk for Medion Akoya E2215T The Medion Akoya E2215T's ACPI _LID implementation is quite broken: 1. For notifications it uses an ActiveLow Edge GpioInt, rather then an ActiveBoth one, meaning that the device is only notified when the lid is closed, not when it is opened. 2. Matching with this its _LID method simply always returns 0 (closed) In order for the Linux LID code to work properly with this implementation, the lid_init_state selection needs to be set to ACPI_BUTTON_LID_INIT_OPEN. This commit adds a DMI quirk for this. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Rafael J. Wysocki --- drivers/acpi/button.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'drivers/acpi/button.c') diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c index d83b15bae515..e4b2aa43265b 100644 --- a/drivers/acpi/button.c +++ b/drivers/acpi/button.c @@ -86,6 +86,17 @@ static const struct dmi_system_id dmi_lid_quirks[] = { }, .driver_data = (void *)(long)ACPI_BUTTON_LID_INIT_DISABLED, }, + { + /* + * Medion Akoya E2215T, notification of the LID device only + * happens on close, not on open and _LID always returns closed. + */ + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "MEDION"), + DMI_MATCH(DMI_PRODUCT_NAME, "E2215T MD60198"), + }, + .driver_data = (void *)(long)ACPI_BUTTON_LID_INIT_OPEN, + }, {} }; -- cgit v1.2.3 From 00e250367cc6c4ab80fea6ec605d464e624bd520 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 26 Oct 2019 22:24:35 +0200 Subject: ACPI: button: Add DMI quirk for Asus T200TA The Asus T200TA lid has some weird behavior where _LID keeps reporting closed after every second openening of the lid. Causing immediate re-suspend after opening every other open. I've looked at the AML code but it involves talking to the EC and we have no idea what the EC is doing. Setting lid_init_state to ACPI_BUTTON_LID_INIT_OPEN fixes the unwanted behavior, so this commit adds a DMI based quirk to use ACPI_BUTTON_LID_INIT_OPEN on the T200TA. Signed-off-by: Hans de Goede Reviewed-by: Andy Shevchenko Signed-off-by: Rafael J. Wysocki --- drivers/acpi/button.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'drivers/acpi/button.c') diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c index e4b2aa43265b..a090e9542d82 100644 --- a/drivers/acpi/button.c +++ b/drivers/acpi/button.c @@ -77,6 +77,18 @@ MODULE_DEVICE_TABLE(acpi, button_device_ids); /* Please keep this list sorted alphabetically by vendor and model */ static const struct dmi_system_id dmi_lid_quirks[] = { + { + /* + * Asus T200TA, _LID keeps reporting closed after every second + * openening of the lid. Causing immediate re-suspend after + * opening every other open. Using LID_INIT_OPEN fixes this. + */ + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), + DMI_MATCH(DMI_PRODUCT_NAME, "T200TA"), + }, + .driver_data = (void *)(long)ACPI_BUTTON_LID_INIT_OPEN, + }, { /* GP-electronic T701, _LID method points to a floating GPIO */ .matches = { -- cgit v1.2.3 From e346d0cf2c0a2dc9e63d5b90824bbe5ac0cc43e2 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 26 Oct 2019 22:24:36 +0200 Subject: ACPI: button: Remove unused acpi_lid_notifier_[un]register() functions There are no users of the acpi_lid_notifier_[un]register functions, so lets remove them. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Rafael J. Wysocki --- drivers/acpi/button.c | 27 +-------------------------- include/acpi/button.h | 12 ------------ 2 files changed, 1 insertion(+), 38 deletions(-) (limited to 'drivers/acpi/button.c') diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c index a090e9542d82..d27b01c0323d 100644 --- a/drivers/acpi/button.c +++ b/drivers/acpi/button.c @@ -147,7 +147,6 @@ struct acpi_button { bool suspended; }; -static BLOCKING_NOTIFIER_HEAD(acpi_lid_notifier); static struct acpi_device *lid_device; static long lid_init_state = -1; @@ -177,7 +176,6 @@ static int acpi_lid_evaluate_state(struct acpi_device *device) static int acpi_lid_notify_state(struct acpi_device *device, int state) { struct acpi_button *button = acpi_driver_data(device); - int ret; ktime_t next_report; bool do_update; @@ -254,18 +252,7 @@ static int acpi_lid_notify_state(struct acpi_device *device, int state) button->last_time = ktime_get(); } - ret = blocking_notifier_call_chain(&acpi_lid_notifier, state, device); - if (ret == NOTIFY_DONE) - ret = blocking_notifier_call_chain(&acpi_lid_notifier, state, - device); - if (ret == NOTIFY_DONE || ret == NOTIFY_OK) { - /* - * It is also regarded as success if the notifier_chain - * returns NOTIFY_OK or NOTIFY_DONE. - */ - ret = 0; - } - return ret; + return 0; } static int __maybe_unused acpi_button_state_seq_show(struct seq_file *seq, @@ -362,18 +349,6 @@ static int acpi_button_remove_fs(struct acpi_device *device) /* -------------------------------------------------------------------------- Driver Interface -------------------------------------------------------------------------- */ -int acpi_lid_notifier_register(struct notifier_block *nb) -{ - return blocking_notifier_chain_register(&acpi_lid_notifier, nb); -} -EXPORT_SYMBOL(acpi_lid_notifier_register); - -int acpi_lid_notifier_unregister(struct notifier_block *nb) -{ - return blocking_notifier_chain_unregister(&acpi_lid_notifier, nb); -} -EXPORT_SYMBOL(acpi_lid_notifier_unregister); - int acpi_lid_open(void) { if (!lid_device) diff --git a/include/acpi/button.h b/include/acpi/button.h index 3a2b8535dec6..340da7784cc8 100644 --- a/include/acpi/button.h +++ b/include/acpi/button.h @@ -2,21 +2,9 @@ #ifndef ACPI_BUTTON_H #define ACPI_BUTTON_H -#include - #if IS_ENABLED(CONFIG_ACPI_BUTTON) -extern int acpi_lid_notifier_register(struct notifier_block *nb); -extern int acpi_lid_notifier_unregister(struct notifier_block *nb); extern int acpi_lid_open(void); #else -static inline int acpi_lid_notifier_register(struct notifier_block *nb) -{ - return 0; -} -static inline int acpi_lid_notifier_unregister(struct notifier_block *nb) -{ - return 0; -} static inline int acpi_lid_open(void) { return 1; -- cgit v1.2.3 From 90ed9c639c1b53556f87b1c5031c7e4c57285a92 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 18 Nov 2019 16:35:56 +0100 Subject: ACPI: button: Add DMI quirk for Acer Switch 10 SW5-032 lid-switch The Acer Switch 10 SW5-032 _LID method is quite broken, it looks like this: Method (_LID, 0, NotSerialized) // _LID: Lid Status { If ((STAS & One)) { Local0 = One PBCG |= 0x05000000 HMCG |= 0x05000000 } Else { Local0 = Zero PBCG &= 0xF0FFFFFF HMCG &= 0xF0FFFFFF } ^^PCI0.GFX0.CLID = Local0 Return (Local0) } The problem here is the accesses to the PBCG and HMCG, these are the pinconf0 registers for the power, resp. the home button GPIO, e.g. PBCG is declared as: OperationRegion (PWBT, SystemMemory, 0xFED0E080, 0x10) Field (PWBT, DWordAcc, NoLock, Preserve) { PBCG, 32, PBV1, 32, PBSA, 32, PBV2, 32 } Where 0xFED0E000 is the base address of the GPO2 device and 0x80 is the offset for the pin used for the powerbutton. The problem here is this line in _LID: PBCG |= 0x05000000 This changes the trigger flags of the GPIO, changing when it generates interrupts. Note it does not clear the original flags. Linux uses an edge triggered interrupt on both positive and negative edges. This |= adds the BYT_TRIG_LVL flag to this, so now it is turned into a level interrupt which fires both when low and high, iow it simply always fires leading to an interrupt storm, the tablet immediately waking up from suspend again, etc. There is nothing we can do to fix this, except for a DSDT override, which the user needs to do manually. The only thing we can do is never call _LID, which requires disabling the lid-switch functionality altogether. This commit adds a quirk for this, as no lid-switch function is better then the interrupt storm. A user manually applying a DSDT override can also override the quirk on the kernel cmdline. Signed-off-by: Hans de Goede Reviewed-by: Mika Westerberg Reviewed-by: Andy Shevchenko Signed-off-by: Rafael J. Wysocki --- drivers/acpi/button.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'drivers/acpi/button.c') diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c index d27b01c0323d..b758b45737f5 100644 --- a/drivers/acpi/button.c +++ b/drivers/acpi/button.c @@ -77,6 +77,19 @@ MODULE_DEVICE_TABLE(acpi, button_device_ids); /* Please keep this list sorted alphabetically by vendor and model */ static const struct dmi_system_id dmi_lid_quirks[] = { + { + /* + * Acer Switch 10 SW5-012. _LID method messes with home and + * power button GPIO IRQ settings causing an interrupt storm on + * both GPIOs. This is unfixable without a DSDT override, so we + * have to disable the lid-switch functionality altogether :| + */ + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), + DMI_MATCH(DMI_PRODUCT_NAME, "Aspire SW5-012"), + }, + .driver_data = (void *)(long)ACPI_BUTTON_LID_INIT_DISABLED, + }, { /* * Asus T200TA, _LID keeps reporting closed after every second -- cgit v1.2.3