[PATCH v2 00/12] platform/x86: lenovo-wmi-{other,capdata,helpers}: Improve robustness on faulty firmware

[PATCH v2 00/12] platform/x86: lenovo-wmi-{other,capdata,helpers}: Improve robustness on faulty firmware

[Rong Zhang]

Some devices do not support LENOVO_CAPABILITY_DATA_01 and define the
query method as a stub that returns zero buffer. Unfortunately, some
devices do not implement the stub properly, causing WMI errors
(including ACPI errors). This was reported by Charles.

The current lenovo-wmi-* implementation enforces the binding between
LENOVO_CAPABILITY_DATA_00+01 and LENOVO_OTHER_MODE because of a
limitation of the device component framework. When the capdata device
bailing out due to a WMI error, lenovo-wmi-other becomes unbound and
unable to provide firmware-attributes or hwmon/power_supply_ext devices
for the other functional capdata device.

Therefore, errors must be non-fatal in order not to break the
assumptions made by the device component framework.

Poison the capdata device by releasing the capability data list in this
case. After that, NULL list will be passed to lenovo-wmi-other on bind.
The latter will provide whatever is available, or unbind the components
if nothing is available.

A poisoned capdata device releases or skips allocating most resources,
e.g., the capability data list and the debugfs directory. The device
itself is only used to satisfy the component dependency of lenovo-wmi-
other and coordinate with the latter about the absence of the capability
data.

Meanwhile, for devices that properly stubs the WMI query method (but
still declares >0 instances), keeping the capability data list with
empty data is meaningless and causes lenovo-wmi-other to call
lwmi_cd*_get_data() to retrieve nonexistent capdata in vain. These
capdata devices are poisoned as well to save resources.

In order to release or skip allocating most resources for poisoned
devices, some preparatory work is done in prior. With the preparatory
work, it also skips allocating most resources for the WMI devices that
declare 0 instance.

Also identify missing components using the new wmidev_exists() interface
(introduced at the very beginning of the series), and skip adding them
to the match list, so that all components in the list must present,
fulfilling the binding requirement. Some devices need this because they
either do not have the WMI GUID of LENOVO_CAPABILITY_DATA_01, or do not
implement the query method, causing the WMI core not to create the
corresponding WMI device. This was reported by Navon.

The new WMI API is also adopted to conform to the behavior of the
Windows WMI-ACPI driver and improve robustness on various WMI ACPI
method implementation.

Finally, add myself as a LENOVO drivers maintainer as previously
suggested by Derek.

Reported-by: Charles <hanker007@gmail.com>
Closes: https://msgid.link/CAKtz0s8UYRQYW_0bh=0TMx47Axm-W-muEay-r3rqUBS1NHMPVw@mail.gmail.com/
Reported-by: Navon John Lukose <navonjohnlukose@gmail.com>
Closes: https://msgid.link/20260928190901.1369497-1-navonjohnlukose@gmail.com
Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Link: https://msgid.link/9789f452-d7eb-4f1e-8a13-7335332193a7@app.fastmail.com
Suggested-by: Derek J. Clark <derekjohn.clark@gmail.com>
Link: https://msgid.link/782FE636-A06A-4E12-9563-786374805947@gmail.com
Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v2:
- Add PATCH 1 ("platform/wmi: Introduce wmidev_exists()") to the series
  as discussed at https://msgid.link/83776200-4A60-4756-A99F-D5A3BB4D834A@rong.moe
- Add PATCH 2 ("lenovo-wmi-capdata: Do not stop the AC notifier chain on
  error") to the series, as adopting the new WMI API will intentionally
  catch more faulty firmware and propagate more errors
- Synchronize mutex initialization with release-acquire barriers (thanks
  Ilpo Järvinen)
- Refine line wrap (ditto)
- Replace the term "poison" with "stub" (ditto)
- Add PATCH 10 ("platform/x86: lenovo-wmi-capdata: Do not match missing
  components") to the series to solve the report made by Navon
- Update outdated comments, function documentations and commit messages
- Link to v1: https://patch.msgid.link/20260914-lwmi-wmi-new-api-v1-0-7a400f2f69f8@rong.moe

---
Armin Wolf (1):
      platform/wmi: Introduce wmidev_exists()

Rong Zhang (11):
      platform/x86: lenovo-wmi-capdata: Do not stop the AC notifier chain on error
      platform/x86: lenovo-wmi-capdata: Only allocate sub-master info when necessary
      platform/x86: lenovo-wmi-capdata: Store a pointer to component info
      platform/x86: lenovo-wmi-capdata: Defer mutex initialization
      platform/x86: lenovo-wmi-{capdata,other}: Only allocate capdata list when necessary
      platform/x86: lenovo-wmi-capdata: Adopt new WMI API
      platform/x86: lenovo-wmi-capdata: Register component even on WMI error
      platform/x86: lenovo-wmi-capdata: Detect stubbed capdata device
      platform/x86: lenovo-wmi-capdata: Do not match missing components
      platform/x86: lenovo-wmi-helpers: Adopt new WMI API
      MAINTAINERS: Add myself as a LENOVO drivers co-maintainer

 MAINTAINERS                               |   1 +
 drivers/platform/wmi/core.c               |  33 ++-
 drivers/platform/x86/lenovo/wmi-capdata.c | 450 ++++++++++++++++++++++--------
 drivers/platform/x86/lenovo/wmi-helpers.c |  61 ++--
 drivers/platform/x86/lenovo/wmi-other.c   |  24 +-
 include/linux/wmi.h                       |   3 +
 6 files changed, 400 insertions(+), 172 deletions(-)
---
base-commit: af32da41b0327b9c6a37856ba82b6760d6c8d10e
change-id: a504a929-lwmi-wmi-new-api-f5344d48a86a

Thanks,
Rong

[PATCH v2 01/12] platform/wmi: Introduce wmidev_exists()

[Rong Zhang]

From: Armin Wolf <W_Armin@gmx.de>

Some WMI drivers need to be able to detect the existence of WMI
devices matching a given GUID in order to determine the availability
of optional features. Using wmi_has_guid() for this will not work for
WMI devices that cannot be used with the legacy GUID-based API (aka
have WMI_GUID_DUPLICATED set).

Introduce wmidev_exists() that also checks WMI devices that are only
available to the bus-based API. Also mark wmi_has_guid() as deprecated
because only WMI drivers using the legacy GUID-based API will continue
to have use for it.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v2:
- New patch in the series as discussed at
  https://msgid.link/83776200-4A60-4756-A99F-D5A3BB4D834A@rong.moe
---
 drivers/platform/wmi/core.c | 33 +++++++++++++++++++++++++++++----
 include/linux/wmi.h         |  3 +++
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/wmi/core.c b/drivers/platform/wmi/core.c
index 529825dcfbfe..a99da9b7039d 100644
--- a/drivers/platform/wmi/core.c
+++ b/drivers/platform/wmi/core.c
@@ -145,7 +145,7 @@ static inline void get_acpi_method_name(const struct wmi_block *wblock,
 	buffer[4] = '\0';
 }
 
-static int wmidev_match_guid(struct device *dev, const void *data)
+static int wmidev_match_guid_legacy(struct device *dev, const void *data)
 {
 	struct wmi_block *wblock = dev_to_wblock(dev);
 	const guid_t *guid = data;
@@ -218,7 +218,7 @@ static struct wmi_device *wmi_find_device_by_guid(const char *guid_string)
 	if (ret < 0)
 		return ERR_PTR(ret);
 
-	dev = bus_find_device(&wmi_bus_type, NULL, &guid, wmidev_match_guid);
+	dev = bus_find_device(&wmi_bus_type, NULL, &guid, wmidev_match_guid_legacy);
 	if (!dev)
 		return ERR_PTR(-ENODEV);
 
@@ -800,10 +800,10 @@ acpi_status wmi_remove_notify_handler(const char *guid)
 EXPORT_SYMBOL_GPL(wmi_remove_notify_handler);
 
 /**
- * wmi_has_guid - Check if a GUID is available
+ * wmi_has_guid - Check if a GUID is available (deprecated)
  * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
  *
- * Check if a given GUID is defined by _WDG.
+ * Check if a given GUID is available and usable with the legacy GUID-based API.
  *
  * Return: True if GUID is available, false otherwise.
  */
@@ -821,6 +821,31 @@ bool wmi_has_guid(const char *guid_string)
 }
 EXPORT_SYMBOL_GPL(wmi_has_guid);
 
+static int wmidev_match_guid(struct device *dev, void *data)
+{
+	struct wmi_block *wblock = dev_to_wblock(dev);
+	const guid_t *guid = data;
+
+	if (guid_equal(guid, &wblock->gblock.guid))
+		return 1;
+
+	return 0;
+}
+
+/**
+ * wmidev_exists - Check if a WMI device with a given GUID exists
+ * @guid: GUID of the WMI device to search for
+ *
+ * Check if a WMI device with a given GUID exists at the given moment.
+ *
+ * Return: True if a matching WMI device exists, false otherwise.
+ */
+bool wmidev_exists(const guid_t *guid)
+{
+	return bus_for_each_dev(&wmi_bus_type, NULL, (void *)guid, wmidev_match_guid) == 1;
+}
+EXPORT_SYMBOL_GPL(wmidev_exists);
+
 /**
  * wmi_get_acpi_device_uid() - Get _UID name of ACPI device that defines GUID (deprecated)
  * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
diff --git a/include/linux/wmi.h b/include/linux/wmi.h
index defcb624a7e2..3dd223f2ff18 100644
--- a/include/linux/wmi.h
+++ b/include/linux/wmi.h
@@ -13,6 +13,7 @@
 #include <linux/acpi.h>
 #include <linux/device-id/wmi.h>
 #include <linux/types.h>
+#include <linux/uuid.h>
 
 /**
  * struct wmi_device - WMI device structure
@@ -83,6 +84,8 @@ acpi_status wmidev_block_set(struct wmi_device *wdev, u8 instance, const struct
 
 u8 wmidev_instance_count(struct wmi_device *wdev);
 
+bool wmidev_exists(const guid_t *guid);
+
 /**
  * struct wmi_driver - WMI driver structure
  * @driver: Driver model structure

-- 
2.55.0

[PATCH v2 02/12] platform/x86: lenovo-wmi-capdata: Do not stop the AC notifier chain on error

[Rong Zhang]

Returning NOTIFY_BAD to the AC notifier chain stops it by preventing
later notifier blocks from being called, but our error is non-fatal for
other AC notifier blocks, which should continue to consume the event.

Return NOTIFY_DONE and add a dev_warn() on error, so that the AC
notifier chain continues. This also aligns with hp-wmi.

The function documentation was copied from elsewhere and makes no sense
here, so correct it as well.

Fixes: e1a5fe662b59 ("platform/x86: Add Lenovo Capability Data 01 WMI Driver")
Cc: stable@vger.kernel.org
Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v2:
- New patch in the series
---
 drivers/platform/x86/lenovo/wmi-capdata.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c b/drivers/platform/x86/lenovo/wmi-capdata.c
index d5e961566136..58787da959b4 100644
--- a/drivers/platform/x86/lenovo/wmi-capdata.c
+++ b/drivers/platform/x86/lenovo/wmi-capdata.c
@@ -774,13 +774,11 @@ static int lwmi_cd_setup(struct lwmi_cd_priv *priv, enum lwmi_cd_type type)
 
 /**
  * lwmi_cd01_notifier_call() - Call method for cd01 notifier.
- * block call chain.
- * @nb: The notifier_block registered to lenovo-wmi-events driver.
+ * @nb: The cd01 ACPI notifier_block.
  * @action: Unused.
  * @data: The ACPI event.
  *
- * For LWMI_EVENT_THERMAL_MODE, set current_mode and notify platform_profile
- * of a change.
+ * Refresh cached capdata on AC status change.
  *
  * Return: notifier_block status.
  */
@@ -799,8 +797,16 @@ static int lwmi_cd01_notifier_call(struct notifier_block *nb, unsigned long acti
 	switch (event->type) {
 	case ACPI_AC_NOTIFY_STATUS:
 		ret = lwmi_cd_cache(priv);
-		if (ret)
-			return NOTIFY_BAD;
+		if (ret) {
+			dev_warn(&priv->wdev->dev,
+				 "failed to refresh capdata on AC change: %d\n", ret);
+
+			/*
+			 * Our error is non-fatal for others, and we still want
+			 * the other event consumers to process it.
+			 */
+			return NOTIFY_DONE;
+		}
 
 		return NOTIFY_OK;
 	default:

-- 
2.55.0

[PATCH v2 03/12] platform/x86: lenovo-wmi-capdata: Only allocate sub-master info when necessary

[Rong Zhang]

When capdata00 declares no fan test support, there is no need to
allocate sub-master info as it's basically unused except for the
priv->sub_master->sub_component_list == ERR_PTR(-ENODEV) placeholder.

Therefore, only allocate priv->sub_master when necessary, otherwise its
absence implies the absence of sub-component. With that,
PTR_ERR(sub_component_list) == -ENODEV is repurposed for the following
changes to represent stubbed sub-component.

Signed-off-by: Rong Zhang <i@rong.moe>
---
 drivers/platform/x86/lenovo/wmi-capdata.c | 36 ++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c b/drivers/platform/x86/lenovo/wmi-capdata.c
index 58787da959b4..3a823aee608a 100644
--- a/drivers/platform/x86/lenovo/wmi-capdata.c
+++ b/drivers/platform/x86/lenovo/wmi-capdata.c
@@ -101,9 +101,11 @@ struct lwmi_cd_priv {
 	struct lwmi_cd_sub_master_priv {
 		struct device *master_dev;
 		cd_list_cb_t master_cb;
-		struct cd_list *sub_component_list; /* ERR_PTR(-ENODEV) implies no sub-component. */
+		struct cd_list *sub_component_list; /* ERR_PTR(-ENODEV) implies stubbed
+						     * sub-component.
+						     */
 		bool registered;                    /* Has the sub-master been registered? */
-	} *sub_master;
+	} *sub_master;                              /* NULL implies no sub-component. */
 };
 
 struct cd_list {
@@ -258,6 +260,12 @@ static int lwmi_cd_component_bind(struct device *cd_dev,
 	case LENOVO_CAPABILITY_DATA_00:
 		binder->cd00_list = priv->list;
 
+		/* Missing sub-master info implies missing sub-component. */
+		if (!priv->sub_master) {
+			binder->cd_fan_list_cb(om_dev, NULL);
+			break;
+		}
+
 		priv->sub_master->master_dev = om_dev;
 		priv->sub_master->master_cb = binder->cd_fan_list_cb;
 		lwmi_cd_call_master_cb(priv);
@@ -288,13 +296,9 @@ static void lwmi_cd_component_unbind(struct device *cd_dev,
 {
 	struct lwmi_cd_priv *priv = dev_get_drvdata(cd_dev);
 
-	switch (priv->list->type) {
-	case LENOVO_CAPABILITY_DATA_00:
+	if (priv->sub_master) {
 		priv->sub_master->master_dev = NULL;
 		priv->sub_master->master_cb = NULL;
-		return;
-	default:
-		return;
 	}
 }
 
@@ -369,17 +373,19 @@ static int lwmi_cd_sub_master_add(struct lwmi_cd_priv *priv,
 	struct component_match *master_match = NULL;
 	int ret;
 
+	/*
+	 * If the sub-master has no sub-component, there is no need to allocate
+	 * or register the sub-master. The master callback will be called with
+	 * NULL on master <-> sub-master bind to inform the master about the
+	 * absence of sub-component.
+	 */
+	if (sub_component_type == CD_TYPE_NONE)
+		return 0;
+
 	priv->sub_master = devm_kzalloc(&priv->wdev->dev, sizeof(*priv->sub_master), GFP_KERNEL);
 	if (!priv->sub_master)
 		return -ENOMEM;
 
-	if (sub_component_type == CD_TYPE_NONE) {
-		/* The master callback will be called with NULL on bind. */
-		priv->sub_master->sub_component_list = ERR_PTR(-ENODEV);
-		priv->sub_master->registered = false;
-		return 0;
-	}
-
 	/*
 	 * lwmi_cd_match() needs a pointer to enum lwmi_cd_type, but on-stack
 	 * data cannot be used here. Steal one from lwmi_cd_table.
@@ -404,7 +410,7 @@ static int lwmi_cd_sub_master_add(struct lwmi_cd_priv *priv,
  */
 static void lwmi_cd_sub_master_del(struct lwmi_cd_priv *priv)
 {
-	if (!priv->sub_master->registered)
+	if (!priv->sub_master || !priv->sub_master->registered)
 		return;
 
 	component_master_del(&priv->wdev->dev, &lwmi_cd_sub_master_ops);

-- 
2.55.0

[PATCH v2 04/12] platform/x86: lenovo-wmi-capdata: Store a pointer to component info

[Rong Zhang]

In the following changes, priv->list will become optional (i.e.,
NULLable). IOW, the type of the component cannot be reliable retrieved
from priv->list->type.

Store a pointer to component info so that the component type is always
available. In this manner, priv->list->type can be eliminated. Note that
the component type check in lwmi_cd*_get_data() must be removed due to
it being called without the availability of priv. Removing it should be
fine as the capdata APIs are really internal methods and their caller,
i.e., lenovo-wmi-other, always passes a matched list.

This also eliminates the need to pass the component type along setup
functions.

Signed-off-by: Rong Zhang <i@rong.moe>
---
 drivers/platform/x86/lenovo/wmi-capdata.c | 44 +++++++++++++------------------
 1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c b/drivers/platform/x86/lenovo/wmi-capdata.c
index 3a823aee608a..3008d6d699c0 100644
--- a/drivers/platform/x86/lenovo/wmi-capdata.c
+++ b/drivers/platform/x86/lenovo/wmi-capdata.c
@@ -87,6 +87,7 @@ static const struct lwmi_cd_info {
 
 struct lwmi_cd_priv {
 	struct notifier_block acpi_nb; /* ACPI events */
+	const struct lwmi_cd_info *info;
 	struct wmi_device *wdev;
 	struct cd_list *list;
 	struct dentry *debugfs_dir;
@@ -110,7 +111,6 @@ struct lwmi_cd_priv {
 
 struct cd_list {
 	struct mutex list_mutex; /* list R/W mutex */
-	enum lwmi_cd_type type;
 	u8 count;
 
 	union {
@@ -139,7 +139,7 @@ static int lwmi_cd_match(struct device *dev, void *type)
 		return false;
 
 	priv = dev_get_drvdata(dev);
-	return priv->list->type == *(enum lwmi_cd_type *)type;
+	return priv->info->type == *(enum lwmi_cd_type *)type;
 }
 
 /**
@@ -256,7 +256,7 @@ static int lwmi_cd_component_bind(struct device *cd_dev,
 	struct lwmi_cd_priv *priv = dev_get_drvdata(cd_dev);
 	struct lwmi_cd_binder *binder = data;
 
-	switch (priv->list->type) {
+	switch (priv->info->type) {
 	case LENOVO_CAPABILITY_DATA_00:
 		binder->cd00_list = priv->list;
 
@@ -454,14 +454,11 @@ static const struct component_ops lwmi_cd_sub_component_ops = {
  *
  * Return: 0 on success, or -EINVAL.
  */
-#define DEF_LWMI_CDXX_GET_DATA(_cdxx, _cd_type, _output_t)					\
+#define DEF_LWMI_CDXX_GET_DATA(_cdxx, _output_t)						\
 	int lwmi_##_cdxx##_get_data(struct cd_list *list, u32 attribute_id, _output_t *output)	\
 	{											\
 		u8 idx;										\
 												\
-		if (WARN_ON(list->type != _cd_type))						\
-			return -EINVAL;								\
-												\
 		guard(mutex)(&list->list_mutex);						\
 		for (idx = 0; idx < list->count; idx++) {					\
 			if (list->_cdxx[idx].id != attribute_id)				\
@@ -472,13 +469,13 @@ static const struct component_ops lwmi_cd_sub_component_ops = {
 		return -EINVAL;									\
 	}
 
-DEF_LWMI_CDXX_GET_DATA(cd00, LENOVO_CAPABILITY_DATA_00, struct capdata00);
+DEF_LWMI_CDXX_GET_DATA(cd00, struct capdata00);
 EXPORT_SYMBOL_NS_GPL(lwmi_cd00_get_data, "LENOVO_WMI_CAPDATA");
 
-DEF_LWMI_CDXX_GET_DATA(cd01, LENOVO_CAPABILITY_DATA_01, struct capdata01);
+DEF_LWMI_CDXX_GET_DATA(cd01, struct capdata01);
 EXPORT_SYMBOL_NS_GPL(lwmi_cd01_get_data, "LENOVO_WMI_CAPDATA");
 
-DEF_LWMI_CDXX_GET_DATA(cd_fan, LENOVO_FAN_TEST_DATA, struct capdata_fan);
+DEF_LWMI_CDXX_GET_DATA(cd_fan, struct capdata_fan);
 EXPORT_SYMBOL_NS_GPL(lwmi_cd_fan_get_data, "LENOVO_WMI_CAPDATA");
 
 /* ======== debugfs ======== */
@@ -552,15 +549,14 @@ static int lwmi_cd_debugfs_show(struct seq_file *s, void *data)
 
 	guard(mutex)(&priv->list->list_mutex);
 
-	/* lwmi_cd_alloc() ensured priv->list->type must be a valid type. */
 	for (idx = 0; idx < priv->list->count; idx++) {
-		seq_printf(s, "%s[%u]:\n", lwmi_cd_table[priv->list->type].name, idx);
+		seq_printf(s, "%s[%u]:\n", priv->info->name, idx);
 
-		if (priv->list->type == LENOVO_CAPABILITY_DATA_00)
+		if (priv->info->type == LENOVO_CAPABILITY_DATA_00)
 			lwmi_cd00_show(s, &priv->list->cd00[idx]);
-		else if (priv->list->type == LENOVO_CAPABILITY_DATA_01)
+		else if (priv->info->type == LENOVO_CAPABILITY_DATA_01)
 			lwmi_cd01_show(s, &priv->list->cd01[idx]);
-		else if (priv->list->type == LENOVO_FAN_TEST_DATA)
+		else if (priv->info->type == LENOVO_FAN_TEST_DATA)
 			lwmi_cd_fan_show(s, &priv->list->cd_fan[idx]);
 	}
 
@@ -605,7 +601,7 @@ static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
 	int idx;
 	void *p;
 
-	switch (priv->list->type) {
+	switch (priv->info->type) {
 	case LENOVO_CAPABILITY_DATA_00:
 		p = &priv->list->cd00[0];
 		size = sizeof(priv->list->cd00[0]);
@@ -708,14 +704,13 @@ static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv, struct cd_lis
 /**
  * lwmi_cd_alloc() - Allocate a cd_list struct in drvdata
  * @priv: lenovo-wmi-capdata driver data.
- * @type: The type of capability data.
  *
  * Allocate a cd_list struct large enough to contain data from all WMI data
  * blocks provided by the interface.
  *
  * Return: 0 on success, or an error.
  */
-static int lwmi_cd_alloc(struct lwmi_cd_priv *priv, enum lwmi_cd_type type)
+static int lwmi_cd_alloc(struct lwmi_cd_priv *priv)
 {
 	struct cd_list *list;
 	size_t list_size;
@@ -723,7 +718,7 @@ static int lwmi_cd_alloc(struct lwmi_cd_priv *priv, enum lwmi_cd_type type)
 
 	count = wmidev_instance_count(priv->wdev);
 
-	switch (type) {
+	switch (priv->info->type) {
 	case LENOVO_CAPABILITY_DATA_00:
 		list_size = struct_size(list, cd00, count);
 		break;
@@ -749,7 +744,6 @@ static int lwmi_cd_alloc(struct lwmi_cd_priv *priv, enum lwmi_cd_type type)
 	if (ret)
 		return ret;
 
-	list->type = type;
 	list->count = count;
 	priv->list = list;
 
@@ -759,7 +753,6 @@ static int lwmi_cd_alloc(struct lwmi_cd_priv *priv, enum lwmi_cd_type type)
 /**
  * lwmi_cd_setup() - Cache all WMI data block information
  * @priv: lenovo-wmi-capdata driver data.
- * @type: The type of capability data.
  *
  * Allocate a cd_list struct large enough to contain data from all WMI data
  * blocks provided by the interface. Then loop through each data block and
@@ -767,11 +760,11 @@ static int lwmi_cd_alloc(struct lwmi_cd_priv *priv, enum lwmi_cd_type type)
  *
  * Return: 0 on success, or an error code.
  */
-static int lwmi_cd_setup(struct lwmi_cd_priv *priv, enum lwmi_cd_type type)
+static int lwmi_cd_setup(struct lwmi_cd_priv *priv)
 {
 	int ret;
 
-	ret = lwmi_cd_alloc(priv, type);
+	ret = lwmi_cd_alloc(priv);
 	if (ret)
 		return ret;
 
@@ -844,10 +837,11 @@ static int lwmi_cd_probe(struct wmi_device *wdev, const void *context)
 	if (!priv)
 		return -ENOMEM;
 
+	priv->info = info;
 	priv->wdev = wdev;
 	dev_set_drvdata(&wdev->dev, priv);
 
-	ret = lwmi_cd_setup(priv, info->type);
+	ret = lwmi_cd_setup(priv);
 	if (ret)
 		goto out;
 
@@ -913,7 +907,7 @@ static void lwmi_cd_remove(struct wmi_device *wdev)
 
 	lwmi_cd_debugfs_remove(priv);
 
-	switch (priv->list->type) {
+	switch (priv->info->type) {
 	case LENOVO_CAPABILITY_DATA_00:
 		lwmi_cd_sub_master_del(priv);
 		fallthrough;

-- 
2.55.0

[PATCH v2 05/12] platform/x86: lenovo-wmi-capdata: Defer mutex initialization

[Rong Zhang]

In the following changes, priv->list may be freed if the first call to
lwmi_cd_cache() fails due to WMI/ACPI errors, so the list_mutex must be
initialized after it in order not to break lockdep, as there is no
devm_mutex_destroy(). Considering that the first call to lwmi_cd_cache()
doesn't need serialization as there is no other reader or writer this
early, the initialization of list_mutex can be deferred.

Therefore, initialize list_mutex only after the first call to
lwmi_cd_cache() succeeds, otherwise it remains uninitialized and can be
devm_kfree()-ed.

Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v2:
- Synchronize the initialization with release-acquire barriers (thanks
  Ilpo Järvinen)
---
 drivers/platform/x86/lenovo/wmi-capdata.c | 86 +++++++++++++++++++++++--------
 1 file changed, 65 insertions(+), 21 deletions(-)

diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c b/drivers/platform/x86/lenovo/wmi-capdata.c
index 3008d6d699c0..e52cb2de49a6 100644
--- a/drivers/platform/x86/lenovo/wmi-capdata.c
+++ b/drivers/platform/x86/lenovo/wmi-capdata.c
@@ -48,6 +48,8 @@
 #include <linux/types.h>
 #include <linux/wmi.h>
 
+#include <asm/barrier.h>
+
 #include "wmi-capdata.h"
 #include "wmi-helpers.h"
 
@@ -91,6 +93,7 @@ struct lwmi_cd_priv {
 	struct wmi_device *wdev;
 	struct cd_list *list;
 	struct dentry *debugfs_dir;
+	bool initialized;
 
 	/*
 	 * A capdata device may be a component master of another capdata device.
@@ -588,14 +591,14 @@ static void lwmi_cd_debugfs_remove(struct lwmi_cd_priv *priv)
 /* ======== WMI interface ======== */
 
 /**
- * lwmi_cd_cache() - Cache all WMI data block information
+ * __lwmi_cd_cache() - Cache all WMI data block information locklessly
  * @priv: lenovo-wmi-capdata driver data.
  *
- * Loop through each WMI data block and cache the data.
+ * Loop through each WMI data block and cache the data locklessly.
  *
  * Return: 0 on success, or an error.
  */
-static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
+static int __lwmi_cd_cache(struct lwmi_cd_priv *priv)
 {
 	size_t size;
 	int idx;
@@ -617,7 +620,6 @@ static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
 		return -EINVAL;
 	}
 
-	guard(mutex)(&priv->list->list_mutex);
 	for (idx = 0; idx < priv->list->count; idx++, p += size) {
 		union acpi_object *ret_obj __free(kfree) = NULL;
 
@@ -635,14 +637,38 @@ static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
 	return 0;
 }
 
+/**
+ * lwmi_cd_cache() - Cache all WMI data block information
+ * @priv: lenovo-wmi-capdata driver data.
+ *
+ * Loop through each WMI data block and cache the data.
+ *
+ * Return: 0 on success, or an error.
+ */
+static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
+{
+	/* Synchronize mutex initialization. Paired with lwmi_cd_finalize(). */
+	if (!smp_load_acquire(&priv->initialized))
+		return __lwmi_cd_cache(priv);
+
+	switch (priv->info->type) {
+	case LENOVO_CAPABILITY_DATA_01:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	guard(mutex)(&priv->list->list_mutex);
+	return __lwmi_cd_cache(priv);
+}
+
 /**
  * lwmi_cd_fan_list_alloc_cache() - Alloc and cache Fan Test Data list
  * @priv: lenovo-wmi-capdata driver data.
- * @listptr: Pointer to returned cd_list pointer.
  *
- * Return: count of fans found, or an error.
+ * Return: 0 on success, or an error.
  */
-static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv, struct cd_list **listptr)
+static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv)
 {
 	struct cd_list *list;
 	size_t size;
@@ -688,6 +714,9 @@ static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv, struct cd_lis
 	if (!list)
 		return -ENOMEM;
 
+	list->count = count;
+	priv->list = list;
+
 	for (idx = 0; idx < count; idx++) {
 		/* Do not calculate array index using count, as it may be truncated. */
 		list->cd_fan[idx] = (struct capdata_fan) {
@@ -697,8 +726,7 @@ static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv, struct cd_lis
 		};
 	}
 
-	*listptr = list;
-	return count;
+	return 0;
 }
 
 /**
@@ -714,7 +742,7 @@ static int lwmi_cd_alloc(struct lwmi_cd_priv *priv)
 {
 	struct cd_list *list;
 	size_t list_size;
-	int count, ret;
+	int count;
 
 	count = wmidev_instance_count(priv->wdev);
 
@@ -726,11 +754,7 @@ static int lwmi_cd_alloc(struct lwmi_cd_priv *priv)
 		list_size = struct_size(list, cd01, count);
 		break;
 	case LENOVO_FAN_TEST_DATA:
-		count = lwmi_cd_fan_list_alloc_cache(priv, &list);
-		if (count < 0)
-			return count;
-
-		goto got_list;
+		return lwmi_cd_fan_list_alloc_cache(priv);
 	default:
 		return -EINVAL;
 	}
@@ -739,17 +763,33 @@ static int lwmi_cd_alloc(struct lwmi_cd_priv *priv)
 	if (!list)
 		return -ENOMEM;
 
-got_list:
-	ret = devm_mutex_init(&priv->wdev->dev, &list->list_mutex);
-	if (ret)
-		return ret;
-
 	list->count = count;
 	priv->list = list;
 
 	return 0;
 }
 
+/**
+ * lwmi_cd_finalize() - Finalize the capability data initialization
+ * @priv: lenovo-wmi-capdata driver data.
+ *
+ * Return: 0 on success, or an error code.
+ */
+static int lwmi_cd_finalize(struct lwmi_cd_priv *priv)
+{
+	int ret;
+
+	if (priv->list) {
+		ret = devm_mutex_init(&priv->wdev->dev, &priv->list->list_mutex);
+		if (ret)
+			return ret;
+	}
+
+	/* Synchronize mutex initialization. Paired with lwmi_cd_cache(). */
+	smp_store_release(&priv->initialized, 1);
+	return 0;
+}
+
 /**
  * lwmi_cd_setup() - Cache all WMI data block information
  * @priv: lenovo-wmi-capdata driver data.
@@ -768,7 +808,11 @@ static int lwmi_cd_setup(struct lwmi_cd_priv *priv)
 	if (ret)
 		return ret;
 
-	return lwmi_cd_cache(priv);
+	ret = lwmi_cd_cache(priv);
+	if (ret)
+		return ret;
+
+	return lwmi_cd_finalize(priv);
 }
 
 /**

-- 
2.55.0

[PATCH v2 06/12] platform/x86: lenovo-wmi-{capdata,other}: Only allocate capdata list when necessary

[Rong Zhang]

When no capability data is available, there is no need to allocate
capability data list as it's basically unused except for the
priv->list->count == 0 placeholder.

Therefore, only allocate priv->list when necessary, otherwise its
absence implies the absence of capability data. In this manner,
lenovo-wmi-other can skip registering unavailable functionalities
accordingly. Meanwhile, skip creating the debugfs directory as it
provides nothing when there is no capability data.

Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v2:
- Refine line wrap (thanks Ilpo Järvinen)
---
 drivers/platform/x86/lenovo/wmi-capdata.c | 42 ++++++++++++++++++++++---------
 drivers/platform/x86/lenovo/wmi-other.c   | 24 +++++++++++-------
 2 files changed, 45 insertions(+), 21 deletions(-)

diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c b/drivers/platform/x86/lenovo/wmi-capdata.c
index e52cb2de49a6..de8044ef68b8 100644
--- a/drivers/platform/x86/lenovo/wmi-capdata.c
+++ b/drivers/platform/x86/lenovo/wmi-capdata.c
@@ -315,8 +315,8 @@ static const struct component_ops lwmi_cd_component_ops = {
  * @dev: The sub-master capdata basic device.
  *
  * Call component_bind_all to bind the sub-component device to the sub-master
- * device. On success, collect the pointer to the sub-component list and try
- * to call the master callback.
+ * device. On success, collect the pointer (or ERR_PTR(-ENODEV) if it's stubbed)
+ * to the sub-component list and try to call the master callback.
  *
  * Return: 0 on success, or an error code.
  */
@@ -330,7 +330,7 @@ static int lwmi_cd_sub_master_bind(struct device *dev)
 	if (ret)
 		return ret;
 
-	priv->sub_master->sub_component_list = sub_component_list;
+	priv->sub_master->sub_component_list = sub_component_list ?: ERR_PTR(-ENODEV);
 	lwmi_cd_call_master_cb(priv);
 
 	return 0;
@@ -462,6 +462,9 @@ static const struct component_ops lwmi_cd_sub_component_ops = {
 	{											\
 		u8 idx;										\
 												\
+		if (WARN_ON(!list))								\
+			return -EINVAL;								\
+												\
 		guard(mutex)(&list->list_mutex);						\
 		for (idx = 0; idx < list->count; idx++) {					\
 			if (list->_cdxx[idx].id != attribute_id)				\
@@ -573,6 +576,9 @@ DEFINE_SHOW_ATTRIBUTE(lwmi_cd_debugfs);
  */
 static void lwmi_cd_debugfs_add(struct lwmi_cd_priv *priv)
 {
+	if (!priv->list)
+		return;
+
 	priv->debugfs_dir = lwmi_debugfs_create_dir(priv->wdev);
 
 	debugfs_create_file("capdata", 0444, priv->debugfs_dir, priv, &lwmi_cd_debugfs_fops);
@@ -584,6 +590,7 @@ static void lwmi_cd_debugfs_add(struct lwmi_cd_priv *priv)
  */
 static void lwmi_cd_debugfs_remove(struct lwmi_cd_priv *priv)
 {
+	/* Debugfs can handle NULL dir, no need to check. */
 	debugfs_remove_recursive(priv->debugfs_dir);
 	priv->debugfs_dir = NULL;
 }
@@ -647,6 +654,9 @@ static int __lwmi_cd_cache(struct lwmi_cd_priv *priv)
  */
 static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
 {
+	if (!priv->list)
+		return 0;
+
 	/* Synchronize mutex initialization. Paired with lwmi_cd_finalize(). */
 	if (!smp_load_acquire(&priv->initialized))
 		return __lwmi_cd_cache(priv);
@@ -710,6 +720,9 @@ static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv)
 		count = 0;
 	}
 
+	if (!count)
+		return 0;
+
 	list = devm_kzalloc(&priv->wdev->dev, struct_size(list, cd_fan, count), GFP_KERNEL);
 	if (!list)
 		return -ENOMEM;
@@ -745,6 +758,8 @@ static int lwmi_cd_alloc(struct lwmi_cd_priv *priv)
 	int count;
 
 	count = wmidev_instance_count(priv->wdev);
+	if (!count)
+		return 0;
 
 	switch (priv->info->type) {
 	case LENOVO_CAPABILITY_DATA_00:
@@ -894,7 +909,8 @@ static int lwmi_cd_probe(struct wmi_device *wdev, const void *context)
 		enum lwmi_cd_type sub_component_type = LENOVO_FAN_TEST_DATA;
 		struct capdata00 capdata00;
 
-		ret = lwmi_cd00_get_data(priv->list, LWMI_ATTR_ID_FAN_TEST, &capdata00);
+		ret = priv->list ? lwmi_cd00_get_data(priv->list, LWMI_ATTR_ID_FAN_TEST,
+						      &capdata00) : -ENODATA;
 		if (ret || !(capdata00.supported & LWMI_SUPP_VALID)) {
 			dev_dbg(&wdev->dev, "capdata00 declares no fan test support\n");
 			sub_component_type = CD_TYPE_NONE;
@@ -915,14 +931,16 @@ static int lwmi_cd_probe(struct wmi_device *wdev, const void *context)
 	case LENOVO_CAPABILITY_DATA_01:
 		priv->acpi_nb.notifier_call = lwmi_cd01_notifier_call;
 
-		ret = register_acpi_notifier(&priv->acpi_nb);
-		if (ret)
-			goto out;
+		if (priv->list) {
+			ret = register_acpi_notifier(&priv->acpi_nb);
+			if (ret)
+				goto out;
 
-		ret = devm_add_action_or_reset(&wdev->dev, lwmi_cd01_unregister,
-					       &priv->acpi_nb);
-		if (ret)
-			goto out;
+			ret = devm_add_action_or_reset(&wdev->dev, lwmi_cd01_unregister,
+						       &priv->acpi_nb);
+			if (ret)
+				goto out;
+		}
 
 		ret = component_add(&wdev->dev, &lwmi_cd_component_ops);
 		goto out;
@@ -940,7 +958,7 @@ static int lwmi_cd_probe(struct wmi_device *wdev, const void *context)
 		lwmi_cd_debugfs_add(priv);
 
 		dev_dbg(&wdev->dev, "registered %s with %u items\n",
-			info->name, priv->list->count);
+			info->name, priv->list ? priv->list->count : 0);
 	}
 	return ret;
 }
diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
index fbb32bf404f2..147e182685a1 100644
--- a/drivers/platform/x86/lenovo/wmi-other.c
+++ b/drivers/platform/x86/lenovo/wmi-other.c
@@ -1619,11 +1619,14 @@ static void lwmi_om_fw_attr_remove(struct lwmi_om_priv *priv)
  * lenovo-wmi-other master driver, with a callback to collect fan info from
  * capdata_fan. On success, assign the capability data list pointers to the
  * driver data struct for later access. These pointers are only valid while the
- * capdata interfaces exist. Finally, collect fan info from capdata00 and
- * register all firmware attribute groups. Note that the HWMON device is
- * registered only if all fan info is collected. Hence, it is not registered
- * here. See lwmi_om_fan_info_collect_cd00() and
- * lwmi_om_fan_info_collect_cd_fan().
+ * capdata interfaces exist.
+ *
+ * If capdata00 is bound, collect fan info from capdata00 and register the power
+ * supply extension. Note that the HWMON device is registered only if all fan
+ * info is collected. Hence, it is not registered here. See
+ * lwmi_om_fan_info_collect_cd00() and lwmi_om_fan_info_collect_cd_fan().
+ *
+ * If capdata01 is bound, register all firmware attribute groups.
  *
  * Return: 0 on success, or an error code.
  */
@@ -1643,16 +1646,19 @@ static int lwmi_om_master_bind(struct device *dev)
 
 	priv->cd00_list = binder.cd00_list;
 	priv->cd01_list = binder.cd01_list;
-	if (!priv->cd00_list || !priv->cd01_list) {
+	if (!priv->cd00_list && !priv->cd01_list) {
 		component_unbind_all(dev, NULL);
 
 		return -ENODEV;
 	}
 
-	lwmi_om_fan_info_collect_cd00(priv);
-	lwmi_om_psy_ext_init(priv);
+	if (priv->cd00_list) {
+		lwmi_om_fan_info_collect_cd00(priv);
+		lwmi_om_psy_ext_init(priv);
+	}
 
-	lwmi_om_fw_attr_add(priv);
+	if (priv->cd01_list)
+		lwmi_om_fw_attr_add(priv);
 
 	return 0;
 }

-- 
2.55.0

[PATCH v2 07/12] platform/x86: lenovo-wmi-capdata: Adopt new WMI API

[Rong Zhang]

The new WMI API supports multiple ACPI types by converting them into a
unified buffer that satisfies alignment and size requirements.

Adopt it to make our life easier.

Note that the new WMI API only accepts a few ACPI types to conform to
the behavior of the Windows WMI-ACPI driver. By adopting the new API, we
intentionally rejects improper ACPI types instead of silently ignoring
them.

Meanwhile, considering that `struct_size(block, data, count * 3)' may
overflow when calculating `count * 3', ignore Fan Test Data with count >
U8_MAX instead of caping `count'.

Signed-off-by: Rong Zhang <i@rong.moe>
---
 drivers/platform/x86/lenovo/wmi-capdata.c | 71 ++++++++++++++-----------------
 1 file changed, 32 insertions(+), 39 deletions(-)

diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c b/drivers/platform/x86/lenovo/wmi-capdata.c
index de8044ef68b8..d70fe4504fc5 100644
--- a/drivers/platform/x86/lenovo/wmi-capdata.c
+++ b/drivers/platform/x86/lenovo/wmi-capdata.c
@@ -628,17 +628,19 @@ static int __lwmi_cd_cache(struct lwmi_cd_priv *priv)
 	}
 
 	for (idx = 0; idx < priv->list->count; idx++, p += size) {
-		union acpi_object *ret_obj __free(kfree) = NULL;
+		struct wmi_buffer wbuf;
+		int ret;
 
-		ret_obj = wmidev_block_query(priv->wdev, idx);
-		if (!ret_obj)
-			return -ENODEV;
-
-		if (ret_obj->type != ACPI_TYPE_BUFFER ||
-		    ret_obj->buffer.length < size)
+		ret = wmidev_query_block(priv->wdev, idx, &wbuf, size);
+		if (ret == -ENODATA) /* The block is too short, probably stubbed. */
 			continue;
+		if (ret)
+			return ret;
+
+		/* Capdata 01 is an extension to capdata 00. */
+		struct capdata00 *capdata __free(kfree) = wbuf.data;
 
-		memcpy(p, ret_obj->buffer.pointer, size);
+		memcpy(p, capdata, size);
 	}
 
 	return 0;
@@ -680,43 +682,35 @@ static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
  */
 static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv)
 {
+	struct wmi_buffer wbuf;
 	struct cd_list *list;
-	size_t size;
+	int ret, idx;
 	u32 count;
-	int idx;
 
-	/* Emit unaligned access to u8 buffer with __packed. */
 	struct cd_fan_block {
 		u32 nr;
 		u32 data[]; /* id[nr], max_rpm[nr], min_rpm[nr] */
-	} __packed * block;
+	};
+
+	ret = wmidev_query_block(priv->wdev, 0, &wbuf, sizeof(struct cd_fan_block));
+	if (ret == -ENODATA) /* The block is too short, probably stubbed. */
+		return 0;
+	if (ret)
+		return ret;
 
-	union acpi_object *ret_obj __free(kfree) = wmidev_block_query(priv->wdev, 0);
-	if (!ret_obj)
-		return -ENODEV;
+	struct cd_fan_block *block __free(kfree) = wbuf.data;
 
-	if (ret_obj->type == ACPI_TYPE_BUFFER) {
-		block = (struct cd_fan_block *)ret_obj->buffer.pointer;
-		size = ret_obj->buffer.length;
+	count = block->nr;
 
-		count = size >= sizeof(*block) ? block->nr : 0;
-		if (size < struct_size(block, data, count * 3)) {
-			dev_warn(&priv->wdev->dev,
-				 "incomplete fan test data block: %zu < %zu, ignoring\n",
-				 size, struct_size(block, data, count * 3));
-			count = 0;
-		} else if (count > U8_MAX) {
-			dev_warn(&priv->wdev->dev,
-				 "too many fans reported: %u > %u, truncating\n",
-				 count, U8_MAX);
-			count = U8_MAX;
-		}
-	} else {
-		/*
-		 * This is usually caused by a dummy ACPI method. Do not return an error
-		 * as failing to probe this device will result in sub-master device being
-		 * unbound. This behavior aligns with lwmi_cd_cache().
-		 */
+	if (count > U8_MAX) {
+		dev_warn(&priv->wdev->dev,
+			 "too many fans reported: %u > %u, ignoring\n", count,
+			 U8_MAX);
+		count = 0;
+	} else if (wbuf.length < struct_size(block, data, count * 3)) {
+		dev_warn(&priv->wdev->dev,
+			 "incomplete fan test data block: %zu < %zu (%u fans), ignoring\n",
+			 wbuf.length, struct_size(block, data, count * 3), count);
 		count = 0;
 	}
 
@@ -731,11 +725,10 @@ static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv)
 	priv->list = list;
 
 	for (idx = 0; idx < count; idx++) {
-		/* Do not calculate array index using count, as it may be truncated. */
 		list->cd_fan[idx] = (struct capdata_fan) {
 			.id      = block->data[idx],
-			.max_rpm = block->data[idx + block->nr],
-			.min_rpm = block->data[idx + (2 * block->nr)],
+			.max_rpm = block->data[idx + count],
+			.min_rpm = block->data[idx + (2 * count)],
 		};
 	}
 

-- 
2.55.0

[PATCH v2 08/12] platform/x86: lenovo-wmi-capdata: Register component even on WMI error

[Rong Zhang]

Some devices do not support LENOVO_CAPABILITY_DATA_01 and define the
query method as a stub that returns zero buffer. Unfortunately, some
devices do not implement the stub properly, causing WMI errors
(including ACPI errors).

The current lenovo-wmi-* implementation enforces the binding between
LENOVO_CAPABILITY_DATA_00+01 and LENOVO_OTHER_MODE because of a
limitation of the device component framework. When the capdata device
bailing out due to a WMI error, lenovo-wmi-other becomes unbound and
unable to provide firmware-attributes or hwmon device for the other
functional capdata device.

Therefore, errors must be non-fatal in order not to break the
assumptions made by the device component framework.

Stub the capdata device by releasing the capability data list in this
case. After that, NULL list will be passed to lenovo-wmi-other on bind.
The latter will provide whatever is available, or unbind the components
if nothing is available.

A stubbed capdata device releases or skips allocating most resources,
e.g., the capability data list and the debugfs directory. The device
itself is only used to satisfy the component dependency of lenovo-wmi-
other and coordinate with the latter about the absence of the capability
data.

Reported-by: Charles <hanker007@gmail.com>
Closes: https://msgid.link/CAKtz0s8UYRQYW_0bh=0TMx47Axm-W-muEay-r3rqUBS1NHMPVw@mail.gmail.com/
Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v2:
- Replace the term "poison" with "stub" (thanks Ilpo Järvinen)
---
 drivers/platform/x86/lenovo/wmi-capdata.c | 94 +++++++++++++++++++++++++++++--
 1 file changed, 90 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c b/drivers/platform/x86/lenovo/wmi-capdata.c
index d70fe4504fc5..d913176d2061 100644
--- a/drivers/platform/x86/lenovo/wmi-capdata.c
+++ b/drivers/platform/x86/lenovo/wmi-capdata.c
@@ -29,6 +29,7 @@
 #include <linux/acpi.h>
 #include <linux/bug.h>
 #include <linux/cleanup.h>
+#include <linux/compiler.h>
 #include <linux/component.h>
 #include <linux/container_of.h>
 #include <linux/debugfs.h>
@@ -38,6 +39,7 @@
 #include <linux/export.h>
 #include <linux/gfp_types.h>
 #include <linux/limits.h>
+#include <linux/lockdep.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/mutex_types.h>
@@ -45,6 +47,7 @@
 #include <linux/overflow.h>
 #include <linux/seq_file.h>
 #include <linux/stddef.h>
+#include <linux/string.h>
 #include <linux/types.h>
 #include <linux/wmi.h>
 
@@ -449,7 +452,7 @@ static const struct component_ops lwmi_cd_sub_component_ops = {
 /*
  * lwmi_cd*_get_data - Get the data of the specified attribute
  * @list: The lenovo-wmi-capdata pointer to its cd_list struct.
- * @attribute_id: The capdata attribute ID to be found.
+ * @attribute_id: The capdata attribute ID (non-zero) to be found.
  * @output: Pointer to a capdata* struct to return the data.
  *
  * Retrieves the capability data struct pointer for the given
@@ -462,7 +465,7 @@ static const struct component_ops lwmi_cd_sub_component_ops = {
 	{											\
 		u8 idx;										\
 												\
-		if (WARN_ON(!list))								\
+		if (WARN_ON(!list || !attribute_id))						\
 			return -EINVAL;								\
 												\
 		guard(mutex)(&list->list_mutex);						\
@@ -597,6 +600,89 @@ static void lwmi_cd_debugfs_remove(struct lwmi_cd_priv *priv)
 
 /* ======== WMI interface ======== */
 
+/**
+ * lwmi_cd_stub() - Stub the device by not providing any capability data
+ * @priv: lenovo-wmi-capdata driver data.
+ * @err: The occurred error.
+ *
+ * The Other Mode driver binds to both Capability Data 00 and 01. If either 00
+ * or 01 fails to probe, the Other Mode device will fail to provide fw-attr or
+ * hwmon/power_supply_ext devices for the other functional capdata device.
+ *
+ * Therefore, errors must be non-fatal in order not to break the assumptions
+ * made by the device component framework, so that the Other Mode device can
+ * provide whatever is functional.
+ *
+ * After stubbing the device, a NULL list will be passed to the Other Mode
+ * device on bind.
+ *
+ * Return: 0 if the @err is suppressed, otherwise its propagated as is.
+ */
+static int lwmi_cd_stub(struct lwmi_cd_priv *priv, int err)
+{
+	/*
+	 * No barrier is required in any possible code paths:
+	 * - called from lwmi_cd_cache(): a proper barrier has been employed
+	 * - called from lwmi_cd_fan_list_alloc_cache(): early setup does not
+	 *   require synchronization, and priv->initialized must be false
+	 */
+	bool initialized = READ_ONCE(priv->initialized);
+
+	dev_warn(&priv->wdev->dev, "%s %s (%u items) due to error: %d\n",
+		 initialized ? "clearing" : "stubbing", priv->info->name,
+		 priv->list ? priv->list->count : 0, err);
+
+	/* Simply print the warning message. */
+	if (!priv->list)
+		return initialized ? err : 0;
+
+	/*
+	 * Stub the device on initialization errors.
+	 *
+	 * Note that if a transient error occurs during initialization, there is
+	 * nothing we can do to re-enable the functionalities of the Other Mode
+	 * driver even if a successful runtime notifier call making the relevant
+	 * capdata emerge, as registering functionalities without the protection
+	 * of the component framework is unsafe. Therefore, registering the
+	 * notifier for a stubbed device is meaningless, and we don't need to
+	 * distinguish transient and pernament errors during initialization.
+	 *
+	 * For runtime errors (see below), the registered functionalities
+	 * (fw-attr) become temporarily unavailable due to the unavailablilty of
+	 * the relevant capdata, and will recover after a successful runtime
+	 * notifier call.
+	 */
+	if (!initialized) {
+		devm_kfree(&priv->wdev->dev, priv->list);
+		priv->list = NULL;
+
+		return 0;
+	}
+
+	/*
+	 * Runtime errors are transient. Simply clear all cached data and
+	 * propagate the error.
+	 *
+	 * Since a valid attribute id is never 0 (the firmware also untilize the
+	 * fact to stub some capabilities according to platform metadata),
+	 * clearing the cached data effectively makes all attributes temporarily
+	 * unavailable until the next notifier call.
+	 */
+
+	lockdep_assert_held(&priv->list->list_mutex);
+
+	switch (priv->info->type) {
+	case LENOVO_CAPABILITY_DATA_01:
+		memset(priv->list->cd01, 0,
+		       flex_array_size(priv->list, cd01, priv->list->count));
+		break;
+	default:
+		unreachable();
+	}
+
+	return err;
+}
+
 /**
  * __lwmi_cd_cache() - Cache all WMI data block information locklessly
  * @priv: lenovo-wmi-capdata driver data.
@@ -635,7 +721,7 @@ static int __lwmi_cd_cache(struct lwmi_cd_priv *priv)
 		if (ret == -ENODATA) /* The block is too short, probably stubbed. */
 			continue;
 		if (ret)
-			return ret;
+			return lwmi_cd_stub(priv, ret);
 
 		/* Capdata 01 is an extension to capdata 00. */
 		struct capdata00 *capdata __free(kfree) = wbuf.data;
@@ -696,7 +782,7 @@ static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv)
 	if (ret == -ENODATA) /* The block is too short, probably stubbed. */
 		return 0;
 	if (ret)
-		return ret;
+		return lwmi_cd_stub(priv, ret); /* Print the warning message. */
 
 	struct cd_fan_block *block __free(kfree) = wbuf.data;
 

-- 
2.55.0

[PATCH v2 09/12] platform/x86: lenovo-wmi-capdata: Detect stubbed capdata device

[Rong Zhang]

Some devices may stub the capdata device's WMI query method because it
doesn't support the relevant interfaces at all. For example, most
ThinkBook devices doesn't support GameZone or Other Mode thermal tuning,
so capdata 01 is useless and stubbed.

Keeping the capability data list with empty data is meaningless and
causes lenovo-wmi-other to call lwmi_cd*_get_data() to retrieve
nonexistent capdata in vain.

Therefore, stub the device and release (or skip allocating) needless
resources, e.g., the capability data list and the debugfs directory.

Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v2:
- Replace the term "poison" with "stub" (thanks Ilpo Järvinen)
---
 drivers/platform/x86/lenovo/wmi-capdata.c | 33 +++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c b/drivers/platform/x86/lenovo/wmi-capdata.c
index d913176d2061..79c72717ea40 100644
--- a/drivers/platform/x86/lenovo/wmi-capdata.c
+++ b/drivers/platform/x86/lenovo/wmi-capdata.c
@@ -693,6 +693,7 @@ static int lwmi_cd_stub(struct lwmi_cd_priv *priv, int err)
  */
 static int __lwmi_cd_cache(struct lwmi_cd_priv *priv)
 {
+	bool got_data = false;
 	size_t size;
 	int idx;
 	void *p;
@@ -727,8 +728,31 @@ static int __lwmi_cd_cache(struct lwmi_cd_priv *priv)
 		struct capdata00 *capdata __free(kfree) = wbuf.data;
 
 		memcpy(p, capdata, size);
+
+		/*
+		 * A valid attribute always has a non-zero id. IOW, a zero id
+		 * implies a stubbed item (or the query method being stubbed).
+		 *
+		 * The stubbed item is still copied to the cache slot anyway, in
+		 * case the firmware dynamically stubs the capdata item after
+		 * switching Game Zone mode. In this manner, the previously
+		 * exposed capdata can be void.
+		 */
+		if (capdata->id)
+			got_data = true;
 	}
 
+	/*
+	 * Some devices may stub the query method because it doesn't support the
+	 * relevant interfaces at all. For example, most ThinkBook devices
+	 * doesn't support Game Zone or Other Mode thermal tuning, so capdata 01
+	 * is useless and stubbed.
+	 *
+	 * Stub the device and release needless resources.
+	 */
+	if (!got_data)
+		return lwmi_cd_stub(priv, -ENODATA);
+
 	return 0;
 }
 
@@ -769,6 +793,7 @@ static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
 static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv)
 {
 	struct wmi_buffer wbuf;
+	bool got_data = false;
 	struct cd_list *list;
 	int ret, idx;
 	u32 count;
@@ -811,13 +836,21 @@ static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv)
 	priv->list = list;
 
 	for (idx = 0; idx < count; idx++) {
+		if (!block->data[idx]) /* Ignore stub. */
+			continue;
+
 		list->cd_fan[idx] = (struct capdata_fan) {
 			.id      = block->data[idx],
 			.max_rpm = block->data[idx + count],
 			.min_rpm = block->data[idx + (2 * count)],
 		};
+
+		got_data = true;
 	}
 
+	if (!got_data)
+		return lwmi_cd_stub(priv, -ENODATA);
+
 	return 0;
 }
 

-- 
2.55.0

[PATCH v2 10/12] platform/x86: lenovo-wmi-capdata: Do not match missing components

[Rong Zhang]

Some devices do not support LENOVO_CAPABILITY_DATA_01. They either do
not have the corresponding WMI GUID, or do not implement the query
method.

In both cases, the WMI core does not create the corresponding WMI
device, leaving the component master (i.e., lenovo-wmi-other) waits for
the missing component forever.

Identify missing components using the new wmidev_exists() interface, and
skip adding them to the match list, so that all components in the list
must present, fulfilling the binding requirement.

Reported-by: Navon John Lukose <navonjohnlukose@gmail.com>
Closes: https://msgid.link/20260928190901.1369497-1-navonjohnlukose@gmail.com
Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Link: https://msgid.link/9789f452-d7eb-4f1e-8a13-7335332193a7@app.fastmail.com
Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v2:
- New patch in the series to solve the report made by Navon
---
 drivers/platform/x86/lenovo/wmi-capdata.c | 36 +++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c b/drivers/platform/x86/lenovo/wmi-capdata.c
index 79c72717ea40..6d98003539a7 100644
--- a/drivers/platform/x86/lenovo/wmi-capdata.c
+++ b/drivers/platform/x86/lenovo/wmi-capdata.c
@@ -49,6 +49,7 @@
 #include <linux/stddef.h>
 #include <linux/string.h>
 #include <linux/types.h>
+#include <linux/uuid.h>
 #include <linux/wmi.h>
 
 #include <asm/barrier.h>
@@ -75,14 +76,19 @@ enum lwmi_cd_type {
 	CD_TYPE_NONE = -1,
 };
 
-#define LWMI_CD_TABLE_ITEM(_type)		\
-	[_type] = {				\
-		.name = #_type,			\
-		.type = _type,			\
+static const struct wmi_device_id lwmi_cd_id_table[];
+
+/* Save some .rodata space by reusing the GUID strings from the id table. */
+#define LWMI_CD_TABLE_ITEM(_type)					\
+	[_type] = {							\
+		.name = #_type,						\
+		.guid_string = lwmi_cd_id_table[_type].guid_string,	\
+		.type = _type,						\
 	}
 
 static const struct lwmi_cd_info {
 	const char *name;
+	const char *guid_string;
 	enum lwmi_cd_type type;
 } lwmi_cd_table[] = {
 	LWMI_CD_TABLE_ITEM(LENOVO_CAPABILITY_DATA_00),
@@ -155,24 +161,46 @@ static int lwmi_cd_match(struct device *dev, void *type)
  *
  * Adds all component matches to the list stored in @matchptr for the @master
  * device. @matchptr must be initialized to NULL.
+ *
+ * Missing components are not added to the match list, or else the master waits
+ * for them forever. For some components, their existence can be determined by
+ * querying other components, so they are handled as sub-components and thus not
+ * matched here. For components with no existence declaration, wmidev_exists()
+ * is used to determine their existence.
+ *
+ * If all components are missing, ERR_PTR(-ENXIO) is returned through @matchptr.
  */
 void lwmi_cd_match_add_all(struct device *master, struct component_match **matchptr)
 {
+	bool matched = false;
 	int i;
 
 	if (WARN_ON(*matchptr))
 		return;
 
 	for (i = 0; i < ARRAY_SIZE(lwmi_cd_table); i++) {
+		guid_t guid;
+
 		/* Skip sub-components. */
 		if (lwmi_cd_table[i].type == LENOVO_FAN_TEST_DATA)
 			continue;
 
+		if (WARN_ON(guid_parse(lwmi_cd_table[i].guid_string, &guid)))
+			return;
+
+		if (!wmidev_exists(&guid))
+			continue;
+
 		component_match_add(master, matchptr, lwmi_cd_match,
 				    (void *)&lwmi_cd_table[i].type);
 		if (IS_ERR(*matchptr))
 			return;
+
+		matched = true;
 	}
+
+	if (!matched)
+		*matchptr = ERR_PTR(-ENXIO);
 }
 EXPORT_SYMBOL_NS_GPL(lwmi_cd_match_add_all, "LENOVO_WMI_CAPDATA");
 

-- 
2.55.0

[PATCH v2 11/12] platform/x86: lenovo-wmi-helpers: Adopt new WMI API

[Rong Zhang]

The new WMI API supports multiple ACPI types by converting them into a
unified buffer that satisfies alignment and size requirements.

Adopt it to make our life easier.

Note that the Windows WMI-ACPI driver converts all ACPI objects into a
common buffer format, so adopting the new API intentionally accepts more
ACPI types as a followup of commit 465dc9da8ff6 ("platform/x86:
lenovo-wmi-helpers: Convert returned buffer into u32").

Suggested-by: Armin Wolf <W_Armin@gmx.de>
Link: https://lore.kernel.org/r/f1787927-b655-4321-b9d9-bc12353c72db@gmx.de/
Signed-off-by: Rong Zhang <i@rong.moe>
---
 drivers/platform/x86/lenovo/wmi-helpers.c | 61 ++++++++++++-------------------
 1 file changed, 23 insertions(+), 38 deletions(-)

diff --git a/drivers/platform/x86/lenovo/wmi-helpers.c b/drivers/platform/x86/lenovo/wmi-helpers.c
index 8f5766c391eb..687caa226263 100644
--- a/drivers/platform/x86/lenovo/wmi-helpers.c
+++ b/drivers/platform/x86/lenovo/wmi-helpers.c
@@ -24,9 +24,10 @@
 #include <linux/export.h>
 #include <linux/module.h>
 #include <linux/notifier.h>
-#include <linux/unaligned.h>
 #include <linux/wmi.h>
 
+#include <asm/byteorder.h>
+
 #include "wmi-helpers.h"
 
 /* Thermal mode notifier chain. */
@@ -42,49 +43,33 @@ static BLOCKING_NOTIFIER_HEAD(tm_chain_head);
  * @size: Length of the buffer.
  * @retval: Pointer for the return value to be assigned.
  *
- * Calls wmidev_evaluate_method for Lenovo WMI devices that return an ACPI
- * integer. Validates the return value type and assigns the value to the
- * retval pointer.
+ * Calls the specified WMI method for Lenovo WMI devices that return a WMI
+ * integer. Validates the return value and assigns it to @retval when it's not
+ * NULL, otherwise the return value is not validated.
  *
  * Return: 0 on success, or an error code.
  */
 int lwmi_dev_evaluate_int(struct wmi_device *wdev, u8 instance, u32 method_id,
 			  unsigned char *buf, size_t size, u32 *retval)
 {
-	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
-	struct acpi_buffer input = { size, buf };
-	acpi_status status;
-
-	status = wmidev_evaluate_method(wdev, instance, method_id, &input,
-					&output);
-	if (ACPI_FAILURE(status))
-		return -EIO;
-
-	union acpi_object *ret_obj __free(kfree) = output.pointer;
-
-	if (retval) {
-		if (!ret_obj)
-			return -ENODATA;
-
-		switch (ret_obj->type) {
-		/*
-		 * The ACPI method may simply return a buffer when a u32
-		 * is expected. This is valid on Windows as its WMI-ACPI
-		 * driver converts everything to a common buffer.
-		 */
-		case ACPI_TYPE_BUFFER:
-			if (ret_obj->buffer.length < sizeof(u32))
-				return -ENXIO;
-
-			*retval = get_unaligned_le32(ret_obj->buffer.pointer);
-			return 0;
-		case ACPI_TYPE_INTEGER:
-			*retval = (u32)ret_obj->integer.value;
-			return 0;
-		default:
-			return -ENXIO;
-		}
-	}
+	struct wmi_buffer input = {
+		.length = size,
+		.data = buf,
+	};
+	struct wmi_buffer output;
+	int ret;
+
+	if (!retval)
+		return wmidev_invoke_procedure(wdev, instance, method_id, &input);
+
+	ret = wmidev_invoke_method(wdev, instance, method_id, &input,
+				   &output, sizeof(__le32));
+	if (ret)
+		return ret;
+
+	__le32 *valp __free(kfree) = output.data;
+
+	*retval = le32_to_cpu(*valp);
 
 	return 0;
 };

-- 
2.55.0

[PATCH v2 12/12] MAINTAINERS: Add myself as a LENOVO drivers co-maintainer

[Rong Zhang]

I regularly review patches and resolve bug reports posted on the pdx86
mailing list or the kernel Bugzilla to improve LENOVO drivers (including
lenovo-wmi-* and ideapad-laptop) along with Derek J. Clark and Mark
Pearson. I've been submitting patches to add new features and fix issues
on my devices, too.

Add myself as a LENOVO drivers co-maintainer to reflect this.

Suggested-by: Derek J. Clark <derekjohn.clark@gmail.com>
Link: https://msgid.link/782FE636-A06A-4E12-9563-786374805947@gmail.com
Signed-off-by: Rong Zhang <i@rong.moe>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index f53047aa8d3c..fa576b4b5919 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14808,6 +14808,7 @@ F:	drivers/usb/misc/legousbtower.c
 LENOVO drivers
 M:	Mark Pearson <mpearson-lenovo@squebb.ca>
 M:	Derek J. Clark <derekjohn.clark@gmail.com>
+M:	Rong Zhang <i@rong.moe>
 L:	platform-driver-x86@vger.kernel.org
 S:	Maintained
 F:	Documentation/wmi/devices/lenovo-wmi-gamezone.rst

-- 
2.55.0

Re: [PATCH v2 03/12] platform/x86: lenovo-wmi-capdata: Only allocate sub-master info when necessary

[Derek J. Clark]

On October 9, 2026 5:53:45 AM PDT, Rong Zhang <i@rong.moe> wrote:
>When capdata00 declares no fan test support, there is no need to
>allocate sub-master info as it's basically unused except for the
>priv->sub_master->sub_component_list == ERR_PTR(-ENODEV) placeholder.
>
>Therefore, only allocate priv->sub_master when necessary, otherwise its
>absence implies the absence of sub-component. With that,
>PTR_ERR(sub_component_list) == -ENODEV is repurposed for the following
>changes to represent stubbed sub-component.
>
>Signed-off-by: Rong Zhang <i@rong.moe>
>---
> drivers/platform/x86/lenovo/wmi-capdata.c | 36 ++++++++++++++++++-------------
> 1 file changed, 21 insertions(+), 15 deletions(-)
>
>diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c b/drivers/platform/x86/lenovo/wmi-capdata.c
>index 58787da959b4..3a823aee608a 100644
>--- a/drivers/platform/x86/lenovo/wmi-capdata.c
>+++ b/drivers/platform/x86/lenovo/wmi-capdata.c
>@@ -101,9 +101,11 @@ struct lwmi_cd_priv {
> 	struct lwmi_cd_sub_master_priv {
> 		struct device *master_dev;
> 		cd_list_cb_t master_cb;
>-		struct cd_list *sub_component_list; /* ERR_PTR(-ENODEV) implies no sub-component. */
>+		struct cd_list *sub_component_list; /* ERR_PTR(-ENODEV) implies stubbed
>+						     * sub-component.
>+						     */

Hi Rong,

Hitting the column limit here looks ugly and harms readability IMO. Perhaps we could rename the variable (sc_list?) or be less explicit about the full error signature?
- Derek

> 		bool registered;                    /* Has the sub-master been registered? */
>-	} *sub_master;
>+	} *sub_master;                              /* NULL impLnlies no sub-component. */
> };
> 
> struct cd_list {
>@@ -258,6 +260,12 @@ static int lwmi_cd_component_bind(struct device *cd_dev,
> 	case LENOVO_CAPABILITY_DATA_00:
> 		binder->cd00_list = priv->list;
> 
>+		/* Missing sub-master info implies missing sub-component. */
>+		if (!priv->sub_master) {
>+			binder->cd_fan_list_cb(om_dev, NULL);
>+			break;
>+		}
>+
> 		priv->sub_master->master_dev = om_dev;
> 		priv->sub_master->master_cb = binder->cd_fan_list_cb;
> 		lwmi_cd_call_master_cb(priv);
>@@ -288,13 +296,9 @@ static void lwmi_cd_component_unbind(struct device *cd_dev,
> {
> 	struct lwmi_cd_priv *priv = dev_get_drvdata(cd_dev);
> 
>-	switch (priv->list->type) {
>-	case LENOVO_CAPABILITY_DATA_00:
>+	if (priv->sub_master) {
> 		priv->sub_master->master_dev = NULL;
> 		priv->sub_master->master_cb = NULL;
>-		return;
>-	default:
>-		return;
> 	}
> }
> 
>@@ -369,17 +373,19 @@ static int lwmi_cd_sub_master_add(struct lwmi_cd_priv *priv,
> 	struct component_match *master_match = NULL;
> 	int ret;
> 
>+	/*
>+	 * If the sub-master has no sub-component, there is no need to allocate
>+	 * or register the sub-master. The master callback will be called with
>+	 * NULL on master <-> sub-master bind to inform the master about the
>+	 * absence of sub-component.
>+	 */
>+	if (sub_component_type == CD_TYPE_NONE)
>+		return 0;
>+
> 	priv->sub_master = devm_kzalloc(&priv->wdev->dev, sizeof(*priv->sub_master), GFP_KERNEL);
> 	if (!priv->sub_master)
> 		return -ENOMEM;
> 
>-	if (sub_component_type == CD_TYPE_NONE) {
>-		/* The master callback will be called with NULL on bind. */
>-		priv->sub_master->sub_component_list = ERR_PTR(-ENODEV);
>-		priv->sub_master->registered = false;
>-		return 0;
>-	}
>-
> 	/*
> 	 * lwmi_cd_match() needs a pointer to enum lwmi_cd_type, but on-stack
> 	 * data cannot be used here. Steal one from lwmi_cd_table.
>@@ -404,7 +410,7 @@ static int lwmi_cd_sub_master_add(struct lwmi_cd_priv *priv,
>  */
> static void lwmi_cd_sub_master_del(struct lwmi_cd_priv *priv)
> {
>-	if (!priv->sub_master->registered)
>+	if (!priv->sub_master || !priv->sub_master->registered)
> 		return;
> 
> 	component_master_del(&priv->wdev->dev, &lwmi_cd_sub_master_ops);
>

Re: [PATCH v2 03/12] platform/x86: lenovo-wmi-capdata: Only allocate sub-master info when necessary

[Rong Zhang]

Hi Derek,

Thanks for your review.

On Fri, 2026-10-09 at 08:15 -0700, Derek J. Clark wrote:
> On October 9, 2026 5:53:45 AM PDT, Rong Zhang <i@rong.moe> wrote:
> > When capdata00 declares no fan test support, there is no need to
> > allocate sub-master info as it's basically unused except for the
> > priv->sub_master->sub_component_list == ERR_PTR(-ENODEV) placeholder.
> > 
> > Therefore, only allocate priv->sub_master when necessary, otherwise its
> > absence implies the absence of sub-component. With that,
> > PTR_ERR(sub_component_list) == -ENODEV is repurposed for the following
> > changes to represent stubbed sub-component.
> > 
> > Signed-off-by: Rong Zhang <i@rong.moe>
> > ---
> > drivers/platform/x86/lenovo/wmi-capdata.c | 36 ++++++++++++++++++-------------
> > 1 file changed, 21 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c b/drivers/platform/x86/lenovo/wmi-capdata.c
> > index 58787da959b4..3a823aee608a 100644
> > --- a/drivers/platform/x86/lenovo/wmi-capdata.c
> > +++ b/drivers/platform/x86/lenovo/wmi-capdata.c
> > @@ -101,9 +101,11 @@ struct lwmi_cd_priv {
> > 	struct lwmi_cd_sub_master_priv {
> > 		struct device *master_dev;
> > 		cd_list_cb_t master_cb;
> > -		struct cd_list *sub_component_list; /* ERR_PTR(-ENODEV) implies no sub-component. */
> > +		struct cd_list *sub_component_list; /* ERR_PTR(-ENODEV) implies stubbed
> > +						     * sub-component.
> > +						     */
> 
> Hi Rong,
> 
> Hitting the column limit here looks ugly and harms readability IMO. Perhaps we could rename the variable (sc_list?) or be less explicit about the full error signature?

Makes sense. Will rephrase it into:

		struct cd_list *sub_component_list; /* ERR_PTR(-ENODEV): stubbed sub-component. */

Thanks,
Rong

> - Derek
> 
> > 		bool registered;                    /* Has the sub-master been registered? */
> > -	} *sub_master;
> > +	} *sub_master;                              /* NULL impLnlies no sub-component. */
> > };
> > 
> > struct cd_list {
> > @@ -258,6 +260,12 @@ static int lwmi_cd_component_bind(struct device *cd_dev,
> > 	case LENOVO_CAPABILITY_DATA_00:
> > 		binder->cd00_list = priv->list;
> > 
> > +		/* Missing sub-master info implies missing sub-component. */
> > +		if (!priv->sub_master) {
> > +			binder->cd_fan_list_cb(om_dev, NULL);
> > +			break;
> > +		}
> > +
> > 		priv->sub_master->master_dev = om_dev;
> > 		priv->sub_master->master_cb = binder->cd_fan_list_cb;
> > 		lwmi_cd_call_master_cb(priv);
> > @@ -288,13 +296,9 @@ static void lwmi_cd_component_unbind(struct device *cd_dev,
> > {
> > 	struct lwmi_cd_priv *priv = dev_get_drvdata(cd_dev);
> > 
> > -	switch (priv->list->type) {
> > -	case LENOVO_CAPABILITY_DATA_00:
> > +	if (priv->sub_master) {
> > 		priv->sub_master->master_dev = NULL;
> > 		priv->sub_master->master_cb = NULL;
> > -		return;
> > -	default:
> > -		return;
> > 	}
> > }
> > 
> > @@ -369,17 +373,19 @@ static int lwmi_cd_sub_master_add(struct lwmi_cd_priv *priv,
> > 	struct component_match *master_match = NULL;
> > 	int ret;
> > 
> > +	/*
> > +	 * If the sub-master has no sub-component, there is no need to allocate
> > +	 * or register the sub-master. The master callback will be called with
> > +	 * NULL on master <-> sub-master bind to inform the master about the
> > +	 * absence of sub-component.
> > +	 */
> > +	if (sub_component_type == CD_TYPE_NONE)
> > +		return 0;
> > +
> > 	priv->sub_master = devm_kzalloc(&priv->wdev->dev, sizeof(*priv->sub_master), GFP_KERNEL);
> > 	if (!priv->sub_master)
> > 		return -ENOMEM;
> > 
> > -	if (sub_component_type == CD_TYPE_NONE) {
> > -		/* The master callback will be called with NULL on bind. */
> > -		priv->sub_master->sub_component_list = ERR_PTR(-ENODEV);
> > -		priv->sub_master->registered = false;
> > -		return 0;
> > -	}
> > -
> > 	/*
> > 	 * lwmi_cd_match() needs a pointer to enum lwmi_cd_type, but on-stack
> > 	 * data cannot be used here. Steal one from lwmi_cd_table.
> > @@ -404,7 +410,7 @@ static int lwmi_cd_sub_master_add(struct lwmi_cd_priv *priv,
> >  */
> > static void lwmi_cd_sub_master_del(struct lwmi_cd_priv *priv)
> > {
> > -	if (!priv->sub_master->registered)
> > +	if (!priv->sub_master || !priv->sub_master->registered)
> > 		return;
> > 
> > 	component_master_del(&priv->wdev->dev, &lwmi_cd_sub_master_ops);
> > 

Re: [PATCH v2 01/12] platform/wmi: Introduce wmidev_exists()

[Mark Pearson]

On Fri, Oct 9, 2026, at 8:53 AM, Rong Zhang wrote:
> From: Armin Wolf <W_Armin@gmx.de>
>
> Some WMI drivers need to be able to detect the existence of WMI
> devices matching a given GUID in order to determine the availability
> of optional features. Using wmi_has_guid() for this will not work for
> WMI devices that cannot be used with the legacy GUID-based API (aka
> have WMI_GUID_DUPLICATED set).
>
> Introduce wmidev_exists() that also checks WMI devices that are only
> available to the bus-based API. Also mark wmi_has_guid() as deprecated
> because only WMI drivers using the legacy GUID-based API will continue
> to have use for it.
>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
> Changes in v2:
> - New patch in the series as discussed at
>   https://msgid.link/83776200-4A60-4756-A99F-D5A3BB4D834A@rong.moe
> ---
>  drivers/platform/wmi/core.c | 33 +++++++++++++++++++++++++++++----
>  include/linux/wmi.h         |  3 +++
>  2 files changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/platform/wmi/core.c b/drivers/platform/wmi/core.c
> index 529825dcfbfe..a99da9b7039d 100644
> --- a/drivers/platform/wmi/core.c
> +++ b/drivers/platform/wmi/core.c
> @@ -145,7 +145,7 @@ static inline void get_acpi_method_name(const 
> struct wmi_block *wblock,
>  	buffer[4] = '\0';
>  }
> 
> -static int wmidev_match_guid(struct device *dev, const void *data)
> +static int wmidev_match_guid_legacy(struct device *dev, const void 
> *data)
>  {
>  	struct wmi_block *wblock = dev_to_wblock(dev);
>  	const guid_t *guid = data;
> @@ -218,7 +218,7 @@ static struct wmi_device 
> *wmi_find_device_by_guid(const char *guid_string)
>  	if (ret < 0)
>  		return ERR_PTR(ret);
> 
> -	dev = bus_find_device(&wmi_bus_type, NULL, &guid, wmidev_match_guid);
> +	dev = bus_find_device(&wmi_bus_type, NULL, &guid, wmidev_match_guid_legacy);
>  	if (!dev)
>  		return ERR_PTR(-ENODEV);
> 
> @@ -800,10 +800,10 @@ acpi_status wmi_remove_notify_handler(const char *guid)
>  EXPORT_SYMBOL_GPL(wmi_remove_notify_handler);
> 
>  /**
> - * wmi_has_guid - Check if a GUID is available
> + * wmi_has_guid - Check if a GUID is available (deprecated)
>   * @guid_string: 36 char string of the form 
> fa50ff2b-f2e8-45de-83fa-65417f2f49ba
>   *
> - * Check if a given GUID is defined by _WDG.
> + * Check if a given GUID is available and usable with the legacy 
> GUID-based API.
>   *
>   * Return: True if GUID is available, false otherwise.
>   */
> @@ -821,6 +821,31 @@ bool wmi_has_guid(const char *guid_string)
>  }
>  EXPORT_SYMBOL_GPL(wmi_has_guid);
> 
> +static int wmidev_match_guid(struct device *dev, void *data)
> +{
> +	struct wmi_block *wblock = dev_to_wblock(dev);
> +	const guid_t *guid = data;
> +
> +	if (guid_equal(guid, &wblock->gblock.guid))
> +		return 1;
> +
> +	return 0;
> +}
> +
> +/**
> + * wmidev_exists - Check if a WMI device with a given GUID exists
> + * @guid: GUID of the WMI device to search for
> + *
> + * Check if a WMI device with a given GUID exists at the given moment.
> + *
> + * Return: True if a matching WMI device exists, false otherwise.
> + */
> +bool wmidev_exists(const guid_t *guid)
> +{
> +	return bus_for_each_dev(&wmi_bus_type, NULL, (void *)guid, 
> wmidev_match_guid) == 1;
> +}
> +EXPORT_SYMBOL_GPL(wmidev_exists);
> +
>  /**
>   * wmi_get_acpi_device_uid() - Get _UID name of ACPI device that 
> defines GUID (deprecated)
>   * @guid_string: 36 char string of the form 
> fa50ff2b-f2e8-45de-83fa-65417f2f49ba
> diff --git a/include/linux/wmi.h b/include/linux/wmi.h
> index defcb624a7e2..3dd223f2ff18 100644
> --- a/include/linux/wmi.h
> +++ b/include/linux/wmi.h
> @@ -13,6 +13,7 @@
>  #include <linux/acpi.h>
>  #include <linux/device-id/wmi.h>
>  #include <linux/types.h>
> +#include <linux/uuid.h>
> 
>  /**
>   * struct wmi_device - WMI device structure
> @@ -83,6 +84,8 @@ acpi_status wmidev_block_set(struct wmi_device *wdev, 
> u8 instance, const struct
> 
>  u8 wmidev_instance_count(struct wmi_device *wdev);
> 
> +bool wmidev_exists(const guid_t *guid);
> +
>  /**
>   * struct wmi_driver - WMI driver structure
>   * @driver: Driver model structure
>
> -- 
> 2.55.0

Tested and confirmed this works.

Tested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>

Mark

Re: [PATCH v2 12/12] MAINTAINERS: Add myself as a LENOVO drivers co-maintainer

[Mark Pearson]

On Fri, Oct 9, 2026, at 8:53 AM, Rong Zhang wrote:
> I regularly review patches and resolve bug reports posted on the pdx86
> mailing list or the kernel Bugzilla to improve LENOVO drivers (including
> lenovo-wmi-* and ideapad-laptop) along with Derek J. Clark and Mark
> Pearson. I've been submitting patches to add new features and fix issues
> on my devices, too.
>
> Add myself as a LENOVO drivers co-maintainer to reflect this.
>
> Suggested-by: Derek J. Clark <derekjohn.clark@gmail.com>
> Link: https://msgid.link/782FE636-A06A-4E12-9563-786374805947@gmail.com
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f53047aa8d3c..fa576b4b5919 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14808,6 +14808,7 @@ F:	drivers/usb/misc/legousbtower.c
>  LENOVO drivers
>  M:	Mark Pearson <mpearson-lenovo@squebb.ca>
>  M:	Derek J. Clark <derekjohn.clark@gmail.com>
> +M:	Rong Zhang <i@rong.moe>
>  L:	platform-driver-x86@vger.kernel.org
>  S:	Maintained
>  F:	Documentation/wmi/devices/lenovo-wmi-gamezone.rst
>
> -- 
> 2.55.0

Sounds good to me. Thanks for all the work you do on these platforms.

Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Mark

Re: [PATCH v2 00/12] platform/x86: lenovo-wmi-{other,capdata,helpers}: Improve robustness on faulty firmware

[Derek J. Clark]

On October 9, 2026 5:53:42 AM PDT, Rong Zhang <i@rong.moe> wrote:
>Some devices do not support LENOVO_CAPABILITY_DATA_01 and define the
>query method as a stub that returns zero buffer. Unfortunately, some
>devices do not implement the stub properly, causing WMI errors
>(including ACPI errors). This was reported by Charles.
>
>The current lenovo-wmi-* implementation enforces the binding between
>LENOVO_CAPABILITY_DATA_00+01 and LENOVO_OTHER_MODE because of a
>limitation of the device component framework. When the capdata device
>bailing out due to a WMI error, lenovo-wmi-other becomes unbound and
>unable to provide firmware-attributes or hwmon/power_supply_ext devices
>for the other functional capdata device.
>
>Therefore, errors must be non-fatal in order not to break the
>assumptions made by the device component framework.
>
>Poison the capdata device by releasing the capability data list in this
>case. After that, NULL list will be passed to lenovo-wmi-other on bind.
>The latter will provide whatever is available, or unbind the components
>if nothing is available.
>
>A poisoned capdata device releases or skips allocating most resources,
>e.g., the capability data list and the debugfs directory. The device
>itself is only used to satisfy the component dependency of lenovo-wmi-
>other and coordinate with the latter about the absence of the capability
>data.
>
>Meanwhile, for devices that properly stubs the WMI query method (but
>still declares >0 instances), keeping the capability data list with
>empty data is meaningless and causes lenovo-wmi-other to call
>lwmi_cd*_get_data() to retrieve nonexistent capdata in vain. These
>capdata devices are poisoned as well to save resources.
>
>In order to release or skip allocating most resources for poisoned
>devices, some preparatory work is done in prior. With the preparatory
>work, it also skips allocating most resources for the WMI devices that
>declare 0 instance.
>
>Also identify missing components using the new wmidev_exists() interface
>(introduced at the very beginning of the series), and skip adding them
>to the match list, so that all components in the list must present,
>fulfilling the binding requirement. Some devices need this because they
>either do not have the WMI GUID of LENOVO_CAPABILITY_DATA_01, or do not
>implement the query method, causing the WMI core not to create the
>corresponding WMI device. This was reported by Navon.
>
>The new WMI API is also adopted to conform to the behavior of the
>Windows WMI-ACPI driver and improve robustness on various WMI ACPI
>method implementation.
>
>Finally, add myself as a LENOVO drivers maintainer as previously
>suggested by Derek.

Hi Rong,

I'll try to fully test all my devices this weekend to add a T/b tag. In the mantime, everything looks good, save for that minor nit you already acked. 

With that resolved, for the series:
Reviewed-by: Derek J. Clark <derekjohn.clark@gmail.com>

Thanks,
- Derek

>Reported-by: Charles <hanker007@gmail.com>
>Closes: https://msgid.link/CAKtz0s8UYRQYW_0bh=0TMx47Axm-W-muEay-r3rqUBS1NHMPVw@mail.gmail.com/
>Reported-by: Navon John Lukose <navonjohnlukose@gmail.com>
>Closes: https://msgid.link/20260928190901.1369497-1-navonjohnlukose@gmail.com
>Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
>Link: https://msgid.link/9789f452-d7eb-4f1e-8a13-7335332193a7@app.fastmail.com
>Suggested-by: Derek J. Clark <derekjohn.clark@gmail.com>
>Link: https://msgid.link/782FE636-A06A-4E12-9563-786374805947@gmail.com
>Signed-off-by: Rong Zhang <i@rong.moe>
>---
>Changes in v2:
>- Add PATCH 1 ("platform/wmi: Introduce wmidev_exists()") to the series
>  as discussed at https://msgid.link/83776200-4A60-4756-A99F-D5A3BB4D834A@rong.moe
>- Add PATCH 2 ("lenovo-wmi-capdata: Do not stop the AC notifier chain on
>  error") to the series, as adopting the new WMI API will intentionally
>  catch more faulty firmware and propagate more errors
>- Synchronize mutex initialization with release-acquire barriers (thanks
>  Ilpo Järvinen)
>- Refine line wrap (ditto)
>- Replace the term "poison" with "stub" (ditto)
>- Add PATCH 10 ("platform/x86: lenovo-wmi-capdata: Do not match missing
>  components") to the series to solve the report made by Navon
>- Update outdated comments, function documentations and commit messages
>- Link to v1: https://patch.msgid.link/20260914-lwmi-wmi-new-api-v1-0-7a400f2f69f8@rong.moe
>
>---
>Armin Wolf (1):
>      platform/wmi: Introduce wmidev_exists()
>
>Rong Zhang (11):
>      platform/x86: lenovo-wmi-capdata: Do not stop the AC notifier chain on error
>      platform/x86: lenovo-wmi-capdata: Only allocate sub-master info when necessary
>      platform/x86: lenovo-wmi-capdata: Store a pointer to component info
>      platform/x86: lenovo-wmi-capdata: Defer mutex initialization
>      platform/x86: lenovo-wmi-{capdata,other}: Only allocate capdata list when necessary
>      platform/x86: lenovo-wmi-capdata: Adopt new WMI API
>      platform/x86: lenovo-wmi-capdata: Register component even on WMI error
>      platform/x86: lenovo-wmi-capdata: Detect stubbed capdata device
>      platform/x86: lenovo-wmi-capdata: Do not match missing components
>      platform/x86: lenovo-wmi-helpers: Adopt new WMI API
>      MAINTAINERS: Add myself as a LENOVO drivers co-maintainer
>
> MAINTAINERS                               |   1 +
> drivers/platform/wmi/core.c               |  33 ++-
> drivers/platform/x86/lenovo/wmi-capdata.c | 450 ++++++++++++++++++++++--------
> drivers/platform/x86/lenovo/wmi-helpers.c |  61 ++--
> drivers/platform/x86/lenovo/wmi-other.c   |  24 +-
> include/linux/wmi.h                       |   3 +
> 6 files changed, 400 insertions(+), 172 deletions(-)
>---
>base-commit: af32da41b0327b9c6a37856ba82b6760d6c8d10e
>change-id: a504a929-lwmi-wmi-new-api-f5344d48a86a
>
>Thanks,
>Rong
>

Re: [PATCH v2 07/12] platform/x86: lenovo-wmi-capdata: Adopt new WMI API

[Armin Wolf]

Am 09.10.26 um 14:53 schrieb Rong Zhang:

> The new WMI API supports multiple ACPI types by converting them into a
> unified buffer that satisfies alignment and size requirements.
>
> Adopt it to make our life easier.
>
> Note that the new WMI API only accepts a few ACPI types to conform to
> the behavior of the Windows WMI-ACPI driver. By adopting the new API, we
> intentionally rejects improper ACPI types instead of silently ignoring
> them.
>
> Meanwhile, considering that `struct_size(block, data, count * 3)' may
> overflow when calculating `count * 3', ignore Fan Test Data with count >
> U8_MAX instead of caping `count'.
>
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
>   drivers/platform/x86/lenovo/wmi-capdata.c | 71 ++++++++++++++-----------------
>   1 file changed, 32 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c b/drivers/platform/x86/lenovo/wmi-capdata.c
> index de8044ef68b8..d70fe4504fc5 100644
> --- a/drivers/platform/x86/lenovo/wmi-capdata.c
> +++ b/drivers/platform/x86/lenovo/wmi-capdata.c
> @@ -628,17 +628,19 @@ static int __lwmi_cd_cache(struct lwmi_cd_priv *priv)
>   	}
>   
>   	for (idx = 0; idx < priv->list->count; idx++, p += size) {
> -		union acpi_object *ret_obj __free(kfree) = NULL;
> +		struct wmi_buffer wbuf;
> +		int ret;
>   
> -		ret_obj = wmidev_block_query(priv->wdev, idx);
> -		if (!ret_obj)
> -			return -ENODEV;
> -
> -		if (ret_obj->type != ACPI_TYPE_BUFFER ||
> -		    ret_obj->buffer.length < size)
> +		ret = wmidev_query_block(priv->wdev, idx, &wbuf, size);
> +		if (ret == -ENODATA) /* The block is too short, probably stubbed. */
>   			continue;
> +		if (ret)
> +			return ret;
> +
> +		/* Capdata 01 is an extension to capdata 00. */
> +		struct capdata00 *capdata __free(kfree) = wbuf.data;
>   
> -		memcpy(p, ret_obj->buffer.pointer, size);
> +		memcpy(p, capdata, size);
>   	}
>   
>   	return 0;
> @@ -680,43 +682,35 @@ static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
>    */
>   static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv)
>   {
> +	struct wmi_buffer wbuf;
>   	struct cd_list *list;
> -	size_t size;
> +	int ret, idx;
>   	u32 count;
> -	int idx;
>   
> -	/* Emit unaligned access to u8 buffer with __packed. */
>   	struct cd_fan_block {
>   		u32 nr;
>   		u32 data[]; /* id[nr], max_rpm[nr], min_rpm[nr] */
> -	} __packed * block;
> +	};

Hi,

i suggest you keep the __packed here. With this being addressed:

Reviewed-by: Armin Wolf <W_Armin@gmx.de>

> +
> +	ret = wmidev_query_block(priv->wdev, 0, &wbuf, sizeof(struct cd_fan_block));
> +	if (ret == -ENODATA) /* The block is too short, probably stubbed. */
> +		return 0;
> +	if (ret)
> +		return ret;
>   
> -	union acpi_object *ret_obj __free(kfree) = wmidev_block_query(priv->wdev, 0);
> -	if (!ret_obj)
> -		return -ENODEV;
> +	struct cd_fan_block *block __free(kfree) = wbuf.data;
>   
> -	if (ret_obj->type == ACPI_TYPE_BUFFER) {
> -		block = (struct cd_fan_block *)ret_obj->buffer.pointer;
> -		size = ret_obj->buffer.length;
> +	count = block->nr;
>   
> -		count = size >= sizeof(*block) ? block->nr : 0;
> -		if (size < struct_size(block, data, count * 3)) {
> -			dev_warn(&priv->wdev->dev,
> -				 "incomplete fan test data block: %zu < %zu, ignoring\n",
> -				 size, struct_size(block, data, count * 3));
> -			count = 0;
> -		} else if (count > U8_MAX) {
> -			dev_warn(&priv->wdev->dev,
> -				 "too many fans reported: %u > %u, truncating\n",
> -				 count, U8_MAX);
> -			count = U8_MAX;
> -		}
> -	} else {
> -		/*
> -		 * This is usually caused by a dummy ACPI method. Do not return an error
> -		 * as failing to probe this device will result in sub-master device being
> -		 * unbound. This behavior aligns with lwmi_cd_cache().
> -		 */
> +	if (count > U8_MAX) {
> +		dev_warn(&priv->wdev->dev,
> +			 "too many fans reported: %u > %u, ignoring\n", count,
> +			 U8_MAX);
> +		count = 0;
> +	} else if (wbuf.length < struct_size(block, data, count * 3)) {
> +		dev_warn(&priv->wdev->dev,
> +			 "incomplete fan test data block: %zu < %zu (%u fans), ignoring\n",
> +			 wbuf.length, struct_size(block, data, count * 3), count);
>   		count = 0;
>   	}
>   
> @@ -731,11 +725,10 @@ static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv)
>   	priv->list = list;
>   
>   	for (idx = 0; idx < count; idx++) {
> -		/* Do not calculate array index using count, as it may be truncated. */
>   		list->cd_fan[idx] = (struct capdata_fan) {
>   			.id      = block->data[idx],
> -			.max_rpm = block->data[idx + block->nr],
> -			.min_rpm = block->data[idx + (2 * block->nr)],
> +			.max_rpm = block->data[idx + count],
> +			.min_rpm = block->data[idx + (2 * count)],
>   		};
>   	}
>   
>

Re: [PATCH v2 11/12] platform/x86: lenovo-wmi-helpers: Adopt new WMI API

[Armin Wolf]

Am 09.10.26 um 14:53 schrieb Rong Zhang:

> The new WMI API supports multiple ACPI types by converting them into a
> unified buffer that satisfies alignment and size requirements.
>
> Adopt it to make our life easier.
>
> Note that the Windows WMI-ACPI driver converts all ACPI objects into a
> common buffer format, so adopting the new API intentionally accepts more
> ACPI types as a followup of commit 465dc9da8ff6 ("platform/x86:
> lenovo-wmi-helpers: Convert returned buffer into u32").

Reviewed-by: Armin Wolf <W_Armin@gmx.de>

> Suggested-by: Armin Wolf <W_Armin@gmx.de>
> Link: https://lore.kernel.org/r/f1787927-b655-4321-b9d9-bc12353c72db@gmx.de/
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
>   drivers/platform/x86/lenovo/wmi-helpers.c | 61 ++++++++++++-------------------
>   1 file changed, 23 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/platform/x86/lenovo/wmi-helpers.c b/drivers/platform/x86/lenovo/wmi-helpers.c
> index 8f5766c391eb..687caa226263 100644
> --- a/drivers/platform/x86/lenovo/wmi-helpers.c
> +++ b/drivers/platform/x86/lenovo/wmi-helpers.c
> @@ -24,9 +24,10 @@
>   #include <linux/export.h>
>   #include <linux/module.h>
>   #include <linux/notifier.h>
> -#include <linux/unaligned.h>
>   #include <linux/wmi.h>
>   
> +#include <asm/byteorder.h>
> +
>   #include "wmi-helpers.h"
>   
>   /* Thermal mode notifier chain. */
> @@ -42,49 +43,33 @@ static BLOCKING_NOTIFIER_HEAD(tm_chain_head);
>    * @size: Length of the buffer.
>    * @retval: Pointer for the return value to be assigned.
>    *
> - * Calls wmidev_evaluate_method for Lenovo WMI devices that return an ACPI
> - * integer. Validates the return value type and assigns the value to the
> - * retval pointer.
> + * Calls the specified WMI method for Lenovo WMI devices that return a WMI
> + * integer. Validates the return value and assigns it to @retval when it's not
> + * NULL, otherwise the return value is not validated.
>    *
>    * Return: 0 on success, or an error code.
>    */
>   int lwmi_dev_evaluate_int(struct wmi_device *wdev, u8 instance, u32 method_id,
>   			  unsigned char *buf, size_t size, u32 *retval)
>   {
> -	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> -	struct acpi_buffer input = { size, buf };
> -	acpi_status status;
> -
> -	status = wmidev_evaluate_method(wdev, instance, method_id, &input,
> -					&output);
> -	if (ACPI_FAILURE(status))
> -		return -EIO;
> -
> -	union acpi_object *ret_obj __free(kfree) = output.pointer;
> -
> -	if (retval) {
> -		if (!ret_obj)
> -			return -ENODATA;
> -
> -		switch (ret_obj->type) {
> -		/*
> -		 * The ACPI method may simply return a buffer when a u32
> -		 * is expected. This is valid on Windows as its WMI-ACPI
> -		 * driver converts everything to a common buffer.
> -		 */
> -		case ACPI_TYPE_BUFFER:
> -			if (ret_obj->buffer.length < sizeof(u32))
> -				return -ENXIO;
> -
> -			*retval = get_unaligned_le32(ret_obj->buffer.pointer);
> -			return 0;
> -		case ACPI_TYPE_INTEGER:
> -			*retval = (u32)ret_obj->integer.value;
> -			return 0;
> -		default:
> -			return -ENXIO;
> -		}
> -	}
> +	struct wmi_buffer input = {
> +		.length = size,
> +		.data = buf,
> +	};
> +	struct wmi_buffer output;
> +	int ret;
> +
> +	if (!retval)
> +		return wmidev_invoke_procedure(wdev, instance, method_id, &input);
> +
> +	ret = wmidev_invoke_method(wdev, instance, method_id, &input,
> +				   &output, sizeof(__le32));
> +	if (ret)
> +		return ret;
> +
> +	__le32 *valp __free(kfree) = output.data;
> +
> +	*retval = le32_to_cpu(*valp);
>   
>   	return 0;
>   };
>

Re: [PATCH v2 07/12] platform/x86: lenovo-wmi-capdata: Adopt new WMI API

[Rong Zhang]

Hi Armin,

Thanks for your review.


于 2026年10月10日 GMT+08:00 09:16:48，Armin Wolf <W_Armin@gmx.de> 写道：
> Am 09.10.26 um 14:53 schrieb Rong Zhang:
> 
> > The new WMI API supports multiple ACPI types by converting them into a
> > unified buffer that satisfies alignment and size requirements.
> > 
> > Adopt it to make our life easier.
> > 
> > Note that the new WMI API only accepts a few ACPI types to conform to
> > the behavior of the Windows WMI-ACPI driver. By adopting the new API, we
> > intentionally rejects improper ACPI types instead of silently ignoring
> > them.
> > 
> > Meanwhile, considering that `struct_size(block, data, count * 3)' may
> > overflow when calculating `count * 3', ignore Fan Test Data with count >
> > U8_MAX instead of caping `count'.
> > 
> > Signed-off-by: Rong Zhang <i@rong.moe>
> > ---
> >   drivers/platform/x86/lenovo/wmi-capdata.c | 71 ++++++++++++++-----------------
> >   1 file changed, 32 insertions(+), 39 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c b/drivers/platform/x86/lenovo/wmi-capdata.c
> > index de8044ef68b8..d70fe4504fc5 100644
> > --- a/drivers/platform/x86/lenovo/wmi-capdata.c
> > +++ b/drivers/platform/x86/lenovo/wmi-capdata.c
> > @@ -628,17 +628,19 @@ static int __lwmi_cd_cache(struct lwmi_cd_priv *priv)
> >   	}
> >     	for (idx = 0; idx < priv->list->count; idx++, p += size) {
> > -		union acpi_object *ret_obj __free(kfree) = NULL;
> > +		struct wmi_buffer wbuf;
> > +		int ret;
> >   -		ret_obj = wmidev_block_query(priv->wdev, idx);
> > -		if (!ret_obj)
> > -			return -ENODEV;
> > -
> > -		if (ret_obj->type != ACPI_TYPE_BUFFER ||
> > -		    ret_obj->buffer.length < size)
> > +		ret = wmidev_query_block(priv->wdev, idx, &wbuf, size);
> > +		if (ret == -ENODATA) /* The block is too short, probably stubbed. */
> >   			continue;
> > +		if (ret)
> > +			return ret;
> > +
> > +		/* Capdata 01 is an extension to capdata 00. */
> > +		struct capdata00 *capdata __free(kfree) = wbuf.data;
> >   -		memcpy(p, ret_obj->buffer.pointer, size);
> > +		memcpy(p, capdata, size);
> >   	}
> >     	return 0;
> > @@ -680,43 +682,35 @@ static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
> >    */
> >   static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv)
> >   {
> > +	struct wmi_buffer wbuf;
> >   	struct cd_list *list;
> > -	size_t size;
> > +	int ret, idx;
> >   	u32 count;
> > -	int idx;
> >   -	/* Emit unaligned access to u8 buffer with __packed. */
> >   	struct cd_fan_block {
> >   		u32 nr;
> >   		u32 data[]; /* id[nr], max_rpm[nr], min_rpm[nr] */
> > -	} __packed * block;
> > +	};
> 
> Hi,
> 
> i suggest you keep the __packed here. With this being addressed:

Hmm, may I ask why?

The data itself is never packed, the only use of __packed was to emit
unaligned access.

WMI driver development guide says:

    Since the WMI driver core guarantees that all buffers received from
    a WMI device are aligned on an 8-byte boundary, WMI drivers can
    simply perform a cast.

Thus, the buffer has a better alignment than sizeof(u32).

Thanks,
Rong

> 
> Reviewed-by: Armin Wolf <W_Armin@gmx.de>
> 
> > +
> > +	ret = wmidev_query_block(priv->wdev, 0, &wbuf, sizeof(struct cd_fan_block));
> > +	if (ret == -ENODATA) /* The block is too short, probably stubbed. */
> > +		return 0;
> > +	if (ret)
> > +		return ret;
> >   -	union acpi_object *ret_obj __free(kfree) = wmidev_block_query(priv->wdev, 0);
> > -	if (!ret_obj)
> > -		return -ENODEV;
> > +	struct cd_fan_block *block __free(kfree) = wbuf.data;
> >   -	if (ret_obj->type == ACPI_TYPE_BUFFER) {
> > -		block = (struct cd_fan_block *)ret_obj->buffer.pointer;
> > -		size = ret_obj->buffer.length;
> > +	count = block->nr;
> >   -		count = size >= sizeof(*block) ? block->nr : 0;
> > -		if (size < struct_size(block, data, count * 3)) {
> > -			dev_warn(&priv->wdev->dev,
> > -				 "incomplete fan test data block: %zu < %zu, ignoring\n",
> > -				 size, struct_size(block, data, count * 3));
> > -			count = 0;
> > -		} else if (count > U8_MAX) {
> > -			dev_warn(&priv->wdev->dev,
> > -				 "too many fans reported: %u > %u, truncating\n",
> > -				 count, U8_MAX);
> > -			count = U8_MAX;
> > -		}
> > -	} else {
> > -		/*
> > -		 * This is usually caused by a dummy ACPI method. Do not return an error
> > -		 * as failing to probe this device will result in sub-master device being
> > -		 * unbound. This behavior aligns with lwmi_cd_cache().
> > -		 */
> > +	if (count > U8_MAX) {
> > +		dev_warn(&priv->wdev->dev,
> > +			 "too many fans reported: %u > %u, ignoring\n", count,
> > +			 U8_MAX);
> > +		count = 0;
> > +	} else if (wbuf.length < struct_size(block, data, count * 3)) {
> > +		dev_warn(&priv->wdev->dev,
> > +			 "incomplete fan test data block: %zu < %zu (%u fans), ignoring\n",
> > +			 wbuf.length, struct_size(block, data, count * 3), count);
> >   		count = 0;
> >   	}
> >   @@ -731,11 +725,10 @@ static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv)
> >   	priv->list = list;
> >     	for (idx = 0; idx < count; idx++) {
> > -		/* Do not calculate array index using count, as it may be truncated. */
> >   		list->cd_fan[idx] = (struct capdata_fan) {
> >   			.id      = block->data[idx],
> > -			.max_rpm = block->data[idx + block->nr],
> > -			.min_rpm = block->data[idx + (2 * block->nr)],
> > +			.max_rpm = block->data[idx + count],
> > +			.min_rpm = block->data[idx + (2 * count)],
> >   		};
> >   	}
> >