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 <[email protected]> > Signed-off-by: Osama Abdelkader <[email protected]>
... > --- 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
