Hi Luca, Esben, On Thu, 16 Jul 2026 17:22:24 +0200 Luca Ceresoli <[email protected]> wrote:
> On Wed, 15 Jul 2026 15:10:32 +0200, Esben Haabendal <[email protected]> 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: > > | [email protected] <[email protected]>: > | > | [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é
