Re: [PATCH v3 04/10] leds: pca995x: Fix fwnode handle leaks in error paths

[Lee Jones <lee@kernel.org>]

--- checkpatch.pl: clean (0 issues) ---

On Tue, 22 Sep 2026, Nora Schiffer wrote:

> Each entry in led_fwnodes needs to be put as long as no LED device has
> been created for it yet - not just in the creation loop, but also in the
> first loop that iterates over the child nodes.
>
> By clearing entries in led_fwnodes once they have been used, the same
> cleanup loop can be used to handle errors in both loops.
>
> Fixes: 82c5ada1f9d0 ("leds: pca995x: Fix device child node usage in pca995x_probe()")
> Signed-off-by: Nora Schiffer <nora.schiffer@ew.tq-group.com>
> ---
>  drivers/leds/leds-pca995x.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/leds/leds-pca995x.c b/drivers/leds/leds-pca995x.c
> index 8b05b3f12cf38..079ce3d8f1e49 100644
> --- a/drivers/leds/leds-pca995x.c
> +++ b/drivers/leds/leds-pca995x.c
> @@ -123,7 +123,7 @@ static int pca995x_probe(struct i2c_client *client)
>  	const struct pca995x_chipdef *chipdef;
>  	struct pca995x_chip *chip;
>  	struct pca995x_led *led;
> -	int i, j, reg, ret;
> +	int i, reg, ret;
>
>  	chipdef = device_get_match_data(&client->dev);
>
> @@ -144,10 +144,12 @@ static int pca995x_probe(struct i2c_client *client)
>  	device_for_each_child_node_scoped(dev, child) {
>  		ret = fwnode_property_read_u32(child, "reg", &reg);
>  		if (ret)
> -			return ret;
> +			goto err_put_nodes;
>
> -		if (reg < 0 || reg >= chipdef->num_leds || led_fwnodes[reg])
> -			return -EINVAL;
> +		if (reg < 0 || reg >= chipdef->num_leds || led_fwnodes[reg]) {
> +			ret = -EINVAL;
> +			goto err_put_nodes;
> +		}
>
>  		led = &chip->leds[reg];
>  		led_fwnodes[reg] = fwnode_handle_get(child);
> @@ -169,12 +171,12 @@ static int pca995x_probe(struct i2c_client *client)
>  						     &chip->leds[i].ldev,
>  						     &init_data);
>  		if (ret < 0) {
> -			for (j = i; j < chipdef->num_leds; j++)
> -				fwnode_handle_put(led_fwnodes[j]);
> -			return dev_err_probe(dev, ret,
> -					     "Could not register LED %s\n",
> -					     chip->leds[i].ldev.name);
> +			dev_err_probe(dev, ret, "Could not register LED %s\n",
> +				      chip->leds[i].ldev.name);
> +			goto err_put_nodes;
>  		}
> +
> +		led_fwnodes[i] = NULL;

Shouldn't we put this before removing it?

>  	}
>
>  	/* Disable LED all-call address and set normal mode */
> @@ -184,6 +186,12 @@ static int pca995x_probe(struct i2c_client *client)
>
>  	/* IREF Output current value for all LEDn outputs */
>  	return regmap_write(chip->regmap, chipdef->irefall, PCA995X_IREFALL_HALF_CFG);
> +
> +err_put_nodes:
> +	for (i = 0; i < chipdef->num_leds; i++)
> +		fwnode_handle_put(led_fwnodes[i]);
> +
> +	return ret;
>  }
>
>  static const struct i2c_device_id pca995x_id[] = {
> -- 
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> https://www.tq-group.com/
>
>

-- 
Lee Jones