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;
+};
+
+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_ */
--
2.47.0On 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.
> +
> +#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_ */
>
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 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 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 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
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 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.
© 2016 - 2026 Red Hat, Inc.