Received-SPF: pass (google.com: domain of linux-kernel+bounces-79774-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Sat, 24 Feb 2024 17:54:48 +0000
From: Jonathan Cameron <jic23@kernel.org>
To: Nuno =?UTF-8?B?U8Oh?= <noname.nuno@gmail.com>
Cc: Dumitru Ceclan <mitrutzceclan@gmail.com>, Lars-Peter Clausen
 <lars@metafoo.de>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski
 <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org>,
 linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
 linux-kernel@vger.kernel.org, Ceclan Dumitru <dumitru.ceclan@analog.com>
Subject: Re: [PATCH v5 5/5] iio: amplifiers: hmc425a: add support for
 LTC6373 Instrumentation Amplifier
Message-ID: <20240224175448.484ccd10@jic23-huawei>
In-Reply-To: <c92c4c71f433c7375588e832d3c084a50190a1b5.camel@gmail.com>
References: <20240220153553.2432-1-mitrutzceclan@gmail.com>
	<20240220153553.2432-6-mitrutzceclan@gmail.com>
	<c92c4c71f433c7375588e832d3c084a50190a1b5.camel@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On Wed, 21 Feb 2024 14:23:51 +0100
Nuno S=C3=A1 <noname.nuno@gmail.com> wrote:

> On Tue, 2024-02-20 at 17:34 +0200, Dumitru Ceclan wrote:
> > This adds support for LTC6373 36 V Fully-Differential Programmable-Gain
> > Instrumentation Amplifier with 25 pA Input Bias Current.
> > The user can program the gain to one of seven available settings through
> > a 3-bit parallel interface (A2 to A0).
> >=20
> > Signed-off-by: Dumitru Ceclan <mitrutzceclan@gmail.com>
> > --- =20
>=20
> Just one minor comment. With that:
>=20
> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
>=20
> > =C2=A0drivers/iio/amplifiers/hmc425a.c | 124 ++++++++++++++++++++++++++=
++++-
> > =C2=A01 file changed, 120 insertions(+), 4 deletions(-)
> >=20
> > diff --git a/drivers/iio/amplifiers/hmc425a.c b/drivers/iio/amplifiers/=
hmc425a.c
> > index 77872e2dfdfe..50c86c2d28d7 100644
> > --- a/drivers/iio/amplifiers/hmc425a.c
> > +++ b/drivers/iio/amplifiers/hmc425a.c
> > @@ -2,9 +2,10 @@
> > =C2=A0/*
> > =C2=A0 * HMC425A and similar Gain Amplifiers
> > =C2=A0 *
> > - * Copyright 2020 Analog Devices Inc.
> > + * Copyright 2020, 2024 Analog Devices Inc.
> > =C2=A0 */ =20
>=20
> ...
>=20
> >=20
> > =C2=A0
> > +static ssize_t ltc6373_read_powerdown(struct iio_dev *indio_dev,
> > +				=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 uintptr_t private,
> > +				=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 const struct iio_chan_spec *chan,
> > +				=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 char *buf)
> > +{
> > +	struct hmc425a_state *st =3D iio_priv(indio_dev);
> > +
> > +	return sysfs_emit(buf, "%d\n", st->powerdown); =20
>=20
> Well, in theory the read should also be protected with the lock...

Only reason I can think of for that is potential read tearing.
If that happens on a bool we are going to be in a mess so I think this
is in practice fine without, though paranoia might suggest locking.

It can race against it being powered down but that effectively happens
even if we do have a lock as we either see the value before or after
a racing power down event and we have no way of knowing which.

Applied rest series to iio.git togreg branch and pushed out as testing.

Duitru, if you can figure out what happened to your message thread before
sending more patches that would be great. The IDs for patches 0-5 go

20240220153553.2432-1-mitrutzceclan@gmail.com
20240220153553.2432-3-mitrutzceclan@gmail.com
20240220153553.2432-5-mitrutzceclan@gmail.com
20240220153553.2432-2-mitrutzceclan@gmail.com
20240220153553.2432-4-mitrutzceclan@gmail.com
20240220153553.2432-6-mitrutzceclan@gmail.com

Which really confuses my email client and patchwork.
https://patchwork.kernel.org/project/linux-iio/list/?series=3D827901


>=20
> - Nuno S=C3=A1
>=20