linux-kernel - Re: [PATCH] drm/bridge: ti-sn65dsi83: use dev_err

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZrtuksiarZNS8L79@ashyti-mobl2.lan>
Date: Tue, 13 Aug 2024 16:32:50 +0200
From: Andi Shyti <andi.shyti@...ux.intel.com>
To: Luca Ceresoli <luca.ceresoli@...tlin.com>
Cc: Andi Shyti <andi.shyti@...ux.intel.com>,
	Andrzej Hajda <andrzej.hajda@...el.com>,
	Neil Armstrong <neil.armstrong@...aro.org>,
	Robert Foss <rfoss@...nel.org>,
	Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
	Jonas Karlman <jonas@...boo.se>,
	Jernej Skrabec <jernej.skrabec@...il.com>,
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
	Maxime Ripard <mripard@...nel.org>,
	Thomas Zimmermann <tzimmermann@...e.de>,
	David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/bridge: ti-sn65dsi83: use dev_err_probe when failing
 to get panel bridge

Hi Luca,

On Tue, Aug 13, 2024 at 10:16:43AM +0200, Luca Ceresoli wrote:
> On Thu, 8 Aug 2024 11:35:23 +0100
> Andi Shyti <andi.shyti@...ux.intel.com> wrote:
> > On Thu, Aug 08, 2024 at 12:26:14PM +0200, Luca Ceresoli wrote:
> > > When devm_drm_of_get_bridge() fails, the probe fails silently. Use
> > > dev_err_probe() instead to log an error or report the deferral reason,
> > > whichever is applicable.
> > > 
> > > Signed-off-by: Luca Ceresoli <luca.ceresoli@...tlin.com>
> > > ---
> > >  drivers/gpu/drm/bridge/ti-sn65dsi83.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > > index 57a7ed13f996..60b9f14d769a 100644
> > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > > @@ -606,7 +606,7 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
> > >  
> > >  	panel_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 2, 0);
> > >  	if (IS_ERR(panel_bridge))
> > > -		return PTR_ERR(panel_bridge);
> > > +		return dev_err_probe(dev, PTR_ERR(panel_bridge), "Failed to get panel bridge\n");  
> > 
> > patch looks good, but the message is a bit misleading. You are
> > not failing to get the panel bridge, but you are failing to find
> > a panel bridge in a DT node. Right?
> 
> As I can see from both the documentation and the code,
> devm_drm_of_get_bridge() is really returning a pointer to a panel
> bridge, potentially allocating and adding it in case it was not present
> before. Navigating the device tree is only a part of what it does.
> 
> Do you think I am missing something?

No, maybe it's me being a bit pedantic. In the sense that we are
not really failing to get the panel, but most probably the panel
is not installed. I'm not strong on this comment, though, so that
feel free to add:

Reviewed-by: Andi Shyti <andi.shyti@...ux.intel.com>

Thanks,
Andi