Re: [PATCH 6/9] platform/x86: lenovo-wmi-capdata: Register component even on WMI error

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Mon, 14 Sep 2026, Rong Zhang 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).
> 
> 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, WMI errors must be non-fatal in order not to break the
> assumptions made by the device component famrework.
> 
> 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.

I'm not sure if using "poison" is really a good word here for this, it 
sounds like you want to inactivate something.

Normally poisoning in kernel means we easy to identify pattern values that 
are supposed to crash the kernel if they're ever used.

-- 
 i.

> Reported-by: Charles <hanker007@gmail.com>
> Link: https://msgid.link/CAKtz0s8UYRQYW_0bh=0TMx47Axm-W-muEay-r3rqUBS1NHMPVw@mail.gmail.com/
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
>  drivers/platform/x86/lenovo/wmi-capdata.c | 71 +++++++++++++++++++++++++++++--
>  1 file changed, 67 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c b/drivers/platform/x86/lenovo/wmi-capdata.c
> index 5e66e6b52720..d4d5e8c97ddb 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>
>  
> @@ -447,7 +450,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
> @@ -460,7 +463,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);						\
> @@ -595,6 +598,66 @@ static void lwmi_cd_debugfs_remove(struct lwmi_cd_priv *priv)
>  
>  /* ======== WMI interface ======== */
>  
> +/**
> + * lwmi_cd_poison() - Poison 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 device for the other functional capdata device.
> + *
> + * Therefore, WMI errors must be non-fatal in order not to break the assumptions
> + * made by the device component famrework, so that the Other Mode device can
> + * provide whatever is functional.
> + *
> + * After poisoning the device, 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_poison(struct lwmi_cd_priv *priv, int err)
> +{
> +	dev_warn(&priv->wdev->dev, "%s %s (%u items) due to error: %d\n",
> +		 priv->initialized ? "clearing" : "poisoning", priv->info->name,
> +		 priv->list ? priv->list->count : 0, err);
> +
> +	/* Simply print the warning message. */
> +	if (!priv->list)
> +		return priv->initialized ? err : 0;
> +
> +	/* Poison the device on initialization errors. */
> +	if (!priv->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.
> @@ -633,7 +696,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_poison(priv, ret);
>  
>  		/* Capdata 01 is an extension to capdata 00. */
>  		struct capdata00 *capdata __free(kfree) = wbuf.data;
> @@ -693,7 +756,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_poison(priv, ret); /* Print the warning message. */
>  
>  	struct cd_fan_block *block __free(kfree) = wbuf.data;
>  
> 
>