Re: [PATCH v2] leds: lp5860: fix LED teardown ordering using devm_add_action_or_reset()

Thank you for your contribution! Sashiko AI review found 2 potential issue(=
s) to consider:
- [High] Unconditional self-deadlock during probe due to incorrect lock lif=
etime using `guard(mutex)`.
- [Low] Undeclared variable `ret` used in `lp5860_probe()`.
--

--- Patch [1]: [PATCH v2] leds: lp5860: fix LED teardown ordering using dev=
m_add_action_or_reset() ---
commit fa14917dcf9d1637bf1292a82596892d65e73078
Author: Surendra Singh Chouhan <[email protected]>

leds: lp5860: fix LED teardown ordering using devm_add_action_or_reset()

The driver previously disabled the chip in its remove callback before devres
unregistered the LEDs. This patch fixes the teardown ordering by registerin=
g a
devm action to disable the chip, ensuring it happens after LED unregistrati=
on.
It also converts mutex initialization to use devm_mutex_init().

> diff --git a/drivers/leds/rgb/leds-lp5860-core.c b/drivers/leds/rgb/leds-=
lp5860-core.c
> index fd0e2f6e6e0f4..bbe299e36ad53 100644
> --- a/drivers/leds/rgb/leds-lp5860-core.c
> +++ b/drivers/leds/rgb/leds-lp5860-core.c
[ ... ]
> @@ -197,38 +204,24 @@ int lp5860_device_init(struct device *dev)
>  	if (ret)
>  		return ret;
> =20
> +	ret =3D devm_add_action_or_reset(dev, lp5860_disable_action, lp);
> +	if (ret)
> +		return ret;
> +
>  	/*
>  	 * Set to 8-bit PWM data without VSYNC.
>  	 * Data is sent out for display instantly after received.
>  	 */
> -	mutex_lock(&lp->lock);
> +	guard(mutex)(&lp->lock);
>  	ret =3D regmap_update_bits(lp->regmap, LP5860_REG_DEV_INITIAL, LP5860_M=
ODE_MASK,
>  				 LP5860_MODE_1 << LP5860_MODE_SHIFT);
>  	if (ret)
> -		goto err_disable;
> -	mutex_unlock(&lp->lock);
> -
> -	ret =3D lp5860_init_dt(lp);
> -	if (ret)
> -		goto err_disable;
> -
> -	return 0;
> +		return ret;
> =20
> -err_disable:
> -	mutex_unlock(&lp->lock);
> -	lp5860_chip_enable(lp, LP5860_CHIP_DISABLE);
> -	return ret;
> +	return lp5860_init_dt(lp);

[Severity: High]
Does this code introduce a self-deadlock during probe?

By replacing the explicit mutex_lock/unlock with guard(mutex), the lock
lifetime is extended to the end of lp5860_device_init(). This means the
lock is held while calling lp5860_init_dt().

However, lp5860_init_dt() eventually calls lp5860_set_mc_brightness(),
which attempts to acquire the exact same lock:

drivers/leds/rgb/leds-lp5860-core.c:lp5860_set_mc_brightness() {
    ...
    guard(mutex)(&led->chip->lock);
    ...
}

Since Linux mutexes are non-recursive, will this cause an unconditional
deadlock during initialization?

>  }
>  EXPORT_SYMBOL_GPL(lp5860_device_init);
> =20
> diff --git a/drivers/leds/rgb/leds-lp5860-spi.c b/drivers/leds/rgb/leds-l=
p5860-spi.c
> index 5e0c44854a680..13ad90362d5f0 100644
> --- a/drivers/leds/rgb/leds-lp5860-spi.c
> +++ b/drivers/leds/rgb/leds-lp5860-spi.c
[ ... ]
> @@ -61,22 +61,15 @@ static int lp5860_probe(struct spi_device *spi)
>  				     "Failed to initialise Regmap.\n");
> =20
>  	lp5860->dev =3D dev;
> -	mutex_init(&lp5860->lock);
> +	ret =3D devm_mutex_init(dev, &lp5860->lock);
> +	if (ret)
> +		return ret;

[Severity: Low]
Is the ret variable declared in lp5860_probe()?

It looks like the local variable ret was never declared in this function,
which will cause a build failure.

> =20
>  	spi_set_drvdata(spi, lp5860);
> =20
>  	return lp5860_device_init(dev);
>  }

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260806145527.1148=
[email protected]?part=3D1