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