"Herve Codina" <[email protected]> writes: > Hi Esben, > > On Mon, 10 Aug 2026 09:19:40 +0200 > Esben Haabendal <[email protected]> wrote: > >> The error handling of sn65dsi83_reset_pipe() failure in >> sn65dsi83_reset_work() has been a bit strange all the time, missing both >> enable_irq() and drm_bridge_exit() in case of failure. But since >> sn65dsi83_reset_pipe() have never been failing (always returning 0), it has >> not caused any problems. >> >> As we are going to change sn65dsi83_reset_pipe() to return error values in >> case of failure, we need to fix this. >> >> Fixes: ad5c6ecef27e ("drm: bridge: ti-sn65dsi83: Add error recovery >> mechanism") >> Cc: [email protected] >> Signed-off-by: Esben Haabendal <[email protected]> >> --- >> drivers/gpu/drm/bridge/ti-sn65dsi83.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c >> b/drivers/gpu/drm/bridge/ti-sn65dsi83.c >> index 42b451432bbb..0ccfff401b2a 100644 >> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c >> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c >> @@ -417,10 +417,9 @@ static void sn65dsi83_reset_work(struct work_struct *ws) >> >> /* Reset the pipe */ >> ret = sn65dsi83_reset_pipe(ctx); >> - if (ret) { >> + if (ret) >> dev_err(ctx->dev, "reset pipe failed %pe\n", ERR_PTR(ret)); >> - return; >> - } >> + >> if (ctx->irq) >> enable_irq(ctx->irq); >> >> > > In this v4, enable_irq() is called even if we cannot reset the pipe. I know > that > sn65dsi83_reset_pipe() always returns 0 (and this will be change) but we have > to > take care here. > > If the pipe cannot be reset, the sn65dsi83 component could be in a state where > it can force the interrupt line. This was the reason the irq is disabled > > https://elixir.bootlin.com/linux/v7.2/source/drivers/gpu/drm/bridge/ti-sn65dsi83.c#L455
Do we agree that this patch series (patch 1 and 2 combined) does not change this behavior compared to the current state? Currently, because sn65dsi83_reset_pipe() always returns 0, we call enable_irq() even if we cannot reset the pipe. So I am not changing behavior. I agree, that this might not be the right approach, but it makes more sense to me to address that specifically on top of the change in this patch, so we can all agree on what we are changing and why. Can we keep this commit? Should I add a new patch that makes the change to this series, or is it okay to postpone this to later? Or alternatively, do you want me to drop this patch? And if so, I guess we should at least add some comment to the next patch about the behavior change that it introduces in sn65dsi83_reset_work(). /Esben
