Re: [PATCH] drm/panel: novatek-nt35950: Improve error handling

[AngeloGioacchino Del Regno]

Il 17/04/23 09:25, Neil Armstrong ha scritto:
> On 15/04/2023 13:00, Konrad Dybcio wrote:
> > In a very peculiar case when probing and registering with the secondary
> > DSI host succeeds, but the OF backlight or DSI attachment fails, the
> > primary DSI device is automatically cleaned up, but the secondary one
> > is not, leading to -EEXIST when the driver core tries to handle
> > -EPROBE_DEFER.
> >
> > Unregister the DSI1 device manually on failure to prevent that.
> >
> > Fixes: 623a3531e9cf ("drm/panel: Add driver for Novatek NT35950 DSI DriverIC 
> > panels")
> > Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
> > ---
> >   drivers/gpu/drm/panel/panel-novatek-nt35950.c | 8 +++++++-
> >   1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35950.c 
> > b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
> > index abf752b36a52..7498fc6258bb 100644
> > --- a/drivers/gpu/drm/panel/panel-novatek-nt35950.c
> > +++ b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
> > @@ -585,8 +585,11 @@ static int nt35950_probe(struct mipi_dsi_device *dsi)
> >                  DRM_MODE_CONNECTOR_DSI);
> >       ret = drm_panel_of_backlight(&nt->panel);
> > -    if (ret)
> > +    if (ret) {

If this is not a dual-DSI case, nt->dsi[1] will be NULL: I agree it's still
fine as in the kernel won't crash in that case, but for logical correctness
I would still account for that:

		if (num_dsis == 2)
			mipi_dsi_device_unregister(nt->dsi[1]);

> > +        mipi_dsi_device_unregister(nt->dsi[1]);
> > +
> >           return dev_err_probe(dev, ret, "Failed to get backlight\n");
> > +    }
> >       drm_panel_add(&nt->panel);
> > @@ -602,6 +605,9 @@ static int nt35950_probe(struct mipi_dsi_device *dsi)
> >           ret = mipi_dsi_attach(nt->dsi[i]);
> >           if (ret < 0) {

Same here, please add a check for num_dsis.

Regards,
Angelo