| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAFqHKTm-T5G3ZyQwjbO3KSaq=T1wS6vSGWxzsuzvqGmoTLj_hg@mail.gmail.com>
Date: Mon, 10 Mar 2025 15:20:07 -0700
From: Derek John Clark <derekjohn.clark@...il.com>
To: Armin Wolf <W_Armin@....de>
Cc: Hans de Goede <hdegoede@...hat.com>, Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
Jonathan Corbet <corbet@....net>, Mario Limonciello <superm1@...nel.org>, Luke Jones <luke@...nes.dev>,
Xino Ni <nijs1@...ovo.com>, Zhixin Zhang <zhangzx36@...ovo.com>, Mia Shao <shaohz1@...ovo.com>,
Mark Pearson <mpearson-lenovo@...ebb.ca>,
"Pierre-Loup A . Griffais" <pgriffais@...vesoftware.com>, "Cody T . -H . Chiu" <codyit@...il.com>,
John Martens <johnfanv2@...il.com>, platform-driver-x86@...r.kernel.org,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/4] platform/x86: Add Lenovo Gamezone WMI Driver
On Fri, Mar 7, 2025 at 4:41 PM Armin Wolf <W_Armin@....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@...il.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@...r.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@...hat.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@...il.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?
- Since other might be initialized before gamezone, should I do a
notifier call chain every time I need to know the platform?
- How would the WMI events notifications work as well, since currently
they are what calls the chain and other has no context for that.
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@...il.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@...il.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@...il.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@...il.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_ */
Powered by blists - more mailing lists