[PATCH v3 0/6] platform-x86: lenovo-wmi: Add fixes and enhancement

[PATCH v3 0/6] platform-x86: lenovo-wmi: Add fixes and enhancement

[Derek J. Clark]

This series adds many much needed features and fixes to the lenovo-wmi
drivers.

Patch 1 adds a macro to make assigning attribute ID's for capdata
cleaner and easier.
Patch 2 addresses bugs where devices that don't support exposed
attributes would still create the attribute, and also attempts to
identify the correct capdata and set/get methods as some legacy
interfaces don't use the custom mode in the method or capdata ID.
Patch 3 adds the remaining CPU attributes that weren't previously
exposed.
Patch 4 adds the GPU attributes that weren't previously exposed.
Patch 5 consolidates name constants in preparation for patch 6.
Patch 6 adds battery charge-type limiting when supported only by WMI.

After applying Rong's series from
https://lore.kernel.org/platform-driver-x86/20260210191938.137234-1-i@rong.moe/
I found that there weren't any conflicts, so rather than combine
the series we can just review each separately.

Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v3:
  - Re-add HWMON name const and jsut rename LWMI_OM_FW_ATTR_BASE_PATH
  - Fix linker warnings by moving acpi/battery include to the end of the
    list.
  - Remove CPU/GPU OC features. These attributes are BOOL type and will
    need a new constructor that I'll add later.
v2:
  - Fix gpu_mode misisng from attributes list.
  - Fix prototypes for power suppy patch.
  - Reorganize CPU and GPU attributes alphabetically.
  - Break out the patch consolidating the driver name cost.
  - Move some of the refactoring of attribute_id back to into patch 1
    where it belongs.
  - Fix some additional typos in function prototypes.
v1: https://lore.kernel.org/platform-driver-x86/20260213081243.794288-1-derekjohn.clark@gmail.com/

Derek J. Clark (6):
  platform/x86: lenovo-wmi-other: Add LWMI_ATTR_ID Macro
  platform/x86: lenovo-wmi-other: Limit adding attributes to supported
    devices
  platform/x86: lenovo-wmi-other: Add missing CPU tunable attributes
  platform/x86: lenovo-wmi-other: Add GPU tunable attributes
  platform-x86: lenovo-wmi-other: Rename LWMI_OM_FW_ATTR_BASE_PATH
  platform/x86: lenovo-wmi-other: Add WMI battery charge limiting.

 .../wmi/devices/lenovo-wmi-other.rst          |  21 +
 drivers/platform/x86/lenovo/wmi-capdata.h     |  13 +-
 drivers/platform/x86/lenovo/wmi-gamezone.h    |   1 +
 drivers/platform/x86/lenovo/wmi-other.c       | 605 ++++++++++++++++--
 4 files changed, 593 insertions(+), 47 deletions(-)

-- 
2.52.0

[PATCH v3 1/6] platform/x86: lenovo-wmi-other: Add LWMI_ATTR_ID Macro

[Derek J. Clark]

Adds LWMI_ATTR_ID macro. In the same vein as LWMI_ATTR_ID_FAN_RPM,
but as a generic, to de-duplicate attribute_id assignment biolerplate.

Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
 drivers/platform/x86/lenovo/wmi-capdata.h  |  6 ++++
 drivers/platform/x86/lenovo/wmi-gamezone.h |  1 +
 drivers/platform/x86/lenovo/wmi-other.c    | 36 ++++++----------------
 3 files changed, 17 insertions(+), 26 deletions(-)

diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
index 8c1df3efcc55..27202e2dc8a5 100644
--- a/drivers/platform/x86/lenovo/wmi-capdata.h
+++ b/drivers/platform/x86/lenovo/wmi-capdata.h
@@ -17,6 +17,12 @@
 #define LWMI_ATTR_MODE_ID_MASK	GENMASK(15, 8)
 #define LWMI_ATTR_TYPE_ID_MASK	GENMASK(7, 0)
 
+#define LWMI_ATTR_ID(dev, feat, mode, type) \
+	(FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, dev) | \
+	 FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, feat) | \
+	 FIELD_PREP(LWMI_ATTR_MODE_ID_MASK, mode) | \
+	 FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, type))
+
 #define LWMI_DEVICE_ID_FAN	0x04
 
 struct component_match;
diff --git a/drivers/platform/x86/lenovo/wmi-gamezone.h b/drivers/platform/x86/lenovo/wmi-gamezone.h
index 6b163a5eeb95..ddb919cf6c36 100644
--- a/drivers/platform/x86/lenovo/wmi-gamezone.h
+++ b/drivers/platform/x86/lenovo/wmi-gamezone.h
@@ -10,6 +10,7 @@ enum gamezone_events_type {
 };
 
 enum thermal_mode {
+	LWMI_GZ_THERMAL_MODE_NONE =	   0x00,
 	LWMI_GZ_THERMAL_MODE_QUIET =	   0x01,
 	LWMI_GZ_THERMAL_MODE_BALANCED =	   0x02,
 	LWMI_GZ_THERMAL_MODE_PERFORMANCE = 0x03,
diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
index 6040f45aa2b0..95886df39c8d 100644
--- a/drivers/platform/x86/lenovo/wmi-other.c
+++ b/drivers/platform/x86/lenovo/wmi-other.c
@@ -71,10 +71,9 @@
 #define LWMI_FAN_NR 4
 #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_ATTR_ID_FAN_RPM(x)                                   \
+	LWMI_ATTR_ID(LWMI_DEVICE_ID_FAN, LWMI_FEATURE_ID_FAN_RPM, \
+		     LWMI_GZ_THERMAL_MODE_NONE, LWMI_FAN_ID(x))
 
 #define LWMI_FAN_DIV 100
 
@@ -716,12 +715,8 @@ static ssize_t attr_capdata01_show(struct kobject *kobj,
 	u32 attribute_id;
 	int value, ret;
 
-	attribute_id =
-		FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, tunable_attr->device_id) |
-		FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, tunable_attr->feature_id) |
-		FIELD_PREP(LWMI_ATTR_MODE_ID_MASK,
-			   LWMI_GZ_THERMAL_MODE_CUSTOM) |
-		FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, tunable_attr->type_id);
+	attribute_id = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
+				    LWMI_GZ_THERMAL_MODE_CUSTOM, tunable_attr->type_id);
 
 	ret = lwmi_cd01_get_data(priv->cd01_list, attribute_id, &capdata);
 	if (ret)
@@ -776,7 +771,6 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
 	struct wmi_method_args_32 args;
 	struct capdata01 capdata;
 	enum thermal_mode mode;
-	u32 attribute_id;
 	u32 value;
 	int ret;
 
@@ -787,13 +781,10 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
 	if (mode != LWMI_GZ_THERMAL_MODE_CUSTOM)
 		return -EBUSY;
 
-	attribute_id =
-		FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, tunable_attr->device_id) |
-		FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, tunable_attr->feature_id) |
-		FIELD_PREP(LWMI_ATTR_MODE_ID_MASK, mode) |
-		FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, tunable_attr->type_id);
+	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
+				 mode, tunable_attr->type_id);
 
-	ret = lwmi_cd01_get_data(priv->cd01_list, attribute_id, &capdata);
+	ret = lwmi_cd01_get_data(priv->cd01_list, args.arg0, &capdata);
 	if (ret)
 		return ret;
 
@@ -804,7 +795,6 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
 	if (value < capdata.min_value || value > capdata.max_value)
 		return -EINVAL;
 
-	args.arg0 = attribute_id;
 	args.arg1 = value;
 
 	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_SET,
@@ -838,7 +828,6 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
 	struct lwmi_om_priv *priv = dev_get_drvdata(tunable_attr->dev);
 	struct wmi_method_args_32 args;
 	enum thermal_mode mode;
-	u32 attribute_id;
 	int retval;
 	int ret;
 
@@ -846,13 +835,8 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
 	if (ret)
 		return ret;
 
-	attribute_id =
-		FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, tunable_attr->device_id) |
-		FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, tunable_attr->feature_id) |
-		FIELD_PREP(LWMI_ATTR_MODE_ID_MASK, mode) |
-		FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, tunable_attr->type_id);
-
-	args.arg0 = attribute_id;
+	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
+				 mode, tunable_attr->type_id);
 
 	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_GET,
 				    (unsigned char *)&args, sizeof(args),
-- 
2.52.0

[PATCH v3 2/6] platform/x86: lenovo-wmi-other: Limit adding attributes to supported devices

[Derek J. Clark]

Adds lwmi_is_attr_01_supported, and only creates the attribute subfolder
if the attribute is supported by the hardware. Due to some poorly
implemented BIOS, this is a multi-step sequence of events. This is
because:
- Some BIOS support getting the capability data from custom mode (0xff),
  while others only support it in no-mode (0x00).
- Similarly, some BIOS support get/set for the current value from custom
  mode (0xff), while others only support it in no-mode (0x00).
- Some BIOS report capability data for a method that is not fully
  implemented.
- Some BIOS have methods fully implemented, but no complimentary
  capability data.

To ensure we only expose fully implemented methods with corresponding
capability data, we check each outcome before reporting that an
attribute can be supported.

Checking for lwmi_is_attr_01_supported during remove is not done to
ensure that we don't attempt to call cd01 or send WMI events if one of
the interfaces being removed was the cause of the driver unloading.

While adding members to tunable_attr_01, remove unused capdata pointer
and limit size of all ID's to the appropriate size.

Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Reported-by: Kurt Borja <kuurtb@gmail.com>
Closes: https://lore.kernel.org/platform-driver-x86/DG60P3SHXR8H.3NSEHMZ6J7XRC@gmail.com/
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
 drivers/platform/x86/lenovo/wmi-other.c | 117 +++++++++++++++++++++---
 1 file changed, 102 insertions(+), 15 deletions(-)

diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
index 95886df39c8d..f3f12303e379 100644
--- a/drivers/platform/x86/lenovo/wmi-other.c
+++ b/drivers/platform/x86/lenovo/wmi-other.c
@@ -545,11 +545,12 @@ static void lwmi_om_fan_info_collect_cd_fan(struct device *dev, struct cd_list *
 /* ======== fw_attributes (component: lenovo-wmi-capdata 01) ======== */
 
 struct tunable_attr_01 {
-	struct capdata01 *capdata;
 	struct device *dev;
-	u32 feature_id;
-	u32 device_id;
-	u32 type_id;
+	u8 feature_id;
+	u8 device_id;
+	u8 type_id;
+	u8 cd_mode_id; /* mode arg for searching capdata */
+	u8 cv_mode_id; /* mode arg for set/get current_value */
 };
 
 static struct tunable_attr_01 ppt_pl1_spl = {
@@ -716,7 +717,7 @@ static ssize_t attr_capdata01_show(struct kobject *kobj,
 	int value, ret;
 
 	attribute_id = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
-				    LWMI_GZ_THERMAL_MODE_CUSTOM, tunable_attr->type_id);
+				    tunable_attr->cd_mode_id, tunable_attr->type_id);
 
 	ret = lwmi_cd01_get_data(priv->cd01_list, attribute_id, &capdata);
 	if (ret)
@@ -782,7 +783,7 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
 		return -EBUSY;
 
 	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
-				 mode, tunable_attr->type_id);
+				 tunable_attr->cd_mode_id, tunable_attr->type_id);
 
 	ret = lwmi_cd01_get_data(priv->cd01_list, args.arg0, &capdata);
 	if (ret)
@@ -795,6 +796,8 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
 	if (value < capdata.min_value || value > capdata.max_value)
 		return -EINVAL;
 
+	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
+				 tunable_attr->cv_mode_id, tunable_attr->type_id);
 	args.arg1 = value;
 
 	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_SET,
@@ -828,13 +831,16 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
 	struct lwmi_om_priv *priv = dev_get_drvdata(tunable_attr->dev);
 	struct wmi_method_args_32 args;
 	enum thermal_mode mode;
-	int retval;
-	int ret;
+	int retval, ret;
 
 	ret = lwmi_om_notifier_call(&mode);
 	if (ret)
 		return ret;
 
+	/* If "no-mode" is the supported mode, ensure we never send current mode */
+	if (tunable_attr->cv_mode_id == LWMI_GZ_THERMAL_MODE_NONE)
+		mode = tunable_attr->cv_mode_id;
+
 	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
 				 mode, tunable_attr->type_id);
 
@@ -847,6 +853,85 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
 	return sysfs_emit(buf, "%d\n", retval);
 }
 
+/**
+ * lwmi_attr_01_is_supported() - Determine if the given attribute is supported.
+ * @tunable_attr: The attribute to verify.
+ *
+ * First check if the attribute has a corresponding capdata01 table in the cd01
+ * module under the "custom" mode (0xff). If that is not present then check if
+ * there is a corresponding "no-mode" (0x00) entry. If either of those passes,
+ * check capdata->supported for values > 0. If capdata is available, attempt to
+ * determine the set/get mode for the current value property using a similar
+ * pattern. If the value returned by either custom or no-mode is 0, or we get
+ * an error, we assume that mode is not supported. If any of the above checks
+ * fail then the attribute is not fully supported.
+ *
+ * The probed cd_mode_id/cv_mode_id are stored on the tunable_attr for later
+ * reference.
+ *
+ * Return: Support level, or an error code.
+ */
+static int lwmi_attr_01_is_supported(struct tunable_attr_01 *tunable_attr)
+{
+	struct lwmi_om_priv *priv = dev_get_drvdata(tunable_attr->dev);
+	u8 mode = LWMI_GZ_THERMAL_MODE_CUSTOM;
+	struct wmi_method_args_32 args;
+	struct capdata01 capdata;
+	int retval, ret;
+
+	/* Determine tunable_attr->cd_mode_id */
+no_mode_fallback_1:
+	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
+				 mode, tunable_attr->type_id);
+
+	ret = lwmi_cd01_get_data(priv->cd01_list, args.arg0, &capdata);
+	if (ret && mode) {
+		dev_dbg(tunable_attr->dev, "Attribute id %x not supported\n", args.arg0);
+		mode = LWMI_GZ_THERMAL_MODE_NONE;
+		goto no_mode_fallback_1;
+	}
+	if (ret)
+		goto not_supported;
+	if (!capdata.supported) {
+		ret = -EOPNOTSUPP;
+		goto not_supported;
+	}
+
+	tunable_attr->cd_mode_id = mode;
+
+	/* Determine tunable_attr->cv_mode_id */
+	mode = LWMI_GZ_THERMAL_MODE_CUSTOM;
+no_mode_fallback_2:
+	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
+				 mode, tunable_attr->type_id);
+
+	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_GET,
+				    (unsigned char *)&args, sizeof(args),
+				    &retval);
+	if ((ret && mode) || (!retval && mode)) {
+		dev_dbg(tunable_attr->dev, "Attribute id %x not supported\n", args.arg0);
+		mode = LWMI_GZ_THERMAL_MODE_NONE;
+		goto no_mode_fallback_2;
+	}
+	if (ret)
+		goto not_supported;
+	if (retval == 0) {
+		ret = -EOPNOTSUPP;
+		goto not_supported;
+	}
+
+	tunable_attr->cv_mode_id = mode;
+	dev_dbg(tunable_attr->dev, "cd_mode_id: %02x%02x%02x%02x, cv_mode_id: %#08x attribute support level: %x\n",
+		tunable_attr->device_id, tunable_attr->feature_id, tunable_attr->cd_mode_id,
+		tunable_attr->type_id, args.arg0, capdata.supported);
+
+	return capdata.supported;
+
+not_supported:
+	dev_dbg(tunable_attr->dev, "Attribute id %x not supported\n", args.arg0);
+	return ret;
+}
+
 /* Lenovo WMI Other Mode Attribute macros */
 #define __LWMI_ATTR_RO(_func, _name)                                  \
 	{                                                             \
@@ -970,19 +1055,21 @@ static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
 	}
 
 	for (i = 0; i < ARRAY_SIZE(cd01_attr_groups) - 1; i++) {
-		err = sysfs_create_group(&priv->fw_attr_kset->kobj,
-					 cd01_attr_groups[i].attr_group);
-		if (err)
-			goto err_remove_groups;
-
 		cd01_attr_groups[i].tunable_attr->dev = &priv->wdev->dev;
+		if (lwmi_attr_01_is_supported(cd01_attr_groups[i].tunable_attr) > 0) {
+			err = sysfs_create_group(&priv->fw_attr_kset->kobj,
+						 cd01_attr_groups[i].attr_group);
+			if (err)
+				goto err_remove_groups;
+		}
 	}
 	return 0;
 
 err_remove_groups:
 	while (i--)
-		sysfs_remove_group(&priv->fw_attr_kset->kobj,
-				   cd01_attr_groups[i].attr_group);
+		if (lwmi_attr_01_is_supported(cd01_attr_groups[i].tunable_attr) > 0)
+			sysfs_remove_group(&priv->fw_attr_kset->kobj,
+					   cd01_attr_groups[i].attr_group);
 
 	kset_unregister(priv->fw_attr_kset);
 
-- 
2.52.0

[PATCH v3 3/6] platform/x86: lenovo-wmi-other: Add missing CPU tunable attributes

[Derek J. Clark]

Use an enum for device ID's and CPU attribute feature ID's, add
missing CPU attributes.

Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
 .../wmi/devices/lenovo-wmi-other.rst          | 10 ++
 drivers/platform/x86/lenovo/wmi-capdata.h     |  5 +-
 drivers/platform/x86/lenovo/wmi-other.c       | 98 ++++++++++++++++++-
 3 files changed, 107 insertions(+), 6 deletions(-)

diff --git a/Documentation/wmi/devices/lenovo-wmi-other.rst b/Documentation/wmi/devices/lenovo-wmi-other.rst
index 01d471156738..f4763ed66cc6 100644
--- a/Documentation/wmi/devices/lenovo-wmi-other.rst
+++ b/Documentation/wmi/devices/lenovo-wmi-other.rst
@@ -68,9 +68,19 @@ Each attribute has the following properties:
  - type
 
 The following firmware-attributes are implemented:
+ - cpu_oc_stat: CPU Overlocking Status
+ - cpu_temp: CPU Thermal Load Limit
+ - ppt_cpu_cl: CPU Cross Loading Power Limit
+ - ppt_pl1_apu_spl: Platform Profile Tracking APU Sustained Power Limit
  - ppt_pl1_spl: Platform Profile Tracking Sustained Power Limit
+ - ppt_pl1_spl_cl: Platform Profile Tracking Cross Loading Sustained Power Limit
+ - ppt_pl1_tau: Exceed Duration for Platform Profile Tracking Sustained Power Limit
  - ppt_pl2_sppt: Platform Profile Tracking Slow Package Power Tracking
+ - ppt_pl2_sppt_cl: Platform Profile Tracking Cross Loading Slow Package Tracking
  - ppt_pl3_fppt: Platform Profile Tracking Fast Package Power Tracking
+ - ppt_pl3_fppt_cl: Platform Profile Tracking Cross Loading Fast Package Power Tracking
+ - ppt_pl4_ipl: Platform Profile Trakcing Instantaneous Power Limit
+ - ppt_pl4_ipl_cl: Platform Profile Tracking Cross Loading Instantaneous Power Limit
 
 LENOVO_FAN_TEST_DATA
 -------------------------
diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
index 27202e2dc8a5..aa48f43cbb43 100644
--- a/drivers/platform/x86/lenovo/wmi-capdata.h
+++ b/drivers/platform/x86/lenovo/wmi-capdata.h
@@ -23,7 +23,10 @@
 	 FIELD_PREP(LWMI_ATTR_MODE_ID_MASK, mode) | \
 	 FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, type))
 
-#define LWMI_DEVICE_ID_FAN	0x04
+enum lwmi_device_id {
+	LWMI_DEVICE_ID_CPU = 0x01,
+	LWMI_DEVICE_ID_FAN = 0x04,
+};
 
 struct component_match;
 struct device;
diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
index f3f12303e379..87aba244da84 100644
--- a/drivers/platform/x86/lenovo/wmi-other.c
+++ b/drivers/platform/x86/lenovo/wmi-other.c
@@ -54,15 +54,21 @@
 
 #define LENOVO_OTHER_MODE_GUID "DC2A8805-3A8C-41BA-A6F7-092E0089CD3B"
 
-#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
+enum lwmi_feature_id_cpu {
+	LWMI_FEATURE_ID_CPU_SPPT = 0x01,
+	LWMI_FEATURE_ID_CPU_SPL = 0x02,
+	LWMI_FEATURE_ID_CPU_FPPT = 0x03,
+	LWMI_FEATURE_ID_CPU_TEMP = 0x04,
+	LWMI_FEATURE_ID_CPU_APU = 0x05,
+	LWMI_FEATURE_ID_CPU_CL = 0x06,
+	LWMI_FEATURE_ID_CPU_TAU = 0x07,
+	LWMI_FEATURE_ID_CPU_IPL = 0x09,
+};
 
 #define LWMI_FEATURE_ID_FAN_RPM 0x03
 
 #define LWMI_TYPE_ID_NONE 0x00
+#define LWMI_TYPE_ID_CROSSLOAD 0x01
 
 #define LWMI_FEATURE_VALUE_GET 17
 #define LWMI_FEATURE_VALUE_SET 18
@@ -559,18 +565,72 @@ static struct tunable_attr_01 ppt_pl1_spl = {
 	.type_id = LWMI_TYPE_ID_NONE,
 };
 
+static struct tunable_attr_01 ppt_pl1_spl_cl = {
+	.device_id = LWMI_DEVICE_ID_CPU,
+	.feature_id = LWMI_FEATURE_ID_CPU_SPL,
+	.type_id = LWMI_TYPE_ID_CROSSLOAD,
+};
+
 static struct tunable_attr_01 ppt_pl2_sppt = {
 	.device_id = LWMI_DEVICE_ID_CPU,
 	.feature_id = LWMI_FEATURE_ID_CPU_SPPT,
 	.type_id = LWMI_TYPE_ID_NONE,
 };
 
+static struct tunable_attr_01 ppt_pl2_sppt_cl = {
+	.device_id = LWMI_DEVICE_ID_CPU,
+	.feature_id = LWMI_FEATURE_ID_CPU_SPPT,
+	.type_id = LWMI_TYPE_ID_CROSSLOAD,
+};
+
 static struct tunable_attr_01 ppt_pl3_fppt = {
 	.device_id = LWMI_DEVICE_ID_CPU,
 	.feature_id = LWMI_FEATURE_ID_CPU_FPPT,
 	.type_id = LWMI_TYPE_ID_NONE,
 };
 
+static struct tunable_attr_01 ppt_pl3_fppt_cl = {
+	.device_id = LWMI_DEVICE_ID_CPU,
+	.feature_id = LWMI_FEATURE_ID_CPU_FPPT,
+	.type_id = LWMI_TYPE_ID_CROSSLOAD,
+};
+
+static struct tunable_attr_01 cpu_temp = {
+	.device_id = LWMI_DEVICE_ID_CPU,
+	.feature_id = LWMI_FEATURE_ID_CPU_TEMP,
+	.type_id = LWMI_TYPE_ID_NONE,
+};
+
+static struct tunable_attr_01 ppt_pl1_apu_spl = {
+	.device_id = LWMI_DEVICE_ID_CPU,
+	.feature_id = LWMI_FEATURE_ID_CPU_APU,
+	.type_id = LWMI_TYPE_ID_NONE,
+};
+
+static struct tunable_attr_01 ppt_cpu_cl = {
+	.device_id = LWMI_DEVICE_ID_CPU,
+	.feature_id = LWMI_FEATURE_ID_CPU_CL,
+	.type_id = LWMI_TYPE_ID_NONE,
+};
+
+static struct tunable_attr_01 ppt_pl1_tau = {
+	.device_id = LWMI_DEVICE_ID_CPU,
+	.feature_id = LWMI_FEATURE_ID_CPU_TAU,
+	.type_id = LWMI_TYPE_ID_NONE,
+};
+
+static struct tunable_attr_01 ppt_pl4_ipl = {
+	.device_id = LWMI_DEVICE_ID_CPU,
+	.feature_id = LWMI_FEATURE_ID_CPU_IPL,
+	.type_id = LWMI_TYPE_ID_NONE,
+};
+
+static struct tunable_attr_01 ppt_pl4_ipl_cl = {
+	.device_id = LWMI_DEVICE_ID_CPU,
+	.feature_id = LWMI_FEATURE_ID_CPU_IPL,
+	.type_id = LWMI_TYPE_ID_CROSSLOAD,
+};
+
 struct capdata01_attr_group {
 	const struct attribute_group *attr_group;
 	struct tunable_attr_01 *tunable_attr;
@@ -1009,17 +1069,45 @@ static int lwmi_attr_01_is_supported(struct tunable_attr_01 *tunable_attr)
 		.name = _fsname, .attrs = _attrname##_attrs               \
 	}
 
+LWMI_ATTR_GROUP_TUNABLE_CAP01(cpu_temp, "cpu_temp",
+			      "Set the CPU thermal load limit");
+LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_cpu_cl, "ppt_cpu_cl",
+			      "Set the CPU cross loading power limit");
+LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl1_apu_spl, "ppt_pl1_apu_spl",
+			      "Set the APU sustained power limit");
 LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl1_spl, "ppt_pl1_spl",
 			      "Set the CPU sustained power limit");
+LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl1_spl_cl, "ppt_pl1_spl_cl",
+			      "Set the CPU cross loading sustained power limit");
 LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl2_sppt, "ppt_pl2_sppt",
 			      "Set the CPU slow package power tracking limit");
+LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl2_sppt_cl, "ppt_pl2_sppt_cl",
+			      "Set the CPU cross loading slow package power tracking limit");
 LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl3_fppt, "ppt_pl3_fppt",
 			      "Set the CPU fast package power tracking limit");
+LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl3_fppt_cl, "ppt_pl3_fppt_cl",
+			      "Set the CPU cross loading fast package power tracking limit");
+LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl1_tau, "ppt_pl1_tau",
+			      "Set the CPU sustained power limit exceed duration");
+LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl4_ipl, "ppt_pl4_ipl",
+			      "Set the CPU instantaneous power limit");
+LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl4_ipl_cl, "ppt_pl4_ipl_cl",
+			      "Set the CPU cross loading instantaneous power limit");
+
 
 static struct capdata01_attr_group cd01_attr_groups[] = {
+	{ &cpu_temp_attr_group, &cpu_temp },
+	{ &ppt_cpu_cl_attr_group, &ppt_cpu_cl },
+	{ &ppt_pl1_apu_spl_attr_group, &ppt_pl1_apu_spl },
 	{ &ppt_pl1_spl_attr_group, &ppt_pl1_spl },
+	{ &ppt_pl1_spl_cl_attr_group, &ppt_pl1_spl_cl },
+	{ &ppt_pl1_tau_attr_group, &ppt_pl1_tau },
 	{ &ppt_pl2_sppt_attr_group, &ppt_pl2_sppt },
+	{ &ppt_pl2_sppt_cl_attr_group, &ppt_pl2_sppt_cl },
 	{ &ppt_pl3_fppt_attr_group, &ppt_pl3_fppt },
+	{ &ppt_pl3_fppt_cl_attr_group, &ppt_pl3_fppt_cl },
+	{ &ppt_pl4_ipl_attr_group, &ppt_pl4_ipl },
+	{ &ppt_pl4_ipl_cl_attr_group, &ppt_pl4_ipl_cl },
 	{},
 };
 
-- 
2.52.0

[PATCH v3 4/6] platform/x86: lenovo-wmi-other: Add GPU tunable attributes

[Derek J. Clark]

Use an enum for all GPU attribute feature ID's and add GPU attributes.

Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
 .../wmi/devices/lenovo-wmi-other.rst          |  11 ++
 drivers/platform/x86/lenovo/wmi-capdata.h     |   1 +
 drivers/platform/x86/lenovo/wmi-other.c       | 105 ++++++++++++++++++
 3 files changed, 117 insertions(+)

diff --git a/Documentation/wmi/devices/lenovo-wmi-other.rst b/Documentation/wmi/devices/lenovo-wmi-other.rst
index f4763ed66cc6..f7564b23bb7f 100644
--- a/Documentation/wmi/devices/lenovo-wmi-other.rst
+++ b/Documentation/wmi/devices/lenovo-wmi-other.rst
@@ -70,6 +70,17 @@ Each attribute has the following properties:
 The following firmware-attributes are implemented:
  - cpu_oc_stat: CPU Overlocking Status
  - cpu_temp: CPU Thermal Load Limit
+ - dgpu_boost_clk: Dedicated GPU Boost Clock
+ - dgpu_enable: Dedicated GPU Enabled Status
+ - gpu_didvid: GPU Device Identifier and Vendor Identifier
+ - gpu_mode: GPU Mode by Power Limit
+ - gpu_nv_ac_offset: Nvidia GPU AC Total Processing Power Baseline Offset
+ - gpu_nv_bpl: Nvidia GPU Base Power Limit
+ - gpu_nv_cpu_boost: Nvidia GPU to CPU Dynamic Boost Limit
+ - gpu_nv_ctgp: Nvidia GPU Configurable Total Graphics Power
+ - gpu_nv_ppab: Nvidia GPU Power Performance Aware Boost Limit
+ - gpu_oc_stat: GPU Overclocking Status
+ - gpu_temp: GPU Thermal Load Limit
  - ppt_cpu_cl: CPU Cross Loading Power Limit
  - ppt_pl1_apu_spl: Platform Profile Tracking APU Sustained Power Limit
  - ppt_pl1_spl: Platform Profile Tracking Sustained Power Limit
diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
index aa48f43cbb43..b7f9ee7b301a 100644
--- a/drivers/platform/x86/lenovo/wmi-capdata.h
+++ b/drivers/platform/x86/lenovo/wmi-capdata.h
@@ -25,6 +25,7 @@
 
 enum lwmi_device_id {
 	LWMI_DEVICE_ID_CPU = 0x01,
+	LWMI_DEVICE_ID_GPU = 0x02,
 	LWMI_DEVICE_ID_FAN = 0x04,
 };
 
diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
index 87aba244da84..67768f6a50e0 100644
--- a/drivers/platform/x86/lenovo/wmi-other.c
+++ b/drivers/platform/x86/lenovo/wmi-other.c
@@ -65,6 +65,19 @@ enum lwmi_feature_id_cpu {
 	LWMI_FEATURE_ID_CPU_IPL = 0x09,
 };
 
+enum lwmi_feature_id_gpu {
+	LWMI_FEATURE_ID_GPU_NV_PPAB = 0x01,
+	LWMI_FEATURE_ID_GPU_NV_CTGP = 0x02,
+	LWMI_FEATURE_ID_GPU_TEMP = 0x03,
+	LWMI_FEATURE_ID_GPU_AC_OFFSET = 0x04,
+	LWMI_FEATURE_ID_DGPU_BOOST_CLK = 0x06,
+	LWMI_FEATURE_ID_DGPU_EN = 0x07,
+	LWMI_FEATURE_ID_GPU_MODE = 0x08,
+	LWMI_FEATURE_ID_DGPU_DIDVID = 0x09,
+	LWMI_FEATURE_ID_GPU_NV_BPL = 0x0a,
+	LWMI_FEATURE_ID_GPU_NV_CPU_BOOST = 0x0b,
+};
+
 #define LWMI_FEATURE_ID_FAN_RPM 0x03
 
 #define LWMI_TYPE_ID_NONE 0x00
@@ -631,6 +644,66 @@ static struct tunable_attr_01 ppt_pl4_ipl_cl = {
 	.type_id = LWMI_TYPE_ID_CROSSLOAD,
 };
 
+static struct tunable_attr_01 gpu_nv_ppab = {
+	.device_id = LWMI_DEVICE_ID_GPU,
+	.feature_id = LWMI_FEATURE_ID_GPU_NV_PPAB,
+	.type_id = LWMI_TYPE_ID_NONE,
+};
+
+static struct tunable_attr_01 gpu_nv_ctgp = {
+	.device_id = LWMI_DEVICE_ID_GPU,
+	.feature_id = LWMI_FEATURE_ID_GPU_NV_CTGP,
+	.type_id = LWMI_TYPE_ID_NONE,
+};
+
+static struct tunable_attr_01 gpu_temp = {
+	.device_id = LWMI_DEVICE_ID_GPU,
+	.feature_id = LWMI_FEATURE_ID_GPU_TEMP,
+	.type_id = LWMI_TYPE_ID_NONE,
+};
+
+static struct tunable_attr_01 gpu_nv_ac_offset = {
+	.device_id = LWMI_DEVICE_ID_GPU,
+	.feature_id = LWMI_FEATURE_ID_GPU_AC_OFFSET,
+	.type_id = LWMI_TYPE_ID_NONE,
+};
+
+static struct tunable_attr_01 dgpu_boost_clk = {
+	.device_id = LWMI_DEVICE_ID_GPU,
+	.feature_id = LWMI_FEATURE_ID_DGPU_BOOST_CLK,
+	.type_id = LWMI_TYPE_ID_NONE,
+};
+
+static struct tunable_attr_01 dgpu_enable = {
+	.device_id = LWMI_DEVICE_ID_GPU,
+	.feature_id = LWMI_FEATURE_ID_DGPU_EN,
+	.type_id = LWMI_TYPE_ID_NONE,
+};
+
+static struct tunable_attr_01 gpu_mode = {
+	.device_id = LWMI_DEVICE_ID_GPU,
+	.feature_id = LWMI_FEATURE_ID_GPU_MODE,
+	.type_id = LWMI_TYPE_ID_NONE,
+};
+
+static struct tunable_attr_01 dgpu_didvid = {
+	.device_id = LWMI_DEVICE_ID_GPU,
+	.feature_id = LWMI_FEATURE_ID_DGPU_DIDVID,
+	.type_id = LWMI_TYPE_ID_NONE,
+};
+
+static struct tunable_attr_01 gpu_nv_bpl = {
+	.device_id = LWMI_DEVICE_ID_GPU,
+	.feature_id = LWMI_FEATURE_ID_GPU_NV_BPL,
+	.type_id = LWMI_TYPE_ID_NONE,
+};
+
+static struct tunable_attr_01 gpu_nv_cpu_boost = {
+	.device_id = LWMI_DEVICE_ID_GPU,
+	.feature_id = LWMI_FEATURE_ID_GPU_NV_CPU_BOOST,
+	.type_id = LWMI_TYPE_ID_NONE,
+};
+
 struct capdata01_attr_group {
 	const struct attribute_group *attr_group;
 	struct tunable_attr_01 *tunable_attr;
@@ -1069,6 +1142,7 @@ static int lwmi_attr_01_is_supported(struct tunable_attr_01 *tunable_attr)
 		.name = _fsname, .attrs = _attrname##_attrs               \
 	}
 
+/* CPU tunable attributes */
 LWMI_ATTR_GROUP_TUNABLE_CAP01(cpu_temp, "cpu_temp",
 			      "Set the CPU thermal load limit");
 LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_cpu_cl, "ppt_cpu_cl",
@@ -1094,9 +1168,40 @@ LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl4_ipl, "ppt_pl4_ipl",
 LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl4_ipl_cl, "ppt_pl4_ipl_cl",
 			      "Set the CPU cross loading instantaneous power limit");
 
+/* GPU tunable attributes */
+LWMI_ATTR_GROUP_TUNABLE_CAP01(dgpu_boost_clk, "gpu_boost_clk",
+			      "Set the dedicated GPU boost clock");
+LWMI_ATTR_GROUP_TUNABLE_CAP01(dgpu_didvid, "gpu_didvid",
+			      "Get the GPU device identifier and vendor identifier");
+LWMI_ATTR_GROUP_TUNABLE_CAP01(dgpu_enable, "dgpu_enable",
+			      "Set the dedicated Nvidia GPU enabled status");
+LWMI_ATTR_GROUP_TUNABLE_CAP01(gpu_mode, "gpu_mode",
+			      "Set the GPU mode by power limit");
+LWMI_ATTR_GROUP_TUNABLE_CAP01(gpu_nv_ac_offset, "gpu_nv_ac_offset",
+			      "Set the Nvidia GPU AC total processing power baseline offset");
+LWMI_ATTR_GROUP_TUNABLE_CAP01(gpu_nv_bpl, "gpu_nv_bpl",
+			      "Set the Nvidia GPU base power limit");
+LWMI_ATTR_GROUP_TUNABLE_CAP01(gpu_nv_cpu_boost, "gpu_nv_cpu_boost",
+			      "Set the Nvidia GPU to CPU dynamic boost limit");
+LWMI_ATTR_GROUP_TUNABLE_CAP01(gpu_nv_ctgp, "gpu_nv_ctgp",
+			      "Set the GPU configurable total graphics power");
+LWMI_ATTR_GROUP_TUNABLE_CAP01(gpu_nv_ppab, "gpu_nv_ppab",
+			      "Set the Nvidia GPU power performance aware boost limit");
+LWMI_ATTR_GROUP_TUNABLE_CAP01(gpu_temp, "gpu_temp",
+			      "Set the GPU thermal load limit");
 
 static struct capdata01_attr_group cd01_attr_groups[] = {
 	{ &cpu_temp_attr_group, &cpu_temp },
+	{ &dgpu_boost_clk_attr_group, &dgpu_boost_clk },
+	{ &dgpu_didvid_attr_group, &dgpu_didvid },
+	{ &dgpu_enable_attr_group, &dgpu_enable },
+	{ &gpu_mode_attr_group, &gpu_mode },
+	{ &gpu_nv_ac_offset_attr_group, &gpu_nv_ac_offset },
+	{ &gpu_nv_bpl_attr_group, &gpu_nv_bpl },
+	{ &gpu_nv_cpu_boost_attr_group, &gpu_nv_cpu_boost },
+	{ &gpu_nv_ctgp_attr_group, &gpu_nv_ctgp },
+	{ &gpu_nv_ppab_attr_group, &gpu_nv_ppab },
+	{ &gpu_temp_attr_group, &gpu_temp },
 	{ &ppt_cpu_cl_attr_group, &ppt_cpu_cl },
 	{ &ppt_pl1_apu_spl_attr_group, &ppt_pl1_apu_spl },
 	{ &ppt_pl1_spl_attr_group, &ppt_pl1_spl },
-- 
2.52.0

[PATCH v3 5/6] platform-x86: lenovo-wmi-other: Rename LWMI_OM_FW_ATTR_BASE_PATH

[Derek J. Clark]

In the next patch a power supply extension is added which requires
a name attribute. Instead of creating another const macro with the
same information, replace LWMI_OM_FW_ATTR_BASE_PATH with
LWMI_OM_NAME and use that for firmware attributes and power supply
extension.

Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
 drivers/platform/x86/lenovo/wmi-other.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
index 67768f6a50e0..7f0d5a17b44f 100644
--- a/drivers/platform/x86/lenovo/wmi-other.c
+++ b/drivers/platform/x86/lenovo/wmi-other.c
@@ -90,13 +90,13 @@ enum lwmi_feature_id_gpu {
 #define LWMI_FAN_NR 4
 #define LWMI_FAN_ID(x) ((x) + LWMI_FAN_ID_BASE)
 
+#define LWMI_FAN_DIV 100
+
 #define LWMI_ATTR_ID_FAN_RPM(x)                                   \
 	LWMI_ATTR_ID(LWMI_DEVICE_ID_FAN, LWMI_FEATURE_ID_FAN_RPM, \
 		     LWMI_GZ_THERMAL_MODE_NONE, LWMI_FAN_ID(x))
 
-#define LWMI_FAN_DIV 100
-
-#define LWMI_OM_FW_ATTR_BASE_PATH "lenovo-wmi-other"
+#define LWMI_OM_SYSFS_NAME "lenovo-wmi-other"
 #define LWMI_OM_HWMON_NAME "lenovo_wmi_other"
 
 static BLOCKING_NOTIFIER_HEAD(om_chain_head);
@@ -1233,8 +1233,7 @@ static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
 
 	priv->fw_attr_dev = device_create(&firmware_attributes_class, NULL,
 					  MKDEV(0, 0), NULL, "%s-%u",
-					  LWMI_OM_FW_ATTR_BASE_PATH,
-					  priv->ida_id);
+					  LWMI_OM_SYSFS_NAME, priv->ida_id);
 	if (IS_ERR(priv->fw_attr_dev)) {
 		err = PTR_ERR(priv->fw_attr_dev);
 		goto err_free_ida;
-- 
2.52.0

[PATCH v3 6/6] platform/x86: lenovo-wmi-other: Add WMI battery charge limiting.

[Derek J. Clark]

Add charge-type power supply extension for devices that support WMI based
charge enable/disable. Lenovo Legion devices that implement function ID
and capdata 00 ID 0x03010001 are able to enable or disable charging
through the lenovo-wmi-other interface. The ideapad_laptop driver
conflicts with this if it can also provide the attribute, so we have to
get the acpi_handle and check for the same ACPI methods that enable the
feature in that driver. The ACPI method is more reliable from my testing
when both are present, so there is no need to modify the ideapad_laptop
driver instead.

Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
 drivers/platform/x86/lenovo/wmi-capdata.h |   1 +
 drivers/platform/x86/lenovo/wmi-other.c   | 230 ++++++++++++++++++++++
 2 files changed, 231 insertions(+)

diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
index b7f9ee7b301a..00471551e7d6 100644
--- a/drivers/platform/x86/lenovo/wmi-capdata.h
+++ b/drivers/platform/x86/lenovo/wmi-capdata.h
@@ -26,6 +26,7 @@
 enum lwmi_device_id {
 	LWMI_DEVICE_ID_CPU = 0x01,
 	LWMI_DEVICE_ID_GPU = 0x02,
+	LWMI_DEVICE_ID_PSU = 0x03,
 	LWMI_DEVICE_ID_FAN = 0x04,
 };
 
diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
index 7f0d5a17b44f..b2daff1b45c2 100644
--- a/drivers/platform/x86/lenovo/wmi-other.c
+++ b/drivers/platform/x86/lenovo/wmi-other.c
@@ -42,9 +42,12 @@
 #include <linux/module.h>
 #include <linux/notifier.h>
 #include <linux/platform_profile.h>
+#include <linux/power_supply.h>
 #include <linux/types.h>
 #include <linux/wmi.h>
 
+#include <acpi/battery.h>
+
 #include "wmi-capdata.h"
 #include "wmi-events.h"
 #include "wmi-gamezone.h"
@@ -78,10 +81,17 @@ enum lwmi_feature_id_gpu {
 	LWMI_FEATURE_ID_GPU_NV_CPU_BOOST = 0x0b,
 };
 
+enum lwmi_feature_id_psu {
+	LWMI_FEATURE_ID_PSU_INSTANT_MODE = 0x01,
+	LWMI_FEATURE_ID_PSU_CHARGE_MODE = 0x02,
+};
+
 #define LWMI_FEATURE_ID_FAN_RPM 0x03
 
 #define LWMI_TYPE_ID_NONE 0x00
 #define LWMI_TYPE_ID_CROSSLOAD 0x01
+#define LWMI_TYPE_ID_PSU_AC 0x01
+#define LWMI_TYPE_ID_PSU_PD 0x02
 
 #define LWMI_FEATURE_VALUE_GET 17
 #define LWMI_FEATURE_VALUE_SET 18
@@ -92,10 +102,17 @@ enum lwmi_feature_id_gpu {
 
 #define LWMI_FAN_DIV 100
 
+#define LWMI_CHARGE_MODE_ENABLED	0x00
+#define LWMI_CHARGE_MODE_DISABLED	0x01
+
 #define LWMI_ATTR_ID_FAN_RPM(x)                                   \
 	LWMI_ATTR_ID(LWMI_DEVICE_ID_FAN, LWMI_FEATURE_ID_FAN_RPM, \
 		     LWMI_GZ_THERMAL_MODE_NONE, LWMI_FAN_ID(x))
 
+#define LWMI_ATTR_ID_PSU(feat, type)		\
+	LWMI_ATTR_ID(LWMI_DEVICE_ID_PSU, feat,		\
+		     LWMI_GZ_THERMAL_MODE_NONE, type)
+
 #define LWMI_OM_SYSFS_NAME "lenovo-wmi-other"
 #define LWMI_OM_HWMON_NAME "lenovo_wmi_other"
 
@@ -137,6 +154,8 @@ struct lwmi_om_priv {
 		bool capdata00_collected : 1;
 		bool capdata_fan_collected : 1;
 	} fan_flags;
+
+	struct acpi_battery_hook battery_hook;
 };
 
 /*
@@ -561,6 +580,216 @@ static void lwmi_om_fan_info_collect_cd_fan(struct device *dev, struct cd_list *
 	lwmi_om_hwmon_add(priv);
 }
 
+/* ======== Power Supply Extension (component: lenovo-wmi-capdata 00) ======== */
+
+/**
+ * lwmi_psy_ext_get_prop() - Get a power_supply_ext property
+ * @ps: The battery that was extended
+ * @ext: The extension
+ * @ext_data: Pointer the lwmi_om_priv drvdata
+ * @prop: The property to read
+ * @val: The value to return
+ *
+ * Writes the given value to the power_supply_ext property
+ *
+ * Return: 0 on success, or an error
+ */
+static int lwmi_psy_ext_get_prop(struct power_supply *ps,
+				 const struct power_supply_ext *ext,
+				 void *ext_data,
+				 enum power_supply_property prop,
+				 union power_supply_propval *val)
+{
+	struct lwmi_om_priv *priv = ext_data;
+	struct wmi_method_args_32 args;
+	u32 retval;
+	int ret;
+
+	args.arg0 = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
+
+	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_GET,
+				    (unsigned char *)&args, sizeof(args),
+				    &retval);
+	if (ret)
+		return ret;
+
+	dev_dbg(&priv->wdev->dev, "Got return value %x for property %x\n", retval, prop);
+
+	if (retval == LWMI_CHARGE_MODE_DISABLED)
+		val->intval = POWER_SUPPLY_CHARGE_TYPE_LONGLIFE;
+	else
+		val->intval = POWER_SUPPLY_CHARGE_TYPE_STANDARD;
+
+	return 0;
+}
+
+/**
+ * lwmi_psy_ext_set_prop() - Set a power_supply_ext property
+ * @ps: The battery that was extended
+ * @ext: The extension
+ * @ext_data: Pointer the lwmi_om_priv drvdata
+ * @prop: The property to write
+ * @val: The value to write
+ *
+ * Writes the given value to the power_supply_ext property
+ *
+ * Return: 0 on success, or an error
+ */
+static int lwmi_psy_ext_set_prop(struct power_supply *ps,
+				 const struct power_supply_ext *ext,
+				 void *ext_data,
+				 enum power_supply_property prop,
+				 const union power_supply_propval *val)
+{
+	struct lwmi_om_priv *priv = ext_data;
+	struct wmi_method_args_32 args;
+
+	args.arg0 = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
+	if (val->intval == POWER_SUPPLY_CHARGE_TYPE_LONGLIFE)
+		args.arg1 = LWMI_CHARGE_MODE_DISABLED;
+	else
+		args.arg1 = LWMI_CHARGE_MODE_ENABLED;
+
+	dev_dbg(&priv->wdev->dev, "Attempting to set %#08x for property %x to %x\n",
+		args.arg0, prop, args.arg1);
+
+	return lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_SET,
+				     (unsigned char *)&args, sizeof(args), NULL);
+}
+
+/**
+ * lwmi_psy_prop_is_writeable() - Determine if the property is supported
+ * @ps: The battery that was extended
+ * @ext: The extension
+ * @ext_data: Pointer the lwmi_om_priv drvdata
+ * @prop: The property to check
+ *
+ * Checks capdata 00 to determine if the property is supported.
+ *
+ * Return: Support level, or false
+ */
+static int lwmi_psy_prop_is_writeable(struct power_supply *ps,
+				      const struct power_supply_ext *ext,
+				      void *ext_data,
+				      enum power_supply_property prop)
+{
+	struct lwmi_om_priv *priv = ext_data;
+	struct capdata00 capdata;
+	u32 attribute_id = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
+	int ret;
+
+	ret = lwmi_cd00_get_data(priv->cd00_list, attribute_id, &capdata);
+	if (ret)
+		return false;
+
+	dev_dbg(&priv->wdev->dev, "Battery charge mode (%#08x) support level: %x\n",
+		attribute_id, capdata.supported);
+
+	return capdata.supported;
+}
+
+static const enum power_supply_property lwmi_psy_ext_props[] = {
+	POWER_SUPPLY_PROP_CHARGE_TYPES,
+};
+
+static const struct power_supply_ext lwmi_psy_ext = {
+	.name			= LWMI_OM_SYSFS_NAME,
+	.properties		= lwmi_psy_ext_props,
+	.num_properties		= ARRAY_SIZE(lwmi_psy_ext_props),
+	.charge_types		= (BIT(POWER_SUPPLY_CHARGE_TYPE_STANDARD) |
+				   BIT(POWER_SUPPLY_CHARGE_TYPE_LONGLIFE)),
+	.get_property		= lwmi_psy_ext_get_prop,
+	.set_property		= lwmi_psy_ext_set_prop,
+	.property_is_writeable	= lwmi_psy_prop_is_writeable,
+};
+
+/**
+ * lwmi_add_battery() - Connect the power_supply_ext
+ * @battery: The battery to extend
+ * @hook: The driver hook used to extend the battery
+ *
+ * Return: 0 on success, or an error.
+ */
+static int lwmi_add_battery(struct power_supply *battery, struct acpi_battery_hook *hook)
+{
+	struct lwmi_om_priv *priv = container_of(hook, struct lwmi_om_priv, battery_hook);
+
+	return power_supply_register_extension(battery, &lwmi_psy_ext, &priv->wdev->dev, priv);
+}
+
+/**
+ * lwmi_remove_battery() - Disconnect the power_supply_ext
+ * @battery: The battery that was extended
+ * @hook: The driver hook used to extend the battery
+ *
+ * Return: 0 on success, or an error.
+ */
+static int lwmi_remove_battery(struct power_supply *battery, struct acpi_battery_hook *hook)
+{
+	power_supply_unregister_extension(battery, &lwmi_psy_ext);
+	return 0;
+}
+
+/**
+ * lwmi_acpi_match() - Attempts to return the ideapad acpi handle
+ * @handle: The ACPI handle that manages battery charging
+ * @lvl: Unused
+ * @context: Void pointer to the acpi_handle object to return
+ * @retval: Unused
+ *
+ * Checks if the ideapad_laptop driver is going to manage charge_type first,
+ * then if not, hooks the battery to our WMI methods.
+ *
+ * Return: AE_CTRL_TERMINATE if found, AE_OK if not found.
+ */
+static acpi_status lwmi_acpi_match(acpi_handle handle, u32 lvl,
+				   void *context, void **retval)
+{
+	if (!handle)
+		return AE_OK;
+
+	acpi_handle *ahand = context;
+	*ahand = handle;
+
+	return AE_CTRL_TERMINATE;
+}
+
+/**
+ * lwmi_om_ps_ext_init() - Hooks power supply extension to device battery
+ * @priv: Driver private data
+ *
+ * Checks if the ideapad_laptop driver is going to manage charge_type first,
+ * then if not, hooks the battery to our WMI methods.
+ */
+static void lwmi_om_ps_ext_init(struct lwmi_om_priv *priv)
+{
+	static const char * const ideapad_hid = "VPC2004";
+	acpi_handle handle = NULL;
+	int ret;
+
+	/* Deconflict ideapad_laptop driver */
+	ret = acpi_get_devices(ideapad_hid, lwmi_acpi_match, &handle, NULL);
+	if (ret)
+		return;
+
+	if (!handle)
+		return;
+
+	if (acpi_has_method(handle, "GBMD") && acpi_has_method(handle, "SBMC")) {
+		dev_dbg(&priv->wdev->dev, "ideapad_laptop driver manages battery for device.\n");
+		return;
+	}
+
+	/* Add battery hooks */
+	priv->battery_hook.add_battery	= lwmi_add_battery,
+	priv->battery_hook.remove_battery	= lwmi_remove_battery,
+	priv->battery_hook.name		= "Lenovo WMI Other Battery Extension",
+
+	ret = devm_battery_hook_register(&priv->wdev->dev, &priv->battery_hook);
+	if (ret)
+		dev_err(&priv->wdev->dev, "Error during battery hook: %i\n", ret);
+}
+
 /* ======== fw_attributes (component: lenovo-wmi-capdata 01) ======== */
 
 struct tunable_attr_01 {
@@ -1325,6 +1554,7 @@ static int lwmi_om_master_bind(struct device *dev)
 		return -ENODEV;
 
 	lwmi_om_fan_info_collect_cd00(priv);
+	lwmi_om_ps_ext_init(priv);
 
 	return lwmi_om_fw_attr_add(priv);
 }
-- 
2.52.0

Re: [PATCH v3 4/6] platform/x86: lenovo-wmi-other: Add GPU tunable attributes

[Derek J. Clark]

On February 23, 2026 8:31:58 PM PST, "Derek J. Clark" <derekjohn.clark@gmail.com> wrote:
>Use an enum for all GPU attribute feature ID's and add GPU attributes.
>
>Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
>Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
>---
> .../wmi/devices/lenovo-wmi-other.rst          |  11 ++
> drivers/platform/x86/lenovo/wmi-capdata.h     |   1 +
> drivers/platform/x86/lenovo/wmi-other.c       | 105 ++++++++++++++++++
> 3 files changed, 117 insertions(+)
>
>diff --git a/Documentation/wmi/devices/lenovo-wmi-other.rst b/Documentation/wmi/devices/lenovo-wmi-other.rst
>index f4763ed66cc6..f7564b23bb7f 100644
>--- a/Documentation/wmi/devices/lenovo-wmi-other.rst
>+++ b/Documentation/wmi/devices/lenovo-wmi-other.rst
>@@ -70,6 +70,17 @@ Each attribute has the following properties:
> The following firmware-attributes are implemented:
>  - cpu_oc_stat: CPU Overlocking Status

I forgot to remove CPU OC status from the documentation, reminding myself for v4.

>  - cpu_temp: CPU Thermal Load Limit
>+ - dgpu_boost_clk: Dedicated GPU Boost Clock
>+ - dgpu_enable: Dedicated GPU Enabled Status
>+ - gpu_didvid: GPU Device Identifier and Vendor Identifier
>+ - gpu_mode: GPU Mode by Power Limit
>+ - gpu_nv_ac_offset: Nvidia GPU AC Total Processing Power Baseline Offset
>+ - gpu_nv_bpl: Nvidia GPU Base Power Limit
>+ - gpu_nv_cpu_boost: Nvidia GPU to CPU Dynamic Boost Limit
>+ - gpu_nv_ctgp: Nvidia GPU Configurable Total Graphics Power
>+ - gpu_nv_ppab: Nvidia GPU Power Performance Aware Boost Limit
>+ - gpu_oc_stat: GPU Overclocking Status

Same for GPU OC Status 

- Derek
>+ - gpu_temp: GPU Thermal Load Limit
>  - ppt_cpu_cl: CPU Cross Loading Power Limit
>  - ppt_pl1_apu_spl: Platform Profile Tracking APU Sustained Power Limit
>  - ppt_pl1_spl: Platform Profile Tracking Sustained Power Limit
>diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
>index aa48f43cbb43..b7f9ee7b301a 100644
>--- a/drivers/platform/x86/lenovo/wmi-capdata.h
>+++ b/drivers/platform/x86/lenovo/wmi-capdata.h
>@@ -25,6 +25,7 @@
> 
> enum lwmi_device_id {
> 	LWMI_DEVICE_ID_CPU = 0x01,
>+	LWMI_DEVICE_ID_GPU = 0x02,
> 	LWMI_DEVICE_ID_FAN = 0x04,
> };
> 
>diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
>index 87aba244da84..67768f6a50e0 100644
>--- a/drivers/platform/x86/lenovo/wmi-other.c
>+++ b/drivers/platform/x86/lenovo/wmi-other.c
>@@ -65,6 +65,19 @@ enum lwmi_feature_id_cpu {
> 	LWMI_FEATURE_ID_CPU_IPL = 0x09,
> };
> 
>+enum lwmi_feature_id_gpu {
>+	LWMI_FEATURE_ID_GPU_NV_PPAB = 0x01,
>+	LWMI_FEATURE_ID_GPU_NV_CTGP = 0x02,
>+	LWMI_FEATURE_ID_GPU_TEMP = 0x03,
>+	LWMI_FEATURE_ID_GPU_AC_OFFSET = 0x04,
>+	LWMI_FEATURE_ID_DGPU_BOOST_CLK = 0x06,
>+	LWMI_FEATURE_ID_DGPU_EN = 0x07,
>+	LWMI_FEATURE_ID_GPU_MODE = 0x08,
>+	LWMI_FEATURE_ID_DGPU_DIDVID = 0x09,
>+	LWMI_FEATURE_ID_GPU_NV_BPL = 0x0a,
>+	LWMI_FEATURE_ID_GPU_NV_CPU_BOOST = 0x0b,
>+};
>+
> #define LWMI_FEATURE_ID_FAN_RPM 0x03
> 
> #define LWMI_TYPE_ID_NONE 0x00
>@@ -631,6 +644,66 @@ static struct tunable_attr_01 ppt_pl4_ipl_cl = {
> 	.type_id = LWMI_TYPE_ID_CROSSLOAD,
> };
> 
>+static struct tunable_attr_01 gpu_nv_ppab = {
>+	.device_id = LWMI_DEVICE_ID_GPU,
>+	.feature_id = LWMI_FEATURE_ID_GPU_NV_PPAB,
>+	.type_id = LWMI_TYPE_ID_NONE,
>+};
>+
>+static struct tunable_attr_01 gpu_nv_ctgp = {
>+	.device_id = LWMI_DEVICE_ID_GPU,
>+	.feature_id = LWMI_FEATURE_ID_GPU_NV_CTGP,
>+	.type_id = LWMI_TYPE_ID_NONE,
>+};
>+
>+static struct tunable_attr_01 gpu_temp = {
>+	.device_id = LWMI_DEVICE_ID_GPU,
>+	.feature_id = LWMI_FEATURE_ID_GPU_TEMP,
>+	.type_id = LWMI_TYPE_ID_NONE,
>+};
>+
>+static struct tunable_attr_01 gpu_nv_ac_offset = {
>+	.device_id = LWMI_DEVICE_ID_GPU,
>+	.feature_id = LWMI_FEATURE_ID_GPU_AC_OFFSET,
>+	.type_id = LWMI_TYPE_ID_NONE,
>+};
>+
>+static struct tunable_attr_01 dgpu_boost_clk = {
>+	.device_id = LWMI_DEVICE_ID_GPU,
>+	.feature_id = LWMI_FEATURE_ID_DGPU_BOOST_CLK,
>+	.type_id = LWMI_TYPE_ID_NONE,
>+};
>+
>+static struct tunable_attr_01 dgpu_enable = {
>+	.device_id = LWMI_DEVICE_ID_GPU,
>+	.feature_id = LWMI_FEATURE_ID_DGPU_EN,
>+	.type_id = LWMI_TYPE_ID_NONE,
>+};
>+
>+static struct tunable_attr_01 gpu_mode = {
>+	.device_id = LWMI_DEVICE_ID_GPU,
>+	.feature_id = LWMI_FEATURE_ID_GPU_MODE,
>+	.type_id = LWMI_TYPE_ID_NONE,
>+};
>+
>+static struct tunable_attr_01 dgpu_didvid = {
>+	.device_id = LWMI_DEVICE_ID_GPU,
>+	.feature_id = LWMI_FEATURE_ID_DGPU_DIDVID,
>+	.type_id = LWMI_TYPE_ID_NONE,
>+};
>+
>+static struct tunable_attr_01 gpu_nv_bpl = {
>+	.device_id = LWMI_DEVICE_ID_GPU,
>+	.feature_id = LWMI_FEATURE_ID_GPU_NV_BPL,
>+	.type_id = LWMI_TYPE_ID_NONE,
>+};
>+
>+static struct tunable_attr_01 gpu_nv_cpu_boost = {
>+	.device_id = LWMI_DEVICE_ID_GPU,
>+	.feature_id = LWMI_FEATURE_ID_GPU_NV_CPU_BOOST,
>+	.type_id = LWMI_TYPE_ID_NONE,
>+};
>+
> struct capdata01_attr_group {
> 	const struct attribute_group *attr_group;
> 	struct tunable_attr_01 *tunable_attr;
>@@ -1069,6 +1142,7 @@ static int lwmi_attr_01_is_supported(struct tunable_attr_01 *tunable_attr)
> 		.name = _fsname, .attrs = _attrname##_attrs               \
> 	}
> 
>+/* CPU tunable attributes */
> LWMI_ATTR_GROUP_TUNABLE_CAP01(cpu_temp, "cpu_temp",
> 			      "Set the CPU thermal load limit");
> LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_cpu_cl, "ppt_cpu_cl",
>@@ -1094,9 +1168,40 @@ LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl4_ipl, "ppt_pl4_ipl",
> LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl4_ipl_cl, "ppt_pl4_ipl_cl",
> 			      "Set the CPU cross loading instantaneous power limit");
> 
>+/* GPU tunable attributes */
>+LWMI_ATTR_GROUP_TUNABLE_CAP01(dgpu_boost_clk, "gpu_boost_clk",
>+			      "Set the dedicated GPU boost clock");
>+LWMI_ATTR_GROUP_TUNABLE_CAP01(dgpu_didvid, "gpu_didvid",
>+			      "Get the GPU device identifier and vendor identifier");
>+LWMI_ATTR_GROUP_TUNABLE_CAP01(dgpu_enable, "dgpu_enable",
>+			      "Set the dedicated Nvidia GPU enabled status");
>+LWMI_ATTR_GROUP_TUNABLE_CAP01(gpu_mode, "gpu_mode",
>+			      "Set the GPU mode by power limit");
>+LWMI_ATTR_GROUP_TUNABLE_CAP01(gpu_nv_ac_offset, "gpu_nv_ac_offset",
>+			      "Set the Nvidia GPU AC total processing power baseline offset");
>+LWMI_ATTR_GROUP_TUNABLE_CAP01(gpu_nv_bpl, "gpu_nv_bpl",
>+			      "Set the Nvidia GPU base power limit");
>+LWMI_ATTR_GROUP_TUNABLE_CAP01(gpu_nv_cpu_boost, "gpu_nv_cpu_boost",
>+			      "Set the Nvidia GPU to CPU dynamic boost limit");
>+LWMI_ATTR_GROUP_TUNABLE_CAP01(gpu_nv_ctgp, "gpu_nv_ctgp",
>+			      "Set the GPU configurable total graphics power");
>+LWMI_ATTR_GROUP_TUNABLE_CAP01(gpu_nv_ppab, "gpu_nv_ppab",
>+			      "Set the Nvidia GPU power performance aware boost limit");
>+LWMI_ATTR_GROUP_TUNABLE_CAP01(gpu_temp, "gpu_temp",
>+			      "Set the GPU thermal load limit");
> 
> static struct capdata01_attr_group cd01_attr_groups[] = {
> 	{ &cpu_temp_attr_group, &cpu_temp },
>+	{ &dgpu_boost_clk_attr_group, &dgpu_boost_clk },
>+	{ &dgpu_didvid_attr_group, &dgpu_didvid },
>+	{ &dgpu_enable_attr_group, &dgpu_enable },
>+	{ &gpu_mode_attr_group, &gpu_mode },
>+	{ &gpu_nv_ac_offset_attr_group, &gpu_nv_ac_offset },
>+	{ &gpu_nv_bpl_attr_group, &gpu_nv_bpl },
>+	{ &gpu_nv_cpu_boost_attr_group, &gpu_nv_cpu_boost },
>+	{ &gpu_nv_ctgp_attr_group, &gpu_nv_ctgp },
>+	{ &gpu_nv_ppab_attr_group, &gpu_nv_ppab },
>+	{ &gpu_temp_attr_group, &gpu_temp },
> 	{ &ppt_cpu_cl_attr_group, &ppt_cpu_cl },
> 	{ &ppt_pl1_apu_spl_attr_group, &ppt_pl1_apu_spl },
> 	{ &ppt_pl1_spl_attr_group, &ppt_pl1_spl },

Re: [PATCH v3 1/6] platform/x86: lenovo-wmi-other: Add LWMI_ATTR_ID Macro

[Ilpo Järvinen]

On Tue, 24 Feb 2026, Derek J. Clark wrote:

> Adds LWMI_ATTR_ID macro. In the same vein as LWMI_ATTR_ID_FAN_RPM,
> but as a generic, to de-duplicate attribute_id assignment biolerplate.
> 
> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
> ---
>  drivers/platform/x86/lenovo/wmi-capdata.h  |  6 ++++
>  drivers/platform/x86/lenovo/wmi-gamezone.h |  1 +
>  drivers/platform/x86/lenovo/wmi-other.c    | 36 ++++++----------------
>  3 files changed, 17 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
> index 8c1df3efcc55..27202e2dc8a5 100644
> --- a/drivers/platform/x86/lenovo/wmi-capdata.h
> +++ b/drivers/platform/x86/lenovo/wmi-capdata.h
> @@ -17,6 +17,12 @@
>  #define LWMI_ATTR_MODE_ID_MASK	GENMASK(15, 8)
>  #define LWMI_ATTR_TYPE_ID_MASK	GENMASK(7, 0)
>  
> +#define LWMI_ATTR_ID(dev, feat, mode, type) \
> +	(FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, dev) | \
> +	 FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, feat) | \
> +	 FIELD_PREP(LWMI_ATTR_MODE_ID_MASK, mode) | \
> +	 FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, type))

Why does this need to be macro? Static inline would have proper types.

> +
>  #define LWMI_DEVICE_ID_FAN	0x04
>  
>  struct component_match;
> diff --git a/drivers/platform/x86/lenovo/wmi-gamezone.h b/drivers/platform/x86/lenovo/wmi-gamezone.h
> index 6b163a5eeb95..ddb919cf6c36 100644
> --- a/drivers/platform/x86/lenovo/wmi-gamezone.h
> +++ b/drivers/platform/x86/lenovo/wmi-gamezone.h
> @@ -10,6 +10,7 @@ enum gamezone_events_type {
>  };
>  
>  enum thermal_mode {
> +	LWMI_GZ_THERMAL_MODE_NONE =	   0x00,
>  	LWMI_GZ_THERMAL_MODE_QUIET =	   0x01,
>  	LWMI_GZ_THERMAL_MODE_BALANCED =	   0x02,
>  	LWMI_GZ_THERMAL_MODE_PERFORMANCE = 0x03,
> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> index 6040f45aa2b0..95886df39c8d 100644
> --- a/drivers/platform/x86/lenovo/wmi-other.c
> +++ b/drivers/platform/x86/lenovo/wmi-other.c
> @@ -71,10 +71,9 @@
>  #define LWMI_FAN_NR 4
>  #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_ATTR_ID_FAN_RPM(x)                                   \
> +	LWMI_ATTR_ID(LWMI_DEVICE_ID_FAN, LWMI_FEATURE_ID_FAN_RPM, \
> +		     LWMI_GZ_THERMAL_MODE_NONE, LWMI_FAN_ID(x))
>  
>  #define LWMI_FAN_DIV 100
>  
> @@ -716,12 +715,8 @@ static ssize_t attr_capdata01_show(struct kobject *kobj,
>  	u32 attribute_id;
>  	int value, ret;
>  
> -	attribute_id =
> -		FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, tunable_attr->device_id) |
> -		FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, tunable_attr->feature_id) |
> -		FIELD_PREP(LWMI_ATTR_MODE_ID_MASK,
> -			   LWMI_GZ_THERMAL_MODE_CUSTOM) |
> -		FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, tunable_attr->type_id);
> +	attribute_id = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> +				    LWMI_GZ_THERMAL_MODE_CUSTOM, tunable_attr->type_id);
>  
>  	ret = lwmi_cd01_get_data(priv->cd01_list, attribute_id, &capdata);
>  	if (ret)
> @@ -776,7 +771,6 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
>  	struct wmi_method_args_32 args;
>  	struct capdata01 capdata;
>  	enum thermal_mode mode;
> -	u32 attribute_id;
>  	u32 value;
>  	int ret;
>  
> @@ -787,13 +781,10 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
>  	if (mode != LWMI_GZ_THERMAL_MODE_CUSTOM)
>  		return -EBUSY;
>  
> -	attribute_id =
> -		FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, tunable_attr->device_id) |
> -		FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, tunable_attr->feature_id) |
> -		FIELD_PREP(LWMI_ATTR_MODE_ID_MASK, mode) |
> -		FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, tunable_attr->type_id);
> +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> +				 mode, tunable_attr->type_id);
>  
> -	ret = lwmi_cd01_get_data(priv->cd01_list, attribute_id, &capdata);
> +	ret = lwmi_cd01_get_data(priv->cd01_list, args.arg0, &capdata);
>  	if (ret)
>  		return ret;
>  
> @@ -804,7 +795,6 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
>  	if (value < capdata.min_value || value > capdata.max_value)
>  		return -EINVAL;
>  
> -	args.arg0 = attribute_id;
>  	args.arg1 = value;
>  
>  	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_SET,
> @@ -838,7 +828,6 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
>  	struct lwmi_om_priv *priv = dev_get_drvdata(tunable_attr->dev);
>  	struct wmi_method_args_32 args;
>  	enum thermal_mode mode;
> -	u32 attribute_id;
>  	int retval;
>  	int ret;
>  
> @@ -846,13 +835,8 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
>  	if (ret)
>  		return ret;
>  
> -	attribute_id =
> -		FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, tunable_attr->device_id) |
> -		FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, tunable_attr->feature_id) |
> -		FIELD_PREP(LWMI_ATTR_MODE_ID_MASK, mode) |
> -		FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, tunable_attr->type_id);
> -
> -	args.arg0 = attribute_id;
> +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> +				 mode, tunable_attr->type_id);
>  
>  	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_GET,
>  				    (unsigned char *)&args, sizeof(args),
> 

-- 
 i.

Re: [PATCH v3 1/6] platform/x86: lenovo-wmi-other: Add LWMI_ATTR_ID Macro

[Ilpo Järvinen]

On Tue, 24 Feb 2026, Derek J. Clark wrote:

> Adds LWMI_ATTR_ID macro. In the same vein as LWMI_ATTR_ID_FAN_RPM,

Add () after any function (or like).

> but as a generic, to de-duplicate attribute_id assignment biolerplate.
> 
> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
> ---
>  drivers/platform/x86/lenovo/wmi-capdata.h  |  6 ++++
>  drivers/platform/x86/lenovo/wmi-gamezone.h |  1 +
>  drivers/platform/x86/lenovo/wmi-other.c    | 36 ++++++----------------
>  3 files changed, 17 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
> index 8c1df3efcc55..27202e2dc8a5 100644
> --- a/drivers/platform/x86/lenovo/wmi-capdata.h
> +++ b/drivers/platform/x86/lenovo/wmi-capdata.h
> @@ -17,6 +17,12 @@
>  #define LWMI_ATTR_MODE_ID_MASK	GENMASK(15, 8)
>  #define LWMI_ATTR_TYPE_ID_MASK	GENMASK(7, 0)
>  
> +#define LWMI_ATTR_ID(dev, feat, mode, type) \
> +	(FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, dev) | \
> +	 FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, feat) | \
> +	 FIELD_PREP(LWMI_ATTR_MODE_ID_MASK, mode) | \
> +	 FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, type))
> +
>  #define LWMI_DEVICE_ID_FAN	0x04
>  
>  struct component_match;
> diff --git a/drivers/platform/x86/lenovo/wmi-gamezone.h b/drivers/platform/x86/lenovo/wmi-gamezone.h
> index 6b163a5eeb95..ddb919cf6c36 100644
> --- a/drivers/platform/x86/lenovo/wmi-gamezone.h
> +++ b/drivers/platform/x86/lenovo/wmi-gamezone.h
> @@ -10,6 +10,7 @@ enum gamezone_events_type {
>  };
>  
>  enum thermal_mode {
> +	LWMI_GZ_THERMAL_MODE_NONE =	   0x00,
>  	LWMI_GZ_THERMAL_MODE_QUIET =	   0x01,
>  	LWMI_GZ_THERMAL_MODE_BALANCED =	   0x02,
>  	LWMI_GZ_THERMAL_MODE_PERFORMANCE = 0x03,
> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> index 6040f45aa2b0..95886df39c8d 100644
> --- a/drivers/platform/x86/lenovo/wmi-other.c
> +++ b/drivers/platform/x86/lenovo/wmi-other.c
> @@ -71,10 +71,9 @@
>  #define LWMI_FAN_NR 4
>  #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_ATTR_ID_FAN_RPM(x)                                   \
> +	LWMI_ATTR_ID(LWMI_DEVICE_ID_FAN, LWMI_FEATURE_ID_FAN_RPM, \
> +		     LWMI_GZ_THERMAL_MODE_NONE, LWMI_FAN_ID(x))
>  
>  #define LWMI_FAN_DIV 100
>  
> @@ -716,12 +715,8 @@ static ssize_t attr_capdata01_show(struct kobject *kobj,
>  	u32 attribute_id;
>  	int value, ret;
>  
> -	attribute_id =
> -		FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, tunable_attr->device_id) |
> -		FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, tunable_attr->feature_id) |
> -		FIELD_PREP(LWMI_ATTR_MODE_ID_MASK,
> -			   LWMI_GZ_THERMAL_MODE_CUSTOM) |
> -		FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, tunable_attr->type_id);
> +	attribute_id = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> +				    LWMI_GZ_THERMAL_MODE_CUSTOM, tunable_attr->type_id);
>  
>  	ret = lwmi_cd01_get_data(priv->cd01_list, attribute_id, &capdata);
>  	if (ret)
> @@ -776,7 +771,6 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
>  	struct wmi_method_args_32 args;
>  	struct capdata01 capdata;
>  	enum thermal_mode mode;
> -	u32 attribute_id;
>  	u32 value;
>  	int ret;
>  
> @@ -787,13 +781,10 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
>  	if (mode != LWMI_GZ_THERMAL_MODE_CUSTOM)
>  		return -EBUSY;
>  
> -	attribute_id =
> -		FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, tunable_attr->device_id) |
> -		FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, tunable_attr->feature_id) |
> -		FIELD_PREP(LWMI_ATTR_MODE_ID_MASK, mode) |
> -		FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, tunable_attr->type_id);
> +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> +				 mode, tunable_attr->type_id);
>  
> -	ret = lwmi_cd01_get_data(priv->cd01_list, attribute_id, &capdata);
> +	ret = lwmi_cd01_get_data(priv->cd01_list, args.arg0, &capdata);
>  	if (ret)
>  		return ret;
>  
> @@ -804,7 +795,6 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
>  	if (value < capdata.min_value || value > capdata.max_value)
>  		return -EINVAL;
>  
> -	args.arg0 = attribute_id;
>  	args.arg1 = value;
>  
>  	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_SET,
> @@ -838,7 +828,6 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
>  	struct lwmi_om_priv *priv = dev_get_drvdata(tunable_attr->dev);
>  	struct wmi_method_args_32 args;
>  	enum thermal_mode mode;
> -	u32 attribute_id;
>  	int retval;
>  	int ret;
>  
> @@ -846,13 +835,8 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
>  	if (ret)
>  		return ret;
>  
> -	attribute_id =
> -		FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, tunable_attr->device_id) |
> -		FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, tunable_attr->feature_id) |
> -		FIELD_PREP(LWMI_ATTR_MODE_ID_MASK, mode) |
> -		FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, tunable_attr->type_id);
> -
> -	args.arg0 = attribute_id;
> +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> +				 mode, tunable_attr->type_id);
>  
>  	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_GET,
>  				    (unsigned char *)&args, sizeof(args),
> 

-- 
 i.

Re: [PATCH v3 2/6] platform/x86: lenovo-wmi-other: Limit adding attributes to supported devices

[Ilpo Järvinen]

On Tue, 24 Feb 2026, Derek J. Clark wrote:

> Adds lwmi_is_attr_01_supported, and only creates the attribute subfolder
> if the attribute is supported by the hardware. Due to some poorly
> implemented BIOS, this is a multi-step sequence of events. This is
> because:
> - Some BIOS support getting the capability data from custom mode (0xff),
>   while others only support it in no-mode (0x00).
> - Similarly, some BIOS support get/set for the current value from custom
>   mode (0xff), while others only support it in no-mode (0x00).
> - Some BIOS report capability data for a method that is not fully
>   implemented.
> - Some BIOS have methods fully implemented, but no complimentary
>   capability data.
> 
> To ensure we only expose fully implemented methods with corresponding
> capability data, we check each outcome before reporting that an
> attribute can be supported.
> 
> Checking for lwmi_is_attr_01_supported during remove is not done to
> ensure that we don't attempt to call cd01 or send WMI events if one of
> the interfaces being removed was the cause of the driver unloading.
> 

> While adding members to tunable_attr_01, remove unused capdata pointer
> and limit size of all ID's to the appropriate size.

Please don't mix changes like this. Create a seprate patch for it.

> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Reported-by: Kurt Borja <kuurtb@gmail.com>
> Closes: https://lore.kernel.org/platform-driver-x86/DG60P3SHXR8H.3NSEHMZ6J7XRC@gmail.com/
> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
> ---
>  drivers/platform/x86/lenovo/wmi-other.c | 117 +++++++++++++++++++++---
>  1 file changed, 102 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> index 95886df39c8d..f3f12303e379 100644
> --- a/drivers/platform/x86/lenovo/wmi-other.c
> +++ b/drivers/platform/x86/lenovo/wmi-other.c
> @@ -545,11 +545,12 @@ static void lwmi_om_fan_info_collect_cd_fan(struct device *dev, struct cd_list *
>  /* ======== fw_attributes (component: lenovo-wmi-capdata 01) ======== */
>  
>  struct tunable_attr_01 {
> -	struct capdata01 *capdata;
>  	struct device *dev;
> -	u32 feature_id;
> -	u32 device_id;
> -	u32 type_id;
> +	u8 feature_id;
> +	u8 device_id;
> +	u8 type_id;
> +	u8 cd_mode_id; /* mode arg for searching capdata */
> +	u8 cv_mode_id; /* mode arg for set/get current_value */
>  };
>  
>  static struct tunable_attr_01 ppt_pl1_spl = {
> @@ -716,7 +717,7 @@ static ssize_t attr_capdata01_show(struct kobject *kobj,
>  	int value, ret;
>  
>  	attribute_id = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> -				    LWMI_GZ_THERMAL_MODE_CUSTOM, tunable_attr->type_id);
> +				    tunable_attr->cd_mode_id, tunable_attr->type_id);
>  
>  	ret = lwmi_cd01_get_data(priv->cd01_list, attribute_id, &capdata);
>  	if (ret)
> @@ -782,7 +783,7 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
>  		return -EBUSY;
>  
>  	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> -				 mode, tunable_attr->type_id);
> +				 tunable_attr->cd_mode_id, tunable_attr->type_id);
>  
>  	ret = lwmi_cd01_get_data(priv->cd01_list, args.arg0, &capdata);
>  	if (ret)
> @@ -795,6 +796,8 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
>  	if (value < capdata.min_value || value > capdata.max_value)
>  		return -EINVAL;
>  
> +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> +				 tunable_attr->cv_mode_id, tunable_attr->type_id);
>  	args.arg1 = value;
>  
>  	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_SET,
> @@ -828,13 +831,16 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
>  	struct lwmi_om_priv *priv = dev_get_drvdata(tunable_attr->dev);
>  	struct wmi_method_args_32 args;
>  	enum thermal_mode mode;
> -	int retval;
> -	int ret;
> +	int retval, ret;
>  
>  	ret = lwmi_om_notifier_call(&mode);
>  	if (ret)
>  		return ret;
>  
> +	/* If "no-mode" is the supported mode, ensure we never send current mode */
> +	if (tunable_attr->cv_mode_id == LWMI_GZ_THERMAL_MODE_NONE)
> +		mode = tunable_attr->cv_mode_id;
> +
>  	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
>  				 mode, tunable_attr->type_id);
>  
> @@ -847,6 +853,85 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
>  	return sysfs_emit(buf, "%d\n", retval);
>  }
>  
> +/**
> + * lwmi_attr_01_is_supported() - Determine if the given attribute is supported.
> + * @tunable_attr: The attribute to verify.
> + *
> + * First check if the attribute has a corresponding capdata01 table in the cd01
> + * module under the "custom" mode (0xff). If that is not present then check if
> + * there is a corresponding "no-mode" (0x00) entry. If either of those passes,
> + * check capdata->supported for values > 0. If capdata is available, attempt to
> + * determine the set/get mode for the current value property using a similar
> + * pattern. If the value returned by either custom or no-mode is 0, or we get
> + * an error, we assume that mode is not supported. If any of the above checks
> + * fail then the attribute is not fully supported.
> + *
> + * The probed cd_mode_id/cv_mode_id are stored on the tunable_attr for later
> + * reference.
> + *
> + * Return: Support level, or an error code.
> + */
> +static int lwmi_attr_01_is_supported(struct tunable_attr_01 *tunable_attr)
> +{
> +	struct lwmi_om_priv *priv = dev_get_drvdata(tunable_attr->dev);
> +	u8 mode = LWMI_GZ_THERMAL_MODE_CUSTOM;
> +	struct wmi_method_args_32 args;
> +	struct capdata01 capdata;
> +	int retval, ret;
> +
> +	/* Determine tunable_attr->cd_mode_id */
> +no_mode_fallback_1:
> +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> +				 mode, tunable_attr->type_id);
> +
> +	ret = lwmi_cd01_get_data(priv->cd01_list, args.arg0, &capdata);
> +	if (ret && mode) {
> +		dev_dbg(tunable_attr->dev, "Attribute id %x not supported\n", args.arg0);

Add include.

> +		mode = LWMI_GZ_THERMAL_MODE_NONE;
> +		goto no_mode_fallback_1;

Is it possible to make a helper so you don't need these back gotos?

> +	}
> +	if (ret)
> +		goto not_supported;
> +	if (!capdata.supported) {
> +		ret = -EOPNOTSUPP;
> +		goto not_supported;
> +	}
> +
> +	tunable_attr->cd_mode_id = mode;
> +
> +	/* Determine tunable_attr->cv_mode_id */
> +	mode = LWMI_GZ_THERMAL_MODE_CUSTOM;
> +no_mode_fallback_2:
> +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> +				 mode, tunable_attr->type_id);
> +
> +	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_GET,
> +				    (unsigned char *)&args, sizeof(args),
> +				    &retval);
> +	if ((ret && mode) || (!retval && mode)) {
> +		dev_dbg(tunable_attr->dev, "Attribute id %x not supported\n", args.arg0);
> +		mode = LWMI_GZ_THERMAL_MODE_NONE;
> +		goto no_mode_fallback_2;

Same question here?

> +	}
> +	if (ret)
> +		goto not_supported;
> +	if (retval == 0) {
> +		ret = -EOPNOTSUPP;
> +		goto not_supported;
> +	}
> +
> +	tunable_attr->cv_mode_id = mode;
> +	dev_dbg(tunable_attr->dev, "cd_mode_id: %02x%02x%02x%02x, cv_mode_id: %#08x attribute support level: %x\n",
> +		tunable_attr->device_id, tunable_attr->feature_id, tunable_attr->cd_mode_id,
> +		tunable_attr->type_id, args.arg0, capdata.supported);
> +
> +	return capdata.supported;
> +
> +not_supported:
> +	dev_dbg(tunable_attr->dev, "Attribute id %x not supported\n", args.arg0);
> +	return ret;
> +}
> +
>  /* Lenovo WMI Other Mode Attribute macros */
>  #define __LWMI_ATTR_RO(_func, _name)                                  \
>  	{                                                             \
> @@ -970,19 +1055,21 @@ static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
>  	}
>  
>  	for (i = 0; i < ARRAY_SIZE(cd01_attr_groups) - 1; i++) {
> -		err = sysfs_create_group(&priv->fw_attr_kset->kobj,
> -					 cd01_attr_groups[i].attr_group);
> -		if (err)
> -			goto err_remove_groups;
> -
>  		cd01_attr_groups[i].tunable_attr->dev = &priv->wdev->dev;
> +		if (lwmi_attr_01_is_supported(cd01_attr_groups[i].tunable_attr) > 0) {

Reverse logic and use continue.

> +			err = sysfs_create_group(&priv->fw_attr_kset->kobj,
> +						 cd01_attr_groups[i].attr_group);
> +			if (err)
> +				goto err_remove_groups;
> +		}
>  	}
>  	return 0;
>  
>  err_remove_groups:
>  	while (i--)
> -		sysfs_remove_group(&priv->fw_attr_kset->kobj,
> -				   cd01_attr_groups[i].attr_group);
> +		if (lwmi_attr_01_is_supported(cd01_attr_groups[i].tunable_attr) > 0)

Reverse logic + continue.

> +			sysfs_remove_group(&priv->fw_attr_kset->kobj,
> +					   cd01_attr_groups[i].attr_group);

You need to add braces for multiline constructs.

>  
>  	kset_unregister(priv->fw_attr_kset);
>  
> 

-- 
 i.

Re: [PATCH v3 3/6] platform/x86: lenovo-wmi-other: Add missing CPU tunable attributes

[Ilpo Järvinen]

On Tue, 24 Feb 2026, Derek J. Clark wrote:

> Use an enum for device ID's and CPU attribute feature ID's, add
> missing CPU attributes.
> 
> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
> ---
>  .../wmi/devices/lenovo-wmi-other.rst          | 10 ++
>  drivers/platform/x86/lenovo/wmi-capdata.h     |  5 +-
>  drivers/platform/x86/lenovo/wmi-other.c       | 98 ++++++++++++++++++-
>  3 files changed, 107 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/wmi/devices/lenovo-wmi-other.rst b/Documentation/wmi/devices/lenovo-wmi-other.rst
> index 01d471156738..f4763ed66cc6 100644
> --- a/Documentation/wmi/devices/lenovo-wmi-other.rst
> +++ b/Documentation/wmi/devices/lenovo-wmi-other.rst
> @@ -68,9 +68,19 @@ Each attribute has the following properties:
>   - type
>  
>  The following firmware-attributes are implemented:
> + - cpu_oc_stat: CPU Overlocking Status
> + - cpu_temp: CPU Thermal Load Limit
> + - ppt_cpu_cl: CPU Cross Loading Power Limit
> + - ppt_pl1_apu_spl: Platform Profile Tracking APU Sustained Power Limit
>   - ppt_pl1_spl: Platform Profile Tracking Sustained Power Limit
> + - ppt_pl1_spl_cl: Platform Profile Tracking Cross Loading Sustained Power Limit
> + - ppt_pl1_tau: Exceed Duration for Platform Profile Tracking Sustained Power Limit
>   - ppt_pl2_sppt: Platform Profile Tracking Slow Package Power Tracking
> + - ppt_pl2_sppt_cl: Platform Profile Tracking Cross Loading Slow Package Tracking
>   - ppt_pl3_fppt: Platform Profile Tracking Fast Package Power Tracking
> + - ppt_pl3_fppt_cl: Platform Profile Tracking Cross Loading Fast Package Power Tracking
> + - ppt_pl4_ipl: Platform Profile Trakcing Instantaneous Power Limit
> + - ppt_pl4_ipl_cl: Platform Profile Tracking Cross Loading Instantaneous Power Limit
>  
>  LENOVO_FAN_TEST_DATA
>  -------------------------
> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
> index 27202e2dc8a5..aa48f43cbb43 100644
> --- a/drivers/platform/x86/lenovo/wmi-capdata.h
> +++ b/drivers/platform/x86/lenovo/wmi-capdata.h
> @@ -23,7 +23,10 @@
>  	 FIELD_PREP(LWMI_ATTR_MODE_ID_MASK, mode) | \
>  	 FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, type))
>  
> -#define LWMI_DEVICE_ID_FAN	0x04
> +enum lwmi_device_id {
> +	LWMI_DEVICE_ID_CPU = 0x01,
> +	LWMI_DEVICE_ID_FAN = 0x04,
> +};
>  
>  struct component_match;
>  struct device;
> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> index f3f12303e379..87aba244da84 100644
> --- a/drivers/platform/x86/lenovo/wmi-other.c
> +++ b/drivers/platform/x86/lenovo/wmi-other.c
> @@ -54,15 +54,21 @@
>  
>  #define LENOVO_OTHER_MODE_GUID "DC2A8805-3A8C-41BA-A6F7-092E0089CD3B"
>  
> -#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
> +enum lwmi_feature_id_cpu {
> +	LWMI_FEATURE_ID_CPU_SPPT = 0x01,
> +	LWMI_FEATURE_ID_CPU_SPL = 0x02,
> +	LWMI_FEATURE_ID_CPU_FPPT = 0x03,
> +	LWMI_FEATURE_ID_CPU_TEMP = 0x04,
> +	LWMI_FEATURE_ID_CPU_APU = 0x05,
> +	LWMI_FEATURE_ID_CPU_CL = 0x06,
> +	LWMI_FEATURE_ID_CPU_TAU = 0x07,
> +	LWMI_FEATURE_ID_CPU_IPL = 0x09,

Please align a groups of values.

> +};
>  
>  #define LWMI_FEATURE_ID_FAN_RPM 0x03
>  
>  #define LWMI_TYPE_ID_NONE 0x00
> +#define LWMI_TYPE_ID_CROSSLOAD 0x01
>  
>  #define LWMI_FEATURE_VALUE_GET 17
>  #define LWMI_FEATURE_VALUE_SET 18
> @@ -559,18 +565,72 @@ static struct tunable_attr_01 ppt_pl1_spl = {
>  	.type_id = LWMI_TYPE_ID_NONE,
>  };
>  
> +static struct tunable_attr_01 ppt_pl1_spl_cl = {
> +	.device_id = LWMI_DEVICE_ID_CPU,
> +	.feature_id = LWMI_FEATURE_ID_CPU_SPL,
> +	.type_id = LWMI_TYPE_ID_CROSSLOAD,
> +};
> +
>  static struct tunable_attr_01 ppt_pl2_sppt = {
>  	.device_id = LWMI_DEVICE_ID_CPU,
>  	.feature_id = LWMI_FEATURE_ID_CPU_SPPT,
>  	.type_id = LWMI_TYPE_ID_NONE,
>  };
>  
> +static struct tunable_attr_01 ppt_pl2_sppt_cl = {
> +	.device_id = LWMI_DEVICE_ID_CPU,
> +	.feature_id = LWMI_FEATURE_ID_CPU_SPPT,
> +	.type_id = LWMI_TYPE_ID_CROSSLOAD,
> +};
> +
>  static struct tunable_attr_01 ppt_pl3_fppt = {
>  	.device_id = LWMI_DEVICE_ID_CPU,
>  	.feature_id = LWMI_FEATURE_ID_CPU_FPPT,
>  	.type_id = LWMI_TYPE_ID_NONE,
>  };
>  
> +static struct tunable_attr_01 ppt_pl3_fppt_cl = {
> +	.device_id = LWMI_DEVICE_ID_CPU,
> +	.feature_id = LWMI_FEATURE_ID_CPU_FPPT,
> +	.type_id = LWMI_TYPE_ID_CROSSLOAD,
> +};
> +
> +static struct tunable_attr_01 cpu_temp = {
> +	.device_id = LWMI_DEVICE_ID_CPU,
> +	.feature_id = LWMI_FEATURE_ID_CPU_TEMP,
> +	.type_id = LWMI_TYPE_ID_NONE,
> +};
> +
> +static struct tunable_attr_01 ppt_pl1_apu_spl = {
> +	.device_id = LWMI_DEVICE_ID_CPU,
> +	.feature_id = LWMI_FEATURE_ID_CPU_APU,
> +	.type_id = LWMI_TYPE_ID_NONE,
> +};
> +
> +static struct tunable_attr_01 ppt_cpu_cl = {
> +	.device_id = LWMI_DEVICE_ID_CPU,
> +	.feature_id = LWMI_FEATURE_ID_CPU_CL,
> +	.type_id = LWMI_TYPE_ID_NONE,
> +};
> +
> +static struct tunable_attr_01 ppt_pl1_tau = {
> +	.device_id = LWMI_DEVICE_ID_CPU,
> +	.feature_id = LWMI_FEATURE_ID_CPU_TAU,
> +	.type_id = LWMI_TYPE_ID_NONE,
> +};
> +
> +static struct tunable_attr_01 ppt_pl4_ipl = {
> +	.device_id = LWMI_DEVICE_ID_CPU,
> +	.feature_id = LWMI_FEATURE_ID_CPU_IPL,
> +	.type_id = LWMI_TYPE_ID_NONE,
> +};
> +
> +static struct tunable_attr_01 ppt_pl4_ipl_cl = {
> +	.device_id = LWMI_DEVICE_ID_CPU,
> +	.feature_id = LWMI_FEATURE_ID_CPU_IPL,
> +	.type_id = LWMI_TYPE_ID_CROSSLOAD,
> +};
> +
>  struct capdata01_attr_group {
>  	const struct attribute_group *attr_group;
>  	struct tunable_attr_01 *tunable_attr;
> @@ -1009,17 +1069,45 @@ static int lwmi_attr_01_is_supported(struct tunable_attr_01 *tunable_attr)
>  		.name = _fsname, .attrs = _attrname##_attrs               \
>  	}
>  
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(cpu_temp, "cpu_temp",
> +			      "Set the CPU thermal load limit");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_cpu_cl, "ppt_cpu_cl",
> +			      "Set the CPU cross loading power limit");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl1_apu_spl, "ppt_pl1_apu_spl",
> +			      "Set the APU sustained power limit");
>  LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl1_spl, "ppt_pl1_spl",
>  			      "Set the CPU sustained power limit");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl1_spl_cl, "ppt_pl1_spl_cl",
> +			      "Set the CPU cross loading sustained power limit");
>  LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl2_sppt, "ppt_pl2_sppt",
>  			      "Set the CPU slow package power tracking limit");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl2_sppt_cl, "ppt_pl2_sppt_cl",
> +			      "Set the CPU cross loading slow package power tracking limit");
>  LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl3_fppt, "ppt_pl3_fppt",
>  			      "Set the CPU fast package power tracking limit");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl3_fppt_cl, "ppt_pl3_fppt_cl",
> +			      "Set the CPU cross loading fast package power tracking limit");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl1_tau, "ppt_pl1_tau",
> +			      "Set the CPU sustained power limit exceed duration");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl4_ipl, "ppt_pl4_ipl",
> +			      "Set the CPU instantaneous power limit");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl4_ipl_cl, "ppt_pl4_ipl_cl",
> +			      "Set the CPU cross loading instantaneous power limit");
> +
>  
>  static struct capdata01_attr_group cd01_attr_groups[] = {
> +	{ &cpu_temp_attr_group, &cpu_temp },
> +	{ &ppt_cpu_cl_attr_group, &ppt_cpu_cl },
> +	{ &ppt_pl1_apu_spl_attr_group, &ppt_pl1_apu_spl },
>  	{ &ppt_pl1_spl_attr_group, &ppt_pl1_spl },
> +	{ &ppt_pl1_spl_cl_attr_group, &ppt_pl1_spl_cl },
> +	{ &ppt_pl1_tau_attr_group, &ppt_pl1_tau },
>  	{ &ppt_pl2_sppt_attr_group, &ppt_pl2_sppt },
> +	{ &ppt_pl2_sppt_cl_attr_group, &ppt_pl2_sppt_cl },
>  	{ &ppt_pl3_fppt_attr_group, &ppt_pl3_fppt },
> +	{ &ppt_pl3_fppt_cl_attr_group, &ppt_pl3_fppt_cl },
> +	{ &ppt_pl4_ipl_attr_group, &ppt_pl4_ipl },
> +	{ &ppt_pl4_ipl_cl_attr_group, &ppt_pl4_ipl_cl },
>  	{},
>  };
>  
> 

-- 
 i.

Re: [PATCH v3 4/6] platform/x86: lenovo-wmi-other: Add GPU tunable attributes

[Ilpo Järvinen]

On Tue, 24 Feb 2026, Derek J. Clark wrote:

> Use an enum for all GPU attribute feature ID's and add GPU attributes.
> 
> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
> ---
>  .../wmi/devices/lenovo-wmi-other.rst          |  11 ++
>  drivers/platform/x86/lenovo/wmi-capdata.h     |   1 +
>  drivers/platform/x86/lenovo/wmi-other.c       | 105 ++++++++++++++++++
>  3 files changed, 117 insertions(+)
> 
> diff --git a/Documentation/wmi/devices/lenovo-wmi-other.rst b/Documentation/wmi/devices/lenovo-wmi-other.rst
> index f4763ed66cc6..f7564b23bb7f 100644
> --- a/Documentation/wmi/devices/lenovo-wmi-other.rst
> +++ b/Documentation/wmi/devices/lenovo-wmi-other.rst
> @@ -70,6 +70,17 @@ Each attribute has the following properties:
>  The following firmware-attributes are implemented:
>   - cpu_oc_stat: CPU Overlocking Status
>   - cpu_temp: CPU Thermal Load Limit
> + - dgpu_boost_clk: Dedicated GPU Boost Clock
> + - dgpu_enable: Dedicated GPU Enabled Status
> + - gpu_didvid: GPU Device Identifier and Vendor Identifier
> + - gpu_mode: GPU Mode by Power Limit
> + - gpu_nv_ac_offset: Nvidia GPU AC Total Processing Power Baseline Offset
> + - gpu_nv_bpl: Nvidia GPU Base Power Limit
> + - gpu_nv_cpu_boost: Nvidia GPU to CPU Dynamic Boost Limit
> + - gpu_nv_ctgp: Nvidia GPU Configurable Total Graphics Power
> + - gpu_nv_ppab: Nvidia GPU Power Performance Aware Boost Limit
> + - gpu_oc_stat: GPU Overclocking Status
> + - gpu_temp: GPU Thermal Load Limit
>   - ppt_cpu_cl: CPU Cross Loading Power Limit
>   - ppt_pl1_apu_spl: Platform Profile Tracking APU Sustained Power Limit
>   - ppt_pl1_spl: Platform Profile Tracking Sustained Power Limit
> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
> index aa48f43cbb43..b7f9ee7b301a 100644
> --- a/drivers/platform/x86/lenovo/wmi-capdata.h
> +++ b/drivers/platform/x86/lenovo/wmi-capdata.h
> @@ -25,6 +25,7 @@
>  
>  enum lwmi_device_id {
>  	LWMI_DEVICE_ID_CPU = 0x01,
> +	LWMI_DEVICE_ID_GPU = 0x02,
>  	LWMI_DEVICE_ID_FAN = 0x04,
>  };
>  
> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> index 87aba244da84..67768f6a50e0 100644
> --- a/drivers/platform/x86/lenovo/wmi-other.c
> +++ b/drivers/platform/x86/lenovo/wmi-other.c
> @@ -65,6 +65,19 @@ enum lwmi_feature_id_cpu {
>  	LWMI_FEATURE_ID_CPU_IPL = 0x09,
>  };
>  
> +enum lwmi_feature_id_gpu {
> +	LWMI_FEATURE_ID_GPU_NV_PPAB = 0x01,
> +	LWMI_FEATURE_ID_GPU_NV_CTGP = 0x02,
> +	LWMI_FEATURE_ID_GPU_TEMP = 0x03,
> +	LWMI_FEATURE_ID_GPU_AC_OFFSET = 0x04,
> +	LWMI_FEATURE_ID_DGPU_BOOST_CLK = 0x06,
> +	LWMI_FEATURE_ID_DGPU_EN = 0x07,
> +	LWMI_FEATURE_ID_GPU_MODE = 0x08,
> +	LWMI_FEATURE_ID_DGPU_DIDVID = 0x09,
> +	LWMI_FEATURE_ID_GPU_NV_BPL = 0x0a,
> +	LWMI_FEATURE_ID_GPU_NV_CPU_BOOST = 0x0b,

Align values.

> +};
> +
>  #define LWMI_FEATURE_ID_FAN_RPM 0x03
>  
>  #define LWMI_TYPE_ID_NONE 0x00
> @@ -631,6 +644,66 @@ static struct tunable_attr_01 ppt_pl4_ipl_cl = {
>  	.type_id = LWMI_TYPE_ID_CROSSLOAD,
>  };
>  
> +static struct tunable_attr_01 gpu_nv_ppab = {
> +	.device_id = LWMI_DEVICE_ID_GPU,
> +	.feature_id = LWMI_FEATURE_ID_GPU_NV_PPAB,
> +	.type_id = LWMI_TYPE_ID_NONE,
> +};
> +
> +static struct tunable_attr_01 gpu_nv_ctgp = {
> +	.device_id = LWMI_DEVICE_ID_GPU,
> +	.feature_id = LWMI_FEATURE_ID_GPU_NV_CTGP,
> +	.type_id = LWMI_TYPE_ID_NONE,
> +};
> +
> +static struct tunable_attr_01 gpu_temp = {
> +	.device_id = LWMI_DEVICE_ID_GPU,
> +	.feature_id = LWMI_FEATURE_ID_GPU_TEMP,
> +	.type_id = LWMI_TYPE_ID_NONE,
> +};
> +
> +static struct tunable_attr_01 gpu_nv_ac_offset = {
> +	.device_id = LWMI_DEVICE_ID_GPU,
> +	.feature_id = LWMI_FEATURE_ID_GPU_AC_OFFSET,
> +	.type_id = LWMI_TYPE_ID_NONE,
> +};
> +
> +static struct tunable_attr_01 dgpu_boost_clk = {
> +	.device_id = LWMI_DEVICE_ID_GPU,
> +	.feature_id = LWMI_FEATURE_ID_DGPU_BOOST_CLK,
> +	.type_id = LWMI_TYPE_ID_NONE,
> +};
> +
> +static struct tunable_attr_01 dgpu_enable = {
> +	.device_id = LWMI_DEVICE_ID_GPU,
> +	.feature_id = LWMI_FEATURE_ID_DGPU_EN,
> +	.type_id = LWMI_TYPE_ID_NONE,
> +};
> +
> +static struct tunable_attr_01 gpu_mode = {
> +	.device_id = LWMI_DEVICE_ID_GPU,
> +	.feature_id = LWMI_FEATURE_ID_GPU_MODE,
> +	.type_id = LWMI_TYPE_ID_NONE,
> +};
> +
> +static struct tunable_attr_01 dgpu_didvid = {
> +	.device_id = LWMI_DEVICE_ID_GPU,
> +	.feature_id = LWMI_FEATURE_ID_DGPU_DIDVID,
> +	.type_id = LWMI_TYPE_ID_NONE,
> +};
> +
> +static struct tunable_attr_01 gpu_nv_bpl = {
> +	.device_id = LWMI_DEVICE_ID_GPU,
> +	.feature_id = LWMI_FEATURE_ID_GPU_NV_BPL,
> +	.type_id = LWMI_TYPE_ID_NONE,
> +};
> +
> +static struct tunable_attr_01 gpu_nv_cpu_boost = {
> +	.device_id = LWMI_DEVICE_ID_GPU,
> +	.feature_id = LWMI_FEATURE_ID_GPU_NV_CPU_BOOST,
> +	.type_id = LWMI_TYPE_ID_NONE,
> +};
> +
>  struct capdata01_attr_group {
>  	const struct attribute_group *attr_group;
>  	struct tunable_attr_01 *tunable_attr;
> @@ -1069,6 +1142,7 @@ static int lwmi_attr_01_is_supported(struct tunable_attr_01 *tunable_attr)
>  		.name = _fsname, .attrs = _attrname##_attrs               \
>  	}
>  
> +/* CPU tunable attributes */
>  LWMI_ATTR_GROUP_TUNABLE_CAP01(cpu_temp, "cpu_temp",
>  			      "Set the CPU thermal load limit");
>  LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_cpu_cl, "ppt_cpu_cl",
> @@ -1094,9 +1168,40 @@ LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl4_ipl, "ppt_pl4_ipl",
>  LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl4_ipl_cl, "ppt_pl4_ipl_cl",
>  			      "Set the CPU cross loading instantaneous power limit");
>  
> +/* GPU tunable attributes */
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(dgpu_boost_clk, "gpu_boost_clk",
> +			      "Set the dedicated GPU boost clock");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(dgpu_didvid, "gpu_didvid",
> +			      "Get the GPU device identifier and vendor identifier");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(dgpu_enable, "dgpu_enable",
> +			      "Set the dedicated Nvidia GPU enabled status");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(gpu_mode, "gpu_mode",
> +			      "Set the GPU mode by power limit");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(gpu_nv_ac_offset, "gpu_nv_ac_offset",
> +			      "Set the Nvidia GPU AC total processing power baseline offset");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(gpu_nv_bpl, "gpu_nv_bpl",
> +			      "Set the Nvidia GPU base power limit");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(gpu_nv_cpu_boost, "gpu_nv_cpu_boost",
> +			      "Set the Nvidia GPU to CPU dynamic boost limit");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(gpu_nv_ctgp, "gpu_nv_ctgp",
> +			      "Set the GPU configurable total graphics power");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(gpu_nv_ppab, "gpu_nv_ppab",
> +			      "Set the Nvidia GPU power performance aware boost limit");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(gpu_temp, "gpu_temp",
> +			      "Set the GPU thermal load limit");
>  
>  static struct capdata01_attr_group cd01_attr_groups[] = {
>  	{ &cpu_temp_attr_group, &cpu_temp },
> +	{ &dgpu_boost_clk_attr_group, &dgpu_boost_clk },
> +	{ &dgpu_didvid_attr_group, &dgpu_didvid },
> +	{ &dgpu_enable_attr_group, &dgpu_enable },
> +	{ &gpu_mode_attr_group, &gpu_mode },
> +	{ &gpu_nv_ac_offset_attr_group, &gpu_nv_ac_offset },
> +	{ &gpu_nv_bpl_attr_group, &gpu_nv_bpl },
> +	{ &gpu_nv_cpu_boost_attr_group, &gpu_nv_cpu_boost },
> +	{ &gpu_nv_ctgp_attr_group, &gpu_nv_ctgp },
> +	{ &gpu_nv_ppab_attr_group, &gpu_nv_ppab },
> +	{ &gpu_temp_attr_group, &gpu_temp },
>  	{ &ppt_cpu_cl_attr_group, &ppt_cpu_cl },
>  	{ &ppt_pl1_apu_spl_attr_group, &ppt_pl1_apu_spl },
>  	{ &ppt_pl1_spl_attr_group, &ppt_pl1_spl },
> 

-- 
 i.

Re: [PATCH v3 5/6] platform-x86: lenovo-wmi-other: Rename LWMI_OM_FW_ATTR_BASE_PATH

[Ilpo Järvinen]

On Tue, 24 Feb 2026, Derek J. Clark wrote:

> In the next patch a power supply extension is added which requires
> a name attribute. Instead of creating another const macro with the
> same information, replace LWMI_OM_FW_ATTR_BASE_PATH with
> LWMI_OM_NAME and use that for firmware attributes and power supply

This doesn't match the code change??

> extension.
> 
> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
> ---
>  drivers/platform/x86/lenovo/wmi-other.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> index 67768f6a50e0..7f0d5a17b44f 100644
> --- a/drivers/platform/x86/lenovo/wmi-other.c
> +++ b/drivers/platform/x86/lenovo/wmi-other.c
> @@ -90,13 +90,13 @@ enum lwmi_feature_id_gpu {
>  #define LWMI_FAN_NR 4
>  #define LWMI_FAN_ID(x) ((x) + LWMI_FAN_ID_BASE)
>  
> +#define LWMI_FAN_DIV 100
> +
>  #define LWMI_ATTR_ID_FAN_RPM(x)                                   \
>  	LWMI_ATTR_ID(LWMI_DEVICE_ID_FAN, LWMI_FEATURE_ID_FAN_RPM, \
>  		     LWMI_GZ_THERMAL_MODE_NONE, LWMI_FAN_ID(x))
>  
> -#define LWMI_FAN_DIV 100

Unrelated change.

> -
> -#define LWMI_OM_FW_ATTR_BASE_PATH "lenovo-wmi-other"
> +#define LWMI_OM_SYSFS_NAME "lenovo-wmi-other"
>  #define LWMI_OM_HWMON_NAME "lenovo_wmi_other"
>  
>  static BLOCKING_NOTIFIER_HEAD(om_chain_head);
> @@ -1233,8 +1233,7 @@ static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
>  
>  	priv->fw_attr_dev = device_create(&firmware_attributes_class, NULL,
>  					  MKDEV(0, 0), NULL, "%s-%u",
> -					  LWMI_OM_FW_ATTR_BASE_PATH,
> -					  priv->ida_id);
> +					  LWMI_OM_SYSFS_NAME, priv->ida_id);
>  	if (IS_ERR(priv->fw_attr_dev)) {
>  		err = PTR_ERR(priv->fw_attr_dev);
>  		goto err_free_ida;
> 

-- 
 i.

Re: [PATCH v3 6/6] platform/x86: lenovo-wmi-other: Add WMI battery charge limiting.

[Ilpo Järvinen]

On Tue, 24 Feb 2026, Derek J. Clark wrote:

> Add charge-type power supply extension for devices that support WMI based
> charge enable/disable. Lenovo Legion devices that implement function ID
> and capdata 00 ID 0x03010001 are able to enable or disable charging
> through the lenovo-wmi-other interface. The ideapad_laptop driver
> conflicts with this if it can also provide the attribute, so we have to
> get the acpi_handle and check for the same ACPI methods that enable the
> feature in that driver. The ACPI method is more reliable from my testing
> when both are present, so there is no need to modify the ideapad_laptop
> driver instead.
> 
> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
> ---
>  drivers/platform/x86/lenovo/wmi-capdata.h |   1 +
>  drivers/platform/x86/lenovo/wmi-other.c   | 230 ++++++++++++++++++++++
>  2 files changed, 231 insertions(+)
> 
> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
> index b7f9ee7b301a..00471551e7d6 100644
> --- a/drivers/platform/x86/lenovo/wmi-capdata.h
> +++ b/drivers/platform/x86/lenovo/wmi-capdata.h
> @@ -26,6 +26,7 @@
>  enum lwmi_device_id {
>  	LWMI_DEVICE_ID_CPU = 0x01,
>  	LWMI_DEVICE_ID_GPU = 0x02,
> +	LWMI_DEVICE_ID_PSU = 0x03,
>  	LWMI_DEVICE_ID_FAN = 0x04,
>  };
>  
> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> index 7f0d5a17b44f..b2daff1b45c2 100644
> --- a/drivers/platform/x86/lenovo/wmi-other.c
> +++ b/drivers/platform/x86/lenovo/wmi-other.c
> @@ -42,9 +42,12 @@
>  #include <linux/module.h>
>  #include <linux/notifier.h>
>  #include <linux/platform_profile.h>
> +#include <linux/power_supply.h>
>  #include <linux/types.h>
>  #include <linux/wmi.h>
>  
> +#include <acpi/battery.h>
> +
>  #include "wmi-capdata.h"
>  #include "wmi-events.h"
>  #include "wmi-gamezone.h"
> @@ -78,10 +81,17 @@ enum lwmi_feature_id_gpu {
>  	LWMI_FEATURE_ID_GPU_NV_CPU_BOOST = 0x0b,
>  };
>  
> +enum lwmi_feature_id_psu {
> +	LWMI_FEATURE_ID_PSU_INSTANT_MODE = 0x01,
> +	LWMI_FEATURE_ID_PSU_CHARGE_MODE = 0x02,

Align.

> +};
> +
>  #define LWMI_FEATURE_ID_FAN_RPM 0x03
>  
>  #define LWMI_TYPE_ID_NONE 0x00
>  #define LWMI_TYPE_ID_CROSSLOAD 0x01
> +#define LWMI_TYPE_ID_PSU_AC 0x01
> +#define LWMI_TYPE_ID_PSU_PD 0x02

These should be aligned as well but that will impact more than this patch 
to avoid back and forth changes within the series.

>  
>  #define LWMI_FEATURE_VALUE_GET 17
>  #define LWMI_FEATURE_VALUE_SET 18
> @@ -92,10 +102,17 @@ enum lwmi_feature_id_gpu {
>  
>  #define LWMI_FAN_DIV 100
>  
> +#define LWMI_CHARGE_MODE_ENABLED	0x00
> +#define LWMI_CHARGE_MODE_DISABLED	0x01
> +
>  #define LWMI_ATTR_ID_FAN_RPM(x)                                   \
>  	LWMI_ATTR_ID(LWMI_DEVICE_ID_FAN, LWMI_FEATURE_ID_FAN_RPM, \
>  		     LWMI_GZ_THERMAL_MODE_NONE, LWMI_FAN_ID(x))
>  
> +#define LWMI_ATTR_ID_PSU(feat, type)		\
> +	LWMI_ATTR_ID(LWMI_DEVICE_ID_PSU, feat,		\
> +		     LWMI_GZ_THERMAL_MODE_NONE, type)
> +
>  #define LWMI_OM_SYSFS_NAME "lenovo-wmi-other"
>  #define LWMI_OM_HWMON_NAME "lenovo_wmi_other"
>  
> @@ -137,6 +154,8 @@ struct lwmi_om_priv {
>  		bool capdata00_collected : 1;
>  		bool capdata_fan_collected : 1;
>  	} fan_flags;
> +
> +	struct acpi_battery_hook battery_hook;
>  };
>  
>  /*
> @@ -561,6 +580,216 @@ static void lwmi_om_fan_info_collect_cd_fan(struct device *dev, struct cd_list *
>  	lwmi_om_hwmon_add(priv);
>  }
>  
> +/* ======== Power Supply Extension (component: lenovo-wmi-capdata 00) ======== */
> +
> +/**
> + * lwmi_psy_ext_get_prop() - Get a power_supply_ext property
> + * @ps: The battery that was extended
> + * @ext: The extension
> + * @ext_data: Pointer the lwmi_om_priv drvdata
> + * @prop: The property to read
> + * @val: The value to return
> + *
> + * Writes the given value to the power_supply_ext property
> + *
> + * Return: 0 on success, or an error
> + */
> +static int lwmi_psy_ext_get_prop(struct power_supply *ps,
> +				 const struct power_supply_ext *ext,
> +				 void *ext_data,
> +				 enum power_supply_property prop,
> +				 union power_supply_propval *val)
> +{
> +	struct lwmi_om_priv *priv = ext_data;
> +	struct wmi_method_args_32 args;
> +	u32 retval;
> +	int ret;
> +
> +	args.arg0 = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
> +
> +	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_GET,
> +				    (unsigned char *)&args, sizeof(args),
> +				    &retval);
> +	if (ret)
> +		return ret;
> +
> +	dev_dbg(&priv->wdev->dev, "Got return value %x for property %x\n", retval, prop);
> +
> +	if (retval == LWMI_CHARGE_MODE_DISABLED)
> +		val->intval = POWER_SUPPLY_CHARGE_TYPE_LONGLIFE;
> +	else
> +		val->intval = POWER_SUPPLY_CHARGE_TYPE_STANDARD;
> +
> +	return 0;
> +}
> +
> +/**
> + * lwmi_psy_ext_set_prop() - Set a power_supply_ext property
> + * @ps: The battery that was extended
> + * @ext: The extension
> + * @ext_data: Pointer the lwmi_om_priv drvdata
> + * @prop: The property to write
> + * @val: The value to write
> + *
> + * Writes the given value to the power_supply_ext property
> + *
> + * Return: 0 on success, or an error
> + */
> +static int lwmi_psy_ext_set_prop(struct power_supply *ps,
> +				 const struct power_supply_ext *ext,
> +				 void *ext_data,
> +				 enum power_supply_property prop,
> +				 const union power_supply_propval *val)
> +{
> +	struct lwmi_om_priv *priv = ext_data;
> +	struct wmi_method_args_32 args;
> +
> +	args.arg0 = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
> +	if (val->intval == POWER_SUPPLY_CHARGE_TYPE_LONGLIFE)
> +		args.arg1 = LWMI_CHARGE_MODE_DISABLED;
> +	else
> +		args.arg1 = LWMI_CHARGE_MODE_ENABLED;
> +
> +	dev_dbg(&priv->wdev->dev, "Attempting to set %#08x for property %x to %x\n",
> +		args.arg0, prop, args.arg1);
> +
> +	return lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_SET,
> +				     (unsigned char *)&args, sizeof(args), NULL);
> +}
> +
> +/**
> + * lwmi_psy_prop_is_writeable() - Determine if the property is supported
> + * @ps: The battery that was extended
> + * @ext: The extension
> + * @ext_data: Pointer the lwmi_om_priv drvdata
> + * @prop: The property to check
> + *
> + * Checks capdata 00 to determine if the property is supported.
> + *
> + * Return: Support level, or false
> + */
> +static int lwmi_psy_prop_is_writeable(struct power_supply *ps,
> +				      const struct power_supply_ext *ext,
> +				      void *ext_data,
> +				      enum power_supply_property prop)
> +{
> +	struct lwmi_om_priv *priv = ext_data;
> +	struct capdata00 capdata;
> +	u32 attribute_id = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
> +	int ret;
> +
> +	ret = lwmi_cd00_get_data(priv->cd00_list, attribute_id, &capdata);
> +	if (ret)
> +		return false;
> +
> +	dev_dbg(&priv->wdev->dev, "Battery charge mode (%#08x) support level: %x\n",

IIRC, 0x should be included into the length of of the field, so perhaps 
%#10x for full u32 but it's long time ago I've used that formatting for 
anything so I might be wrong.

> +		attribute_id, capdata.supported);
> +
> +	return capdata.supported;
> +}
> +
> +static const enum power_supply_property lwmi_psy_ext_props[] = {
> +	POWER_SUPPLY_PROP_CHARGE_TYPES,
> +};
> +
> +static const struct power_supply_ext lwmi_psy_ext = {
> +	.name			= LWMI_OM_SYSFS_NAME,
> +	.properties		= lwmi_psy_ext_props,
> +	.num_properties		= ARRAY_SIZE(lwmi_psy_ext_props),
> +	.charge_types		= (BIT(POWER_SUPPLY_CHARGE_TYPE_STANDARD) |
> +				   BIT(POWER_SUPPLY_CHARGE_TYPE_LONGLIFE)),
> +	.get_property		= lwmi_psy_ext_get_prop,
> +	.set_property		= lwmi_psy_ext_set_prop,
> +	.property_is_writeable	= lwmi_psy_prop_is_writeable,
> +};
> +
> +/**
> + * lwmi_add_battery() - Connect the power_supply_ext
> + * @battery: The battery to extend
> + * @hook: The driver hook used to extend the battery
> + *
> + * Return: 0 on success, or an error.
> + */
> +static int lwmi_add_battery(struct power_supply *battery, struct acpi_battery_hook *hook)
> +{
> +	struct lwmi_om_priv *priv = container_of(hook, struct lwmi_om_priv, battery_hook);
> +
> +	return power_supply_register_extension(battery, &lwmi_psy_ext, &priv->wdev->dev, priv);
> +}
> +
> +/**
> + * lwmi_remove_battery() - Disconnect the power_supply_ext
> + * @battery: The battery that was extended
> + * @hook: The driver hook used to extend the battery
> + *
> + * Return: 0 on success, or an error.
> + */
> +static int lwmi_remove_battery(struct power_supply *battery, struct acpi_battery_hook *hook)
> +{
> +	power_supply_unregister_extension(battery, &lwmi_psy_ext);
> +	return 0;
> +}
> +
> +/**
> + * lwmi_acpi_match() - Attempts to return the ideapad acpi handle
> + * @handle: The ACPI handle that manages battery charging
> + * @lvl: Unused
> + * @context: Void pointer to the acpi_handle object to return
> + * @retval: Unused
> + *
> + * Checks if the ideapad_laptop driver is going to manage charge_type first,
> + * then if not, hooks the battery to our WMI methods.
> + *
> + * Return: AE_CTRL_TERMINATE if found, AE_OK if not found.
> + */
> +static acpi_status lwmi_acpi_match(acpi_handle handle, u32 lvl,
> +				   void *context, void **retval)
> +{
> +	if (!handle)
> +		return AE_OK;
> +
> +	acpi_handle *ahand = context;

Please don't declare variable mid-function (except cleanup.h related 
variables which is why it had to be allowed so compiler won't anymore 
complain about it).

> +	*ahand = handle;
> +
> +	return AE_CTRL_TERMINATE;
> +}
> +
> +/**
> + * lwmi_om_ps_ext_init() - Hooks power supply extension to device battery
> + * @priv: Driver private data
> + *
> + * Checks if the ideapad_laptop driver is going to manage charge_type first,
> + * then if not, hooks the battery to our WMI methods.
> + */
> +static void lwmi_om_ps_ext_init(struct lwmi_om_priv *priv)
> +{
> +	static const char * const ideapad_hid = "VPC2004";
> +	acpi_handle handle = NULL;
> +	int ret;
> +
> +	/* Deconflict ideapad_laptop driver */
> +	ret = acpi_get_devices(ideapad_hid, lwmi_acpi_match, &handle, NULL);
> +	if (ret)
> +		return;
> +
> +	if (!handle)
> +		return;
> +
> +	if (acpi_has_method(handle, "GBMD") && acpi_has_method(handle, "SBMC")) {
> +		dev_dbg(&priv->wdev->dev, "ideapad_laptop driver manages battery for device.\n");
> +		return;
> +	}
> +
> +	/* Add battery hooks */
> +	priv->battery_hook.add_battery	= lwmi_add_battery,
> +	priv->battery_hook.remove_battery	= lwmi_remove_battery,
> +	priv->battery_hook.name		= "Lenovo WMI Other Battery Extension",

????

Use semicolons to separate statements.

And those tabs look odd too (not even aligning the lines), I'd prefer 
just using just normal single space as aligning with tabs will cause 
churn if adding new fields that need adjusting alignment of all the 
members.

> +
> +	ret = devm_battery_hook_register(&priv->wdev->dev, &priv->battery_hook);
> +	if (ret)
> +		dev_err(&priv->wdev->dev, "Error during battery hook: %i\n", ret);
> +}
> +
>  /* ======== fw_attributes (component: lenovo-wmi-capdata 01) ======== */
>  
>  struct tunable_attr_01 {
> @@ -1325,6 +1554,7 @@ static int lwmi_om_master_bind(struct device *dev)
>  		return -ENODEV;
>  
>  	lwmi_om_fan_info_collect_cd00(priv);
> +	lwmi_om_ps_ext_init(priv);
>  
>  	return lwmi_om_fw_attr_add(priv);
>  }
> 

-- 
 i.

Re: [PATCH v3 6/6] platform/x86: lenovo-wmi-other: Add WMI battery charge limiting.

[Ilpo Järvinen]

On Tue, 24 Feb 2026, Derek J. Clark wrote:

> Add charge-type power supply extension for devices that support WMI based
> charge enable/disable. Lenovo Legion devices that implement function ID
> and capdata 00 ID 0x03010001 are able to enable or disable charging
> through the lenovo-wmi-other interface.

"are able to ... through the lenovo-wmi-other", does that refer to state 
after this patch? The wording choice is a bit odd if that's what you 
wanted to say.

> The ideapad_laptop driver
> conflicts with this if it can also provide the attribute, so we have to

What "this"? What "the attribute"?

> get the acpi_handle and check for the same ACPI methods that enable the
> feature in that driver. The ACPI method is more reliable from my testing
> when both are present, so there is no need to modify the ideapad_laptop

both drivers?

> driver instead.

instead of what? You probably tried to say that ideapad_laptop and this 
driver can coexist despite checking the same ACPI methods without 
modifications to the ideapad_laptop driver.

Could you try to rephrase this entire changelog text? Perhaps make it 
more than one paragraph and avoid using vague references. As is, it 
doesn't read very well and it's hard for the reader to connect the dots.

-- 
 i.

> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
> ---
>  drivers/platform/x86/lenovo/wmi-capdata.h |   1 +
>  drivers/platform/x86/lenovo/wmi-other.c   | 230 ++++++++++++++++++++++
>  2 files changed, 231 insertions(+)
> 
> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
> index b7f9ee7b301a..00471551e7d6 100644
> --- a/drivers/platform/x86/lenovo/wmi-capdata.h
> +++ b/drivers/platform/x86/lenovo/wmi-capdata.h
> @@ -26,6 +26,7 @@
>  enum lwmi_device_id {
>  	LWMI_DEVICE_ID_CPU = 0x01,
>  	LWMI_DEVICE_ID_GPU = 0x02,
> +	LWMI_DEVICE_ID_PSU = 0x03,
>  	LWMI_DEVICE_ID_FAN = 0x04,
>  };
>  
> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> index 7f0d5a17b44f..b2daff1b45c2 100644
> --- a/drivers/platform/x86/lenovo/wmi-other.c
> +++ b/drivers/platform/x86/lenovo/wmi-other.c
> @@ -42,9 +42,12 @@
>  #include <linux/module.h>
>  #include <linux/notifier.h>
>  #include <linux/platform_profile.h>
> +#include <linux/power_supply.h>
>  #include <linux/types.h>
>  #include <linux/wmi.h>
>  
> +#include <acpi/battery.h>
> +
>  #include "wmi-capdata.h"
>  #include "wmi-events.h"
>  #include "wmi-gamezone.h"
> @@ -78,10 +81,17 @@ enum lwmi_feature_id_gpu {
>  	LWMI_FEATURE_ID_GPU_NV_CPU_BOOST = 0x0b,
>  };
>  
> +enum lwmi_feature_id_psu {
> +	LWMI_FEATURE_ID_PSU_INSTANT_MODE = 0x01,
> +	LWMI_FEATURE_ID_PSU_CHARGE_MODE = 0x02,
> +};
> +
>  #define LWMI_FEATURE_ID_FAN_RPM 0x03
>  
>  #define LWMI_TYPE_ID_NONE 0x00
>  #define LWMI_TYPE_ID_CROSSLOAD 0x01
> +#define LWMI_TYPE_ID_PSU_AC 0x01
> +#define LWMI_TYPE_ID_PSU_PD 0x02
>  
>  #define LWMI_FEATURE_VALUE_GET 17
>  #define LWMI_FEATURE_VALUE_SET 18
> @@ -92,10 +102,17 @@ enum lwmi_feature_id_gpu {
>  
>  #define LWMI_FAN_DIV 100
>  
> +#define LWMI_CHARGE_MODE_ENABLED	0x00
> +#define LWMI_CHARGE_MODE_DISABLED	0x01
> +
>  #define LWMI_ATTR_ID_FAN_RPM(x)                                   \
>  	LWMI_ATTR_ID(LWMI_DEVICE_ID_FAN, LWMI_FEATURE_ID_FAN_RPM, \
>  		     LWMI_GZ_THERMAL_MODE_NONE, LWMI_FAN_ID(x))
>  
> +#define LWMI_ATTR_ID_PSU(feat, type)		\
> +	LWMI_ATTR_ID(LWMI_DEVICE_ID_PSU, feat,		\
> +		     LWMI_GZ_THERMAL_MODE_NONE, type)
> +
>  #define LWMI_OM_SYSFS_NAME "lenovo-wmi-other"
>  #define LWMI_OM_HWMON_NAME "lenovo_wmi_other"
>  
> @@ -137,6 +154,8 @@ struct lwmi_om_priv {
>  		bool capdata00_collected : 1;
>  		bool capdata_fan_collected : 1;
>  	} fan_flags;
> +
> +	struct acpi_battery_hook battery_hook;
>  };
>  
>  /*
> @@ -561,6 +580,216 @@ static void lwmi_om_fan_info_collect_cd_fan(struct device *dev, struct cd_list *
>  	lwmi_om_hwmon_add(priv);
>  }
>  
> +/* ======== Power Supply Extension (component: lenovo-wmi-capdata 00) ======== */
> +
> +/**
> + * lwmi_psy_ext_get_prop() - Get a power_supply_ext property
> + * @ps: The battery that was extended
> + * @ext: The extension
> + * @ext_data: Pointer the lwmi_om_priv drvdata
> + * @prop: The property to read
> + * @val: The value to return
> + *
> + * Writes the given value to the power_supply_ext property
> + *
> + * Return: 0 on success, or an error
> + */
> +static int lwmi_psy_ext_get_prop(struct power_supply *ps,
> +				 const struct power_supply_ext *ext,
> +				 void *ext_data,
> +				 enum power_supply_property prop,
> +				 union power_supply_propval *val)
> +{
> +	struct lwmi_om_priv *priv = ext_data;
> +	struct wmi_method_args_32 args;
> +	u32 retval;
> +	int ret;
> +
> +	args.arg0 = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
> +
> +	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_GET,
> +				    (unsigned char *)&args, sizeof(args),
> +				    &retval);
> +	if (ret)
> +		return ret;
> +
> +	dev_dbg(&priv->wdev->dev, "Got return value %x for property %x\n", retval, prop);
> +
> +	if (retval == LWMI_CHARGE_MODE_DISABLED)
> +		val->intval = POWER_SUPPLY_CHARGE_TYPE_LONGLIFE;
> +	else
> +		val->intval = POWER_SUPPLY_CHARGE_TYPE_STANDARD;
> +
> +	return 0;
> +}
> +
> +/**
> + * lwmi_psy_ext_set_prop() - Set a power_supply_ext property
> + * @ps: The battery that was extended
> + * @ext: The extension
> + * @ext_data: Pointer the lwmi_om_priv drvdata
> + * @prop: The property to write
> + * @val: The value to write
> + *
> + * Writes the given value to the power_supply_ext property
> + *
> + * Return: 0 on success, or an error
> + */
> +static int lwmi_psy_ext_set_prop(struct power_supply *ps,
> +				 const struct power_supply_ext *ext,
> +				 void *ext_data,
> +				 enum power_supply_property prop,
> +				 const union power_supply_propval *val)
> +{
> +	struct lwmi_om_priv *priv = ext_data;
> +	struct wmi_method_args_32 args;
> +
> +	args.arg0 = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
> +	if (val->intval == POWER_SUPPLY_CHARGE_TYPE_LONGLIFE)
> +		args.arg1 = LWMI_CHARGE_MODE_DISABLED;
> +	else
> +		args.arg1 = LWMI_CHARGE_MODE_ENABLED;
> +
> +	dev_dbg(&priv->wdev->dev, "Attempting to set %#08x for property %x to %x\n",
> +		args.arg0, prop, args.arg1);
> +
> +	return lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_SET,
> +				     (unsigned char *)&args, sizeof(args), NULL);
> +}
> +
> +/**
> + * lwmi_psy_prop_is_writeable() - Determine if the property is supported
> + * @ps: The battery that was extended
> + * @ext: The extension
> + * @ext_data: Pointer the lwmi_om_priv drvdata
> + * @prop: The property to check
> + *
> + * Checks capdata 00 to determine if the property is supported.
> + *
> + * Return: Support level, or false
> + */
> +static int lwmi_psy_prop_is_writeable(struct power_supply *ps,
> +				      const struct power_supply_ext *ext,
> +				      void *ext_data,
> +				      enum power_supply_property prop)
> +{
> +	struct lwmi_om_priv *priv = ext_data;
> +	struct capdata00 capdata;
> +	u32 attribute_id = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
> +	int ret;
> +
> +	ret = lwmi_cd00_get_data(priv->cd00_list, attribute_id, &capdata);
> +	if (ret)
> +		return false;
> +
> +	dev_dbg(&priv->wdev->dev, "Battery charge mode (%#08x) support level: %x\n",
> +		attribute_id, capdata.supported);
> +
> +	return capdata.supported;
> +}
> +
> +static const enum power_supply_property lwmi_psy_ext_props[] = {
> +	POWER_SUPPLY_PROP_CHARGE_TYPES,
> +};
> +
> +static const struct power_supply_ext lwmi_psy_ext = {
> +	.name			= LWMI_OM_SYSFS_NAME,
> +	.properties		= lwmi_psy_ext_props,
> +	.num_properties		= ARRAY_SIZE(lwmi_psy_ext_props),
> +	.charge_types		= (BIT(POWER_SUPPLY_CHARGE_TYPE_STANDARD) |
> +				   BIT(POWER_SUPPLY_CHARGE_TYPE_LONGLIFE)),
> +	.get_property		= lwmi_psy_ext_get_prop,
> +	.set_property		= lwmi_psy_ext_set_prop,
> +	.property_is_writeable	= lwmi_psy_prop_is_writeable,
> +};
> +
> +/**
> + * lwmi_add_battery() - Connect the power_supply_ext
> + * @battery: The battery to extend
> + * @hook: The driver hook used to extend the battery
> + *
> + * Return: 0 on success, or an error.
> + */
> +static int lwmi_add_battery(struct power_supply *battery, struct acpi_battery_hook *hook)
> +{
> +	struct lwmi_om_priv *priv = container_of(hook, struct lwmi_om_priv, battery_hook);
> +
> +	return power_supply_register_extension(battery, &lwmi_psy_ext, &priv->wdev->dev, priv);
> +}
> +
> +/**
> + * lwmi_remove_battery() - Disconnect the power_supply_ext
> + * @battery: The battery that was extended
> + * @hook: The driver hook used to extend the battery
> + *
> + * Return: 0 on success, or an error.
> + */
> +static int lwmi_remove_battery(struct power_supply *battery, struct acpi_battery_hook *hook)
> +{
> +	power_supply_unregister_extension(battery, &lwmi_psy_ext);
> +	return 0;
> +}
> +
> +/**
> + * lwmi_acpi_match() - Attempts to return the ideapad acpi handle
> + * @handle: The ACPI handle that manages battery charging
> + * @lvl: Unused
> + * @context: Void pointer to the acpi_handle object to return
> + * @retval: Unused
> + *
> + * Checks if the ideapad_laptop driver is going to manage charge_type first,
> + * then if not, hooks the battery to our WMI methods.
> + *
> + * Return: AE_CTRL_TERMINATE if found, AE_OK if not found.
> + */
> +static acpi_status lwmi_acpi_match(acpi_handle handle, u32 lvl,
> +				   void *context, void **retval)
> +{
> +	if (!handle)
> +		return AE_OK;
> +
> +	acpi_handle *ahand = context;
> +	*ahand = handle;
> +
> +	return AE_CTRL_TERMINATE;
> +}
> +
> +/**
> + * lwmi_om_ps_ext_init() - Hooks power supply extension to device battery
> + * @priv: Driver private data
> + *
> + * Checks if the ideapad_laptop driver is going to manage charge_type first,
> + * then if not, hooks the battery to our WMI methods.
> + */
> +static void lwmi_om_ps_ext_init(struct lwmi_om_priv *priv)
> +{
> +	static const char * const ideapad_hid = "VPC2004";
> +	acpi_handle handle = NULL;
> +	int ret;
> +
> +	/* Deconflict ideapad_laptop driver */
> +	ret = acpi_get_devices(ideapad_hid, lwmi_acpi_match, &handle, NULL);
> +	if (ret)
> +		return;
> +
> +	if (!handle)
> +		return;
> +
> +	if (acpi_has_method(handle, "GBMD") && acpi_has_method(handle, "SBMC")) {
> +		dev_dbg(&priv->wdev->dev, "ideapad_laptop driver manages battery for device.\n");
> +		return;
> +	}
> +
> +	/* Add battery hooks */
> +	priv->battery_hook.add_battery	= lwmi_add_battery,
> +	priv->battery_hook.remove_battery	= lwmi_remove_battery,
> +	priv->battery_hook.name		= "Lenovo WMI Other Battery Extension",
> +
> +	ret = devm_battery_hook_register(&priv->wdev->dev, &priv->battery_hook);
> +	if (ret)
> +		dev_err(&priv->wdev->dev, "Error during battery hook: %i\n", ret);
> +}
> +
>  /* ======== fw_attributes (component: lenovo-wmi-capdata 01) ======== */
>  
>  struct tunable_attr_01 {
> @@ -1325,6 +1554,7 @@ static int lwmi_om_master_bind(struct device *dev)
>  		return -ENODEV;
>  
>  	lwmi_om_fan_info_collect_cd00(priv);
> +	lwmi_om_ps_ext_init(priv);
>  
>  	return lwmi_om_fw_attr_add(priv);
>  }
> 

Re: [PATCH v3 2/6] platform/x86: lenovo-wmi-other: Limit adding attributes to supported devices

[Rong Zhang]

Hi Derek,

On Tue, 2026-02-24 at 04:31 +0000, Derek J. Clark wrote:
> Adds lwmi_is_attr_01_supported, and only creates the attribute subfolder
> if the attribute is supported by the hardware. Due to some poorly
> implemented BIOS, this is a multi-step sequence of events. This is
> because:
> - Some BIOS support getting the capability data from custom mode (0xff),
>   while others only support it in no-mode (0x00).
> - Similarly, some BIOS support get/set for the current value from custom
>   mode (0xff), while others only support it in no-mode (0x00).
> - Some BIOS report capability data for a method that is not fully
>   implemented.
> - Some BIOS have methods fully implemented, but no complimentary
>   capability data.
> 
> To ensure we only expose fully implemented methods with corresponding
> capability data, we check each outcome before reporting that an
> attribute can be supported.
> 
> Checking for lwmi_is_attr_01_supported during remove is not done to
> ensure that we don't attempt to call cd01 or send WMI events if one of
> the interfaces being removed was the cause of the driver unloading.
> 
> While adding members to tunable_attr_01, remove unused capdata pointer
> and limit size of all ID's to the appropriate size.
> 
> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Reported-by: Kurt Borja <kuurtb@gmail.com>
> Closes: https://lore.kernel.org/platform-driver-x86/DG60P3SHXR8H.3NSEHMZ6J7XRC@gmail.com/
> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
> ---
>  drivers/platform/x86/lenovo/wmi-other.c | 117 +++++++++++++++++++++---
>  1 file changed, 102 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> index 95886df39c8d..f3f12303e379 100644
> --- a/drivers/platform/x86/lenovo/wmi-other.c
> +++ b/drivers/platform/x86/lenovo/wmi-other.c
> @@ -545,11 +545,12 @@ static void lwmi_om_fan_info_collect_cd_fan(struct device *dev, struct cd_list *
>  /* ======== fw_attributes (component: lenovo-wmi-capdata 01) ======== */
>  
>  struct tunable_attr_01 {
> -	struct capdata01 *capdata;
>  	struct device *dev;
> -	u32 feature_id;
> -	u32 device_id;
> -	u32 type_id;
> +	u8 feature_id;
> +	u8 device_id;
> +	u8 type_id;
> +	u8 cd_mode_id; /* mode arg for searching capdata */
> +	u8 cv_mode_id; /* mode arg for set/get current_value */
>  };
>  
>  static struct tunable_attr_01 ppt_pl1_spl = {
> @@ -716,7 +717,7 @@ static ssize_t attr_capdata01_show(struct kobject *kobj,
>  	int value, ret;
>  
>  	attribute_id = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> -				    LWMI_GZ_THERMAL_MODE_CUSTOM, tunable_attr->type_id);
> +				    tunable_attr->cd_mode_id, tunable_attr->type_id);
>  
>  	ret = lwmi_cd01_get_data(priv->cd01_list, attribute_id, &capdata);
>  	if (ret)
> @@ -782,7 +783,7 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
>  		return -EBUSY;
>  
>  	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> -				 mode, tunable_attr->type_id);
> +				 tunable_attr->cd_mode_id, tunable_attr->type_id);
>  
>  	ret = lwmi_cd01_get_data(priv->cd01_list, args.arg0, &capdata);
>  	if (ret)
> @@ -795,6 +796,8 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
>  	if (value < capdata.min_value || value > capdata.max_value)
>  		return -EINVAL;
>  
> +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> +				 tunable_attr->cv_mode_id, tunable_attr->type_id);
>  	args.arg1 = value;
>  
>  	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_SET,
> @@ -828,13 +831,16 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
>  	struct lwmi_om_priv *priv = dev_get_drvdata(tunable_attr->dev);
>  	struct wmi_method_args_32 args;
>  	enum thermal_mode mode;
> -	int retval;
> -	int ret;
> +	int retval, ret;
>  
>  	ret = lwmi_om_notifier_call(&mode);
>  	if (ret)
>  		return ret;
>  
> +	/* If "no-mode" is the supported mode, ensure we never send current mode */
> +	if (tunable_attr->cv_mode_id == LWMI_GZ_THERMAL_MODE_NONE)
> +		mode = tunable_attr->cv_mode_id;
> +
>  	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
>  				 mode, tunable_attr->type_id);
>  
> @@ -847,6 +853,85 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
>  	return sysfs_emit(buf, "%d\n", retval);
>  }
>  
> +/**
> + * lwmi_attr_01_is_supported() - Determine if the given attribute is supported.
> + * @tunable_attr: The attribute to verify.
> + *
> + * First check if the attribute has a corresponding capdata01 table in the cd01
> + * module under the "custom" mode (0xff). If that is not present then check if
> + * there is a corresponding "no-mode" (0x00) entry. If either of those passes,
> + * check capdata->supported for values > 0. If capdata is available, attempt to
> + * determine the set/get mode for the current value property using a similar
> + * pattern. If the value returned by either custom or no-mode is 0, or we get
> + * an error, we assume that mode is not supported. If any of the above checks
> + * fail then the attribute is not fully supported.
> + *
> + * The probed cd_mode_id/cv_mode_id are stored on the tunable_attr for later
> + * reference.
> + *
> + * Return: Support level, or an error code.
> + */
> +static int lwmi_attr_01_is_supported(struct tunable_attr_01 *tunable_attr)
> +{
> +	struct lwmi_om_priv *priv = dev_get_drvdata(tunable_attr->dev);
> +	u8 mode = LWMI_GZ_THERMAL_MODE_CUSTOM;
> +	struct wmi_method_args_32 args;
> +	struct capdata01 capdata;
> +	int retval, ret;
> +
> +	/* Determine tunable_attr->cd_mode_id */
> +no_mode_fallback_1:
> +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> +				 mode, tunable_attr->type_id);
> +
> +	ret = lwmi_cd01_get_data(priv->cd01_list, args.arg0, &capdata);
> +	if (ret && mode) {
> +		dev_dbg(tunable_attr->dev, "Attribute id %x not supported\n", args.arg0);
> +		mode = LWMI_GZ_THERMAL_MODE_NONE;
> +		goto no_mode_fallback_1;
> +	}
> +	if (ret)
> +		goto not_supported;
> +	if (!capdata.supported) {
> +		ret = -EOPNOTSUPP;
> +		goto not_supported;
> +	}
> +
> +	tunable_attr->cd_mode_id = mode;
> +
> +	/* Determine tunable_attr->cv_mode_id */
> +	mode = LWMI_GZ_THERMAL_MODE_CUSTOM;
> +no_mode_fallback_2:
> +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> +				 mode, tunable_attr->type_id);
> +
> +	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_GET,
> +				    (unsigned char *)&args, sizeof(args),
> +				    &retval);
> +	if ((ret && mode) || (!retval && mode)) {
> +		dev_dbg(tunable_attr->dev, "Attribute id %x not supported\n", args.arg0);
> +		mode = LWMI_GZ_THERMAL_MODE_NONE;
> +		goto no_mode_fallback_2;
> +	}
> +	if (ret)
> +		goto not_supported;
> +	if (retval == 0) {
> +		ret = -EOPNOTSUPP;
> +		goto not_supported;
> +	}
> +

I agree with Ilpo that backward gotos are hard to read and should be
avoided if possible.

> +	tunable_attr->cv_mode_id = mode;
> +	dev_dbg(tunable_attr->dev, "cd_mode_id: %02x%02x%02x%02x, cv_mode_id: %#08x attribute support level: %x\n",

It's a long line. Wrap at the first comma?

> +		tunable_attr->device_id, tunable_attr->feature_id, tunable_attr->cd_mode_id,
> +		tunable_attr->type_id, args.arg0, capdata.supported);
> +
> +	return capdata.supported;
> +
> +not_supported:
> +	dev_dbg(tunable_attr->dev, "Attribute id %x not supported\n", args.arg0);
> +	return ret;
> +}
> +
>  /* Lenovo WMI Other Mode Attribute macros */
>  #define __LWMI_ATTR_RO(_func, _name)                                  \
>  	{                                                             \
> @@ -970,19 +1055,21 @@ static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
>  	}
>  
>  	for (i = 0; i < ARRAY_SIZE(cd01_attr_groups) - 1; i++) {
> -		err = sysfs_create_group(&priv->fw_attr_kset->kobj,
> -					 cd01_attr_groups[i].attr_group);
> -		if (err)
> -			goto err_remove_groups;
> -
>  		cd01_attr_groups[i].tunable_attr->dev = &priv->wdev->dev;
> +		if (lwmi_attr_01_is_supported(cd01_attr_groups[i].tunable_attr) > 0) {
> +			err = sysfs_create_group(&priv->fw_attr_kset->kobj,
> +						 cd01_attr_groups[i].attr_group);
> +			if (err)
> +				goto err_remove_groups;
> +		}
>  	}
>  	return 0;
>  
>  err_remove_groups:
>  	while (i--)
> -		sysfs_remove_group(&priv->fw_attr_kset->kobj,
> -				   cd01_attr_groups[i].attr_group);
> +		if (lwmi_attr_01_is_supported(cd01_attr_groups[i].tunable_attr) > 0)

I don't see lwmi_om_fw_attr_remove() doing the same. If we care about
symmetric cleanup, I would imagine a bitmap or so recording what have
been created is used there.

Also it's doubtful if the firmware always return the same result on the
same parameters. If a broken firmware somehow returns different results
here, some attributes may be left uncleaned.

That being said, I am not sure if we really need symmetric cleanup.
IIUC, sysfs_remove_group() is a no-op if we pass an attr_group that has
not been created.

> +			sysfs_remove_group(&priv->fw_attr_kset->kobj,
> +					   cd01_attr_groups[i].attr_group);
>  
>  	kset_unregister(priv->fw_attr_kset);
>  

Thanks,
Rong

Re: [PATCH v3 5/6] platform-x86: lenovo-wmi-other: Rename LWMI_OM_FW_ATTR_BASE_PATH

[Rong Zhang]

Hi Derek,

On Tue, 2026-02-24 at 04:31 +0000, Derek J. Clark wrote:
> In the next patch a power supply extension is added which requires
> a name attribute. Instead of creating another const macro with the
> same information, replace LWMI_OM_FW_ATTR_BASE_PATH with
> LWMI_OM_NAME and use that for firmware attributes and power supply
> extension.
> 
> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
> ---
>  drivers/platform/x86/lenovo/wmi-other.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> index 67768f6a50e0..7f0d5a17b44f 100644
> --- a/drivers/platform/x86/lenovo/wmi-other.c
> +++ b/drivers/platform/x86/lenovo/wmi-other.c
> @@ -90,13 +90,13 @@ enum lwmi_feature_id_gpu {
>  #define LWMI_FAN_NR 4
>  #define LWMI_FAN_ID(x) ((x) + LWMI_FAN_ID_BASE)
>  
> +#define LWMI_FAN_DIV 100
> +
>  #define LWMI_ATTR_ID_FAN_RPM(x)                                   \
>  	LWMI_ATTR_ID(LWMI_DEVICE_ID_FAN, LWMI_FEATURE_ID_FAN_RPM, \
>  		     LWMI_GZ_THERMAL_MODE_NONE, LWMI_FAN_ID(x))
>  
> -#define LWMI_FAN_DIV 100

I placed it here intentionally as it has nothing to do with
LWMI_ATTR_ID (contrary to other fan-related macros) and is just a
descriptive macro.

> -
> -#define LWMI_OM_FW_ATTR_BASE_PATH "lenovo-wmi-other"
> +#define LWMI_OM_SYSFS_NAME "lenovo-wmi-other"
>  #define LWMI_OM_HWMON_NAME "lenovo_wmi_other"
>  
>  static BLOCKING_NOTIFIER_HEAD(om_chain_head);
> @@ -1233,8 +1233,7 @@ static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
>  
>  	priv->fw_attr_dev = device_create(&firmware_attributes_class, NULL,
>  					  MKDEV(0, 0), NULL, "%s-%u",
> -					  LWMI_OM_FW_ATTR_BASE_PATH,
> -					  priv->ida_id);
> +					  LWMI_OM_SYSFS_NAME, priv->ida_id);
>  	if (IS_ERR(priv->fw_attr_dev)) {
>  		err = PTR_ERR(priv->fw_attr_dev);
>  		goto err_free_ida;

Thanks,
Rong

Re: [PATCH v3 1/6] platform/x86: lenovo-wmi-other: Add LWMI_ATTR_ID Macro

[Derek J. Clark]

On February 24, 2026 12:42:08 AM PST, "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com> wrote:
>On Tue, 24 Feb 2026, Derek J. Clark wrote:
>
>> Adds LWMI_ATTR_ID macro. In the same vein as LWMI_ATTR_ID_FAN_RPM,
>> but as a generic, to de-duplicate attribute_id assignment biolerplate.
>> 
>> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
>> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
>> ---
>>  drivers/platform/x86/lenovo/wmi-capdata.h  |  6 ++++
>>  drivers/platform/x86/lenovo/wmi-gamezone.h |  1 +
>>  drivers/platform/x86/lenovo/wmi-other.c    | 36 ++++++----------------
>>  3 files changed, 17 insertions(+), 26 deletions(-)
>> 
>> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
>> index 8c1df3efcc55..27202e2dc8a5 100644
>> --- a/drivers/platform/x86/lenovo/wmi-capdata.h
>> +++ b/drivers/platform/x86/lenovo/wmi-capdata.h
>> @@ -17,6 +17,12 @@
>>  #define LWMI_ATTR_MODE_ID_MASK	GENMASK(15, 8)
>>  #define LWMI_ATTR_TYPE_ID_MASK	GENMASK(7, 0)
>>  
>> +#define LWMI_ATTR_ID(dev, feat, mode, type) \
>> +	(FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, dev) | \
>> +	 FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, feat) | \
>> +	 FIELD_PREP(LWMI_ATTR_MODE_ID_MASK, mode) | \
>> +	 FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, type))
>
>Why does this need to be macro? Static inline would have proper types.
>

Hi Ilpo,

It doesn't, I was just keeping it consistent with other previous uses. It's not a problem to change it. I'll need to move it somewhere else since this is a header.

Thanks,
Derek

>> +
>>  #define LWMI_DEVICE_ID_FAN	0x04
>>  
>>  struct component_match;
>> diff --git a/drivers/platform/x86/lenovo/wmi-gamezone.h b/drivers/platform/x86/lenovo/wmi-gamezone.h
>> index 6b163a5eeb95..ddb919cf6c36 100644
>> --- a/drivers/platform/x86/lenovo/wmi-gamezone.h
>> +++ b/drivers/platform/x86/lenovo/wmi-gamezone.h
>> @@ -10,6 +10,7 @@ enum gamezone_events_type {
>>  };
>>  
>>  enum thermal_mode {
>> +	LWMI_GZ_THERMAL_MODE_NONE =	   0x00,
>>  	LWMI_GZ_THERMAL_MODE_QUIET =	   0x01,
>>  	LWMI_GZ_THERMAL_MODE_BALANCED =	   0x02,
>>  	LWMI_GZ_THERMAL_MODE_PERFORMANCE = 0x03,
>> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
>> index 6040f45aa2b0..95886df39c8d 100644
>> --- a/drivers/platform/x86/lenovo/wmi-other.c
>> +++ b/drivers/platform/x86/lenovo/wmi-other.c
>> @@ -71,10 +71,9 @@
>>  #define LWMI_FAN_NR 4
>>  #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_ATTR_ID_FAN_RPM(x)                                   \
>> +	LWMI_ATTR_ID(LWMI_DEVICE_ID_FAN, LWMI_FEATURE_ID_FAN_RPM, \
>> +		     LWMI_GZ_THERMAL_MODE_NONE, LWMI_FAN_ID(x))
>>  
>>  #define LWMI_FAN_DIV 100
>>  
>> @@ -716,12 +715,8 @@ static ssize_t attr_capdata01_show(struct kobject *kobj,
>>  	u32 attribute_id;
>>  	int value, ret;
>>  
>> -	attribute_id =
>> -		FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, tunable_attr->device_id) |
>> -		FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, tunable_attr->feature_id) |
>> -		FIELD_PREP(LWMI_ATTR_MODE_ID_MASK,
>> -			   LWMI_GZ_THERMAL_MODE_CUSTOM) |
>> -		FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, tunable_attr->type_id);
>> +	attribute_id = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
>> +				    LWMI_GZ_THERMAL_MODE_CUSTOM, tunable_attr->type_id);
>>  
>>  	ret = lwmi_cd01_get_data(priv->cd01_list, attribute_id, &capdata);
>>  	if (ret)
>> @@ -776,7 +771,6 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
>>  	struct wmi_method_args_32 args;
>>  	struct capdata01 capdata;
>>  	enum thermal_mode mode;
>> -	u32 attribute_id;
>>  	u32 value;
>>  	int ret;
>>  
>> @@ -787,13 +781,10 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
>>  	if (mode != LWMI_GZ_THERMAL_MODE_CUSTOM)
>>  		return -EBUSY;
>>  
>> -	attribute_id =
>> -		FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, tunable_attr->device_id) |
>> -		FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, tunable_attr->feature_id) |
>> -		FIELD_PREP(LWMI_ATTR_MODE_ID_MASK, mode) |
>> -		FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, tunable_attr->type_id);
>> +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
>> +				 mode, tunable_attr->type_id);
>>  
>> -	ret = lwmi_cd01_get_data(priv->cd01_list, attribute_id, &capdata);
>> +	ret = lwmi_cd01_get_data(priv->cd01_list, args.arg0, &capdata);
>>  	if (ret)
>>  		return ret;
>>  
>> @@ -804,7 +795,6 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
>>  	if (value < capdata.min_value || value > capdata.max_value)
>>  		return -EINVAL;
>>  
>> -	args.arg0 = attribute_id;
>>  	args.arg1 = value;
>>  
>>  	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_SET,
>> @@ -838,7 +828,6 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
>>  	struct lwmi_om_priv *priv = dev_get_drvdata(tunable_attr->dev);
>>  	struct wmi_method_args_32 args;
>>  	enum thermal_mode mode;
>> -	u32 attribute_id;
>>  	int retval;
>>  	int ret;
>>  
>> @@ -846,13 +835,8 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
>>  	if (ret)
>>  		return ret;
>>  
>> -	attribute_id =
>> -		FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, tunable_attr->device_id) |
>> -		FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, tunable_attr->feature_id) |
>> -		FIELD_PREP(LWMI_ATTR_MODE_ID_MASK, mode) |
>> -		FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, tunable_attr->type_id);
>> -
>> -	args.arg0 = attribute_id;
>> +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
>> +				 mode, tunable_attr->type_id);
>>  
>>  	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_GET,
>>  				    (unsigned char *)&args, sizeof(args),
>> 
>

Re: [PATCH v3 6/6] platform/x86: lenovo-wmi-other: Add WMI battery charge limiting.

[Rong Zhang]

Hi Derek,

There is an extra dot (period) in the title. Please remove it.

On Tue, 2026-02-24 at 04:32 +0000, Derek J. Clark wrote:
> Add charge-type power supply extension for devices that support WMI based
> charge enable/disable. Lenovo Legion devices that implement function ID
> and capdata 00 ID 0x03010001 are able to enable or disable charging
> through the lenovo-wmi-other interface.

"disable charging"...

I'd expect it pauses charging immediately and unconditionally instead
of setting a threshold, especially considering the its name
"INSTANT_MODE". If this is the case, I don't think it fits the
definition of POWER_SUPPLY_CHARGE_TYPE_LONGLIFE. It'd better to use
POWER_SUPPLY_CHARGE_TYPE_NONE.

> The ideapad_laptop driver
> conflicts with this if it can also provide the attribute, so we have to
> get the acpi_handle and check for the same ACPI methods that enable the
> feature in that driver. The ACPI method is more reliable from my testing
> when both are present, so there is no need to modify the ideapad_laptop
> driver instead.

When ideapad-laptop is blacklisted, there should be no conflict. I
would suggest adding a module parameter to force wmi-other to register
psy_ext anyway so that users can have their own choice.

> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
> ---
>  drivers/platform/x86/lenovo/wmi-capdata.h |   1 +
>  drivers/platform/x86/lenovo/wmi-other.c   | 230 ++++++++++++++++++++++
>  2 files changed, 231 insertions(+)
> 
> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
> index b7f9ee7b301a..00471551e7d6 100644
> --- a/drivers/platform/x86/lenovo/wmi-capdata.h
> +++ b/drivers/platform/x86/lenovo/wmi-capdata.h
> @@ -26,6 +26,7 @@
>  enum lwmi_device_id {
>  	LWMI_DEVICE_ID_CPU = 0x01,
>  	LWMI_DEVICE_ID_GPU = 0x02,
> +	LWMI_DEVICE_ID_PSU = 0x03,
>  	LWMI_DEVICE_ID_FAN = 0x04,
>  };
>  
> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> index 7f0d5a17b44f..b2daff1b45c2 100644
> --- a/drivers/platform/x86/lenovo/wmi-other.c
> +++ b/drivers/platform/x86/lenovo/wmi-other.c
> @@ -42,9 +42,12 @@
>  #include <linux/module.h>
>  #include <linux/notifier.h>
>  #include <linux/platform_profile.h>
> +#include <linux/power_supply.h>
>  #include <linux/types.h>
>  #include <linux/wmi.h>
>  
> +#include <acpi/battery.h>
> +
>  #include "wmi-capdata.h"
>  #include "wmi-events.h"
>  #include "wmi-gamezone.h"
> @@ -78,10 +81,17 @@ enum lwmi_feature_id_gpu {
>  	LWMI_FEATURE_ID_GPU_NV_CPU_BOOST = 0x0b,
>  };
>  
> +enum lwmi_feature_id_psu {
> +	LWMI_FEATURE_ID_PSU_INSTANT_MODE = 0x01,
> +	LWMI_FEATURE_ID_PSU_CHARGE_MODE = 0x02,

I see no code referencing LWMI_FEATURE_ID_PSU_CHARGE_MODE. What's the
mode for?

> +};
> +
>  #define LWMI_FEATURE_ID_FAN_RPM 0x03
>  
>  #define LWMI_TYPE_ID_NONE 0x00
>  #define LWMI_TYPE_ID_CROSSLOAD 0x01
> +#define LWMI_TYPE_ID_PSU_AC 0x01
> +#define LWMI_TYPE_ID_PSU_PD 0x02
>  
>  #define LWMI_FEATURE_VALUE_GET 17
>  #define LWMI_FEATURE_VALUE_SET 18
> @@ -92,10 +102,17 @@ enum lwmi_feature_id_gpu {
>  
>  #define LWMI_FAN_DIV 100
>  
> +#define LWMI_CHARGE_MODE_ENABLED	0x00
> +#define LWMI_CHARGE_MODE_DISABLED	0x01
> +
>  #define LWMI_ATTR_ID_FAN_RPM(x)                                   \
>  	LWMI_ATTR_ID(LWMI_DEVICE_ID_FAN, LWMI_FEATURE_ID_FAN_RPM, \
>  		     LWMI_GZ_THERMAL_MODE_NONE, LWMI_FAN_ID(x))
>  
> +#define LWMI_ATTR_ID_PSU(feat, type)		\
> +	LWMI_ATTR_ID(LWMI_DEVICE_ID_PSU, feat,		\
> +		     LWMI_GZ_THERMAL_MODE_NONE, type)
> +

Unaligned backslashes.

>  #define LWMI_OM_SYSFS_NAME "lenovo-wmi-other"
>  #define LWMI_OM_HWMON_NAME "lenovo_wmi_other"
>  
> @@ -137,6 +154,8 @@ struct lwmi_om_priv {
>  		bool capdata00_collected : 1;
>  		bool capdata_fan_collected : 1;
>  	} fan_flags;
> +
> +	struct acpi_battery_hook battery_hook;
>  };
>  
>  /*
> @@ -561,6 +580,216 @@ static void lwmi_om_fan_info_collect_cd_fan(struct device *dev, struct cd_list *
>  	lwmi_om_hwmon_add(priv);
>  }
>  
> +/* ======== Power Supply Extension (component: lenovo-wmi-capdata 00) ======== */
> +
> +/**
> + * lwmi_psy_ext_get_prop() - Get a power_supply_ext property
> + * @ps: The battery that was extended
> + * @ext: The extension
> + * @ext_data: Pointer the lwmi_om_priv drvdata
> + * @prop: The property to read
> + * @val: The value to return
> + *
> + * Writes the given value to the power_supply_ext property
> + *
> + * Return: 0 on success, or an error
> + */
> +static int lwmi_psy_ext_get_prop(struct power_supply *ps,
> +				 const struct power_supply_ext *ext,
> +				 void *ext_data,
> +				 enum power_supply_property prop,
> +				 union power_supply_propval *val)
> +{
> +	struct lwmi_om_priv *priv = ext_data;
> +	struct wmi_method_args_32 args;
> +	u32 retval;
> +	int ret;
> +
> +	args.arg0 = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
> +
> +	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_GET,
> +				    (unsigned char *)&args, sizeof(args),
> +				    &retval);
> +	if (ret)
> +		return ret;
> +
> +	dev_dbg(&priv->wdev->dev, "Got return value %x for property %x\n", retval, prop);
> +
> +	if (retval == LWMI_CHARGE_MODE_DISABLED)
> +		val->intval = POWER_SUPPLY_CHARGE_TYPE_LONGLIFE;
> +	else
> +		val->intval = POWER_SUPPLY_CHARGE_TYPE_STANDARD;
> +
> +	return 0;
> +}
> +
> +/**
> + * lwmi_psy_ext_set_prop() - Set a power_supply_ext property
> + * @ps: The battery that was extended
> + * @ext: The extension
> + * @ext_data: Pointer the lwmi_om_priv drvdata
> + * @prop: The property to write
> + * @val: The value to write
> + *
> + * Writes the given value to the power_supply_ext property
> + *
> + * Return: 0 on success, or an error
> + */
> +static int lwmi_psy_ext_set_prop(struct power_supply *ps,
> +				 const struct power_supply_ext *ext,
> +				 void *ext_data,
> +				 enum power_supply_property prop,
> +				 const union power_supply_propval *val)
> +{
> +	struct lwmi_om_priv *priv = ext_data;
> +	struct wmi_method_args_32 args;
> +
> +	args.arg0 = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
> +	if (val->intval == POWER_SUPPLY_CHARGE_TYPE_LONGLIFE)
> +		args.arg1 = LWMI_CHARGE_MODE_DISABLED;
> +	else
> +		args.arg1 = LWMI_CHARGE_MODE_ENABLED;
> +
> +	dev_dbg(&priv->wdev->dev, "Attempting to set %#08x for property %x to %x\n",
> +		args.arg0, prop, args.arg1);
> +
> +	return lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_SET,
> +				     (unsigned char *)&args, sizeof(args), NULL);
> +}
> +
> +/**
> + * lwmi_psy_prop_is_writeable() - Determine if the property is supported
> + * @ps: The battery that was extended
> + * @ext: The extension
> + * @ext_data: Pointer the lwmi_om_priv drvdata
> + * @prop: The property to check
> + *
> + * Checks capdata 00 to determine if the property is supported.
> + *
> + * Return: Support level, or false
> + */
> +static int lwmi_psy_prop_is_writeable(struct power_supply *ps,
> +				      const struct power_supply_ext *ext,
> +				      void *ext_data,
> +				      enum power_supply_property prop)
> +{
> +	struct lwmi_om_priv *priv = ext_data;
> +	struct capdata00 capdata;
> +	u32 attribute_id = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
> +	int ret;
> +
> +	ret = lwmi_cd00_get_data(priv->cd00_list, attribute_id, &capdata);
> +	if (ret)
> +		return false;
> +
> +	dev_dbg(&priv->wdev->dev, "Battery charge mode (%#08x) support level: %x\n",
> +		attribute_id, capdata.supported);
> +
> +	return capdata.supported;
> +}
> +
> +static const enum power_supply_property lwmi_psy_ext_props[] = {
> +	POWER_SUPPLY_PROP_CHARGE_TYPES,
> +};
> +
> +static const struct power_supply_ext lwmi_psy_ext = {
> +	.name			= LWMI_OM_SYSFS_NAME,
> +	.properties		= lwmi_psy_ext_props,
> +	.num_properties		= ARRAY_SIZE(lwmi_psy_ext_props),
> +	.charge_types		= (BIT(POWER_SUPPLY_CHARGE_TYPE_STANDARD) |
> +				   BIT(POWER_SUPPLY_CHARGE_TYPE_LONGLIFE)),
> +	.get_property		= lwmi_psy_ext_get_prop,
> +	.set_property		= lwmi_psy_ext_set_prop,
> +	.property_is_writeable	= lwmi_psy_prop_is_writeable,
> +};
> +
> +/**
> + * lwmi_add_battery() - Connect the power_supply_ext
> + * @battery: The battery to extend
> + * @hook: The driver hook used to extend the battery
> + *
> + * Return: 0 on success, or an error.
> + */
> +static int lwmi_add_battery(struct power_supply *battery, struct acpi_battery_hook *hook)
> +{
> +	struct lwmi_om_priv *priv = container_of(hook, struct lwmi_om_priv, battery_hook);
> +
> +	return power_supply_register_extension(battery, &lwmi_psy_ext, &priv->wdev->dev, priv);
> +}
> +
> +/**
> + * lwmi_remove_battery() - Disconnect the power_supply_ext
> + * @battery: The battery that was extended
> + * @hook: The driver hook used to extend the battery
> + *
> + * Return: 0 on success, or an error.
> + */
> +static int lwmi_remove_battery(struct power_supply *battery, struct acpi_battery_hook *hook)
> +{
> +	power_supply_unregister_extension(battery, &lwmi_psy_ext);
> +	return 0;
> +}
> +
> +/**
> + * lwmi_acpi_match() - Attempts to return the ideapad acpi handle
> + * @handle: The ACPI handle that manages battery charging
> + * @lvl: Unused
> + * @context: Void pointer to the acpi_handle object to return
> + * @retval: Unused
> + *
> + * Checks if the ideapad_laptop driver is going to manage charge_type first,
> + * then if not, hooks the battery to our WMI methods.
> + *
> + * Return: AE_CTRL_TERMINATE if found, AE_OK if not found.
> + */
> +static acpi_status lwmi_acpi_match(acpi_handle handle, u32 lvl,
> +				   void *context, void **retval)
> +{
> +	if (!handle)
> +		return AE_OK;
> +
> +	acpi_handle *ahand = context;
> +	*ahand = handle;
> +
> +	return AE_CTRL_TERMINATE;
> +}
> +
> +/**
> + * lwmi_om_ps_ext_init() - Hooks power supply extension to device battery
> + * @priv: Driver private data
> + *
> + * Checks if the ideapad_laptop driver is going to manage charge_type first,
> + * then if not, hooks the battery to our WMI methods.
> + */
> +static void lwmi_om_ps_ext_init(struct lwmi_om_priv *priv)
> +{
> +	static const char * const ideapad_hid = "VPC2004";
> +	acpi_handle handle = NULL;
> +	int ret;
> +
> +	/* Deconflict ideapad_laptop driver */
> +	ret = acpi_get_devices(ideapad_hid, lwmi_acpi_match, &handle, NULL);
> +	if (ret)
> +		return;
> +
> +	if (!handle)
> +		return;

I am a bit confused here. If VPC2004 is not found, handle should remain
NULL. In this case we have no conflict, right?

Thanks,
Rong

> +
> +	if (acpi_has_method(handle, "GBMD") && acpi_has_method(handle, "SBMC")) {
> +		dev_dbg(&priv->wdev->dev, "ideapad_laptop driver manages battery for device.\n");
> +		return;
> +	}
> +
> +	/* Add battery hooks */
> +	priv->battery_hook.add_battery	= lwmi_add_battery,
> +	priv->battery_hook.remove_battery	= lwmi_remove_battery,
> +	priv->battery_hook.name		= "Lenovo WMI Other Battery Extension",
> +
> +	ret = devm_battery_hook_register(&priv->wdev->dev, &priv->battery_hook);
> +	if (ret)
> +		dev_err(&priv->wdev->dev, "Error during battery hook: %i\n", ret);
> +}
> +
>  /* ======== fw_attributes (component: lenovo-wmi-capdata 01) ======== */
>  
>  struct tunable_attr_01 {
> @@ -1325,6 +1554,7 @@ static int lwmi_om_master_bind(struct device *dev)
>  		return -ENODEV;
>  
>  	lwmi_om_fan_info_collect_cd00(priv);
> +	lwmi_om_ps_ext_init(priv);
>  
>  	return lwmi_om_fw_attr_add(priv);
>  }

Re: [PATCH v3 2/6] platform/x86: lenovo-wmi-other: Limit adding attributes to supported devices

[Derek J. Clark]

On February 24, 2026 12:47:56 AM PST, "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com> wrote:
>On Tue, 24 Feb 2026, Derek J. Clark wrote:
>
>> Adds lwmi_is_attr_01_supported, and only creates the attribute subfolder
>> if the attribute is supported by the hardware. Due to some poorly
>> implemented BIOS, this is a multi-step sequence of events. This is
>> because:
>> - Some BIOS support getting the capability data from custom mode (0xff),
>>   while others only support it in no-mode (0x00).
>> - Similarly, some BIOS support get/set for the current value from custom
>>   mode (0xff), while others only support it in no-mode (0x00).
>> - Some BIOS report capability data for a method that is not fully
>>   implemented.
>> - Some BIOS have methods fully implemented, but no complimentary
>>   capability data.
>> 
>> To ensure we only expose fully implemented methods with corresponding
>> capability data, we check each outcome before reporting that an
>> attribute can be supported.
>> 
>> Checking for lwmi_is_attr_01_supported during remove is not done to
>> ensure that we don't attempt to call cd01 or send WMI events if one of
>> the interfaces being removed was the cause of the driver unloading.
>> 
>
>> While adding members to tunable_attr_01, remove unused capdata pointer
>> and limit size of all ID's to the appropriate size.
>
>Please don't mix changes like this. Create a seprate patch for it.
>
>> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
>> Reported-by: Kurt Borja <kuurtb@gmail.com>
>> Closes: https://lore.kernel.org/platform-driver-x86/DG60P3SHXR8H.3NSEHMZ6J7XRC@gmail.com/
>> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
>> ---
>>  drivers/platform/x86/lenovo/wmi-other.c | 117 +++++++++++++++++++++---
>>  1 file changed, 102 insertions(+), 15 deletions(-)
>> 
>> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
>> index 95886df39c8d..f3f12303e379 100644
>> --- a/drivers/platform/x86/lenovo/wmi-other.c
>> +++ b/drivers/platform/x86/lenovo/wmi-other.c
>> @@ -545,11 +545,12 @@ static void lwmi_om_fan_info_collect_cd_fan(struct device *dev, struct cd_list *
>>  /* ======== fw_attributes (component: lenovo-wmi-capdata 01) ======== */
>>  
>>  struct tunable_attr_01 {
>> -	struct capdata01 *capdata;
>>  	struct device *dev;
>> -	u32 feature_id;
>> -	u32 device_id;
>> -	u32 type_id;
>> +	u8 feature_id;
>> +	u8 device_id;
>> +	u8 type_id;
>> +	u8 cd_mode_id; /* mode arg for searching capdata */
>> +	u8 cv_mode_id; /* mode arg for set/get current_value */
>>  };
>>  
>>  static struct tunable_attr_01 ppt_pl1_spl = {
>> @@ -716,7 +717,7 @@ static ssize_t attr_capdata01_show(struct kobject *kobj,
>>  	int value, ret;
>>  
>>  	attribute_id = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
>> -				    LWMI_GZ_THERMAL_MODE_CUSTOM, tunable_attr->type_id);
>> +				    tunable_attr->cd_mode_id, tunable_attr->type_id);
>>  
>>  	ret = lwmi_cd01_get_data(priv->cd01_list, attribute_id, &capdata);
>>  	if (ret)
>> @@ -782,7 +783,7 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
>>  		return -EBUSY;
>>  
>>  	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
>> -				 mode, tunable_attr->type_id);
>> +				 tunable_attr->cd_mode_id, tunable_attr->type_id);
>>  
>>  	ret = lwmi_cd01_get_data(priv->cd01_list, args.arg0, &capdata);
>>  	if (ret)
>> @@ -795,6 +796,8 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
>>  	if (value < capdata.min_value || value > capdata.max_value)
>>  		return -EINVAL;
>>  
>> +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
>> +				 tunable_attr->cv_mode_id, tunable_attr->type_id);
>>  	args.arg1 = value;
>>  
>>  	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_SET,
>> @@ -828,13 +831,16 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
>>  	struct lwmi_om_priv *priv = dev_get_drvdata(tunable_attr->dev);
>>  	struct wmi_method_args_32 args;
>>  	enum thermal_mode mode;
>> -	int retval;
>> -	int ret;
>> +	int retval, ret;
>>  
>>  	ret = lwmi_om_notifier_call(&mode);
>>  	if (ret)
>>  		return ret;
>>  
>> +	/* If "no-mode" is the supported mode, ensure we never send current mode */
>> +	if (tunable_attr->cv_mode_id == LWMI_GZ_THERMAL_MODE_NONE)
>> +		mode = tunable_attr->cv_mode_id;
>> +
>>  	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
>>  				 mode, tunable_attr->type_id);
>>  
>> @@ -847,6 +853,85 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
>>  	return sysfs_emit(buf, "%d\n", retval);
>>  }
>>  
>> +/**
>> + * lwmi_attr_01_is_supported() - Determine if the given attribute is supported.
>> + * @tunable_attr: The attribute to verify.
>> + *
>> + * First check if the attribute has a corresponding capdata01 table in the cd01
>> + * module under the "custom" mode (0xff). If that is not present then check if
>> + * there is a corresponding "no-mode" (0x00) entry. If either of those passes,
>> + * check capdata->supported for values > 0. If capdata is available, attempt to
>> + * determine the set/get mode for the current value property using a similar
>> + * pattern. If the value returned by either custom or no-mode is 0, or we get
>> + * an error, we assume that mode is not supported. If any of the above checks
>> + * fail then the attribute is not fully supported.
>> + *
>> + * The probed cd_mode_id/cv_mode_id are stored on the tunable_attr for later
>> + * reference.
>> + *
>> + * Return: Support level, or an error code.
>> + */
>> +static int lwmi_attr_01_is_supported(struct tunable_attr_01 *tunable_attr)
>> +{
>> +	struct lwmi_om_priv *priv = dev_get_drvdata(tunable_attr->dev);
>> +	u8 mode = LWMI_GZ_THERMAL_MODE_CUSTOM;
>> +	struct wmi_method_args_32 args;
>> +	struct capdata01 capdata;
>> +	int retval, ret;
>> +
>> +	/* Determine tunable_attr->cd_mode_id */
>> +no_mode_fallback_1:
>> +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
>> +				 mode, tunable_attr->type_id);
>> +
>> +	ret = lwmi_cd01_get_data(priv->cd01_list, args.arg0, &capdata);
>> +	if (ret && mode) {
>> +		dev_dbg(tunable_attr->dev, "Attribute id %x not supported\n", args.arg0);
>
>Add include.
>
>> +		mode = LWMI_GZ_THERMAL_MODE_NONE;
>> +		goto no_mode_fallback_1;
>
>Is it possible to make a helper so you don't need these back gotos?
>

Sure. How about I put both modes into an array and loop through them, passing the values to a function that will evaluate it. I can break if there is a match and throw an error if there's no match at the end.

>> +	}
>> +	if (ret)
>> +		goto not_supported;
>> +	if (!capdata.supported) {
>> +		ret = -EOPNOTSUPP;
>> +		goto not_supported;
>> +	}
>> +
>> +	tunable_attr->cd_mode_id = mode;
>> +
>> +	/* Determine tunable_attr->cv_mode_id */
>> +	mode = LWMI_GZ_THERMAL_MODE_CUSTOM;
>> +no_mode_fallback_2:
>> +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
>> +				 mode, tunable_attr->type_id);
>> +
>> +	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_GET,
>> +				    (unsigned char *)&args, sizeof(args),
>> +				    &retval);
>> +	if ((ret && mode) || (!retval && mode)) {
>> +		dev_dbg(tunable_attr->dev, "Attribute id %x not supported\n", args.arg0);
>> +		mode = LWMI_GZ_THERMAL_MODE_NONE;
>> +		goto no_mode_fallback_2;
>
>Same question here?
>
>> +	}
>> +	if (ret)
>> +		goto not_supported;
>> +	if (retval == 0) {
>> +		ret = -EOPNOTSUPP;
>> +		goto not_supported;
>> +	}
>> +
>> +	tunable_attr->cv_mode_id = mode;
>> +	dev_dbg(tunable_attr->dev, "cd_mode_id: %02x%02x%02x%02x, cv_mode_id: %#08x attribute support level: %x\n",
>> +		tunable_attr->device_id, tunable_attr->feature_id, tunable_attr->cd_mode_id,
>> +		tunable_attr->type_id, args.arg0, capdata.supported);
>> +
>> +	return capdata.supported;
>> +
>> +not_supported:
>> +	dev_dbg(tunable_attr->dev, "Attribute id %x not supported\n", args.arg0);
>> +	return ret;
>> +}
>> +
>>  /* Lenovo WMI Other Mode Attribute macros */
>>  #define __LWMI_ATTR_RO(_func, _name)                                  \
>>  	{                                                             \
>> @@ -970,19 +1055,21 @@ static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
>>  	}
>>  
>>  	for (i = 0; i < ARRAY_SIZE(cd01_attr_groups) - 1; i++) {
>> -		err = sysfs_create_group(&priv->fw_attr_kset->kobj,
>> -					 cd01_attr_groups[i].attr_group);
>> -		if (err)
>> -			goto err_remove_groups;
>> -
>>  		cd01_attr_groups[i].tunable_attr->dev = &priv->wdev->dev;
>> +		if (lwmi_attr_01_is_supported(cd01_attr_groups[i].tunable_attr) > 0) {
>
>Reverse logic and use continue.
>
>> +			err = sysfs_create_group(&priv->fw_attr_kset->kobj,
>> +						 cd01_attr_groups[i].attr_group);
>> +			if (err)
>> +				goto err_remove_groups;
>> +		}
>>  	}
>>  	return 0;
>>  
>>  err_remove_groups:
>>  	while (i--)
>> -		sysfs_remove_group(&priv->fw_attr_kset->kobj,
>> -				   cd01_attr_groups[i].attr_group);
>> +		if (lwmi_attr_01_is_supported(cd01_attr_groups[i].tunable_attr) > 0)
>
>Reverse logic + continue.
>
>> +			sysfs_remove_group(&priv->fw_attr_kset->kobj,
>> +					   cd01_attr_groups[i].attr_group);
>
>You need to add braces for multiline constructs.
>
>>  
>>  	kset_unregister(priv->fw_attr_kset);
>>  
>> 
>

Re: [PATCH v3 5/6] platform-x86: lenovo-wmi-other: Rename LWMI_OM_FW_ATTR_BASE_PATH

[Derek J. Clark]

On February 24, 2026 12:51:09 AM PST, "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com> wrote:
>On Tue, 24 Feb 2026, Derek J. Clark wrote:
>
>> In the next patch a power supply extension is added which requires
>> a name attribute. Instead of creating another const macro with the
>> same information, replace LWMI_OM_FW_ATTR_BASE_PATH with
>> LWMI_OM_NAME and use that for firmware attributes and power supply
>
>This doesn't match the code change??

Woops, sorry, I forgot to update the variable name in the commit message after I changed it.

Thanks.

>> extension.
>> 
>> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
>> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
>> ---
>>  drivers/platform/x86/lenovo/wmi-other.c | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
>> index 67768f6a50e0..7f0d5a17b44f 100644
>> --- a/drivers/platform/x86/lenovo/wmi-other.c
>> +++ b/drivers/platform/x86/lenovo/wmi-other.c
>> @@ -90,13 +90,13 @@ enum lwmi_feature_id_gpu {
>>  #define LWMI_FAN_NR 4
>>  #define LWMI_FAN_ID(x) ((x) + LWMI_FAN_ID_BASE)
>>  
>> +#define LWMI_FAN_DIV 100
>> +
>>  #define LWMI_ATTR_ID_FAN_RPM(x)                                   \
>>  	LWMI_ATTR_ID(LWMI_DEVICE_ID_FAN, LWMI_FEATURE_ID_FAN_RPM, \
>>  		     LWMI_GZ_THERMAL_MODE_NONE, LWMI_FAN_ID(x))
>>  
>> -#define LWMI_FAN_DIV 100
>
>Unrelated change.
>
>> -
>> -#define LWMI_OM_FW_ATTR_BASE_PATH "lenovo-wmi-other"
>> +#define LWMI_OM_SYSFS_NAME "lenovo-wmi-other"
>>  #define LWMI_OM_HWMON_NAME "lenovo_wmi_other"
>>  
>>  static BLOCKING_NOTIFIER_HEAD(om_chain_head);
>> @@ -1233,8 +1233,7 @@ static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
>>  
>>  	priv->fw_attr_dev = device_create(&firmware_attributes_class, NULL,
>>  					  MKDEV(0, 0), NULL, "%s-%u",
>> -					  LWMI_OM_FW_ATTR_BASE_PATH,
>> -					  priv->ida_id);
>> +					  LWMI_OM_SYSFS_NAME, priv->ida_id);
>>  	if (IS_ERR(priv->fw_attr_dev)) {
>>  		err = PTR_ERR(priv->fw_attr_dev);
>>  		goto err_free_ida;
>> 
>

Re: [PATCH v3 0/6] platform-x86: lenovo-wmi: Add fixes and enhancement

[Rong Zhang]

Hi Derek,

Sorry for the late reply. I was busier (yet happier ;-)) than I had
expected during the Spring Festival/Chinese New Year.

On Tue, 2026-02-24 at 04:31 +0000, Derek J. Clark wrote:
> This series adds many much needed features and fixes to the lenovo-wmi
> drivers.
> 
> Patch 1 adds a macro to make assigning attribute ID's for capdata
> cleaner and easier.
> Patch 2 addresses bugs where devices that don't support exposed
> attributes would still create the attribute, and also attempts to
> identify the correct capdata and set/get methods as some legacy
> interfaces don't use the custom mode in the method or capdata ID.
> Patch 3 adds the remaining CPU attributes that weren't previously
> exposed.
> Patch 4 adds the GPU attributes that weren't previously exposed.
> Patch 5 consolidates name constants in preparation for patch 6.
> Patch 6 adds battery charge-type limiting when supported only by WMI.
> 
> After applying Rong's series from
> https://lore.kernel.org/platform-driver-x86/20260210191938.137234-1-i@rong.moe/
> I found that there weren't any conflicts, so rather than combine
> the series we can just review each separately.
> 
> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>

The patch series makes sense to me in general except for some issues
mentioned in my previous replies.

With style issues fixed, you can add

Reviewed-By: Rong Zhang <i@rong.moe>

to patch 1,3,4,5.

Thanks,
Rong

> ---
> v3:
>   - Re-add HWMON name const and jsut rename LWMI_OM_FW_ATTR_BASE_PATH
>   - Fix linker warnings by moving acpi/battery include to the end of the
>     list.
>   - Remove CPU/GPU OC features. These attributes are BOOL type and will
>     need a new constructor that I'll add later.
> v2:
>   - Fix gpu_mode misisng from attributes list.
>   - Fix prototypes for power suppy patch.
>   - Reorganize CPU and GPU attributes alphabetically.
>   - Break out the patch consolidating the driver name cost.
>   - Move some of the refactoring of attribute_id back to into patch 1
>     where it belongs.
>   - Fix some additional typos in function prototypes.
> v1: https://lore.kernel.org/platform-driver-x86/20260213081243.794288-1-derekjohn.clark@gmail.com/
> 
> Derek J. Clark (6):
>   platform/x86: lenovo-wmi-other: Add LWMI_ATTR_ID Macro
>   platform/x86: lenovo-wmi-other: Limit adding attributes to supported
>     devices
>   platform/x86: lenovo-wmi-other: Add missing CPU tunable attributes
>   platform/x86: lenovo-wmi-other: Add GPU tunable attributes
>   platform-x86: lenovo-wmi-other: Rename LWMI_OM_FW_ATTR_BASE_PATH
>   platform/x86: lenovo-wmi-other: Add WMI battery charge limiting.
> 
>  .../wmi/devices/lenovo-wmi-other.rst          |  21 +
>  drivers/platform/x86/lenovo/wmi-capdata.h     |  13 +-
>  drivers/platform/x86/lenovo/wmi-gamezone.h    |   1 +
>  drivers/platform/x86/lenovo/wmi-other.c       | 605 ++++++++++++++++--
>  4 files changed, 593 insertions(+), 47 deletions(-)

Re: [PATCH v3 6/6] platform/x86: lenovo-wmi-other: Add WMI battery charge limiting.

[Derek J. Clark]

On February 24, 2026 1:05:06 AM PST, "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com> wrote:
>On Tue, 24 Feb 2026, Derek J. Clark wrote:
>
>> Add charge-type power supply extension for devices that support WMI based
>> charge enable/disable. Lenovo Legion devices that implement function ID
>> and capdata 00 ID 0x03010001 are able to enable or disable charging
>> through the lenovo-wmi-other interface. The ideapad_laptop driver
>> conflicts with this if it can also provide the attribute, so we have to
>> get the acpi_handle and check for the same ACPI methods that enable the
>> feature in that driver. The ACPI method is more reliable from my testing
>> when both are present, so there is no need to modify the ideapad_laptop
>> driver instead.
>> 
>> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
>> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
>> ---
>>  drivers/platform/x86/lenovo/wmi-capdata.h |   1 +
>>  drivers/platform/x86/lenovo/wmi-other.c   | 230 ++++++++++++++++++++++
>>  2 files changed, 231 insertions(+)
>> 
>> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
>> index b7f9ee7b301a..00471551e7d6 100644
>> --- a/drivers/platform/x86/lenovo/wmi-capdata.h
>> +++ b/drivers/platform/x86/lenovo/wmi-capdata.h
>> @@ -26,6 +26,7 @@
>>  enum lwmi_device_id {
>>  	LWMI_DEVICE_ID_CPU = 0x01,
>>  	LWMI_DEVICE_ID_GPU = 0x02,
>> +	LWMI_DEVICE_ID_PSU = 0x03,
>>  	LWMI_DEVICE_ID_FAN = 0x04,
>>  };
>>  
>> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
>> index 7f0d5a17b44f..b2daff1b45c2 100644
>> --- a/drivers/platform/x86/lenovo/wmi-other.c
>> +++ b/drivers/platform/x86/lenovo/wmi-other.c
>> @@ -42,9 +42,12 @@
>>  #include <linux/module.h>
>>  #include <linux/notifier.h>
>>  #include <linux/platform_profile.h>
>> +#include <linux/power_supply.h>
>>  #include <linux/types.h>
>>  #include <linux/wmi.h>
>>  
>> +#include <acpi/battery.h>
>> +
>>  #include "wmi-capdata.h"
>>  #include "wmi-events.h"
>>  #include "wmi-gamezone.h"
>> @@ -78,10 +81,17 @@ enum lwmi_feature_id_gpu {
>>  	LWMI_FEATURE_ID_GPU_NV_CPU_BOOST = 0x0b,
>>  };
>>  
>> +enum lwmi_feature_id_psu {
>> +	LWMI_FEATURE_ID_PSU_INSTANT_MODE = 0x01,
>> +	LWMI_FEATURE_ID_PSU_CHARGE_MODE = 0x02,
>
>Align.
>
>> +};
>> +
>>  #define LWMI_FEATURE_ID_FAN_RPM 0x03
>>  
>>  #define LWMI_TYPE_ID_NONE 0x00
>>  #define LWMI_TYPE_ID_CROSSLOAD 0x01
>> +#define LWMI_TYPE_ID_PSU_AC 0x01
>> +#define LWMI_TYPE_ID_PSU_PD 0x02
>
>These should be aligned as well but that will impact more than this patch 
>to avoid back and forth changes within the series.

I can start aligning them in the CPU attrs patch that adds crossload and keep that spacing here since it's longer.

>>  
>>  #define LWMI_FEATURE_VALUE_GET 17
>>  #define LWMI_FEATURE_VALUE_SET 18
>> @@ -92,10 +102,17 @@ enum lwmi_feature_id_gpu {
>>  
>>  #define LWMI_FAN_DIV 100
>>  
>> +#define LWMI_CHARGE_MODE_ENABLED	0x00
>> +#define LWMI_CHARGE_MODE_DISABLED	0x01
>> +
>>  #define LWMI_ATTR_ID_FAN_RPM(x)                                   \
>>  	LWMI_ATTR_ID(LWMI_DEVICE_ID_FAN, LWMI_FEATURE_ID_FAN_RPM, \
>>  		     LWMI_GZ_THERMAL_MODE_NONE, LWMI_FAN_ID(x))
>>  
>> +#define LWMI_ATTR_ID_PSU(feat, type)		\
>> +	LWMI_ATTR_ID(LWMI_DEVICE_ID_PSU, feat,		\
>> +		     LWMI_GZ_THERMAL_MODE_NONE, type)
>> +
>>  #define LWMI_OM_SYSFS_NAME "lenovo-wmi-other"
>>  #define LWMI_OM_HWMON_NAME "lenovo_wmi_other"
>>  
>> @@ -137,6 +154,8 @@ struct lwmi_om_priv {
>>  		bool capdata00_collected : 1;
>>  		bool capdata_fan_collected : 1;
>>  	} fan_flags;
>> +
>> +	struct acpi_battery_hook battery_hook;
>>  };
>>  
>>  /*
>> @@ -561,6 +580,216 @@ static void lwmi_om_fan_info_collect_cd_fan(struct device *dev, struct cd_list *
>>  	lwmi_om_hwmon_add(priv);
>>  }
>>  
>> +/* ======== Power Supply Extension (component: lenovo-wmi-capdata 00) ======== */
>> +
>> +/**
>> + * lwmi_psy_ext_get_prop() - Get a power_supply_ext property
>> + * @ps: The battery that was extended
>> + * @ext: The extension
>> + * @ext_data: Pointer the lwmi_om_priv drvdata
>> + * @prop: The property to read
>> + * @val: The value to return
>> + *
>> + * Writes the given value to the power_supply_ext property
>> + *
>> + * Return: 0 on success, or an error
>> + */
>> +static int lwmi_psy_ext_get_prop(struct power_supply *ps,
>> +				 const struct power_supply_ext *ext,
>> +				 void *ext_data,
>> +				 enum power_supply_property prop,
>> +				 union power_supply_propval *val)
>> +{
>> +	struct lwmi_om_priv *priv = ext_data;
>> +	struct wmi_method_args_32 args;
>> +	u32 retval;
>> +	int ret;
>> +
>> +	args.arg0 = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
>> +
>> +	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_GET,
>> +				    (unsigned char *)&args, sizeof(args),
>> +				    &retval);
>> +	if (ret)
>> +		return ret;
>> +
>> +	dev_dbg(&priv->wdev->dev, "Got return value %x for property %x\n", retval, prop);
>> +
>> +	if (retval == LWMI_CHARGE_MODE_DISABLED)
>> +		val->intval = POWER_SUPPLY_CHARGE_TYPE_LONGLIFE;
>> +	else
>> +		val->intval = POWER_SUPPLY_CHARGE_TYPE_STANDARD;
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * lwmi_psy_ext_set_prop() - Set a power_supply_ext property
>> + * @ps: The battery that was extended
>> + * @ext: The extension
>> + * @ext_data: Pointer the lwmi_om_priv drvdata
>> + * @prop: The property to write
>> + * @val: The value to write
>> + *
>> + * Writes the given value to the power_supply_ext property
>> + *
>> + * Return: 0 on success, or an error
>> + */
>> +static int lwmi_psy_ext_set_prop(struct power_supply *ps,
>> +				 const struct power_supply_ext *ext,
>> +				 void *ext_data,
>> +				 enum power_supply_property prop,
>> +				 const union power_supply_propval *val)
>> +{
>> +	struct lwmi_om_priv *priv = ext_data;
>> +	struct wmi_method_args_32 args;
>> +
>> +	args.arg0 = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
>> +	if (val->intval == POWER_SUPPLY_CHARGE_TYPE_LONGLIFE)
>> +		args.arg1 = LWMI_CHARGE_MODE_DISABLED;
>> +	else
>> +		args.arg1 = LWMI_CHARGE_MODE_ENABLED;
>> +
>> +	dev_dbg(&priv->wdev->dev, "Attempting to set %#08x for property %x to %x\n",
>> +		args.arg0, prop, args.arg1);
>> +
>> +	return lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_SET,
>> +				     (unsigned char *)&args, sizeof(args), NULL);
>> +}
>> +
>> +/**
>> + * lwmi_psy_prop_is_writeable() - Determine if the property is supported
>> + * @ps: The battery that was extended
>> + * @ext: The extension
>> + * @ext_data: Pointer the lwmi_om_priv drvdata
>> + * @prop: The property to check
>> + *
>> + * Checks capdata 00 to determine if the property is supported.
>> + *
>> + * Return: Support level, or false
>> + */
>> +static int lwmi_psy_prop_is_writeable(struct power_supply *ps,
>> +				      const struct power_supply_ext *ext,
>> +				      void *ext_data,
>> +				      enum power_supply_property prop)
>> +{
>> +	struct lwmi_om_priv *priv = ext_data;
>> +	struct capdata00 capdata;
>> +	u32 attribute_id = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
>> +	int ret;
>> +
>> +	ret = lwmi_cd00_get_data(priv->cd00_list, attribute_id, &capdata);
>> +	if (ret)
>> +		return false;
>> +
>> +	dev_dbg(&priv->wdev->dev, "Battery charge mode (%#08x) support level: %x\n",
>
>IIRC, 0x should be included into the length of of the field, so perhaps 
>%#10x for full u32 but it's long time ago I've used that formatting for 
>anything so I might be wrong.

I got here after some trial and error, seems to print alright.

>> +		attribute_id, capdata.supported);
>> +
>> +	return capdata.supported;
>> +}
>> +
>> +static const enum power_supply_property lwmi_psy_ext_props[] = {
>> +	POWER_SUPPLY_PROP_CHARGE_TYPES,
>> +};
>> +
>> +static const struct power_supply_ext lwmi_psy_ext = {
>> +	.name			= LWMI_OM_SYSFS_NAME,
>> +	.properties		= lwmi_psy_ext_props,
>> +	.num_properties		= ARRAY_SIZE(lwmi_psy_ext_props),
>> +	.charge_types		= (BIT(POWER_SUPPLY_CHARGE_TYPE_STANDARD) |
>> +				   BIT(POWER_SUPPLY_CHARGE_TYPE_LONGLIFE)),
>> +	.get_property		= lwmi_psy_ext_get_prop,
>> +	.set_property		= lwmi_psy_ext_set_prop,
>> +	.property_is_writeable	= lwmi_psy_prop_is_writeable,
>> +};
>> +
>> +/**
>> + * lwmi_add_battery() - Connect the power_supply_ext
>> + * @battery: The battery to extend
>> + * @hook: The driver hook used to extend the battery
>> + *
>> + * Return: 0 on success, or an error.
>> + */
>> +static int lwmi_add_battery(struct power_supply *battery, struct acpi_battery_hook *hook)
>> +{
>> +	struct lwmi_om_priv *priv = container_of(hook, struct lwmi_om_priv, battery_hook);
>> +
>> +	return power_supply_register_extension(battery, &lwmi_psy_ext, &priv->wdev->dev, priv);
>> +}
>> +
>> +/**
>> + * lwmi_remove_battery() - Disconnect the power_supply_ext
>> + * @battery: The battery that was extended
>> + * @hook: The driver hook used to extend the battery
>> + *
>> + * Return: 0 on success, or an error.
>> + */
>> +static int lwmi_remove_battery(struct power_supply *battery, struct acpi_battery_hook *hook)
>> +{
>> +	power_supply_unregister_extension(battery, &lwmi_psy_ext);
>> +	return 0;
>> +}
>> +
>> +/**
>> + * lwmi_acpi_match() - Attempts to return the ideapad acpi handle
>> + * @handle: The ACPI handle that manages battery charging
>> + * @lvl: Unused
>> + * @context: Void pointer to the acpi_handle object to return
>> + * @retval: Unused
>> + *
>> + * Checks if the ideapad_laptop driver is going to manage charge_type first,
>> + * then if not, hooks the battery to our WMI methods.
>> + *
>> + * Return: AE_CTRL_TERMINATE if found, AE_OK if not found.
>> + */
>> +static acpi_status lwmi_acpi_match(acpi_handle handle, u32 lvl,
>> +				   void *context, void **retval)
>> +{
>> +	if (!handle)
>> +		return AE_OK;
>> +
>> +	acpi_handle *ahand = context;
>
>Please don't declare variable mid-function (except cleanup.h related 
>variables which is why it had to be allowed so compiler won't anymore 
>complain about it).
>
>> +	*ahand = handle;
>> +
>> +	return AE_CTRL_TERMINATE;
>> +}
>> +
>> +/**
>> + * lwmi_om_ps_ext_init() - Hooks power supply extension to device battery
>> + * @priv: Driver private data
>> + *
>> + * Checks if the ideapad_laptop driver is going to manage charge_type first,
>> + * then if not, hooks the battery to our WMI methods.
>> + */
>> +static void lwmi_om_ps_ext_init(struct lwmi_om_priv *priv)
>> +{
>> +	static const char * const ideapad_hid = "VPC2004";
>> +	acpi_handle handle = NULL;
>> +	int ret;
>> +
>> +	/* Deconflict ideapad_laptop driver */
>> +	ret = acpi_get_devices(ideapad_hid, lwmi_acpi_match, &handle, NULL);
>> +	if (ret)
>> +		return;
>> +
>> +	if (!handle)
>> +		return;
>> +
>> +	if (acpi_has_method(handle, "GBMD") && acpi_has_method(handle, "SBMC")) {
>> +		dev_dbg(&priv->wdev->dev, "ideapad_laptop driver manages battery for device.\n");
>> +		return;
>> +	}
>> +
>> +	/* Add battery hooks */
>> +	priv->battery_hook.add_battery	= lwmi_add_battery,
>> +	priv->battery_hook.remove_battery	= lwmi_remove_battery,
>> +	priv->battery_hook.name		= "Lenovo WMI Other Battery Extension",
>
>????
>
>Use semicolons to separate statements.
>
>And those tabs look odd too (not even aligning the lines), I'd prefer 
>just using just normal single space as aligning with tabs will cause 
>churn if adding new fields that need adjusting alignment of all the 
>members.
>
>> +
>> +	ret = devm_battery_hook_register(&priv->wdev->dev, &priv->battery_hook);
>> +	if (ret)
>> +		dev_err(&priv->wdev->dev, "Error during battery hook: %i\n", ret);
>> +}
>> +
>>  /* ======== fw_attributes (component: lenovo-wmi-capdata 01) ======== */
>>  
>>  struct tunable_attr_01 {
>> @@ -1325,6 +1554,7 @@ static int lwmi_om_master_bind(struct device *dev)
>>  		return -ENODEV;
>>  
>>  	lwmi_om_fan_info_collect_cd00(priv);
>> +	lwmi_om_ps_ext_init(priv);
>>  
>>  	return lwmi_om_fw_attr_add(priv);
>>  }
>> 
>

Re: [PATCH v3 2/6] platform/x86: lenovo-wmi-other: Limit adding attributes to supported devices

[Derek J. Clark]

On February 25, 2026 9:14:12 AM PST, Rong Zhang <i@rong.moe> wrote:
>Hi Derek,
>
>On Tue, 2026-02-24 at 04:31 +0000, Derek J. Clark wrote:
>> Adds lwmi_is_attr_01_supported, and only creates the attribute subfolder
>> if the attribute is supported by the hardware. Due to some poorly
>> implemented BIOS, this is a multi-step sequence of events. This is
>> because:
>> - Some BIOS support getting the capability data from custom mode (0xff),
>>   while others only support it in no-mode (0x00).
>> - Similarly, some BIOS support get/set for the current value from custom
>>   mode (0xff), while others only support it in no-mode (0x00).
>> - Some BIOS report capability data for a method that is not fully
>>   implemented.
>> - Some BIOS have methods fully implemented, but no complimentary
>>   capability data.
>> 
>> To ensure we only expose fully implemented methods with corresponding
>> capability data, we check each outcome before reporting that an
>> attribute can be supported.
>> 
>> Checking for lwmi_is_attr_01_supported during remove is not done to
>> ensure that we don't attempt to call cd01 or send WMI events if one of
>> the interfaces being removed was the cause of the driver unloading.
>> 
>> While adding members to tunable_attr_01, remove unused capdata pointer
>> and limit size of all ID's to the appropriate size.
>> 
>> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
>> Reported-by: Kurt Borja <kuurtb@gmail.com>
>> Closes: https://lore.kernel.org/platform-driver-x86/DG60P3SHXR8H.3NSEHMZ6J7XRC@gmail.com/
>> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
>> ---
>>  drivers/platform/x86/lenovo/wmi-other.c | 117 +++++++++++++++++++++---
>>  1 file changed, 102 insertions(+), 15 deletions(-)
>> 
>> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
>> index 95886df39c8d..f3f12303e379 100644
>> --- a/drivers/platform/x86/lenovo/wmi-other.c
>> +++ b/drivers/platform/x86/lenovo/wmi-other.c
>> @@ -545,11 +545,12 @@ static void lwmi_om_fan_info_collect_cd_fan(struct device *dev, struct cd_list *
>>  /* ======== fw_attributes (component: lenovo-wmi-capdata 01) ======== */
>>  
>>  struct tunable_attr_01 {
>> -	struct capdata01 *capdata;
>>  	struct device *dev;
>> -	u32 feature_id;
>> -	u32 device_id;
>> -	u32 type_id;
>> +	u8 feature_id;
>> +	u8 device_id;
>> +	u8 type_id;
>> +	u8 cd_mode_id; /* mode arg for searching capdata */
>> +	u8 cv_mode_id; /* mode arg for set/get current_value */
>>  };
>>  
>>  static struct tunable_attr_01 ppt_pl1_spl = {
>> @@ -716,7 +717,7 @@ static ssize_t attr_capdata01_show(struct kobject *kobj,
>>  	int value, ret;
>>  
>>  	attribute_id = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
>> -				    LWMI_GZ_THERMAL_MODE_CUSTOM, tunable_attr->type_id);
>> +				    tunable_attr->cd_mode_id, tunable_attr->type_id);
>>  
>>  	ret = lwmi_cd01_get_data(priv->cd01_list, attribute_id, &capdata);
>>  	if (ret)
>> @@ -782,7 +783,7 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
>>  		return -EBUSY;
>>  
>>  	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
>> -				 mode, tunable_attr->type_id);
>> +				 tunable_attr->cd_mode_id, tunable_attr->type_id);
>>  
>>  	ret = lwmi_cd01_get_data(priv->cd01_list, args.arg0, &capdata);
>>  	if (ret)
>> @@ -795,6 +796,8 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
>>  	if (value < capdata.min_value || value > capdata.max_value)
>>  		return -EINVAL;
>>  
>> +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
>> +				 tunable_attr->cv_mode_id, tunable_attr->type_id);
>>  	args.arg1 = value;
>>  
>>  	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_SET,
>> @@ -828,13 +831,16 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
>>  	struct lwmi_om_priv *priv = dev_get_drvdata(tunable_attr->dev);
>>  	struct wmi_method_args_32 args;
>>  	enum thermal_mode mode;
>> -	int retval;
>> -	int ret;
>> +	int retval, ret;
>>  
>>  	ret = lwmi_om_notifier_call(&mode);
>>  	if (ret)
>>  		return ret;
>>  
>> +	/* If "no-mode" is the supported mode, ensure we never send current mode */
>> +	if (tunable_attr->cv_mode_id == LWMI_GZ_THERMAL_MODE_NONE)
>> +		mode = tunable_attr->cv_mode_id;
>> +
>>  	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
>>  				 mode, tunable_attr->type_id);
>>  
>> @@ -847,6 +853,85 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
>>  	return sysfs_emit(buf, "%d\n", retval);
>>  }
>>  
>> +/**
>> + * lwmi_attr_01_is_supported() - Determine if the given attribute is supported.
>> + * @tunable_attr: The attribute to verify.
>> + *
>> + * First check if the attribute has a corresponding capdata01 table in the cd01
>> + * module under the "custom" mode (0xff). If that is not present then check if
>> + * there is a corresponding "no-mode" (0x00) entry. If either of those passes,
>> + * check capdata->supported for values > 0. If capdata is available, attempt to
>> + * determine the set/get mode for the current value property using a similar
>> + * pattern. If the value returned by either custom or no-mode is 0, or we get
>> + * an error, we assume that mode is not supported. If any of the above checks
>> + * fail then the attribute is not fully supported.
>> + *
>> + * The probed cd_mode_id/cv_mode_id are stored on the tunable_attr for later
>> + * reference.
>> + *
>> + * Return: Support level, or an error code.
>> + */
>> +static int lwmi_attr_01_is_supported(struct tunable_attr_01 *tunable_attr)
>> +{
>> +	struct lwmi_om_priv *priv = dev_get_drvdata(tunable_attr->dev);
>> +	u8 mode = LWMI_GZ_THERMAL_MODE_CUSTOM;
>> +	struct wmi_method_args_32 args;
>> +	struct capdata01 capdata;
>> +	int retval, ret;
>> +
>> +	/* Determine tunable_attr->cd_mode_id */
>> +no_mode_fallback_1:
>> +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
>> +				 mode, tunable_attr->type_id);
>> +
>> +	ret = lwmi_cd01_get_data(priv->cd01_list, args.arg0, &capdata);
>> +	if (ret && mode) {
>> +		dev_dbg(tunable_attr->dev, "Attribute id %x not supported\n", args.arg0);
>> +		mode = LWMI_GZ_THERMAL_MODE_NONE;
>> +		goto no_mode_fallback_1;
>> +	}
>> +	if (ret)
>> +		goto not_supported;
>> +	if (!capdata.supported) {
>> +		ret = -EOPNOTSUPP;
>> +		goto not_supported;
>> +	}
>> +
>> +	tunable_attr->cd_mode_id = mode;
>> +
>> +	/* Determine tunable_attr->cv_mode_id */
>> +	mode = LWMI_GZ_THERMAL_MODE_CUSTOM;
>> +no_mode_fallback_2:
>> +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
>> +				 mode, tunable_attr->type_id);
>> +
>> +	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_GET,
>> +				    (unsigned char *)&args, sizeof(args),
>> +				    &retval);
>> +	if ((ret && mode) || (!retval && mode)) {
>> +		dev_dbg(tunable_attr->dev, "Attribute id %x not supported\n", args.arg0);
>> +		mode = LWMI_GZ_THERMAL_MODE_NONE;
>> +		goto no_mode_fallback_2;
>> +	}
>> +	if (ret)
>> +		goto not_supported;
>> +	if (retval == 0) {
>> +		ret = -EOPNOTSUPP;
>> +		goto not_supported;
>> +	}
>> +
>
>I agree with Ilpo that backward gotos are hard to read and should be
>avoided if possible.
>
>> +	tunable_attr->cv_mode_id = mode;
>> +	dev_dbg(tunable_attr->dev, "cd_mode_id: %02x%02x%02x%02x, cv_mode_id: %#08x attribute support level: %x\n",
>
>It's a long line. Wrap at the first comma?
>
>> +		tunable_attr->device_id, tunable_attr->feature_id, tunable_attr->cd_mode_id,
>> +		tunable_attr->type_id, args.arg0, capdata.supported);
>> +
>> +	return capdata.supported;
>> +
>> +not_supported:
>> +	dev_dbg(tunable_attr->dev, "Attribute id %x not supported\n", args.arg0);
>> +	return ret;
>> +}
>> +
>>  /* Lenovo WMI Other Mode Attribute macros */
>>  #define __LWMI_ATTR_RO(_func, _name)                                  \
>>  	{                                                             \
>> @@ -970,19 +1055,21 @@ static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
>>  	}
>>  
>>  	for (i = 0; i < ARRAY_SIZE(cd01_attr_groups) - 1; i++) {
>> -		err = sysfs_create_group(&priv->fw_attr_kset->kobj,
>> -					 cd01_attr_groups[i].attr_group);
>> -		if (err)
>> -			goto err_remove_groups;
>> -
>>  		cd01_attr_groups[i].tunable_attr->dev = &priv->wdev->dev;
>> +		if (lwmi_attr_01_is_supported(cd01_attr_groups[i].tunable_attr) > 0) {
>> +			err = sysfs_create_group(&priv->fw_attr_kset->kobj,
>> +						 cd01_attr_groups[i].attr_group);
>> +			if (err)
>> +				goto err_remove_groups;
>> +		}
>>  	}
>>  	return 0;
>>  
>>  err_remove_groups:
>>  	while (i--)
>> -		sysfs_remove_group(&priv->fw_attr_kset->kobj,
>> -				   cd01_attr_groups[i].attr_group);
>> +		if (lwmi_attr_01_is_supported(cd01_attr_groups[i].tunable_attr) > 0)
>
>I don't see lwmi_om_fw_attr_remove() doing the same. If we care about
>symmetric cleanup, I would imagine a bitmap or so recording what have
>been created is used there.
>
>Also it's doubtful if the firmware always return the same result on the
>same parameters. If a broken firmware somehow returns different results
>here, some attributes may be left uncleaned.
>
>That being said, I am not sure if we really need symmetric cleanup.
>IIUC, sysfs_remove_group() is a no-op if we pass an attr_group that has
>not been created.

Hi Rong,
We don't want to query the interface if it was removed, and the no-op is harmless. It doesn't seem worth it to me to retain that in memory to achieve the same results.

>> +			sysfs_remove_group(&priv->fw_attr_kset->kobj,
>> +					   cd01_attr_groups[i].attr_group);
>>  
>>  	kset_unregister(priv->fw_attr_kset);
>>  
>
>Thanks,
>Rong

Re: [PATCH v3 5/6] platform-x86: lenovo-wmi-other: Rename LWMI_OM_FW_ATTR_BASE_PATH

[Derek J. Clark]

On February 25, 2026 9:25:45 AM PST, Rong Zhang <i@rong.moe> wrote:
>Hi Derek,
>
>On Tue, 2026-02-24 at 04:31 +0000, Derek J. Clark wrote:
>> In the next patch a power supply extension is added which requires
>> a name attribute. Instead of creating another const macro with the
>> same information, replace LWMI_OM_FW_ATTR_BASE_PATH with
>> LWMI_OM_NAME and use that for firmware attributes and power supply
>> extension.
>> 
>> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
>> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
>> ---
>>  drivers/platform/x86/lenovo/wmi-other.c | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
>> index 67768f6a50e0..7f0d5a17b44f 100644
>> --- a/drivers/platform/x86/lenovo/wmi-other.c
>> +++ b/drivers/platform/x86/lenovo/wmi-other.c
>> @@ -90,13 +90,13 @@ enum lwmi_feature_id_gpu {
>>  #define LWMI_FAN_NR 4
>>  #define LWMI_FAN_ID(x) ((x) + LWMI_FAN_ID_BASE)
>>  
>> +#define LWMI_FAN_DIV 100
>> +
>>  #define LWMI_ATTR_ID_FAN_RPM(x)                                   \
>>  	LWMI_ATTR_ID(LWMI_DEVICE_ID_FAN, LWMI_FEATURE_ID_FAN_RPM, \
>>  		     LWMI_GZ_THERMAL_MODE_NONE, LWMI_FAN_ID(x))
>>  
>> -#define LWMI_FAN_DIV 100
>
>I placed it here intentionally as it has nothing to do with
>LWMI_ATTR_ID (contrary to other fan-related macros) and is just a
>descriptive macro.

Understandable. I think it makes sense to keep the fan related defines all together since I'll be adding additional LWMI_ATTR_ID defines here later.

>> -
>> -#define LWMI_OM_FW_ATTR_BASE_PATH "lenovo-wmi-other"
>> +#define LWMI_OM_SYSFS_NAME "lenovo-wmi-other"
>>  #define LWMI_OM_HWMON_NAME "lenovo_wmi_other"
>>  
>>  static BLOCKING_NOTIFIER_HEAD(om_chain_head);
>> @@ -1233,8 +1233,7 @@ static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
>>  
>>  	priv->fw_attr_dev = device_create(&firmware_attributes_class, NULL,
>>  					  MKDEV(0, 0), NULL, "%s-%u",
>> -					  LWMI_OM_FW_ATTR_BASE_PATH,
>> -					  priv->ida_id);
>> +					  LWMI_OM_SYSFS_NAME, priv->ida_id);
>>  	if (IS_ERR(priv->fw_attr_dev)) {
>>  		err = PTR_ERR(priv->fw_attr_dev);
>>  		goto err_free_ida;
>
>Thanks,
>Rong

Re: [PATCH v3 6/6] platform/x86: lenovo-wmi-other: Add WMI battery charge limiting.

[Derek J. Clark]

On February 25, 2026 9:55:40 AM PST, Rong Zhang <i@rong.moe> wrote:
>Hi Derek,
>
>There is an extra dot (period) in the title. Please remove it.
>
>On Tue, 2026-02-24 at 04:32 +0000, Derek J. Clark wrote:
>> Add charge-type power supply extension for devices that support WMI based
>> charge enable/disable. Lenovo Legion devices that implement function ID
>> and capdata 00 ID 0x03010001 are able to enable or disable charging
>> through the lenovo-wmi-other interface.
>
>"disable charging"...
>
>I'd expect it pauses charging immediately and unconditionally instead
>of setting a threshold, especially considering the its name
>"INSTANT_MODE". If this is the case, I don't think it fits the
>definition of POWER_SUPPLY_CHARGE_TYPE_LONGLIFE. It'd better to use
>POWER_SUPPLY_CHARGE_TYPE_NONE.

It really just sets a charge threshold of 80% and lowers the charge speed a bit. I'll come up with a better name than the one Lenovo uses in their docs.

>> The ideapad_laptop driver
>> conflicts with this if it can also provide the attribute, so we have to
>> get the acpi_handle and check for the same ACPI methods that enable the
>> feature in that driver. The ACPI method is more reliable from my testing
>> when both are present, so there is no need to modify the ideapad_laptop
>> driver instead.
>
>When ideapad-laptop is blacklisted, there should be no conflict. I
>would suggest adding a module parameter to force wmi-other to register
>psy_ext anyway so that users can have their own choice.
>

That should be fine. I've found the WMI interface is a little buggy on devices that support the ACPI method. I.e. on the Go S and Go 2 it always reports the "Enabled" status, but changing it shows the ACPI method reporting the correct value. (I used a different attr name to test this)

>> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
>> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
>> ---
>>  drivers/platform/x86/lenovo/wmi-capdata.h |   1 +
>>  drivers/platform/x86/lenovo/wmi-other.c   | 230 ++++++++++++++++++++++
>>  2 files changed, 231 insertions(+)
>> 
>> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
>> index b7f9ee7b301a..00471551e7d6 100644
>> --- a/drivers/platform/x86/lenovo/wmi-capdata.h
>> +++ b/drivers/platform/x86/lenovo/wmi-capdata.h
>> @@ -26,6 +26,7 @@
>>  enum lwmi_device_id {
>>  	LWMI_DEVICE_ID_CPU = 0x01,
>>  	LWMI_DEVICE_ID_GPU = 0x02,
>> +	LWMI_DEVICE_ID_PSU = 0x03,
>>  	LWMI_DEVICE_ID_FAN = 0x04,
>>  };
>>  
>> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
>> index 7f0d5a17b44f..b2daff1b45c2 100644
>> --- a/drivers/platform/x86/lenovo/wmi-other.c
>> +++ b/drivers/platform/x86/lenovo/wmi-other.c
>> @@ -42,9 +42,12 @@
>>  #include <linux/module.h>
>>  #include <linux/notifier.h>
>>  #include <linux/platform_profile.h>
>> +#include <linux/power_supply.h>
>>  #include <linux/types.h>
>>  #include <linux/wmi.h>
>>  
>> +#include <acpi/battery.h>
>> +
>>  #include "wmi-capdata.h"
>>  #include "wmi-events.h"
>>  #include "wmi-gamezone.h"
>> @@ -78,10 +81,17 @@ enum lwmi_feature_id_gpu {
>>  	LWMI_FEATURE_ID_GPU_NV_CPU_BOOST = 0x0b,
>>  };
>>  
>> +enum lwmi_feature_id_psu {
>> +	LWMI_FEATURE_ID_PSU_INSTANT_MODE = 0x01,
>> +	LWMI_FEATURE_ID_PSU_CHARGE_MODE = 0x02,
>
>I see no code referencing LWMI_FEATURE_ID_PSU_CHARGE_MODE. What's the
>mode for?

It's another option exposed by some BIOS that is supposed to have similar functionality. None of my devices support it properly so I removed it but forgot to clean up this define.

>> +};
>> +
>>  #define LWMI_FEATURE_ID_FAN_RPM 0x03
>>  
>>  #define LWMI_TYPE_ID_NONE 0x00
>>  #define LWMI_TYPE_ID_CROSSLOAD 0x01
>> +#define LWMI_TYPE_ID_PSU_AC 0x01
>> +#define LWMI_TYPE_ID_PSU_PD 0x02
>>  
>>  #define LWMI_FEATURE_VALUE_GET 17
>>  #define LWMI_FEATURE_VALUE_SET 18
>> @@ -92,10 +102,17 @@ enum lwmi_feature_id_gpu {
>>  
>>  #define LWMI_FAN_DIV 100
>>  
>> +#define LWMI_CHARGE_MODE_ENABLED	0x00
>> +#define LWMI_CHARGE_MODE_DISABLED	0x01
>> +
>>  #define LWMI_ATTR_ID_FAN_RPM(x)                                   \
>>  	LWMI_ATTR_ID(LWMI_DEVICE_ID_FAN, LWMI_FEATURE_ID_FAN_RPM, \
>>  		     LWMI_GZ_THERMAL_MODE_NONE, LWMI_FAN_ID(x))
>>  
>> +#define LWMI_ATTR_ID_PSU(feat, type)		\
>> +	LWMI_ATTR_ID(LWMI_DEVICE_ID_PSU, feat,		\
>> +		     LWMI_GZ_THERMAL_MODE_NONE, type)
>> +
>
>Unaligned backslashes.
>
>>  #define LWMI_OM_SYSFS_NAME "lenovo-wmi-other"
>>  #define LWMI_OM_HWMON_NAME "lenovo_wmi_other"
>>  
>> @@ -137,6 +154,8 @@ struct lwmi_om_priv {
>>  		bool capdata00_collected : 1;
>>  		bool capdata_fan_collected : 1;
>>  	} fan_flags;
>> +
>> +	struct acpi_battery_hook battery_hook;
>>  };
>>  
>>  /*
>> @@ -561,6 +580,216 @@ static void lwmi_om_fan_info_collect_cd_fan(struct device *dev, struct cd_list *
>>  	lwmi_om_hwmon_add(priv);
>>  }
>>  
>> +/* ======== Power Supply Extension (component: lenovo-wmi-capdata 00) ======== */
>> +
>> +/**
>> + * lwmi_psy_ext_get_prop() - Get a power_supply_ext property
>> + * @ps: The battery that was extended
>> + * @ext: The extension
>> + * @ext_data: Pointer the lwmi_om_priv drvdata
>> + * @prop: The property to read
>> + * @val: The value to return
>> + *
>> + * Writes the given value to the power_supply_ext property
>> + *
>> + * Return: 0 on success, or an error
>> + */
>> +static int lwmi_psy_ext_get_prop(struct power_supply *ps,
>> +				 const struct power_supply_ext *ext,
>> +				 void *ext_data,
>> +				 enum power_supply_property prop,
>> +				 union power_supply_propval *val)
>> +{
>> +	struct lwmi_om_priv *priv = ext_data;
>> +	struct wmi_method_args_32 args;
>> +	u32 retval;
>> +	int ret;
>> +
>> +	args.arg0 = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
>> +
>> +	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_GET,
>> +				    (unsigned char *)&args, sizeof(args),
>> +				    &retval);
>> +	if (ret)
>> +		return ret;
>> +
>> +	dev_dbg(&priv->wdev->dev, "Got return value %x for property %x\n", retval, prop);
>> +
>> +	if (retval == LWMI_CHARGE_MODE_DISABLED)
>> +		val->intval = POWER_SUPPLY_CHARGE_TYPE_LONGLIFE;
>> +	else
>> +		val->intval = POWER_SUPPLY_CHARGE_TYPE_STANDARD;
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * lwmi_psy_ext_set_prop() - Set a power_supply_ext property
>> + * @ps: The battery that was extended
>> + * @ext: The extension
>> + * @ext_data: Pointer the lwmi_om_priv drvdata
>> + * @prop: The property to write
>> + * @val: The value to write
>> + *
>> + * Writes the given value to the power_supply_ext property
>> + *
>> + * Return: 0 on success, or an error
>> + */
>> +static int lwmi_psy_ext_set_prop(struct power_supply *ps,
>> +				 const struct power_supply_ext *ext,
>> +				 void *ext_data,
>> +				 enum power_supply_property prop,
>> +				 const union power_supply_propval *val)
>> +{
>> +	struct lwmi_om_priv *priv = ext_data;
>> +	struct wmi_method_args_32 args;
>> +
>> +	args.arg0 = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
>> +	if (val->intval == POWER_SUPPLY_CHARGE_TYPE_LONGLIFE)
>> +		args.arg1 = LWMI_CHARGE_MODE_DISABLED;
>> +	else
>> +		args.arg1 = LWMI_CHARGE_MODE_ENABLED;
>> +
>> +	dev_dbg(&priv->wdev->dev, "Attempting to set %#08x for property %x to %x\n",
>> +		args.arg0, prop, args.arg1);
>> +
>> +	return lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_SET,
>> +				     (unsigned char *)&args, sizeof(args), NULL);
>> +}
>> +
>> +/**
>> + * lwmi_psy_prop_is_writeable() - Determine if the property is supported
>> + * @ps: The battery that was extended
>> + * @ext: The extension
>> + * @ext_data: Pointer the lwmi_om_priv drvdata
>> + * @prop: The property to check
>> + *
>> + * Checks capdata 00 to determine if the property is supported.
>> + *
>> + * Return: Support level, or false
>> + */
>> +static int lwmi_psy_prop_is_writeable(struct power_supply *ps,
>> +				      const struct power_supply_ext *ext,
>> +				      void *ext_data,
>> +				      enum power_supply_property prop)
>> +{
>> +	struct lwmi_om_priv *priv = ext_data;
>> +	struct capdata00 capdata;
>> +	u32 attribute_id = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
>> +	int ret;
>> +
>> +	ret = lwmi_cd00_get_data(priv->cd00_list, attribute_id, &capdata);
>> +	if (ret)
>> +		return false;
>> +
>> +	dev_dbg(&priv->wdev->dev, "Battery charge mode (%#08x) support level: %x\n",
>> +		attribute_id, capdata.supported);
>> +
>> +	return capdata.supported;
>> +}
>> +
>> +static const enum power_supply_property lwmi_psy_ext_props[] = {
>> +	POWER_SUPPLY_PROP_CHARGE_TYPES,
>> +};
>> +
>> +static const struct power_supply_ext lwmi_psy_ext = {
>> +	.name			= LWMI_OM_SYSFS_NAME,
>> +	.properties		= lwmi_psy_ext_props,
>> +	.num_properties		= ARRAY_SIZE(lwmi_psy_ext_props),
>> +	.charge_types		= (BIT(POWER_SUPPLY_CHARGE_TYPE_STANDARD) |
>> +				   BIT(POWER_SUPPLY_CHARGE_TYPE_LONGLIFE)),
>> +	.get_property		= lwmi_psy_ext_get_prop,
>> +	.set_property		= lwmi_psy_ext_set_prop,
>> +	.property_is_writeable	= lwmi_psy_prop_is_writeable,
>> +};
>> +
>> +/**
>> + * lwmi_add_battery() - Connect the power_supply_ext
>> + * @battery: The battery to extend
>> + * @hook: The driver hook used to extend the battery
>> + *
>> + * Return: 0 on success, or an error.
>> + */
>> +static int lwmi_add_battery(struct power_supply *battery, struct acpi_battery_hook *hook)
>> +{
>> +	struct lwmi_om_priv *priv = container_of(hook, struct lwmi_om_priv, battery_hook);
>> +
>> +	return power_supply_register_extension(battery, &lwmi_psy_ext, &priv->wdev->dev, priv);
>> +}
>> +
>> +/**
>> + * lwmi_remove_battery() - Disconnect the power_supply_ext
>> + * @battery: The battery that was extended
>> + * @hook: The driver hook used to extend the battery
>> + *
>> + * Return: 0 on success, or an error.
>> + */
>> +static int lwmi_remove_battery(struct power_supply *battery, struct acpi_battery_hook *hook)
>> +{
>> +	power_supply_unregister_extension(battery, &lwmi_psy_ext);
>> +	return 0;
>> +}
>> +
>> +/**
>> + * lwmi_acpi_match() - Attempts to return the ideapad acpi handle
>> + * @handle: The ACPI handle that manages battery charging
>> + * @lvl: Unused
>> + * @context: Void pointer to the acpi_handle object to return
>> + * @retval: Unused
>> + *
>> + * Checks if the ideapad_laptop driver is going to manage charge_type first,
>> + * then if not, hooks the battery to our WMI methods.
>> + *
>> + * Return: AE_CTRL_TERMINATE if found, AE_OK if not found.
>> + */
>> +static acpi_status lwmi_acpi_match(acpi_handle handle, u32 lvl,
>> +				   void *context, void **retval)
>> +{
>> +	if (!handle)
>> +		return AE_OK;
>> +
>> +	acpi_handle *ahand = context;
>> +	*ahand = handle;
>> +
>> +	return AE_CTRL_TERMINATE;
>> +}
>> +
>> +/**
>> + * lwmi_om_ps_ext_init() - Hooks power supply extension to device battery
>> + * @priv: Driver private data
>> + *
>> + * Checks if the ideapad_laptop driver is going to manage charge_type first,
>> + * then if not, hooks the battery to our WMI methods.
>> + */
>> +static void lwmi_om_ps_ext_init(struct lwmi_om_priv *priv)
>> +{
>> +	static const char * const ideapad_hid = "VPC2004";
>> +	acpi_handle handle = NULL;
>> +	int ret;
>> +
>> +	/* Deconflict ideapad_laptop driver */
>> +	ret = acpi_get_devices(ideapad_hid, lwmi_acpi_match, &handle, NULL);
>> +	if (ret)
>> +		return;
>> +
>> +	if (!handle)
>> +		return;
>
>I am a bit confused here. If VPC2004 is not found, handle should remain
>NULL. In this case we have no conflict, right?
>
>Thanks,
>Rong
>
>> +
>> +	if (acpi_has_method(handle, "GBMD") && acpi_has_method(handle, "SBMC")) {
>> +		dev_dbg(&priv->wdev->dev, "ideapad_laptop driver manages battery for device.\n");
>> +		return;
>> +	}
>> +
>> +	/* Add battery hooks */
>> +	priv->battery_hook.add_battery	= lwmi_add_battery,
>> +	priv->battery_hook.remove_battery	= lwmi_remove_battery,
>> +	priv->battery_hook.name		= "Lenovo WMI Other Battery Extension",
>> +
>> +	ret = devm_battery_hook_register(&priv->wdev->dev, &priv->battery_hook);
>> +	if (ret)
>> +		dev_err(&priv->wdev->dev, "Error during battery hook: %i\n", ret);
>> +}
>> +
>>  /* ======== fw_attributes (component: lenovo-wmi-capdata 01) ======== */
>>  
>>  struct tunable_attr_01 {
>> @@ -1325,6 +1554,7 @@ static int lwmi_om_master_bind(struct device *dev)
>>  		return -ENODEV;
>>  
>>  	lwmi_om_fan_info_collect_cd00(priv);
>> +	lwmi_om_ps_ext_init(priv);
>>  
>>  	return lwmi_om_fw_attr_add(priv);
>>  }

Re: [PATCH v3 0/6] platform-x86: lenovo-wmi: Add fixes and enhancement

[Derek J. Clark]

On February 25, 2026 10:02:21 AM PST, Rong Zhang <i@rong.moe> wrote:
>Hi Derek,
>
>Sorry for the late reply. I was busier (yet happier ;-)) than I had
>expected during the Spring Festival/Chinese New Year.

Not a problem, glad you enjoyed your time off.

>On Tue, 2026-02-24 at 04:31 +0000, Derek J. Clark wrote:
>> This series adds many much needed features and fixes to the lenovo-wmi
>> drivers.
>> 
>> Patch 1 adds a macro to make assigning attribute ID's for capdata
>> cleaner and easier.
>> Patch 2 addresses bugs where devices that don't support exposed
>> attributes would still create the attribute, and also attempts to
>> identify the correct capdata and set/get methods as some legacy
>> interfaces don't use the custom mode in the method or capdata ID.
>> Patch 3 adds the remaining CPU attributes that weren't previously
>> exposed.
>> Patch 4 adds the GPU attributes that weren't previously exposed.
>> Patch 5 consolidates name constants in preparation for patch 6.
>> Patch 6 adds battery charge-type limiting when supported only by WMI.
>> 
>> After applying Rong's series from
>> https://lore.kernel.org/platform-driver-x86/20260210191938.137234-1-i@rong.moe/
>> I found that there weren't any conflicts, so rather than combine
>> the series we can just review each separately.
>> 
>> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
>
>The patch series makes sense to me in general except for some issues
>mentioned in my previous replies.
>
>With style issues fixed, you can add
>
>Reviewed-By: Rong Zhang <i@rong.moe>
>
>to patch 1,3,4,5.
>
>Thanks,
>Rong

Thanks, will do after we agree on some of the details.

Cheers,
Derek
>> ---
>> v3:
>>   - Re-add HWMON name const and jsut rename LWMI_OM_FW_ATTR_BASE_PATH
>>   - Fix linker warnings by moving acpi/battery include to the end of the
>>     list.
>>   - Remove CPU/GPU OC features. These attributes are BOOL type and will
>>     need a new constructor that I'll add later.
>> v2:
>>   - Fix gpu_mode misisng from attributes list.
>>   - Fix prototypes for power suppy patch.
>>   - Reorganize CPU and GPU attributes alphabetically.
>>   - Break out the patch consolidating the driver name cost.
>>   - Move some of the refactoring of attribute_id back to into patch 1
>>     where it belongs.
>>   - Fix some additional typos in function prototypes.
>> v1: https://lore.kernel.org/platform-driver-x86/20260213081243.794288-1-derekjohn.clark@gmail.com/
>> 
>> Derek J. Clark (6):
>>   platform/x86: lenovo-wmi-other: Add LWMI_ATTR_ID Macro
>>   platform/x86: lenovo-wmi-other: Limit adding attributes to supported
>>     devices
>>   platform/x86: lenovo-wmi-other: Add missing CPU tunable attributes
>>   platform/x86: lenovo-wmi-other: Add GPU tunable attributes
>>   platform-x86: lenovo-wmi-other: Rename LWMI_OM_FW_ATTR_BASE_PATH
>>   platform/x86: lenovo-wmi-other: Add WMI battery charge limiting.
>> 
>>  .../wmi/devices/lenovo-wmi-other.rst          |  21 +
>>  drivers/platform/x86/lenovo/wmi-capdata.h     |  13 +-
>>  drivers/platform/x86/lenovo/wmi-gamezone.h    |   1 +
>>  drivers/platform/x86/lenovo/wmi-other.c       | 605 ++++++++++++++++--
>>  4 files changed, 593 insertions(+), 47 deletions(-)

Re: [PATCH v3 2/6] platform/x86: lenovo-wmi-other: Limit adding attributes to supported devices

[Rong Zhang]

Hi Derek,

On Wed, 2026-02-25 at 10:09 -0800, Derek J. Clark wrote:
> On February 25, 2026 9:14:12 AM PST, Rong Zhang <i@rong.moe> wrote:
> > Hi Derek,
> > 
> > On Tue, 2026-02-24 at 04:31 +0000, Derek J. Clark wrote:
> > > Adds lwmi_is_attr_01_supported, and only creates the attribute subfolder
> > > if the attribute is supported by the hardware. Due to some poorly
> > > implemented BIOS, this is a multi-step sequence of events. This is
> > > because:
> > > - Some BIOS support getting the capability data from custom mode (0xff),
> > >   while others only support it in no-mode (0x00).
> > > - Similarly, some BIOS support get/set for the current value from custom
> > >   mode (0xff), while others only support it in no-mode (0x00).
> > > - Some BIOS report capability data for a method that is not fully
> > >   implemented.
> > > - Some BIOS have methods fully implemented, but no complimentary
> > >   capability data.
> > > 
> > > To ensure we only expose fully implemented methods with corresponding
> > > capability data, we check each outcome before reporting that an
> > > attribute can be supported.
> > > 
> > > Checking for lwmi_is_attr_01_supported during remove is not done to
> > > ensure that we don't attempt to call cd01 or send WMI events if one of
> > > the interfaces being removed was the cause of the driver unloading.
> > > 
> > > While adding members to tunable_attr_01, remove unused capdata pointer
> > > and limit size of all ID's to the appropriate size.
> > > 
> > > Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> > > Reported-by: Kurt Borja <kuurtb@gmail.com>
> > > Closes: https://lore.kernel.org/platform-driver-x86/DG60P3SHXR8H.3NSEHMZ6J7XRC@gmail.com/
> > > Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
> > > ---
> > >  drivers/platform/x86/lenovo/wmi-other.c | 117 +++++++++++++++++++++---
> > >  1 file changed, 102 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> > > index 95886df39c8d..f3f12303e379 100644
> > > --- a/drivers/platform/x86/lenovo/wmi-other.c
> > > +++ b/drivers/platform/x86/lenovo/wmi-other.c
> > > @@ -545,11 +545,12 @@ static void lwmi_om_fan_info_collect_cd_fan(struct device *dev, struct cd_list *
> > >  /* ======== fw_attributes (component: lenovo-wmi-capdata 01) ======== */
> > >  
> > >  struct tunable_attr_01 {
> > > -	struct capdata01 *capdata;
> > >  	struct device *dev;
> > > -	u32 feature_id;
> > > -	u32 device_id;
> > > -	u32 type_id;
> > > +	u8 feature_id;
> > > +	u8 device_id;
> > > +	u8 type_id;
> > > +	u8 cd_mode_id; /* mode arg for searching capdata */
> > > +	u8 cv_mode_id; /* mode arg for set/get current_value */
> > >  };
> > >  
> > >  static struct tunable_attr_01 ppt_pl1_spl = {
> > > @@ -716,7 +717,7 @@ static ssize_t attr_capdata01_show(struct kobject *kobj,
> > >  	int value, ret;
> > >  
> > >  	attribute_id = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> > > -				    LWMI_GZ_THERMAL_MODE_CUSTOM, tunable_attr->type_id);
> > > +				    tunable_attr->cd_mode_id, tunable_attr->type_id);
> > >  
> > >  	ret = lwmi_cd01_get_data(priv->cd01_list, attribute_id, &capdata);
> > >  	if (ret)
> > > @@ -782,7 +783,7 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
> > >  		return -EBUSY;
> > >  
> > >  	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> > > -				 mode, tunable_attr->type_id);
> > > +				 tunable_attr->cd_mode_id, tunable_attr->type_id);
> > >  
> > >  	ret = lwmi_cd01_get_data(priv->cd01_list, args.arg0, &capdata);
> > >  	if (ret)
> > > @@ -795,6 +796,8 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
> > >  	if (value < capdata.min_value || value > capdata.max_value)
> > >  		return -EINVAL;
> > >  
> > > +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> > > +				 tunable_attr->cv_mode_id, tunable_attr->type_id);
> > >  	args.arg1 = value;
> > >  
> > >  	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_SET,
> > > @@ -828,13 +831,16 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
> > >  	struct lwmi_om_priv *priv = dev_get_drvdata(tunable_attr->dev);
> > >  	struct wmi_method_args_32 args;
> > >  	enum thermal_mode mode;
> > > -	int retval;
> > > -	int ret;
> > > +	int retval, ret;
> > >  
> > >  	ret = lwmi_om_notifier_call(&mode);
> > >  	if (ret)
> > >  		return ret;
> > >  
> > > +	/* If "no-mode" is the supported mode, ensure we never send current mode */
> > > +	if (tunable_attr->cv_mode_id == LWMI_GZ_THERMAL_MODE_NONE)
> > > +		mode = tunable_attr->cv_mode_id;
> > > +
> > >  	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> > >  				 mode, tunable_attr->type_id);
> > >  
> > > @@ -847,6 +853,85 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
> > >  	return sysfs_emit(buf, "%d\n", retval);
> > >  }
> > >  
> > > +/**
> > > + * lwmi_attr_01_is_supported() - Determine if the given attribute is supported.
> > > + * @tunable_attr: The attribute to verify.
> > > + *
> > > + * First check if the attribute has a corresponding capdata01 table in the cd01
> > > + * module under the "custom" mode (0xff). If that is not present then check if
> > > + * there is a corresponding "no-mode" (0x00) entry. If either of those passes,
> > > + * check capdata->supported for values > 0. If capdata is available, attempt to
> > > + * determine the set/get mode for the current value property using a similar
> > > + * pattern. If the value returned by either custom or no-mode is 0, or we get
> > > + * an error, we assume that mode is not supported. If any of the above checks
> > > + * fail then the attribute is not fully supported.
> > > + *
> > > + * The probed cd_mode_id/cv_mode_id are stored on the tunable_attr for later
> > > + * reference.
> > > + *
> > > + * Return: Support level, or an error code.
> > > + */
> > > +static int lwmi_attr_01_is_supported(struct tunable_attr_01 *tunable_attr)
> > > +{
> > > +	struct lwmi_om_priv *priv = dev_get_drvdata(tunable_attr->dev);
> > > +	u8 mode = LWMI_GZ_THERMAL_MODE_CUSTOM;
> > > +	struct wmi_method_args_32 args;
> > > +	struct capdata01 capdata;
> > > +	int retval, ret;
> > > +
> > > +	/* Determine tunable_attr->cd_mode_id */
> > > +no_mode_fallback_1:
> > > +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> > > +				 mode, tunable_attr->type_id);
> > > +
> > > +	ret = lwmi_cd01_get_data(priv->cd01_list, args.arg0, &capdata);
> > > +	if (ret && mode) {
> > > +		dev_dbg(tunable_attr->dev, "Attribute id %x not supported\n", args.arg0);
> > > +		mode = LWMI_GZ_THERMAL_MODE_NONE;
> > > +		goto no_mode_fallback_1;
> > > +	}
> > > +	if (ret)
> > > +		goto not_supported;
> > > +	if (!capdata.supported) {
> > > +		ret = -EOPNOTSUPP;
> > > +		goto not_supported;
> > > +	}
> > > +
> > > +	tunable_attr->cd_mode_id = mode;
> > > +
> > > +	/* Determine tunable_attr->cv_mode_id */
> > > +	mode = LWMI_GZ_THERMAL_MODE_CUSTOM;
> > > +no_mode_fallback_2:
> > > +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> > > +				 mode, tunable_attr->type_id);
> > > +
> > > +	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_GET,
> > > +				    (unsigned char *)&args, sizeof(args),
> > > +				    &retval);
> > > +	if ((ret && mode) || (!retval && mode)) {
> > > +		dev_dbg(tunable_attr->dev, "Attribute id %x not supported\n", args.arg0);
> > > +		mode = LWMI_GZ_THERMAL_MODE_NONE;
> > > +		goto no_mode_fallback_2;
> > > +	}
> > > +	if (ret)
> > > +		goto not_supported;
> > > +	if (retval == 0) {
> > > +		ret = -EOPNOTSUPP;
> > > +		goto not_supported;
> > > +	}
> > > +
> > 
> > I agree with Ilpo that backward gotos are hard to read and should be
> > avoided if possible.
> > 
> > > +	tunable_attr->cv_mode_id = mode;
> > > +	dev_dbg(tunable_attr->dev, "cd_mode_id: %02x%02x%02x%02x, cv_mode_id: %#08x attribute support level: %x\n",
> > 
> > It's a long line. Wrap at the first comma?
> > 
> > > +		tunable_attr->device_id, tunable_attr->feature_id, tunable_attr->cd_mode_id,
> > > +		tunable_attr->type_id, args.arg0, capdata.supported);
> > > +
> > > +	return capdata.supported;
> > > +
> > > +not_supported:
> > > +	dev_dbg(tunable_attr->dev, "Attribute id %x not supported\n", args.arg0);
> > > +	return ret;
> > > +}
> > > +
> > >  /* Lenovo WMI Other Mode Attribute macros */
> > >  #define __LWMI_ATTR_RO(_func, _name)                                  \
> > >  	{                                                             \
> > > @@ -970,19 +1055,21 @@ static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
> > >  	}
> > >  
> > >  	for (i = 0; i < ARRAY_SIZE(cd01_attr_groups) - 1; i++) {
> > > -		err = sysfs_create_group(&priv->fw_attr_kset->kobj,
> > > -					 cd01_attr_groups[i].attr_group);
> > > -		if (err)
> > > -			goto err_remove_groups;
> > > -
> > >  		cd01_attr_groups[i].tunable_attr->dev = &priv->wdev->dev;
> > > +		if (lwmi_attr_01_is_supported(cd01_attr_groups[i].tunable_attr) > 0) {
> > > +			err = sysfs_create_group(&priv->fw_attr_kset->kobj,
> > > +						 cd01_attr_groups[i].attr_group);
> > > +			if (err)
> > > +				goto err_remove_groups;
> > > +		}
> > >  	}
> > >  	return 0;
> > >  
> > >  err_remove_groups:
> > >  	while (i--)
> > > -		sysfs_remove_group(&priv->fw_attr_kset->kobj,
> > > -				   cd01_attr_groups[i].attr_group);
> > > +		if (lwmi_attr_01_is_supported(cd01_attr_groups[i].tunable_attr) > 0)
> > 
> > I don't see lwmi_om_fw_attr_remove() doing the same. If we care about
> > symmetric cleanup, I would imagine a bitmap or so recording what have
> > been created is used there.
> > 
> > Also it's doubtful if the firmware always return the same result on the
> > same parameters. If a broken firmware somehow returns different results
> > here, some attributes may be left uncleaned.
> > 
> > That being said, I am not sure if we really need symmetric cleanup.
> > IIUC, sysfs_remove_group() is a no-op if we pass an attr_group that has
> > not been created.
> 
> Hi Rong,
> We don't want to query the interface if it was removed, and the no-op is harmless. It doesn't seem worth it to me to retain that in memory to achieve the same results.

Makes sense. Then I would suggest drop the lwmi_attr_01_is_supported()
check here so that we don't need to worry about broken firmware.

Thanks,
Rong

> > > +			sysfs_remove_group(&priv->fw_attr_kset->kobj,
> > > +					   cd01_attr_groups[i].attr_group);
> > >  
> > >  	kset_unregister(priv->fw_attr_kset);
> > >  
> > 
> > Thanks,
> > Rong

Re: [PATCH v3 6/6] platform/x86: lenovo-wmi-other: Add WMI battery charge limiting.

[Rong Zhang]

Hi Derek,

On Wed, 2026-02-25 at 10:23 -0800, Derek J. Clark wrote:
> On February 25, 2026 9:55:40 AM PST, Rong Zhang <i@rong.moe> wrote:
> > Hi Derek,
> > 
> > There is an extra dot (period) in the title. Please remove it.
> > 
> > On Tue, 2026-02-24 at 04:32 +0000, Derek J. Clark wrote:
> > > Add charge-type power supply extension for devices that support WMI based
> > > charge enable/disable. Lenovo Legion devices that implement function ID
> > > and capdata 00 ID 0x03010001 are able to enable or disable charging
> > > through the lenovo-wmi-other interface.
> > 
> > "disable charging"...
> > 
> > I'd expect it pauses charging immediately and unconditionally instead
> > of setting a threshold, especially considering the its name
> > "INSTANT_MODE". If this is the case, I don't think it fits the
> > definition of POWER_SUPPLY_CHARGE_TYPE_LONGLIFE. It'd better to use
> > POWER_SUPPLY_CHARGE_TYPE_NONE.
> 
> It really just sets a charge threshold of 80% and lowers the charge speed a bit. I'll come up with a better name than the one Lenovo uses in their docs.

Makes sense. Thanks for clarification.

FYI, ideapad-laptop conservative mode (i.e.,
POWER_SUPPLY_CHARGE_TYPE_LONGLIFE) doesn't lower the charge speed. It's
just a trivial divergence and isn't a blocker of using
POWER_SUPPLY_CHARGE_TYPE_LONGLIFE here of course.

Thanks,
Rong

> > > The ideapad_laptop driver
> > > conflicts with this if it can also provide the attribute, so we have to
> > > get the acpi_handle and check for the same ACPI methods that enable the
> > > feature in that driver. The ACPI method is more reliable from my testing
> > > when both are present, so there is no need to modify the ideapad_laptop
> > > driver instead.
> > 
> > When ideapad-laptop is blacklisted, there should be no conflict. I
> > would suggest adding a module parameter to force wmi-other to register
> > psy_ext anyway so that users can have their own choice.
> > 
> 
> That should be fine. I've found the WMI interface is a little buggy on devices that support the ACPI method. I.e. on the Go S and Go 2 it always reports the "Enabled" status, but changing it shows the ACPI method reporting the correct value. (I used a different attr name to test this)
> 
> > > Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> > > Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
> > > ---
> > >  drivers/platform/x86/lenovo/wmi-capdata.h |   1 +
> > >  drivers/platform/x86/lenovo/wmi-other.c   | 230 ++++++++++++++++++++++
> > >  2 files changed, 231 insertions(+)
> > > 
> > > diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
> > > index b7f9ee7b301a..00471551e7d6 100644
> > > --- a/drivers/platform/x86/lenovo/wmi-capdata.h
> > > +++ b/drivers/platform/x86/lenovo/wmi-capdata.h
> > > @@ -26,6 +26,7 @@
> > >  enum lwmi_device_id {
> > >  	LWMI_DEVICE_ID_CPU = 0x01,
> > >  	LWMI_DEVICE_ID_GPU = 0x02,
> > > +	LWMI_DEVICE_ID_PSU = 0x03,
> > >  	LWMI_DEVICE_ID_FAN = 0x04,
> > >  };
> > >  
> > > diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> > > index 7f0d5a17b44f..b2daff1b45c2 100644
> > > --- a/drivers/platform/x86/lenovo/wmi-other.c
> > > +++ b/drivers/platform/x86/lenovo/wmi-other.c
> > > @@ -42,9 +42,12 @@
> > >  #include <linux/module.h>
> > >  #include <linux/notifier.h>
> > >  #include <linux/platform_profile.h>
> > > +#include <linux/power_supply.h>
> > >  #include <linux/types.h>
> > >  #include <linux/wmi.h>
> > >  
> > > +#include <acpi/battery.h>
> > > +
> > >  #include "wmi-capdata.h"
> > >  #include "wmi-events.h"
> > >  #include "wmi-gamezone.h"
> > > @@ -78,10 +81,17 @@ enum lwmi_feature_id_gpu {
> > >  	LWMI_FEATURE_ID_GPU_NV_CPU_BOOST = 0x0b,
> > >  };
> > >  
> > > +enum lwmi_feature_id_psu {
> > > +	LWMI_FEATURE_ID_PSU_INSTANT_MODE = 0x01,
> > > +	LWMI_FEATURE_ID_PSU_CHARGE_MODE = 0x02,
> > 
> > I see no code referencing LWMI_FEATURE_ID_PSU_CHARGE_MODE. What's the
> > mode for?
> 
> It's another option exposed by some BIOS that is supposed to have similar functionality. None of my devices support it properly so I removed it but forgot to clean up this define.
> 
> > > +};
> > > +
> > >  #define LWMI_FEATURE_ID_FAN_RPM 0x03
> > >  
> > >  #define LWMI_TYPE_ID_NONE 0x00
> > >  #define LWMI_TYPE_ID_CROSSLOAD 0x01
> > > +#define LWMI_TYPE_ID_PSU_AC 0x01
> > > +#define LWMI_TYPE_ID_PSU_PD 0x02
> > >  
> > >  #define LWMI_FEATURE_VALUE_GET 17
> > >  #define LWMI_FEATURE_VALUE_SET 18
> > > @@ -92,10 +102,17 @@ enum lwmi_feature_id_gpu {
> > >  
> > >  #define LWMI_FAN_DIV 100
> > >  
> > > +#define LWMI_CHARGE_MODE_ENABLED	0x00
> > > +#define LWMI_CHARGE_MODE_DISABLED	0x01
> > > +
> > >  #define LWMI_ATTR_ID_FAN_RPM(x)                                   \
> > >  	LWMI_ATTR_ID(LWMI_DEVICE_ID_FAN, LWMI_FEATURE_ID_FAN_RPM, \
> > >  		     LWMI_GZ_THERMAL_MODE_NONE, LWMI_FAN_ID(x))
> > >  
> > > +#define LWMI_ATTR_ID_PSU(feat, type)		\
> > > +	LWMI_ATTR_ID(LWMI_DEVICE_ID_PSU, feat,		\
> > > +		     LWMI_GZ_THERMAL_MODE_NONE, type)
> > > +
> > 
> > Unaligned backslashes.
> > 
> > >  #define LWMI_OM_SYSFS_NAME "lenovo-wmi-other"
> > >  #define LWMI_OM_HWMON_NAME "lenovo_wmi_other"
> > >  
> > > @@ -137,6 +154,8 @@ struct lwmi_om_priv {
> > >  		bool capdata00_collected : 1;
> > >  		bool capdata_fan_collected : 1;
> > >  	} fan_flags;
> > > +
> > > +	struct acpi_battery_hook battery_hook;
> > >  };
> > >  
> > >  /*
> > > @@ -561,6 +580,216 @@ static void lwmi_om_fan_info_collect_cd_fan(struct device *dev, struct cd_list *
> > >  	lwmi_om_hwmon_add(priv);
> > >  }
> > >  
> > > +/* ======== Power Supply Extension (component: lenovo-wmi-capdata 00) ======== */
> > > +
> > > +/**
> > > + * lwmi_psy_ext_get_prop() - Get a power_supply_ext property
> > > + * @ps: The battery that was extended
> > > + * @ext: The extension
> > > + * @ext_data: Pointer the lwmi_om_priv drvdata
> > > + * @prop: The property to read
> > > + * @val: The value to return
> > > + *
> > > + * Writes the given value to the power_supply_ext property
> > > + *
> > > + * Return: 0 on success, or an error
> > > + */
> > > +static int lwmi_psy_ext_get_prop(struct power_supply *ps,
> > > +				 const struct power_supply_ext *ext,
> > > +				 void *ext_data,
> > > +				 enum power_supply_property prop,
> > > +				 union power_supply_propval *val)
> > > +{
> > > +	struct lwmi_om_priv *priv = ext_data;
> > > +	struct wmi_method_args_32 args;
> > > +	u32 retval;
> > > +	int ret;
> > > +
> > > +	args.arg0 = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
> > > +
> > > +	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_GET,
> > > +				    (unsigned char *)&args, sizeof(args),
> > > +				    &retval);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	dev_dbg(&priv->wdev->dev, "Got return value %x for property %x\n", retval, prop);
> > > +
> > > +	if (retval == LWMI_CHARGE_MODE_DISABLED)
> > > +		val->intval = POWER_SUPPLY_CHARGE_TYPE_LONGLIFE;
> > > +	else
> > > +		val->intval = POWER_SUPPLY_CHARGE_TYPE_STANDARD;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +/**
> > > + * lwmi_psy_ext_set_prop() - Set a power_supply_ext property
> > > + * @ps: The battery that was extended
> > > + * @ext: The extension
> > > + * @ext_data: Pointer the lwmi_om_priv drvdata
> > > + * @prop: The property to write
> > > + * @val: The value to write
> > > + *
> > > + * Writes the given value to the power_supply_ext property
> > > + *
> > > + * Return: 0 on success, or an error
> > > + */
> > > +static int lwmi_psy_ext_set_prop(struct power_supply *ps,
> > > +				 const struct power_supply_ext *ext,
> > > +				 void *ext_data,
> > > +				 enum power_supply_property prop,
> > > +				 const union power_supply_propval *val)
> > > +{
> > > +	struct lwmi_om_priv *priv = ext_data;
> > > +	struct wmi_method_args_32 args;
> > > +
> > > +	args.arg0 = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
> > > +	if (val->intval == POWER_SUPPLY_CHARGE_TYPE_LONGLIFE)
> > > +		args.arg1 = LWMI_CHARGE_MODE_DISABLED;
> > > +	else
> > > +		args.arg1 = LWMI_CHARGE_MODE_ENABLED;
> > > +
> > > +	dev_dbg(&priv->wdev->dev, "Attempting to set %#08x for property %x to %x\n",
> > > +		args.arg0, prop, args.arg1);
> > > +
> > > +	return lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_SET,
> > > +				     (unsigned char *)&args, sizeof(args), NULL);
> > > +}
> > > +
> > > +/**
> > > + * lwmi_psy_prop_is_writeable() - Determine if the property is supported
> > > + * @ps: The battery that was extended
> > > + * @ext: The extension
> > > + * @ext_data: Pointer the lwmi_om_priv drvdata
> > > + * @prop: The property to check
> > > + *
> > > + * Checks capdata 00 to determine if the property is supported.
> > > + *
> > > + * Return: Support level, or false
> > > + */
> > > +static int lwmi_psy_prop_is_writeable(struct power_supply *ps,
> > > +				      const struct power_supply_ext *ext,
> > > +				      void *ext_data,
> > > +				      enum power_supply_property prop)
> > > +{
> > > +	struct lwmi_om_priv *priv = ext_data;
> > > +	struct capdata00 capdata;
> > > +	u32 attribute_id = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
> > > +	int ret;
> > > +
> > > +	ret = lwmi_cd00_get_data(priv->cd00_list, attribute_id, &capdata);
> > > +	if (ret)
> > > +		return false;
> > > +
> > > +	dev_dbg(&priv->wdev->dev, "Battery charge mode (%#08x) support level: %x\n",
> > > +		attribute_id, capdata.supported);
> > > +
> > > +	return capdata.supported;
> > > +}
> > > +
> > > +static const enum power_supply_property lwmi_psy_ext_props[] = {
> > > +	POWER_SUPPLY_PROP_CHARGE_TYPES,
> > > +};
> > > +
> > > +static const struct power_supply_ext lwmi_psy_ext = {
> > > +	.name			= LWMI_OM_SYSFS_NAME,
> > > +	.properties		= lwmi_psy_ext_props,
> > > +	.num_properties		= ARRAY_SIZE(lwmi_psy_ext_props),
> > > +	.charge_types		= (BIT(POWER_SUPPLY_CHARGE_TYPE_STANDARD) |
> > > +				   BIT(POWER_SUPPLY_CHARGE_TYPE_LONGLIFE)),
> > > +	.get_property		= lwmi_psy_ext_get_prop,
> > > +	.set_property		= lwmi_psy_ext_set_prop,
> > > +	.property_is_writeable	= lwmi_psy_prop_is_writeable,
> > > +};
> > > +
> > > +/**
> > > + * lwmi_add_battery() - Connect the power_supply_ext
> > > + * @battery: The battery to extend
> > > + * @hook: The driver hook used to extend the battery
> > > + *
> > > + * Return: 0 on success, or an error.
> > > + */
> > > +static int lwmi_add_battery(struct power_supply *battery, struct acpi_battery_hook *hook)
> > > +{
> > > +	struct lwmi_om_priv *priv = container_of(hook, struct lwmi_om_priv, battery_hook);
> > > +
> > > +	return power_supply_register_extension(battery, &lwmi_psy_ext, &priv->wdev->dev, priv);
> > > +}
> > > +
> > > +/**
> > > + * lwmi_remove_battery() - Disconnect the power_supply_ext
> > > + * @battery: The battery that was extended
> > > + * @hook: The driver hook used to extend the battery
> > > + *
> > > + * Return: 0 on success, or an error.
> > > + */
> > > +static int lwmi_remove_battery(struct power_supply *battery, struct acpi_battery_hook *hook)
> > > +{
> > > +	power_supply_unregister_extension(battery, &lwmi_psy_ext);
> > > +	return 0;
> > > +}
> > > +
> > > +/**
> > > + * lwmi_acpi_match() - Attempts to return the ideapad acpi handle
> > > + * @handle: The ACPI handle that manages battery charging
> > > + * @lvl: Unused
> > > + * @context: Void pointer to the acpi_handle object to return
> > > + * @retval: Unused
> > > + *
> > > + * Checks if the ideapad_laptop driver is going to manage charge_type first,
> > > + * then if not, hooks the battery to our WMI methods.
> > > + *
> > > + * Return: AE_CTRL_TERMINATE if found, AE_OK if not found.
> > > + */
> > > +static acpi_status lwmi_acpi_match(acpi_handle handle, u32 lvl,
> > > +				   void *context, void **retval)
> > > +{
> > > +	if (!handle)
> > > +		return AE_OK;
> > > +
> > > +	acpi_handle *ahand = context;
> > > +	*ahand = handle;
> > > +
> > > +	return AE_CTRL_TERMINATE;
> > > +}
> > > +
> > > +/**
> > > + * lwmi_om_ps_ext_init() - Hooks power supply extension to device battery
> > > + * @priv: Driver private data
> > > + *
> > > + * Checks if the ideapad_laptop driver is going to manage charge_type first,
> > > + * then if not, hooks the battery to our WMI methods.
> > > + */
> > > +static void lwmi_om_ps_ext_init(struct lwmi_om_priv *priv)
> > > +{
> > > +	static const char * const ideapad_hid = "VPC2004";
> > > +	acpi_handle handle = NULL;
> > > +	int ret;
> > > +
> > > +	/* Deconflict ideapad_laptop driver */
> > > +	ret = acpi_get_devices(ideapad_hid, lwmi_acpi_match, &handle, NULL);
> > > +	if (ret)
> > > +		return;
> > > +
> > > +	if (!handle)
> > > +		return;
> > 
> > I am a bit confused here. If VPC2004 is not found, handle should remain
> > NULL. In this case we have no conflict, right?
> > 
> > Thanks,
> > Rong
> > 
> > > +
> > > +	if (acpi_has_method(handle, "GBMD") && acpi_has_method(handle, "SBMC")) {
> > > +		dev_dbg(&priv->wdev->dev, "ideapad_laptop driver manages battery for device.\n");
> > > +		return;
> > > +	}
> > > +
> > > +	/* Add battery hooks */
> > > +	priv->battery_hook.add_battery	= lwmi_add_battery,
> > > +	priv->battery_hook.remove_battery	= lwmi_remove_battery,
> > > +	priv->battery_hook.name		= "Lenovo WMI Other Battery Extension",
> > > +
> > > +	ret = devm_battery_hook_register(&priv->wdev->dev, &priv->battery_hook);
> > > +	if (ret)
> > > +		dev_err(&priv->wdev->dev, "Error during battery hook: %i\n", ret);
> > > +}
> > > +
> > >  /* ======== fw_attributes (component: lenovo-wmi-capdata 01) ======== */
> > >  
> > >  struct tunable_attr_01 {
> > > @@ -1325,6 +1554,7 @@ static int lwmi_om_master_bind(struct device *dev)
> > >  		return -ENODEV;
> > >  
> > >  	lwmi_om_fan_info_collect_cd00(priv);
> > > +	lwmi_om_ps_ext_init(priv);
> > >  
> > >  	return lwmi_om_fw_attr_add(priv);
> > >  }

Re: [PATCH v3 1/6] platform/x86: lenovo-wmi-other: Add LWMI_ATTR_ID Macro

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1802 bytes --]

On Wed, 25 Feb 2026, Derek J. Clark wrote:

> On February 24, 2026 12:42:08 AM PST, "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com> wrote:
> >On Tue, 24 Feb 2026, Derek J. Clark wrote:
> >
> >> Adds LWMI_ATTR_ID macro. In the same vein as LWMI_ATTR_ID_FAN_RPM,
> >> but as a generic, to de-duplicate attribute_id assignment biolerplate.
> >> 
> >> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> >> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
> >> ---
> >>  drivers/platform/x86/lenovo/wmi-capdata.h  |  6 ++++
> >>  drivers/platform/x86/lenovo/wmi-gamezone.h |  1 +
> >>  drivers/platform/x86/lenovo/wmi-other.c    | 36 ++++++----------------
> >>  3 files changed, 17 insertions(+), 26 deletions(-)
> >> 
> >> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
> >> index 8c1df3efcc55..27202e2dc8a5 100644
> >> --- a/drivers/platform/x86/lenovo/wmi-capdata.h
> >> +++ b/drivers/platform/x86/lenovo/wmi-capdata.h
> >> @@ -17,6 +17,12 @@
> >>  #define LWMI_ATTR_MODE_ID_MASK	GENMASK(15, 8)
> >>  #define LWMI_ATTR_TYPE_ID_MASK	GENMASK(7, 0)
> >>  
> >> +#define LWMI_ATTR_ID(dev, feat, mode, type) \
> >> +	(FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, dev) | \
> >> +	 FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, feat) | \
> >> +	 FIELD_PREP(LWMI_ATTR_MODE_ID_MASK, mode) | \
> >> +	 FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, type))
> >
> >Why does this need to be macro? Static inline would have proper types.
> >
> 
> Hi Ilpo,
> 
> It doesn't, I was just keeping it consistent with other previous uses. 
> It's not a problem to change it. I'll need to move it somewhere else 
> since this is a header.

Did you mean to some other file? (As you can have static inline 
functions just fine in a header.)

-- 
 i.

Re: [PATCH v3 2/6] platform/x86: lenovo-wmi-other: Limit adding attributes to supported devices

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 10247 bytes --]

On Wed, 25 Feb 2026, Derek J. Clark wrote:

> On February 24, 2026 12:47:56 AM PST, "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com> wrote:
> >On Tue, 24 Feb 2026, Derek J. Clark wrote:
> >
> >> Adds lwmi_is_attr_01_supported, and only creates the attribute subfolder
> >> if the attribute is supported by the hardware. Due to some poorly
> >> implemented BIOS, this is a multi-step sequence of events. This is
> >> because:
> >> - Some BIOS support getting the capability data from custom mode (0xff),
> >>   while others only support it in no-mode (0x00).
> >> - Similarly, some BIOS support get/set for the current value from custom
> >>   mode (0xff), while others only support it in no-mode (0x00).
> >> - Some BIOS report capability data for a method that is not fully
> >>   implemented.
> >> - Some BIOS have methods fully implemented, but no complimentary
> >>   capability data.
> >> 
> >> To ensure we only expose fully implemented methods with corresponding
> >> capability data, we check each outcome before reporting that an
> >> attribute can be supported.
> >> 
> >> Checking for lwmi_is_attr_01_supported during remove is not done to
> >> ensure that we don't attempt to call cd01 or send WMI events if one of
> >> the interfaces being removed was the cause of the driver unloading.
> >> 
> >
> >> While adding members to tunable_attr_01, remove unused capdata pointer
> >> and limit size of all ID's to the appropriate size.
> >
> >Please don't mix changes like this. Create a seprate patch for it.
> >
> >> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> >> Reported-by: Kurt Borja <kuurtb@gmail.com>
> >> Closes: https://lore.kernel.org/platform-driver-x86/DG60P3SHXR8H.3NSEHMZ6J7XRC@gmail.com/
> >> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
> >> ---
> >>  drivers/platform/x86/lenovo/wmi-other.c | 117 +++++++++++++++++++++---
> >>  1 file changed, 102 insertions(+), 15 deletions(-)
> >> 
> >> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> >> index 95886df39c8d..f3f12303e379 100644
> >> --- a/drivers/platform/x86/lenovo/wmi-other.c
> >> +++ b/drivers/platform/x86/lenovo/wmi-other.c
> >> @@ -545,11 +545,12 @@ static void lwmi_om_fan_info_collect_cd_fan(struct device *dev, struct cd_list *
> >>  /* ======== fw_attributes (component: lenovo-wmi-capdata 01) ======== */
> >>  
> >>  struct tunable_attr_01 {
> >> -	struct capdata01 *capdata;
> >>  	struct device *dev;
> >> -	u32 feature_id;
> >> -	u32 device_id;
> >> -	u32 type_id;
> >> +	u8 feature_id;
> >> +	u8 device_id;
> >> +	u8 type_id;
> >> +	u8 cd_mode_id; /* mode arg for searching capdata */
> >> +	u8 cv_mode_id; /* mode arg for set/get current_value */
> >>  };
> >>  
> >>  static struct tunable_attr_01 ppt_pl1_spl = {
> >> @@ -716,7 +717,7 @@ static ssize_t attr_capdata01_show(struct kobject *kobj,
> >>  	int value, ret;
> >>  
> >>  	attribute_id = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> >> -				    LWMI_GZ_THERMAL_MODE_CUSTOM, tunable_attr->type_id);
> >> +				    tunable_attr->cd_mode_id, tunable_attr->type_id);
> >>  
> >>  	ret = lwmi_cd01_get_data(priv->cd01_list, attribute_id, &capdata);
> >>  	if (ret)
> >> @@ -782,7 +783,7 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
> >>  		return -EBUSY;
> >>  
> >>  	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> >> -				 mode, tunable_attr->type_id);
> >> +				 tunable_attr->cd_mode_id, tunable_attr->type_id);
> >>  
> >>  	ret = lwmi_cd01_get_data(priv->cd01_list, args.arg0, &capdata);
> >>  	if (ret)
> >> @@ -795,6 +796,8 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
> >>  	if (value < capdata.min_value || value > capdata.max_value)
> >>  		return -EINVAL;
> >>  
> >> +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> >> +				 tunable_attr->cv_mode_id, tunable_attr->type_id);
> >>  	args.arg1 = value;
> >>  
> >>  	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_SET,
> >> @@ -828,13 +831,16 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
> >>  	struct lwmi_om_priv *priv = dev_get_drvdata(tunable_attr->dev);
> >>  	struct wmi_method_args_32 args;
> >>  	enum thermal_mode mode;
> >> -	int retval;
> >> -	int ret;
> >> +	int retval, ret;
> >>  
> >>  	ret = lwmi_om_notifier_call(&mode);
> >>  	if (ret)
> >>  		return ret;
> >>  
> >> +	/* If "no-mode" is the supported mode, ensure we never send current mode */
> >> +	if (tunable_attr->cv_mode_id == LWMI_GZ_THERMAL_MODE_NONE)
> >> +		mode = tunable_attr->cv_mode_id;
> >> +
> >>  	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> >>  				 mode, tunable_attr->type_id);
> >>  
> >> @@ -847,6 +853,85 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
> >>  	return sysfs_emit(buf, "%d\n", retval);
> >>  }
> >>  
> >> +/**
> >> + * lwmi_attr_01_is_supported() - Determine if the given attribute is supported.
> >> + * @tunable_attr: The attribute to verify.
> >> + *
> >> + * First check if the attribute has a corresponding capdata01 table in the cd01
> >> + * module under the "custom" mode (0xff). If that is not present then check if
> >> + * there is a corresponding "no-mode" (0x00) entry. If either of those passes,
> >> + * check capdata->supported for values > 0. If capdata is available, attempt to
> >> + * determine the set/get mode for the current value property using a similar
> >> + * pattern. If the value returned by either custom or no-mode is 0, or we get
> >> + * an error, we assume that mode is not supported. If any of the above checks
> >> + * fail then the attribute is not fully supported.
> >> + *
> >> + * The probed cd_mode_id/cv_mode_id are stored on the tunable_attr for later
> >> + * reference.
> >> + *
> >> + * Return: Support level, or an error code.
> >> + */
> >> +static int lwmi_attr_01_is_supported(struct tunable_attr_01 *tunable_attr)
> >> +{
> >> +	struct lwmi_om_priv *priv = dev_get_drvdata(tunable_attr->dev);
> >> +	u8 mode = LWMI_GZ_THERMAL_MODE_CUSTOM;
> >> +	struct wmi_method_args_32 args;
> >> +	struct capdata01 capdata;
> >> +	int retval, ret;
> >> +
> >> +	/* Determine tunable_attr->cd_mode_id */
> >> +no_mode_fallback_1:
> >> +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> >> +				 mode, tunable_attr->type_id);
> >> +
> >> +	ret = lwmi_cd01_get_data(priv->cd01_list, args.arg0, &capdata);
> >> +	if (ret && mode) {
> >> +		dev_dbg(tunable_attr->dev, "Attribute id %x not supported\n", args.arg0);
> >
> >Add include.
> >
> >> +		mode = LWMI_GZ_THERMAL_MODE_NONE;
> >> +		goto no_mode_fallback_1;
> >
> >Is it possible to make a helper so you don't need these back gotos?
> >
> 
> Sure. How about I put both modes into an array and loop through them, 
> passing the values to a function that will evaluate it. I can break if 
> there is a match and throw an error if there's no match at the end.

It would probably work okay with a loop (which you've basically done now 
in a custom way using that goto) and array.

> >> +	}
> >> +	if (ret)
> >> +		goto not_supported;
> >> +	if (!capdata.supported) {
> >> +		ret = -EOPNOTSUPP;
> >> +		goto not_supported;
> >> +	}
> >> +
> >> +	tunable_attr->cd_mode_id = mode;
> >> +
> >> +	/* Determine tunable_attr->cv_mode_id */
> >> +	mode = LWMI_GZ_THERMAL_MODE_CUSTOM;
> >> +no_mode_fallback_2:
> >> +	args.arg0 = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
> >> +				 mode, tunable_attr->type_id);
> >> +
> >> +	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_GET,
> >> +				    (unsigned char *)&args, sizeof(args),
> >> +				    &retval);
> >> +	if ((ret && mode) || (!retval && mode)) {
> >> +		dev_dbg(tunable_attr->dev, "Attribute id %x not supported\n", args.arg0);
> >> +		mode = LWMI_GZ_THERMAL_MODE_NONE;
> >> +		goto no_mode_fallback_2;
> >
> >Same question here?
> >
> >> +	}
> >> +	if (ret)
> >> +		goto not_supported;
> >> +	if (retval == 0) {
> >> +		ret = -EOPNOTSUPP;
> >> +		goto not_supported;
> >> +	}
> >> +
> >> +	tunable_attr->cv_mode_id = mode;
> >> +	dev_dbg(tunable_attr->dev, "cd_mode_id: %02x%02x%02x%02x, cv_mode_id: %#08x attribute support level: %x\n",
> >> +		tunable_attr->device_id, tunable_attr->feature_id, tunable_attr->cd_mode_id,
> >> +		tunable_attr->type_id, args.arg0, capdata.supported);
> >> +
> >> +	return capdata.supported;
> >> +
> >> +not_supported:
> >> +	dev_dbg(tunable_attr->dev, "Attribute id %x not supported\n", args.arg0);
> >> +	return ret;
> >> +}
> >> +
> >>  /* Lenovo WMI Other Mode Attribute macros */
> >>  #define __LWMI_ATTR_RO(_func, _name)                                  \
> >>  	{                                                             \
> >> @@ -970,19 +1055,21 @@ static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
> >>  	}
> >>  
> >>  	for (i = 0; i < ARRAY_SIZE(cd01_attr_groups) - 1; i++) {
> >> -		err = sysfs_create_group(&priv->fw_attr_kset->kobj,
> >> -					 cd01_attr_groups[i].attr_group);
> >> -		if (err)
> >> -			goto err_remove_groups;
> >> -
> >>  		cd01_attr_groups[i].tunable_attr->dev = &priv->wdev->dev;
> >> +		if (lwmi_attr_01_is_supported(cd01_attr_groups[i].tunable_attr) > 0) {
> >
> >Reverse logic and use continue.
> >
> >> +			err = sysfs_create_group(&priv->fw_attr_kset->kobj,
> >> +						 cd01_attr_groups[i].attr_group);
> >> +			if (err)
> >> +				goto err_remove_groups;
> >> +		}
> >>  	}
> >>  	return 0;
> >>  
> >>  err_remove_groups:
> >>  	while (i--)
> >> -		sysfs_remove_group(&priv->fw_attr_kset->kobj,
> >> -				   cd01_attr_groups[i].attr_group);
> >> +		if (lwmi_attr_01_is_supported(cd01_attr_groups[i].tunable_attr) > 0)
> >
> >Reverse logic + continue.
> >
> >> +			sysfs_remove_group(&priv->fw_attr_kset->kobj,
> >> +					   cd01_attr_groups[i].attr_group);
> >
> >You need to add braces for multiline constructs.
> >
> >>  
> >>  	kset_unregister(priv->fw_attr_kset);
> >>  
> >> 
> >
> 

-- 
 i.