Re: [PATCH v3 3/6] platform/x86: lenovo-wmi-{capdata,other}: Support multiple Capability Data

Re: [PATCH v3 3/6] platform/x86: lenovo-wmi-{capdata,other}: Support multiple Capability Data

[Rong Zhang]

Hi Armin,

On Tue, 2025-11-04 at 21:20 +0100, Armin Wolf wrote:
> Am 31.10.25 um 16:51 schrieb Rong Zhang:
> 
> > The current implementation are heavily bound to capdata01. Rewrite it so
> > that it is suitable to utilize other Capability Data as well.
> > 
> > No functional change intended.
> > 
> > Signed-off-by: Rong Zhang <i@rong.moe>
> > Reviewed-by: Derek J. Clark <derekjohn.clark@gmail.com>
> > Tested-by: Derek J. Clark <derekjohn.clark@gmail.com>
> > ---
> > Changes in v2:
> > - Fix function parameter 'type' not described in 'lwmi_cd_match' (thanks
> >    kernel test bot)
> > ---
> >   drivers/platform/x86/lenovo/wmi-capdata.c | 208 +++++++++++++++++-----
> >   drivers/platform/x86/lenovo/wmi-capdata.h |   7 +-
> >   drivers/platform/x86/lenovo/wmi-other.c   |  27 ++-
> >   3 files changed, 190 insertions(+), 52 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c b/drivers/platform/x86/lenovo/wmi-capdata.c
> > index c5e74b2bfeb3..1f7fc09b7c3f 100644
> > --- a/drivers/platform/x86/lenovo/wmi-capdata.c
> > +++ b/drivers/platform/x86/lenovo/wmi-capdata.c
> > @@ -12,8 +12,13 @@
> >    *
> >    * Copyright (C) 2025 Derek J. Clark <derekjohn.clark@gmail.com>
> >    *   - Initial implementation (formerly named lenovo-wmi-capdata01)
> > + *
> > + * Copyright (C) 2025 Rong Zhang <i@rong.moe>
> > + *   - Unified implementation
> >    */
> >   
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> >   #include <linux/acpi.h>
> >   #include <linux/cleanup.h>
> >   #include <linux/component.h>
> > @@ -36,6 +41,25 @@
> >   #define ACPI_AC_CLASS "ac_adapter"
> >   #define ACPI_AC_NOTIFY_STATUS 0x80
> >   
> > +enum lwmi_cd_type {
> > +	LENOVO_CAPABILITY_DATA_01,
> > +};
> > +
> > +#define LWMI_CD_TABLE_ITEM(_type)		\
> > +	[_type] = {				\
> > +		.guid_string = _type##_GUID,	\
> > +		.name = #_type,			\
> > +		.type = _type,			\
> > +	}
> > +
> > +static const struct lwmi_cd_info {
> > +	const char *guid_string;
> > +	const char *name;
> > +	enum lwmi_cd_type type;
> > +} lwmi_cd_table[] = {
> > +	LWMI_CD_TABLE_ITEM(LENOVO_CAPABILITY_DATA_01),
> > +};
> > +
> >   struct lwmi_cd_priv {
> >   	struct notifier_block acpi_nb; /* ACPI events */
> >   	struct wmi_device *wdev;
> > @@ -44,15 +68,19 @@ struct lwmi_cd_priv {
> >   
> >   struct cd_list {
> >   	struct mutex list_mutex; /* list R/W mutex */
> > +	enum lwmi_cd_type type;
> >   	u8 count;
> > -	struct capdata01 data[];
> > +
> > +	union {
> > +		DECLARE_FLEX_ARRAY(struct capdata01, cd01);
> > +	};
> >   };
> >   
> >   /**
> >    * lwmi_cd_component_bind() - Bind component to master device.
> >    * @cd_dev: Pointer to the lenovo-wmi-capdata driver parent device.
> >    * @om_dev: Pointer to the lenovo-wmi-other driver parent device.
> > - * @data: cd_list object pointer used to return the capability data.
> > + * @data: lwmi_cd_binder object pointer used to return the capability data.
> >    *
> >    * On lenovo-wmi-other's master bind, provide a pointer to the local capdata
> >    * list. This is used to call lwmi_cd*_get_data to look up attribute data
> > @@ -64,9 +92,15 @@ static int lwmi_cd_component_bind(struct device *cd_dev,
> >   				  struct device *om_dev, void *data)
> >   {
> >   	struct lwmi_cd_priv *priv = dev_get_drvdata(cd_dev);
> > -	struct cd_list **cd_list = data;
> > +	struct lwmi_cd_binder *binder = data;
> >   
> > -	*cd_list = priv->list;
> > +	switch (priv->list->type) {
> > +	case LENOVO_CAPABILITY_DATA_01:
> > +		binder->cd01_list = priv->list;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> >   
> >   	return 0;
> >   }
> > @@ -76,30 +110,33 @@ static const struct component_ops lwmi_cd_component_ops = {
> >   };
> >   
> >   /**
> > - * lwmi_cd01_get_data - Get the data of the specified attribute
> > + * 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.
> > - * @output: Pointer to a capdata01 struct to return the data.
> > + * @output: Pointer to a capdata* struct to return the data.
> >    *
> > - * Retrieves the capability data 01 struct pointer for the given
> > - * attribute for its specified thermal mode.
> > + * Retrieves the capability data struct pointer for the given
> > + * attribute.
> >    *
> >    * Return: 0 on success, or -EINVAL.
> >    */
> > -int lwmi_cd01_get_data(struct cd_list *list, u32 attribute_id, struct capdata01 *output)
> > -{
> > -	u8 idx;
> > -
> > -	guard(mutex)(&list->list_mutex);
> > -	for (idx = 0; idx < list->count; idx++) {
> > -		if (list->data[idx].id != attribute_id)
> > -			continue;
> > -		memcpy(output, &list->data[idx], sizeof(list->data[idx]));
> > -		return 0;
> > +#define DEF_LWMI_CDXX_GET_DATA(_cdxx, _cd_type, _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)				\
> > +				continue;							\
> > +			memcpy(output, &list->_cdxx[idx], sizeof(list->_cdxx[idx]));		\
> > +			return 0;								\
> > +		}										\
> > +		return -EINVAL;									\
> >   	}
> >   
> > -	return -EINVAL;
> > -}
> > +DEF_LWMI_CDXX_GET_DATA(cd01, LENOVO_CAPABILITY_DATA_01, struct capdata01);
> >   EXPORT_SYMBOL_NS_GPL(lwmi_cd01_get_data, "LENOVO_WMI_CD");
> >   
> >   /**
> > @@ -112,10 +149,21 @@ EXPORT_SYMBOL_NS_GPL(lwmi_cd01_get_data, "LENOVO_WMI_CD");
> >    */
> >   static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
> >   {
> > +	size_t size;
> >   	int idx;
> > +	void *p;
> > +
> > +	switch (priv->list->type) {
> > +	case LENOVO_CAPABILITY_DATA_01:
> > +		p = &priv->list->cd01[0];
> > +		size = sizeof(priv->list->cd01[0]);
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> >   
> >   	guard(mutex)(&priv->list->list_mutex);
> > -	for (idx = 0; idx < priv->list->count; idx++) {
> > +	for (idx = 0; idx < priv->list->count; idx++, p += size) {
> >   		union acpi_object *ret_obj __free(kfree) = NULL;
> >   
> >   		ret_obj = wmidev_block_query(priv->wdev, idx);
> > @@ -123,11 +171,10 @@ static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
> >   			return -ENODEV;
> >   
> >   		if (ret_obj->type != ACPI_TYPE_BUFFER ||
> > -		    ret_obj->buffer.length < sizeof(priv->list->data[idx]))
> > +		    ret_obj->buffer.length < size)
> >   			continue;
> >   
> > -		memcpy(&priv->list->data[idx], ret_obj->buffer.pointer,
> > -		       ret_obj->buffer.length);
> > +		memcpy(p, ret_obj->buffer.pointer, size);
> >   	}
> >   
> >   	return 0;
> > @@ -136,20 +183,28 @@ static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
> >   /**
> >    * 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)
> > +static int lwmi_cd_alloc(struct lwmi_cd_priv *priv, enum lwmi_cd_type type)
> >   {
> >   	struct cd_list *list;
> >   	size_t list_size;
> >   	int count, ret;
> >   
> >   	count = wmidev_instance_count(priv->wdev);
> > -	list_size = struct_size(list, data, count);
> > +
> > +	switch (type) {
> > +	case LENOVO_CAPABILITY_DATA_01:
> > +		list_size = struct_size(list, cd01, count);
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> >   
> >   	list = devm_kzalloc(&priv->wdev->dev, list_size, GFP_KERNEL);
> >   	if (!list)
> > @@ -159,6 +214,7 @@ static int lwmi_cd_alloc(struct lwmi_cd_priv *priv)
> >   	if (ret)
> >   		return ret;
> >   
> > +	list->type = type;
> >   	list->count = count;
> >   	priv->list = list;
> >   
> > @@ -168,6 +224,7 @@ static int lwmi_cd_alloc(struct lwmi_cd_priv *priv)
> >   /**
> >    * 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
> > @@ -175,11 +232,11 @@ static int lwmi_cd_alloc(struct lwmi_cd_priv *priv)
> >    *
> >    * Return: 0 on success, or an error code.
> >    */
> > -static int lwmi_cd_setup(struct lwmi_cd_priv *priv)
> > +static int lwmi_cd_setup(struct lwmi_cd_priv *priv, enum lwmi_cd_type type)
> >   {
> >   	int ret;
> >   
> > -	ret = lwmi_cd_alloc(priv);
> > +	ret = lwmi_cd_alloc(priv, type);
> >   	if (ret)
> >   		return ret;
> >   
> > @@ -235,9 +292,13 @@ static void lwmi_cd01_unregister(void *data)
> >   
> >   static int lwmi_cd_probe(struct wmi_device *wdev, const void *context)
> >   {
> > +	const struct lwmi_cd_info *info = context;
> >   	struct lwmi_cd_priv *priv;
> >   	int ret;
> >   
> > +	if (!info)
> > +		return -EINVAL;
> > +
> >   	priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL);
> >   	if (!priv)
> >   		return -ENOMEM;
> > @@ -245,21 +306,34 @@ static int lwmi_cd_probe(struct wmi_device *wdev, const void *context)
> >   	priv->wdev = wdev;
> >   	dev_set_drvdata(&wdev->dev, priv);
> >   
> > -	ret = lwmi_cd_setup(priv);
> > +	ret = lwmi_cd_setup(priv, info->type);
> >   	if (ret)
> > -		return ret;
> > +		goto out;
> >   
> > -	priv->acpi_nb.notifier_call = lwmi_cd01_notifier_call;
> > +	if (info->type == LENOVO_CAPABILITY_DATA_01) {
> > +		priv->acpi_nb.notifier_call = lwmi_cd01_notifier_call;
> >   
> > -	ret = register_acpi_notifier(&priv->acpi_nb);
> > -	if (ret)
> > -		return ret;
> > +		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)
> > -		return ret;
> > +		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);
> >   
> > -	return component_add(&wdev->dev, &lwmi_cd_component_ops);
> > +out:
> > +	if (ret) {
> > +		dev_err(&wdev->dev, "failed to register %s: %d\n",
> > +			info->name, ret);
> > +	} else {
> > +		dev_info(&wdev->dev, "registered %s with %u items\n",
> > +			 info->name, priv->list->count);
> > +	}
> > +	return ret;
> >   }
> >   
> >   static void lwmi_cd_remove(struct wmi_device *wdev)
> > @@ -267,8 +341,12 @@ static void lwmi_cd_remove(struct wmi_device *wdev)
> >   	component_del(&wdev->dev, &lwmi_cd_component_ops);
> >   }
> >   
> > +#define LWMI_CD_WDEV_ID(_type)				\
> > +	.guid_string = _type##_GUID,			\
> > +	.context = &lwmi_cd_table[_type]
> > +
> >   static const struct wmi_device_id lwmi_cd_id_table[] = {
> > -	{ LENOVO_CAPABILITY_DATA_01_GUID, NULL },
> > +	{ LWMI_CD_WDEV_ID(LENOVO_CAPABILITY_DATA_01) },
> >   	{}
> >   };
> >   
> > @@ -284,21 +362,61 @@ static struct wmi_driver lwmi_cd_driver = {
> >   };
> >   
> >   /**
> > - * lwmi_cd01_match() - Match rule for the master driver.
> > - * @dev: Pointer to the capability data 01 parent device.
> > - * @data: Unused void pointer for passing match criteria.
> > + * lwmi_cd_match() - Match rule for the master driver.
> > + * @dev: Pointer to the capability data parent device.
> > + * @type: Pointer to capability data type (enum lwmi_cd_type *) to match.
> >    *
> >    * Return: int.
> >    */
> > -int lwmi_cd01_match(struct device *dev, void *data)
> > +static int lwmi_cd_match(struct device *dev, void *type)
> > +{
> > +	struct lwmi_cd_priv *priv;
> > +
> > +	if (dev->driver != &lwmi_cd_driver.driver)
> > +		return false;
> > +
> > +	priv = dev_get_drvdata(dev);
> > +	return priv->list->type == *(enum lwmi_cd_type *)type;
> > +}
> > +
> > +/**
> > + * lwmi_cd_match_add_all() - Add all match rule for the master driver.
> > + * @master: Pointer to the master device.
> > + * @matchptr: Pointer to the returned component_match pointer.
> > + *
> > + * Adds all component matches to the list stored in @matchptr for the @master
> > + * device. @matchptr must be initialized to NULL. This matches all available
> > + * capdata types on the machine.
> > + */
> > +void lwmi_cd_match_add_all(struct device *master, struct component_match **matchptr)
> >   {
> > -	return dev->driver == &lwmi_cd_driver.driver;
> > +	int i;
> > +
> > +	if (WARN_ON(*matchptr))
> > +		return;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(lwmi_cd_table); i++) {
> > +		if (!lwmi_cd_table[i].guid_string ||
> > +		    !wmi_has_guid(lwmi_cd_table[i].guid_string))
> > +			continue;
> 
> I am still not happy about this. AFAIK as soon as the ordinary capdata WMI devices are bound together,
> the firmware tells you whether or not the additional fan data WMI device is available. Maybe you can do
> something like this:
> 
> 1. Bind both capdata WMI devices as usual.
> 2. Check if a fan data WMI device is available (you can use a DMI-based quirk list for devices were
>     the firmware reports invalid data).
> 3. Register an additional component that binds to the fan data WMI device.
> 4. Bind both the additional component and the component registered by the fan data WMI device.
> 5. Register the hwmon device with additional fan data.
> 
> If the fan data WMI device is not available, you can simply skip steps 3 and 4 and simply
> register the hwmon device with any additional fan data.
> 
> What do you think?

I tried your approach. I looks pretty well except for step 4:

   debugfs: 'DC2A8805-3A8C-41BA-A6F7-092E0089CD3B-21' already exists in 'device_component'

Moreover, component_[un]bind_all() calls __aggregate_find() with ops ==
NULL, which implies that it can't really distinguish the two aggregate
devices we have. Thus, we are rely on the insertion sequence of
aggregate_devices (see component_master_add_with_match()) to make it
just work. Pseudo code:

   lwmi_other_probe()
   {
   	component_match_add(..., lwmi_cd00_match, ...);
   	component_match_add(..., lwmi_cd01_match, ...);
   	component_master_add_with_match(..., &ops1, ...);
   	component_match_add(..., lwmi_cd_fan_match, ...);
   	component_master_add_with_match(..., &ops2, ...);
   }
   
   lwmi_other_remove()
   {
   	/* This just works (TM). */
   	component_master_del(..., &ops2); /* unbinds cd_fan */
   	component_master_del(..., &ops1); /* unbinds cd00/01 */
   
   	/* WARNING: at drivers/base/component.c:589 */
   	/*
   	component_master_del(..., &ops1); /* unbinds cd_fan!!! */
   	component_master_del(..., &ops2); /* unbinds cd_fan!!! */
   	*/
   }

It seems that the component framework is not prepared to allow multi-
aggregation master device.

Since we are talking about the component framework: all efforts we made
here is to work around one of its limitations -- all components must be
found or else it won't bring up the aggregate device. Do you think
allowing partially bound aggregate device a good idea? E.g., adding a
flag, indicating the master device opts in to such behavior, to the
definition of struct component_master_ops. I can prepare a separate
patch for that.

CC'ing maintainers and lists of driver core and component framework.

> Thanks,
> Armin Wolf

Thanks,
Rong

> > +
> > +		component_match_add(master, matchptr, lwmi_cd_match,
> > +				    (void *)&lwmi_cd_table[i].type);
> > +		if (IS_ERR(matchptr))
> > +			return;
> > +	}
> > +
> > +	if (!*matchptr) {
> > +		pr_warn("a master driver requested capability data, but nothing is available\n");
> > +		*matchptr = ERR_PTR(-ENODEV);
> > +	}
> >   }
> > -EXPORT_SYMBOL_NS_GPL(lwmi_cd01_match, "LENOVO_WMI_CD");
> > +EXPORT_SYMBOL_NS_GPL(lwmi_cd_match_add_all, "LENOVO_WMI_CD");
> >   
> >   module_wmi_driver(lwmi_cd_driver);
> >   
> >   MODULE_DEVICE_TABLE(wmi, lwmi_cd_id_table);
> >   MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>");
> > +MODULE_AUTHOR("Rong Zhang <i@rong.moe>");
> >   MODULE_DESCRIPTION("Lenovo Capability Data WMI Driver");
> >   MODULE_LICENSE("GPL");
> > diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
> > index 2a4746e38ad4..1e5fce7836cb 100644
> > --- a/drivers/platform/x86/lenovo/wmi-capdata.h
> > +++ b/drivers/platform/x86/lenovo/wmi-capdata.h
> > @@ -7,6 +7,7 @@
> >   
> >   #include <linux/types.h>
> >   
> > +struct component_match;
> >   struct device;
> >   struct cd_list;
> >   
> > @@ -19,7 +20,11 @@ struct capdata01 {
> >   	u32 max_value;
> >   };
> >   
> > +struct lwmi_cd_binder {
> > +	struct cd_list *cd01_list;
> > +};
> > +
> >   int lwmi_cd01_get_data(struct cd_list *list, u32 attribute_id, struct capdata01 *output);
> > -int lwmi_cd01_match(struct device *dev, void *data);
> > +void lwmi_cd_match_add_all(struct device *master, struct component_match **matchptr);
> >   
> >   #endif /* !_LENOVO_WMI_CAPDATA_H_ */
> > diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> > index c6dc1b4cff84..20c6ff0be37a 100644
> > --- a/drivers/platform/x86/lenovo/wmi-other.c
> > +++ b/drivers/platform/x86/lenovo/wmi-other.c
> > @@ -579,14 +579,14 @@ static void lwmi_om_fw_attr_remove(struct lwmi_om_priv *priv)
> >   static int lwmi_om_master_bind(struct device *dev)
> >   {
> >   	struct lwmi_om_priv *priv = dev_get_drvdata(dev);
> > -	struct cd_list *tmp_list;
> > +	struct lwmi_cd_binder binder = { 0 };
> >   	int ret;
> >   
> > -	ret = component_bind_all(dev, &tmp_list);
> > +	ret = component_bind_all(dev, &binder);
> >   	if (ret)
> >   		return ret;
> >   
> > -	priv->cd01_list = tmp_list;
> > +	priv->cd01_list = binder.cd01_list;
> >   	if (!priv->cd01_list)
> >   		return -ENODEV;
> >   
> > @@ -618,6 +618,7 @@ static int lwmi_other_probe(struct wmi_device *wdev, const void *context)
> >   {
> >   	struct component_match *master_match = NULL;
> >   	struct lwmi_om_priv *priv;
> > +	int ret;
> >   
> >   	priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL);
> >   	if (!priv)
> > @@ -626,12 +627,26 @@ static int lwmi_other_probe(struct wmi_device *wdev, const void *context)
> >   	priv->wdev = wdev;
> >   	dev_set_drvdata(&wdev->dev, priv);
> >   
> > -	component_match_add(&wdev->dev, &master_match, lwmi_cd01_match, NULL);
> > +	lwmi_cd_match_add_all(&wdev->dev, &master_match);
> >   	if (IS_ERR(master_match))
> >   		return PTR_ERR(master_match);
> >   
> > -	return component_master_add_with_match(&wdev->dev, &lwmi_om_master_ops,
> > -					       master_match);
> > +	ret = component_master_add_with_match(&wdev->dev, &lwmi_om_master_ops,
> > +					      master_match);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (likely(component_master_is_bound(&wdev->dev, &lwmi_om_master_ops)))
> > +		return 0;
> > +
> > +	/*
> > +	 * The bind callbacks of both master and components were never called in
> > +	 * this case - this driver won't work at all. Failing...
> > +	 */
> > +	dev_err(&wdev->dev, "unbound master; is any component failing to be probed?");
> > +
> > +	component_master_del(&wdev->dev, &lwmi_om_master_ops);
> > +	return -EXDEV;
> >   }
> >   
> >   static void lwmi_other_remove(struct wmi_device *wdev)

Re: [PATCH v3 3/6] platform/x86: lenovo-wmi-{capdata, other}: Support multiple Capability Data

[Armin Wolf]

Am 06.11.25 um 13:36 schrieb Rong Zhang:

> Hi Armin,
>
> On Tue, 2025-11-04 at 21:20 +0100, Armin Wolf wrote:
>> Am 31.10.25 um 16:51 schrieb Rong Zhang:
>>
>>> The current implementation are heavily bound to capdata01. Rewrite it so
>>> that it is suitable to utilize other Capability Data as well.
>>>
>>> No functional change intended.
>>>
>>> Signed-off-by: Rong Zhang <i@rong.moe>
>>> Reviewed-by: Derek J. Clark <derekjohn.clark@gmail.com>
>>> Tested-by: Derek J. Clark <derekjohn.clark@gmail.com>
>>> ---
>>> Changes in v2:
>>> - Fix function parameter 'type' not described in 'lwmi_cd_match' (thanks
>>>     kernel test bot)
>>> ---
>>>    drivers/platform/x86/lenovo/wmi-capdata.c | 208 +++++++++++++++++-----
>>>    drivers/platform/x86/lenovo/wmi-capdata.h |   7 +-
>>>    drivers/platform/x86/lenovo/wmi-other.c   |  27 ++-
>>>    3 files changed, 190 insertions(+), 52 deletions(-)
>>>
>>> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c b/drivers/platform/x86/lenovo/wmi-capdata.c
>>> index c5e74b2bfeb3..1f7fc09b7c3f 100644
>>> --- a/drivers/platform/x86/lenovo/wmi-capdata.c
>>> +++ b/drivers/platform/x86/lenovo/wmi-capdata.c
>>> @@ -12,8 +12,13 @@
>>>     *
>>>     * Copyright (C) 2025 Derek J. Clark <derekjohn.clark@gmail.com>
>>>     *   - Initial implementation (formerly named lenovo-wmi-capdata01)
>>> + *
>>> + * Copyright (C) 2025 Rong Zhang <i@rong.moe>
>>> + *   - Unified implementation
>>>     */
>>>    
>>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>> +
>>>    #include <linux/acpi.h>
>>>    #include <linux/cleanup.h>
>>>    #include <linux/component.h>
>>> @@ -36,6 +41,25 @@
>>>    #define ACPI_AC_CLASS "ac_adapter"
>>>    #define ACPI_AC_NOTIFY_STATUS 0x80
>>>    
>>> +enum lwmi_cd_type {
>>> +	LENOVO_CAPABILITY_DATA_01,
>>> +};
>>> +
>>> +#define LWMI_CD_TABLE_ITEM(_type)		\
>>> +	[_type] = {				\
>>> +		.guid_string = _type##_GUID,	\
>>> +		.name = #_type,			\
>>> +		.type = _type,			\
>>> +	}
>>> +
>>> +static const struct lwmi_cd_info {
>>> +	const char *guid_string;
>>> +	const char *name;
>>> +	enum lwmi_cd_type type;
>>> +} lwmi_cd_table[] = {
>>> +	LWMI_CD_TABLE_ITEM(LENOVO_CAPABILITY_DATA_01),
>>> +};
>>> +
>>>    struct lwmi_cd_priv {
>>>    	struct notifier_block acpi_nb; /* ACPI events */
>>>    	struct wmi_device *wdev;
>>> @@ -44,15 +68,19 @@ struct lwmi_cd_priv {
>>>    
>>>    struct cd_list {
>>>    	struct mutex list_mutex; /* list R/W mutex */
>>> +	enum lwmi_cd_type type;
>>>    	u8 count;
>>> -	struct capdata01 data[];
>>> +
>>> +	union {
>>> +		DECLARE_FLEX_ARRAY(struct capdata01, cd01);
>>> +	};
>>>    };
>>>    
>>>    /**
>>>     * lwmi_cd_component_bind() - Bind component to master device.
>>>     * @cd_dev: Pointer to the lenovo-wmi-capdata driver parent device.
>>>     * @om_dev: Pointer to the lenovo-wmi-other driver parent device.
>>> - * @data: cd_list object pointer used to return the capability data.
>>> + * @data: lwmi_cd_binder object pointer used to return the capability data.
>>>     *
>>>     * On lenovo-wmi-other's master bind, provide a pointer to the local capdata
>>>     * list. This is used to call lwmi_cd*_get_data to look up attribute data
>>> @@ -64,9 +92,15 @@ static int lwmi_cd_component_bind(struct device *cd_dev,
>>>    				  struct device *om_dev, void *data)
>>>    {
>>>    	struct lwmi_cd_priv *priv = dev_get_drvdata(cd_dev);
>>> -	struct cd_list **cd_list = data;
>>> +	struct lwmi_cd_binder *binder = data;
>>>    
>>> -	*cd_list = priv->list;
>>> +	switch (priv->list->type) {
>>> +	case LENOVO_CAPABILITY_DATA_01:
>>> +		binder->cd01_list = priv->list;
>>> +		break;
>>> +	default:
>>> +		return -EINVAL;
>>> +	}
>>>    
>>>    	return 0;
>>>    }
>>> @@ -76,30 +110,33 @@ static const struct component_ops lwmi_cd_component_ops = {
>>>    };
>>>    
>>>    /**
>>> - * lwmi_cd01_get_data - Get the data of the specified attribute
>>> + * 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.
>>> - * @output: Pointer to a capdata01 struct to return the data.
>>> + * @output: Pointer to a capdata* struct to return the data.
>>>     *
>>> - * Retrieves the capability data 01 struct pointer for the given
>>> - * attribute for its specified thermal mode.
>>> + * Retrieves the capability data struct pointer for the given
>>> + * attribute.
>>>     *
>>>     * Return: 0 on success, or -EINVAL.
>>>     */
>>> -int lwmi_cd01_get_data(struct cd_list *list, u32 attribute_id, struct capdata01 *output)
>>> -{
>>> -	u8 idx;
>>> -
>>> -	guard(mutex)(&list->list_mutex);
>>> -	for (idx = 0; idx < list->count; idx++) {
>>> -		if (list->data[idx].id != attribute_id)
>>> -			continue;
>>> -		memcpy(output, &list->data[idx], sizeof(list->data[idx]));
>>> -		return 0;
>>> +#define DEF_LWMI_CDXX_GET_DATA(_cdxx, _cd_type, _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)				\
>>> +				continue;							\
>>> +			memcpy(output, &list->_cdxx[idx], sizeof(list->_cdxx[idx]));		\
>>> +			return 0;								\
>>> +		}										\
>>> +		return -EINVAL;									\
>>>    	}
>>>    
>>> -	return -EINVAL;
>>> -}
>>> +DEF_LWMI_CDXX_GET_DATA(cd01, LENOVO_CAPABILITY_DATA_01, struct capdata01);
>>>    EXPORT_SYMBOL_NS_GPL(lwmi_cd01_get_data, "LENOVO_WMI_CD");
>>>    
>>>    /**
>>> @@ -112,10 +149,21 @@ EXPORT_SYMBOL_NS_GPL(lwmi_cd01_get_data, "LENOVO_WMI_CD");
>>>     */
>>>    static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
>>>    {
>>> +	size_t size;
>>>    	int idx;
>>> +	void *p;
>>> +
>>> +	switch (priv->list->type) {
>>> +	case LENOVO_CAPABILITY_DATA_01:
>>> +		p = &priv->list->cd01[0];
>>> +		size = sizeof(priv->list->cd01[0]);
>>> +		break;
>>> +	default:
>>> +		return -EINVAL;
>>> +	}
>>>    
>>>    	guard(mutex)(&priv->list->list_mutex);
>>> -	for (idx = 0; idx < priv->list->count; idx++) {
>>> +	for (idx = 0; idx < priv->list->count; idx++, p += size) {
>>>    		union acpi_object *ret_obj __free(kfree) = NULL;
>>>    
>>>    		ret_obj = wmidev_block_query(priv->wdev, idx);
>>> @@ -123,11 +171,10 @@ static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
>>>    			return -ENODEV;
>>>    
>>>    		if (ret_obj->type != ACPI_TYPE_BUFFER ||
>>> -		    ret_obj->buffer.length < sizeof(priv->list->data[idx]))
>>> +		    ret_obj->buffer.length < size)
>>>    			continue;
>>>    
>>> -		memcpy(&priv->list->data[idx], ret_obj->buffer.pointer,
>>> -		       ret_obj->buffer.length);
>>> +		memcpy(p, ret_obj->buffer.pointer, size);
>>>    	}
>>>    
>>>    	return 0;
>>> @@ -136,20 +183,28 @@ static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
>>>    /**
>>>     * 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)
>>> +static int lwmi_cd_alloc(struct lwmi_cd_priv *priv, enum lwmi_cd_type type)
>>>    {
>>>    	struct cd_list *list;
>>>    	size_t list_size;
>>>    	int count, ret;
>>>    
>>>    	count = wmidev_instance_count(priv->wdev);
>>> -	list_size = struct_size(list, data, count);
>>> +
>>> +	switch (type) {
>>> +	case LENOVO_CAPABILITY_DATA_01:
>>> +		list_size = struct_size(list, cd01, count);
>>> +		break;
>>> +	default:
>>> +		return -EINVAL;
>>> +	}
>>>    
>>>    	list = devm_kzalloc(&priv->wdev->dev, list_size, GFP_KERNEL);
>>>    	if (!list)
>>> @@ -159,6 +214,7 @@ static int lwmi_cd_alloc(struct lwmi_cd_priv *priv)
>>>    	if (ret)
>>>    		return ret;
>>>    
>>> +	list->type = type;
>>>    	list->count = count;
>>>    	priv->list = list;
>>>    
>>> @@ -168,6 +224,7 @@ static int lwmi_cd_alloc(struct lwmi_cd_priv *priv)
>>>    /**
>>>     * 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
>>> @@ -175,11 +232,11 @@ static int lwmi_cd_alloc(struct lwmi_cd_priv *priv)
>>>     *
>>>     * Return: 0 on success, or an error code.
>>>     */
>>> -static int lwmi_cd_setup(struct lwmi_cd_priv *priv)
>>> +static int lwmi_cd_setup(struct lwmi_cd_priv *priv, enum lwmi_cd_type type)
>>>    {
>>>    	int ret;
>>>    
>>> -	ret = lwmi_cd_alloc(priv);
>>> +	ret = lwmi_cd_alloc(priv, type);
>>>    	if (ret)
>>>    		return ret;
>>>    
>>> @@ -235,9 +292,13 @@ static void lwmi_cd01_unregister(void *data)
>>>    
>>>    static int lwmi_cd_probe(struct wmi_device *wdev, const void *context)
>>>    {
>>> +	const struct lwmi_cd_info *info = context;
>>>    	struct lwmi_cd_priv *priv;
>>>    	int ret;
>>>    
>>> +	if (!info)
>>> +		return -EINVAL;
>>> +
>>>    	priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL);
>>>    	if (!priv)
>>>    		return -ENOMEM;
>>> @@ -245,21 +306,34 @@ static int lwmi_cd_probe(struct wmi_device *wdev, const void *context)
>>>    	priv->wdev = wdev;
>>>    	dev_set_drvdata(&wdev->dev, priv);
>>>    
>>> -	ret = lwmi_cd_setup(priv);
>>> +	ret = lwmi_cd_setup(priv, info->type);
>>>    	if (ret)
>>> -		return ret;
>>> +		goto out;
>>>    
>>> -	priv->acpi_nb.notifier_call = lwmi_cd01_notifier_call;
>>> +	if (info->type == LENOVO_CAPABILITY_DATA_01) {
>>> +		priv->acpi_nb.notifier_call = lwmi_cd01_notifier_call;
>>>    
>>> -	ret = register_acpi_notifier(&priv->acpi_nb);
>>> -	if (ret)
>>> -		return ret;
>>> +		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)
>>> -		return ret;
>>> +		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);
>>>    
>>> -	return component_add(&wdev->dev, &lwmi_cd_component_ops);
>>> +out:
>>> +	if (ret) {
>>> +		dev_err(&wdev->dev, "failed to register %s: %d\n",
>>> +			info->name, ret);
>>> +	} else {
>>> +		dev_info(&wdev->dev, "registered %s with %u items\n",
>>> +			 info->name, priv->list->count);
>>> +	}
>>> +	return ret;
>>>    }
>>>    
>>>    static void lwmi_cd_remove(struct wmi_device *wdev)
>>> @@ -267,8 +341,12 @@ static void lwmi_cd_remove(struct wmi_device *wdev)
>>>    	component_del(&wdev->dev, &lwmi_cd_component_ops);
>>>    }
>>>    
>>> +#define LWMI_CD_WDEV_ID(_type)				\
>>> +	.guid_string = _type##_GUID,			\
>>> +	.context = &lwmi_cd_table[_type]
>>> +
>>>    static const struct wmi_device_id lwmi_cd_id_table[] = {
>>> -	{ LENOVO_CAPABILITY_DATA_01_GUID, NULL },
>>> +	{ LWMI_CD_WDEV_ID(LENOVO_CAPABILITY_DATA_01) },
>>>    	{}
>>>    };
>>>    
>>> @@ -284,21 +362,61 @@ static struct wmi_driver lwmi_cd_driver = {
>>>    };
>>>    
>>>    /**
>>> - * lwmi_cd01_match() - Match rule for the master driver.
>>> - * @dev: Pointer to the capability data 01 parent device.
>>> - * @data: Unused void pointer for passing match criteria.
>>> + * lwmi_cd_match() - Match rule for the master driver.
>>> + * @dev: Pointer to the capability data parent device.
>>> + * @type: Pointer to capability data type (enum lwmi_cd_type *) to match.
>>>     *
>>>     * Return: int.
>>>     */
>>> -int lwmi_cd01_match(struct device *dev, void *data)
>>> +static int lwmi_cd_match(struct device *dev, void *type)
>>> +{
>>> +	struct lwmi_cd_priv *priv;
>>> +
>>> +	if (dev->driver != &lwmi_cd_driver.driver)
>>> +		return false;
>>> +
>>> +	priv = dev_get_drvdata(dev);
>>> +	return priv->list->type == *(enum lwmi_cd_type *)type;
>>> +}
>>> +
>>> +/**
>>> + * lwmi_cd_match_add_all() - Add all match rule for the master driver.
>>> + * @master: Pointer to the master device.
>>> + * @matchptr: Pointer to the returned component_match pointer.
>>> + *
>>> + * Adds all component matches to the list stored in @matchptr for the @master
>>> + * device. @matchptr must be initialized to NULL. This matches all available
>>> + * capdata types on the machine.
>>> + */
>>> +void lwmi_cd_match_add_all(struct device *master, struct component_match **matchptr)
>>>    {
>>> -	return dev->driver == &lwmi_cd_driver.driver;
>>> +	int i;
>>> +
>>> +	if (WARN_ON(*matchptr))
>>> +		return;
>>> +
>>> +	for (i = 0; i < ARRAY_SIZE(lwmi_cd_table); i++) {
>>> +		if (!lwmi_cd_table[i].guid_string ||
>>> +		    !wmi_has_guid(lwmi_cd_table[i].guid_string))
>>> +			continue;
>> I am still not happy about this. AFAIK as soon as the ordinary capdata WMI devices are bound together,
>> the firmware tells you whether or not the additional fan data WMI device is available. Maybe you can do
>> something like this:
>>
>> 1. Bind both capdata WMI devices as usual.
>> 2. Check if a fan data WMI device is available (you can use a DMI-based quirk list for devices were
>>      the firmware reports invalid data).
>> 3. Register an additional component that binds to the fan data WMI device.
>> 4. Bind both the additional component and the component registered by the fan data WMI device.
>> 5. Register the hwmon device with additional fan data.
>>
>> If the fan data WMI device is not available, you can simply skip steps 3 and 4 and simply
>> register the hwmon device with any additional fan data.
>>
>> What do you think?
> I tried your approach. I looks pretty well except for step 4:
>
>     debugfs: 'DC2A8805-3A8C-41BA-A6F7-092E0089CD3B-21' already exists in 'device_component'
>
> Moreover, component_[un]bind_all() calls __aggregate_find() with ops ==
> NULL, which implies that it can't really distinguish the two aggregate
> devices we have. Thus, we are rely on the insertion sequence of
> aggregate_devices (see component_master_add_with_match()) to make it
> just work. Pseudo code:
>
>     lwmi_other_probe()
>     {
>     	component_match_add(..., lwmi_cd00_match, ...);
>     	component_match_add(..., lwmi_cd01_match, ...);
>     	component_master_add_with_match(..., &ops1, ...);
>     	component_match_add(..., lwmi_cd_fan_match, ...);
>     	component_master_add_with_match(..., &ops2, ...);
>     }
>     
>     lwmi_other_remove()
>     {
>     	/* This just works (TM). */
>     	component_master_del(..., &ops2); /* unbinds cd_fan */
>     	component_master_del(..., &ops1); /* unbinds cd00/01 */
>     
>     	/* WARNING: at drivers/base/component.c:589 */
>     	/*
>     	component_master_del(..., &ops1); /* unbinds cd_fan!!! */
>     	component_master_del(..., &ops2); /* unbinds cd_fan!!! */
>     	*/
>     }
>
> It seems that the component framework is not prepared to allow multi-
> aggregation master device.
>
> Since we are talking about the component framework: all efforts we made
> here is to work around one of its limitations -- all components must be
> found or else it won't bring up the aggregate device. Do you think
> allowing partially bound aggregate device a good idea? E.g., adding a
> flag, indicating the master device opts in to such behavior, to the
> definition of struct component_master_ops. I can prepare a separate
> patch for that.
>
> CC'ing maintainers and lists of driver core and component framework.

Oh my, i did not know about this limitation. I think adding support for
optional components will be quite complicated to get right, inside i propose
the following:

The current component master is lenovo-wmi-other, with capdata01 and capdata00
being its components (correct me if i am wrong). Instead of registering an additional
component master on lenovo-wmi-other, how about registering it on capdata00?

Basically when probing, capdata00 will register the component for lenovo-wmi-other
and then check whether attribute 0x04050000 indicates support for LENOVO_FAN_TEST_DATA
(or a DMI check overrides this). Is yes then capdata00 registers an additional component
master, otherwise the hwmon device is created right away.
The driver for LENOVO_FAN_TEST_DATA registers a component for capdata00. As soon as the
component master from capdata00 is bound to the component from LENOVO_FAN_TEST_DATA, a
hwmon device is created.

Do you thing this would be more feasible than extending the component framework itself?

Thanks,
Armin Wolf

>> Thanks,
>> Armin Wolf
> Thanks,
> Rong
>
>>> +
>>> +		component_match_add(master, matchptr, lwmi_cd_match,
>>> +				    (void *)&lwmi_cd_table[i].type);
>>> +		if (IS_ERR(matchptr))
>>> +			return;
>>> +	}
>>> +
>>> +	if (!*matchptr) {
>>> +		pr_warn("a master driver requested capability data, but nothing is available\n");
>>> +		*matchptr = ERR_PTR(-ENODEV);
>>> +	}
>>>    }
>>> -EXPORT_SYMBOL_NS_GPL(lwmi_cd01_match, "LENOVO_WMI_CD");
>>> +EXPORT_SYMBOL_NS_GPL(lwmi_cd_match_add_all, "LENOVO_WMI_CD");
>>>    
>>>    module_wmi_driver(lwmi_cd_driver);
>>>    
>>>    MODULE_DEVICE_TABLE(wmi, lwmi_cd_id_table);
>>>    MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>");
>>> +MODULE_AUTHOR("Rong Zhang <i@rong.moe>");
>>>    MODULE_DESCRIPTION("Lenovo Capability Data WMI Driver");
>>>    MODULE_LICENSE("GPL");
>>> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
>>> index 2a4746e38ad4..1e5fce7836cb 100644
>>> --- a/drivers/platform/x86/lenovo/wmi-capdata.h
>>> +++ b/drivers/platform/x86/lenovo/wmi-capdata.h
>>> @@ -7,6 +7,7 @@
>>>    
>>>    #include <linux/types.h>
>>>    
>>> +struct component_match;
>>>    struct device;
>>>    struct cd_list;
>>>    
>>> @@ -19,7 +20,11 @@ struct capdata01 {
>>>    	u32 max_value;
>>>    };
>>>    
>>> +struct lwmi_cd_binder {
>>> +	struct cd_list *cd01_list;
>>> +};
>>> +
>>>    int lwmi_cd01_get_data(struct cd_list *list, u32 attribute_id, struct capdata01 *output);
>>> -int lwmi_cd01_match(struct device *dev, void *data);
>>> +void lwmi_cd_match_add_all(struct device *master, struct component_match **matchptr);
>>>    
>>>    #endif /* !_LENOVO_WMI_CAPDATA_H_ */
>>> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
>>> index c6dc1b4cff84..20c6ff0be37a 100644
>>> --- a/drivers/platform/x86/lenovo/wmi-other.c
>>> +++ b/drivers/platform/x86/lenovo/wmi-other.c
>>> @@ -579,14 +579,14 @@ static void lwmi_om_fw_attr_remove(struct lwmi_om_priv *priv)
>>>    static int lwmi_om_master_bind(struct device *dev)
>>>    {
>>>    	struct lwmi_om_priv *priv = dev_get_drvdata(dev);
>>> -	struct cd_list *tmp_list;
>>> +	struct lwmi_cd_binder binder = { 0 };
>>>    	int ret;
>>>    
>>> -	ret = component_bind_all(dev, &tmp_list);
>>> +	ret = component_bind_all(dev, &binder);
>>>    	if (ret)
>>>    		return ret;
>>>    
>>> -	priv->cd01_list = tmp_list;
>>> +	priv->cd01_list = binder.cd01_list;
>>>    	if (!priv->cd01_list)
>>>    		return -ENODEV;
>>>    
>>> @@ -618,6 +618,7 @@ static int lwmi_other_probe(struct wmi_device *wdev, const void *context)
>>>    {
>>>    	struct component_match *master_match = NULL;
>>>    	struct lwmi_om_priv *priv;
>>> +	int ret;
>>>    
>>>    	priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL);
>>>    	if (!priv)
>>> @@ -626,12 +627,26 @@ static int lwmi_other_probe(struct wmi_device *wdev, const void *context)
>>>    	priv->wdev = wdev;
>>>    	dev_set_drvdata(&wdev->dev, priv);
>>>    
>>> -	component_match_add(&wdev->dev, &master_match, lwmi_cd01_match, NULL);
>>> +	lwmi_cd_match_add_all(&wdev->dev, &master_match);
>>>    	if (IS_ERR(master_match))
>>>    		return PTR_ERR(master_match);
>>>    
>>> -	return component_master_add_with_match(&wdev->dev, &lwmi_om_master_ops,
>>> -					       master_match);
>>> +	ret = component_master_add_with_match(&wdev->dev, &lwmi_om_master_ops,
>>> +					      master_match);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	if (likely(component_master_is_bound(&wdev->dev, &lwmi_om_master_ops)))
>>> +		return 0;
>>> +
>>> +	/*
>>> +	 * The bind callbacks of both master and components were never called in
>>> +	 * this case - this driver won't work at all. Failing...
>>> +	 */
>>> +	dev_err(&wdev->dev, "unbound master; is any component failing to be probed?");
>>> +
>>> +	component_master_del(&wdev->dev, &lwmi_om_master_ops);
>>> +	return -EXDEV;
>>>    }
>>>    
>>>    static void lwmi_other_remove(struct wmi_device *wdev)