Re: [PATCH v3 2/4] platform/x86: Add Lenovo Gamezone WMI Driver

Armin Wolf
2025-03-11T20:21:20Z

Am 10.03.25 um 23:20 schrieb Derek John Clark:

> On Fri, Mar 7, 2025 at 4:41 PM Armin Wolf <W_Armin@gmx.de> wrote:
>> Am 07.03.25 um 23:47 schrieb Armin Wolf:
>>
>>> Am 25.02.25 um 22:59 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.
>>>>
>>>> Adds lenovo-wmi.h and lenovo-wmi.c which provide helper functions for
>>>> wmidev_evaluate_method as well as prototypes for exported functions.
>>>> v3:
>>>> - Use notifier chain to report platform profile changes to any
>>>> subscribed drivers.
>>>> - Adds THERMAL_MODE_EVENT GUID and .notify function to trigger notifier
>>>> chain.
>>>> - Adds support for Extreme Mode profile on supported hardware, as well
>>>> as a DMI quirk table for some devices that report extreme mode
>>>> version
>>>> support but so not have it fully implemented.
>>>> - Update to include recent changes to platform-profile.
>>>> v2:
>>>> - Use devm_kmalloc 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 | 3 +
>>>> drivers/platform/x86/Kconfig | 16 +
>>>> drivers/platform/x86/Makefile | 2 +
>>>> drivers/platform/x86/lenovo-wmi-gamezone.c | 374 +++++++++++++++++++++
>>>> drivers/platform/x86/lenovo-wmi.c | 77 +++++
>>>> drivers/platform/x86/lenovo-wmi.h | 62 ++++
>>>> 6 files changed, 534 insertions(+)
>>>> create mode 100644 drivers/platform/x86/lenovo-wmi-gamezone.c
>>>> create mode 100644 drivers/platform/x86/lenovo-wmi.c
>>>> create mode 100644 drivers/platform/x86/lenovo-wmi.h
>>>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index e20c32b3c480..cf7f4fce1a25 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -13157,6 +13157,9 @@ L: platform-driver-x86@vger.kernel.org
>>>> S: Maintained
>>>> F: Documentation/wmi/devices/lenovo-wmi-gamezone.rst
>>>> F: Documentation/wmi/devices/lenovo-wmi-other.rst
>>>> +F: drivers/platform/x86/lenovo-wmi-gamezone.c
>>>> +F: drivers/platform/x86/lenovo-wmi.c
>>>> +F: drivers/platform/x86/lenovo-wmi.h
>>>> LETSKETCH HID TABLET DRIVER
>>>> M: Hans de Goede <hdegoede@redhat.com>
>>>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>>>> index 7e20a58861eb..875822e6bd65 100644
>>>> --- a/drivers/platform/x86/Kconfig
>>>> +++ b/drivers/platform/x86/Kconfig
>>>> @@ -459,6 +459,22 @@ config IBM_RTL
>>>> state = 0 (BIOS SMIs on)
>>>> state = 1 (BIOS SMIs off)
>>>> +config LENOVO_WMI
>>>> + tristate
>>>> + depends on ACPI_WMI
>>> Please rename this module to LENOVO_WMI_HELPERS.
>>>
>>>> +
>>>> +config LENOVO_WMI_GAMEZONE
>>>> + tristate "Lenovo GameZone WMI Driver"
>>>> + depends on ACPI_WMI
>>>> + select ACPI_PLATFORM_PROFILE
>>>> + select LENOVO_WMI
>>>> + help
>>>> + Say Y here if you have a WMI aware Lenovo Legion device and
>>>> would like to use the
>>>> + platform-profile firmware interface to manage power usage.
>>>> +
>>>> + 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 5f6307246e69..4a7b2d14eb82 100644
>>>> --- a/drivers/platform/x86/Makefile
>>>> +++ b/drivers/platform/x86/Makefile
>>>> @@ -67,7 +67,9 @@ obj-$(CONFIG_THINKPAD_ACPI) += thinkpad_acpi.o
>>>> 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) += lenovo-wmi.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..d5329fecba47
>>>> --- /dev/null
>>>> +++ b/drivers/platform/x86/lenovo-wmi-gamezone.c
>>>> @@ -0,0 +1,374 @@
>>>> +// 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/container_of.h"
>>>> +#include "linux/printk.h"
>>>> +#include <linux/cleanup.h>
>>>> +#include <linux/dev_printk.h>
>>>> +#include <linux/dmi.h>
>>>> +#include <linux/list.h>
>>>> +#include <linux/notifier.h>
>>>> +#include <linux/platform_profile.h>
>>>> +#include <linux/types.h>
>>>> +#include <linux/wmi.h>
>>>> +#include "lenovo-wmi.h"
>>>> +
>>>> +/* Interface GUIDs */
>>>> +#define LENOVO_GAMEZONE_GUID "887B54E3-DDDC-4B2C-8B88-68A26A8835D0"
>>>> +#define THERMAL_MODE_EVENT_GUID "D320289E-8FEA-41E0-86F9-911D83151B5F"
>>>> +
>>>> +/* 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 */
>>>> +
>>>> +enum lenovo_wmi_gz_type {
>>>> + GAMEZONE_FULL = 1,
>>>> + THERMAL_MODE,
>>>> +};
>>>> +
>>>> +#define GAMEZONE_WMI_DEVICE(guid, type) \
>>>> + .guid_string = (guid), .context = &(enum lenovo_wmi_gz_type) \
>>>> + { \
>>>> + type \
>>>> + }
>>>> +
>>>> +static BLOCKING_NOTIFIER_HEAD(gz_chain_head);
>>>> +static DEFINE_MUTEX(gz_chain_mutex);
>>>> +
>>>> +struct lenovo_wmi_gz_priv {
>>>> + enum platform_profile_option current_profile;
>>> This variable is only assigned and never read, please remove it.
>>>
>>>> + struct wmi_device *wdev;
>>>> + bool extreme_supported;
>>>> + struct device *ppdev; /*platform profile device */
>>>> + enum lenovo_wmi_gz_type type;
>>>> + struct blocking_notifier_head nhead;
>>>> +};
>>>> +
>>>> +struct quirk_entry {
>>>> + bool extreme_supported;
>>>> +};
>>>> +
>>>> +static struct quirk_entry quirk_no_extreme_bug = {
>>>> + .extreme_supported = false,
>>>> +};
>>> Can you make this const?
>>>
>>>> +
>>>> +/* Platform Profile Methods & Setup */
>>>> +static int
>>>> +lenovo_wmi_gz_platform_profile_supported(struct lenovo_wmi_gz_priv
>>>> *priv,
>>>> + int *supported)
>>>> +{
>>>> + return lenovo_wmidev_evaluate_method_1(priv->wdev, 0x0,
>>>> + WMI_METHOD_ID_SMARTFAN_SUPP, 0, supported);
>>>> +}
>>>> +
>>>> +static int lenovo_wmi_gz_profile_get(struct device *dev,
>>>> + enum platform_profile_option *profile)
>>>> +{
>>>> + struct lenovo_wmi_gz_priv *priv = dev_get_drvdata(dev);
>>>> + int sel_prof;
>>>> + int ret;
>>>> +
>>>> + ret = lenovo_wmidev_evaluate_method_1(priv->wdev, 0x0,
>>>> + WMI_METHOD_ID_SMARTFAN_GET, 0, &sel_prof);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + switch (sel_prof) {
>>>> + case SMARTFAN_MODE_QUIET:
>>>> + *profile = PLATFORM_PROFILE_LOW_POWER;
>>>> + break;
>>>> + case SMARTFAN_MODE_BALANCED:
>>>> + *profile = PLATFORM_PROFILE_BALANCED;
>>>> + break;
>>>> + case SMARTFAN_MODE_PERFORMANCE:
>>>> + if (priv->extreme_supported) {
>>>> + *profile = PLATFORM_PROFILE_BALANCED_PERFORMANCE;
>>>> + break;
>>>> + }
>>>> + *profile = PLATFORM_PROFILE_PERFORMANCE;
>>>> + break;
>>>> + case SMARTFAN_MODE_EXTREME:
>>>> + *profile = PLATFORM_PROFILE_PERFORMANCE;
>>>> + break;
>>>> + case SMARTFAN_MODE_CUSTOM:
>>>> + *profile = PLATFORM_PROFILE_CUSTOM;
>>>> + break;
>>>> + default:
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + priv->current_profile = *profile;
>>>> +
>>>> + ret = blocking_notifier_call_chain(&gz_chain_head,
>>>> THERMAL_MODE_EVENT,
>>>> + &sel_prof);
>>> Is it really necessary to call the notifier here? AFAIK the notifier
>>> needs to be called
>>> only:
>>>
>>> - when the platform profile was changed either by the user or the
>>> firmware.
>>> - when a new notifier handler was registered so that the handler does
>>> not have to wait for the next user input
>>>
>>> Please only call the notifier in those two situations.
>> I thought about this a bit and i came to the conclusion that my proposal would not work. The problem is that if no
>> gamezone driver is available the notifier handler will not be called at all.
>>
>> I instead suggest that you move the notifier inside the lenovo-wmi-other driver. Then the gamezone driver registers itself
>> with this notifier and supplies the current platform profile if the lenovo-wmi-other driver requests it. In this case
>> it would make sense to cache the current platform profile.
> I have a few questions as I want to make sure I understand how to
> "reverse" the information flow for the notifier chain.
> - Would it be as simple as filling the data pointer sent by other with
> a pointer to the profile in gamezone? If so, would gamezone return
> NOTIFY_STOP or NOTIFY_DONE to indicate the pointer was filled so that
> other could verify?

Yes, the data pointer would ideally point to a variable with a type of "enum platform_profile_option" inside the
lenovo-wmi-other driver which will then be modified by the gamezone driver.

> - Since other might be initialized before gamezone, should I do a
> notifier call chain every time I need to know the platform?

Yes.

> - How would the WMI events notifications work as well, since currently
> they are what calls the chain and other has no context for that.

WMI events that change the platform profile are of no concern to the lenovo-wmi-other driver because
he always asks for the current platform profile before performing an operation anyway.

The notifier used by the WMI event driver can be constructed like the notifier inside the uniwill-wmi
driver available at https://github.com/Wer-Wolf/uniwill-laptop.

Thanks,
Armin Wolf

> Cheers,
> - Derek
>
>> Thanks,
>> Armin Wolf
>>
>>>> + if (ret == NOTIFY_BAD)
>>>> + pr_err("Failed to send notification to call chain for WMI
>>>> event %u\n",
>>>> + priv->type);
>>> Use dev_err() here please.
>>>
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int lenovo_wmi_gz_profile_set(struct device *dev,
>>>> + enum platform_profile_option profile)
>>>> +{
>>>> + struct lenovo_wmi_gz_priv *priv = dev_get_drvdata(dev);
>>>> + int sel_prof;
>>>> + int ret;
>>>> +
>>>> + switch (profile) {
>>>> + case PLATFORM_PROFILE_LOW_POWER:
>>>> + sel_prof = SMARTFAN_MODE_QUIET;
>>>> + break;
>>>> + case PLATFORM_PROFILE_BALANCED:
>>>> + sel_prof = SMARTFAN_MODE_BALANCED;
>>>> + break;
>>>> + case PLATFORM_PROFILE_BALANCED_PERFORMANCE:
>>>> + sel_prof = SMARTFAN_MODE_PERFORMANCE;
>>>> + break;
>>>> + case PLATFORM_PROFILE_PERFORMANCE:
>>>> + if (priv->extreme_supported) {
>>>> + sel_prof = SMARTFAN_MODE_EXTREME;
>>>> + break;
>>>> + }
>>>> + sel_prof = SMARTFAN_MODE_PERFORMANCE;
>>>> + break;
>>>> + case PLATFORM_PROFILE_CUSTOM:
>>>> + sel_prof = SMARTFAN_MODE_CUSTOM;
>>>> + break;
>>>> + default:
>>>> + return -EOPNOTSUPP;
>>>> + }
>>>> +
>>>> + ret = lenovo_wmidev_evaluate_method_1(priv->wdev, 0x0,
>>>> + WMI_METHOD_ID_SMARTFAN_SET, sel_prof, NULL);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static const struct dmi_system_id fwbug_list[] = {
>>>> + {
>>>> + .ident = "Legion Go 8APU1",
>>>> + .matches = {
>>>> + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>>>> + DMI_MATCH(DMI_PRODUCT_VERSION, "Legion Go 8APU1"),
>>>> + },
>>>> + .driver_data = &quirk_no_extreme_bug,
>>>> + },
>>>> + {
>>>> + .ident = "Legion Go S 8ARP1",
>>>> + .matches = {
>>>> + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>>>> + DMI_MATCH(DMI_PRODUCT_VERSION, "Legion Go S 8ARP1"),
>>>> + },
>>>> + .driver_data = &quirk_no_extreme_bug,
>>>> + },
>>>> + {
>>>> + .ident = "Legion Go S 8APU1",
>>>> + .matches = {
>>>> + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>>>> + DMI_MATCH(DMI_PRODUCT_VERSION, "Legion Go S 8APU1"),
>>>> + },
>>>> + .driver_data = &quirk_no_extreme_bug,
>>>> + },
>>>> + {},
>>>> +
>>>> +};
>>>> +
>>>> +static bool extreme_supported(int profile_support_ver)
>>>> +{
>>>> + const struct dmi_system_id *dmi_id;
>>>> + struct quirk_entry *quirks;
>>>> +
>>>> + if (profile_support_ver < 6)
>>>> + return false;
>>>> +
>>>> + dmi_id = dmi_first_match(fwbug_list);
>>>> + if (!dmi_id)
>>>> + return true;
>>>> +
>>>> + quirks = dmi_id->driver_data;
>>>> + return quirks->extreme_supported;
>>>> +}
>>>> +
>>>> +static int lenovo_wmi_platform_profile_probe(void *drvdata,
>>>> + unsigned long *choices)
>>>> +{
>>>> + struct lenovo_wmi_gz_priv *priv = drvdata;
>>>> + enum platform_profile_option profile;
>>> Unused variable, please remove.
>>>
>>>> + int profile_support_ver;
>>>> + int ret;
>>>> +
>>>> + ret = lenovo_wmi_gz_platform_profile_supported(priv,
>>>> + &profile_support_ver);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + if (profile_support_ver < 1)
>>>> + return -ENODEV;
>>>> +
>>>> + priv->extreme_supported = extreme_supported(profile_support_ver);
>>>> +
>>>> + set_bit(PLATFORM_PROFILE_LOW_POWER, choices);
>>>> + set_bit(PLATFORM_PROFILE_BALANCED, choices);
>>>> + set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
>>>> + set_bit(PLATFORM_PROFILE_CUSTOM, choices);
>>>> +
>>>> + if (priv->extreme_supported)
>>>> + set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE, choices);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static const struct platform_profile_ops
>>>> lenovo_wmi_gz_platform_profile_ops = {
>>>> + .probe = lenovo_wmi_platform_profile_probe,
>>>> + .profile_get = lenovo_wmi_gz_profile_get,
>>>> + .profile_set = lenovo_wmi_gz_profile_set,
>>>> +};
>>>> +
>>>> +/* Notifier Methods */
>>>> +int lenovo_wmi_gz_register_notifier(struct notifier_block *nb)
>>>> +{
>>>> + guard(mutex)(&gz_chain_mutex);
>>> The blocking notifier already does the locking itself. Please remove
>>> this mutex.
>>>
>>>> + return blocking_notifier_chain_register(&gz_chain_head, nb);
>>>> +}
>>>> +EXPORT_SYMBOL_NS_GPL(lenovo_wmi_gz_register_notifier, "GZ_WMI");
>>> Can you name the namespace similar to "LENOVO_GAMEZONE_WMI" please?
>>>
>>>> +
>>>> +int lenovo_wmi_gz_unregister_notifier(struct notifier_block *nb)
>>>> +{
>>>> + guard(mutex)(&gz_chain_mutex);
>>>> + return blocking_notifier_chain_unregister(&gz_chain_head, nb);
>>>> +}
>>>> +EXPORT_SYMBOL_NS_GPL(lenovo_wmi_gz_unregister_notifier, "GZ_WMI");
>>>> +
>>>> +static void devm_lenovo_wmi_gz_unregister_notifier(void *data)
>>>> +{
>>>> + struct notifier_block *nb = data;
>>>> +
>>>> + lenovo_wmi_gz_unregister_notifier(nb);
>>>> +}
>>>> +
>>>> +int devm_lenovo_wmi_gz_register_notifier(struct device *dev,
>>>> + struct notifier_block *nb)
>>>> +{
>>>> + int ret;
>>>> +
>>>> + ret = lenovo_wmi_gz_register_notifier(nb);
>>>> + if (ret < 0)
>>>> + return ret;
>>>> +
>>>> + return devm_add_action_or_reset(dev,
>>>> devm_lenovo_wmi_gz_unregister_notifier, nb);
>>>> +}
>>>> +EXPORT_SYMBOL_NS_GPL(devm_lenovo_wmi_gz_register_notifier, "GZ_WMI");
>>>> +
>>>> +/* Driver Methods */
>>>> +static void lenovo_wmi_gz_notify(struct wmi_device *wdev,
>>>> + union acpi_object *obj)
>>>> +{
>>>> + struct lenovo_wmi_gz_priv *tm_priv = dev_get_drvdata(&wdev->dev);
>>>> + struct lenovo_wmi_gz_priv *gz_priv =
>>>> + container_of(&gz_chain_head, struct lenovo_wmi_gz_priv, nhead);
>>> I fear that this will not work because gz_chain_head is a global
>>> variable, not a field inside
>>> struct lenovo_wmi_gz_priv. Also this would crash the kernel should the
>>> main gamezone driver be
>>> unbound from its WMI device.
>>>
>>> I suggest you move the WMI driver for the WMI event into a separate
>>> module. Then you use another notifier
>>> inside the new module to allow the gamezone driver to listen for
>>> events. For example this separate WMI event driver
>>> could use the "val" argument inside blocking_notifier_call_chain() to
>>> specify the type of event (like THERMAL_MODE_CHANGED)
>>> and the "v" argument to pass a pointer to a u32 variable containing
>>> the new thermal mode.
>>>
>>> This also allows you to extend the separate WMI driver later to
>>> support more WMI event GUIDs.
>>>
>>>> + int sel_prof;
>>>> + int ret;
>>>> +
>>>> + if (obj->type != ACPI_TYPE_INTEGER)
>>>> + return;
>>>> +
>>>> + switch (tm_priv->type) {
>>>> + case THERMAL_MODE:
>>>> + sel_prof = obj->integer.value;
>>>> + break;
>>>> + default:
>>>> + return;
>>>> + }
>>>> +
>>>> + /* Update primary Gamezone instance */
>>>> + switch (sel_prof) {
>>>> + case SMARTFAN_MODE_QUIET:
>>>> + gz_priv->current_profile = PLATFORM_PROFILE_LOW_POWER;
>>>> + break;
>>>> + case SMARTFAN_MODE_BALANCED:
>>>> + gz_priv->current_profile = PLATFORM_PROFILE_BALANCED;
>>>> + break;
>>>> + case SMARTFAN_MODE_PERFORMANCE:
>>>> + if (gz_priv->extreme_supported) {
>>>> + gz_priv->current_profile =
>>>> + PLATFORM_PROFILE_BALANCED_PERFORMANCE;
>>>> + break;
>>>> + }
>>>> + gz_priv->current_profile = PLATFORM_PROFILE_PERFORMANCE;
>>>> + break;
>>>> + case SMARTFAN_MODE_EXTREME:
>>>> + gz_priv->current_profile = PLATFORM_PROFILE_PERFORMANCE;
>>>> + break;
>>>> + case SMARTFAN_MODE_CUSTOM:
>>>> + gz_priv->current_profile = PLATFORM_PROFILE_CUSTOM;
>>>> + break;
>>>> + default:
>>>> + break;
>>>> + }
>>> Please use platform_profile_notify() to notify userspace of the new
>>> platform profile settings.
>>>
>>>> +
>>>> + ret = blocking_notifier_call_chain(&gz_chain_head,
>>>> THERMAL_MODE_EVENT,
>>>> + &sel_prof);
>>>> + if (ret == NOTIFY_BAD)
>>>> + pr_err("Failed to send notification to call chain for WMI
>>>> event %u\n",
>>>> + tm_priv->type);
>>>> +}
>>>> +
>>>> +static int lenovo_wmi_gz_probe(struct wmi_device *wdev, const void
>>>> *context)
>>>> +{
>>>> + struct lenovo_wmi_gz_priv *priv =
>>>> + devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL);
>>> Please do the call to devm_kzalloc() on a separate line:
>>>
>>> struct lenovo_wmi_gz_priv *priv;
>>>
>>> priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL);
>>>
>>>> +
>>>> + if (!priv)
>>>> + return -ENOMEM;
>>>> +
>>>> + if (!context)
>>>> + return -EINVAL;
>>>> +
>>>> + priv->wdev = wdev;
>>>> + priv->type = *((enum lenovo_wmi_gz_type *)context);
>>>> +
>>>> + dev_set_drvdata(&wdev->dev, priv);
>>>> +
>>>> + if (priv->type != GAMEZONE_FULL)
>>>> + return 0;
>>>> +
>>>> + priv->nhead = gz_chain_head;
>>>> + priv->ppdev = platform_profile_register(&wdev->dev,
>>>> "lenovo-wmi-gamezone",
>>>> + priv, &lenovo_wmi_gz_platform_profile_ops);
>>> Please check if platform_profile_register() was successful and return
>>> an error if not.
>>>
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static const struct wmi_device_id lenovo_wmi_gz_id_table[] = {
>>>> + { GAMEZONE_WMI_DEVICE(LENOVO_GAMEZONE_GUID, GAMEZONE_FULL) },
>>>> + { GAMEZONE_WMI_DEVICE(THERMAL_MODE_EVENT_GUID, THERMAL_MODE) },
>>>> + {}
>>>> +};
>>>> +
>>>> +static struct wmi_driver lenovo_wmi_gz_driver = {
>>>> + .driver = {
>>>> + .name = "lenovo_wmi_gamezone",
>>>> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
>>>> + },
>>>> + .id_table = lenovo_wmi_gz_id_table,
>>>> + .probe = lenovo_wmi_gz_probe,
>>>> + .notify = lenovo_wmi_gz_notify,
>>>> + .no_singleton = true,
>>>> +};
>>>> +
>>>> +module_wmi_driver(lenovo_wmi_gz_driver);
>>>> +
>>>> +MODULE_IMPORT_NS("LENOVO_WMI");
>>>> +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_gz_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.c
>>>> b/drivers/platform/x86/lenovo-wmi.c
>>>> new file mode 100644
>>>> index 000000000000..0de2c37e69bd
>>>> --- /dev/null
>>>> +++ b/drivers/platform/x86/lenovo-wmi.c
>>>> @@ -0,0 +1,77 @@
>>>> +// 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 Method"
>>>> 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 Method" 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 Mode" 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>
>>>> + *
>>>> + */
>>>> +
>>>> +#include <linux/wmi.h>
>>>> +#include "lenovo-wmi.h"
>>>> +
>>>> +/* wmidev_evaluate_method helper 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)
>>>> +{
>>> Please give this method a more descriptive name.
>>>
>>>> + struct wmi_method_args args = { arg0, arg1 };
>>>> + struct acpi_buffer input = { (acpi_size)sizeof(args), &args };
>>> Cast to acpi_size is unnecessary here.
>>>
>>>> + struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>>>> + union acpi_object *ret_obj __free(kfree) = NULL;
>>>> + int err;
>>>> +
>>>> + err = lenovo_wmidev_evaluate_method(wdev, instance, method_id,
>>>> &input,
>>>> + &output);
>>>> +
>>>> + if (err)
>>>> + return err;
>>>> +
>>>> + if (retval) {
>>>> + ret_obj = (union acpi_object *)output.pointer;
>>>> + if (!ret_obj)
>>>> + return -ENODATA;
>>>> +
>>>> + if (ret_obj->type != ACPI_TYPE_INTEGER)
>>>> + return -ENXIO;
>>>> +
>>>> + *retval = (u32)ret_obj->integer.value;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +EXPORT_SYMBOL_NS_GPL(lenovo_wmidev_evaluate_method_2, "LENOVO_WMI");
>>> Can you please rename the namespace to "LENOVO_WMI_HELPERS"?
>>>
>>>> +
>>>> +int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8
>>>> instance,
>>>> + u32 method_id, u32 arg0, u32 *retval)
>>>> +{
>>> Same as above.
>>>
>>>> + return lenovo_wmidev_evaluate_method_2(wdev, instance,
>>>> method_id, arg0,
>>>> + 0, retval);
>>>> +}
>>>> +EXPORT_SYMBOL_NS_GPL(lenovo_wmidev_evaluate_method_1, "LENOVO_WMI");
>>>> +MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>");
>>>> +MODULE_DESCRIPTION("Lenovo WMI Common 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..113928b4fc0f
>>>> --- /dev/null
>>>> +++ b/drivers/platform/x86/lenovo-wmi.h
>>>> @@ -0,0 +1,62 @@
>>>> +/* 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 Method"
>>>> 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 Method" 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 Mode" 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>
>>>> + *
>>>> + */
>>>> +#include <linux/notifier.h>
>>>> +#include <linux/platform_profile.h>
>>>> +
>>>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>> This causes a build error on my machine. Please only use this macro
>>> inside source files
>>> for modules and not inside header files.
>>>
>>>> +
>>>> +#ifndef _LENOVO_WMI_H_
>>>> +#define _LENOVO_WMI_H_
>>>> +
>>>> +#include <linux/bits.h>
>>>> +#include <linux/types.h>
>>>> +#include <linux/wmi.h>
>>>> +
>>>> +struct wmi_method_args {
>>>> + u32 arg0;
>>>> + u32 arg1;
>>>> +};
>>>> +
>>>> +/* gamezone structs */
>>>> +enum thermal_mode {
>>>> + SMARTFAN_MODE_QUIET = 0x01,
>>>> + SMARTFAN_MODE_BALANCED = 0x02,
>>>> + SMARTFAN_MODE_PERFORMANCE = 0x03,
>>>> + SMARTFAN_MODE_EXTREME = 0xE0, /* Ver 6+ */
>>>> + SMARTFAN_MODE_CUSTOM = 0xFF,
>>>> +};
>>>> +
>>>> +enum lenovo_wmi_action {
>>>> + THERMAL_MODE_EVENT = 1,
>>>> +};
>>>> +
>>>> +/* wmidev_evaluate_method helper functions */
>>>> +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_1(struct wmi_device *wdev, u8
>>>> instance,
>>>> + u32 method_id, u32 arg0, u32 *retval);
>>>> +
>>>> +/* lenovo_wmi_gz_driver notifier functions */
>>>> +int lenovo_wmi_gz_notifier_call(struct notifier_block *nb, unsigned
>>>> long action,
>>>> + enum platform_profile_option *profile);
>>>> +int lenovo_wmi_gz_register_notifier(struct notifier_block *nb);
>>>> +int lenovo_wmi_gz_unregister_notifier(struct notifier_block *nb);
>>>> +int devm_lenovo_wmi_gz_register_notifier(struct device *dev,
>>>> + struct notifier_block *nb);
>>> Can you please create a separate header file for each driver?
>>> Otherwise this header file
>>> will contain many different things from different drivers, which will
>>> maybe not even be
>>> available depending on the Kconfig settings.
>>>
>>> Thanks,
>>> Armin Wolf
>>>
>>>> +#endif /* !_LENOVO_WMI_H_ */