Re: [PATCH] media: i2c: imx219: Use dev_err_probe on probe

From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Tommaso Merciai <tomm.merciai@gmail.com>
Cc: Umang Jain <umang.jain@ideasonboard.com>,
	linux-media@vger.kernel.org, dave.stevenson@raspberrypi.com,
	Kieran Bingham <kieran.bingham@ideasonboard.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Subject: Re: [PATCH] media: i2c: imx219: Use dev_err_probe on probe
Date: Fri, 15 Mar 2024 06:38:00 +0000	[thread overview]
Message-ID: <ZfPsyFXrlep9sQco@kekkonen.localdomain> (raw)
In-Reply-To: <ZfMQpPyRrLAiRk6I@tom-HP-ZBook-Fury-15-G7-Mobile-Workstation>

On Thu, Mar 14, 2024 at 03:58:44PM +0100, Tommaso Merciai wrote:
> > > > @@ -1025,15 +1026,15 @@ static int imx219_identify_module(struct imx219 *imx219)
> > > >   	ret = cci_read(imx219->regmap, IMX219_REG_CHIP_ID, &val, NULL);
> > > >   	if (ret) {
> > > > -		dev_err(&client->dev, "failed to read chip id %x\n",
> > > > -			IMX219_CHIP_ID);
> > > > -		return ret;
> > > > +		return dev_err_probe(&client->dev, ret,
> > > > +				     "failed to read chip id %x\n",
> > > > +				     IMX219_CHIP_ID);
> > > >   	}
> > > I think you can remove also here the curve brakets we don't need that
> > > anymore.
> > 
> > I think multi-line single statement like this one, is better with { ... }
> > and is actually preferred?
> 
> Yep, some driver is using your pattern some other not.
> I'm curious about the truth :) (for my education :))

I'd prefer it without braces.

See the end of section 3 in Documentation/process/coding-style.rst .

-- 
Sakari Ailus