Re: [PATCH] leds: lp5860: fix LED teardown ordering

[Lee Jones <lee@kernel.org>]

On Mon, 20 Jul 2026, kr494167@gmail.com wrote:

> From: surendra <kr494167@gmail.com>
> 
> The driver disables the chip from its remove callback before devres
> unregisters the LEDs. LED unregistration turns the LEDs off through the
> driver brightness callback, which can then access disabled hardware.
> 
> Unregister the LEDs before disabling the chip, including when probe fails
> after some LEDs have been registered. Keep track of registered LEDs so the
> devres actions can be released explicitly.
> 
> Also remove the early mutex destruction from the SPI remove callback:
> hardware teardown takes the same mutex.
> 
> Fixes: f0a66563aa2d ("leds: Add support for TI LP5860 LED driver chip")
> Signed-off-by: surendra <kr494167@gmail.com>

Submissions should include your full name.

> ---
>  drivers/leds/rgb/leds-lp5860-core.c | 13 ++++++++++---
>  drivers/leds/rgb/leds-lp5860-spi.c  |  4 ----
>  drivers/leds/rgb/leds-lp5860.h      |  1 +
>  3 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/leds/rgb/leds-lp5860-core.c b/drivers/leds/rgb/leds-lp5860-core.c
> index 75498311b080..9646d3985239 100644
> --- a/drivers/leds/rgb/leds-lp5860-core.c
> +++ b/drivers/leds/rgb/leds-lp5860-core.c
> @@ -149,12 +149,11 @@ static int lp5860_init_dt(struct lp5860 *lp)
>  	struct led_classdev *led_cdev;
>  	struct mc_subled *mc_led_info;
>  	struct lp5860_led *led;
> -	int led_index = 0;
>  	int chan;
>  	int ret;
>  
>  	device_for_each_child_node_scoped(lp->dev, multi_led) {
> -		led = &lp->leds[led_index];
> +		led = &lp->leds[lp->num_leds];
>  
>  		init_data.fwnode = multi_led;
>  
> @@ -188,12 +187,18 @@ static int lp5860_init_dt(struct lp5860 *lp)
>  		if (ret)
>  			return dev_err_probe(lp->dev, ret, "%pfwP: Failed to register Multi-Color LEDs\n",
>  					     multi_led);
> -		led_index++;
> +		lp->num_leds++;
>  	}
>  
>  	return 0;
>  }
>  
> +static void lp5860_unregister_leds(struct lp5860 *lp)
> +{
> +	while (lp->num_leds)
> +		devm_led_classdev_multicolor_unregister(lp->dev, &lp->leds[--lp->num_leds].mc_cdev);
> +}

Please look into using 'devm_add_action_or_reset()' to handle the chip
disable sequence.  Doing so might allow the managed resource framework
to handle the teardown order naturally, potentially letting us avoid
manual unregistration and even remove the '.remove' callback entirely.

>  int lp5860_device_init(struct device *dev)
>  {
>  	struct lp5860 *lp = dev_get_drvdata(dev);
> @@ -221,6 +226,7 @@ int lp5860_device_init(struct device *dev)
>  	return 0;
>  
>  err_disable:
> +	lp5860_unregister_leds(lp);
>  	lp5860_chip_enable(lp, LP5860_CHIP_DISABLE);
>  	return ret;
>  }
> @@ -230,6 +236,7 @@ void lp5860_device_remove(struct device *dev)
>  {
>  	struct lp5860 *lp = dev_get_drvdata(dev);
>  
> +	lp5860_unregister_leds(lp);
>  	lp5860_chip_enable(lp, LP5860_CHIP_DISABLE);
>  }
>  EXPORT_SYMBOL_GPL(lp5860_device_remove);
> diff --git a/drivers/leds/rgb/leds-lp5860-spi.c b/drivers/leds/rgb/leds-lp5860-spi.c
> index 5e0c44854a68..e1565eac90f9 100644
> --- a/drivers/leds/rgb/leds-lp5860-spi.c
> +++ b/drivers/leds/rgb/leds-lp5860-spi.c
> @@ -70,10 +70,6 @@ static int lp5860_probe(struct spi_device *spi)
>  
>  static void lp5860_remove(struct spi_device *spi)
>  {
> -	struct lp5860 *lp5860 = spi_get_drvdata(spi);
> -
> -	mutex_destroy(&lp5860->lock);
> -
>  	lp5860_device_remove(&spi->dev);
>  }
>  
> diff --git a/drivers/leds/rgb/leds-lp5860.h b/drivers/leds/rgb/leds-lp5860.h
> index 940be0c6e8da..ce30fb5596c0 100644
> --- a/drivers/leds/rgb/leds-lp5860.h
> +++ b/drivers/leds/rgb/leds-lp5860.h
> @@ -258,6 +258,7 @@ struct lp5860 {
>  	struct device *dev;
>  	struct regmap *regmap;
>  	struct mutex lock;
> +	unsigned int num_leds;
>  
>  	DECLARE_FLEX_ARRAY(struct lp5860_led, leds);
>  };
> -- 
> 2.55.0
> 

-- 
Lee Jones