Re: [PATCH v2] leds: blinkm: Clean up sysfs group on LED registration failure

[Lee Jones <lee@kernel.org>]

On Fri, 25 Sep 2026, Myeonghun Pak wrote:

> blinkm_probe() creates the BlinkM sysfs group before registering the LED
> class devices. If multicolor sub-LED allocation fails, probe returns
> without removing the group. If multicolor LED registration fails, the
> helper removes the group but returns success, leaving the device bound
> without an LED class device.
>
> Keep LED class device rollback in the registration helpers, but remove
> the BlinkM sysfs group in blinkm_probe() when either registration mode
> fails. Propagate the multicolor registration error so probe can perform
> this cleanup and report the failure.
>
> Fixes: 56e8c56c9af0 ("leds: Add multicolor support to BlinkM LED driver")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Co-developed-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> ---
> Changes in v2:
> - Move sysfs group cleanup to blinkm_probe() for both LED registration modes.
> - Propagate multicolor LED registration errors to blinkm_probe().
>
>  drivers/leds/leds-blinkm.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/leds/leds-blinkm.c b/drivers/leds/leds-blinkm.c
> --- a/drivers/leds/leds-blinkm.c
> +++ b/drivers/leds/leds-blinkm.c
> @@ -671,7 +671,6 @@ failblue:
>  failgreen:
>  	led_classdev_unregister(&leds[RED]->cdev.led_cdev);
>  failred:
> -	sysfs_remove_group(&client->dev.kobj, &blinkm_group);
>
>  	return err;
>  }
> @@ -714,9 +713,8 @@ static int register_multicolor(struct i2c_client *client, struct blinkm_data *da
>  	if (err < 0) {
>  		dev_err(&client->dev, "couldn't register LED %s\n",
>  				mc_led->cdev.led_cdev.name);
> -		sysfs_remove_group(&client->dev.kobj, &blinkm_group);
>  	}
> -	return 0;
> +	return err;
>  }
>
>  static int blinkm_probe(struct i2c_client *client)
> @@ -749,13 +747,12 @@ static int blinkm_probe(struct i2c_client *client)
>  	}
>
> -	if (!IS_ENABLED(CONFIG_LEDS_BLINKM_MULTICOLOR)) {
> +	if (!IS_ENABLED(CONFIG_LEDS_BLINKM_MULTICOLOR))
>  		err = register_separate_colors(client, data);
> -		if (err < 0)
> -			return err;
> -	} else {
> +	else
>  		err = register_multicolor(client, data);

Please add a newline after the 'if-else' block.

> -		if (err < 0)
> -			return err;
> +	if (err < 0) {
> +		sysfs_remove_group(&client->dev.kobj, &blinkm_group);

How about using 'devm_device_add_group() to automate clean-up?

> +		return err;
>  	}
>
>  	blinkm_init_hw(client);

-- 
Lee Jones