From 56678a5f44ef5f0ad9a67194bbee2280c6286534 Mon Sep 17 00:00:00 2001 From: Alban Bedel Date: Mon, 22 Feb 2021 15:15:59 +0100 Subject: [PATCH 01/16] platform/x86: intel-hid: Support Lenovo ThinkPad X1 Tablet Gen 2 Like a few other system the Lenovo ThinkPad X1 Tablet Gen 2 miss the HEBC method, which prevent the power button from working. Add a quirk to enable the button array on this system family and fix the power button. Signed-off-by: Alban Bedel Tested-by: Alexander Kobel Link: https://lore.kernel.org/r/20210222141559.3775-1-albeu@free.fr Signed-off-by: Hans de Goede --- drivers/platform/x86/intel-hid.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/platform/x86/intel-hid.c b/drivers/platform/x86/intel-hid.c index 2f5b8d09143e3..57cc92891a570 100644 --- a/drivers/platform/x86/intel-hid.c +++ b/drivers/platform/x86/intel-hid.c @@ -90,6 +90,13 @@ static const struct dmi_system_id button_array_table[] = { DMI_MATCH(DMI_PRODUCT_NAME, "HP Spectre x2 Detachable"), }, }, + { + .ident = "Lenovo ThinkPad X1 Tablet Gen 2", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_FAMILY, "ThinkPad X1 Tablet Gen 2"), + }, + }, { } }; From 053191b6a4bc2d429e94e866e3b2bc611d5add31 Mon Sep 17 00:00:00 2001 From: Mark Pearson Date: Thu, 11 Mar 2021 12:48:43 -0500 Subject: [PATCH 02/16] platform/x86: thinkpad_acpi: check dytc version for lapmode sysfs Lenovo platforms with DYTC versions earlier than version 5 don't set the lapmode interface correctly, causing issues with thermald on older platforms. Add checking to only create the dytc_lapmode interface for version 5 and later. Fixes: 1ac09656bded ("platform/x86: thinkpad_acpi: Add palm sensor support") Signed-off-by: Mark Pearson Link: https://lore.kernel.org/r/20210311174843.3161-1-markpearson@lenovo.com Signed-off-by: Hans de Goede --- drivers/platform/x86/thinkpad_acpi.c | 91 ++++++++++++++++++++-------- 1 file changed, 65 insertions(+), 26 deletions(-) diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c index b881044b31b0b..f7de90a47e28b 100644 --- a/drivers/platform/x86/thinkpad_acpi.c +++ b/drivers/platform/x86/thinkpad_acpi.c @@ -9845,6 +9845,11 @@ static struct ibm_struct lcdshadow_driver_data = { * Thinkpad sensor interfaces */ +#define DYTC_CMD_QUERY 0 /* To get DYTC status - enable/revision */ +#define DYTC_QUERY_ENABLE_BIT 8 /* Bit 8 - 0 = disabled, 1 = enabled */ +#define DYTC_QUERY_SUBREV_BIT 16 /* Bits 16 - 27 - sub revision */ +#define DYTC_QUERY_REV_BIT 28 /* Bits 28 - 31 - revision */ + #define DYTC_CMD_GET 2 /* To get current IC function and mode */ #define DYTC_GET_LAPMODE_BIT 17 /* Set when in lapmode */ @@ -9855,6 +9860,7 @@ static bool has_palmsensor; static bool has_lapsensor; static bool palm_state; static bool lap_state; +static int dytc_version; static int dytc_command(int command, int *output) { @@ -9869,6 +9875,33 @@ static int dytc_command(int command, int *output) return 0; } +static int dytc_get_version(void) +{ + int err, output; + + /* Check if we've been called before - and just return cached value */ + if (dytc_version) + return dytc_version; + + /* Otherwise query DYTC and extract version information */ + err = dytc_command(DYTC_CMD_QUERY, &output); + /* + * If support isn't available (ENODEV) then don't return an error + * and don't create the sysfs group + */ + if (err == -ENODEV) + return 0; + /* For all other errors we can flag the failure */ + if (err) + return err; + + /* Check DYTC is enabled and supports mode setting */ + if (output & BIT(DYTC_QUERY_ENABLE_BIT)) + dytc_version = (output >> DYTC_QUERY_REV_BIT) & 0xF; + + return 0; +} + static int lapsensor_get(bool *present, bool *state) { int output, err; @@ -9974,7 +10007,18 @@ static int tpacpi_proxsensor_init(struct ibm_init_struct *iibm) if (err) return err; } - if (has_lapsensor) { + + /* Check if we know the DYTC version, if we don't then get it */ + if (!dytc_version) { + err = dytc_get_version(); + if (err) + return err; + } + /* + * Platforms before DYTC version 5 claim to have a lap sensor, but it doesn't work, so we + * ignore them + */ + if (has_lapsensor && (dytc_version >= 5)) { err = sysfs_create_file(&tpacpi_pdev->dev.kobj, &dev_attr_dytc_lapmode.attr); if (err) return err; @@ -9999,14 +10043,9 @@ static struct ibm_struct proxsensor_driver_data = { * DYTC Platform Profile interface */ -#define DYTC_CMD_QUERY 0 /* To get DYTC status - enable/revision */ #define DYTC_CMD_SET 1 /* To enable/disable IC function mode */ #define DYTC_CMD_RESET 0x1ff /* To reset back to default */ -#define DYTC_QUERY_ENABLE_BIT 8 /* Bit 8 - 0 = disabled, 1 = enabled */ -#define DYTC_QUERY_SUBREV_BIT 16 /* Bits 16 - 27 - sub revision */ -#define DYTC_QUERY_REV_BIT 28 /* Bits 28 - 31 - revision */ - #define DYTC_GET_FUNCTION_BIT 8 /* Bits 8-11 - function setting */ #define DYTC_GET_MODE_BIT 12 /* Bits 12-15 - mode setting */ @@ -10211,28 +10250,28 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm) if (err) return err; + /* Check if we know the DYTC version, if we don't then get it */ + if (!dytc_version) { + err = dytc_get_version(); + if (err) + return err; + } /* Check DYTC is enabled and supports mode setting */ - if (output & BIT(DYTC_QUERY_ENABLE_BIT)) { - /* Only DYTC v5.0 and later has this feature. */ - int dytc_version; - - dytc_version = (output >> DYTC_QUERY_REV_BIT) & 0xF; - if (dytc_version >= 5) { - dbg_printk(TPACPI_DBG_INIT, - "DYTC version %d: thermal mode available\n", dytc_version); - /* Create platform_profile structure and register */ - err = platform_profile_register(&dytc_profile); - /* - * If for some reason platform_profiles aren't enabled - * don't quit terminally. - */ - if (err) - return 0; + if (dytc_version >= 5) { + dbg_printk(TPACPI_DBG_INIT, + "DYTC version %d: thermal mode available\n", dytc_version); + /* Create platform_profile structure and register */ + err = platform_profile_register(&dytc_profile); + /* + * If for some reason platform_profiles aren't enabled + * don't quit terminally. + */ + if (err) + return 0; - dytc_profile_available = true; - /* Ensure initial values are correct */ - dytc_profile_refresh(); - } + dytc_profile_available = true; + /* Ensure initial values are correct */ + dytc_profile_refresh(); } return 0; } From 3d677f12ea3a2097a16ded570623567403dea959 Mon Sep 17 00:00:00 2001 From: Esteve Varela Colominas Date: Mon, 15 Mar 2021 20:58:24 +0100 Subject: [PATCH 03/16] platform/x86: thinkpad_acpi: Allow the FnLock LED to change state On many recent ThinkPad laptops, there's a new LED next to the ESC key, that indicates the FnLock status. When the Fn+ESC combo is pressed, FnLock is toggled, which causes the Media Key functionality to change, making it so that the media keys either perform their media key function, or function as an F-key by default. The Fn key can be used the access the alternate function at any time. With the current linux kernel, the LED doens't change state if you press the Fn+ESC key combo. However, the media key functionality *does* change. This is annoying, since the LED will stay on if it was on during bootup, and it makes it hard to keep track what the current state of the FnLock is. This patch calls an ACPI function, that gets the current media key state, when the Fn+ESC key combo is pressed. Through testing it was discovered that this function causes the LED to update correctly to reflect the current state when this function is called. The relevant ACPI calls are the following: \_SB_.PCI0.LPC0.EC0_.HKEY.GMKS: Get media key state, returns 0x603 if the FnLock mode is enabled, and 0x602 if it's disabled. \_SB_.PCI0.LPC0.EC0_.HKEY.SMKS: Set media key state, sending a 1 will enable FnLock mode, and a 0 will disable it. Relevant discussion: https://bugzilla.kernel.org/show_bug.cgi?id=207841 https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1881015 Signed-off-by: Esteve Varela Colominas Link: https://lore.kernel.org/r/20210315195823.23212-1-esteve.varela@gmail.com Signed-off-by: Hans de Goede --- drivers/platform/x86/thinkpad_acpi.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c index f7de90a47e28b..8569c1d2a52cd 100644 --- a/drivers/platform/x86/thinkpad_acpi.c +++ b/drivers/platform/x86/thinkpad_acpi.c @@ -4081,13 +4081,19 @@ static bool hotkey_notify_6xxx(const u32 hkey, case TP_HKEY_EV_KEY_NUMLOCK: case TP_HKEY_EV_KEY_FN: - case TP_HKEY_EV_KEY_FN_ESC: /* key press events, we just ignore them as long as the EC * is still reporting them in the normal keyboard stream */ *send_acpi_ev = false; *ignore_acpi_ev = true; return true; + case TP_HKEY_EV_KEY_FN_ESC: + /* Get the media key status to foce the status LED to update */ + acpi_evalf(hkey_handle, NULL, "GMKS", "v"); + *send_acpi_ev = false; + *ignore_acpi_ev = true; + return true; + case TP_HKEY_EV_TABLET_CHANGED: tpacpi_input_send_tabletsw(); hotkey_tablet_mode_notify_change(); From ec0e8fc416f70645608c146dcf90d264b2ad6e3c Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 21 Mar 2021 12:31:08 +0100 Subject: [PATCH 04/16] platform/x86: thinkpad_acpi: Disable DYTC CQL mode around switching to balanced mode Testing has shown that setting /sys/firmware/acpi/platform_profile to "balanced" when /sys/bus/platform/devices/thinkpad_acpi/dytc_lapmode reports 1, causes dytc_lapmode to get reset to 0 and then it becomes stuck at 0 for aprox. 30 minutes even if the laptop is used on a lap. Disabling CQL (when enabled) before issuing the DYTC_CMD_RESET to get back to balanced mode and re-enabling it afterwards again, like the code already does when switching to low-power / performance mode fixes this. Fixes: c3bfcd4c6762 ("platform/x86: thinkpad_acpi: Add platform profile support") Tested-by: Mark Pearson Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20210321113108.7069-1-hdegoede@redhat.com --- drivers/platform/x86/thinkpad_acpi.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c index 8569c1d2a52cd..0d9e2ddbf904d 100644 --- a/drivers/platform/x86/thinkpad_acpi.c +++ b/drivers/platform/x86/thinkpad_acpi.c @@ -10187,8 +10187,13 @@ static int dytc_profile_set(struct platform_profile_handler *pprof, return err; if (profile == PLATFORM_PROFILE_BALANCED) { - /* To get back to balanced mode we just issue a reset command */ - err = dytc_command(DYTC_CMD_RESET, &output); + /* + * To get back to balanced mode we need to issue a reset command. + * Note we still need to disable CQL mode before hand and re-enable + * it afterwards, otherwise dytc_lapmode gets reset to 0 and stays + * stuck at 0 for aprox. 30 minutes. + */ + err = dytc_cql_command(DYTC_CMD_RESET, &output); if (err) goto unlock; } else { From d939cd96b9df6dcde1605fab23bbd6307e11f930 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 21 Mar 2021 12:58:55 +0100 Subject: [PATCH 05/16] platform/x86: dell-wmi-sysman: Fix crash caused by calling kset_unregister twice On some system the WMI GUIDs used by dell-wmi-sysman are present but there are no enum type attributes, this causes init_bios_attributes() to return -ENODEV, after which sysman_init() does a "goto fail_create_group" and then calls release_attributes_data(). release_attributes_data() calls kset_unregister(wmi_priv.main_dir_kset); but before this commit it was missing a "wmi_priv.main_dir_kset = NULL;" statement; and after calling release_attributes_data() the sysman_init() error handling does this: if (wmi_priv.main_dir_kset) { kset_unregister(wmi_priv.main_dir_kset); wmi_priv.main_dir_kset = NULL; } Which causes a second kset_unregister(wmi_priv.main_dir_kset), leading to a double-free, which causes a crash. Add the missing "wmi_priv.main_dir_kset = NULL;" statement to release_attributes_data() to fix this double-free crash. Fixes: e8a60aa7404b ("platform/x86: Introduce support for Systems Management Driver over WMI for Dell Systems") Cc: Divya Bharathi Cc: Mario Limonciello Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20210321115901.35072-2-hdegoede@redhat.com --- drivers/platform/x86/dell/dell-wmi-sysman/sysman.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c index cb81010ba1a21..c1997db74cca5 100644 --- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c +++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c @@ -388,6 +388,7 @@ static void release_attributes_data(void) if (wmi_priv.main_dir_kset) { destroy_attribute_objs(wmi_priv.main_dir_kset); kset_unregister(wmi_priv.main_dir_kset); + wmi_priv.main_dir_kset = NULL; } mutex_unlock(&wmi_priv.mutex); From c59ab4cedab70a1a117a2dba3c48bb78e66c55ca Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 21 Mar 2021 12:58:56 +0100 Subject: [PATCH 06/16] platform/x86: dell-wmi-sysman: Fix possible NULL pointer deref on exit It is possible for release_attributes_data() to get called when the main_dir_kset has not been created yet, move the removal of the bios-reset sysfs attr to under a if (main_dir_kset) check to avoid a NULL pointer deref. Fixes: e8a60aa7404b ("platform/x86: Introduce support for Systems Management Driver over WMI for Dell Systems") Cc: Divya Bharathi Cc: Mario Limonciello Reported-by: Alexander Naumann Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20210321115901.35072-3-hdegoede@redhat.com --- drivers/platform/x86/dell/dell-wmi-sysman/sysman.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c index c1997db74cca5..8b251b2c37a2d 100644 --- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c +++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c @@ -225,12 +225,6 @@ static int create_attributes_level_sysfs_files(void) return ret; } -static void release_reset_bios_data(void) -{ - sysfs_remove_file(&wmi_priv.main_dir_kset->kobj, &reset_bios.attr); - sysfs_remove_file(&wmi_priv.main_dir_kset->kobj, &pending_reboot.attr); -} - static ssize_t wmi_sysman_attr_show(struct kobject *kobj, struct attribute *attr, char *buf) { @@ -373,8 +367,6 @@ static void destroy_attribute_objs(struct kset *kset) */ static void release_attributes_data(void) { - release_reset_bios_data(); - mutex_lock(&wmi_priv.mutex); exit_enum_attributes(); exit_int_attributes(); @@ -386,12 +378,13 @@ static void release_attributes_data(void) wmi_priv.authentication_dir_kset = NULL; } if (wmi_priv.main_dir_kset) { + sysfs_remove_file(&wmi_priv.main_dir_kset->kobj, &reset_bios.attr); + sysfs_remove_file(&wmi_priv.main_dir_kset->kobj, &pending_reboot.attr); destroy_attribute_objs(wmi_priv.main_dir_kset); kset_unregister(wmi_priv.main_dir_kset); wmi_priv.main_dir_kset = NULL; } mutex_unlock(&wmi_priv.mutex); - } /** From 2d0c418c91d8c86a1b9fb254dda842ada9919513 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 21 Mar 2021 12:58:57 +0100 Subject: [PATCH 07/16] platform/x86: dell-wmi-sysman: Make it safe to call exit_foo_attributes() multiple times During some of the error-exit paths it is possible that release_attributes_data() will get called multiple times, which results in exit_foo_attributes() getting called multiple times. Make it safe to call exit_foo_attributes() multiple times, avoiding double-free()s in this case. Note that release_attributes_data() really should only be called once during error-exit paths. This will be fixed in a separate patch and it is good to have the exit_foo_attributes() functions modified this way regardless. Fixes: e8a60aa7404b ("platform/x86: Introduce support for Systems Management Driver over WMI for Dell Systems") Cc: Divya Bharathi Cc: Mario Limonciello Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20210321115901.35072-4-hdegoede@redhat.com --- drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c | 3 +++ drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c | 3 +++ drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c | 3 +++ drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c | 3 +++ 4 files changed, 12 insertions(+) diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c b/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c index 80f4b7785c6c9..091e48c217ed8 100644 --- a/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c +++ b/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c @@ -185,5 +185,8 @@ void exit_enum_attributes(void) sysfs_remove_group(wmi_priv.enumeration_data[instance_id].attr_name_kobj, &enumeration_attr_group); } + wmi_priv.enumeration_instances_count = 0; + kfree(wmi_priv.enumeration_data); + wmi_priv.enumeration_data = NULL; } diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c b/drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c index 75aedbb733be2..8a49ba6e44f9a 100644 --- a/drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c +++ b/drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c @@ -175,5 +175,8 @@ void exit_int_attributes(void) sysfs_remove_group(wmi_priv.integer_data[instance_id].attr_name_kobj, &integer_attr_group); } + wmi_priv.integer_instances_count = 0; + kfree(wmi_priv.integer_data); + wmi_priv.integer_data = NULL; } diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c b/drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c index 3abcd95477c07..834b3e82ad9f9 100644 --- a/drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c +++ b/drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c @@ -183,5 +183,8 @@ void exit_po_attributes(void) sysfs_remove_group(wmi_priv.po_data[instance_id].attr_name_kobj, &po_attr_group); } + wmi_priv.po_instances_count = 0; + kfree(wmi_priv.po_data); + wmi_priv.po_data = NULL; } diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c b/drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c index ac75dce88a4c4..552537852459a 100644 --- a/drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c +++ b/drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c @@ -155,5 +155,8 @@ void exit_str_attributes(void) sysfs_remove_group(wmi_priv.str_data[instance_id].attr_name_kobj, &str_attr_group); } + wmi_priv.str_instances_count = 0; + kfree(wmi_priv.str_data); + wmi_priv.str_data = NULL; } From 59bbbeb9c22cc7c55965cd5ea8c16af7f16e61eb Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 21 Mar 2021 12:58:58 +0100 Subject: [PATCH 08/16] platform/x86: dell-wmi-sysman: Fix release_attributes_data() getting called twice on init_bios_attributes() failure All calls of init_bios_attributes() will result in a goto fail_create_group if they fail, which calls release_attributes_data(). So there is no need to call release_attributes_data() from init_bios_attributes() on failure itself. Fixes: e8a60aa7404b ("platform/x86: Introduce support for Systems Management Driver over WMI for Dell Systems") Cc: Divya Bharathi Cc: Mario Limonciello Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20210321115901.35072-5-hdegoede@redhat.com --- drivers/platform/x86/dell/dell-wmi-sysman/sysman.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c index 8b251b2c37a2d..58dc4571f987b 100644 --- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c +++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c @@ -491,7 +491,6 @@ static int init_bios_attributes(int attr_type, const char *guid) err_attr_init: mutex_unlock(&wmi_priv.mutex); - release_attributes_data(); kfree(obj); return retval; } From 9c90cd869747e3492a9306dcd8123c17502ff1fc Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 21 Mar 2021 12:58:59 +0100 Subject: [PATCH 09/16] platform/x86: dell-wmi-sysman: Cleanup sysman_init() error-exit handling Cleanup sysman_init() error-exit handling: 1. There is no need for the fail_reset_bios and fail_authentication_kset eror-exit cases, these can be handled by release_attributes_data() 2. Rename all the labels from fail_what_failed, to err_what_to_cleanup this is the usual way to name these and avoids the need to rename them when extra steps are added. Fixes: e8a60aa7404b ("platform/x86: Introduce support for Systems Management Driver over WMI for Dell Systems") Cc: Divya Bharathi Cc: Mario Limonciello Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20210321115901.35072-6-hdegoede@redhat.com --- .../x86/dell/dell-wmi-sysman/sysman.c | 45 +++++++------------ 1 file changed, 16 insertions(+), 29 deletions(-) diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c index 58dc4571f987b..99dc2f3bdf49e 100644 --- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c +++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c @@ -508,100 +508,87 @@ static int __init sysman_init(void) ret = init_bios_attr_set_interface(); if (ret || !wmi_priv.bios_attr_wdev) { pr_debug("failed to initialize set interface\n"); - goto fail_set_interface; + return ret; } ret = init_bios_attr_pass_interface(); if (ret || !wmi_priv.password_attr_wdev) { pr_debug("failed to initialize pass interface\n"); - goto fail_pass_interface; + goto err_exit_bios_attr_set_interface; } ret = class_register(&firmware_attributes_class); if (ret) - goto fail_class; + goto err_exit_bios_attr_pass_interface; wmi_priv.class_dev = device_create(&firmware_attributes_class, NULL, MKDEV(0, 0), NULL, "%s", DRIVER_NAME); if (IS_ERR(wmi_priv.class_dev)) { ret = PTR_ERR(wmi_priv.class_dev); - goto fail_classdev; + goto err_unregister_class; } wmi_priv.main_dir_kset = kset_create_and_add("attributes", NULL, &wmi_priv.class_dev->kobj); if (!wmi_priv.main_dir_kset) { ret = -ENOMEM; - goto fail_main_kset; + goto err_destroy_classdev; } wmi_priv.authentication_dir_kset = kset_create_and_add("authentication", NULL, &wmi_priv.class_dev->kobj); if (!wmi_priv.authentication_dir_kset) { ret = -ENOMEM; - goto fail_authentication_kset; + goto err_release_attributes_data; } ret = create_attributes_level_sysfs_files(); if (ret) { pr_debug("could not create reset BIOS attribute\n"); - goto fail_reset_bios; + goto err_release_attributes_data; } ret = init_bios_attributes(ENUM, DELL_WMI_BIOS_ENUMERATION_ATTRIBUTE_GUID); if (ret) { pr_debug("failed to populate enumeration type attributes\n"); - goto fail_create_group; + goto err_release_attributes_data; } ret = init_bios_attributes(INT, DELL_WMI_BIOS_INTEGER_ATTRIBUTE_GUID); if (ret) { pr_debug("failed to populate integer type attributes\n"); - goto fail_create_group; + goto err_release_attributes_data; } ret = init_bios_attributes(STR, DELL_WMI_BIOS_STRING_ATTRIBUTE_GUID); if (ret) { pr_debug("failed to populate string type attributes\n"); - goto fail_create_group; + goto err_release_attributes_data; } ret = init_bios_attributes(PO, DELL_WMI_BIOS_PASSOBJ_ATTRIBUTE_GUID); if (ret) { pr_debug("failed to populate pass object type attributes\n"); - goto fail_create_group; + goto err_release_attributes_data; } return 0; -fail_create_group: +err_release_attributes_data: release_attributes_data(); -fail_reset_bios: - if (wmi_priv.authentication_dir_kset) { - kset_unregister(wmi_priv.authentication_dir_kset); - wmi_priv.authentication_dir_kset = NULL; - } - -fail_authentication_kset: - if (wmi_priv.main_dir_kset) { - kset_unregister(wmi_priv.main_dir_kset); - wmi_priv.main_dir_kset = NULL; - } - -fail_main_kset: +err_destroy_classdev: device_destroy(&firmware_attributes_class, MKDEV(0, 0)); -fail_classdev: +err_unregister_class: class_unregister(&firmware_attributes_class); -fail_class: +err_exit_bios_attr_pass_interface: exit_bios_attr_pass_interface(); -fail_pass_interface: +err_exit_bios_attr_set_interface: exit_bios_attr_set_interface(); -fail_set_interface: return ret; } From 32418dd58c957f8fef25b97450d00275967604f1 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 21 Mar 2021 12:59:00 +0100 Subject: [PATCH 10/16] platform/x86: dell-wmi-sysman: Make sysman_init() return -ENODEV of the interfaces are not found When either the attributes or the password interface is not found, then unregister the 2 wmi drivers again and return -ENODEV from sysman_init(). Fixes: e8a60aa7404b ("platform/x86: Introduce support for Systems Management Driver over WMI for Dell Systems") Cc: Divya Bharathi Cc: Mario Limonciello Reported-by: Alexander Naumann Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20210321115901.35072-7-hdegoede@redhat.com --- drivers/platform/x86/dell/dell-wmi-sysman/sysman.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c index 99dc2f3bdf49e..5dd9b29d939c7 100644 --- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c +++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c @@ -506,15 +506,17 @@ static int __init sysman_init(void) } ret = init_bios_attr_set_interface(); - if (ret || !wmi_priv.bios_attr_wdev) { - pr_debug("failed to initialize set interface\n"); + if (ret) return ret; - } ret = init_bios_attr_pass_interface(); - if (ret || !wmi_priv.password_attr_wdev) { - pr_debug("failed to initialize pass interface\n"); + if (ret) goto err_exit_bios_attr_set_interface; + + if (!wmi_priv.bios_attr_wdev || !wmi_priv.password_attr_wdev) { + pr_debug("failed to find set or pass interface\n"); + ret = -ENODEV; + goto err_exit_bios_attr_pass_interface; } ret = class_register(&firmware_attributes_class); From 35471138a9f7193482a2019e39643f575f8098dc Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 21 Mar 2021 12:59:01 +0100 Subject: [PATCH 11/16] platform/x86: dell-wmi-sysman: Cleanup create_attributes_level_sysfs_files() Cleanup create_attributes_level_sysfs_files(): 1. There is no need to call sysfs_remove_file() on error, sysman_init() will already call release_attributes_data() on failure which already does this. 2. There is no need for the pr_debug() calls sysfs_create_file() should never fail and if it does it will already complain about the problem itself. Fixes: e8a60aa7404b ("platform/x86: Introduce support for Systems Management Driver over WMI for Dell Systems") Cc: Divya Bharathi Cc: Mario Limonciello Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20210321115901.35072-8-hdegoede@redhat.com --- .../platform/x86/dell/dell-wmi-sysman/sysman.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c index 5dd9b29d939c7..7410ccae650c2 100644 --- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c +++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c @@ -210,19 +210,17 @@ static struct kobj_attribute pending_reboot = __ATTR_RO(pending_reboot); */ static int create_attributes_level_sysfs_files(void) { - int ret = sysfs_create_file(&wmi_priv.main_dir_kset->kobj, &reset_bios.attr); + int ret; - if (ret) { - pr_debug("could not create reset_bios file\n"); + ret = sysfs_create_file(&wmi_priv.main_dir_kset->kobj, &reset_bios.attr); + if (ret) return ret; - } ret = sysfs_create_file(&wmi_priv.main_dir_kset->kobj, &pending_reboot.attr); - if (ret) { - pr_debug("could not create changing_pending_reboot file\n"); - sysfs_remove_file(&wmi_priv.main_dir_kset->kobj, &reset_bios.attr); - } - return ret; + if (ret) + return ret; + + return 0; } static ssize_t wmi_sysman_attr_show(struct kobject *kobj, struct attribute *attr, From 538d2dd0b9920334e6596977a664e9e7bac73703 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 21 Mar 2021 17:35:13 +0100 Subject: [PATCH 12/16] platform/x86: intel-vbtn: Stop reporting SW_DOCK events Stop reporting SW_DOCK events because this breaks suspend-on-lid-close. SW_DOCK should only be reported for docking stations, but all the DSDTs in my DSDT collection which use the intel-vbtn code, always seem to use this for 2-in-1s / convertibles and set SW_DOCK=1 when in laptop-mode (in tandem with setting SW_TABLET_MODE=0). This causes userspace to think the laptop is docked to a port-replicator and to disable suspend-on-lid-close, which is undesirable. Map the dock events to KEY_IGNORE to avoid this broken SW_DOCK reporting. Note this may theoretically cause us to stop reporting SW_DOCK on some device where the 0xCA and 0xCB intel-vbtn events are actually used for reporting docking to a classic docking-station / port-replicator but I'm not aware of any such devices. Also the most important thing is that we only report SW_DOCK when it reliably reports being docked to a classic docking-station without any false positives, which clearly is not the case here. If there is a chance of reporting false positives then it is better to not report SW_DOCK at all. Cc: stable@vger.kernel.org Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20210321163513.72328-1-hdegoede@redhat.com --- drivers/platform/x86/intel-vbtn.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-vbtn.c index 8a8017f9ca913..3fdf4cbec9ad2 100644 --- a/drivers/platform/x86/intel-vbtn.c +++ b/drivers/platform/x86/intel-vbtn.c @@ -48,8 +48,16 @@ static const struct key_entry intel_vbtn_keymap[] = { }; static const struct key_entry intel_vbtn_switchmap[] = { - { KE_SW, 0xCA, { .sw = { SW_DOCK, 1 } } }, /* Docked */ - { KE_SW, 0xCB, { .sw = { SW_DOCK, 0 } } }, /* Undocked */ + /* + * SW_DOCK should only be reported for docking stations, but DSDTs using the + * intel-vbtn code, always seem to use this for 2-in-1s / convertibles and set + * SW_DOCK=1 when in laptop-mode (in tandem with setting SW_TABLET_MODE=0). + * This causes userspace to think the laptop is docked to a port-replicator + * and to disable suspend-on-lid-close, which is undesirable. + * Map the dock events to KEY_IGNORE to avoid this broken SW_DOCK reporting. + */ + { KE_IGNORE, 0xCA, { .sw = { SW_DOCK, 1 } } }, /* Docked */ + { KE_IGNORE, 0xCB, { .sw = { SW_DOCK, 0 } } }, /* Undocked */ { KE_SW, 0xCC, { .sw = { SW_TABLET_MODE, 1 } } }, /* Tablet */ { KE_SW, 0xCD, { .sw = { SW_TABLET_MODE, 0 } } }, /* Laptop */ { KE_END } From 7547deff8a221e6bf1e563cf1b636844a8e5378a Mon Sep 17 00:00:00 2001 From: "David E. Box" Date: Tue, 16 Mar 2021 19:44:54 -0700 Subject: [PATCH 13/16] platform/x86: intel_pmt_class: Initial resource to 0 Initialize the struct resource in intel_pmt_dev_register to zero to avoid a fault should the char *name field be non-zero. Signed-off-by: David E. Box Link: https://lore.kernel.org/r/20210317024455.3071477-1-david.e.box@linux.intel.com Signed-off-by: Hans de Goede --- drivers/platform/x86/intel_pmt_class.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/platform/x86/intel_pmt_class.c b/drivers/platform/x86/intel_pmt_class.c index c8939fba45090..ee2b3bbeb83da 100644 --- a/drivers/platform/x86/intel_pmt_class.c +++ b/drivers/platform/x86/intel_pmt_class.c @@ -173,7 +173,7 @@ static int intel_pmt_dev_register(struct intel_pmt_entry *entry, struct intel_pmt_namespace *ns, struct device *parent) { - struct resource res; + struct resource res = {0}; struct device *dev; int ret; From 10c931cdfe64ebc38a15a485dd794915044f2111 Mon Sep 17 00:00:00 2001 From: "David E. Box" Date: Tue, 16 Mar 2021 19:44:55 -0700 Subject: [PATCH 14/16] platform/x86: intel_pmt_crashlog: Fix incorrect macros Fixes off-by-one bugs in the macro assignments for the crashlog control bits. Was initially tested on emulation but bug revealed after testing on silicon. Fixes: 5ef9998c96b0 ("platform/x86: Intel PMT Crashlog capability driver") Signed-off-by: David E. Box Link: https://lore.kernel.org/r/20210317024455.3071477-2-david.e.box@linux.intel.com Signed-off-by: Hans de Goede --- drivers/platform/x86/intel_pmt_crashlog.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/platform/x86/intel_pmt_crashlog.c b/drivers/platform/x86/intel_pmt_crashlog.c index 97dd749c8290d..92d315a16cfd3 100644 --- a/drivers/platform/x86/intel_pmt_crashlog.c +++ b/drivers/platform/x86/intel_pmt_crashlog.c @@ -23,18 +23,17 @@ #define CRASH_TYPE_OOBMSM 1 /* Control Flags */ -#define CRASHLOG_FLAG_DISABLE BIT(27) +#define CRASHLOG_FLAG_DISABLE BIT(28) /* - * Bits 28 and 29 control the state of bit 31. + * Bits 29 and 30 control the state of bit 31. * - * Bit 28 will clear bit 31, if set, allowing a new crashlog to be captured. - * Bit 29 will immediately trigger a crashlog to be generated, setting bit 31. - * Bit 30 is read-only and reserved as 0. + * Bit 29 will clear bit 31, if set, allowing a new crashlog to be captured. + * Bit 30 will immediately trigger a crashlog to be generated, setting bit 31. * Bit 31 is the read-only status with a 1 indicating log is complete. */ -#define CRASHLOG_FLAG_TRIGGER_CLEAR BIT(28) -#define CRASHLOG_FLAG_TRIGGER_EXECUTE BIT(29) +#define CRASHLOG_FLAG_TRIGGER_CLEAR BIT(29) +#define CRASHLOG_FLAG_TRIGGER_EXECUTE BIT(30) #define CRASHLOG_FLAG_TRIGGER_COMPLETE BIT(31) #define CRASHLOG_FLAG_TRIGGER_MASK GENMASK(31, 28) From 269b04a50992d8defab869079049ecfc60b6b3e5 Mon Sep 17 00:00:00 2001 From: "David E. Box" Date: Fri, 19 Mar 2021 13:18:43 -0700 Subject: [PATCH 15/16] platform/x86: intel_pmc_core: Update Kconfig The intel_pmc_core driver is mostly used as a debugging driver for Intel platforms that support SLPS0 (S0ix). But the driver may also be used to communicate actions to the PMC in order to ensure transition to SLPS0 on some systems and architectures. As such the driver should be built on all platforms it supports. Indicate this in the Kconfig. Also update the list of supported features. Signed-off-by: David E. Box Suggested-by: Mario Limonciello Link: https://lore.kernel.org/r/20210319201844.3305399-1-david.e.box@linux.intel.com Signed-off-by: Hans de Goede --- drivers/platform/x86/Kconfig | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index ad4e630e73e26..461ec61530ebf 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -1173,15 +1173,20 @@ config INTEL_PMC_CORE depends on PCI help The Intel Platform Controller Hub for Intel Core SoCs provides access - to Power Management Controller registers via a PCI interface. This + to Power Management Controller registers via various interfaces. This driver can utilize debugging capabilities and supported features as - exposed by the Power Management Controller. + exposed by the Power Management Controller. It also may perform some + tasks in the PMC in order to enable transition into the SLPS0 state. + It should be selected on all Intel platforms supported by the driver. Supported features: - SLP_S0_RESIDENCY counter - PCH IP Power Gating status - - LTR Ignore + - LTR Ignore / LTR Show - MPHY/PLL gating status (Sunrisepoint PCH only) + - SLPS0 Debug registers (Cannonlake/Icelake PCH) + - Low Power Mode registers (Tigerlake and beyond) + - PMC quirks as needed to enable SLPS0/S0ix config INTEL_PMT_CLASS tristate From d1635448f1105e549b4041aab930dbc6945fc635 Mon Sep 17 00:00:00 2001 From: "David E. Box" Date: Fri, 19 Mar 2021 13:18:44 -0700 Subject: [PATCH 16/16] platform/x86: intel_pmc_core: Ignore GBE LTR on Tiger Lake platforms Due to a HW limitation, the Latency Tolerance Reporting (LTR) value programmed in the Tiger Lake GBE controller is not large enough to allow the platform to enter Package C10, which in turn prevents the platform from achieving its low power target during suspend-to-idle. Ignore the GBE LTR value on Tiger Lake. LTR ignore functionality is currently performed solely by a debugfs write call. Split out the LTR code into its own function that can be called by both the debugfs writer and by this work around. Signed-off-by: David E. Box Reviewed-by: Sasha Neftin Cc: intel-wired-lan@lists.osuosl.org Reviewed-by: Rajneesh Bhardwaj Link: https://lore.kernel.org/r/20210319201844.3305399-2-david.e.box@linux.intel.com Signed-off-by: Hans de Goede --- drivers/platform/x86/intel_pmc_core.c | 50 +++++++++++++++++++-------- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/drivers/platform/x86/intel_pmc_core.c b/drivers/platform/x86/intel_pmc_core.c index ee2f757515b0a..b5888aeb4bcff 100644 --- a/drivers/platform/x86/intel_pmc_core.c +++ b/drivers/platform/x86/intel_pmc_core.c @@ -863,34 +863,45 @@ static int pmc_core_pll_show(struct seq_file *s, void *unused) } DEFINE_SHOW_ATTRIBUTE(pmc_core_pll); -static ssize_t pmc_core_ltr_ignore_write(struct file *file, - const char __user *userbuf, - size_t count, loff_t *ppos) +static int pmc_core_send_ltr_ignore(u32 value) { struct pmc_dev *pmcdev = &pmc; const struct pmc_reg_map *map = pmcdev->map; - u32 val, buf_size, fd; - int err; - - buf_size = count < 64 ? count : 64; - - err = kstrtou32_from_user(userbuf, buf_size, 10, &val); - if (err) - return err; + u32 reg; + int err = 0; mutex_lock(&pmcdev->lock); - if (val > map->ltr_ignore_max) { + if (value > map->ltr_ignore_max) { err = -EINVAL; goto out_unlock; } - fd = pmc_core_reg_read(pmcdev, map->ltr_ignore_offset); - fd |= (1U << val); - pmc_core_reg_write(pmcdev, map->ltr_ignore_offset, fd); + reg = pmc_core_reg_read(pmcdev, map->ltr_ignore_offset); + reg |= BIT(value); + pmc_core_reg_write(pmcdev, map->ltr_ignore_offset, reg); out_unlock: mutex_unlock(&pmcdev->lock); + + return err; +} + +static ssize_t pmc_core_ltr_ignore_write(struct file *file, + const char __user *userbuf, + size_t count, loff_t *ppos) +{ + u32 buf_size, value; + int err; + + buf_size = min_t(u32, count, 64); + + err = kstrtou32_from_user(userbuf, buf_size, 10, &value); + if (err) + return err; + + err = pmc_core_send_ltr_ignore(value); + return err == 0 ? count : err; } @@ -1244,6 +1255,15 @@ static int pmc_core_probe(struct platform_device *pdev) pmcdev->pmc_xram_read_bit = pmc_core_check_read_lock_bit(); dmi_check_system(pmc_core_dmi_table); + /* + * On TGL, due to a hardware limitation, the GBE LTR blocks PC10 when + * a cable is attached. Tell the PMC to ignore it. + */ + if (pmcdev->map == &tgl_reg_map) { + dev_dbg(&pdev->dev, "ignoring GBE LTR\n"); + pmc_core_send_ltr_ignore(3); + } + pmc_core_dbgfs_register(pmcdev); device_initialized = true;