| 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: <4ed5435a-a35f-4794-8d05-2cc0e34538d7@gmx.de>
Date: Mon, 27 Oct 2025 00:04:08 +0100
From: Armin Wolf <W_Armin@....de>
To: Rong Zhang <i@...g.moe>, Derek John Clark <derekjohn.clark@...il.com>
Cc: Mark Pearson <mpearson-lenovo@...ebb.ca>, 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
Am 26.10.25 um 20:42 schrieb Rong Zhang:
> 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.
Can you please use this information instead of wmi_has_guid() when matching the
components? I would prefer if we can phase out wmi_has_guid() eventually.
Thanks,
Armin Wolf
>
> 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