From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtpout-02.galae.net (smtpout-02.galae.net [185.246.84.56]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BC6804398E3 for ; Thu, 16 Jul 2026 17:09:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.246.84.56 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784221757; cv=none; b=ckWNZ/+Mdb8e1akpMNX0LeVYGLldyILxoUlX69OSY7pXNstMnfKhlwlCGTj2pcQNhmixyMIdRbHsZbocZDg0ZCNe9hI7D0g5mZMe/95XH8dXeMkOFxMBNPQ39qPYm0pCLp+scThRyfCFuY0aB5FyHe/J3ReV1uJiQh8q5CvYAQw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784221757; c=relaxed/simple; bh=ArebUNFDiFjXTmh7k/mldtj4JjTz02RR3x3EtRJrCbg=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=Wuz1cbs94J7uxFhy4Eeltmjni4rMyrbKT0YxpRzdzToZ1KiPs/whQusKl1kyYy7/tIPsPUPDipNaYju1qg9Y+5kHXKcghPHzPmzP6Z8mgX+kB+3/PmPWZfg6umpfC5qkeyvF59+AuxdvNHoWXGwEB/D2FYw3FBUBibkni7QKa9s= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com; spf=pass smtp.mailfrom=bootlin.com; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b=aoMC+Qd8; arc=none smtp.client-ip=185.246.84.56 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="aoMC+Qd8" Received: from smtpout-01.galae.net (smtpout-01.galae.net [212.83.139.233]) by smtpout-02.galae.net (Postfix) with ESMTPS id 4BF581A101B; Thu, 16 Jul 2026 17:09:10 +0000 (UTC) Received: from mail.galae.net (mail.galae.net [212.83.136.155]) by smtpout-01.galae.net (Postfix) with ESMTPS id 0F86B6035F; Thu, 16 Jul 2026 17:09:10 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id 3629E11BD3D27; Thu, 16 Jul 2026 19:09:03 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=dkim; t=1784221748; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:in-reply-to:references; bh=athdqcsoQXwS2d0D/Za30Y4KtbUbBG7ZgM8ivAaVLFc=; b=aoMC+Qd8YFn+M5txmi1HS0zHGNsle13KATqdZfJBH+C89VMIk4c9bHA/VKynxEqS2R5nWI nL+uJBkqtGK1dGRRpmU6wg2KgE1wdKzu9XE9G5BHBrRaNVmAO4lEIC9LWPOgiAXJkAd8rt 1o6YcbuD6iRMx8wFpr6xmdcAbOn7b2JmkbCOglv7IhTR5qLt+0Eg/DXEUn+VEW0QQlr4Mf JhOMO3YvLknAvJ1NpS5xjyS7eT+MYCAQPdJeq2ri2PW9MFGw5gjSKq0UaffSeM2Dndq32a OxSw0qZDTLG/fcgNHJJJ0z4U4hEoyfNCEPtCYPi/lurk7uI/gxZFroSB1Pg/eA== Date: Thu, 16 Jul 2026 19:09:01 +0200 From: Herve Codina To: Luca Ceresoli Cc: 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 , Esben Haabendal , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 2/3] drm/bridge: ti-sn65dsi83: Fix error handling in sn65dsi83_reset_work() Message-ID: <20260716190901.29f44aea@bootlin.com> In-Reply-To: <178421534408.28815.5022512273977369395.b4-review@b4> References: <20260715-ti-sn65dsi83-fixes-v2-0-ebc4c3fe29b6@geanix.com> <20260715-ti-sn65dsi83-fixes-v2-2-ebc4c3fe29b6@geanix.com> <178421534408.28815.5022512273977369395.b4-review@b4> Organization: Bootlin X-Mailer: Claws Mail 4.4.0 (GTK 3.24.52; x86_64-redhat-linux-gnu) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Last-TLS-Session-Version: TLSv1.3 Hi Luca, Esben, On Thu, 16 Jul 2026 17:22:24 +0200 Luca Ceresoli wrote: > On Wed, 15 Jul 2026 15:10:32 +0200, Esben Haabendal 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 : > | > | [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é