* [PATCH] leds: lp5860: fix LED teardown ordering
@ 2026-07-20 10:48 kr494167
2026-08-06 12:28 ` Lee Jones
0 siblings, 1 reply; 2+ messages in thread
From: kr494167 @ 2026-07-20 10:48 UTC (permalink / raw)
To: lee, pavel; +Cc: s.trumtrar, linux-leds, linux-kernel, surendra
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>
---
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);
+}
+
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] leds: lp5860: fix LED teardown ordering
2026-07-20 10:48 [PATCH] leds: lp5860: fix LED teardown ordering kr494167
@ 2026-08-06 12:28 ` Lee Jones
0 siblings, 0 replies; 2+ messages in thread
From: Lee Jones @ 2026-08-06 12:28 UTC (permalink / raw)
To: kr494167; +Cc: pavel, s.trumtrar, linux-leds, linux-kernel
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-08-06 12:28 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-20 10:48 [PATCH] leds: lp5860: fix LED teardown ordering kr494167
2026-08-06 12:28 ` Lee Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome