From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 8E28CC61DBD for ; Fri, 28 Aug 2026 14:55:57 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C980110E032; Fri, 28 Aug 2026 14:55:56 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=geanix.com header.i=@geanix.com header.b="EDUkg90q"; dkim-atps=neutral Received: from mail-43172.protonmail.ch (mail-43172.protonmail.ch [185.70.43.172]) by gabe.freedesktop.org (Postfix) with ESMTPS id F3D7910E032 for ; Fri, 28 Aug 2026 14:55:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=geanix.com; s=protonmail; t=1787928952; x=1788188152; bh=V0hb2voKgSJqDmgc2p7cOkarJ7UY6K7MCbTZpq1OfA8=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID:From:To: Cc:Date:Subject:Reply-To:Feedback-ID:Message-ID:BIMI-Selector; b=EDUkg90q7sYiVD99e5kRcamXugkXAPb7OJCzk3f6XUt8OO4OQkr9khgExN4TPVqGB cJG5cj4F4GqjgZdyg0enFW0VzGhYnZQU/9dk4i+VykNrNgZ3sm2tzGhnKvGaUqZHai +JT6fsMy04hv+GgeZyqlEtXYHmDQRnX4kHYOiFT6AHkd9VcUUN+ilodlmmgQSQTQKH Z/Ntp1lefA88utBtyHfXqoE2ndrsr3+1VxhpegkXKt3jFix4WWyrwLREXT8N1rpE0b rjtqtNDvSJW2eDW5vqAVAz9wUD4y6qFzpnAs6cFilxixubKNblBNdSCs50HQbqC0E8 ZIApObSSBRv1A== X-Pm-Submission-Id: 4hWhK12291z2Scpm From: Esben Haabendal To: "Herve Codina" Cc: "Luca Ceresoli" , "Andrzej Hajda" , "Neil Armstrong" , "Robert Foss" , "Laurent Pinchart" , "Jonas Karlman" , "Jernej Skrabec" , "Maarten Lankhorst" , "Maxime Ripard" , "Thomas Zimmermann" , "David Airlie" , "Simona Vetter" , "Linus Walleij" , "Frieder Schrempf" , "Marek Vasut" , , , Subject: Re: [PATCH v4 1/3] drm/bridge: ti-sn65dsi83: Fix error handling in sn65dsi83_reset_work() In-Reply-To: <87cxv2xwz0.fsf@geanix.com> (Esben Haabendal's message of "Fri, 28 Aug 2026 12:15:16 +0000") References: <20260810-ti-sn65dsi83-fixes-v4-0-ae9c95c8d162@geanix.com> <20260810-ti-sn65dsi83-fixes-v4-1-ae9c95c8d162@geanix.com> <20260819113253.5d602b2d@bootlin.com> <87cxv2xwz0.fsf@geanix.com> Date: Fri, 28 Aug 2026 16:55:48 +0200 Message-ID: <874igewayz.fsf@geanix.com> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" "Esben Haabendal" writes: > "Herve Codina" writes: > >> Hi Esben, >> >> On Mon, 10 Aug 2026 09:19:40 +0200 >> Esben Haabendal 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: stable@vger.kernel.org >>> Signed-off-by: Esben Haabendal >>> --- >>> 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