Re: [PATCH v2 2/3] drm/bridge: ti-sn65dsi83: Fix error handling in sn65dsi83_reset_work()

[Herve Codina <herve.codina@bootlin.com>]

Hi Luca, Esben,

On Thu, 16 Jul 2026 17:22:24 +0200
Luca Ceresoli <luca.ceresoli@bootlin.com> wrote:

> On Wed, 15 Jul 2026 15:10:32 +0200, Esben Haabendal <esben@geanix.com> wrote:
> 
> Hi Esben,
> 
> +Cc Hervé
> 
> >
> > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > index b4b220eee790..7e73035d7798 100644
> > --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > @@ -419,11 +419,12 @@ static void sn65dsi83_reset_work(struct work_struct *ws)
> >  	ret = sn65dsi83_reset_pipe(ctx);
> >  	if (ret) {
> >  		dev_err(ctx->dev, "reset pipe failed %pe\n", ERR_PTR(ret));
> > -		return;
> > +		goto err_exit;
> >  	}  
> 
> This looked like a nasty bug! But it is not, out of pure luck. As sashiko
> noticed:
> 
>  | sashiko-bot@kernel.org <sashiko-bot@kernel.org>:
>  |
>  | [Severity: High]
>  | This isn't a bug introduced by this patch, but is this error handling block
>  | actually dead code?
>  |
>  | Looking at sn65dsi83_reset_pipe(), it appears to unconditionally return 0,
>  | even if drm_bridge_helper_reset_crtc() returns an error like -EINVAL or
>  | -ENOMEM:
> 
> So perhaps we should just remove the dead code, and also make
> sn65dsi83_reset_pipe() return void.
> 

I would keep an error code from sn65dsi83_reset_pipe() but change its code
from
---- 8< ----
retry:
	err = drm_bridge_helper_reset_crtc(&sn65dsi83->bridge, &ctx);
	if (err == -EDEADLK) {
		drm_modeset_backoff(&ctx);
		goto retry;
	}

	drm_modeset_drop_locks(&ctx);
	drm_modeset_acquire_fini(&ctx);

	return 0;
---- 8< ----

to 

---- 8< ----
retry:
	err = drm_bridge_helper_reset_crtc(&sn65dsi83->bridge, &ctx);
	if (err == -EDEADLK) {
		drm_modeset_backoff(&ctx);
		goto retry;
	}

	drm_modeset_drop_locks(&ctx);
	drm_modeset_acquire_fini(&ctx);

	return err;
---- 8< ----

Indeed, if drm_bridge_helper_reset_crtc() fails, we can consider that
sn65dsi83_reset_pipe() fails.

With that done, Esben's modification is still relevant (i.e. goto err_exit
on failure).

Best regards,
Hervé