linux-kernel - Re: [PATCH v5 5/5] iio: amplifiers: hmc425a: add support for LTC6373 Instrumentation Amplifier

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: <20240224175448.484ccd10@jic23-huawei>
Date: Sat, 24 Feb 2024 17:54:48 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Nuno Sá <noname.nuno@...il.com>
Cc: Dumitru Ceclan <mitrutzceclan@...il.com>, Lars-Peter Clausen
 <lars@...afoo.de>, Rob Herring <robh+dt@...nel.org>, Krzysztof Kozlowski
 <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>,
 linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, Ceclan Dumitru <dumitru.ceclan@...log.com>
Subject: Re: [PATCH v5 5/5] iio: amplifiers: hmc425a: add support for
 LTC6373 Instrumentation Amplifier

On Wed, 21 Feb 2024 14:23:51 +0100
Nuno Sá <noname.nuno@...il.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).
> > 
> > Signed-off-by: Dumitru Ceclan <mitrutzceclan@...il.com>
> > ---  
> 
> Just one minor comment. With that:
> 
> Reviewed-by: Nuno Sa <nuno.sa@...log.com>
> 
> >  drivers/iio/amplifiers/hmc425a.c | 124 ++++++++++++++++++++++++++++++-
> >  1 file changed, 120 insertions(+), 4 deletions(-)
> > 
> > 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 @@
> >  /*
> >   * HMC425A and similar Gain Amplifiers
> >   *
> > - * Copyright 2020 Analog Devices Inc.
> > + * Copyright 2020, 2024 Analog Devices Inc.
> >   */  
> 
> ...
> 
> > 
> >  
> > +static ssize_t ltc6373_read_powerdown(struct iio_dev *indio_dev,
> > +				      uintptr_t private,
> > +				      const struct iio_chan_spec *chan,
> > +				      char *buf)
> > +{
> > +	struct hmc425a_state *st = iio_priv(indio_dev);
> > +
> > +	return sysfs_emit(buf, "%d\n", st->powerdown);  
> 
> 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@...il.com
20240220153553.2432-3-mitrutzceclan@...il.com
20240220153553.2432-5-mitrutzceclan@...il.com
20240220153553.2432-2-mitrutzceclan@...il.com
20240220153553.2432-4-mitrutzceclan@...il.com
20240220153553.2432-6-mitrutzceclan@...il.com

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



> 
> - Nuno Sá
>