Re: [PATCH] platform/x86: lenovo: Decouple lenovo-wmi-gamezone and lenovo-wmi-other

[Rong Zhang]

Hi Derek,

On Mon, 2026-03-30 at 13:04 -0700, Derek John Clark wrote:
> On Fri, Mar 27, 2026 at 8:21 AM Rong Zhang <i@xxxxxxxx> wrote:
> > 
> > Hi Ilpo,
> > 
> > On Fri, 2026-03-27 at 13:25 +0200, Ilpo Järvinen wrote:
> > > On Fri, 27 Mar 2026, Rong Zhang wrote:
> > > 
> > > > Currently, lenovo-wmi-gamezone depends on lenovo-wmi-other as the former
> > > > imports symbols from the latter. The imported symbols are just used to
> > > > register a notifier block. However, there is no runtime dependency
> > > > between both drivers, and either of them can run without the other,
> > > > which is the major purpose of using the notifier framework.
> > > > 
> > > > Such a link-time dependency is non-optimal. A previous attempt to "fix"
> > > > it made LENOVO_WMI_GAMEZONE select LENOVO_WMI_TUNING, which was
> > > > fundamentally broken and resulted in undefined Kconfig behavior, as
> > > > `select' cannot be used on a symbol with potentially unmet dependencies.
> > > > 
> > > > Decouple both drivers by moving the thermal mode notifier chain to
> > > > lenovo-wmi-helpers. Methods for notifier block (un)registration are
> > > > exported for lenovo-wmi-gamezone, while a method for querying the
> > > > current thermal mode are exported for lenovo-wmi-other.
> > > > 
> > > > This turns the dependency graph from
> > > > 
> > > >             +------------ lenovo-wmi-gamezone
> > > >             |                     |
> > > >             v                     |
> > > >     lenovo-wmi-helpers            |
> > > >             ^                     |
> > > >             |                     V
> > > >             +------------ lenovo-wmi-other
> > > > 
> > > > into
> > > > 
> > > >             +------------ lenovo-wmi-gamezone
> > > >             |
> > > >             v
> > > >     lenovo-wmi-helpers
> > > >             ^
> > > >             |
> > > >             +------------ lenovo-wmi-other
> > > > 
> > > > To make it clear, the name of the notifier chain is also renamed from
> > > > `om_chain_head' to `tm_chain_head', indicating that it's used to query
> > > > the current thermal mode.
> > > > 
> > > > No functional change intended.
> > > > 
> > > > Fixes: 6e38b9fcbfa3 ("platform/x86: lenovo: gamezone needs "other mode"")
> > > > Cc: stable@xxxxxxxxxxxxxxx
> > > > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > > > Closes: https://lore.kernel.org/oe-kbuild-all/202603252259.gHvJDyh3-lkp@xxxxxxxxx/
> > > > Closes: https://lore.kernel.org/oe-kbuild-all/202603260302.X0NjQOda-lkp@xxxxxxxxx/
> > > > Signed-off-by: Rong Zhang <i@xxxxxxxx>
> > > > ---
> > > >  drivers/platform/x86/lenovo/Kconfig        |   1 -
> > > >  drivers/platform/x86/lenovo/wmi-gamezone.c |   4 +-
> > > >  drivers/platform/x86/lenovo/wmi-helpers.c  | 102 ++++++++++++++++++++
> > > >  drivers/platform/x86/lenovo/wmi-helpers.h  |   8 ++
> > > >  drivers/platform/x86/lenovo/wmi-other.c    | 104 +--------------------
> > > >  drivers/platform/x86/lenovo/wmi-other.h    |  16 ----
> > > >  6 files changed, 113 insertions(+), 122 deletions(-)
> > > >  delete mode 100644 drivers/platform/x86/lenovo/wmi-other.h
> > > > 
> > > > diff --git a/drivers/platform/x86/lenovo/Kconfig b/drivers/platform/x86/lenovo/Kconfig
> > > > index f885127b007f..09b1b055d2e0 100644
> > > > --- a/drivers/platform/x86/lenovo/Kconfig
> > > > +++ b/drivers/platform/x86/lenovo/Kconfig
> > > > @@ -252,7 +252,6 @@ config LENOVO_WMI_GAMEZONE
> > > >     select ACPI_PLATFORM_PROFILE
> > > >     select LENOVO_WMI_EVENTS
> > > >     select LENOVO_WMI_HELPERS
> > > > -   select LENOVO_WMI_TUNING
> > > >     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.
> > > > diff --git a/drivers/platform/x86/lenovo/wmi-gamezone.c b/drivers/platform/x86/lenovo/wmi-gamezone.c
> > > > index c7fe7e3c9f17..92020225db27 100644
> > > > --- a/drivers/platform/x86/lenovo/wmi-gamezone.c
> > > > +++ b/drivers/platform/x86/lenovo/wmi-gamezone.c
> > > > @@ -23,7 +23,6 @@
> > > >  #include "wmi-events.h"
> > > >  #include "wmi-gamezone.h"
> > > >  #include "wmi-helpers.h"
> > > > -#include "wmi-other.h"
> > > > 
> > > >  #define LENOVO_GAMEZONE_GUID "887B54E3-DDDC-4B2C-8B88-68A26A8835D0"
> > > > 
> > > > @@ -383,7 +382,7 @@ static int lwmi_gz_probe(struct wmi_device *wdev, const void *context)
> > > >             return ret;
> > > > 
> > > >     priv->mode_nb.notifier_call = lwmi_gz_mode_call;
> > > > -   return devm_lwmi_om_register_notifier(&wdev->dev, &priv->mode_nb);
> > > > +   return devm_lwmi_tm_register_notifier(&wdev->dev, &priv->mode_nb);
> > > >  }
> > > > 
> > > >  static const struct wmi_device_id lwmi_gz_id_table[] = {
> > > > @@ -405,7 +404,6 @@ module_wmi_driver(lwmi_gz_driver);
> > > > 
> > > >  MODULE_IMPORT_NS("LENOVO_WMI_EVENTS");
> > > >  MODULE_IMPORT_NS("LENOVO_WMI_HELPERS");
> > > > -MODULE_IMPORT_NS("LENOVO_WMI_OTHER");
> > > >  MODULE_DEVICE_TABLE(wmi, lwmi_gz_id_table);
> > > >  MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@xxxxxxxxx>");
> > > >  MODULE_DESCRIPTION("Lenovo GameZone WMI Driver");
> > > > diff --git a/drivers/platform/x86/lenovo/wmi-helpers.c b/drivers/platform/x86/lenovo/wmi-helpers.c
> > > > index 7379defac500..5a88bccb5037 100644
> > > > --- a/drivers/platform/x86/lenovo/wmi-helpers.c
> > > > +++ b/drivers/platform/x86/lenovo/wmi-helpers.c
> > > > @@ -21,11 +21,16 @@
> > > >  #include <linux/errno.h>
> > > >  #include <linux/export.h>
> > > >  #include <linux/module.h>
> > > > +#include <linux/notifier.h>
> > > >  #include <linux/unaligned.h>
> > > >  #include <linux/wmi.h>
> > > > 
> > > > +#include "wmi-gamezone.h"
> > > >  #include "wmi-helpers.h"
> > > > 
> > > > +/* Thermal mode notifier chain. */
> > > > +static BLOCKING_NOTIFIER_HEAD(tm_chain_head);
> > > > +
> > > >  /**
> > > >   * lwmi_dev_evaluate_int() - Helper function for calling WMI methods that
> > > >   * return an integer.
> > > > @@ -84,6 +89,103 @@ int lwmi_dev_evaluate_int(struct wmi_device *wdev, u8 instance, u32 method_id,
> > > >  };
> > > >  EXPORT_SYMBOL_NS_GPL(lwmi_dev_evaluate_int, "LENOVO_WMI_HELPERS");
> > > > 
> > > > +/**
> > > > + * lwmi_tm_register_notifier() - Add a notifier to the blocking notifier chain
> > > > + * @nb: The notifier_block struct to register
> > > > + *
> > > > + * Call blocking_notifier_chain_register to register the notifier block to the
> > > > + * thermal mode notifier chain.
> > > > + *
> > > > + * Return: 0 on success, %-EEXIST on error.
> > > > + */
> > > > +int lwmi_tm_register_notifier(struct notifier_block *nb)
> > > > +{
> > > > +   return blocking_notifier_chain_register(&tm_chain_head, nb);
> > > > +}
> > > > +EXPORT_SYMBOL_NS_GPL(lwmi_tm_register_notifier, "LENOVO_WMI_HELPERS");
> > > > +
> > > > +/**
> > > > + * lwmi_tm_unregister_notifier() - Remove a notifier from the blocking notifier
> > > > + * chain.
> > > > + * @nb: The notifier_block struct to register
> > > > + *
> > > > + * Call blocking_notifier_chain_unregister to unregister the notifier block from the
> > > > + * thermal mode notifier chain.
> > > > + *
> > > > + * Return: 0 on success, %-ENOENT on error.
> > > > + */
> > > > +int lwmi_tm_unregister_notifier(struct notifier_block *nb)
> > > > +{
> > > > +   return blocking_notifier_chain_unregister(&tm_chain_head, nb);
> > > > +}
> > > > +EXPORT_SYMBOL_NS_GPL(lwmi_tm_unregister_notifier, "LENOVO_WMI_HELPERS");
> > > > +
> > > > +/**
> > > > + * devm_lwmi_tm_unregister_notifier() - Remove a notifier from the blocking
> > > > + * notifier chain.
> > > > + * @data: Void pointer to the notifier_block struct to register.
> > > > + *
> > > > + * Call lwmi_tm_unregister_notifier to unregister the notifier block from the
> > > > + * thermal mode notifier chain.
> > > > + *
> > > > + * Return: 0 on success, %-ENOENT on error.
> > > > + */
> > > > +static void devm_lwmi_tm_unregister_notifier(void *data)
> > > > +{
> > > > +   struct notifier_block *nb = data;
> > > > +
> > > > +   lwmi_tm_unregister_notifier(nb);
> > > > +}
> > > > +
> > > > +/**
> > > > + * devm_lwmi_tm_register_notifier() - Add a notifier to the blocking notifier
> > > > + * chain.
> > > > + * @dev: The parent device of the notifier_block struct.
> > > > + * @nb: The notifier_block struct to register
> > > > + *
> > > > + * Call lwmi_tm_register_notifier to register the notifier block to the
> > > > + * thermal mode notifier chain. Then add devm_lwmi_tm_unregister_notifier
> > > > + * as a device managed action to automatically unregister the notifier block
> > > > + * upon parent device removal.
> > > > + *
> > > > + * Return: 0 on success, or an error code.
> > > > + */
> > > > +int devm_lwmi_tm_register_notifier(struct device *dev,
> > > > +                              struct notifier_block *nb)
> > > > +{
> > > > +   int ret;
> > > > +
> > > > +   ret = lwmi_tm_register_notifier(nb);
> > > > +   if (ret < 0)
> > > > +           return ret;
> > > > +
> > > > +   return devm_add_action_or_reset(dev, devm_lwmi_tm_unregister_notifier,
> > > > +                                   nb);
> > > > +}
> > > > +EXPORT_SYMBOL_NS_GPL(devm_lwmi_tm_register_notifier, "LENOVO_WMI_HELPERS");
> > > > +
> > > > +/**
> > > > + * lwmi_tm_notifier_call() - Call functions for the notifier call chain.
> > > > + * @mode: Pointer to a thermal mode enum to retrieve the data from.
> > > > + *
> > > > + * Call blocking_notifier_call_chain to retrieve the thermal mode from the
> > > > + * lenovo-wmi-gamezone driver.
> > > > + *
> > > > + * Return: 0 on success, or an error code.
> > > > + */
> > > > +int lwmi_tm_notifier_call(enum thermal_mode *mode)
> > > > +{
> > > > +   int ret;
> > > > +
> > > > +   ret = blocking_notifier_call_chain(&tm_chain_head,
> > > > +                                      LWMI_GZ_GET_THERMAL_MODE, &mode);
> > > > +   if ((ret & ~NOTIFY_STOP_MASK) != NOTIFY_OK)
> > > > +           return -EINVAL;
> > > > +
> > > > +   return 0;
> > > > +}
> > > > +EXPORT_SYMBOL_NS_GPL(lwmi_tm_notifier_call, "LENOVO_WMI_HELPERS");
> > > > +
> > > >  MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@xxxxxxxxx>");
> > > >  MODULE_DESCRIPTION("Lenovo WMI Helpers Driver");
> > > >  MODULE_LICENSE("GPL");
> > > > diff --git a/drivers/platform/x86/lenovo/wmi-helpers.h b/drivers/platform/x86/lenovo/wmi-helpers.h
> > > > index 20fd21749803..651a039228ed 100644
> > > > --- a/drivers/platform/x86/lenovo/wmi-helpers.h
> > > > +++ b/drivers/platform/x86/lenovo/wmi-helpers.h
> > > > @@ -7,6 +7,8 @@
> > > > 
> > > >  #include <linux/types.h>
> > > > 
> > > > +struct device;
> > > > +struct notifier_block;
> > > >  struct wmi_device;
> > > > 
> > > >  struct wmi_method_args_32 {
> > > > @@ -17,4 +19,10 @@ struct wmi_method_args_32 {
> > > >  int lwmi_dev_evaluate_int(struct wmi_device *wdev, u8 instance, u32 method_id,
> > > >                       unsigned char *buf, size_t size, u32 *retval);
> > > > 
> > > > +int lwmi_tm_register_notifier(struct notifier_block *nb);
> > > > +int lwmi_tm_unregister_notifier(struct notifier_block *nb);
> > > > +int devm_lwmi_tm_register_notifier(struct device *dev,
> > > > +                              struct notifier_block *nb);
> > > > +int lwmi_tm_notifier_call(enum thermal_mode *mode);
> > > 
> > > This enum is not introduced earlier within this header?
> > 
> > Hmm, no. Declaring a opaque enum earlier should be enough to fix it, as
> > wmi-gamezone.h shouldn't be included here. Derek, what do you think?
> 
> I think it makes more sense at this point to move everything from
> wmi_gamezone.h into wmi_helpers.h. This prevents wmi_capdata.c from
> needing to import wmi_gamezone.h, since the thermal mode enum will be
> moved (now used by capdata, gamezone, and other). Then the only thing
> in the gamezone header would be the gamezone_events_type enum, which
> is only used by gamezone and helpers anyway, so that can safely move
> and we can delete the entire file. To make that enum name more generic
> I'll rename it in the move from gamezone_events_type to
> lwmi_event_type (the enum isn't directly referenced anywhere anyway).
> 
> If that works for everyone I'll add one more patch to do the move &
> cleanup before  "platform/x86: lenovo-wmi-other: Add lwmi_attr_id()
> function" where I'm adding the .._NONE thermal mode. That will keep
> the purpose of each patch clean and avoid me needing to modify your
> patches too much.

That sounds good to me.

Moving everything from wmi_gamezone.h into wmi_helpers.h also seems more
semantically correct from my perspective, since the former was just used
to glue things together, which instead matches the purpose of the
latter.

Thanks,
Rong

> 
> Thanks,
> Derek.
> 
> > Thanks,
> > Rong