Re: [PATCH v3 04/10] leds: pca995x: Fix fwnode handle leaks in error paths
From: Lee Jones
Date: Tue Sep 29 2026 - 05:34:32 EST
--- 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@xxxxxxxxxxxxxxx>
> ---
> 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", ®);
> 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