Received-SPF: pass (google.com: domain of linux-kernel+bounces-135091-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <e989d0ca1d3369d274af5651f340e28e4808e7e4.camel@gmail.com>
Subject: Re: [PATCH 3/4] iio: backend: make use of dev_errp_probe()
From: Nuno =?ISO-8859-1?Q?S=E1?= <noname.nuno@gmail.com>
To: Jonathan Cameron <jic23@kernel.org>, Andy Shevchenko
	 <andriy.shevchenko@linux.intel.com>
Cc: Nuno Sa <nuno.sa@analog.com>, Petr Mladek <pmladek@suse.com>, 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>,  linux-kernel@vger.kernel.org,
 linux-iio@vger.kernel.org
Date: Mon, 08 Apr 2024 11:01:11 +0200
In-Reply-To: <20240406170717.31ab7e47@jic23-huawei>
References: <20240404-dev-add_dev_errp_probe-v1-0-d18e3eb7ec3f@analog.com>
	 <20240404-dev-add_dev_errp_probe-v1-3-d18e3eb7ec3f@analog.com>
	 <Zg6bsS3WMvp-nY3S@smile.fi.intel.com>
	 <9f91388faee750e16021f2c0544e7a158a16202b.camel@gmail.com>
	 <Zg7DWQtY6SP9E_1m@smile.fi.intel.com>
	 <20240406170717.31ab7e47@jic23-huawei>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.52.0 
Precedence: bulk
MIME-Version: 1.0

On Sat, 2024-04-06 at 17:07 +0100, Jonathan Cameron wrote:
> On Thu, 4 Apr 2024 18:12:25 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>=20
> > On Thu, Apr 04, 2024 at 04:58:27PM +0200, Nuno S=C3=A1 wrote:
> > > On Thu, 2024-04-04 at 15:23 +0300, Andy Shevchenko wrote:=C2=A0=20
> > > > On Thu, Apr 04, 2024 at 01:06:25PM +0200, Nuno Sa wrote:=C2=A0=20
> > > > > Using dev_errp_probe() to simplify the code.=C2=A0=20
> >=20
> > ...
> >=20
> > > > > +	if (IS_ERR(fwnode))
> > > > > +		return dev_errp_probe(dev, PTR_ERR(fwnode),
> > > > > +				=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 "Cannot get Firmware
> > > > > reference\n");=C2=A0=20
> > > >=20
> > > > ERR_CAST() seems quite good candidate to have here.
> > > >=20
> > > > 		return dev_errp_probe(dev, fwnode, "Cannot get Firmware
> > > > reference\n");
> > > >=20
> > > > (Assuming dev_errp_probe() magically understands that, note you may=
 have
> > > > it as
> > > > =C2=A0a macro and distinguish parameter type with _Generic() or so =
and behave
> > > > =C2=A0differently: ERR_PTR() vs. ERR_CAST(), see acpi_dev_hid_uid_m=
atch()
> > > > =C2=A0implementation, but also keep in mind that it doesn't disting=
uish
> > > > NULL/0,
> > > > there
> > > > =C2=A0is a patch available in the mailing list to fix that, though.=
)=C2=A0=20
> > >=20
> > > Do we care that much for going with that trouble?=C2=A0=20
> >=20
> > I don't think we do. We are not supposed to be called with ret =3D=3D 0=
/NULL.
> > That's why I pointed out to the current version.
> >=20
> > > I understand like this we go
> > > PTR_ERR() to then comeback to ERR_PTR() but this for probe() which is=
 not
> > > a
> > > fastpath. So perhaps we could just keep it simple?=C2=A0=20
> >=20
> > It's not about performance, it's about readability. See the difference
> > between
> > yours and mine.
> >=20
>=20
> You are suggesting making it transparently take an error ptr or an intege=
r?
> Whilst clever, I'm not seeing that as a good idea for readability /
> reviewability.
> I expect something that looks like a function to take the same parameters
> (other vargs)
> always.=C2=A0 _Generic messes with that.

> Maybe I just don't like to learn new things!=C2=A0 If consensus comes dow=
n in
> favour
> of _Generic trickery then I'll get used to it eventually.
>=20

Yeah, I agree with the above. Not fully convinced but for the ERR_CAST() ca=
se I
would very much prefer to have another explicit helper rather than hiding s=
tuff
in the same macro.

- Nuno S=C3=A1