Re: [PATCH v4 3/6] drm/panel: Use managed helpers in Novatek and Raydium panels

["Luca Ceresoli" <luca.ceresoli@bootlin.com>]

Hello Osama,

On Wed Aug 12, 2026 at 4:59 PM CEST, Osama Abdelkader wrote:
> Use the managed MIPI DSI attach and panel add helpers in Novatek and
> Raydium panel drivers and drop the corresponding manual detach and panel
> remove calls from remove paths.
>
> Suggested-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>

...

> --- a/drivers/gpu/drm/panel/panel-novatek-nt35510.c
> +++ b/drivers/gpu/drm/panel/panel-novatek-nt35510.c
> @@ -1173,13 +1173,11 @@ static int nt35510_probe(struct mipi_dsi_device *dsi)
>  		nt->panel.backlight = bl;
>  	}
>
> -	drm_panel_add(&nt->panel);
> -
> -	ret = mipi_dsi_attach(dsi);
> -	if (ret < 0)
> -		drm_panel_remove(&nt->panel);
> +	ret = devm_drm_panel_add(dev, &nt->panel);
> +	if (ret)
> +		return ret;
>
> -	return 0;
> +	return devm_mipi_dsi_attach(dev, dsi);
>  }
>
>  static void nt35510_remove(struct mipi_dsi_device *dsi)
> @@ -1187,13 +1185,10 @@ static void nt35510_remove(struct mipi_dsi_device *dsi)
>  	struct nt35510 *nt = mipi_dsi_get_drvdata(dsi);
>  	int ret;
>
> -	mipi_dsi_detach(dsi);
>  	/* Power off */
>  	ret = nt35510_power_off(nt);
>  	if (ret)
>  		dev_err(&dsi->dev, "Failed to power off\n");
> -
> -	drm_panel_remove(&nt->panel);
>  }

This will change the removal sequence, from

  mipi_dsi_detach, nt35510_power_off, drm_panel_remove

to

  nt35510_power_off, mipi_dsi_detach, drm_panel_remove

Can this create issues? In a doubt, we shouldn't change the sequence.

Adding an explicit devm action to call nt35510_power_off in the correct
sequence is a possible way out. But perhaps it's safer and simpler if you
just change drm_panel_remove() to devm, not mipi_dsi_attach().

The rest of the patch LGTM.

> --- a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
> +++ b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
> @@ -428,9 +428,7 @@ static int nt36672a_panel_add(struct nt36672a_panel *pinfo)

Looking at this driver, the code split between nt36672a_panel_probe() and
nt36672a_panel_add() appears very arbitrary. Definitely unrelated to your
patch however, just thinking out loud, no need to change your patch about
this.

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com