Received-SPF: pass (google.com: domain of linux-kernel+bounces-204264-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <da57a64e4f5cdda7ee6b794c448995eee648c436.camel@gmail.com>
Subject: Re: [PATCH v3 2/4] iio: temperature: ltc2983: convert to
 dev_err_probe()
From: Nuno =?ISO-8859-1?Q?S=E1?= <noname.nuno@gmail.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>, Nuno Sa
	 <nuno.sa@analog.com>
Cc: Petr Mladek <pmladek@suse.com>, Jonathan Cameron <jic23@kernel.org>, 
 Lars-Peter Clausen <lars@metafoo.de>, Olivier Moysan
 <olivier.moysan@foss.st.com>, Jyoti Bhayana <jbhayana@google.com>, Chris
 Down <chris@chrisdown.name>, John Ogness <john.ogness@linutronix.de>, Greg
 Kroah-Hartman <gregkh@linuxfoundation.org>,  Andi Shyti
 <andi.shyti@kernel.org>, linux-kernel@vger.kernel.org,
 linux-iio@vger.kernel.org
Date: Thu, 06 Jun 2024 14:27:03 +0200
In-Reply-To: <ZmGMwwglUlS6_NI_@smile.fi.intel.com>
References: <20240606-dev-add_dev_errp_probe-v3-0-51bb229edd79@analog.com>
	 <20240606-dev-add_dev_errp_probe-v3-2-51bb229edd79@analog.com>
	 <ZmGMwwglUlS6_NI_@smile.fi.intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.52.2 
Precedence: bulk
MIME-Version: 1.0

On Thu, 2024-06-06 at 13:17 +0300, Andy Shevchenko wrote:
> On Thu, Jun 06, 2024 at 09:22:38AM +0200, Nuno Sa wrote:
> > Use dev_err_probe() (and variants) in the probe() path. While at it, ma=
de
> > some simple improvements:
> > =C2=A0* Explicitly included the err.h and errno.h headers;
> > =C2=A0* Removed some unnecessary line breaks;
> > =C2=A0* Removed a redundant 'else';
> > =C2=A0* Added some missing \n to prink.
>=20
> ...
>=20
> > -		if (ret) {
> > +		if (ret)
> > =C2=A0			/*
> > =C2=A0			 * This would be catched later but we can just
> > return
> > =C2=A0			 * the error right away.
> > =C2=A0			 */
> > -			dev_err(&st->spi->dev, "Property reg must be
> > given\n");
> > -			return ERR_PTR(ret);
> > -		}
> > +			return dev_err_ptr_probe(&st->spi->dev, ret,
> > +						 "Property reg must be
> > given\n");
>=20
> Even if it becomes a one line of code, it's still a multiline branch, due=
 to
> comment. I think {} is better to be there. What does checkpatch say about
> this?

Checkpatch is fine about it...

>=20
>=20
> ...
>=20
> > +			return dev_err_ptr_probe(&st->spi->dev, -EINVAL,
>=20
> You can make all these lines shorter by using
>=20
> 	struct device *dev =3D &st->spi->dev; // or analogue
>=20
> at the top of the function.
>=20

Well, I had that in v2 (making the whole driver coherent with the local str=
uct
device helper but you kind of "complained" for a precursor patch (on a
devm_kzalloc() call). So basically I deferred that change for a follow up p=
atch.

- Nuno S=C3=A1

>=20