| Message ID | 20250102004854.14874-3-derekjohn.clark@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | platform/x86: Add Lenovo Gaming Series WMI Drivers | expand |
On 1/1/25 18:47, Derek J. Clark wrote: > Adds lenovo-wmi-gamezone.c which provides a driver for the Lenovo > GameZone WMI interface that comes on Lenovo "Gaming Series" hardware. > Provides ACPI platform profiles over WMI. > > v2: > - Use devm_kzalloc to ensure driver can be instanced, remove global > reference. > - Ensure reverse Christmas tree for all variable declarations. > - Remove extra whitespace. > - Use guard(mutex) in all mutex instances, global mutex. > - Use pr_fmt instead of adding the driver name to each pr_err. > - Remove noisy pr_info usage. > - Rename gamezone_wmi to lenovo_wmi_gz_priv and gz_wmi to priv. > - Remove GZ_WMI symbol exporting. > > Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> > --- > MAINTAINERS | 7 + > drivers/platform/x86/Kconfig | 11 ++ > drivers/platform/x86/Makefile | 1 + > drivers/platform/x86/lenovo-wmi-gamezone.c | 203 +++++++++++++++++++++ > drivers/platform/x86/lenovo-wmi.h | 105 +++++++++++ > 5 files changed, 327 insertions(+) > create mode 100644 drivers/platform/x86/lenovo-wmi-gamezone.c > create mode 100644 drivers/platform/x86/lenovo-wmi.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index baf0eeb9a355..8f8a6aec6b92 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -13034,6 +13034,13 @@ S: Maintained > W: http://legousb.sourceforge.net/ > F: drivers/usb/misc/legousbtower.c > > +LENOVO WMI drivers > +M: Derek J. Clark <derekjohn.clark@gmail.com> > +L: platform-driver-x86@vger.kernel.org > +S: Maintained > +F: drivers/platform/x86/lenovo-wmi-gamezone.c > +F: drivers/platform/x86/lenovo-wmi.h > + > LETSKETCH HID TABLET DRIVER > M: Hans de Goede <hdegoede@redhat.com> > L: linux-input@vger.kernel.org > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 0258dd879d64..9a6ac7fdec9f 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -459,6 +459,17 @@ config IBM_RTL > state = 0 (BIOS SMIs on) > state = 1 (BIOS SMIs off) > > +config LENOVO_WMI_GAMEZONE > + tristate "Lenovo GameZone WMI Driver" > + depends on ACPI_WMI > + select ACPI_PLATFORM_PROFILE > + help > + Say Y here if you have a WMI aware Lenovo Legion device and would like to use the > + platform-profile firmware interface. > + > + To compile this driver as a module, choose M here: the module will > + be called lenovo_wmi_gamezone. > + > config IDEAPAD_LAPTOP > tristate "Lenovo IdeaPad Laptop Extras" > depends on ACPI > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > index e1b142947067..7cb29a480ed2 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -68,6 +68,7 @@ obj-$(CONFIG_THINKPAD_LMI) += think-lmi.o > obj-$(CONFIG_YOGABOOK) += lenovo-yogabook.o > obj-$(CONFIG_YT2_1380) += lenovo-yoga-tab2-pro-1380-fastcharger.o > obj-$(CONFIG_LENOVO_WMI_CAMERA) += lenovo-wmi-camera.o > +obj-$(CONFIG_LENOVO_WMI_GAMEZONE) += lenovo-wmi-gamezone.o > > # Intel > obj-y += intel/ > diff --git a/drivers/platform/x86/lenovo-wmi-gamezone.c b/drivers/platform/x86/lenovo-wmi-gamezone.c > new file mode 100644 > index 000000000000..da5e2bc41f39 > --- /dev/null > +++ b/drivers/platform/x86/lenovo-wmi-gamezone.c > @@ -0,0 +1,203 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Lenovo GameZone WMI interface driver. The GameZone WMI interface provides > + * platform profile and fan curve settings for devices that fall under the > + * "Gaming Series" of Lenovo Legion devices. > + * > + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> > + */ > + > +#include <linux/platform_profile.h> > +#include "lenovo-wmi.h" > + > +#define LENOVO_GAMEZONE_GUID "887B54E3-DDDC-4B2C-8B88-68A26A8835D0" > + > +/* Method IDs */ > +#define WMI_METHOD_ID_SMARTFAN_SUPP 43 /* IsSupportSmartFan */ > +#define WMI_METHOD_ID_SMARTFAN_SET 44 /* SetSmartFanMode */ > +#define WMI_METHOD_ID_SMARTFAN_GET 45 /* GetSmartFanMode */ > + > +static DEFINE_MUTEX(call_mutex); > + > +static const struct wmi_device_id lenovo_wmi_gamezone_id_table[] = { > + { LENOVO_GAMEZONE_GUID, NULL }, /* LENOVO_GAMEZONE_DATA */ > + {} > +}; > + > +struct lenovo_wmi_gz_priv { > + struct wmi_device *wdev; > + enum platform_profile_option current_profile; > + struct platform_profile_handler pprof; > + bool platform_profile_support; > +}; > + > +/* Platform Profile Methods */ > +static int lenovo_wmi_gamezone_platform_profile_supported( > + struct platform_profile_handler *pprof, int *supported) > +{ > + struct lenovo_wmi_gz_priv *priv; > + > + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); > + > + guard(mutex)(&call_mutex); > + return lenovo_wmidev_evaluate_method_1( > + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_SUPP, 0, supported); > +} > + > +static int > +lenovo_wmi_gamezone_profile_get(struct platform_profile_handler *pprof, > + enum platform_profile_option *profile) > +{ > + struct lenovo_wmi_gz_priv *priv; > + int sel_prof; > + int err; > + > + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); > + > + guard(mutex)(&call_mutex); > + err = lenovo_wmidev_evaluate_method_1( > + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_GET, 0, &sel_prof); > + if (err) { > + pr_err("Error getting fan profile from WMI interface: %d\n", > + err); > + return err; > + } > + > + switch (sel_prof) { > + case SMARTFAN_MODE_QUIET: > + *profile = PLATFORM_PROFILE_QUIET; > + break; > + case SMARTFAN_MODE_BALANCED: > + *profile = PLATFORM_PROFILE_BALANCED; > + break; > + case SMARTFAN_MODE_PERFORMANCE: > + *profile = PLATFORM_PROFILE_PERFORMANCE; > + break; > + case SMARTFAN_MODE_CUSTOM: > + *profile = PLATFORM_PROFILE_CUSTOM; > + break; > + default: > + return -EINVAL; > + } > + priv->current_profile = *profile; > + > + return 0; > +} > + > +static int > +lenovo_wmi_gamezone_profile_set(struct platform_profile_handler *pprof, > + enum platform_profile_option profile) > +{ > + struct lenovo_wmi_gz_priv *priv; > + int sel_prof; > + int err; > + > + switch (profile) { > + case PLATFORM_PROFILE_QUIET: > + sel_prof = SMARTFAN_MODE_QUIET; > + break; > + case PLATFORM_PROFILE_BALANCED: > + sel_prof = SMARTFAN_MODE_BALANCED; > + break; > + case PLATFORM_PROFILE_PERFORMANCE: > + sel_prof = SMARTFAN_MODE_PERFORMANCE; > + break; > + case PLATFORM_PROFILE_CUSTOM: > + sel_prof = SMARTFAN_MODE_CUSTOM; > + break; > + default: > + return -EOPNOTSUPP; > + } > + > + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); > + > + guard(mutex)(&call_mutex); > + err = lenovo_wmidev_evaluate_method_1( > + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_SET, sel_prof, NULL); > + if (err) { > + pr_err("Error setting fan profile on WMI interface: %u\n", err); > + return err; > + } > + > + priv->current_profile = profile; > + return 0; > +} > + > +/* Driver Setup */ > +static int platform_profile_setup(struct lenovo_wmi_gz_priv *priv) > +{ > + int supported; > + int err; > + > + err = lenovo_wmi_gamezone_platform_profile_supported(&priv->pprof, > + &supported); > + if (err) { > + pr_err("Error checking platform profile support: %d\n", err); > + return err; > + } > + > + priv->platform_profile_support = supported; > + > + if (!supported) > + return -EOPNOTSUPP; > + > + priv->pprof.name = "lenovo-wmi-gamezone"; > + priv->pprof.profile_get = lenovo_wmi_gamezone_profile_get; > + priv->pprof.profile_set = lenovo_wmi_gamezone_profile_set; > + > + set_bit(PLATFORM_PROFILE_QUIET, priv->pprof.choices); > + set_bit(PLATFORM_PROFILE_BALANCED, priv->pprof.choices); > + set_bit(PLATFORM_PROFILE_PERFORMANCE, priv->pprof.choices); > + set_bit(PLATFORM_PROFILE_CUSTOM, priv->pprof.choices); > + > + err = lenovo_wmi_gamezone_profile_get(&priv->pprof, > + &priv->current_profile); > + if (err) { > + pr_err("Error getting current platform profile: %d\n", err); > + return err; > + } > + > + guard(mutex)(&call_mutex); > + err = platform_profile_register(&priv->pprof); > + if (err) { > + pr_err("Error registering platform profile: %d\n", err); > + return err; > + } > + > + return 0; > +} > + > +static int lenovo_wmi_gamezone_probe(struct wmi_device *wdev, > + const void *context) > +{ > + struct lenovo_wmi_gz_priv *priv; > + > + priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->wdev = wdev; > + return platform_profile_setup(priv); > +} > + > +static void lenovo_wmi_gamezone_remove(struct wmi_device *wdev) > +{ > + struct lenovo_wmi_gz_priv *priv = dev_get_drvdata(&wdev->dev); > + > + guard(mutex)(&call_mutex); > + platform_profile_remove(&priv->pprof); > +} > + > +static struct wmi_driver lenovo_wmi_gamezone_driver = { > + .driver = { .name = "lenovo_wmi_gamezone" }, > + .id_table = lenovo_wmi_gamezone_id_table, > + .probe = lenovo_wmi_gamezone_probe, > + .remove = lenovo_wmi_gamezone_remove, > +}; > + > +module_wmi_driver(lenovo_wmi_gamezone_driver); > + > +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_gamezone_id_table); > +MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>"); > +MODULE_DESCRIPTION("Lenovo GameZone WMI Driver"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/platform/x86/lenovo-wmi.h b/drivers/platform/x86/lenovo-wmi.h > new file mode 100644 > index 000000000000..8a302c6c47cb > --- /dev/null > +++ b/drivers/platform/x86/lenovo-wmi.h > @@ -0,0 +1,105 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later > + * > + * Lenovo Legion WMI interface driver. The Lenovo Legion WMI interface is > + * broken up into multiple GUID interfaces that require cross-references > + * between GUID's for some functionality. The "Custom Mode" interface is a > + * legacy interface for managing and displaying CPU & GPU power and hwmon > + * settings and readings. The "Other Mode" interface is a modern interface > + * that replaces or extends the "Custom Mode" interface methods. The > + * "GameZone" interface adds advanced features such as fan profiles and > + * overclocking. The "Lighting" interface adds control of various status > + * lights related to different hardware components. "Other Method" uses > + * the data structs LENOVO_CAPABILITY_DATA_00, LENOVO_CAPABILITY_DATA_01 > + * and LENOVO_CAPABILITY_DATA_02 structs for capability information. > + * > + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> > + * > + */ > + > +#define pr_fmt(fmt) "%s:%s: " fmt, KBUILD_MODNAME, __func__ > + > +#ifndef _LENOVO_WMI_H_ > +#define _LENOVO_WMI_H_ > + > +#include <linux/mutex.h> > +#include <linux/types.h> > +#include <linux/wmi.h> > + > +/* Platform Profile Modes */ > +#define SMARTFAN_MODE_QUIET 0x01 > +#define SMARTFAN_MODE_BALANCED 0x02 > +#define SMARTFAN_MODE_PERFORMANCE 0x03 > +#define SMARTFAN_MODE_CUSTOM 0xFF > + > +struct wmi_method_args { > + u32 arg0; > + u32 arg1; > +}; > + > +/* General Use functions */ > +static int lenovo_wmidev_evaluate_method(struct wmi_device *wdev, u8 instance, > + u32 method_id, struct acpi_buffer *in, > + struct acpi_buffer *out) > +{ > + acpi_status status; > + > + status = wmidev_evaluate_method(wdev, instance, method_id, in, out); > + > + if (ACPI_FAILURE(status)) > + return -EIO; > + > + return 0; > +}; You can't go and put a static function in a header. It needs to be in it's own source file. > + > +int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, > + u32 method_id, u32 arg0, u32 arg1, > + u32 *retval); > +> +int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, > + u32 method_id, u32 arg0, u32 arg1, > + u32 *retval) > +{ Likewise you can't put this here even if it's used by multiple drivers. You can leave the prototypes here, but the implementation needs to be moved to a C source file and the symbol needs to be exported from one driver and used by all the others that need it (maybe a "common" one?) > + struct wmi_method_args args = { arg0, arg1 }; > + struct acpi_buffer input = { (acpi_size)sizeof(args), &args }; > + struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL }; > + union acpi_object *ret_obj = NULL; > + int err; > + > + err = lenovo_wmidev_evaluate_method(wdev, instance, method_id, &input, > + &output); > + > + if (err) { > + pr_err("Attempt to get method value failed.\n"); > + return err; > + } > + > + if (retval) { > + ret_obj = (union acpi_object *)output.pointer; > + if (!ret_obj) { > + pr_err("Failed to get valid ACPI object from WMI interface\n"); > + return -EIO; > + } > + if (ret_obj->type != ACPI_TYPE_INTEGER) { > + pr_err("WMI query returnd ACPI object with wrong type.\n"); > + kfree(ret_obj); > + return -EIO; > + } > + *retval = (u32)ret_obj->integer.value; > + } > + > + kfree(ret_obj); Can you use __free on the acpi_object so you don't need to worry about cleanup in the error paths? > + > + return 0; > +} > + > +int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 instance, > + u32 method_id, u32 arg0, u32 *retval); > + > +int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 instance,> + u32 method_id, u32 arg0, u32 *retval) > +{ > + return lenovo_wmidev_evaluate_method_2(wdev, instance, method_id, arg0, > + 0, retval); Same comment as above about source code in the headers. > +} > + > +#endif /* !_LENOVO_WMI_H_ */
On Wed, Jan 1, 2025 at 8:09 PM Mario Limonciello <superm1@kernel.org> wrote: > > > > On 1/1/25 18:47, Derek J. Clark wrote: > > Adds lenovo-wmi-gamezone.c which provides a driver for the Lenovo > > GameZone WMI interface that comes on Lenovo "Gaming Series" hardware. > > Provides ACPI platform profiles over WMI. > > > > v2: > > - Use devm_kzalloc to ensure driver can be instanced, remove global > > reference. > > - Ensure reverse Christmas tree for all variable declarations. > > - Remove extra whitespace. > > - Use guard(mutex) in all mutex instances, global mutex. > > - Use pr_fmt instead of adding the driver name to each pr_err. > > - Remove noisy pr_info usage. > > - Rename gamezone_wmi to lenovo_wmi_gz_priv and gz_wmi to priv. > > - Remove GZ_WMI symbol exporting. > > > > Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> > > --- > > MAINTAINERS | 7 + > > drivers/platform/x86/Kconfig | 11 ++ > > drivers/platform/x86/Makefile | 1 + > > drivers/platform/x86/lenovo-wmi-gamezone.c | 203 +++++++++++++++++++++ > > drivers/platform/x86/lenovo-wmi.h | 105 +++++++++++ > > 5 files changed, 327 insertions(+) > > create mode 100644 drivers/platform/x86/lenovo-wmi-gamezone.c > > create mode 100644 drivers/platform/x86/lenovo-wmi.h > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index baf0eeb9a355..8f8a6aec6b92 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -13034,6 +13034,13 @@ S: Maintained > > W: http://legousb.sourceforge.net/ > > F: drivers/usb/misc/legousbtower.c > > > > +LENOVO WMI drivers > > +M: Derek J. Clark <derekjohn.clark@gmail.com> > > +L: platform-driver-x86@vger.kernel.org > > +S: Maintained > > +F: drivers/platform/x86/lenovo-wmi-gamezone.c > > +F: drivers/platform/x86/lenovo-wmi.h > > + > > LETSKETCH HID TABLET DRIVER > > M: Hans de Goede <hdegoede@redhat.com> > > L: linux-input@vger.kernel.org > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > > index 0258dd879d64..9a6ac7fdec9f 100644 > > --- a/drivers/platform/x86/Kconfig > > +++ b/drivers/platform/x86/Kconfig > > @@ -459,6 +459,17 @@ config IBM_RTL > > state = 0 (BIOS SMIs on) > > state = 1 (BIOS SMIs off) > > > > +config LENOVO_WMI_GAMEZONE > > + tristate "Lenovo GameZone WMI Driver" > > + depends on ACPI_WMI > > + select ACPI_PLATFORM_PROFILE > > + help > > + Say Y here if you have a WMI aware Lenovo Legion device and would like to use the > > + platform-profile firmware interface. > > + > > + To compile this driver as a module, choose M here: the module will > > + be called lenovo_wmi_gamezone. > > + > > config IDEAPAD_LAPTOP > > tristate "Lenovo IdeaPad Laptop Extras" > > depends on ACPI > > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > > index e1b142947067..7cb29a480ed2 100644 > > --- a/drivers/platform/x86/Makefile > > +++ b/drivers/platform/x86/Makefile > > @@ -68,6 +68,7 @@ obj-$(CONFIG_THINKPAD_LMI) += think-lmi.o > > obj-$(CONFIG_YOGABOOK) += lenovo-yogabook.o > > obj-$(CONFIG_YT2_1380) += lenovo-yoga-tab2-pro-1380-fastcharger.o > > obj-$(CONFIG_LENOVO_WMI_CAMERA) += lenovo-wmi-camera.o > > +obj-$(CONFIG_LENOVO_WMI_GAMEZONE) += lenovo-wmi-gamezone.o > > > > # Intel > > obj-y += intel/ > > diff --git a/drivers/platform/x86/lenovo-wmi-gamezone.c b/drivers/platform/x86/lenovo-wmi-gamezone.c > > new file mode 100644 > > index 000000000000..da5e2bc41f39 > > --- /dev/null > > +++ b/drivers/platform/x86/lenovo-wmi-gamezone.c > > @@ -0,0 +1,203 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +/* > > + * Lenovo GameZone WMI interface driver. The GameZone WMI interface provides > > + * platform profile and fan curve settings for devices that fall under the > > + * "Gaming Series" of Lenovo Legion devices. > > + * > > + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> > > + */ > > + > > +#include <linux/platform_profile.h> > > +#include "lenovo-wmi.h" > > + > > +#define LENOVO_GAMEZONE_GUID "887B54E3-DDDC-4B2C-8B88-68A26A8835D0" > > + > > +/* Method IDs */ > > +#define WMI_METHOD_ID_SMARTFAN_SUPP 43 /* IsSupportSmartFan */ > > +#define WMI_METHOD_ID_SMARTFAN_SET 44 /* SetSmartFanMode */ > > +#define WMI_METHOD_ID_SMARTFAN_GET 45 /* GetSmartFanMode */ > > + > > +static DEFINE_MUTEX(call_mutex); > > + > > +static const struct wmi_device_id lenovo_wmi_gamezone_id_table[] = { > > + { LENOVO_GAMEZONE_GUID, NULL }, /* LENOVO_GAMEZONE_DATA */ > > + {} > > +}; > > + > > +struct lenovo_wmi_gz_priv { > > + struct wmi_device *wdev; > > + enum platform_profile_option current_profile; > > + struct platform_profile_handler pprof; > > + bool platform_profile_support; > > +}; > > + > > +/* Platform Profile Methods */ > > +static int lenovo_wmi_gamezone_platform_profile_supported( > > + struct platform_profile_handler *pprof, int *supported) > > +{ > > + struct lenovo_wmi_gz_priv *priv; > > + > > + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); > > + > > + guard(mutex)(&call_mutex); > > + return lenovo_wmidev_evaluate_method_1( > > + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_SUPP, 0, supported); > > +} > > + > > +static int > > +lenovo_wmi_gamezone_profile_get(struct platform_profile_handler *pprof, > > + enum platform_profile_option *profile) > > +{ > > + struct lenovo_wmi_gz_priv *priv; > > + int sel_prof; > > + int err; > > + > > + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); > > + > > + guard(mutex)(&call_mutex); > > + err = lenovo_wmidev_evaluate_method_1( > > + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_GET, 0, &sel_prof); > > + if (err) { > > + pr_err("Error getting fan profile from WMI interface: %d\n", > > + err); > > + return err; > > + } > > + > > + switch (sel_prof) { > > + case SMARTFAN_MODE_QUIET: > > + *profile = PLATFORM_PROFILE_QUIET; > > + break; > > + case SMARTFAN_MODE_BALANCED: > > + *profile = PLATFORM_PROFILE_BALANCED; > > + break; > > + case SMARTFAN_MODE_PERFORMANCE: > > + *profile = PLATFORM_PROFILE_PERFORMANCE; > > + break; > > + case SMARTFAN_MODE_CUSTOM: > > + *profile = PLATFORM_PROFILE_CUSTOM; > > + break; > > + default: > > + return -EINVAL; > > + } > > + priv->current_profile = *profile; > > + > > + return 0; > > +} > > + > > +static int > > +lenovo_wmi_gamezone_profile_set(struct platform_profile_handler *pprof, > > + enum platform_profile_option profile) > > +{ > > + struct lenovo_wmi_gz_priv *priv; > > + int sel_prof; > > + int err; > > + > > + switch (profile) { > > + case PLATFORM_PROFILE_QUIET: > > + sel_prof = SMARTFAN_MODE_QUIET; > > + break; > > + case PLATFORM_PROFILE_BALANCED: > > + sel_prof = SMARTFAN_MODE_BALANCED; > > + break; > > + case PLATFORM_PROFILE_PERFORMANCE: > > + sel_prof = SMARTFAN_MODE_PERFORMANCE; > > + break; > > + case PLATFORM_PROFILE_CUSTOM: > > + sel_prof = SMARTFAN_MODE_CUSTOM; > > + break; > > + default: > > + return -EOPNOTSUPP; > > + } > > + > > + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); > > + > > + guard(mutex)(&call_mutex); > > + err = lenovo_wmidev_evaluate_method_1( > > + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_SET, sel_prof, NULL); > > + if (err) { > > + pr_err("Error setting fan profile on WMI interface: %u\n", err); > > + return err; > > + } > > + > > + priv->current_profile = profile; > > + return 0; > > +} > > + > > +/* Driver Setup */ > > +static int platform_profile_setup(struct lenovo_wmi_gz_priv *priv) > > +{ > > + int supported; > > + int err; > > + > > + err = lenovo_wmi_gamezone_platform_profile_supported(&priv->pprof, > > + &supported); > > + if (err) { > > + pr_err("Error checking platform profile support: %d\n", err); > > + return err; > > + } > > + > > + priv->platform_profile_support = supported; > > + > > + if (!supported) > > + return -EOPNOTSUPP; > > + > > + priv->pprof.name = "lenovo-wmi-gamezone"; > > + priv->pprof.profile_get = lenovo_wmi_gamezone_profile_get; > > + priv->pprof.profile_set = lenovo_wmi_gamezone_profile_set; > > + > > + set_bit(PLATFORM_PROFILE_QUIET, priv->pprof.choices); > > + set_bit(PLATFORM_PROFILE_BALANCED, priv->pprof.choices); > > + set_bit(PLATFORM_PROFILE_PERFORMANCE, priv->pprof.choices); > > + set_bit(PLATFORM_PROFILE_CUSTOM, priv->pprof.choices); > > + > > + err = lenovo_wmi_gamezone_profile_get(&priv->pprof, > > + &priv->current_profile); > > + if (err) { > > + pr_err("Error getting current platform profile: %d\n", err); > > + return err; > > + } > > + > > + guard(mutex)(&call_mutex); > > + err = platform_profile_register(&priv->pprof); > > + if (err) { > > + pr_err("Error registering platform profile: %d\n", err); > > + return err; > > + } > > + > > + return 0; > > +} > > + > > +static int lenovo_wmi_gamezone_probe(struct wmi_device *wdev, > > + const void *context) > > +{ > > + struct lenovo_wmi_gz_priv *priv; > > + > > + priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL); > > + if (!priv) > > + return -ENOMEM; > > + > > + priv->wdev = wdev; > > + return platform_profile_setup(priv); > > +} > > + > > +static void lenovo_wmi_gamezone_remove(struct wmi_device *wdev) > > +{ > > + struct lenovo_wmi_gz_priv *priv = dev_get_drvdata(&wdev->dev); > > + > > + guard(mutex)(&call_mutex); > > + platform_profile_remove(&priv->pprof); > > +} > > + > > +static struct wmi_driver lenovo_wmi_gamezone_driver = { > > + .driver = { .name = "lenovo_wmi_gamezone" }, > > + .id_table = lenovo_wmi_gamezone_id_table, > > + .probe = lenovo_wmi_gamezone_probe, > > + .remove = lenovo_wmi_gamezone_remove, > > +}; > > + > > +module_wmi_driver(lenovo_wmi_gamezone_driver); > > + > > +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_gamezone_id_table); > > +MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>"); > > +MODULE_DESCRIPTION("Lenovo GameZone WMI Driver"); > > +MODULE_LICENSE("GPL"); > > diff --git a/drivers/platform/x86/lenovo-wmi.h b/drivers/platform/x86/lenovo-wmi.h > > new file mode 100644 > > index 000000000000..8a302c6c47cb > > --- /dev/null > > +++ b/drivers/platform/x86/lenovo-wmi.h > > @@ -0,0 +1,105 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later > > + * > > + * Lenovo Legion WMI interface driver. The Lenovo Legion WMI interface is > > + * broken up into multiple GUID interfaces that require cross-references > > + * between GUID's for some functionality. The "Custom Mode" interface is a > > + * legacy interface for managing and displaying CPU & GPU power and hwmon > > + * settings and readings. The "Other Mode" interface is a modern interface > > + * that replaces or extends the "Custom Mode" interface methods. The > > + * "GameZone" interface adds advanced features such as fan profiles and > > + * overclocking. The "Lighting" interface adds control of various status > > + * lights related to different hardware components. "Other Method" uses > > + * the data structs LENOVO_CAPABILITY_DATA_00, LENOVO_CAPABILITY_DATA_01 > > + * and LENOVO_CAPABILITY_DATA_02 structs for capability information. > > + * > > + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> > > + * > > + */ > > + > > +#define pr_fmt(fmt) "%s:%s: " fmt, KBUILD_MODNAME, __func__ > > + > > +#ifndef _LENOVO_WMI_H_ > > +#define _LENOVO_WMI_H_ > > + > > +#include <linux/mutex.h> > > +#include <linux/types.h> > > +#include <linux/wmi.h> > > + > > +/* Platform Profile Modes */ > > +#define SMARTFAN_MODE_QUIET 0x01 > > +#define SMARTFAN_MODE_BALANCED 0x02 > > +#define SMARTFAN_MODE_PERFORMANCE 0x03 > > +#define SMARTFAN_MODE_CUSTOM 0xFF > > + > > +struct wmi_method_args { > > + u32 arg0; > > + u32 arg1; > > +}; > > + > > +/* General Use functions */ > > +static int lenovo_wmidev_evaluate_method(struct wmi_device *wdev, u8 instance, > > + u32 method_id, struct acpi_buffer *in, > > + struct acpi_buffer *out) > > +{ > > + acpi_status status; > > + > > + status = wmidev_evaluate_method(wdev, instance, method_id, in, out); > > + > > + if (ACPI_FAILURE(status)) > > + return -EIO; > > + > > + return 0; > > +}; > > You can't go and put a static function in a header. It needs to be in > it's own source file. > > > + > > +int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, > > + u32 method_id, u32 arg0, u32 arg1, > > + u32 *retval); > > +> +int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 > instance, > > + u32 method_id, u32 arg0, u32 arg1, > > + u32 *retval) > > +{ > > Likewise you can't put this here even if it's used by multiple drivers. > > You can leave the prototypes here, but the implementation needs to be > moved to a C source file and the symbol needs to be exported from one > driver and used by all the others that need it (maybe a "common" one?) > Simple fix. lenovo-wmi.c or lenovo-wmi-common.c is preferred? > > + struct wmi_method_args args = { arg0, arg1 }; > > + struct acpi_buffer input = { (acpi_size)sizeof(args), &args }; > > + struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL }; > > + union acpi_object *ret_obj = NULL; > > + int err; > > + > > + err = lenovo_wmidev_evaluate_method(wdev, instance, method_id, &input, > > + &output); > > + > > + if (err) { > > + pr_err("Attempt to get method value failed.\n"); > > + return err; > > + } > > + > > + if (retval) { > > + ret_obj = (union acpi_object *)output.pointer; > > + if (!ret_obj) { > > + pr_err("Failed to get valid ACPI object from WMI interface\n"); > > + return -EIO; > > + } > > + if (ret_obj->type != ACPI_TYPE_INTEGER) { > > + pr_err("WMI query returnd ACPI object with wrong type.\n"); > > + kfree(ret_obj); > > + return -EIO; > > + } > > + *retval = (u32)ret_obj->integer.value; > > + } > > + > > + kfree(ret_obj); > > Can you use __free on the acpi_object so you don't need to worry about > cleanup in the error paths? > Acked. > > + > > + return 0; > > +} > > + > > +int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 instance, > > + u32 method_id, u32 arg0, u32 *retval); > > + > > +int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 > instance,> + u32 method_id, u32 arg0, u32 *retval) > > +{ > > + return lenovo_wmidev_evaluate_method_2(wdev, instance, method_id, arg0, > > + 0, retval); > > Same comment as above about source code in the headers. > > > +} > > + > > +#endif /* !_LENOVO_WMI_H_ */ >
> > Simple fix. lenovo-wmi.c or lenovo-wmi-common.c is preferred? > I think let's wait and see what the discussion on 0/4 lands on. From your most recent comments I'm personally leaning it's best that "everything" is linked together as part of a single kernel object that happens to have a modalias that can let it auto-load from any one of the drivers. That would mean you can put the helpers "between" drivers of that kernel object in a -common.c and use them as needed. You can also avoid stuff like IS_REACHABLE because it all comes together as part of the kernel object. You instead would just check if bound. But let's see Armin's thoughts before you start moving things around.
Am 02.01.25 um 01:47 schrieb Derek J. Clark: > Adds lenovo-wmi-gamezone.c which provides a driver for the Lenovo > GameZone WMI interface that comes on Lenovo "Gaming Series" hardware. > Provides ACPI platform profiles over WMI. > > v2: > - Use devm_kzalloc to ensure driver can be instanced, remove global > reference. > - Ensure reverse Christmas tree for all variable declarations. > - Remove extra whitespace. > - Use guard(mutex) in all mutex instances, global mutex. > - Use pr_fmt instead of adding the driver name to each pr_err. > - Remove noisy pr_info usage. > - Rename gamezone_wmi to lenovo_wmi_gz_priv and gz_wmi to priv. > - Remove GZ_WMI symbol exporting. > > Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> > --- > MAINTAINERS | 7 + > drivers/platform/x86/Kconfig | 11 ++ > drivers/platform/x86/Makefile | 1 + > drivers/platform/x86/lenovo-wmi-gamezone.c | 203 +++++++++++++++++++++ > drivers/platform/x86/lenovo-wmi.h | 105 +++++++++++ > 5 files changed, 327 insertions(+) > create mode 100644 drivers/platform/x86/lenovo-wmi-gamezone.c > create mode 100644 drivers/platform/x86/lenovo-wmi.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index baf0eeb9a355..8f8a6aec6b92 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -13034,6 +13034,13 @@ S: Maintained > W: http://legousb.sourceforge.net/ > F: drivers/usb/misc/legousbtower.c > > +LENOVO WMI drivers > +M: Derek J. Clark <derekjohn.clark@gmail.com> > +L: platform-driver-x86@vger.kernel.org > +S: Maintained > +F: drivers/platform/x86/lenovo-wmi-gamezone.c > +F: drivers/platform/x86/lenovo-wmi.h > + > LETSKETCH HID TABLET DRIVER > M: Hans de Goede <hdegoede@redhat.com> > L: linux-input@vger.kernel.org > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 0258dd879d64..9a6ac7fdec9f 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -459,6 +459,17 @@ config IBM_RTL > state = 0 (BIOS SMIs on) > state = 1 (BIOS SMIs off) > > +config LENOVO_WMI_GAMEZONE > + tristate "Lenovo GameZone WMI Driver" > + depends on ACPI_WMI > + select ACPI_PLATFORM_PROFILE > + help > + Say Y here if you have a WMI aware Lenovo Legion device and would like to use the > + platform-profile firmware interface. > + > + To compile this driver as a module, choose M here: the module will > + be called lenovo_wmi_gamezone. Could it be that the resulting kernel module is actually named lenovo-wmi-gamezone?. If yes then please adjust the config description. > + > config IDEAPAD_LAPTOP > tristate "Lenovo IdeaPad Laptop Extras" > depends on ACPI > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > index e1b142947067..7cb29a480ed2 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -68,6 +68,7 @@ obj-$(CONFIG_THINKPAD_LMI) += think-lmi.o > obj-$(CONFIG_YOGABOOK) += lenovo-yogabook.o > obj-$(CONFIG_YT2_1380) += lenovo-yoga-tab2-pro-1380-fastcharger.o > obj-$(CONFIG_LENOVO_WMI_CAMERA) += lenovo-wmi-camera.o > +obj-$(CONFIG_LENOVO_WMI_GAMEZONE) += lenovo-wmi-gamezone.o > > # Intel > obj-y += intel/ > diff --git a/drivers/platform/x86/lenovo-wmi-gamezone.c b/drivers/platform/x86/lenovo-wmi-gamezone.c > new file mode 100644 > index 000000000000..da5e2bc41f39 > --- /dev/null > +++ b/drivers/platform/x86/lenovo-wmi-gamezone.c > @@ -0,0 +1,203 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Lenovo GameZone WMI interface driver. The GameZone WMI interface provides > + * platform profile and fan curve settings for devices that fall under the > + * "Gaming Series" of Lenovo Legion devices. > + * > + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> > + */ > + > +#include <linux/platform_profile.h> > +#include "lenovo-wmi.h" Please add the necessary includes here and do not rely on the header file to pull them in. > + > +#define LENOVO_GAMEZONE_GUID "887B54E3-DDDC-4B2C-8B88-68A26A8835D0" > + > +/* Method IDs */ > +#define WMI_METHOD_ID_SMARTFAN_SUPP 43 /* IsSupportSmartFan */ > +#define WMI_METHOD_ID_SMARTFAN_SET 44 /* SetSmartFanMode */ > +#define WMI_METHOD_ID_SMARTFAN_GET 45 /* GetSmartFanMode */ > + > +static DEFINE_MUTEX(call_mutex); > + > +static const struct wmi_device_id lenovo_wmi_gamezone_id_table[] = { > + { LENOVO_GAMEZONE_GUID, NULL }, /* LENOVO_GAMEZONE_DATA */ > + {} > +}; Please move those device IDs closer to the driver struct which uses them. > + > +struct lenovo_wmi_gz_priv { > + struct wmi_device *wdev; > + enum platform_profile_option current_profile; > + struct platform_profile_handler pprof; > + bool platform_profile_support; > +}; > + > +/* Platform Profile Methods */ > +static int lenovo_wmi_gamezone_platform_profile_supported( > + struct platform_profile_handler *pprof, int *supported) Please use ./scripts/checkpatch --strict to catch any coding style violations like this one. > +{ > + struct lenovo_wmi_gz_priv *priv; > + > + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); Is there a reason why you are not passing priv as an argument? If no then please pass priv as an argument so you can avoid having to use container_of(). > + > + guard(mutex)(&call_mutex); Is there a technical reason why you have to use a mutex for WMI method access? If no then please remove this mutex. > + return lenovo_wmidev_evaluate_method_1( > + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_SUPP, 0, supported); > +} > + > +static int > +lenovo_wmi_gamezone_profile_get(struct platform_profile_handler *pprof, > + enum platform_profile_option *profile) > +{ > + struct lenovo_wmi_gz_priv *priv; > + int sel_prof; > + int err; > + > + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); > + > + guard(mutex)(&call_mutex); > + err = lenovo_wmidev_evaluate_method_1( > + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_GET, 0, &sel_prof); > + if (err) { > + pr_err("Error getting fan profile from WMI interface: %d\n", > + err); Please just return here without printing anything, userspace does not benefit from such an error message which only states the obvious. > + return err; > + } > + > + switch (sel_prof) { > + case SMARTFAN_MODE_QUIET: > + *profile = PLATFORM_PROFILE_QUIET; > + break; > + case SMARTFAN_MODE_BALANCED: > + *profile = PLATFORM_PROFILE_BALANCED; > + break; > + case SMARTFAN_MODE_PERFORMANCE: > + *profile = PLATFORM_PROFILE_PERFORMANCE; > + break; > + case SMARTFAN_MODE_CUSTOM: > + *profile = PLATFORM_PROFILE_CUSTOM; > + break; > + default: > + return -EINVAL; > + } > + priv->current_profile = *profile; Please remove this unused variable from priv. > + > + return 0; > +} > + > +static int > +lenovo_wmi_gamezone_profile_set(struct platform_profile_handler *pprof, > + enum platform_profile_option profile) > +{ > + struct lenovo_wmi_gz_priv *priv; > + int sel_prof; > + int err; > + > + switch (profile) { > + case PLATFORM_PROFILE_QUIET: > + sel_prof = SMARTFAN_MODE_QUIET; > + break; > + case PLATFORM_PROFILE_BALANCED: > + sel_prof = SMARTFAN_MODE_BALANCED; > + break; > + case PLATFORM_PROFILE_PERFORMANCE: > + sel_prof = SMARTFAN_MODE_PERFORMANCE; > + break; > + case PLATFORM_PROFILE_CUSTOM: > + sel_prof = SMARTFAN_MODE_CUSTOM; > + break; > + default: > + return -EOPNOTSUPP; > + } > + > + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); Please assign priv during declaration. > + > + guard(mutex)(&call_mutex); > + err = lenovo_wmidev_evaluate_method_1( > + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_SET, sel_prof, NULL); > + if (err) { > + pr_err("Error setting fan profile on WMI interface: %u\n", err); Again, this error message only states the obvious, please remove it. > + return err; > + } > + > + priv->current_profile = profile; > + return 0; > +} > + > +/* Driver Setup */ > +static int platform_profile_setup(struct lenovo_wmi_gz_priv *priv) > +{ > + int supported; > + int err; > + > + err = lenovo_wmi_gamezone_platform_profile_supported(&priv->pprof, > + &supported); > + if (err) { > + pr_err("Error checking platform profile support: %d\n", err); > + return err; Please use dev_err() here. > + } > + > + priv->platform_profile_support = supported; The value of platform_profile_support is not used anywhere, please remove it. > + > + if (!supported) > + return -EOPNOTSUPP; IMHO returning -ENODEV would make more sense here. > + > + priv->pprof.name = "lenovo-wmi-gamezone"; > + priv->pprof.profile_get = lenovo_wmi_gamezone_profile_get; > + priv->pprof.profile_set = lenovo_wmi_gamezone_profile_set; > + > + set_bit(PLATFORM_PROFILE_QUIET, priv->pprof.choices); > + set_bit(PLATFORM_PROFILE_BALANCED, priv->pprof.choices); > + set_bit(PLATFORM_PROFILE_PERFORMANCE, priv->pprof.choices); > + set_bit(PLATFORM_PROFILE_CUSTOM, priv->pprof.choices); > + > + err = lenovo_wmi_gamezone_profile_get(&priv->pprof, > + &priv->current_profile); > + if (err) { > + pr_err("Error getting current platform profile: %d\n", err); > + return err; > + } Is there a technical reason for reading the current platform profile during device initialization(? If no then please remove this call. > + > + guard(mutex)(&call_mutex); > + err = platform_profile_register(&priv->pprof); Using devm_platform_profile_register() would make sense here. This function was added very recently so you have to base your patch series onto the for-next branch. > + if (err) { > + pr_err("Error registering platform profile: %d\n", err); > + return err; > + } > + > + return 0; > +} > + > +static int lenovo_wmi_gamezone_probe(struct wmi_device *wdev, > + const void *context) > +{ > + struct lenovo_wmi_gz_priv *priv; > + > + priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->wdev = wdev; Since you are using dev_get_drvdata(), you also need to use dev_set_drvdata() here, otherwise dev_get_drvdata() will return no valid value. > + return platform_profile_setup(priv); > +} > + > +static void lenovo_wmi_gamezone_remove(struct wmi_device *wdev) > +{ > + struct lenovo_wmi_gz_priv *priv = dev_get_drvdata(&wdev->dev); > + > + guard(mutex)(&call_mutex); > + platform_profile_remove(&priv->pprof); > +} > + > +static struct wmi_driver lenovo_wmi_gamezone_driver = { > + .driver = { .name = "lenovo_wmi_gamezone" }, Please set ".probe_type = PROBE_PREFER_ASYNCHRONOUS" here. Also does the selected fan profile remain the same after suspending or hibernating? If no then please add the necessary PM callbacks to save/restore the fan profile before suspend/after resume if necessary. > + .id_table = lenovo_wmi_gamezone_id_table, > + .probe = lenovo_wmi_gamezone_probe, > + .remove = lenovo_wmi_gamezone_remove, Please set ".no_singleton = true" here. > +}; > + > +module_wmi_driver(lenovo_wmi_gamezone_driver); > + > +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_gamezone_id_table); > +MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>"); > +MODULE_DESCRIPTION("Lenovo GameZone WMI Driver"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/platform/x86/lenovo-wmi.h b/drivers/platform/x86/lenovo-wmi.h > new file mode 100644 > index 000000000000..8a302c6c47cb > --- /dev/null > +++ b/drivers/platform/x86/lenovo-wmi.h > @@ -0,0 +1,105 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later > + * > + * Lenovo Legion WMI interface driver. The Lenovo Legion WMI interface is > + * broken up into multiple GUID interfaces that require cross-references > + * between GUID's for some functionality. The "Custom Mode" interface is a > + * legacy interface for managing and displaying CPU & GPU power and hwmon > + * settings and readings. The "Other Mode" interface is a modern interface > + * that replaces or extends the "Custom Mode" interface methods. The > + * "GameZone" interface adds advanced features such as fan profiles and > + * overclocking. The "Lighting" interface adds control of various status > + * lights related to different hardware components. "Other Method" uses > + * the data structs LENOVO_CAPABILITY_DATA_00, LENOVO_CAPABILITY_DATA_01 > + * and LENOVO_CAPABILITY_DATA_02 structs for capability information. > + * > + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> > + * > + */ > + > +#define pr_fmt(fmt) "%s:%s: " fmt, KBUILD_MODNAME, __func__ > + > +#ifndef _LENOVO_WMI_H_ > +#define _LENOVO_WMI_H_ > + > +#include <linux/mutex.h> > +#include <linux/types.h> > +#include <linux/wmi.h> > + > +/* Platform Profile Modes */ > +#define SMARTFAN_MODE_QUIET 0x01 > +#define SMARTFAN_MODE_BALANCED 0x02 > +#define SMARTFAN_MODE_PERFORMANCE 0x03 > +#define SMARTFAN_MODE_CUSTOM 0xFF > + > +struct wmi_method_args { > + u32 arg0; > + u32 arg1; > +}; > + > +/* General Use functions */ > +static int lenovo_wmidev_evaluate_method(struct wmi_device *wdev, u8 instance, > + u32 method_id, struct acpi_buffer *in, > + struct acpi_buffer *out) > +{ > + acpi_status status; > + > + status = wmidev_evaluate_method(wdev, instance, method_id, in, out); > + > + if (ACPI_FAILURE(status)) > + return -EIO; > + > + return 0; > +}; > + > +int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, > + u32 method_id, u32 arg0, u32 arg1, > + u32 *retval); > + > +int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, > + u32 method_id, u32 arg0, u32 arg1, > + u32 *retval) > +{ > + struct wmi_method_args args = { arg0, arg1 }; > + struct acpi_buffer input = { (acpi_size)sizeof(args), &args }; > + struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL }; > + union acpi_object *ret_obj = NULL; > + int err; Please order the variable declarations in reverse XMAS tree order. > + > + err = lenovo_wmidev_evaluate_method(wdev, instance, method_id, &input, > + &output); > + > + if (err) { > + pr_err("Attempt to get method value failed.\n"); Please remove any error messages in this part of the code, printing error messages should ideally happen at the higher layers of the driver if necessary. > + return err; > + } > + > + if (retval) { > + ret_obj = (union acpi_object *)output.pointer; > + if (!ret_obj) { > + pr_err("Failed to get valid ACPI object from WMI interface\n"); > + return -EIO; -ENODATA. > + } > + if (ret_obj->type != ACPI_TYPE_INTEGER) { > + pr_err("WMI query returnd ACPI object with wrong type.\n"); > + kfree(ret_obj); > + return -EIO; -ENXIO. > + } > + *retval = (u32)ret_obj->integer.value; > + } > + > + kfree(ret_obj); > + > + return 0; > +} > + > +int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 instance, > + u32 method_id, u32 arg0, u32 *retval); > + > +int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 instance, > + u32 method_id, u32 arg0, u32 *retval) > +{ > + return lenovo_wmidev_evaluate_method_2(wdev, instance, method_id, arg0, > + 0, retval); > +} > + > +#endif /* !_LENOVO_WMI_H_ */
On Wed, 1 Jan 2025, Derek J. Clark wrote: > Adds lenovo-wmi-gamezone.c which provides a driver for the Lenovo > GameZone WMI interface that comes on Lenovo "Gaming Series" hardware. > Provides ACPI platform profiles over WMI. > > v2: > - Use devm_kzalloc to ensure driver can be instanced, remove global > reference. > - Ensure reverse Christmas tree for all variable declarations. > - Remove extra whitespace. > - Use guard(mutex) in all mutex instances, global mutex. > - Use pr_fmt instead of adding the driver name to each pr_err. > - Remove noisy pr_info usage. > - Rename gamezone_wmi to lenovo_wmi_gz_priv and gz_wmi to priv. > - Remove GZ_WMI symbol exporting. > > Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> > --- > MAINTAINERS | 7 + > drivers/platform/x86/Kconfig | 11 ++ > drivers/platform/x86/Makefile | 1 + > drivers/platform/x86/lenovo-wmi-gamezone.c | 203 +++++++++++++++++++++ > drivers/platform/x86/lenovo-wmi.h | 105 +++++++++++ > 5 files changed, 327 insertions(+) > create mode 100644 drivers/platform/x86/lenovo-wmi-gamezone.c > create mode 100644 drivers/platform/x86/lenovo-wmi.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index baf0eeb9a355..8f8a6aec6b92 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -13034,6 +13034,13 @@ S: Maintained > W: http://legousb.sourceforge.net/ > F: drivers/usb/misc/legousbtower.c > > +LENOVO WMI drivers > +M: Derek J. Clark <derekjohn.clark@gmail.com> > +L: platform-driver-x86@vger.kernel.org > +S: Maintained > +F: drivers/platform/x86/lenovo-wmi-gamezone.c > +F: drivers/platform/x86/lenovo-wmi.h > + > LETSKETCH HID TABLET DRIVER > M: Hans de Goede <hdegoede@redhat.com> > L: linux-input@vger.kernel.org > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 0258dd879d64..9a6ac7fdec9f 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -459,6 +459,17 @@ config IBM_RTL > state = 0 (BIOS SMIs on) > state = 1 (BIOS SMIs off) > > +config LENOVO_WMI_GAMEZONE > + tristate "Lenovo GameZone WMI Driver" > + depends on ACPI_WMI > + select ACPI_PLATFORM_PROFILE > + help > + Say Y here if you have a WMI aware Lenovo Legion device and would like to use the > + platform-profile firmware interface. > + > + To compile this driver as a module, choose M here: the module will > + be called lenovo_wmi_gamezone. > + > config IDEAPAD_LAPTOP > tristate "Lenovo IdeaPad Laptop Extras" > depends on ACPI > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > index e1b142947067..7cb29a480ed2 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -68,6 +68,7 @@ obj-$(CONFIG_THINKPAD_LMI) += think-lmi.o > obj-$(CONFIG_YOGABOOK) += lenovo-yogabook.o > obj-$(CONFIG_YT2_1380) += lenovo-yoga-tab2-pro-1380-fastcharger.o > obj-$(CONFIG_LENOVO_WMI_CAMERA) += lenovo-wmi-camera.o > +obj-$(CONFIG_LENOVO_WMI_GAMEZONE) += lenovo-wmi-gamezone.o > > # Intel > obj-y += intel/ > diff --git a/drivers/platform/x86/lenovo-wmi-gamezone.c b/drivers/platform/x86/lenovo-wmi-gamezone.c > new file mode 100644 > index 000000000000..da5e2bc41f39 > --- /dev/null > +++ b/drivers/platform/x86/lenovo-wmi-gamezone.c > @@ -0,0 +1,203 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Lenovo GameZone WMI interface driver. The GameZone WMI interface provides > + * platform profile and fan curve settings for devices that fall under the > + * "Gaming Series" of Lenovo Legion devices. > + * > + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> > + */ > + > +#include <linux/platform_profile.h> > +#include "lenovo-wmi.h" > + > +#define LENOVO_GAMEZONE_GUID "887B54E3-DDDC-4B2C-8B88-68A26A8835D0" > + > +/* Method IDs */ > +#define WMI_METHOD_ID_SMARTFAN_SUPP 43 /* IsSupportSmartFan */ > +#define WMI_METHOD_ID_SMARTFAN_SET 44 /* SetSmartFanMode */ > +#define WMI_METHOD_ID_SMARTFAN_GET 45 /* GetSmartFanMode */ > + > +static DEFINE_MUTEX(call_mutex); > + > +static const struct wmi_device_id lenovo_wmi_gamezone_id_table[] = { > + { LENOVO_GAMEZONE_GUID, NULL }, /* LENOVO_GAMEZONE_DATA */ > + {} > +}; > + > +struct lenovo_wmi_gz_priv { > + struct wmi_device *wdev; > + enum platform_profile_option current_profile; > + struct platform_profile_handler pprof; > + bool platform_profile_support; > +}; > + > +/* Platform Profile Methods */ > +static int lenovo_wmi_gamezone_platform_profile_supported( > + struct platform_profile_handler *pprof, int *supported) > +{ > + struct lenovo_wmi_gz_priv *priv; > + > + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); > + > + guard(mutex)(&call_mutex); > + return lenovo_wmidev_evaluate_method_1( > + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_SUPP, 0, supported); > +} > + > +static int > +lenovo_wmi_gamezone_profile_get(struct platform_profile_handler *pprof, > + enum platform_profile_option *profile) > +{ > + struct lenovo_wmi_gz_priv *priv; > + int sel_prof; > + int err; > + > + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); > + > + guard(mutex)(&call_mutex); > + err = lenovo_wmidev_evaluate_method_1( > + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_GET, 0, &sel_prof); > + if (err) { > + pr_err("Error getting fan profile from WMI interface: %d\n", > + err); > + return err; > + } > + > + switch (sel_prof) { > + case SMARTFAN_MODE_QUIET: > + *profile = PLATFORM_PROFILE_QUIET; > + break; > + case SMARTFAN_MODE_BALANCED: > + *profile = PLATFORM_PROFILE_BALANCED; > + break; > + case SMARTFAN_MODE_PERFORMANCE: > + *profile = PLATFORM_PROFILE_PERFORMANCE; > + break; > + case SMARTFAN_MODE_CUSTOM: > + *profile = PLATFORM_PROFILE_CUSTOM; > + break; > + default: > + return -EINVAL; > + } > + priv->current_profile = *profile; > + > + return 0; > +} > + > +static int > +lenovo_wmi_gamezone_profile_set(struct platform_profile_handler *pprof, > + enum platform_profile_option profile) > +{ > + struct lenovo_wmi_gz_priv *priv; > + int sel_prof; > + int err; > + > + switch (profile) { > + case PLATFORM_PROFILE_QUIET: > + sel_prof = SMARTFAN_MODE_QUIET; > + break; > + case PLATFORM_PROFILE_BALANCED: > + sel_prof = SMARTFAN_MODE_BALANCED; > + break; > + case PLATFORM_PROFILE_PERFORMANCE: > + sel_prof = SMARTFAN_MODE_PERFORMANCE; > + break; > + case PLATFORM_PROFILE_CUSTOM: > + sel_prof = SMARTFAN_MODE_CUSTOM; > + break; > + default: > + return -EOPNOTSUPP; > + } > + > + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); > + > + guard(mutex)(&call_mutex); > + err = lenovo_wmidev_evaluate_method_1( > + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_SET, sel_prof, NULL); > + if (err) { > + pr_err("Error setting fan profile on WMI interface: %u\n", err); > + return err; > + } > + > + priv->current_profile = profile; > + return 0; > +} > + > +/* Driver Setup */ > +static int platform_profile_setup(struct lenovo_wmi_gz_priv *priv) > +{ > + int supported; > + int err; > + > + err = lenovo_wmi_gamezone_platform_profile_supported(&priv->pprof, > + &supported); > + if (err) { > + pr_err("Error checking platform profile support: %d\n", err); > + return err; > + } > + > + priv->platform_profile_support = supported; > + > + if (!supported) > + return -EOPNOTSUPP; > + > + priv->pprof.name = "lenovo-wmi-gamezone"; > + priv->pprof.profile_get = lenovo_wmi_gamezone_profile_get; > + priv->pprof.profile_set = lenovo_wmi_gamezone_profile_set; > + > + set_bit(PLATFORM_PROFILE_QUIET, priv->pprof.choices); > + set_bit(PLATFORM_PROFILE_BALANCED, priv->pprof.choices); > + set_bit(PLATFORM_PROFILE_PERFORMANCE, priv->pprof.choices); > + set_bit(PLATFORM_PROFILE_CUSTOM, priv->pprof.choices); > + > + err = lenovo_wmi_gamezone_profile_get(&priv->pprof, > + &priv->current_profile); > + if (err) { > + pr_err("Error getting current platform profile: %d\n", err); > + return err; > + } > + > + guard(mutex)(&call_mutex); > + err = platform_profile_register(&priv->pprof); > + if (err) { > + pr_err("Error registering platform profile: %d\n", err); > + return err; > + } > + > + return 0; > +} > + > +static int lenovo_wmi_gamezone_probe(struct wmi_device *wdev, > + const void *context) > +{ > + struct lenovo_wmi_gz_priv *priv; > + > + priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->wdev = wdev; > + return platform_profile_setup(priv); > +} > + > +static void lenovo_wmi_gamezone_remove(struct wmi_device *wdev) > +{ > + struct lenovo_wmi_gz_priv *priv = dev_get_drvdata(&wdev->dev); > + > + guard(mutex)(&call_mutex); > + platform_profile_remove(&priv->pprof); > +} > + > +static struct wmi_driver lenovo_wmi_gamezone_driver = { > + .driver = { .name = "lenovo_wmi_gamezone" }, > + .id_table = lenovo_wmi_gamezone_id_table, > + .probe = lenovo_wmi_gamezone_probe, > + .remove = lenovo_wmi_gamezone_remove, > +}; > + > +module_wmi_driver(lenovo_wmi_gamezone_driver); > + > +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_gamezone_id_table); > +MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>"); > +MODULE_DESCRIPTION("Lenovo GameZone WMI Driver"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/platform/x86/lenovo-wmi.h b/drivers/platform/x86/lenovo-wmi.h > new file mode 100644 > index 000000000000..8a302c6c47cb > --- /dev/null > +++ b/drivers/platform/x86/lenovo-wmi.h > @@ -0,0 +1,105 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later > + * > + * Lenovo Legion WMI interface driver. The Lenovo Legion WMI interface is > + * broken up into multiple GUID interfaces that require cross-references > + * between GUID's for some functionality. The "Custom Mode" interface is a > + * legacy interface for managing and displaying CPU & GPU power and hwmon > + * settings and readings. The "Other Mode" interface is a modern interface > + * that replaces or extends the "Custom Mode" interface methods. The > + * "GameZone" interface adds advanced features such as fan profiles and > + * overclocking. The "Lighting" interface adds control of various status > + * lights related to different hardware components. "Other Method" uses > + * the data structs LENOVO_CAPABILITY_DATA_00, LENOVO_CAPABILITY_DATA_01 > + * and LENOVO_CAPABILITY_DATA_02 structs for capability information. > + * > + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> > + * > + */ > + > +#define pr_fmt(fmt) "%s:%s: " fmt, KBUILD_MODNAME, __func__ Don't include __func__ into pr_fmt(). Including __func__ into any >dbg level message is not helpful to user, the error/warning/info should be written in plain English, not in terms of code/function names. The usual pr_fmt() boilerplate is this: #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
On Thu, Jan 9, 2025 at 2:12 PM Armin Wolf <W_Armin@gmx.de> wrote: > > Am 02.01.25 um 01:47 schrieb Derek J. Clark: > > > Adds lenovo-wmi-gamezone.c which provides a driver for the Lenovo > > GameZone WMI interface that comes on Lenovo "Gaming Series" hardware. > > Provides ACPI platform profiles over WMI. > > > > v2: > > - Use devm_kzalloc to ensure driver can be instanced, remove global > > reference. > > - Ensure reverse Christmas tree for all variable declarations. > > - Remove extra whitespace. > > - Use guard(mutex) in all mutex instances, global mutex. > > - Use pr_fmt instead of adding the driver name to each pr_err. > > - Remove noisy pr_info usage. > > - Rename gamezone_wmi to lenovo_wmi_gz_priv and gz_wmi to priv. > > - Remove GZ_WMI symbol exporting. > > > > Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> > > --- > > MAINTAINERS | 7 + > > drivers/platform/x86/Kconfig | 11 ++ > > drivers/platform/x86/Makefile | 1 + > > drivers/platform/x86/lenovo-wmi-gamezone.c | 203 +++++++++++++++++++++ > > drivers/platform/x86/lenovo-wmi.h | 105 +++++++++++ > > 5 files changed, 327 insertions(+) > > create mode 100644 drivers/platform/x86/lenovo-wmi-gamezone.c > > create mode 100644 drivers/platform/x86/lenovo-wmi.h > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index baf0eeb9a355..8f8a6aec6b92 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -13034,6 +13034,13 @@ S: Maintained > > W: http://legousb.sourceforge.net/ > > F: drivers/usb/misc/legousbtower.c > > > > +LENOVO WMI drivers > > +M: Derek J. Clark <derekjohn.clark@gmail.com> > > +L: platform-driver-x86@vger.kernel.org > > +S: Maintained > > +F: drivers/platform/x86/lenovo-wmi-gamezone.c > > +F: drivers/platform/x86/lenovo-wmi.h > > + > > LETSKETCH HID TABLET DRIVER > > M: Hans de Goede <hdegoede@redhat.com> > > L: linux-input@vger.kernel.org > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > > index 0258dd879d64..9a6ac7fdec9f 100644 > > --- a/drivers/platform/x86/Kconfig > > +++ b/drivers/platform/x86/Kconfig > > @@ -459,6 +459,17 @@ config IBM_RTL > > state = 0 (BIOS SMIs on) > > state = 1 (BIOS SMIs off) > > > > +config LENOVO_WMI_GAMEZONE > > + tristate "Lenovo GameZone WMI Driver" > > + depends on ACPI_WMI > > + select ACPI_PLATFORM_PROFILE > > + help > > + Say Y here if you have a WMI aware Lenovo Legion device and would like to use the > > + platform-profile firmware interface. > > + > > + To compile this driver as a module, choose M here: the module will > > + be called lenovo_wmi_gamezone. > > Could it be that the resulting kernel module is actually named lenovo-wmi-gamezone?. > If yes then please adjust the config description. > the .o/.ko are named as you described with -, but lsmod lists them with _ which is how most would interact with the driver if manually loading or blocking it. I'll put whichever you think is most appropriate. > > + > > config IDEAPAD_LAPTOP > > tristate "Lenovo IdeaPad Laptop Extras" > > depends on ACPI > > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > > index e1b142947067..7cb29a480ed2 100644 > > --- a/drivers/platform/x86/Makefile > > +++ b/drivers/platform/x86/Makefile > > @@ -68,6 +68,7 @@ obj-$(CONFIG_THINKPAD_LMI) += think-lmi.o > > obj-$(CONFIG_YOGABOOK) += lenovo-yogabook.o > > obj-$(CONFIG_YT2_1380) += lenovo-yoga-tab2-pro-1380-fastcharger.o > > obj-$(CONFIG_LENOVO_WMI_CAMERA) += lenovo-wmi-camera.o > > +obj-$(CONFIG_LENOVO_WMI_GAMEZONE) += lenovo-wmi-gamezone.o > > > > # Intel > > obj-y += intel/ > > diff --git a/drivers/platform/x86/lenovo-wmi-gamezone.c b/drivers/platform/x86/lenovo-wmi-gamezone.c > > new file mode 100644 > > index 000000000000..da5e2bc41f39 > > --- /dev/null > > +++ b/drivers/platform/x86/lenovo-wmi-gamezone.c > > @@ -0,0 +1,203 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +/* > > + * Lenovo GameZone WMI interface driver. The GameZone WMI interface provides > > + * platform profile and fan curve settings for devices that fall under the > > + * "Gaming Series" of Lenovo Legion devices. > > + * > > + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> > > + */ > > + > > +#include <linux/platform_profile.h> > > +#include "lenovo-wmi.h" > > Please add the necessary includes here and do not rely on the header file to pull them in. > Ack > > + > > +#define LENOVO_GAMEZONE_GUID "887B54E3-DDDC-4B2C-8B88-68A26A8835D0" > > + > > +/* Method IDs */ > > +#define WMI_METHOD_ID_SMARTFAN_SUPP 43 /* IsSupportSmartFan */ > > +#define WMI_METHOD_ID_SMARTFAN_SET 44 /* SetSmartFanMode */ > > +#define WMI_METHOD_ID_SMARTFAN_GET 45 /* GetSmartFanMode */ > > + > > +static DEFINE_MUTEX(call_mutex); > > + > > +static const struct wmi_device_id lenovo_wmi_gamezone_id_table[] = { > > + { LENOVO_GAMEZONE_GUID, NULL }, /* LENOVO_GAMEZONE_DATA */ > > + {} > > +}; > > Please move those device IDs closer to the driver struct which uses them. > Ack > > + > > +struct lenovo_wmi_gz_priv { > > + struct wmi_device *wdev; > > + enum platform_profile_option current_profile; > > + struct platform_profile_handler pprof; > > + bool platform_profile_support; > > +}; > > + > > +/* Platform Profile Methods */ > > +static int lenovo_wmi_gamezone_platform_profile_supported( > > + struct platform_profile_handler *pprof, int *supported) > > Please use ./scripts/checkpatch --strict to catch any coding style violations like this one. > Ack. Sorry about that. > > +{ > > + struct lenovo_wmi_gz_priv *priv; > > + > > + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); > > Is there a reason why you are not passing priv as an argument? If no then please pass priv > as an argument so you can avoid having to use container_of(). > > > + > > + guard(mutex)(&call_mutex); > > Is there a technical reason why you have to use a mutex for WMI method access? If no then please remove > this mutex. > We weren't sure and figured you would know best practice. I'll remove them. > > + return lenovo_wmidev_evaluate_method_1( > > + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_SUPP, 0, supported); > > +} > > + > > +static int > > +lenovo_wmi_gamezone_profile_get(struct platform_profile_handler *pprof, > > + enum platform_profile_option *profile) > > +{ > > + struct lenovo_wmi_gz_priv *priv; > > + int sel_prof; > > + int err; > > + > > + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); > > + > > + guard(mutex)(&call_mutex); > > + err = lenovo_wmidev_evaluate_method_1( > > + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_GET, 0, &sel_prof); > > + if (err) { > > + pr_err("Error getting fan profile from WMI interface: %d\n", > > + err); > > Please just return here without printing anything, userspace does not benefit from such > an error message which only states the obvious. > Ack all debug return messages. > > + return err; > > + } > > + > > + switch (sel_prof) { > > + case SMARTFAN_MODE_QUIET: > > + *profile = PLATFORM_PROFILE_QUIET; > > + break; > > + case SMARTFAN_MODE_BALANCED: > > + *profile = PLATFORM_PROFILE_BALANCED; > > + break; > > + case SMARTFAN_MODE_PERFORMANCE: > > + *profile = PLATFORM_PROFILE_PERFORMANCE; > > + break; > > + case SMARTFAN_MODE_CUSTOM: > > + *profile = PLATFORM_PROFILE_CUSTOM; > > + break; > > + default: > > + return -EINVAL; > > + } > > + priv->current_profile = *profile; > > Please remove this unused variable from priv. > Ack > > + > > + return 0; > > +} > > + > > +static int > > +lenovo_wmi_gamezone_profile_set(struct platform_profile_handler *pprof, > > + enum platform_profile_option profile) > > +{ > > + struct lenovo_wmi_gz_priv *priv; > > + int sel_prof; > > + int err; > > + > > + switch (profile) { > > + case PLATFORM_PROFILE_QUIET: > > + sel_prof = SMARTFAN_MODE_QUIET; > > + break; > > + case PLATFORM_PROFILE_BALANCED: > > + sel_prof = SMARTFAN_MODE_BALANCED; > > + break; > > + case PLATFORM_PROFILE_PERFORMANCE: > > + sel_prof = SMARTFAN_MODE_PERFORMANCE; > > + break; > > + case PLATFORM_PROFILE_CUSTOM: > > + sel_prof = SMARTFAN_MODE_CUSTOM; > > + break; > > + default: > > + return -EOPNOTSUPP; > > + } > > + > > + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); > > Please assign priv during declaration. > Ack > > + > > + guard(mutex)(&call_mutex); > > + err = lenovo_wmidev_evaluate_method_1( > > + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_SET, sel_prof, NULL); > > + if (err) { > > + pr_err("Error setting fan profile on WMI interface: %u\n", err); > > Again, this error message only states the obvious, please remove it. > > > + return err; > > + } > > + > > + priv->current_profile = profile; > > + return 0; > > +} > > + > > +/* Driver Setup */ > > +static int platform_profile_setup(struct lenovo_wmi_gz_priv *priv) > > +{ > > + int supported; > > + int err; > > + > > + err = lenovo_wmi_gamezone_platform_profile_supported(&priv->pprof, > > + &supported); > > + if (err) { > > + pr_err("Error checking platform profile support: %d\n", err); > > + return err; > > Please use dev_err() here. Ack > > + } > > + > > + priv->platform_profile_support = supported; > > The value of platform_profile_support is not used anywhere, please remove it. > > > + > > + if (!supported) > > + return -EOPNOTSUPP; > > IMHO returning -ENODEV would make more sense here. > Ack > > + > > + priv->pprof.name = "lenovo-wmi-gamezone"; > > + priv->pprof.profile_get = lenovo_wmi_gamezone_profile_get; > > + priv->pprof.profile_set = lenovo_wmi_gamezone_profile_set; > > + > > + set_bit(PLATFORM_PROFILE_QUIET, priv->pprof.choices); > > + set_bit(PLATFORM_PROFILE_BALANCED, priv->pprof.choices); > > + set_bit(PLATFORM_PROFILE_PERFORMANCE, priv->pprof.choices); > > + set_bit(PLATFORM_PROFILE_CUSTOM, priv->pprof.choices); > > + > > + err = lenovo_wmi_gamezone_profile_get(&priv->pprof, > > + &priv->current_profile); > > + if (err) { > > + pr_err("Error getting current platform profile: %d\n", err); > > + return err; > > + } > > Is there a technical reason for reading the current platform profile during device > initialization(? If no then please remove this call. > There isn't, just a misconception on my part. WIll remove. > > + > > + guard(mutex)(&call_mutex); > > + err = platform_profile_register(&priv->pprof); > > Using devm_platform_profile_register() would make sense here. This function was added very recently > so you have to base your patch series onto the for-next branch. > Ack > > + if (err) { > > + pr_err("Error registering platform profile: %d\n", err); > > + return err; > > + } > > + > > + return 0; > > +} > > + > > +static int lenovo_wmi_gamezone_probe(struct wmi_device *wdev, > > + const void *context) > > +{ > > + struct lenovo_wmi_gz_priv *priv; > > + > > + priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL); > > + if (!priv) > > + return -ENOMEM; > > + > > + priv->wdev = wdev; > > Since you are using dev_get_drvdata(), you also need to use dev_set_drvdata() here, otherwise > dev_get_drvdata() will return no valid value. > Ack > > + return platform_profile_setup(priv); > > +} > > + > > +static void lenovo_wmi_gamezone_remove(struct wmi_device *wdev) > > +{ > > + struct lenovo_wmi_gz_priv *priv = dev_get_drvdata(&wdev->dev); > > + > > + guard(mutex)(&call_mutex); > > + platform_profile_remove(&priv->pprof); > > +} > > + > > +static struct wmi_driver lenovo_wmi_gamezone_driver = { > > + .driver = { .name = "lenovo_wmi_gamezone" }, > > Please set ".probe_type = PROBE_PREFER_ASYNCHRONOUS" here. > Ack > Also does the selected fan profile remain the same after suspending or hibernating? > If no then please add the necessary PM callbacks to save/restore the fan profile > before suspend/after resume if necessary. > It remains the same after suspend, hibernate, reboot, and shutdown. > > + .id_table = lenovo_wmi_gamezone_id_table, > > + .probe = lenovo_wmi_gamezone_probe, > > + .remove = lenovo_wmi_gamezone_remove, > > Please set ".no_singleton = true" here. > Ack > > +}; > > + > > +module_wmi_driver(lenovo_wmi_gamezone_driver); > > + > > +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_gamezone_id_table); > > +MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>"); > > +MODULE_DESCRIPTION("Lenovo GameZone WMI Driver"); > > +MODULE_LICENSE("GPL"); > > diff --git a/drivers/platform/x86/lenovo-wmi.h b/drivers/platform/x86/lenovo-wmi.h > > new file mode 100644 > > index 000000000000..8a302c6c47cb > > --- /dev/null > > +++ b/drivers/platform/x86/lenovo-wmi.h > > @@ -0,0 +1,105 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later > > + * > > + * Lenovo Legion WMI interface driver. The Lenovo Legion WMI interface is > > + * broken up into multiple GUID interfaces that require cross-references > > + * between GUID's for some functionality. The "Custom Mode" interface is a > > + * legacy interface for managing and displaying CPU & GPU power and hwmon > > + * settings and readings. The "Other Mode" interface is a modern interface > > + * that replaces or extends the "Custom Mode" interface methods. The > > + * "GameZone" interface adds advanced features such as fan profiles and > > + * overclocking. The "Lighting" interface adds control of various status > > + * lights related to different hardware components. "Other Method" uses > > + * the data structs LENOVO_CAPABILITY_DATA_00, LENOVO_CAPABILITY_DATA_01 > > + * and LENOVO_CAPABILITY_DATA_02 structs for capability information. > > + * > > + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> > > + * > > + */ > > + > > +#define pr_fmt(fmt) "%s:%s: " fmt, KBUILD_MODNAME, __func__ > > + > > +#ifndef _LENOVO_WMI_H_ > > +#define _LENOVO_WMI_H_ > > + > > +#include <linux/mutex.h> > > +#include <linux/types.h> > > +#include <linux/wmi.h> > > + > > +/* Platform Profile Modes */ > > +#define SMARTFAN_MODE_QUIET 0x01 > > +#define SMARTFAN_MODE_BALANCED 0x02 > > +#define SMARTFAN_MODE_PERFORMANCE 0x03 > > +#define SMARTFAN_MODE_CUSTOM 0xFF > > + > > +struct wmi_method_args { > > + u32 arg0; > > + u32 arg1; > > +}; > > + > > +/* General Use functions */ > > +static int lenovo_wmidev_evaluate_method(struct wmi_device *wdev, u8 instance, > > + u32 method_id, struct acpi_buffer *in, > > + struct acpi_buffer *out) > > +{ > > + acpi_status status; > > + > > + status = wmidev_evaluate_method(wdev, instance, method_id, in, out); > > + > > + if (ACPI_FAILURE(status)) > > + return -EIO; > > + > > + return 0; > > +}; > > + > > +int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, > > + u32 method_id, u32 arg0, u32 arg1, > > + u32 *retval); > > + > > +int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, > > + u32 method_id, u32 arg0, u32 arg1, > > + u32 *retval) > > +{ > > + struct wmi_method_args args = { arg0, arg1 }; > > + struct acpi_buffer input = { (acpi_size)sizeof(args), &args }; > > + struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL }; > > + union acpi_object *ret_obj = NULL; > > + int err; > > Please order the variable declarations in reverse XMAS tree order. > Ack > > + > > + err = lenovo_wmidev_evaluate_method(wdev, instance, method_id, &input, > > + &output); > > + > > + if (err) { > > + pr_err("Attempt to get method value failed.\n"); > > Please remove any error messages in this part of the code, printing error messages should > ideally happen at the higher layers of the driver if necessary. > > > + return err; > > + } > > + > > + if (retval) { > > + ret_obj = (union acpi_object *)output.pointer; > > + if (!ret_obj) { > > + pr_err("Failed to get valid ACPI object from WMI interface\n"); > > + return -EIO; > > -ENODATA. > Ack > > + } > > + if (ret_obj->type != ACPI_TYPE_INTEGER) { > > + pr_err("WMI query returnd ACPI object with wrong type.\n"); > > + kfree(ret_obj); > > + return -EIO; > > -ENXIO. > Ack > > + } > > + *retval = (u32)ret_obj->integer.value; > > + } > > + > > + kfree(ret_obj); > > + > > + return 0; > > +} > > + > > +int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 instance, > > + u32 method_id, u32 arg0, u32 *retval); > > + > > +int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 instance, > > + u32 method_id, u32 arg0, u32 *retval) > > +{ > > + return lenovo_wmidev_evaluate_method_2(wdev, instance, method_id, arg0, > > + 0, retval); > > +} > > + > > +#endif /* !_LENOVO_WMI_H_ */ Thanks Armin, Derek
On Fri, Jan 10, 2025 at 4:27 AM Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote: > > On Wed, 1 Jan 2025, Derek J. Clark wrote: > > > Adds lenovo-wmi-gamezone.c which provides a driver for the Lenovo > > GameZone WMI interface that comes on Lenovo "Gaming Series" hardware. > > Provides ACPI platform profiles over WMI. > > > > v2: > > - Use devm_kzalloc to ensure driver can be instanced, remove global > > reference. > > - Ensure reverse Christmas tree for all variable declarations. > > - Remove extra whitespace. > > - Use guard(mutex) in all mutex instances, global mutex. > > - Use pr_fmt instead of adding the driver name to each pr_err. > > - Remove noisy pr_info usage. > > - Rename gamezone_wmi to lenovo_wmi_gz_priv and gz_wmi to priv. > > - Remove GZ_WMI symbol exporting. > > > > Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> > > --- > > MAINTAINERS | 7 + > > drivers/platform/x86/Kconfig | 11 ++ > > drivers/platform/x86/Makefile | 1 + > > drivers/platform/x86/lenovo-wmi-gamezone.c | 203 +++++++++++++++++++++ > > drivers/platform/x86/lenovo-wmi.h | 105 +++++++++++ > > 5 files changed, 327 insertions(+) > > create mode 100644 drivers/platform/x86/lenovo-wmi-gamezone.c > > create mode 100644 drivers/platform/x86/lenovo-wmi.h > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index baf0eeb9a355..8f8a6aec6b92 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -13034,6 +13034,13 @@ S: Maintained > > W: http://legousb.sourceforge.net/ > > F: drivers/usb/misc/legousbtower.c > > > > +LENOVO WMI drivers > > +M: Derek J. Clark <derekjohn.clark@gmail.com> > > +L: platform-driver-x86@vger.kernel.org > > +S: Maintained > > +F: drivers/platform/x86/lenovo-wmi-gamezone.c > > +F: drivers/platform/x86/lenovo-wmi.h > > + > > LETSKETCH HID TABLET DRIVER > > M: Hans de Goede <hdegoede@redhat.com> > > L: linux-input@vger.kernel.org > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > > index 0258dd879d64..9a6ac7fdec9f 100644 > > --- a/drivers/platform/x86/Kconfig > > +++ b/drivers/platform/x86/Kconfig > > @@ -459,6 +459,17 @@ config IBM_RTL > > state = 0 (BIOS SMIs on) > > state = 1 (BIOS SMIs off) > > > > +config LENOVO_WMI_GAMEZONE > > + tristate "Lenovo GameZone WMI Driver" > > + depends on ACPI_WMI > > + select ACPI_PLATFORM_PROFILE > > + help > > + Say Y here if you have a WMI aware Lenovo Legion device and would like to use the > > + platform-profile firmware interface. > > + > > + To compile this driver as a module, choose M here: the module will > > + be called lenovo_wmi_gamezone. > > + > > config IDEAPAD_LAPTOP > > tristate "Lenovo IdeaPad Laptop Extras" > > depends on ACPI > > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > > index e1b142947067..7cb29a480ed2 100644 > > --- a/drivers/platform/x86/Makefile > > +++ b/drivers/platform/x86/Makefile > > @@ -68,6 +68,7 @@ obj-$(CONFIG_THINKPAD_LMI) += think-lmi.o > > obj-$(CONFIG_YOGABOOK) += lenovo-yogabook.o > > obj-$(CONFIG_YT2_1380) += lenovo-yoga-tab2-pro-1380-fastcharger.o > > obj-$(CONFIG_LENOVO_WMI_CAMERA) += lenovo-wmi-camera.o > > +obj-$(CONFIG_LENOVO_WMI_GAMEZONE) += lenovo-wmi-gamezone.o > > > > # Intel > > obj-y += intel/ > > diff --git a/drivers/platform/x86/lenovo-wmi-gamezone.c b/drivers/platform/x86/lenovo-wmi-gamezone.c > > new file mode 100644 > > index 000000000000..da5e2bc41f39 > > --- /dev/null > > +++ b/drivers/platform/x86/lenovo-wmi-gamezone.c > > @@ -0,0 +1,203 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +/* > > + * Lenovo GameZone WMI interface driver. The GameZone WMI interface provides > > + * platform profile and fan curve settings for devices that fall under the > > + * "Gaming Series" of Lenovo Legion devices. > > + * > > + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> > > + */ > > + > > +#include <linux/platform_profile.h> > > +#include "lenovo-wmi.h" > > + > > +#define LENOVO_GAMEZONE_GUID "887B54E3-DDDC-4B2C-8B88-68A26A8835D0" > > + > > +/* Method IDs */ > > +#define WMI_METHOD_ID_SMARTFAN_SUPP 43 /* IsSupportSmartFan */ > > +#define WMI_METHOD_ID_SMARTFAN_SET 44 /* SetSmartFanMode */ > > +#define WMI_METHOD_ID_SMARTFAN_GET 45 /* GetSmartFanMode */ > > + > > +static DEFINE_MUTEX(call_mutex); > > + > > +static const struct wmi_device_id lenovo_wmi_gamezone_id_table[] = { > > + { LENOVO_GAMEZONE_GUID, NULL }, /* LENOVO_GAMEZONE_DATA */ > > + {} > > +}; > > + > > +struct lenovo_wmi_gz_priv { > > + struct wmi_device *wdev; > > + enum platform_profile_option current_profile; > > + struct platform_profile_handler pprof; > > + bool platform_profile_support; > > +}; > > + > > +/* Platform Profile Methods */ > > +static int lenovo_wmi_gamezone_platform_profile_supported( > > + struct platform_profile_handler *pprof, int *supported) > > +{ > > + struct lenovo_wmi_gz_priv *priv; > > + > > + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); > > + > > + guard(mutex)(&call_mutex); > > + return lenovo_wmidev_evaluate_method_1( > > + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_SUPP, 0, supported); > > +} > > + > > +static int > > +lenovo_wmi_gamezone_profile_get(struct platform_profile_handler *pprof, > > + enum platform_profile_option *profile) > > +{ > > + struct lenovo_wmi_gz_priv *priv; > > + int sel_prof; > > + int err; > > + > > + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); > > + > > + guard(mutex)(&call_mutex); > > + err = lenovo_wmidev_evaluate_method_1( > > + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_GET, 0, &sel_prof); > > + if (err) { > > + pr_err("Error getting fan profile from WMI interface: %d\n", > > + err); > > + return err; > > + } > > + > > + switch (sel_prof) { > > + case SMARTFAN_MODE_QUIET: > > + *profile = PLATFORM_PROFILE_QUIET; > > + break; > > + case SMARTFAN_MODE_BALANCED: > > + *profile = PLATFORM_PROFILE_BALANCED; > > + break; > > + case SMARTFAN_MODE_PERFORMANCE: > > + *profile = PLATFORM_PROFILE_PERFORMANCE; > > + break; > > + case SMARTFAN_MODE_CUSTOM: > > + *profile = PLATFORM_PROFILE_CUSTOM; > > + break; > > + default: > > + return -EINVAL; > > + } > > + priv->current_profile = *profile; > > + > > + return 0; > > +} > > + > > +static int > > +lenovo_wmi_gamezone_profile_set(struct platform_profile_handler *pprof, > > + enum platform_profile_option profile) > > +{ > > + struct lenovo_wmi_gz_priv *priv; > > + int sel_prof; > > + int err; > > + > > + switch (profile) { > > + case PLATFORM_PROFILE_QUIET: > > + sel_prof = SMARTFAN_MODE_QUIET; > > + break; > > + case PLATFORM_PROFILE_BALANCED: > > + sel_prof = SMARTFAN_MODE_BALANCED; > > + break; > > + case PLATFORM_PROFILE_PERFORMANCE: > > + sel_prof = SMARTFAN_MODE_PERFORMANCE; > > + break; > > + case PLATFORM_PROFILE_CUSTOM: > > + sel_prof = SMARTFAN_MODE_CUSTOM; > > + break; > > + default: > > + return -EOPNOTSUPP; > > + } > > + > > + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); > > + > > + guard(mutex)(&call_mutex); > > + err = lenovo_wmidev_evaluate_method_1( > > + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_SET, sel_prof, NULL); > > + if (err) { > > + pr_err("Error setting fan profile on WMI interface: %u\n", err); > > + return err; > > + } > > + > > + priv->current_profile = profile; > > + return 0; > > +} > > + > > +/* Driver Setup */ > > +static int platform_profile_setup(struct lenovo_wmi_gz_priv *priv) > > +{ > > + int supported; > > + int err; > > + > > + err = lenovo_wmi_gamezone_platform_profile_supported(&priv->pprof, > > + &supported); > > + if (err) { > > + pr_err("Error checking platform profile support: %d\n", err); > > + return err; > > + } > > + > > + priv->platform_profile_support = supported; > > + > > + if (!supported) > > + return -EOPNOTSUPP; > > + > > + priv->pprof.name = "lenovo-wmi-gamezone"; > > + priv->pprof.profile_get = lenovo_wmi_gamezone_profile_get; > > + priv->pprof.profile_set = lenovo_wmi_gamezone_profile_set; > > + > > + set_bit(PLATFORM_PROFILE_QUIET, priv->pprof.choices); > > + set_bit(PLATFORM_PROFILE_BALANCED, priv->pprof.choices); > > + set_bit(PLATFORM_PROFILE_PERFORMANCE, priv->pprof.choices); > > + set_bit(PLATFORM_PROFILE_CUSTOM, priv->pprof.choices); > > + > > + err = lenovo_wmi_gamezone_profile_get(&priv->pprof, > > + &priv->current_profile); > > + if (err) { > > + pr_err("Error getting current platform profile: %d\n", err); > > + return err; > > + } > > + > > + guard(mutex)(&call_mutex); > > + err = platform_profile_register(&priv->pprof); > > + if (err) { > > + pr_err("Error registering platform profile: %d\n", err); > > + return err; > > + } > > + > > + return 0; > > +} > > + > > +static int lenovo_wmi_gamezone_probe(struct wmi_device *wdev, > > + const void *context) > > +{ > > + struct lenovo_wmi_gz_priv *priv; > > + > > + priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL); > > + if (!priv) > > + return -ENOMEM; > > + > > + priv->wdev = wdev; > > + return platform_profile_setup(priv); > > +} > > + > > +static void lenovo_wmi_gamezone_remove(struct wmi_device *wdev) > > +{ > > + struct lenovo_wmi_gz_priv *priv = dev_get_drvdata(&wdev->dev); > > + > > + guard(mutex)(&call_mutex); > > + platform_profile_remove(&priv->pprof); > > +} > > + > > +static struct wmi_driver lenovo_wmi_gamezone_driver = { > > + .driver = { .name = "lenovo_wmi_gamezone" }, > > + .id_table = lenovo_wmi_gamezone_id_table, > > + .probe = lenovo_wmi_gamezone_probe, > > + .remove = lenovo_wmi_gamezone_remove, > > +}; > > + > > +module_wmi_driver(lenovo_wmi_gamezone_driver); > > + > > +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_gamezone_id_table); > > +MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>"); > > +MODULE_DESCRIPTION("Lenovo GameZone WMI Driver"); > > +MODULE_LICENSE("GPL"); > > diff --git a/drivers/platform/x86/lenovo-wmi.h b/drivers/platform/x86/lenovo-wmi.h > > new file mode 100644 > > index 000000000000..8a302c6c47cb > > --- /dev/null > > +++ b/drivers/platform/x86/lenovo-wmi.h > > @@ -0,0 +1,105 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later > > + * > > + * Lenovo Legion WMI interface driver. The Lenovo Legion WMI interface is > > + * broken up into multiple GUID interfaces that require cross-references > > + * between GUID's for some functionality. The "Custom Mode" interface is a > > + * legacy interface for managing and displaying CPU & GPU power and hwmon > > + * settings and readings. The "Other Mode" interface is a modern interface > > + * that replaces or extends the "Custom Mode" interface methods. The > > + * "GameZone" interface adds advanced features such as fan profiles and > > + * overclocking. The "Lighting" interface adds control of various status > > + * lights related to different hardware components. "Other Method" uses > > + * the data structs LENOVO_CAPABILITY_DATA_00, LENOVO_CAPABILITY_DATA_01 > > + * and LENOVO_CAPABILITY_DATA_02 structs for capability information. > > + * > > + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> > > + * > > + */ > > + > > +#define pr_fmt(fmt) "%s:%s: " fmt, KBUILD_MODNAME, __func__ > > Don't include __func__ into pr_fmt(). Including __func__ into any >dbg > level message is not helpful to user, the error/warning/info should be > written in plain English, not in terms of code/function names. > > The usual pr_fmt() boilerplate is this: > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > -- > i. > Will do, thanks Ilpo. Derek > > + > > +#ifndef _LENOVO_WMI_H_ > > +#define _LENOVO_WMI_H_ > > + > > +#include <linux/mutex.h> > > +#include <linux/types.h> > > +#include <linux/wmi.h> > > + > > +/* Platform Profile Modes */ > > +#define SMARTFAN_MODE_QUIET 0x01 > > +#define SMARTFAN_MODE_BALANCED 0x02 > > +#define SMARTFAN_MODE_PERFORMANCE 0x03 > > +#define SMARTFAN_MODE_CUSTOM 0xFF > > + > > +struct wmi_method_args { > > + u32 arg0; > > + u32 arg1; > > +}; > > + > > +/* General Use functions */ > > +static int lenovo_wmidev_evaluate_method(struct wmi_device *wdev, u8 instance, > > + u32 method_id, struct acpi_buffer *in, > > + struct acpi_buffer *out) > > +{ > > + acpi_status status; > > + > > + status = wmidev_evaluate_method(wdev, instance, method_id, in, out); > > + > > + if (ACPI_FAILURE(status)) > > + return -EIO; > > + > > + return 0; > > +}; > > + > > +int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, > > + u32 method_id, u32 arg0, u32 arg1, > > + u32 *retval); > > + > > +int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, > > + u32 method_id, u32 arg0, u32 arg1, > > + u32 *retval) > > +{ > > + struct wmi_method_args args = { arg0, arg1 }; > > + struct acpi_buffer input = { (acpi_size)sizeof(args), &args }; > > + struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL }; > > + union acpi_object *ret_obj = NULL; > > + int err; > > + > > + err = lenovo_wmidev_evaluate_method(wdev, instance, method_id, &input, > > + &output); > > + > > + if (err) { > > + pr_err("Attempt to get method value failed.\n"); > > + return err; > > + } > > + > > + if (retval) { > > + ret_obj = (union acpi_object *)output.pointer; > > + if (!ret_obj) { > > + pr_err("Failed to get valid ACPI object from WMI interface\n"); > > + return -EIO; > > + } > > + if (ret_obj->type != ACPI_TYPE_INTEGER) { > > + pr_err("WMI query returnd ACPI object with wrong type.\n"); > > + kfree(ret_obj); > > + return -EIO; > > + } > > + *retval = (u32)ret_obj->integer.value; > > + } > > + > > + kfree(ret_obj); > > + > > + return 0; > > +} > > + > > +int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 instance, > > + u32 method_id, u32 arg0, u32 *retval); > > + > > +int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 instance, > > + u32 method_id, u32 arg0, u32 *retval) > > +{ > > + return lenovo_wmidev_evaluate_method_2(wdev, instance, method_id, arg0, > > + 0, retval); > > +} > > + > > +#endif /* !_LENOVO_WMI_H_ */ > >
Am 10.01.25 um 22:33 schrieb Derek John Clark: > On Thu, Jan 9, 2025 at 2:12 PM Armin Wolf <W_Armin@gmx.de> wrote: >> Am 02.01.25 um 01:47 schrieb Derek J. Clark: >> >>> Adds lenovo-wmi-gamezone.c which provides a driver for the Lenovo >>> GameZone WMI interface that comes on Lenovo "Gaming Series" hardware. >>> Provides ACPI platform profiles over WMI. >>> >>> v2: >>> - Use devm_kzalloc to ensure driver can be instanced, remove global >>> reference. >>> - Ensure reverse Christmas tree for all variable declarations. >>> - Remove extra whitespace. >>> - Use guard(mutex) in all mutex instances, global mutex. >>> - Use pr_fmt instead of adding the driver name to each pr_err. >>> - Remove noisy pr_info usage. >>> - Rename gamezone_wmi to lenovo_wmi_gz_priv and gz_wmi to priv. >>> - Remove GZ_WMI symbol exporting. >>> >>> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> >>> --- >>> MAINTAINERS | 7 + >>> drivers/platform/x86/Kconfig | 11 ++ >>> drivers/platform/x86/Makefile | 1 + >>> drivers/platform/x86/lenovo-wmi-gamezone.c | 203 +++++++++++++++++++++ >>> drivers/platform/x86/lenovo-wmi.h | 105 +++++++++++ >>> 5 files changed, 327 insertions(+) >>> create mode 100644 drivers/platform/x86/lenovo-wmi-gamezone.c >>> create mode 100644 drivers/platform/x86/lenovo-wmi.h >>> >>> diff --git a/MAINTAINERS b/MAINTAINERS >>> index baf0eeb9a355..8f8a6aec6b92 100644 >>> --- a/MAINTAINERS >>> +++ b/MAINTAINERS >>> @@ -13034,6 +13034,13 @@ S: Maintained >>> W: http://legousb.sourceforge.net/ >>> F: drivers/usb/misc/legousbtower.c >>> >>> +LENOVO WMI drivers >>> +M: Derek J. Clark <derekjohn.clark@gmail.com> >>> +L: platform-driver-x86@vger.kernel.org >>> +S: Maintained >>> +F: drivers/platform/x86/lenovo-wmi-gamezone.c >>> +F: drivers/platform/x86/lenovo-wmi.h >>> + >>> LETSKETCH HID TABLET DRIVER >>> M: Hans de Goede <hdegoede@redhat.com> >>> L: linux-input@vger.kernel.org >>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig >>> index 0258dd879d64..9a6ac7fdec9f 100644 >>> --- a/drivers/platform/x86/Kconfig >>> +++ b/drivers/platform/x86/Kconfig >>> @@ -459,6 +459,17 @@ config IBM_RTL >>> state = 0 (BIOS SMIs on) >>> state = 1 (BIOS SMIs off) >>> >>> +config LENOVO_WMI_GAMEZONE >>> + tristate "Lenovo GameZone WMI Driver" >>> + depends on ACPI_WMI >>> + select ACPI_PLATFORM_PROFILE >>> + help >>> + Say Y here if you have a WMI aware Lenovo Legion device and would like to use the >>> + platform-profile firmware interface. >>> + >>> + To compile this driver as a module, choose M here: the module will >>> + be called lenovo_wmi_gamezone. >> Could it be that the resulting kernel module is actually named lenovo-wmi-gamezone?. >> If yes then please adjust the config description. >> > the .o/.ko are named as you described with -, but lsmod lists them > with _ which is how most would interact with the driver if manually > loading or blocking it. I'll put whichever you think is most > appropriate. I would prefer if you use the .ko names. >>> + >>> config IDEAPAD_LAPTOP >>> tristate "Lenovo IdeaPad Laptop Extras" >>> depends on ACPI >>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile >>> index e1b142947067..7cb29a480ed2 100644 >>> --- a/drivers/platform/x86/Makefile >>> +++ b/drivers/platform/x86/Makefile >>> @@ -68,6 +68,7 @@ obj-$(CONFIG_THINKPAD_LMI) += think-lmi.o >>> obj-$(CONFIG_YOGABOOK) += lenovo-yogabook.o >>> obj-$(CONFIG_YT2_1380) += lenovo-yoga-tab2-pro-1380-fastcharger.o >>> obj-$(CONFIG_LENOVO_WMI_CAMERA) += lenovo-wmi-camera.o >>> +obj-$(CONFIG_LENOVO_WMI_GAMEZONE) += lenovo-wmi-gamezone.o >>> >>> # Intel >>> obj-y += intel/ >>> diff --git a/drivers/platform/x86/lenovo-wmi-gamezone.c b/drivers/platform/x86/lenovo-wmi-gamezone.c >>> new file mode 100644 >>> index 000000000000..da5e2bc41f39 >>> --- /dev/null >>> +++ b/drivers/platform/x86/lenovo-wmi-gamezone.c >>> @@ -0,0 +1,203 @@ >>> +// SPDX-License-Identifier: GPL-2.0-or-later >>> +/* >>> + * Lenovo GameZone WMI interface driver. The GameZone WMI interface provides >>> + * platform profile and fan curve settings for devices that fall under the >>> + * "Gaming Series" of Lenovo Legion devices. >>> + * >>> + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> >>> + */ >>> + >>> +#include <linux/platform_profile.h> >>> +#include "lenovo-wmi.h" >> Please add the necessary includes here and do not rely on the header file to pull them in. >> > Ack > >>> + >>> +#define LENOVO_GAMEZONE_GUID "887B54E3-DDDC-4B2C-8B88-68A26A8835D0" >>> + >>> +/* Method IDs */ >>> +#define WMI_METHOD_ID_SMARTFAN_SUPP 43 /* IsSupportSmartFan */ >>> +#define WMI_METHOD_ID_SMARTFAN_SET 44 /* SetSmartFanMode */ >>> +#define WMI_METHOD_ID_SMARTFAN_GET 45 /* GetSmartFanMode */ >>> + >>> +static DEFINE_MUTEX(call_mutex); >>> + >>> +static const struct wmi_device_id lenovo_wmi_gamezone_id_table[] = { >>> + { LENOVO_GAMEZONE_GUID, NULL }, /* LENOVO_GAMEZONE_DATA */ >>> + {} >>> +}; >> Please move those device IDs closer to the driver struct which uses them. >> > Ack > >>> + >>> +struct lenovo_wmi_gz_priv { >>> + struct wmi_device *wdev; >>> + enum platform_profile_option current_profile; >>> + struct platform_profile_handler pprof; >>> + bool platform_profile_support; >>> +}; >>> + >>> +/* Platform Profile Methods */ >>> +static int lenovo_wmi_gamezone_platform_profile_supported( >>> + struct platform_profile_handler *pprof, int *supported) >> Please use ./scripts/checkpatch --strict to catch any coding style violations like this one. >> > Ack. Sorry about that. > >>> +{ >>> + struct lenovo_wmi_gz_priv *priv; >>> + >>> + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); >> Is there a reason why you are not passing priv as an argument? If no then please pass priv >> as an argument so you can avoid having to use container_of(). >> >>> + >>> + guard(mutex)(&call_mutex); >> Is there a technical reason why you have to use a mutex for WMI method access? If no then please remove >> this mutex. >> > We weren't sure and figured you would know best practice. I'll remove them. > >>> + return lenovo_wmidev_evaluate_method_1( >>> + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_SUPP, 0, supported); >>> +} >>> + >>> +static int >>> +lenovo_wmi_gamezone_profile_get(struct platform_profile_handler *pprof, >>> + enum platform_profile_option *profile) >>> +{ >>> + struct lenovo_wmi_gz_priv *priv; >>> + int sel_prof; >>> + int err; >>> + >>> + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); >>> + >>> + guard(mutex)(&call_mutex); >>> + err = lenovo_wmidev_evaluate_method_1( >>> + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_GET, 0, &sel_prof); >>> + if (err) { >>> + pr_err("Error getting fan profile from WMI interface: %d\n", >>> + err); >> Please just return here without printing anything, userspace does not benefit from such >> an error message which only states the obvious. >> > Ack all debug return messages. > >>> + return err; >>> + } >>> + >>> + switch (sel_prof) { >>> + case SMARTFAN_MODE_QUIET: >>> + *profile = PLATFORM_PROFILE_QUIET; >>> + break; >>> + case SMARTFAN_MODE_BALANCED: >>> + *profile = PLATFORM_PROFILE_BALANCED; >>> + break; >>> + case SMARTFAN_MODE_PERFORMANCE: >>> + *profile = PLATFORM_PROFILE_PERFORMANCE; >>> + break; >>> + case SMARTFAN_MODE_CUSTOM: >>> + *profile = PLATFORM_PROFILE_CUSTOM; >>> + break; >>> + default: >>> + return -EINVAL; >>> + } >>> + priv->current_profile = *profile; >> Please remove this unused variable from priv. >> > Ack > >>> + >>> + return 0; >>> +} >>> + >>> +static int >>> +lenovo_wmi_gamezone_profile_set(struct platform_profile_handler *pprof, >>> + enum platform_profile_option profile) >>> +{ >>> + struct lenovo_wmi_gz_priv *priv; >>> + int sel_prof; >>> + int err; >>> + >>> + switch (profile) { >>> + case PLATFORM_PROFILE_QUIET: >>> + sel_prof = SMARTFAN_MODE_QUIET; >>> + break; >>> + case PLATFORM_PROFILE_BALANCED: >>> + sel_prof = SMARTFAN_MODE_BALANCED; >>> + break; >>> + case PLATFORM_PROFILE_PERFORMANCE: >>> + sel_prof = SMARTFAN_MODE_PERFORMANCE; >>> + break; >>> + case PLATFORM_PROFILE_CUSTOM: >>> + sel_prof = SMARTFAN_MODE_CUSTOM; >>> + break; >>> + default: >>> + return -EOPNOTSUPP; >>> + } >>> + >>> + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); >> Please assign priv during declaration. >> > Ack > >>> + >>> + guard(mutex)(&call_mutex); >>> + err = lenovo_wmidev_evaluate_method_1( >>> + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_SET, sel_prof, NULL); >>> + if (err) { >>> + pr_err("Error setting fan profile on WMI interface: %u\n", err); >> Again, this error message only states the obvious, please remove it. >> >>> + return err; >>> + } >>> + >>> + priv->current_profile = profile; >>> + return 0; >>> +} >>> + >>> +/* Driver Setup */ >>> +static int platform_profile_setup(struct lenovo_wmi_gz_priv *priv) >>> +{ >>> + int supported; >>> + int err; >>> + >>> + err = lenovo_wmi_gamezone_platform_profile_supported(&priv->pprof, >>> + &supported); >>> + if (err) { >>> + pr_err("Error checking platform profile support: %d\n", err); >>> + return err; >> Please use dev_err() here. > Ack > >>> + } >>> + >>> + priv->platform_profile_support = supported; >> The value of platform_profile_support is not used anywhere, please remove it. >> >>> + >>> + if (!supported) >>> + return -EOPNOTSUPP; >> IMHO returning -ENODEV would make more sense here. >> > Ack > >>> + >>> + priv->pprof.name = "lenovo-wmi-gamezone"; >>> + priv->pprof.profile_get = lenovo_wmi_gamezone_profile_get; >>> + priv->pprof.profile_set = lenovo_wmi_gamezone_profile_set; >>> + >>> + set_bit(PLATFORM_PROFILE_QUIET, priv->pprof.choices); >>> + set_bit(PLATFORM_PROFILE_BALANCED, priv->pprof.choices); >>> + set_bit(PLATFORM_PROFILE_PERFORMANCE, priv->pprof.choices); >>> + set_bit(PLATFORM_PROFILE_CUSTOM, priv->pprof.choices); >>> + >>> + err = lenovo_wmi_gamezone_profile_get(&priv->pprof, >>> + &priv->current_profile); >>> + if (err) { >>> + pr_err("Error getting current platform profile: %d\n", err); >>> + return err; >>> + } >> Is there a technical reason for reading the current platform profile during device >> initialization(? If no then please remove this call. >> > There isn't, just a misconception on my part. WIll remove. > >>> + >>> + guard(mutex)(&call_mutex); >>> + err = platform_profile_register(&priv->pprof); >> Using devm_platform_profile_register() would make sense here. This function was added very recently >> so you have to base your patch series onto the for-next branch. >> > Ack > >>> + if (err) { >>> + pr_err("Error registering platform profile: %d\n", err); >>> + return err; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static int lenovo_wmi_gamezone_probe(struct wmi_device *wdev, >>> + const void *context) >>> +{ >>> + struct lenovo_wmi_gz_priv *priv; >>> + >>> + priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL); >>> + if (!priv) >>> + return -ENOMEM; >>> + >>> + priv->wdev = wdev; >> Since you are using dev_get_drvdata(), you also need to use dev_set_drvdata() here, otherwise >> dev_get_drvdata() will return no valid value. >> > Ack > >>> + return platform_profile_setup(priv); >>> +} >>> + >>> +static void lenovo_wmi_gamezone_remove(struct wmi_device *wdev) >>> +{ >>> + struct lenovo_wmi_gz_priv *priv = dev_get_drvdata(&wdev->dev); >>> + >>> + guard(mutex)(&call_mutex); >>> + platform_profile_remove(&priv->pprof); >>> +} >>> + >>> +static struct wmi_driver lenovo_wmi_gamezone_driver = { >>> + .driver = { .name = "lenovo_wmi_gamezone" }, >> Please set ".probe_type = PROBE_PREFER_ASYNCHRONOUS" here. >> > Ack > >> Also does the selected fan profile remain the same after suspending or hibernating? >> If no then please add the necessary PM callbacks to save/restore the fan profile >> before suspend/after resume if necessary. >> > It remains the same after suspend, hibernate, reboot, and shutdown. Nice, so no special pm handling is necessary here :). Thanks, Armin Wolf > >>> + .id_table = lenovo_wmi_gamezone_id_table, >>> + .probe = lenovo_wmi_gamezone_probe, >>> + .remove = lenovo_wmi_gamezone_remove, >> Please set ".no_singleton = true" here. >> > Ack > >>> +}; >>> + >>> +module_wmi_driver(lenovo_wmi_gamezone_driver); >>> + >>> +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_gamezone_id_table); >>> +MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>"); >>> +MODULE_DESCRIPTION("Lenovo GameZone WMI Driver"); >>> +MODULE_LICENSE("GPL"); >>> diff --git a/drivers/platform/x86/lenovo-wmi.h b/drivers/platform/x86/lenovo-wmi.h >>> new file mode 100644 >>> index 000000000000..8a302c6c47cb >>> --- /dev/null >>> +++ b/drivers/platform/x86/lenovo-wmi.h >>> @@ -0,0 +1,105 @@ >>> +/* SPDX-License-Identifier: GPL-2.0-or-later >>> + * >>> + * Lenovo Legion WMI interface driver. The Lenovo Legion WMI interface is >>> + * broken up into multiple GUID interfaces that require cross-references >>> + * between GUID's for some functionality. The "Custom Mode" interface is a >>> + * legacy interface for managing and displaying CPU & GPU power and hwmon >>> + * settings and readings. The "Other Mode" interface is a modern interface >>> + * that replaces or extends the "Custom Mode" interface methods. The >>> + * "GameZone" interface adds advanced features such as fan profiles and >>> + * overclocking. The "Lighting" interface adds control of various status >>> + * lights related to different hardware components. "Other Method" uses >>> + * the data structs LENOVO_CAPABILITY_DATA_00, LENOVO_CAPABILITY_DATA_01 >>> + * and LENOVO_CAPABILITY_DATA_02 structs for capability information. >>> + * >>> + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> >>> + * >>> + */ >>> + >>> +#define pr_fmt(fmt) "%s:%s: " fmt, KBUILD_MODNAME, __func__ >>> + >>> +#ifndef _LENOVO_WMI_H_ >>> +#define _LENOVO_WMI_H_ >>> + >>> +#include <linux/mutex.h> >>> +#include <linux/types.h> >>> +#include <linux/wmi.h> >>> + >>> +/* Platform Profile Modes */ >>> +#define SMARTFAN_MODE_QUIET 0x01 >>> +#define SMARTFAN_MODE_BALANCED 0x02 >>> +#define SMARTFAN_MODE_PERFORMANCE 0x03 >>> +#define SMARTFAN_MODE_CUSTOM 0xFF >>> + >>> +struct wmi_method_args { >>> + u32 arg0; >>> + u32 arg1; >>> +}; >>> + >>> +/* General Use functions */ >>> +static int lenovo_wmidev_evaluate_method(struct wmi_device *wdev, u8 instance, >>> + u32 method_id, struct acpi_buffer *in, >>> + struct acpi_buffer *out) >>> +{ >>> + acpi_status status; >>> + >>> + status = wmidev_evaluate_method(wdev, instance, method_id, in, out); >>> + >>> + if (ACPI_FAILURE(status)) >>> + return -EIO; >>> + >>> + return 0; >>> +}; >>> + >>> +int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, >>> + u32 method_id, u32 arg0, u32 arg1, >>> + u32 *retval); >>> + >>> +int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, >>> + u32 method_id, u32 arg0, u32 arg1, >>> + u32 *retval) >>> +{ >>> + struct wmi_method_args args = { arg0, arg1 }; >>> + struct acpi_buffer input = { (acpi_size)sizeof(args), &args }; >>> + struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL }; >>> + union acpi_object *ret_obj = NULL; >>> + int err; >> Please order the variable declarations in reverse XMAS tree order. >> > Ack > >>> + >>> + err = lenovo_wmidev_evaluate_method(wdev, instance, method_id, &input, >>> + &output); >>> + >>> + if (err) { >>> + pr_err("Attempt to get method value failed.\n"); >> Please remove any error messages in this part of the code, printing error messages should >> ideally happen at the higher layers of the driver if necessary. >> >>> + return err; >>> + } >>> + >>> + if (retval) { >>> + ret_obj = (union acpi_object *)output.pointer; >>> + if (!ret_obj) { >>> + pr_err("Failed to get valid ACPI object from WMI interface\n"); >>> + return -EIO; >> -ENODATA. >> > Ack > >>> + } >>> + if (ret_obj->type != ACPI_TYPE_INTEGER) { >>> + pr_err("WMI query returnd ACPI object with wrong type.\n"); >>> + kfree(ret_obj); >>> + return -EIO; >> -ENXIO. >> > Ack > >>> + } >>> + *retval = (u32)ret_obj->integer.value; >>> + } >>> + >>> + kfree(ret_obj); >>> + >>> + return 0; >>> +} >>> + >>> +int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 instance, >>> + u32 method_id, u32 arg0, u32 *retval); >>> + >>> +int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 instance, >>> + u32 method_id, u32 arg0, u32 *retval) >>> +{ >>> + return lenovo_wmidev_evaluate_method_2(wdev, instance, method_id, arg0, >>> + 0, retval); >>> +} >>> + >>> +#endif /* !_LENOVO_WMI_H_ */ > Thanks Armin, > Derek
On Fri, Jan 10, 2025 at 1:33 PM Derek John Clark <derekjohn.clark@gmail.com> wrote: > > On Thu, Jan 9, 2025 at 2:12 PM Armin Wolf <W_Armin@gmx.de> wrote: > > > > Am 02.01.25 um 01:47 schrieb Derek J. Clark: > > > > > Adds lenovo-wmi-gamezone.c which provides a driver for the Lenovo > > > GameZone WMI interface that comes on Lenovo "Gaming Series" hardware. > > > Provides ACPI platform profiles over WMI. > > > > > > v2: > > > - Use devm_kzalloc to ensure driver can be instanced, remove global > > > reference. > > > - Ensure reverse Christmas tree for all variable declarations. > > > - Remove extra whitespace. > > > - Use guard(mutex) in all mutex instances, global mutex. > > > - Use pr_fmt instead of adding the driver name to each pr_err. > > > - Remove noisy pr_info usage. > > > - Rename gamezone_wmi to lenovo_wmi_gz_priv and gz_wmi to priv. > > > - Remove GZ_WMI symbol exporting. > > > > > > Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> > > > --- > > > MAINTAINERS | 7 + > > > drivers/platform/x86/Kconfig | 11 ++ > > > drivers/platform/x86/Makefile | 1 + > > > drivers/platform/x86/lenovo-wmi-gamezone.c | 203 +++++++++++++++++++++ > > > drivers/platform/x86/lenovo-wmi.h | 105 +++++++++++ > > > 5 files changed, 327 insertions(+) > > > create mode 100644 drivers/platform/x86/lenovo-wmi-gamezone.c > > > create mode 100644 drivers/platform/x86/lenovo-wmi.h > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index baf0eeb9a355..8f8a6aec6b92 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -13034,6 +13034,13 @@ S: Maintained > > > W: http://legousb.sourceforge.net/ > > > F: drivers/usb/misc/legousbtower.c > > > > > > +LENOVO WMI drivers > > > +M: Derek J. Clark <derekjohn.clark@gmail.com> > > > +L: platform-driver-x86@vger.kernel.org > > > +S: Maintained > > > +F: drivers/platform/x86/lenovo-wmi-gamezone.c > > > +F: drivers/platform/x86/lenovo-wmi.h > > > + > > > LETSKETCH HID TABLET DRIVER > > > M: Hans de Goede <hdegoede@redhat.com> > > > L: linux-input@vger.kernel.org > > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > > > index 0258dd879d64..9a6ac7fdec9f 100644 > > > --- a/drivers/platform/x86/Kconfig > > > +++ b/drivers/platform/x86/Kconfig > > > @@ -459,6 +459,17 @@ config IBM_RTL > > > state = 0 (BIOS SMIs on) > > > state = 1 (BIOS SMIs off) > > > > > > +config LENOVO_WMI_GAMEZONE > > > + tristate "Lenovo GameZone WMI Driver" > > > + depends on ACPI_WMI > > > + select ACPI_PLATFORM_PROFILE > > > + help > > > + Say Y here if you have a WMI aware Lenovo Legion device and would like to use the > > > + platform-profile firmware interface. > > > + > > > + To compile this driver as a module, choose M here: the module will > > > + be called lenovo_wmi_gamezone. > > > > Could it be that the resulting kernel module is actually named lenovo-wmi-gamezone?. > > If yes then please adjust the config description. > > > > the .o/.ko are named as you described with -, but lsmod lists them > with _ which is how most would interact with the driver if manually > loading or blocking it. I'll put whichever you think is most > appropriate. > > > > + > > > config IDEAPAD_LAPTOP > > > tristate "Lenovo IdeaPad Laptop Extras" > > > depends on ACPI > > > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > > > index e1b142947067..7cb29a480ed2 100644 > > > --- a/drivers/platform/x86/Makefile > > > +++ b/drivers/platform/x86/Makefile > > > @@ -68,6 +68,7 @@ obj-$(CONFIG_THINKPAD_LMI) += think-lmi.o > > > obj-$(CONFIG_YOGABOOK) += lenovo-yogabook.o > > > obj-$(CONFIG_YT2_1380) += lenovo-yoga-tab2-pro-1380-fastcharger.o > > > obj-$(CONFIG_LENOVO_WMI_CAMERA) += lenovo-wmi-camera.o > > > +obj-$(CONFIG_LENOVO_WMI_GAMEZONE) += lenovo-wmi-gamezone.o > > > > > > # Intel > > > obj-y += intel/ > > > diff --git a/drivers/platform/x86/lenovo-wmi-gamezone.c b/drivers/platform/x86/lenovo-wmi-gamezone.c > > > new file mode 100644 > > > index 000000000000..da5e2bc41f39 > > > --- /dev/null > > > +++ b/drivers/platform/x86/lenovo-wmi-gamezone.c > > > @@ -0,0 +1,203 @@ > > > +// SPDX-License-Identifier: GPL-2.0-or-later > > > +/* > > > + * Lenovo GameZone WMI interface driver. The GameZone WMI interface provides > > > + * platform profile and fan curve settings for devices that fall under the > > > + * "Gaming Series" of Lenovo Legion devices. > > > + * > > > + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> > > > + */ > > > + > > > +#include <linux/platform_profile.h> > > > +#include "lenovo-wmi.h" > > > > Please add the necessary includes here and do not rely on the header file to pull them in. > > > > Ack > > > > + > > > +#define LENOVO_GAMEZONE_GUID "887B54E3-DDDC-4B2C-8B88-68A26A8835D0" > > > + > > > +/* Method IDs */ > > > +#define WMI_METHOD_ID_SMARTFAN_SUPP 43 /* IsSupportSmartFan */ > > > +#define WMI_METHOD_ID_SMARTFAN_SET 44 /* SetSmartFanMode */ > > > +#define WMI_METHOD_ID_SMARTFAN_GET 45 /* GetSmartFanMode */ > > > + > > > +static DEFINE_MUTEX(call_mutex); > > > + > > > +static const struct wmi_device_id lenovo_wmi_gamezone_id_table[] = { > > > + { LENOVO_GAMEZONE_GUID, NULL }, /* LENOVO_GAMEZONE_DATA */ > > > + {} > > > +}; > > > > Please move those device IDs closer to the driver struct which uses them. > > > > Ack > > > > + > > > +struct lenovo_wmi_gz_priv { > > > + struct wmi_device *wdev; > > > + enum platform_profile_option current_profile; > > > + struct platform_profile_handler pprof; > > > + bool platform_profile_support; > > > +}; > > > + > > > +/* Platform Profile Methods */ > > > +static int lenovo_wmi_gamezone_platform_profile_supported( > > > + struct platform_profile_handler *pprof, int *supported) > > > > Please use ./scripts/checkpatch --strict to catch any coding style violations like this one. > > > > Ack. Sorry about that. > > > > +{ > > > + struct lenovo_wmi_gz_priv *priv; > > > + > > > + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); > > > > Is there a reason why you are not passing priv as an argument? If no then please pass priv > > as an argument so you can avoid having to use container_of(). > > > > > + > > > + guard(mutex)(&call_mutex); > > > > Is there a technical reason why you have to use a mutex for WMI method access? If no then please remove > > this mutex. > > > > We weren't sure and figured you would know best practice. I'll remove them. > > > > + return lenovo_wmidev_evaluate_method_1( > > > + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_SUPP, 0, supported); > > > +} > > > + > > > +static int > > > +lenovo_wmi_gamezone_profile_get(struct platform_profile_handler *pprof, > > > + enum platform_profile_option *profile) > > > +{ > > > + struct lenovo_wmi_gz_priv *priv; > > > + int sel_prof; > > > + int err; > > > + > > > + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); > > > + > > > + guard(mutex)(&call_mutex); > > > + err = lenovo_wmidev_evaluate_method_1( > > > + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_GET, 0, &sel_prof); > > > + if (err) { > > > + pr_err("Error getting fan profile from WMI interface: %d\n", > > > + err); > > > > Please just return here without printing anything, userspace does not benefit from such > > an error message which only states the obvious. > > > > Ack all debug return messages. > > > > + return err; > > > + } > > > + > > > + switch (sel_prof) { > > > + case SMARTFAN_MODE_QUIET: > > > + *profile = PLATFORM_PROFILE_QUIET; > > > + break; > > > + case SMARTFAN_MODE_BALANCED: > > > + *profile = PLATFORM_PROFILE_BALANCED; > > > + break; > > > + case SMARTFAN_MODE_PERFORMANCE: > > > + *profile = PLATFORM_PROFILE_PERFORMANCE; > > > + break; > > > + case SMARTFAN_MODE_CUSTOM: > > > + *profile = PLATFORM_PROFILE_CUSTOM; > > > + break; > > > + default: > > > + return -EINVAL; > > > + } > > > + priv->current_profile = *profile; > > > > Please remove this unused variable from priv. > > > > Ack > > > > + > > > + return 0; > > > +} > > > + > > > +static int > > > +lenovo_wmi_gamezone_profile_set(struct platform_profile_handler *pprof, > > > + enum platform_profile_option profile) > > > +{ > > > + struct lenovo_wmi_gz_priv *priv; > > > + int sel_prof; > > > + int err; > > > + > > > + switch (profile) { > > > + case PLATFORM_PROFILE_QUIET: > > > + sel_prof = SMARTFAN_MODE_QUIET; > > > + break; > > > + case PLATFORM_PROFILE_BALANCED: > > > + sel_prof = SMARTFAN_MODE_BALANCED; > > > + break; > > > + case PLATFORM_PROFILE_PERFORMANCE: > > > + sel_prof = SMARTFAN_MODE_PERFORMANCE; > > > + break; > > > + case PLATFORM_PROFILE_CUSTOM: > > > + sel_prof = SMARTFAN_MODE_CUSTOM; > > > + break; > > > + default: > > > + return -EOPNOTSUPP; > > > + } > > > + > > > + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); > > > > Please assign priv during declaration. > > > > Ack > > > > + > > > + guard(mutex)(&call_mutex); > > > + err = lenovo_wmidev_evaluate_method_1( > > > + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_SET, sel_prof, NULL); > > > + if (err) { > > > + pr_err("Error setting fan profile on WMI interface: %u\n", err); > > > > Again, this error message only states the obvious, please remove it. > > > > > + return err; > > > + } > > > + > > > + priv->current_profile = profile; > > > + return 0; > > > +} > > > + > > > +/* Driver Setup */ > > > +static int platform_profile_setup(struct lenovo_wmi_gz_priv *priv) > > > +{ > > > + int supported; > > > + int err; > > > + > > > + err = lenovo_wmi_gamezone_platform_profile_supported(&priv->pprof, > > > + &supported); > > > + if (err) { > > > + pr_err("Error checking platform profile support: %d\n", err); > > > + return err; > > > > Please use dev_err() here. > > Ack > > > > + } > > > + > > > + priv->platform_profile_support = supported; > > > > The value of platform_profile_support is not used anywhere, please remove it. > > > > > + > > > + if (!supported) > > > + return -EOPNOTSUPP; > > > > IMHO returning -ENODEV would make more sense here. > > > > Ack > > > > + > > > + priv->pprof.name = "lenovo-wmi-gamezone"; > > > + priv->pprof.profile_get = lenovo_wmi_gamezone_profile_get; > > > + priv->pprof.profile_set = lenovo_wmi_gamezone_profile_set; > > > + > > > + set_bit(PLATFORM_PROFILE_QUIET, priv->pprof.choices); > > > + set_bit(PLATFORM_PROFILE_BALANCED, priv->pprof.choices); > > > + set_bit(PLATFORM_PROFILE_PERFORMANCE, priv->pprof.choices); > > > + set_bit(PLATFORM_PROFILE_CUSTOM, priv->pprof.choices); > > > + > > > + err = lenovo_wmi_gamezone_profile_get(&priv->pprof, > > > + &priv->current_profile); > > > + if (err) { > > > + pr_err("Error getting current platform profile: %d\n", err); > > > + return err; > > > + } > > > > Is there a technical reason for reading the current platform profile during device > > initialization(? If no then please remove this call. > > > > There isn't, just a misconception on my part. WIll remove. > > > > + > > > + guard(mutex)(&call_mutex); > > > + err = platform_profile_register(&priv->pprof); > > > > Using devm_platform_profile_register() would make sense here. This function was added very recently > > so you have to base your patch series onto the for-next branch. > > > > Ack > > > > + if (err) { > > > + pr_err("Error registering platform profile: %d\n", err); > > > + return err; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +static int lenovo_wmi_gamezone_probe(struct wmi_device *wdev, > > > + const void *context) > > > +{ > > > + struct lenovo_wmi_gz_priv *priv; > > > + > > > + priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL); > > > + if (!priv) > > > + return -ENOMEM; > > > + > > > + priv->wdev = wdev; > > > > Since you are using dev_get_drvdata(), you also need to use dev_set_drvdata() here, otherwise > > dev_get_drvdata() will return no valid value. > > > > Ack > > > > + return platform_profile_setup(priv); > > > +} > > > + > > > +static void lenovo_wmi_gamezone_remove(struct wmi_device *wdev) > > > +{ > > > + struct lenovo_wmi_gz_priv *priv = dev_get_drvdata(&wdev->dev); > > > + > > > + guard(mutex)(&call_mutex); > > > + platform_profile_remove(&priv->pprof); > > > +} > > > + > > > +static struct wmi_driver lenovo_wmi_gamezone_driver = { > > > + .driver = { .name = "lenovo_wmi_gamezone" }, > > > > Please set ".probe_type = PROBE_PREFER_ASYNCHRONOUS" here. > > > > Ack > > > Also does the selected fan profile remain the same after suspending or hibernating? > > If no then please add the necessary PM callbacks to save/restore the fan profile > > before suspend/after resume if necessary. > > > > It remains the same after suspend, hibernate, reboot, and shutdown. > > > > + .id_table = lenovo_wmi_gamezone_id_table, > > > + .probe = lenovo_wmi_gamezone_probe, > > > + .remove = lenovo_wmi_gamezone_remove, > > > > Please set ".no_singleton = true" here. > > > > Ack > > > > +}; > > > + > > > +module_wmi_driver(lenovo_wmi_gamezone_driver); > > > + > > > +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_gamezone_id_table); > > > +MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>"); > > > +MODULE_DESCRIPTION("Lenovo GameZone WMI Driver"); > > > +MODULE_LICENSE("GPL"); > > > diff --git a/drivers/platform/x86/lenovo-wmi.h b/drivers/platform/x86/lenovo-wmi.h > > > new file mode 100644 > > > index 000000000000..8a302c6c47cb > > > --- /dev/null > > > +++ b/drivers/platform/x86/lenovo-wmi.h > > > @@ -0,0 +1,105 @@ > > > +/* SPDX-License-Identifier: GPL-2.0-or-later > > > + * > > > + * Lenovo Legion WMI interface driver. The Lenovo Legion WMI interface is > > > + * broken up into multiple GUID interfaces that require cross-references > > > + * between GUID's for some functionality. The "Custom Mode" interface is a > > > + * legacy interface for managing and displaying CPU & GPU power and hwmon > > > + * settings and readings. The "Other Mode" interface is a modern interface > > > + * that replaces or extends the "Custom Mode" interface methods. The > > > + * "GameZone" interface adds advanced features such as fan profiles and > > > + * overclocking. The "Lighting" interface adds control of various status > > > + * lights related to different hardware components. "Other Method" uses > > > + * the data structs LENOVO_CAPABILITY_DATA_00, LENOVO_CAPABILITY_DATA_01 > > > + * and LENOVO_CAPABILITY_DATA_02 structs for capability information. > > > + * > > > + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> > > > + * > > > + */ > > > + > > > +#define pr_fmt(fmt) "%s:%s: " fmt, KBUILD_MODNAME, __func__ > > > + > > > +#ifndef _LENOVO_WMI_H_ > > > +#define _LENOVO_WMI_H_ > > > + > > > +#include <linux/mutex.h> > > > +#include <linux/types.h> > > > +#include <linux/wmi.h> > > > + > > > +/* Platform Profile Modes */ > > > +#define SMARTFAN_MODE_QUIET 0x01 > > > +#define SMARTFAN_MODE_BALANCED 0x02 > > > +#define SMARTFAN_MODE_PERFORMANCE 0x03 > > > +#define SMARTFAN_MODE_CUSTOM 0xFF > > > + > > > +struct wmi_method_args { > > > + u32 arg0; > > > + u32 arg1; > > > +}; > > > + > > > +/* General Use functions */ > > > +static int lenovo_wmidev_evaluate_method(struct wmi_device *wdev, u8 instance, > > > + u32 method_id, struct acpi_buffer *in, > > > + struct acpi_buffer *out) > > > +{ > > > + acpi_status status; > > > + > > > + status = wmidev_evaluate_method(wdev, instance, method_id, in, out); > > > + > > > + if (ACPI_FAILURE(status)) > > > + return -EIO; > > > + > > > + return 0; > > > +}; > > > + > > > +int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, > > > + u32 method_id, u32 arg0, u32 arg1, > > > + u32 *retval); > > > + > > > +int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, > > > + u32 method_id, u32 arg0, u32 arg1, > > > + u32 *retval) > > > +{ > > > + struct wmi_method_args args = { arg0, arg1 }; > > > + struct acpi_buffer input = { (acpi_size)sizeof(args), &args }; > > > + struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL }; > > > + union acpi_object *ret_obj = NULL; > > > + int err; > > > > Please order the variable declarations in reverse XMAS tree order. > > > > Ack > Armin, After looking at this again, args is used by output so it needs to be before it. I'll swap input and output unless you have another suggestion. Cheers, Derek > > > + > > > + err = lenovo_wmidev_evaluate_method(wdev, instance, method_id, &input, > > > + &output); > > > + > > > + if (err) { > > > + pr_err("Attempt to get method value failed.\n"); > > > > Please remove any error messages in this part of the code, printing error messages should > > ideally happen at the higher layers of the driver if necessary. > > > > > + return err; > > > + } > > > + > > > + if (retval) { > > > + ret_obj = (union acpi_object *)output.pointer; > > > + if (!ret_obj) { > > > + pr_err("Failed to get valid ACPI object from WMI interface\n"); > > > + return -EIO; > > > > -ENODATA. > > > > Ack > > > > + } > > > + if (ret_obj->type != ACPI_TYPE_INTEGER) { > > > + pr_err("WMI query returnd ACPI object with wrong type.\n"); > > > + kfree(ret_obj); > > > + return -EIO; > > > > -ENXIO. > > > > Ack > > > > + } > > > + *retval = (u32)ret_obj->integer.value; > > > + } > > > + > > > + kfree(ret_obj); > > > + > > > + return 0; > > > +} > > > + > > > +int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 instance, > > > + u32 method_id, u32 arg0, u32 *retval); > > > + > > > +int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 instance, > > > + u32 method_id, u32 arg0, u32 *retval) > > > +{ > > > + return lenovo_wmidev_evaluate_method_2(wdev, instance, method_id, arg0, > > > + 0, retval); > > > +} > > > + > > > +#endif /* !_LENOVO_WMI_H_ */ > > Thanks Armin, > Derek
diff --git a/MAINTAINERS b/MAINTAINERS index baf0eeb9a355..8f8a6aec6b92 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13034,6 +13034,13 @@ S: Maintained W: http://legousb.sourceforge.net/ F: drivers/usb/misc/legousbtower.c +LENOVO WMI drivers +M: Derek J. Clark <derekjohn.clark@gmail.com> +L: platform-driver-x86@vger.kernel.org +S: Maintained +F: drivers/platform/x86/lenovo-wmi-gamezone.c +F: drivers/platform/x86/lenovo-wmi.h + LETSKETCH HID TABLET DRIVER M: Hans de Goede <hdegoede@redhat.com> L: linux-input@vger.kernel.org diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 0258dd879d64..9a6ac7fdec9f 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -459,6 +459,17 @@ config IBM_RTL state = 0 (BIOS SMIs on) state = 1 (BIOS SMIs off) +config LENOVO_WMI_GAMEZONE + tristate "Lenovo GameZone WMI Driver" + depends on ACPI_WMI + select ACPI_PLATFORM_PROFILE + help + Say Y here if you have a WMI aware Lenovo Legion device and would like to use the + platform-profile firmware interface. + + To compile this driver as a module, choose M here: the module will + be called lenovo_wmi_gamezone. + config IDEAPAD_LAPTOP tristate "Lenovo IdeaPad Laptop Extras" depends on ACPI diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index e1b142947067..7cb29a480ed2 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -68,6 +68,7 @@ obj-$(CONFIG_THINKPAD_LMI) += think-lmi.o obj-$(CONFIG_YOGABOOK) += lenovo-yogabook.o obj-$(CONFIG_YT2_1380) += lenovo-yoga-tab2-pro-1380-fastcharger.o obj-$(CONFIG_LENOVO_WMI_CAMERA) += lenovo-wmi-camera.o +obj-$(CONFIG_LENOVO_WMI_GAMEZONE) += lenovo-wmi-gamezone.o # Intel obj-y += intel/ diff --git a/drivers/platform/x86/lenovo-wmi-gamezone.c b/drivers/platform/x86/lenovo-wmi-gamezone.c new file mode 100644 index 000000000000..da5e2bc41f39 --- /dev/null +++ b/drivers/platform/x86/lenovo-wmi-gamezone.c @@ -0,0 +1,203 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Lenovo GameZone WMI interface driver. The GameZone WMI interface provides + * platform profile and fan curve settings for devices that fall under the + * "Gaming Series" of Lenovo Legion devices. + * + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> + */ + +#include <linux/platform_profile.h> +#include "lenovo-wmi.h" + +#define LENOVO_GAMEZONE_GUID "887B54E3-DDDC-4B2C-8B88-68A26A8835D0" + +/* Method IDs */ +#define WMI_METHOD_ID_SMARTFAN_SUPP 43 /* IsSupportSmartFan */ +#define WMI_METHOD_ID_SMARTFAN_SET 44 /* SetSmartFanMode */ +#define WMI_METHOD_ID_SMARTFAN_GET 45 /* GetSmartFanMode */ + +static DEFINE_MUTEX(call_mutex); + +static const struct wmi_device_id lenovo_wmi_gamezone_id_table[] = { + { LENOVO_GAMEZONE_GUID, NULL }, /* LENOVO_GAMEZONE_DATA */ + {} +}; + +struct lenovo_wmi_gz_priv { + struct wmi_device *wdev; + enum platform_profile_option current_profile; + struct platform_profile_handler pprof; + bool platform_profile_support; +}; + +/* Platform Profile Methods */ +static int lenovo_wmi_gamezone_platform_profile_supported( + struct platform_profile_handler *pprof, int *supported) +{ + struct lenovo_wmi_gz_priv *priv; + + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); + + guard(mutex)(&call_mutex); + return lenovo_wmidev_evaluate_method_1( + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_SUPP, 0, supported); +} + +static int +lenovo_wmi_gamezone_profile_get(struct platform_profile_handler *pprof, + enum platform_profile_option *profile) +{ + struct lenovo_wmi_gz_priv *priv; + int sel_prof; + int err; + + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); + + guard(mutex)(&call_mutex); + err = lenovo_wmidev_evaluate_method_1( + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_GET, 0, &sel_prof); + if (err) { + pr_err("Error getting fan profile from WMI interface: %d\n", + err); + return err; + } + + switch (sel_prof) { + case SMARTFAN_MODE_QUIET: + *profile = PLATFORM_PROFILE_QUIET; + break; + case SMARTFAN_MODE_BALANCED: + *profile = PLATFORM_PROFILE_BALANCED; + break; + case SMARTFAN_MODE_PERFORMANCE: + *profile = PLATFORM_PROFILE_PERFORMANCE; + break; + case SMARTFAN_MODE_CUSTOM: + *profile = PLATFORM_PROFILE_CUSTOM; + break; + default: + return -EINVAL; + } + priv->current_profile = *profile; + + return 0; +} + +static int +lenovo_wmi_gamezone_profile_set(struct platform_profile_handler *pprof, + enum platform_profile_option profile) +{ + struct lenovo_wmi_gz_priv *priv; + int sel_prof; + int err; + + switch (profile) { + case PLATFORM_PROFILE_QUIET: + sel_prof = SMARTFAN_MODE_QUIET; + break; + case PLATFORM_PROFILE_BALANCED: + sel_prof = SMARTFAN_MODE_BALANCED; + break; + case PLATFORM_PROFILE_PERFORMANCE: + sel_prof = SMARTFAN_MODE_PERFORMANCE; + break; + case PLATFORM_PROFILE_CUSTOM: + sel_prof = SMARTFAN_MODE_CUSTOM; + break; + default: + return -EOPNOTSUPP; + } + + priv = container_of(pprof, struct lenovo_wmi_gz_priv, pprof); + + guard(mutex)(&call_mutex); + err = lenovo_wmidev_evaluate_method_1( + priv->wdev, 0x0, WMI_METHOD_ID_SMARTFAN_SET, sel_prof, NULL); + if (err) { + pr_err("Error setting fan profile on WMI interface: %u\n", err); + return err; + } + + priv->current_profile = profile; + return 0; +} + +/* Driver Setup */ +static int platform_profile_setup(struct lenovo_wmi_gz_priv *priv) +{ + int supported; + int err; + + err = lenovo_wmi_gamezone_platform_profile_supported(&priv->pprof, + &supported); + if (err) { + pr_err("Error checking platform profile support: %d\n", err); + return err; + } + + priv->platform_profile_support = supported; + + if (!supported) + return -EOPNOTSUPP; + + priv->pprof.name = "lenovo-wmi-gamezone"; + priv->pprof.profile_get = lenovo_wmi_gamezone_profile_get; + priv->pprof.profile_set = lenovo_wmi_gamezone_profile_set; + + set_bit(PLATFORM_PROFILE_QUIET, priv->pprof.choices); + set_bit(PLATFORM_PROFILE_BALANCED, priv->pprof.choices); + set_bit(PLATFORM_PROFILE_PERFORMANCE, priv->pprof.choices); + set_bit(PLATFORM_PROFILE_CUSTOM, priv->pprof.choices); + + err = lenovo_wmi_gamezone_profile_get(&priv->pprof, + &priv->current_profile); + if (err) { + pr_err("Error getting current platform profile: %d\n", err); + return err; + } + + guard(mutex)(&call_mutex); + err = platform_profile_register(&priv->pprof); + if (err) { + pr_err("Error registering platform profile: %d\n", err); + return err; + } + + return 0; +} + +static int lenovo_wmi_gamezone_probe(struct wmi_device *wdev, + const void *context) +{ + struct lenovo_wmi_gz_priv *priv; + + priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->wdev = wdev; + return platform_profile_setup(priv); +} + +static void lenovo_wmi_gamezone_remove(struct wmi_device *wdev) +{ + struct lenovo_wmi_gz_priv *priv = dev_get_drvdata(&wdev->dev); + + guard(mutex)(&call_mutex); + platform_profile_remove(&priv->pprof); +} + +static struct wmi_driver lenovo_wmi_gamezone_driver = { + .driver = { .name = "lenovo_wmi_gamezone" }, + .id_table = lenovo_wmi_gamezone_id_table, + .probe = lenovo_wmi_gamezone_probe, + .remove = lenovo_wmi_gamezone_remove, +}; + +module_wmi_driver(lenovo_wmi_gamezone_driver); + +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_gamezone_id_table); +MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>"); +MODULE_DESCRIPTION("Lenovo GameZone WMI Driver"); +MODULE_LICENSE("GPL"); diff --git a/drivers/platform/x86/lenovo-wmi.h b/drivers/platform/x86/lenovo-wmi.h new file mode 100644 index 000000000000..8a302c6c47cb --- /dev/null +++ b/drivers/platform/x86/lenovo-wmi.h @@ -0,0 +1,105 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later + * + * Lenovo Legion WMI interface driver. The Lenovo Legion WMI interface is + * broken up into multiple GUID interfaces that require cross-references + * between GUID's for some functionality. The "Custom Mode" interface is a + * legacy interface for managing and displaying CPU & GPU power and hwmon + * settings and readings. The "Other Mode" interface is a modern interface + * that replaces or extends the "Custom Mode" interface methods. The + * "GameZone" interface adds advanced features such as fan profiles and + * overclocking. The "Lighting" interface adds control of various status + * lights related to different hardware components. "Other Method" uses + * the data structs LENOVO_CAPABILITY_DATA_00, LENOVO_CAPABILITY_DATA_01 + * and LENOVO_CAPABILITY_DATA_02 structs for capability information. + * + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> + * + */ + +#define pr_fmt(fmt) "%s:%s: " fmt, KBUILD_MODNAME, __func__ + +#ifndef _LENOVO_WMI_H_ +#define _LENOVO_WMI_H_ + +#include <linux/mutex.h> +#include <linux/types.h> +#include <linux/wmi.h> + +/* Platform Profile Modes */ +#define SMARTFAN_MODE_QUIET 0x01 +#define SMARTFAN_MODE_BALANCED 0x02 +#define SMARTFAN_MODE_PERFORMANCE 0x03 +#define SMARTFAN_MODE_CUSTOM 0xFF + +struct wmi_method_args { + u32 arg0; + u32 arg1; +}; + +/* General Use functions */ +static int lenovo_wmidev_evaluate_method(struct wmi_device *wdev, u8 instance, + u32 method_id, struct acpi_buffer *in, + struct acpi_buffer *out) +{ + acpi_status status; + + status = wmidev_evaluate_method(wdev, instance, method_id, in, out); + + if (ACPI_FAILURE(status)) + return -EIO; + + return 0; +}; + +int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, + u32 method_id, u32 arg0, u32 arg1, + u32 *retval); + +int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, + u32 method_id, u32 arg0, u32 arg1, + u32 *retval) +{ + struct wmi_method_args args = { arg0, arg1 }; + struct acpi_buffer input = { (acpi_size)sizeof(args), &args }; + struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL }; + union acpi_object *ret_obj = NULL; + int err; + + err = lenovo_wmidev_evaluate_method(wdev, instance, method_id, &input, + &output); + + if (err) { + pr_err("Attempt to get method value failed.\n"); + return err; + } + + if (retval) { + ret_obj = (union acpi_object *)output.pointer; + if (!ret_obj) { + pr_err("Failed to get valid ACPI object from WMI interface\n"); + return -EIO; + } + if (ret_obj->type != ACPI_TYPE_INTEGER) { + pr_err("WMI query returnd ACPI object with wrong type.\n"); + kfree(ret_obj); + return -EIO; + } + *retval = (u32)ret_obj->integer.value; + } + + kfree(ret_obj); + + return 0; +} + +int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 instance, + u32 method_id, u32 arg0, u32 *retval); + +int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 instance, + u32 method_id, u32 arg0, u32 *retval) +{ + return lenovo_wmidev_evaluate_method_2(wdev, instance, method_id, arg0, + 0, retval); +} + +#endif /* !_LENOVO_WMI_H_ */
Adds lenovo-wmi-gamezone.c which provides a driver for the Lenovo GameZone WMI interface that comes on Lenovo "Gaming Series" hardware. Provides ACPI platform profiles over WMI. v2: - Use devm_kzalloc to ensure driver can be instanced, remove global reference. - Ensure reverse Christmas tree for all variable declarations. - Remove extra whitespace. - Use guard(mutex) in all mutex instances, global mutex. - Use pr_fmt instead of adding the driver name to each pr_err. - Remove noisy pr_info usage. - Rename gamezone_wmi to lenovo_wmi_gz_priv and gz_wmi to priv. - Remove GZ_WMI symbol exporting. Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> --- MAINTAINERS | 7 + drivers/platform/x86/Kconfig | 11 ++ drivers/platform/x86/Makefile | 1 + drivers/platform/x86/lenovo-wmi-gamezone.c | 203 +++++++++++++++++++++ drivers/platform/x86/lenovo-wmi.h | 105 +++++++++++ 5 files changed, 327 insertions(+) create mode 100644 drivers/platform/x86/lenovo-wmi-gamezone.c create mode 100644 drivers/platform/x86/lenovo-wmi.h