Hi Luca,
On Tue, 2024-09-17 at 10:53 +0200, Luca Ceresoli wrote:
> led-backlight is a consumer of one or multiple LED class devices, but no
> devlink is created for such supplier-producer relationship. One consequence
> is that removal ordered is not correctly enforced.
>
> Issues happen for example with the following sections in a device tree
> overlay:
>
> // An LED driver chip
> pca9632@62 {
> compatible = "nxp,pca9632";
> reg = <0x62>;
>
> // ...
>
> addon_led_pwm: led-pwm@3 {
> reg = <3>;
> label = "addon:led:pwm";
> };
> };
>
> backlight-addon {
> compatible = "led-backlight";
> leds = <&addon_led_pwm>;
> brightness-levels = <255>;
> default-brightness-level = <255>;
> };
>
> On removal of the above overlay, the LED driver can be removed before the
> backlight device, resulting in:
>
> Unable to handle kernel NULL pointer dereference at virtual address
> 0000000000000010
> ...
> Call trace:
> led_put+0xe0/0x140
> devm_led_release+0x6c/0x98
>
> Fix by adding a devlink between the consuming led-backlight device and the
> supplying LED device.
>
> Signed-off-by: Luca Ceresoli <luca.ceres...@bootlin.com>
I've tested the patch wit LP8864 LED as a provider for led_bl, removing the
underlying I2C bus. The patch avoids the crash for me (by removing led_bl
device as well),
thanks for fixing it!
Tested-by: Alexander Sverdlin <alexander.sverd...@siemens.com>
> ---
>
> This patch first appeared in v4.
> ---
> drivers/video/backlight/led_bl.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/video/backlight/led_bl.c
> b/drivers/video/backlight/led_bl.c
> index c7aefcd6e4e3..bfbd80728036 100644
> --- a/drivers/video/backlight/led_bl.c
> +++ b/drivers/video/backlight/led_bl.c
> @@ -209,6 +209,19 @@ static int led_bl_probe(struct platform_device *pdev)
> return PTR_ERR(priv->bl_dev);
> }
>
> + for (i = 0; i < priv->nb_leds; i++) {
> + struct device_link *link;
> +
> + link = device_link_add(&pdev->dev, priv->leds[0]->dev->parent,
> + DL_FLAG_AUTOREMOVE_CONSUMER);
> + if (!link) {
> + dev_err(&pdev->dev, "Failed to add devlink (consumer
> %s, supplier %s)\n",
> + dev_name(&pdev->dev),
> dev_name(priv->leds[0]->dev->parent));
> + backlight_device_unregister(priv->bl_dev);
> + return -EINVAL;
> + }
> + }
> +
> for (i = 0; i < priv->nb_leds; i++) {
> mutex_lock(&priv->leds[i]->led_access);
> led_sysfs_disable(priv->leds[i]);
--
Alexander Sverdlin
Siemens AG
www.siemens.com
