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@xxxxxxxx> 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@xxxxxxxx>
> > ---
> > 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);
> >