Message-ID: <d498a1ca58eac5689dae68fffc29440ba75a5faf.camel@rong.moe>
Date: Mon, 27 Oct 2025 03:42:22 +0800
From: Rong Zhang <i@...g.moe>
To: Derek John Clark <derekjohn.clark@...il.com>
Cc: Mark Pearson <mpearson-lenovo@...ebb.ca>, Armin Wolf <W_Armin@....de>, 
 Hans de Goede <hansg@...nel.org>, Ilpo Järvinen
 <ilpo.jarvinen@...ux.intel.com>,  Guenter Roeck <linux@...ck-us.net>,
 platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-hwmon@...r.kernel.org
Subject: Re: [PATCH 4/6] platform/x86: lenovo-wmi-other: Add HWMON for fan
 speed RPM

Hi Derek,

On Sat, 2025-10-25 at 22:23 -0700, Derek John Clark wrote:
> On Sun, Oct 19, 2025 at 2:05 PM Rong Zhang <i@...g.moe> wrote:
> > 
> > Register an HWMON device for fan spped RPM according to Capability Data
> > 00 provided by lenovo-wmi-capdata. The corresponding HWMON nodes are:
> > 
> >  - fanX_enable: enable/disable the fan (tunable)
> >  - fanX_input: current RPM
> >  - fanX_target: target RPM (tunable)
> > 
> > Signed-off-by: Rong Zhang <i@...g.moe>
> > ---
> >  .../wmi/devices/lenovo-wmi-other.rst          |   5 +
> >  drivers/platform/x86/lenovo/Kconfig           |   1 +
> >  drivers/platform/x86/lenovo/wmi-other.c       | 324 +++++++++++++++++-
> >  3 files changed, 317 insertions(+), 13 deletions(-)
> > 
> > diff --git a/Documentation/wmi/devices/lenovo-wmi-other.rst b/Documentation/wmi/devices/lenovo-wmi-other.rst
> > index adbd7943c6756..cb6a9bfe5a79e 100644
> > --- a/Documentation/wmi/devices/lenovo-wmi-other.rst
> > +++ b/Documentation/wmi/devices/lenovo-wmi-other.rst
> > @@ -31,6 +31,11 @@ under the following path:
> > 
> >    /sys/class/firmware-attributes/lenovo-wmi-other/attributes/<attribute>/
> > 
> > +Besides, this driver also exports fan speed RPM to HWMON:
> > + - fanX_enable: enable/disable the fan (tunable)
> > + - fanX_input: current RPM
> > + - fanX_target: target RPM (tunable)
> > +
> >  LENOVO_CAPABILITY_DATA_00
> >  -------------------------
> > 
> > diff --git a/drivers/platform/x86/lenovo/Kconfig b/drivers/platform/x86/lenovo/Kconfig
> > index fb96a0f908f03..be9af04511462 100644
> > --- a/drivers/platform/x86/lenovo/Kconfig
> > +++ b/drivers/platform/x86/lenovo/Kconfig
> > @@ -263,6 +263,7 @@ config LENOVO_WMI_GAMEZONE
> >  config LENOVO_WMI_TUNING
> >         tristate "Lenovo Other Mode WMI Driver"
> >         depends on ACPI_WMI
> > +       select HWMON
> >         select FW_ATTR_CLASS
> >         select LENOVO_WMI_DATA
> >         select LENOVO_WMI_EVENTS
> > diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> > index 20c6ff0be37a1..f8771ed3c6642 100644
> > --- a/drivers/platform/x86/lenovo/wmi-other.c
> > +++ b/drivers/platform/x86/lenovo/wmi-other.c
> > @@ -14,7 +14,15 @@
> >   * These attributes typically don't fit anywhere else in the sysfs and are set
> >   * in Windows using one of Lenovo's multiple user applications.
> >   *
> > + * Besides, this driver also exports tunable fan speed RPM to HWMON.
> > + *
> >   * Copyright (C) 2025 Derek J. Clark <derekjohn.clark@...il.com>
> > + *   - fw_attributes
> > + *   - binding to Capability Data 01
> > + *
> > + * Copyright (C) 2025 Rong Zhang <i@...g.moe>
> > + *   - HWMON
> > + *   - binding to Capability Data 00
> >   */
> > 
> >  #include <linux/acpi.h>
> > @@ -25,6 +33,7 @@
> >  #include <linux/device.h>
> >  #include <linux/export.h>
> >  #include <linux/gfp_types.h>
> > +#include <linux/hwmon.h>
> >  #include <linux/idr.h>
> >  #include <linux/kdev_t.h>
> >  #include <linux/kobject.h>
> > @@ -43,12 +52,20 @@
> > 
> >  #define LENOVO_OTHER_MODE_GUID "DC2A8805-3A8C-41BA-A6F7-092E0089CD3B"
> > 
> > +#define LWMI_SUPP_VALID BIT(0)
> > +#define LWMI_SUPP_MAY_GET (LWMI_SUPP_VALID | BIT(1))
> > +#define LWMI_SUPP_MAY_SET (LWMI_SUPP_VALID | BIT(2))
> > +
> >  #define LWMI_DEVICE_ID_CPU 0x01
> > 
> >  #define LWMI_FEATURE_ID_CPU_SPPT 0x01
> >  #define LWMI_FEATURE_ID_CPU_SPL 0x02
> >  #define LWMI_FEATURE_ID_CPU_FPPT 0x03
> > 
> > +#define LWMI_DEVICE_ID_FAN 0x04
> > +
> > +#define LWMI_FEATURE_ID_FAN_RPM 0x03
> > +
> >  #define LWMI_TYPE_ID_NONE 0x00
> > 
> >  #define LWMI_FEATURE_VALUE_GET 17
> > @@ -59,7 +76,18 @@
> >  #define LWMI_ATTR_MODE_ID_MASK GENMASK(15, 8)
> >  #define LWMI_ATTR_TYPE_ID_MASK GENMASK(7, 0)
> > 
> > +/* Only fan1 and fan2 are present on supported devices. */
> > +#define LWMI_FAN_ID_BASE 1
> > +#define LWMI_FAN_NR 2
> > +#define LWMI_FAN_ID(x) ((x) + LWMI_FAN_ID_BASE)
> > +
> > +#define LWMI_ATTR_ID_FAN_RPM(x)                                                \
> > +       (FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, LWMI_DEVICE_ID_FAN) |        \
> > +        FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, LWMI_FEATURE_ID_FAN_RPM) |  \
> > +        FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, LWMI_FAN_ID(x)))
> > +
> >  #define LWMI_OM_FW_ATTR_BASE_PATH "lenovo-wmi-other"
> > +#define LWMI_OM_HWMON_NAME "lenovo_wmi_other"
> > 
> >  static BLOCKING_NOTIFIER_HEAD(om_chain_head);
> >  static DEFINE_IDA(lwmi_om_ida);
> > @@ -76,15 +104,256 @@ struct lwmi_om_priv {
> >         struct component_master_ops *ops;
> > 
> >         /* only valid after capdata bind */
> > +       struct cd_list *cd00_list;
> >         struct cd_list *cd01_list;
> > 
> > +       struct device *hwmon_dev;
> >         struct device *fw_attr_dev;
> >         struct kset *fw_attr_kset;
> >         struct notifier_block nb;
> >         struct wmi_device *wdev;
> >         int ida_id;
> > +
> > +       struct fan_info {
> > +               u32 supported;
> > +               long target;
> > +       } fan_info[LWMI_FAN_NR];
> >  };
> > 
> > +/* ======== HWMON (component: lenovo-wmi-capdata 00) ======== */
> > +
> > +/**
> > + * lwmi_om_fan_get_set() - Get or set fan RPM value of specified fan
> > + * @priv: Driver private data structure
> > + * @channel: Fan channel index (0-based)
> > + * @val: Pointer to value (input for set, output for get)
> > + * @set: True to set value, false to get value
> > + *
> > + * Communicates with WMI interface to either retrieve current fan RPM
> > + * or set target fan speed.
> > + *
> > + * Return: 0 on success, or an error code.
> > + */
> > +static int lwmi_om_fan_get_set(struct lwmi_om_priv *priv, int channel, u32 *val, bool set)
> > +{
> > +       struct wmi_method_args_32 args;
> > +       u32 method_id, retval;
> > +       int err;
> > +
> > +       method_id = set ? LWMI_FEATURE_VALUE_SET : LWMI_FEATURE_VALUE_GET;
> > +       args.arg0 = LWMI_ATTR_ID_FAN_RPM(channel);
> > +       args.arg1 = set ? *val : 0;
> > +
> > +       err = lwmi_dev_evaluate_int(priv->wdev, 0x0, method_id,
> > +                                   (unsigned char *)&args, sizeof(args), &retval);
> > +       if (err)
> > +               return err;
> > +
> > +       if (!set)
> > +               *val = retval;
> > +       else if (retval != 1)
> > +               return -EIO;
> > +
> > +       return 0;
> > +}
> > +
> > +/**
> > + * lwmi_om_hwmon_is_visible() - Determine visibility of HWMON attributes
> > + * @drvdata: Driver private data
> > + * @type: Sensor type
> > + * @attr: Attribute identifier
> > + * @channel: Channel index
> > + *
> > + * Determines whether a HWMON attribute should be visible in sysfs
> > + * based on hardware capabilities and current configuration.
> > + *
> > + * Return: permission mode, or 0 if invisible.
> > + */
> > +static umode_t lwmi_om_hwmon_is_visible(const void *drvdata, enum hwmon_sensor_types type,
> > +                                       u32 attr, int channel)
> > +{
> > +       struct lwmi_om_priv *priv = (struct lwmi_om_priv *)drvdata;
> > +       bool r = false, w = false;
> > +
> > +       if (type == hwmon_fan) {
> > +               switch (attr) {
> > +               case hwmon_fan_enable:
> > +               case hwmon_fan_target:
> > +                       r = w = priv->fan_info[channel].supported & LWMI_SUPP_MAY_SET;
> > +                       break;
> > +               case hwmon_fan_input:
> > +                       r = priv->fan_info[channel].supported & LWMI_SUPP_MAY_GET;
> > +                       break;
> > +               }
> > +       }
> > +
> 
> There is another method in capdata00 that could be useful here
> 
> Fan Test For Diagnostic Software
> uint32 IDs //0x04050000
> uint32 Capability //9:by project
> bit 3: 0: not support LENOVO_FAN_TEST_DATA, 1 support LENOVO_FAN_TEST_DATA
> bit 2: 0: not support SetFeatureValue(), 1: support SetFeatureValue()
> bit 1: 0: not support GetFeatureValue(), 1: support GetFeatureValue()
> bit 0: 0: not support fan test for diagnostic software, 1: support an
> test for diagnostic software

The information is useful, thanks for that!

A quick look at the decompiled ASL code of my device's ACPI tables:

   Package (0x03)
   {
       0x04050000,
       0x07,
       One
   },

I've confirmed that the corresponding ACPI method didn't modify the
return value of 0x04050000.

0x07 means my device supports this interface, GetFeatureValue() and
SetFeatureValue(); but does not support LENOVO_FAN_TEST_DATA. Is BIT(3)
only defined in some models (but not on my device)? The data returned
by LENOVO_FAN_TEST_DATA seems correct and is probably the min/max auto
points.

My device didn't implement {Get,Set}FeatureValue(0x04050000). What will
it do when it's implemented?

> I'll discuss below, but it seems like knowing min/max is a good idea
> before making the sysfs visible.
> 
> > +       if (!r)
> > +               return 0;
> > +
> > +       return w ? 0644 : 0444;
> > +}
> > +
> > +/**
> > + * lwmi_om_hwmon_read() - Read HWMON sensor data
> > + * @dev: Device pointer
> > + * @type: Sensor type
> > + * @attr: Attribute identifier
> > + * @channel: Channel index
> > + * @val: Pointer to store value
> > + *
> > + * Reads current sensor values from hardware through WMI interface.
> > + *
> > + * Return: 0 on success, or an error code.
> > + */
> > +static int lwmi_om_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> > +                             u32 attr, int channel, long *val)
> > +{
> > +       struct lwmi_om_priv *priv = dev_get_drvdata(dev);
> > +       u32 retval = 0;
> > +       int err;
> > +
> > +       if (type == hwmon_fan) {
> > +               switch (attr) {
> > +               case hwmon_fan_input:
> > +                       err = lwmi_om_fan_get_set(priv, channel, &retval, false);
> > +                       if (err)
> > +                               return err;
> > +
> > +                       *val = retval;
> > +                       return 0;
> > +               case hwmon_fan_enable:
> > +               case hwmon_fan_target:
> > +                       /* -ENODATA before first set. */
> 
> Why not query the interface in realtime to know the system state? That
> would avoid this problem.

My implementation follows the approach of corsair-cpro
(drivers/hwmon/corsair-cpro.c). hwmon_fan_target is about "how much RPM
*should* the fan reach?", while hwmon_fan_input is about "how much RPM
does the fan *really* reach?"

Calling SetFeatureValue(0x040300*) sets the former, while calling
GetFeatureValue(0x040300*) queries the latter. IIUC, using
GetFeatureValue(0x040300*) for hwmon_fan_target violates the
definition, especially when the fan is in auto mode.

> > +                       err = (int)priv->fan_info[channel].target;
> > +                       if (err < 0)
> > +                               return err;
> > +
> > +                       if (attr == hwmon_fan_enable)
> > +                               *val = priv->fan_info[channel].target != 1;
> > +                       else
> > +                               *val = priv->fan_info[channel].target;
> > +                       return 0;
> > +               }
> > +       }
> > +
> > +       return -EOPNOTSUPP;
> > +}
> > +
> > +/**
> > + * lwmi_om_hwmon_write() - Write HWMON sensor data
> > + * @dev: Device pointer
> > + * @type: Sensor type
> > + * @attr: Attribute identifier
> > + * @channel: Channel index
> > + * @val: Value to write
> > + *
> > + * Writes configuration values to hardware through WMI interface.
> > + *
> > + * Return: 0 on success, or an error code.
> > + */
> > +static int lwmi_om_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
> > +                              u32 attr, int channel, long val)
> > +{
> > +       struct lwmi_om_priv *priv = dev_get_drvdata(dev);
> > +       u32 raw;
> > +       int err;
> > +
> > +       if (type == hwmon_fan) {
> > +               switch (attr) {
> > +               case hwmon_fan_enable:
> > +               case hwmon_fan_target:
> > +                       if (attr == hwmon_fan_enable) {
> > +                               if (val == 0)
> > +                                       raw = 1; /* stop */
> > +                               else if (val == 1)
> > +                                       raw = 0; /* auto */
> > +                               else
> > +                                       return -EINVAL;
> > +                       } else {
> > +                               /*
> > +                                * val > U16_MAX seems safe but meaningless.
> > +                                */
> > +                               if (val < 0 || val > U16_MAX)
> 
> I think it might be prudent to only permit these settings if fan speed
> params can't be known. Pragmatically it ensures userspace is aware of
> the range of the interface. Per the documentation it should be "safe"
> as is, but setting below the min fan speed will return it to "auto"
> mode and the hwmon will be out of sync. Anything above should just be
> set to the max, if the BIOS is working properly.

On my device, the data returned by LENOVO_FAN_TEST_DATA seems to be the
min/max auto points. The fan can spin much slower/faster than the
min/max RPM. Setting below the "real" min RPM stops the fan - setting 0
is the only way to return it to auto mode.

   # grep . fan1_*
   grep: fan1_enable: No data available
   fan1_input:2200
   fan1_max:5000
   fan1_min:2200
   grep: fan1_target: No data available
   # echo 800 >fan1_target
   # cat fan1_input
   800
   # echo 700 >fan1_target
   # cat fan1_input
   700
   # echo 10000 >fan1_target
   # cat fan1_input
   6500
   # echo 100 >fan1_target
   # cat fan1_input
   0
   # taskset -c 2 stress-ng -c 1 >/dev/null &
   [1] 37967
   # cat fan1_input
   0
   # echo 0 >fan1_target
   # cat fan1_input
   2200
   # cat fan1_input
   2600

> IMO the fan speed data is essential to ensuring the hwmon interface is
> usable and synced. I'd move that patch before this one in the series
> and make the 0x04050000 method reporting IsSupported required for any
> of the attributes to be visible, with value checks against the min/max
> when setting a given fan.

I agree that setting the RPM too low/high may results in HWMON being
out of sync, which is usually not desired. Will do these in v2.

My extra idea:
- drop the parameter "ignore_fan_cap".
- new parameter "expose_all_fans": does not hide fans when missing from
  LENOVO_FAN_TEST_DATA or when 0x04050000 reports unsupported.
  0x040300* is always checked to hide missing fans.
- new parameter "enforce_fan_rpm_range": defaults to true, checks
  against the min/max RPM from LENOVO_FAN_TEST_DATA while setting
  target RPM. dev_warn_once() when it exceeds min/max RPM.

> > +                                       return -EINVAL;
> > +                               raw = val;
> > +                       }
> > +
> > +                       err = lwmi_om_fan_get_set(priv, channel, &raw, true);
> > +                       if (err)
> > +                               return err;
> > +
> > +                       priv->fan_info[channel].target = raw;
> > +                       return 0;
> > +               }
> > +       }
> > +
> > +       return -EOPNOTSUPP;
> > +}
> > +
> > +static const struct hwmon_channel_info * const lwmi_om_hwmon_info[] = {
> > +       /* Must match LWMI_FAN_NR. */
> > +       HWMON_CHANNEL_INFO(fan,
> > +                          HWMON_F_ENABLE | HWMON_F_INPUT | HWMON_F_TARGET,
> > +                          HWMON_F_ENABLE | HWMON_F_INPUT | HWMON_F_TARGET),
> > +       NULL
> > +};
> > +
> > +static const struct hwmon_ops lwmi_om_hwmon_ops = {
> > +       .is_visible = lwmi_om_hwmon_is_visible,
> > +       .read = lwmi_om_hwmon_read,
> > +       .write = lwmi_om_hwmon_write,
> > +};
> > +
> > +static const struct hwmon_chip_info lwmi_om_hwmon_chip_info = {
> > +       .ops = &lwmi_om_hwmon_ops,
> > +       .info = lwmi_om_hwmon_info,
> > +};
> > +
> > +/**
> > + * lwmi_om_hwmon_add() - Register HWMON device
> > + * @priv: Driver private data
> > + *
> > + * Initializes capability data and registers the HWMON device.
> > + *
> > + * Return: 0 on success, or an error code.
> > + */
> > +static int lwmi_om_hwmon_add(struct lwmi_om_priv *priv)
> > +{
> > +       struct capdata00 capdata00;
> > +       int i, err;
> > +
> > +       for (i = 0; i < LWMI_FAN_NR; i++) {
> 
> There is an assumption here that isn't accurate. Each fan ID
> corresponds to a specific fan functionality. 01 is CPU Fan, 02 is GPU
> Fan, 02 is GPU Power Fan, and 04 is System Fan. Not every fan needs to
> exist, so an ID table might look like this (example from docs):
> 
> illustrate：
> UINT32 NumOfFans = 3;
> NoteBook:
> 1: CPU Fan ID
> 2: GPU Fan ID
> 3: GPU Power Fan ID
> 4: System Fan ID
> UINT32 FanId [1,2,4]
> UINT32 FanMaxSpeed[5400, 5400, 9000];
> UINT32 FanMinSpeed[1900, 1900, 2000];

Thanks for the information. My device only defines 0x0403000{1,2} in
LENOVO_CAPABILITY_DATA_00, so I assumed LWMI_FAN_NR == 2.

> In such a case, "count" would be 3, but the idx should be 4 going to
> the hardware because the GPU Power Fan isn't present, while the case
> fan is.

LWMI_FAN_NR has nothing to do with the actual "count". It is about "how
many HWMON fan channels are defined?" It exists because HWMON channels
are defined statically - we hide defined channels when they are missing
from LENOVO_CAPABILITY_DATA_00 (and LENOVO_FAN_TEST_DATA, if
available).

The implementation of lenovo-wmi-other doesn't use NumOfFans either -
it queries LENOVO_FAN_TEST_DATA using fan ID directly. NumOfFans is
only used when lenovo-wmi-capdata retrieves the data.

This implementation has another advantage: the X in fanX_* is always
the same as the fan ID in
LENOVO_CAPABILITY_DATA_00/LENOVO_FAN_TEST_DATA even in your example
where fan 3 is missing - fan3_* is invisible, fan{1,2,4}_* are exposed.

Given the information, I will define 4 fan channels in v2.

> Thanks,
> Derek

Thanks,
Rong

> > +               err = lwmi_cd00_get_data(priv->cd00_list, LWMI_ATTR_ID_FAN_RPM(i),
> > +                                        &capdata00);
> > +               if (err)
> > +                       continue;
> > +
> > +               priv->fan_info[i] = (struct fan_info) {
> > +                       .supported = capdata00.supported,
> > +                       .target = -ENODATA,
> > +               };
> > +       }
> > +
> > +       priv->hwmon_dev = hwmon_device_register_with_info(&priv->wdev->dev, LWMI_OM_HWMON_NAME,
> > +                                                         priv, &lwmi_om_hwmon_chip_info, NULL);
> > +
> > +       return PTR_ERR_OR_ZERO(priv->hwmon_dev);
> > +}
> > +
> > +/**
> > + * lwmi_om_hwmon_remove() - Unregister HWMON device
> > + * @priv: Driver private data
> > + *
> > + * Unregisters the HWMON device and resets all fans to automatic mode.
> > + * Ensures hardware doesn't remain in manual mode after driver removal.
> > + */
> > +static void lwmi_om_hwmon_remove(struct lwmi_om_priv *priv)
> > +{
> > +       hwmon_device_unregister(priv->hwmon_dev);
> > +}
> > +
> > +/* ======== fw_attributes (component: lenovo-wmi-capdata 01) ======== */
> > +
> >  struct tunable_attr_01 {
> >         struct capdata01 *capdata;
> >         struct device *dev;
> > @@ -564,15 +833,17 @@ static void lwmi_om_fw_attr_remove(struct lwmi_om_priv *priv)
> >         device_unregister(priv->fw_attr_dev);
> >  }
> > 
> > +/* ======== Self (master: lenovo-wmi-other) ======== */
> > +
> >  /**
> >   * lwmi_om_master_bind() - Bind all components of the other mode driver
> >   * @dev: The lenovo-wmi-other driver basic device.
> >   *
> > - * Call component_bind_all to bind the lenovo-wmi-capdata01 driver to the
> > - * lenovo-wmi-other master driver. On success, assign the capability data 01
> > - * list pointer to the driver data struct for later access. This pointer
> > - * is only valid while the capdata01 interface exists. Finally, register all
> > - * firmware attribute groups.
> > + * Call component_bind_all to bind the lenovo-wmi-capdata devices to the
> > + * lenovo-wmi-other master driver. On success, assign the capability data
> > + * list pointers to the driver data struct for later access. These pointers
> > + * are only valid while the capdata interfaces exist. Finally, register the
> > + * HWMON device and all firmware attribute groups.
> >   *
> >   * Return: 0 on success, or an error code.
> >   */
> > @@ -586,26 +857,47 @@ static int lwmi_om_master_bind(struct device *dev)
> >         if (ret)
> >                 return ret;
> > 
> > -       priv->cd01_list = binder.cd01_list;
> > -       if (!priv->cd01_list)
> > +       if (!binder.cd00_list && !binder.cd01_list)
> >                 return -ENODEV;
> > 
> > -       return lwmi_om_fw_attr_add(priv);
> > +       priv->cd00_list = binder.cd00_list;
> > +       if (priv->cd00_list) {
> > +               ret = lwmi_om_hwmon_add(priv);
> > +               if (ret)
> > +                       return ret;
> > +       }
> > +
> > +       priv->cd01_list = binder.cd01_list;
> > +       if (priv->cd01_list) {
> > +               ret = lwmi_om_fw_attr_add(priv);
> > +               if (ret) {
> > +                       if (priv->cd00_list)
> > +                               lwmi_om_hwmon_remove(priv);
> > +                       return ret;
> > +               }
> > +       }
> > +
> > +       return 0;
> >  }
> > 
> >  /**
> >   * lwmi_om_master_unbind() - Unbind all components of the other mode driver
> >   * @dev: The lenovo-wmi-other driver basic device
> >   *
> > - * Unregister all capability data attribute groups. Then call
> > - * component_unbind_all to unbind the lenovo-wmi-capdata01 driver from the
> > - * lenovo-wmi-other master driver. Finally, free the IDA for this device.
> > + * Unregister the HWMON device and all capability data attribute groups. Then
> > + * call component_unbind_all to unbind the lenovo-wmi-capdata driver from the
> > + * lenovo-wmi-other master driver.
> >   */
> >  static void lwmi_om_master_unbind(struct device *dev)
> >  {
> >         struct lwmi_om_priv *priv = dev_get_drvdata(dev);
> > 
> > -       lwmi_om_fw_attr_remove(priv);
> > +       if (priv->cd00_list)
> > +               lwmi_om_hwmon_remove(priv);
> > +
> > +       if (priv->cd01_list)
> > +               lwmi_om_fw_attr_remove(priv);
> > +
> >         component_unbind_all(dev, NULL);
> >  }
> > 
> > @@ -624,6 +916,9 @@ static int lwmi_other_probe(struct wmi_device *wdev, const void *context)
> >         if (!priv)
> >                 return -ENOMEM;
> > 
> > +       /* Sentinel for on-demand ida_free(). */
> > +       priv->ida_id = -EIDRM;
> > +
> >         priv->wdev = wdev;
> >         dev_set_drvdata(&wdev->dev, priv);
> > 
> > @@ -654,7 +949,9 @@ static void lwmi_other_remove(struct wmi_device *wdev)
> >         struct lwmi_om_priv *priv = dev_get_drvdata(&wdev->dev);
> > 
> >         component_master_del(&wdev->dev, &lwmi_om_master_ops);
> > -       ida_free(&lwmi_om_ida, priv->ida_id);
> > +
> > +       if (priv->ida_id >= 0)
> > +               ida_free(&lwmi_om_ida, priv->ida_id);
> >  }
> > 
> >  static const struct wmi_device_id lwmi_other_id_table[] = {
> > @@ -679,5 +976,6 @@ MODULE_IMPORT_NS("LENOVO_WMI_CD");
> >  MODULE_IMPORT_NS("LENOVO_WMI_HELPERS");
> >  MODULE_DEVICE_TABLE(wmi, lwmi_other_id_table);
> >  MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@...il.com>");
> > +MODULE_AUTHOR("Rong Zhang <i@...g.moe>");
> >  MODULE_DESCRIPTION("Lenovo Other Mode WMI Driver");
> >  MODULE_LICENSE("GPL");
> > --
> > 2.51.0
> > 

Powered by blists - more mailing lists