From 70246f89c55fb1be0367a584000fd68502a0933f Mon Sep 17 00:00:00 2001 From: Mario Limonciello Date: Thu, 5 Dec 2024 21:19:14 -0600 Subject: [PATCH 01/16] ACPI: platform_profile: Check all profile handler to calculate next MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit As multiple platform profile handlers might not all support the same profile, cycling to the next profile could have a different result depending on what handler are registered. Check what is active and supported by all handlers to decide what to do. Reviewed-by: Armin Wolf Tested-by: Mark Pearson Reviewed-by: Mark Pearson Signed-off-by: Mario Limonciello Link: https://lore.kernel.org/r/20241206031918.1537-19-mario.limonciello@amd.com Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/acpi/platform_profile.c | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c index 95aff045a1eb..a1fdc56537ba 100644 --- a/drivers/acpi/platform_profile.c +++ b/drivers/acpi/platform_profile.c @@ -418,25 +418,37 @@ EXPORT_SYMBOL_GPL(platform_profile_notify); int platform_profile_cycle(void) { - enum platform_profile_option profile; - enum platform_profile_option next; + enum platform_profile_option next = PLATFORM_PROFILE_LAST; + enum platform_profile_option profile = PLATFORM_PROFILE_LAST; + unsigned long choices[BITS_TO_LONGS(PLATFORM_PROFILE_LAST)]; int err; + set_bit(PLATFORM_PROFILE_LAST, choices); scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &profile_lock) { - if (!cur_profile) - return -ENODEV; + err = class_for_each_device(&platform_profile_class, NULL, + &profile, _aggregate_profiles); + if (err) + return err; - err = cur_profile->profile_get(cur_profile, &profile); + if (profile == PLATFORM_PROFILE_CUSTOM || + profile == PLATFORM_PROFILE_LAST) + return -EINVAL; + + err = class_for_each_device(&platform_profile_class, NULL, + choices, _aggregate_choices); if (err) return err; - next = find_next_bit_wrap(cur_profile->choices, PLATFORM_PROFILE_LAST, + /* never iterate into a custom if all drivers supported it */ + clear_bit(PLATFORM_PROFILE_CUSTOM, choices); + + next = find_next_bit_wrap(choices, + PLATFORM_PROFILE_LAST, profile + 1); - if (WARN_ON(next == PLATFORM_PROFILE_LAST)) - return -EINVAL; + err = class_for_each_device(&platform_profile_class, NULL, &next, + _store_and_notify); - err = cur_profile->profile_set(cur_profile, next); if (err) return err; } -- 2.50.1 From 37a6853d83f8af1b68346b555a7dd78e2c7f10e5 Mon Sep 17 00:00:00 2001 From: Mario Limonciello Date: Thu, 5 Dec 2024 21:19:15 -0600 Subject: [PATCH 02/16] ACPI: platform_profile: Notify class device from platform_profile_notify() MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit When a driver has called platform_profile_notify() both the legacy sysfs interface and the class device should be notified as userspace may listen to either. Reviewed-by: Armin Wolf Reviewed-by: Mark Pearson Reviewed-by: Ilpo Järvinen Signed-off-by: Mario Limonciello Link: https://lore.kernel.org/r/20241206031918.1537-20-mario.limonciello@amd.com Signed-off-by: Ilpo Järvinen --- drivers/acpi/platform_profile.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c index a1fdc56537ba..e105ef48409e 100644 --- a/drivers/acpi/platform_profile.c +++ b/drivers/acpi/platform_profile.c @@ -412,6 +412,9 @@ void platform_profile_notify(struct platform_profile_handler *pprof) { if (!cur_profile) return; + scoped_cond_guard(mutex_intr, return, &profile_lock) { + _notify_class_profile(pprof->class_dev, NULL); + } sysfs_notify(acpi_kobj, NULL, "platform_profile"); } EXPORT_SYMBOL_GPL(platform_profile_notify); -- 2.50.1 From 688834743d672b55ce357e11e34be835c78467e8 Mon Sep 17 00:00:00 2001 From: Mario Limonciello Date: Thu, 5 Dec 2024 21:19:16 -0600 Subject: [PATCH 03/16] ACPI: platform_profile: Allow multiple handlers MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Multiple drivers may attempt to register platform profile handlers, but only one may be registered and the behavior is non-deterministic for which one wins. It's mostly controlled by probing order. This can be problematic if one driver changes CPU settings and another driver notifies the EC for changing fan curves. Modify the ACPI platform profile handler to let multiple drivers register platform profile handlers and abstract this detail from userspace. To avoid undefined behaviors only offer profiles that are commonly advertised across multiple handlers. If any problems occur when changing profiles for any driver, then the drivers that were already changed remain changed and the legacy sysfs handler will report 'custom'. Tested-by: Mark Pearson Tested-by: Matthew Schwartz Reviewed-by: Mark Pearson Reviewed-by: Armin Wolf Reviewed-by: Ilpo Järvinen Signed-off-by: Mario Limonciello Link: https://lore.kernel.org/r/20241206031918.1537-21-mario.limonciello@amd.com Signed-off-by: Ilpo Järvinen --- drivers/acpi/platform_profile.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c index e105ef48409e..75a1415190ac 100644 --- a/drivers/acpi/platform_profile.c +++ b/drivers/acpi/platform_profile.c @@ -10,7 +10,6 @@ #include #include -static struct platform_profile_handler *cur_profile; static DEFINE_MUTEX(profile_lock); static const char * const profile_names[] = { @@ -410,8 +409,6 @@ static const struct attribute_group platform_profile_group = { void platform_profile_notify(struct platform_profile_handler *pprof) { - if (!cur_profile) - return; scoped_cond_guard(mutex_intr, return, &profile_lock) { _notify_class_profile(pprof->class_dev, NULL); } @@ -474,9 +471,6 @@ int platform_profile_register(struct platform_profile_handler *pprof) } guard(mutex)(&profile_lock); - /* We can only have one active profile */ - if (cur_profile) - return -EEXIST; /* create class interface for individual handler */ pprof->minor = ida_alloc(&platform_profile_ida, GFP_KERNEL); @@ -492,8 +486,6 @@ int platform_profile_register(struct platform_profile_handler *pprof) sysfs_notify(acpi_kobj, NULL, "platform_profile"); - cur_profile = pprof; - err = sysfs_update_group(acpi_kobj, &platform_profile_group); if (err) goto cleanup_cur; @@ -501,7 +493,6 @@ int platform_profile_register(struct platform_profile_handler *pprof) return 0; cleanup_cur: - cur_profile = NULL; device_unregister(pprof->class_dev); cleanup_ida: @@ -516,8 +507,6 @@ int platform_profile_remove(struct platform_profile_handler *pprof) int id; guard(mutex)(&profile_lock); - cur_profile = NULL; - id = pprof->minor; device_unregister(pprof->class_dev); ida_free(&platform_profile_ida, id); -- 2.50.1 From dd7ba84afa0667c818d380605ab3e11440925287 Mon Sep 17 00:00:00 2001 From: Mario Limonciello Date: Thu, 5 Dec 2024 21:19:17 -0600 Subject: [PATCH 04/16] platform/x86/amd: pmf: Drop all quirks MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit As multiple platform profile handlers can now be registered, the quirks to avoid registering amd-pmf as a handler are no longer necessary. Drop them. Reviewed-by: Armin Wolf Tested-by: Mark Pearson Tested-by: Matthew Schwartz Acked-by: Shyam Sundar S K Reviewed-by: Mark Pearson Reviewed-by: Ilpo Järvinen Signed-off-by: Mario Limonciello Link: https://lore.kernel.org/r/20241206031918.1537-22-mario.limonciello@amd.com Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/amd/pmf/Makefile | 2 +- drivers/platform/x86/amd/pmf/core.c | 1 - drivers/platform/x86/amd/pmf/pmf-quirks.c | 66 ----------------------- drivers/platform/x86/amd/pmf/pmf.h | 3 -- 4 files changed, 1 insertion(+), 71 deletions(-) delete mode 100644 drivers/platform/x86/amd/pmf/pmf-quirks.c diff --git a/drivers/platform/x86/amd/pmf/Makefile b/drivers/platform/x86/amd/pmf/Makefile index 7d6079b02589..6b26e48ce8ad 100644 --- a/drivers/platform/x86/amd/pmf/Makefile +++ b/drivers/platform/x86/amd/pmf/Makefile @@ -7,4 +7,4 @@ obj-$(CONFIG_AMD_PMF) += amd-pmf.o amd-pmf-objs := core.o acpi.o sps.o \ auto-mode.o cnqf.o \ - tee-if.o spc.o pmf-quirks.o + tee-if.o spc.o diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c index 06a97c533cb8..01eb9ee1eccd 100644 --- a/drivers/platform/x86/amd/pmf/core.c +++ b/drivers/platform/x86/amd/pmf/core.c @@ -456,7 +456,6 @@ static int amd_pmf_probe(struct platform_device *pdev) mutex_init(&dev->lock); mutex_init(&dev->update_mutex); - amd_pmf_quirks_init(dev); apmf_acpi_init(dev); platform_set_drvdata(pdev, dev); amd_pmf_dbgfs_register(dev); diff --git a/drivers/platform/x86/amd/pmf/pmf-quirks.c b/drivers/platform/x86/amd/pmf/pmf-quirks.c deleted file mode 100644 index 7cde5733b9ca..000000000000 --- a/drivers/platform/x86/amd/pmf/pmf-quirks.c +++ /dev/null @@ -1,66 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-or-later -/* - * AMD Platform Management Framework Driver Quirks - * - * Copyright (c) 2024, Advanced Micro Devices, Inc. - * All Rights Reserved. - * - * Author: Mario Limonciello - */ - -#include - -#include "pmf.h" - -struct quirk_entry { - u32 supported_func; -}; - -static struct quirk_entry quirk_no_sps_bug = { - .supported_func = 0x4003, -}; - -static const struct dmi_system_id fwbug_list[] = { - { - .ident = "ROG Zephyrus G14", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), - DMI_MATCH(DMI_PRODUCT_NAME, "GA403U"), - }, - .driver_data = &quirk_no_sps_bug, - }, - { - .ident = "ROG Ally X", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), - DMI_MATCH(DMI_PRODUCT_NAME, "RC72LA"), - }, - .driver_data = &quirk_no_sps_bug, - }, - { - .ident = "ASUS TUF Gaming A14", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), - DMI_MATCH(DMI_PRODUCT_NAME, "FA401W"), - }, - .driver_data = &quirk_no_sps_bug, - }, - {} -}; - -void amd_pmf_quirks_init(struct amd_pmf_dev *dev) -{ - const struct dmi_system_id *dmi_id; - struct quirk_entry *quirks; - - dmi_id = dmi_first_match(fwbug_list); - if (!dmi_id) - return; - - quirks = dmi_id->driver_data; - if (quirks->supported_func) { - dev->supported_func = quirks->supported_func; - pr_info("Using supported funcs quirk to avoid %s platform firmware bug\n", - dmi_id->ident); - } -} diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h index a79808fda1d8..64ab532550ac 100644 --- a/drivers/platform/x86/amd/pmf/pmf.h +++ b/drivers/platform/x86/amd/pmf/pmf.h @@ -797,7 +797,4 @@ int amd_pmf_smartpc_apply_bios_output(struct amd_pmf_dev *dev, u32 val, u32 preq void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in); void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in); -/* Quirk infrastructure */ -void amd_pmf_quirks_init(struct amd_pmf_dev *dev); - #endif /* PMF_H */ -- 2.50.1 From 0056b0852c326cf21201661f68ab41002ae9a845 Mon Sep 17 00:00:00 2001 From: Mario Limonciello Date: Thu, 5 Dec 2024 21:19:18 -0600 Subject: [PATCH 05/16] Documentation: Add documentation about class interface for platform profiles MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The class interface allows changing multiple platform profiles on a system to different values. The semantics of it are similar to the legacy interface. Reviewed-by: Armin Wolf Tested-by: Mark Pearson Reviewed-by: Mark Pearson Reviewed-by: Ilpo Järvinen Signed-off-by: Mario Limonciello Link: https://lore.kernel.org/r/20241206031918.1537-23-mario.limonciello@amd.com Signed-off-by: Ilpo Järvinen --- .../ABI/testing/sysfs-platform_profile | 5 +++ .../userspace-api/sysfs-platform_profile.rst | 38 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-platform_profile b/Documentation/ABI/testing/sysfs-platform_profile index baf1d125f9f8..125324ab53a9 100644 --- a/Documentation/ABI/testing/sysfs-platform_profile +++ b/Documentation/ABI/testing/sysfs-platform_profile @@ -33,3 +33,8 @@ Description: Reading this file gives the current selected profile for this source such as e.g. a hotkey triggered profile change handled either directly by the embedded-controller or fully handled inside the kernel. + + This file may also emit the string 'custom' to indicate + that multiple platform profiles drivers are in use but + have different values. This string can not be written to + this interface and is solely for informational purposes. diff --git a/Documentation/userspace-api/sysfs-platform_profile.rst b/Documentation/userspace-api/sysfs-platform_profile.rst index 4fccde2e4563..7f013356118a 100644 --- a/Documentation/userspace-api/sysfs-platform_profile.rst +++ b/Documentation/userspace-api/sysfs-platform_profile.rst @@ -40,3 +40,41 @@ added. Drivers which wish to introduce new profile names must: 1. Explain why the existing profile names cannot be used. 2. Add the new profile name, along with a clear description of the expected behaviour, to the sysfs-platform_profile ABI documentation. + +"Custom" profile support +======================== +The platform_profile class also supports profiles advertising a "custom" +profile. This is intended to be set by drivers when the setttings in the +driver have been modified in a way that a standard profile doesn't represent +the current state. + +Multiple driver support +======================= +When multiple drivers on a system advertise a platform profile handler, the +platform profile handler core will only advertise the profiles that are +common between all drivers to the ``/sys/firmware/acpi`` interfaces. + +This is to ensure there is no ambiguity on what the profile names mean when +all handlers don't support a profile. + +Individual drivers will register a 'platform_profile' class device that has +similar semantics as the ``/sys/firmware/acpi/platform_profile`` interface. + +To discover which driver is associated with a platform profile handler the +user can read the ``name`` attribute of the class device. + +To discover available profiles from the class interface the user can read the +``choices`` attribute. + +If a user wants to select a profile for a specific driver, they can do so +by writing to the ``profile`` attribute of the driver's class device. + +This will allow users to set different profiles for different drivers on the +same system. If the selected profile by individual drivers differs the +platform profile handler core will display the profile 'custom' to indicate +that the profiles are not the same. + +While the ``platform_profile`` attribute has the value ``custom``, writing a +common profile from ``platform_profile_choices`` to the platform_profile +attribute of the platform profile handler core will set the profile for all +drivers. -- 2.50.1 From c0cc60b39269395b0a55c3ea8825690ec756d18e Mon Sep 17 00:00:00 2001 From: =?utf8?q?Thomas=20Wei=C3=9Fschuh?= Date: Mon, 2 Dec 2024 20:38:32 +0100 Subject: [PATCH 06/16] platform/x86: dell: dcdbas: Constify 'struct bin_attribute' MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The sysfs core now allows instances of 'struct bin_attribute' to be moved into read-only memory. Make use of that to protect them against accidental or malicious modifications. While at it switch from the custom DCDBAS_BIN_ATTR_RW() to the identical BIN_ATTR_RW() macro. Signed-off-by: Thomas Weißschuh Link: https://lore.kernel.org/r/20241202-sysfs-const-bin_attr-pdx86-v1-1-9ab204c2a814@weissschuh.net Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/dell/dcdbas.c | 10 +++++----- drivers/platform/x86/dell/dcdbas.h | 8 -------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/drivers/platform/x86/dell/dcdbas.c b/drivers/platform/x86/dell/dcdbas.c index 0aeb8149c16b..8149be25fa26 100644 --- a/drivers/platform/x86/dell/dcdbas.c +++ b/drivers/platform/x86/dell/dcdbas.c @@ -163,7 +163,7 @@ static ssize_t smi_data_buf_size_store(struct device *dev, } static ssize_t smi_data_read(struct file *filp, struct kobject *kobj, - struct bin_attribute *bin_attr, + const struct bin_attribute *bin_attr, char *buf, loff_t pos, size_t count) { ssize_t ret; @@ -176,7 +176,7 @@ static ssize_t smi_data_read(struct file *filp, struct kobject *kobj, } static ssize_t smi_data_write(struct file *filp, struct kobject *kobj, - struct bin_attribute *bin_attr, + const struct bin_attribute *bin_attr, char *buf, loff_t pos, size_t count) { ssize_t ret; @@ -636,9 +636,9 @@ static struct notifier_block dcdbas_reboot_nb = { .priority = INT_MIN }; -static DCDBAS_BIN_ATTR_RW(smi_data); +static const BIN_ATTR_ADMIN_RW(smi_data, 0); -static struct bin_attribute *dcdbas_bin_attrs[] = { +static const struct bin_attribute *const dcdbas_bin_attrs[] = { &bin_attr_smi_data, NULL }; @@ -662,7 +662,7 @@ static struct attribute *dcdbas_dev_attrs[] = { static const struct attribute_group dcdbas_attr_group = { .attrs = dcdbas_dev_attrs, - .bin_attrs = dcdbas_bin_attrs, + .bin_attrs_new = dcdbas_bin_attrs, }; static int dcdbas_probe(struct platform_device *dev) diff --git a/drivers/platform/x86/dell/dcdbas.h b/drivers/platform/x86/dell/dcdbas.h index 942a23ddded0..a05d7f667586 100644 --- a/drivers/platform/x86/dell/dcdbas.h +++ b/drivers/platform/x86/dell/dcdbas.h @@ -56,14 +56,6 @@ #define DCDBAS_DEV_ATTR_WO(_name) \ DEVICE_ATTR(_name,0200,NULL,_name##_store); -#define DCDBAS_BIN_ATTR_RW(_name) \ -struct bin_attribute bin_attr_##_name = { \ - .attr = { .name = __stringify(_name), \ - .mode = 0600 }, \ - .read = _name##_read, \ - .write = _name##_write, \ -} - struct smi_cmd { __u32 magic; __u32 ebx; -- 2.50.1 From fbabd3dbb55fc46392193a2e3c0366905b2b26f9 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Thomas=20Wei=C3=9Fschuh?= Date: Mon, 2 Dec 2024 20:38:33 +0100 Subject: [PATCH 07/16] platform/x86: dell_rbu: Constify 'struct bin_attribute' MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The sysfs core now allows instances of 'struct bin_attribute' to be moved into read-only memory. Make use of that to protect them against accidental or malicious modifications. Signed-off-by: Thomas Weißschuh Link: https://lore.kernel.org/r/20241202-sysfs-const-bin_attr-pdx86-v1-2-9ab204c2a814@weissschuh.net Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/dell/dell_rbu.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/platform/x86/dell/dell_rbu.c b/drivers/platform/x86/dell/dell_rbu.c index 9f51e0fcab04..e30ca325938c 100644 --- a/drivers/platform/x86/dell/dell_rbu.c +++ b/drivers/platform/x86/dell/dell_rbu.c @@ -475,7 +475,7 @@ static ssize_t read_rbu_mono_data(char *buffer, loff_t pos, size_t count) } static ssize_t data_read(struct file *filp, struct kobject *kobj, - struct bin_attribute *bin_attr, + const struct bin_attribute *bin_attr, char *buffer, loff_t pos, size_t count) { ssize_t ret_count = 0; @@ -492,7 +492,7 @@ static ssize_t data_read(struct file *filp, struct kobject *kobj, spin_unlock(&rbu_data.lock); return ret_count; } -static BIN_ATTR_RO(data, 0); +static const BIN_ATTR_RO(data, 0); static void callbackfn_rbu(const struct firmware *fw, void *context) { @@ -530,7 +530,7 @@ static void callbackfn_rbu(const struct firmware *fw, void *context) } static ssize_t image_type_read(struct file *filp, struct kobject *kobj, - struct bin_attribute *bin_attr, + const struct bin_attribute *bin_attr, char *buffer, loff_t pos, size_t count) { int size = 0; @@ -540,7 +540,7 @@ static ssize_t image_type_read(struct file *filp, struct kobject *kobj, } static ssize_t image_type_write(struct file *filp, struct kobject *kobj, - struct bin_attribute *bin_attr, + const struct bin_attribute *bin_attr, char *buffer, loff_t pos, size_t count) { int rc = count; @@ -597,10 +597,10 @@ static ssize_t image_type_write(struct file *filp, struct kobject *kobj, return rc; } -static BIN_ATTR_RW(image_type, 0); +static const BIN_ATTR_RW(image_type, 0); static ssize_t packet_size_read(struct file *filp, struct kobject *kobj, - struct bin_attribute *bin_attr, + const struct bin_attribute *bin_attr, char *buffer, loff_t pos, size_t count) { int size = 0; @@ -613,7 +613,7 @@ static ssize_t packet_size_read(struct file *filp, struct kobject *kobj, } static ssize_t packet_size_write(struct file *filp, struct kobject *kobj, - struct bin_attribute *bin_attr, + const struct bin_attribute *bin_attr, char *buffer, loff_t pos, size_t count) { unsigned long temp; @@ -626,9 +626,9 @@ static ssize_t packet_size_write(struct file *filp, struct kobject *kobj, spin_unlock(&rbu_data.lock); return count; } -static BIN_ATTR_RW(packet_size, 0); +static const BIN_ATTR_RW(packet_size, 0); -static struct bin_attribute *rbu_bin_attrs[] = { +static const struct bin_attribute *const rbu_bin_attrs[] = { &bin_attr_data, &bin_attr_image_type, &bin_attr_packet_size, @@ -636,7 +636,7 @@ static struct bin_attribute *rbu_bin_attrs[] = { }; static const struct attribute_group rbu_group = { - .bin_attrs = rbu_bin_attrs, + .bin_attrs_new = rbu_bin_attrs, }; static int __init dcdrbu_init(void) -- 2.50.1 From b0034f206f140ecdd78e8f5fe4d61fb367124f3b Mon Sep 17 00:00:00 2001 From: =?utf8?q?Thomas=20Wei=C3=9Fschuh?= Date: Mon, 2 Dec 2024 20:38:34 +0100 Subject: [PATCH 08/16] platform/x86/intel/sdsi: Constify 'struct bin_attribute' MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The sysfs core now allows instances of 'struct bin_attribute' to be moved into read-only memory. Make use of that to protect them against accidental or malicious modifications. Signed-off-by: Thomas Weißschuh Link: https://lore.kernel.org/r/20241202-sysfs-const-bin_attr-pdx86-v1-3-9ab204c2a814@weissschuh.net Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/intel/sdsi.c | 34 +++++++++++++++---------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c index 33f33b1070fd..30d1c2caf984 100644 --- a/drivers/platform/x86/intel/sdsi.c +++ b/drivers/platform/x86/intel/sdsi.c @@ -398,8 +398,8 @@ free_payload: } static ssize_t provision_akc_write(struct file *filp, struct kobject *kobj, - struct bin_attribute *attr, char *buf, loff_t off, - size_t count) + const struct bin_attribute *attr, char *buf, + loff_t off, size_t count) { struct device *dev = kobj_to_dev(kobj); struct sdsi_priv *priv = dev_get_drvdata(dev); @@ -409,11 +409,11 @@ static ssize_t provision_akc_write(struct file *filp, struct kobject *kobj, return sdsi_provision(priv, buf, count, SDSI_CMD_PROVISION_AKC); } -static BIN_ATTR_WO(provision_akc, SDSI_SIZE_WRITE_MSG); +static const BIN_ATTR_WO(provision_akc, SDSI_SIZE_WRITE_MSG); static ssize_t provision_cap_write(struct file *filp, struct kobject *kobj, - struct bin_attribute *attr, char *buf, loff_t off, - size_t count) + const struct bin_attribute *attr, char *buf, + loff_t off, size_t count) { struct device *dev = kobj_to_dev(kobj); struct sdsi_priv *priv = dev_get_drvdata(dev); @@ -423,7 +423,7 @@ static ssize_t provision_cap_write(struct file *filp, struct kobject *kobj, return sdsi_provision(priv, buf, count, SDSI_CMD_PROVISION_CAP); } -static BIN_ATTR_WO(provision_cap, SDSI_SIZE_WRITE_MSG); +static const BIN_ATTR_WO(provision_cap, SDSI_SIZE_WRITE_MSG); static ssize_t certificate_read(u64 command, u64 control_flags, struct sdsi_priv *priv, @@ -469,7 +469,7 @@ free_buffer: static ssize_t state_certificate_read(struct file *filp, struct kobject *kobj, - struct bin_attribute *attr, char *buf, loff_t off, + const struct bin_attribute *attr, char *buf, loff_t off, size_t count) { struct device *dev = kobj_to_dev(kobj); @@ -477,11 +477,11 @@ state_certificate_read(struct file *filp, struct kobject *kobj, return certificate_read(SDSI_CMD_READ_STATE, 0, priv, buf, off, count); } -static BIN_ATTR_ADMIN_RO(state_certificate, SDSI_SIZE_READ_MSG); +static const BIN_ATTR_ADMIN_RO(state_certificate, SDSI_SIZE_READ_MSG); static ssize_t meter_certificate_read(struct file *filp, struct kobject *kobj, - struct bin_attribute *attr, char *buf, loff_t off, + const struct bin_attribute *attr, char *buf, loff_t off, size_t count) { struct device *dev = kobj_to_dev(kobj); @@ -489,11 +489,11 @@ meter_certificate_read(struct file *filp, struct kobject *kobj, return certificate_read(SDSI_CMD_READ_METER, 0, priv, buf, off, count); } -static BIN_ATTR_ADMIN_RO(meter_certificate, SDSI_SIZE_READ_MSG); +static const BIN_ATTR_ADMIN_RO(meter_certificate, SDSI_SIZE_READ_MSG); static ssize_t meter_current_read(struct file *filp, struct kobject *kobj, - struct bin_attribute *attr, char *buf, loff_t off, + const struct bin_attribute *attr, char *buf, loff_t off, size_t count) { struct device *dev = kobj_to_dev(kobj); @@ -502,11 +502,11 @@ meter_current_read(struct file *filp, struct kobject *kobj, return certificate_read(SDSI_CMD_READ_METER, CTRL_METER_ENABLE_DRAM, priv, buf, off, count); } -static BIN_ATTR_ADMIN_RO(meter_current, SDSI_SIZE_READ_MSG); +static const BIN_ATTR_ADMIN_RO(meter_current, SDSI_SIZE_READ_MSG); static ssize_t registers_read(struct file *filp, struct kobject *kobj, - struct bin_attribute *attr, char *buf, loff_t off, - size_t count) + const struct bin_attribute *attr, char *buf, + loff_t off, size_t count) { struct device *dev = kobj_to_dev(kobj); struct sdsi_priv *priv = dev_get_drvdata(dev); @@ -528,9 +528,9 @@ static ssize_t registers_read(struct file *filp, struct kobject *kobj, return count; } -static BIN_ATTR_ADMIN_RO(registers, SDSI_SIZE_REGS); +static const BIN_ATTR_ADMIN_RO(registers, SDSI_SIZE_REGS); -static struct bin_attribute *sdsi_bin_attrs[] = { +static const struct bin_attribute *const sdsi_bin_attrs[] = { &bin_attr_registers, &bin_attr_state_certificate, &bin_attr_meter_certificate, @@ -576,7 +576,7 @@ static struct attribute *sdsi_attrs[] = { static const struct attribute_group sdsi_group = { .attrs = sdsi_attrs, - .bin_attrs = sdsi_bin_attrs, + .bin_attrs_new = sdsi_bin_attrs, .is_bin_visible = sdsi_battr_is_visible, }; __ATTRIBUTE_GROUPS(sdsi); -- 2.50.1 From 7ff2fecc8bc2e855bd17bd75358973c6bb4accbd Mon Sep 17 00:00:00 2001 From: =?utf8?q?Thomas=20Wei=C3=9Fschuh?= Date: Mon, 2 Dec 2024 20:38:35 +0100 Subject: [PATCH 09/16] platform/x86/intel/pmt: Constify 'struct bin_attribute' MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The sysfs core now allows instances of 'struct bin_attribute' to be moved into read-only memory. Make use of that to protect them against accidental or malicious modifications. Signed-off-by: Thomas Weißschuh Link: https://lore.kernel.org/r/20241202-sysfs-const-bin_attr-pdx86-v1-4-9ab204c2a814@weissschuh.net Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/intel/pmt/class.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c index 8ed54b7a3333..7233b654bbad 100644 --- a/drivers/platform/x86/intel/pmt/class.c +++ b/drivers/platform/x86/intel/pmt/class.c @@ -81,7 +81,7 @@ EXPORT_SYMBOL_NS_GPL(pmt_telem_read_mmio, "INTEL_PMT"); */ static ssize_t intel_pmt_read(struct file *filp, struct kobject *kobj, - struct bin_attribute *attr, char *buf, loff_t off, + const struct bin_attribute *attr, char *buf, loff_t off, size_t count) { struct intel_pmt_entry *entry = container_of(attr, @@ -308,7 +308,7 @@ static int intel_pmt_dev_register(struct intel_pmt_entry *entry, entry->pmt_bin_attr.attr.name = ns->name; entry->pmt_bin_attr.attr.mode = 0440; entry->pmt_bin_attr.mmap = intel_pmt_mmap; - entry->pmt_bin_attr.read = intel_pmt_read; + entry->pmt_bin_attr.read_new = intel_pmt_read; entry->pmt_bin_attr.size = entry->size; ret = sysfs_create_bin_file(&dev->kobj, &entry->pmt_bin_attr); -- 2.50.1 From 8fa1a2e1053ad780eaf48fc05e3a4a24bc809ee2 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Thomas=20Wei=C3=9Fschuh?= Date: Mon, 2 Dec 2024 20:38:36 +0100 Subject: [PATCH 10/16] platform/x86/amd/hsmp: Constify 'struct bin_attribute' MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The sysfs core now allows instances of 'struct bin_attribute' to be moved into read-only memory. Make use of that to protect them against accidental or malicious modifications. Signed-off-by: Thomas Weißschuh Link: https://lore.kernel.org/r/20241202-sysfs-const-bin_attr-pdx86-v1-5-9ab204c2a814@weissschuh.net Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/amd/hsmp/acpi.c | 12 ++++++------ drivers/platform/x86/amd/hsmp/plat.c | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c index e981d45e1c12..444b43be35a2 100644 --- a/drivers/platform/x86/amd/hsmp/acpi.c +++ b/drivers/platform/x86/amd/hsmp/acpi.c @@ -226,7 +226,7 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind) } static ssize_t hsmp_metric_tbl_acpi_read(struct file *filp, struct kobject *kobj, - struct bin_attribute *bin_attr, char *buf, + const struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count) { struct device *dev = container_of(kobj, struct device, kobj); @@ -285,19 +285,19 @@ static int init_acpi(struct device *dev) return ret; } -static struct bin_attribute hsmp_metric_tbl_attr = { +static const struct bin_attribute hsmp_metric_tbl_attr = { .attr = { .name = HSMP_METRICS_TABLE_NAME, .mode = 0444}, - .read = hsmp_metric_tbl_acpi_read, + .read_new = hsmp_metric_tbl_acpi_read, .size = sizeof(struct hsmp_metric_table), }; -static struct bin_attribute *hsmp_attr_list[] = { +static const struct bin_attribute *hsmp_attr_list[] = { &hsmp_metric_tbl_attr, NULL }; -static struct attribute_group hsmp_attr_grp = { - .bin_attrs = hsmp_attr_list, +static const struct attribute_group hsmp_attr_grp = { + .bin_attrs_new = hsmp_attr_list, .is_bin_visible = hsmp_is_sock_attr_visible, }; diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c index a61f815c9f80..02ca85762b68 100644 --- a/drivers/platform/x86/amd/hsmp/plat.c +++ b/drivers/platform/x86/amd/hsmp/plat.c @@ -59,7 +59,7 @@ static int amd_hsmp_pci_rdwr(struct hsmp_socket *sock, u32 offset, } static ssize_t hsmp_metric_tbl_plat_read(struct file *filp, struct kobject *kobj, - struct bin_attribute *bin_attr, char *buf, + const struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count) { struct hsmp_socket *sock; @@ -97,13 +97,13 @@ static umode_t hsmp_is_sock_attr_visible(struct kobject *kobj, * is_bin_visible function is used to show / hide the necessary groups. */ #define HSMP_BIN_ATTR(index, _list) \ -static struct bin_attribute attr##index = { \ +static const struct bin_attribute attr##index = { \ .attr = { .name = HSMP_METRICS_TABLE_NAME, .mode = 0444}, \ .private = (void *)index, \ - .read = hsmp_metric_tbl_plat_read, \ + .read_new = hsmp_metric_tbl_plat_read, \ .size = sizeof(struct hsmp_metric_table), \ }; \ -static struct bin_attribute _list[] = { \ +static const struct bin_attribute _list[] = { \ &attr##index, \ NULL \ } @@ -118,8 +118,8 @@ HSMP_BIN_ATTR(6, *sock6_attr_list); HSMP_BIN_ATTR(7, *sock7_attr_list); #define HSMP_BIN_ATTR_GRP(index, _list, _name) \ -static struct attribute_group sock##index##_attr_grp = { \ - .bin_attrs = _list, \ +static const struct attribute_group sock##index##_attr_grp = { \ + .bin_attrs_new = _list, \ .is_bin_visible = hsmp_is_sock_attr_visible, \ .name = #_name, \ } -- 2.50.1 From 3bc018395f106d11d8f882219bad76ca422d66df Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ilpo=20J=C3=A4rvinen?= Date: Tue, 10 Dec 2024 16:01:14 +0200 Subject: [PATCH 11/16] platform/x86/intel/tpmi/plr: Make char[] longer to silence warning MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit W=1 build triggers this warning: drivers/platform/x86/intel/plr_tpmi.c:315:55: error: ‘snprintf’ output may be truncated before the last format character [-Werror=format-truncation=] 315 | snprintf(name, sizeof(name), "domain%d", i); | ^ drivers/platform/x86/intel/plr_tpmi.c:315:17: note: ‘snprintf’ output between 8 and 17 bytes into a destination of size 16 315 | snprintf(name, sizeof(name), "domain%d", i); Inspecting the code tells that maximum i in intel_plr_probe() will fit into u8 because it comes from: struct intel_tpmi_pfs_entry { ... u64 num_entries:8; ...but compiler does not know that. Saving one byte in name[] at the expense of a warning with W=1 seems a bad trade so simply make it name[17]. Link: https://lore.kernel.org/r/20241210140115.1375-1-ilpo.jarvinen@linux.intel.com Signed-off-by: Ilpo Järvinen Reviewed-by: Ilpo Järvinen --- drivers/platform/x86/intel/plr_tpmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/platform/x86/intel/plr_tpmi.c b/drivers/platform/x86/intel/plr_tpmi.c index 691d43c3592c..2b55347a5a93 100644 --- a/drivers/platform/x86/intel/plr_tpmi.c +++ b/drivers/platform/x86/intel/plr_tpmi.c @@ -262,7 +262,7 @@ static int intel_plr_probe(struct auxiliary_device *auxdev, const struct auxilia struct resource *res; struct tpmi_plr *plr; void __iomem *base; - char name[16]; + char name[17]; int err; plat_info = tpmi_get_platform_data(auxdev); -- 2.50.1 From bdf0ea9d521c390c4d7bff254eafbe7712441292 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 9 Dec 2024 19:35:54 +0100 Subject: [PATCH 12/16] platform/x86: dell-smo8800: Move SMO88xx acpi_device_ids to dell-smo8800-ids.h MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Move the SMO88xx acpi_device_ids to a new dell-smo8800-ids.h header, so that these can be shared with the new dell-lis3lv02d code. Reviewed-by: Pali Rohár Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20241209183557.7560-2-hdegoede@redhat.com Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/dell/dell-smo8800-ids.h | 27 ++++++++++++++++++++ drivers/platform/x86/dell/dell-smo8800.c | 16 +----------- 2 files changed, 28 insertions(+), 15 deletions(-) create mode 100644 drivers/platform/x86/dell/dell-smo8800-ids.h diff --git a/drivers/platform/x86/dell/dell-smo8800-ids.h b/drivers/platform/x86/dell/dell-smo8800-ids.h new file mode 100644 index 000000000000..ec58e229ba7a --- /dev/null +++ b/drivers/platform/x86/dell/dell-smo8800-ids.h @@ -0,0 +1,27 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * ACPI SMO88XX lis3lv02d freefall / accelerometer device-ids. + * + * Copyright (C) 2012 Sonal Santan + * Copyright (C) 2014 Pali Rohár + */ +#ifndef _DELL_SMO8800_IDS_H_ +#define _DELL_SMO8800_IDS_H_ + +#include +#include + +static const struct acpi_device_id smo8800_ids[] = { + { "SMO8800" }, + { "SMO8801" }, + { "SMO8810" }, + { "SMO8811" }, + { "SMO8820" }, + { "SMO8821" }, + { "SMO8830" }, + { "SMO8831" }, + { } +}; +MODULE_DEVICE_TABLE(acpi, smo8800_ids); + +#endif diff --git a/drivers/platform/x86/dell/dell-smo8800.c b/drivers/platform/x86/dell/dell-smo8800.c index 87fe03f23f24..8872f9b57fce 100644 --- a/drivers/platform/x86/dell/dell-smo8800.c +++ b/drivers/platform/x86/dell/dell-smo8800.c @@ -14,10 +14,10 @@ #include #include #include -#include #include #include #include +#include "dell-smo8800-ids.h" struct smo8800_device { u32 irq; /* acpi device irq */ @@ -163,20 +163,6 @@ static void smo8800_remove(struct platform_device *device) dev_dbg(&device->dev, "device /dev/freefall unregistered\n"); } -/* NOTE: Keep this list in sync with drivers/i2c/busses/i2c-i801.c */ -static const struct acpi_device_id smo8800_ids[] = { - { "SMO8800", 0 }, - { "SMO8801", 0 }, - { "SMO8810", 0 }, - { "SMO8811", 0 }, - { "SMO8820", 0 }, - { "SMO8821", 0 }, - { "SMO8830", 0 }, - { "SMO8831", 0 }, - { "", 0 }, -}; -MODULE_DEVICE_TABLE(acpi, smo8800_ids); - static struct platform_driver smo8800_driver = { .probe = smo8800_probe, .remove = smo8800_remove, -- 2.50.1 From cc0809b752c74b768845f6e049b419c32d954a1a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 9 Dec 2024 19:35:55 +0100 Subject: [PATCH 13/16] platform/x86: dell-smo8800: Move instantiation of lis3lv02d i2c_client from i2c-i801 to dell-lis3lv02d MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Various Dell laptops have an lis3lv02d freefall/accelerometer sensor. The lis3lv02d chip has an interrupt line as well as an I2C connection to the system's main SMBus. The lis3lv02d is described in the ACPI tables by an SMO88xx ACPI device, but the SMO88xx ACPI fwnodes are incomplete and only list an IRQ resource. So far this has been worked around with some SMO88xx specific quirk code in the generic i2c-i801 driver, but it is not necessary to handle the Dell specific instantiation of i2c_client-s for SMO88xx ACPI devices there. The kernel already instantiates platform_device-s for these with an acpi:SMO88xx modalias. The drivers/platform/x86/dell/dell-smo8800.c driver binds to this platform device but this only deals with the interrupt resource. Add a drivers/platform/x86/dell/dell-lis3lv02d.c which will matches on the same acpi:SMO88xx modaliases and move the i2c_client instantiation from the generic i2c-i801 driver there. Moving the i2c_client instantiation has the following advantages: 1. This moves the SMO88xx ACPI device quirk handling away from the generic i2c-i801 module which is loaded on all Intel x86 machines to a module which will only be loaded when there is an ACPI SMO88xx device. 2. This removes the duplication of the SMO88xx ACPI Hardware ID (HID) table between the i2c-i801 and dell-smo8800 drivers. 3. This allows extending the quirk handling by adding new code and related module parameters to the dell-lis3lv02d driver, without needing to modify the i2c-i801 code. Reviewed-by: Pali Rohár Signed-off-by: Hans de Goede Acked-by: Wolfram Sang Link: https://lore.kernel.org/r/20241209183557.7560-3-hdegoede@redhat.com Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/i2c/busses/i2c-i801.c | 124 ------------- drivers/platform/x86/dell/Kconfig | 1 + drivers/platform/x86/dell/Makefile | 1 + drivers/platform/x86/dell/dell-lis3lv02d.c | 204 +++++++++++++++++++++ 4 files changed, 206 insertions(+), 124 deletions(-) create mode 100644 drivers/platform/x86/dell/dell-lis3lv02d.c diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c index 75dab01d43a7..b72712743cd7 100644 --- a/drivers/i2c/busses/i2c-i801.c +++ b/drivers/i2c/busses/i2c-i801.c @@ -1162,127 +1162,6 @@ static void dmi_check_onboard_devices(const struct dmi_header *dm, void *adap) } } -/* NOTE: Keep this list in sync with drivers/platform/x86/dell-smo8800.c */ -static const char *const acpi_smo8800_ids[] = { - "SMO8800", - "SMO8801", - "SMO8810", - "SMO8811", - "SMO8820", - "SMO8821", - "SMO8830", - "SMO8831", -}; - -static acpi_status check_acpi_smo88xx_device(acpi_handle obj_handle, - u32 nesting_level, - void *context, - void **return_value) -{ - struct acpi_device_info *info; - acpi_status status; - char *hid; - int i; - - status = acpi_get_object_info(obj_handle, &info); - if (ACPI_FAILURE(status)) - return AE_OK; - - if (!(info->valid & ACPI_VALID_HID)) - goto smo88xx_not_found; - - hid = info->hardware_id.string; - if (!hid) - goto smo88xx_not_found; - - i = match_string(acpi_smo8800_ids, ARRAY_SIZE(acpi_smo8800_ids), hid); - if (i < 0) - goto smo88xx_not_found; - - kfree(info); - - *return_value = NULL; - return AE_CTRL_TERMINATE; - -smo88xx_not_found: - kfree(info); - return AE_OK; -} - -static bool is_dell_system_with_lis3lv02d(void) -{ - void *err = ERR_PTR(-ENOENT); - - if (!dmi_match(DMI_SYS_VENDOR, "Dell Inc.")) - return false; - - /* - * Check that ACPI device SMO88xx is present and is functioning. - * Function acpi_get_devices() already filters all ACPI devices - * which are not present or are not functioning. - * ACPI device SMO88xx represents our ST microelectronics lis3lv02d - * accelerometer but unfortunately ACPI does not provide any other - * information (like I2C address). - */ - acpi_get_devices(NULL, check_acpi_smo88xx_device, NULL, &err); - - return !IS_ERR(err); -} - -/* - * Accelerometer's I2C address is not specified in DMI nor ACPI, - * so it is needed to define mapping table based on DMI product names. - */ -static const struct { - const char *dmi_product_name; - unsigned short i2c_addr; -} dell_lis3lv02d_devices[] = { - /* - * Dell platform team told us that these Latitude devices have - * ST microelectronics accelerometer at I2C address 0x29. - */ - { "Latitude E5250", 0x29 }, - { "Latitude E5450", 0x29 }, - { "Latitude E5550", 0x29 }, - { "Latitude E6440", 0x29 }, - { "Latitude E6440 ATG", 0x29 }, - { "Latitude E6540", 0x29 }, - /* - * Additional individual entries were added after verification. - */ - { "Latitude 5480", 0x29 }, - { "Precision 3540", 0x29 }, - { "Vostro V131", 0x1d }, - { "Vostro 5568", 0x29 }, - { "XPS 15 7590", 0x29 }, -}; - -static void register_dell_lis3lv02d_i2c_device(struct i801_priv *priv) -{ - struct i2c_board_info info; - const char *dmi_product_name; - int i; - - dmi_product_name = dmi_get_system_info(DMI_PRODUCT_NAME); - for (i = 0; i < ARRAY_SIZE(dell_lis3lv02d_devices); ++i) { - if (strcmp(dmi_product_name, - dell_lis3lv02d_devices[i].dmi_product_name) == 0) - break; - } - - if (i == ARRAY_SIZE(dell_lis3lv02d_devices)) { - dev_warn(&priv->pci_dev->dev, - "Accelerometer lis3lv02d is present on SMBus but its" - " address is unknown, skipping registration\n"); - return; - } - - memset(&info, 0, sizeof(struct i2c_board_info)); - info.addr = dell_lis3lv02d_devices[i].i2c_addr; - strscpy(info.type, "lis3lv02d", I2C_NAME_SIZE); - i2c_new_client_device(&priv->adapter, &info); -} - /* Register optional targets */ static void i801_probe_optional_targets(struct i801_priv *priv) { @@ -1302,9 +1181,6 @@ static void i801_probe_optional_targets(struct i801_priv *priv) if (dmi_name_in_vendors("FUJITSU")) dmi_walk(dmi_check_onboard_devices, &priv->adapter); - if (is_dell_system_with_lis3lv02d()) - register_dell_lis3lv02d_i2c_device(priv); - /* Instantiate SPD EEPROMs unless the SMBus is multiplexed */ #ifdef CONFIG_I2C_I801_MUX if (!priv->mux_pdev) diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig index 2dddafb3f7fa..d09060aedd3f 100644 --- a/drivers/platform/x86/dell/Kconfig +++ b/drivers/platform/x86/dell/Kconfig @@ -152,6 +152,7 @@ config DELL_SMBIOS_SMM config DELL_SMO8800 tristate "Dell Latitude freefall driver (ACPI SMO88XX)" default m + depends on I2C depends on ACPI || COMPILE_TEST help Say Y here if you want to support SMO88XX freefall devices diff --git a/drivers/platform/x86/dell/Makefile b/drivers/platform/x86/dell/Makefile index 79d60f1bf4c1..bb3cbd470a46 100644 --- a/drivers/platform/x86/dell/Makefile +++ b/drivers/platform/x86/dell/Makefile @@ -15,6 +15,7 @@ dell-smbios-objs := dell-smbios-base.o dell-smbios-$(CONFIG_DELL_SMBIOS_WMI) += dell-smbios-wmi.o dell-smbios-$(CONFIG_DELL_SMBIOS_SMM) += dell-smbios-smm.o obj-$(CONFIG_DELL_SMO8800) += dell-smo8800.o +obj-$(CONFIG_DELL_SMO8800) += dell-lis3lv02d.o obj-$(CONFIG_DELL_UART_BACKLIGHT) += dell-uart-backlight.o obj-$(CONFIG_DELL_WMI) += dell-wmi.o dell-wmi-objs := dell-wmi-base.o diff --git a/drivers/platform/x86/dell/dell-lis3lv02d.c b/drivers/platform/x86/dell/dell-lis3lv02d.c new file mode 100644 index 000000000000..6dc04c89e6c9 --- /dev/null +++ b/drivers/platform/x86/dell/dell-lis3lv02d.c @@ -0,0 +1,204 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * lis3lv02d i2c-client instantiation for ACPI SMO88xx devices without I2C resources. + * + * Copyright (C) 2024 Hans de Goede + */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include +#include +#include +#include +#include +#include +#include +#include "dell-smo8800-ids.h" + +#define DELL_LIS3LV02D_DMI_ENTRY(product_name, i2c_addr) \ + { \ + .matches = { \ + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Dell Inc."), \ + DMI_EXACT_MATCH(DMI_PRODUCT_NAME, product_name), \ + }, \ + .driver_data = (void *)(uintptr_t)(i2c_addr), \ + } + +/* + * Accelerometer's I2C address is not specified in DMI nor ACPI, + * so it is needed to define mapping table based on DMI product names. + */ +static const struct dmi_system_id lis3lv02d_devices[] __initconst = { + /* + * Dell platform team told us that these Latitude devices have + * ST microelectronics accelerometer at I2C address 0x29. + */ + DELL_LIS3LV02D_DMI_ENTRY("Latitude E5250", 0x29), + DELL_LIS3LV02D_DMI_ENTRY("Latitude E5450", 0x29), + DELL_LIS3LV02D_DMI_ENTRY("Latitude E5550", 0x29), + DELL_LIS3LV02D_DMI_ENTRY("Latitude E6440", 0x29), + DELL_LIS3LV02D_DMI_ENTRY("Latitude E6440 ATG", 0x29), + DELL_LIS3LV02D_DMI_ENTRY("Latitude E6540", 0x29), + /* + * Additional individual entries were added after verification. + */ + DELL_LIS3LV02D_DMI_ENTRY("Latitude 5480", 0x29), + DELL_LIS3LV02D_DMI_ENTRY("Precision 3540", 0x29), + DELL_LIS3LV02D_DMI_ENTRY("Vostro V131", 0x1d), + DELL_LIS3LV02D_DMI_ENTRY("Vostro 5568", 0x29), + DELL_LIS3LV02D_DMI_ENTRY("XPS 15 7590", 0x29), + { } +}; + +static u8 i2c_addr; +static struct i2c_client *i2c_dev; +static bool notifier_registered; + +static bool i2c_adapter_is_main_i801(struct i2c_adapter *adap) +{ + /* + * Only match the main I801 adapter and reject secondary adapters + * which names start with "SMBus I801 IDF adapter". + */ + return strstarts(adap->name, "SMBus I801 adapter"); +} + +static int find_i801(struct device *dev, void *data) +{ + struct i2c_adapter *adap, **adap_ret = data; + + adap = i2c_verify_adapter(dev); + if (!adap) + return 0; + + if (!i2c_adapter_is_main_i801(adap)) + return 0; + + *adap_ret = i2c_get_adapter(adap->nr); + return 1; +} + +static void instantiate_i2c_client(struct work_struct *work) +{ + struct i2c_board_info info = { }; + struct i2c_adapter *adap = NULL; + + if (i2c_dev) + return; + + /* + * bus_for_each_dev() and not i2c_for_each_dev() to avoid + * a deadlock when find_i801() calls i2c_get_adapter(). + */ + bus_for_each_dev(&i2c_bus_type, NULL, &adap, find_i801); + if (!adap) + return; + + info.addr = i2c_addr; + strscpy(info.type, "lis3lv02d", I2C_NAME_SIZE); + + i2c_dev = i2c_new_client_device(adap, &info); + if (IS_ERR(i2c_dev)) { + dev_err(&adap->dev, "error %ld registering i2c_client\n", PTR_ERR(i2c_dev)); + i2c_dev = NULL; + } else { + dev_dbg(&adap->dev, "registered lis3lv02d on address 0x%02x\n", info.addr); + } + + i2c_put_adapter(adap); +} +static DECLARE_WORK(i2c_work, instantiate_i2c_client); + +static int i2c_bus_notify(struct notifier_block *nb, unsigned long action, void *data) +{ + struct device *dev = data; + struct i2c_client *client; + struct i2c_adapter *adap; + + switch (action) { + case BUS_NOTIFY_ADD_DEVICE: + adap = i2c_verify_adapter(dev); + if (!adap) + break; + + if (i2c_adapter_is_main_i801(adap)) + queue_work(system_long_wq, &i2c_work); + break; + case BUS_NOTIFY_REMOVED_DEVICE: + client = i2c_verify_client(dev); + if (!client) + break; + + if (i2c_dev == client) { + dev_dbg(&client->adapter->dev, "lis3lv02d i2c_client removed\n"); + i2c_dev = NULL; + } + break; + default: + break; + } + + return 0; +} +static struct notifier_block i2c_nb = { .notifier_call = i2c_bus_notify }; + +static int __init match_acpi_device_ids(struct device *dev, const void *data) +{ + return acpi_match_device(data, dev) ? 1 : 0; +} + +static int __init dell_lis3lv02d_init(void) +{ + const struct dmi_system_id *lis3lv02d_dmi_id; + struct device *dev; + int err; + + /* + * First check for a matching platform_device. This protects against + * SMO88xx ACPI fwnodes which actually do have an I2C resource, which + * will already have an i2c_client instantiated (not a platform_device). + */ + dev = bus_find_device(&platform_bus_type, NULL, smo8800_ids, match_acpi_device_ids); + if (!dev) { + pr_debug("No SMO88xx platform-device found\n"); + return 0; + } + put_device(dev); + + lis3lv02d_dmi_id = dmi_first_match(lis3lv02d_devices); + if (!lis3lv02d_dmi_id) { + pr_warn("accelerometer is present on SMBus but its address is unknown, skipping registration\n"); + return 0; + } + + i2c_addr = (long)lis3lv02d_dmi_id->driver_data; + + /* + * Register i2c-bus notifier + queue initial scan for lis3lv02d + * i2c_client instantiation. + */ + err = bus_register_notifier(&i2c_bus_type, &i2c_nb); + if (err) + return err; + + notifier_registered = true; + + queue_work(system_long_wq, &i2c_work); + return 0; +} +module_init(dell_lis3lv02d_init); + +static void __exit dell_lis3lv02d_module_exit(void) +{ + if (!notifier_registered) + return; + + bus_unregister_notifier(&i2c_bus_type, &i2c_nb); + cancel_work_sync(&i2c_work); + i2c_unregister_device(i2c_dev); +} +module_exit(dell_lis3lv02d_module_exit); + +MODULE_DESCRIPTION("lis3lv02d i2c-client instantiation for ACPI SMO88xx devices"); +MODULE_AUTHOR("Hans de Goede "); +MODULE_LICENSE("GPL"); -- 2.50.1 From e21bff27f2061f75a51951104e16fac752c3b840 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 9 Dec 2024 19:35:56 +0100 Subject: [PATCH 14/16] platform/x86: dell-smo8800: Add a couple more models to lis3lv02d_devices[] MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Add the accelerometer address for the following laptop models to lis3lv02d_devices[]: Dell Latitude E6330 Dell Latitude E6430 Dell XPS 15 9550 Tested-by: Hans de Goede Reviewed-by: Pali Rohár Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20241209183557.7560-4-hdegoede@redhat.com Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/dell/dell-lis3lv02d.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/platform/x86/dell/dell-lis3lv02d.c b/drivers/platform/x86/dell/dell-lis3lv02d.c index 6dc04c89e6c9..d2b34e10c5eb 100644 --- a/drivers/platform/x86/dell/dell-lis3lv02d.c +++ b/drivers/platform/x86/dell/dell-lis3lv02d.c @@ -43,10 +43,13 @@ static const struct dmi_system_id lis3lv02d_devices[] __initconst = { * Additional individual entries were added after verification. */ DELL_LIS3LV02D_DMI_ENTRY("Latitude 5480", 0x29), + DELL_LIS3LV02D_DMI_ENTRY("Latitude E6330", 0x29), + DELL_LIS3LV02D_DMI_ENTRY("Latitude E6430", 0x29), DELL_LIS3LV02D_DMI_ENTRY("Precision 3540", 0x29), DELL_LIS3LV02D_DMI_ENTRY("Vostro V131", 0x1d), DELL_LIS3LV02D_DMI_ENTRY("Vostro 5568", 0x29), DELL_LIS3LV02D_DMI_ENTRY("XPS 15 7590", 0x29), + DELL_LIS3LV02D_DMI_ENTRY("XPS 15 9550", 0x29), { } }; -- 2.50.1 From 204d45fe680be98be356acb7c603622606356be7 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Mon, 16 Dec 2024 09:34:05 +0100 Subject: [PATCH 15/16] platform/x86/intel: bytcrc_pwrsrc: fix power_supply dependency MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The driver now fails to link when the power supply core is missing or in a loadable module: _64-linux/bin/x86_64-linux-ld: drivers/platform/x86/intel/bytcrc_pwrsrc.o: in function `crc_pwrsrc_irq_handler': bytcrc_pwrsrc.c:(.text+0x2aa): undefined reference to `power_supply_changed' x86_64-linux-ld: drivers/platform/x86/intel/bytcrc_pwrsrc.o: in function `crc_pwrsrc_psy_get_property': bytcrc_pwrsrc.c:(.text+0x2f6): undefined reference to `power_supply_get_drvdata' x86_64-linux-ld: drivers/platform/x86/intel/bytcrc_pwrsrc.o: in function `crc_pwrsrc_probe': bytcrc_pwrsrc.c:(.text+0x644): undefined reference to `devm_power_supply_register' Add the appropriate dependency for it. Fixes: 0130ec83c553 ("platform/x86/intel: bytcrc_pwrsrc: Optionally register a power_supply dev") Signed-off-by: Arnd Bergmann Reviewed-by: Andy Shevchenko Link: https://lore.kernel.org/r/20241216083409.1885677-1-arnd@kernel.org Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/intel/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/platform/x86/intel/Kconfig b/drivers/platform/x86/intel/Kconfig index eb698dcb9af9..19a2246f2770 100644 --- a/drivers/platform/x86/intel/Kconfig +++ b/drivers/platform/x86/intel/Kconfig @@ -83,6 +83,7 @@ config INTEL_BXTWC_PMIC_TMU config INTEL_BYTCRC_PWRSRC tristate "Intel Bay Trail Crystal Cove power source driver" depends on INTEL_SOC_PMIC + depends on POWER_SUPPLY help This option adds a power source driver for Crystal Cove PMICs on Intel Bay Trail devices. -- 2.50.1 From 0c91d916af898e481444d17eed8ce8df6c50f70d Mon Sep 17 00:00:00 2001 From: Armin Wolf Date: Fri, 6 Dec 2024 22:56:50 +0100 Subject: [PATCH 16/16] platform/x86: wmi-bmof: Make use of .bin_size() callback MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Until now the wmi-bmof driver had to allocate the binary sysfs attribute dynamically since its size depends on the bmof buffer returned by the firmware. Use the new .bin_size() callback to avoid having to do this memory allocation. Tested on a Asus Prime B650-Plus. Signed-off-by: Armin Wolf Link: https://lore.kernel.org/r/20241206215650.2977-1-W_Armin@gmx.de Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/wmi-bmof.c | 75 +++++++++++++++++---------------- 1 file changed, 38 insertions(+), 37 deletions(-) diff --git a/drivers/platform/x86/wmi-bmof.c b/drivers/platform/x86/wmi-bmof.c index df6f0ae6e6c7..3e33da36da8a 100644 --- a/drivers/platform/x86/wmi-bmof.c +++ b/drivers/platform/x86/wmi-bmof.c @@ -20,66 +20,66 @@ #define WMI_BMOF_GUID "05901221-D566-11D1-B2F0-00A0C9062910" -struct bmof_priv { - union acpi_object *bmofdata; - struct bin_attribute bmof_bin_attr; -}; - -static ssize_t read_bmof(struct file *filp, struct kobject *kobj, struct bin_attribute *attr, +static ssize_t bmof_read(struct file *filp, struct kobject *kobj, const struct bin_attribute *attr, char *buf, loff_t off, size_t count) { - struct bmof_priv *priv = container_of(attr, struct bmof_priv, bmof_bin_attr); + struct device *dev = kobj_to_dev(kobj); + union acpi_object *obj = dev_get_drvdata(dev); - return memory_read_from_buffer(buf, count, &off, priv->bmofdata->buffer.pointer, - priv->bmofdata->buffer.length); + return memory_read_from_buffer(buf, count, &off, obj->buffer.pointer, obj->buffer.length); } -static int wmi_bmof_probe(struct wmi_device *wdev, const void *context) +static const BIN_ATTR_ADMIN_RO(bmof, 0); + +static const struct bin_attribute * const bmof_attrs[] = { + &bin_attr_bmof, + NULL +}; + +static size_t bmof_bin_size(struct kobject *kobj, const struct bin_attribute *attr, int n) { - struct bmof_priv *priv; - int ret; + struct device *dev = kobj_to_dev(kobj); + union acpi_object *obj = dev_get_drvdata(dev); + + return obj->buffer.length; +} - priv = devm_kzalloc(&wdev->dev, sizeof(struct bmof_priv), GFP_KERNEL); - if (!priv) - return -ENOMEM; +static const struct attribute_group bmof_group = { + .bin_size = bmof_bin_size, + .bin_attrs_new = bmof_attrs, +}; + +static const struct attribute_group *bmof_groups[] = { + &bmof_group, + NULL +}; - dev_set_drvdata(&wdev->dev, priv); +static int wmi_bmof_probe(struct wmi_device *wdev, const void *context) +{ + union acpi_object *obj; - priv->bmofdata = wmidev_block_query(wdev, 0); - if (!priv->bmofdata) { + obj = wmidev_block_query(wdev, 0); + if (!obj) { dev_err(&wdev->dev, "failed to read Binary MOF\n"); return -EIO; } - if (priv->bmofdata->type != ACPI_TYPE_BUFFER) { + if (obj->type != ACPI_TYPE_BUFFER) { dev_err(&wdev->dev, "Binary MOF is not a buffer\n"); - ret = -EIO; - goto err_free; + kfree(obj); + return -EIO; } - sysfs_bin_attr_init(&priv->bmof_bin_attr); - priv->bmof_bin_attr.attr.name = "bmof"; - priv->bmof_bin_attr.attr.mode = 0400; - priv->bmof_bin_attr.read = read_bmof; - priv->bmof_bin_attr.size = priv->bmofdata->buffer.length; - - ret = device_create_bin_file(&wdev->dev, &priv->bmof_bin_attr); - if (ret) - goto err_free; + dev_set_drvdata(&wdev->dev, obj); return 0; - - err_free: - kfree(priv->bmofdata); - return ret; } static void wmi_bmof_remove(struct wmi_device *wdev) { - struct bmof_priv *priv = dev_get_drvdata(&wdev->dev); + union acpi_object *obj = dev_get_drvdata(&wdev->dev); - device_remove_bin_file(&wdev->dev, &priv->bmof_bin_attr); - kfree(priv->bmofdata); + kfree(obj); } static const struct wmi_device_id wmi_bmof_id_table[] = { @@ -90,6 +90,7 @@ static const struct wmi_device_id wmi_bmof_id_table[] = { static struct wmi_driver wmi_bmof_driver = { .driver = { .name = "wmi-bmof", + .dev_groups = bmof_groups, }, .probe = wmi_bmof_probe, .remove = wmi_bmof_remove, -- 2.50.1