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

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

[Myeonghun Pak]

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);
-		if (err < 0)
-			return err;
+	if (err < 0) {
+		sysfs_remove_group(&client->dev.kobj, &blinkm_group);
+		return err;
 	}
 
 	blinkm_init_hw(client);

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

[Lee Jones]

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

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

[Greg KH]

On Thu, Oct 15, 2026 at 12:54:33PM +0100, Lee Jones wrote:
> 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?

Wait, why is a driver manually adding sysfs files at all?  Shouldn't
this "just work" if it's a default group for the driver?  This way (or
with the devm_device_add_group() call) will race with userspace and
loose.

thanks,

greg k-h