"Esben Haabendal" <[email protected]> writes: > "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().
Scratch that. I will squash the two patches into one, go back to only calling enable_irq() when sn65dsi83_reset_pipe() succeeds, but ensure that drm_bridge_exit() is always called. Basically what I had in v3, which you both (Luca and Herve) seemed fine with. I try to explain the situation a bit better in the new squashed commit message. If nothing else, it helped myself understand what was going on :) Is it okay to send v5 with this or do I have to wait a bit longer? /Esben
