linux-kernel - Re: [PATCH v3 2/6] iio: adc: ad4030: add driver for ad4030-24

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] [day] [month] [year] [list]

Message-Id: <D7JODK8W4W9W.3OO4GRGNDU5ZX@baylibre.com>
Date: Tue, 04 Feb 2025 13:54:27 +0100
From: "Esteban Blanc" <eblanc@...libre.com>
To: "Jonathan Cameron" <Jonathan.Cameron@...wei.com>
Cc: "Lars-Peter Clausen" <lars@...afoo.de>, "Michael Hennerich"
 <Michael.Hennerich@...log.com>, Nuno Sá
 <nuno.sa@...log.com>, "Jonathan Cameron" <jic23@...nel.org>, "Rob Herring"
 <robh@...nel.org>, "Krzysztof Kozlowski" <krzk+dt@...nel.org>, "Conor
 Dooley" <conor+dt@...nel.org>, "Jonathan Corbet" <corbet@....net>,
 <linux-iio@...r.kernel.org>, <devicetree@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>, <linux-doc@...r.kernel.org>
Subject: Re: [PATCH v3 2/6] iio: adc: ad4030: add driver for ad4030-24

On Fri Jan 31, 2025 at 7:14 PM CET, Jonathan Cameron wrote:
> On Thu, 30 Jan 2025 12:08:26 +0100
> Esteban Blanc <eblanc@...libre.com> wrote:
>
> > This adds a new driver for the Analog Devices INC. AD4030-24 ADC.
> > 
> > The driver implements basic support for the AD4030-24 1 channel
> > differential ADC with hardware gain and offset control.
> > 
> > Signed-off-by: Esteban Blanc <eblanc@...libre.com>
> Hi Esteban,
>
> Just one thing in here that actually matters. Question about scaling of
> the common channel.  The others I could tidy up whilst applying if
> nothing much else comes up.
>
> Jonathan
>
>
> > +static int ad4030_get_chan_scale(struct iio_dev *indio_dev,
> > +				 struct iio_chan_spec const *chan,
> > +				 int *val,
> > +				 int *val2)
> > +{
> > +	struct ad4030_state *st = iio_priv(indio_dev);
> > +
> > +	if (chan->differential) {
> > +		*val = (st->vref_uv * 2) / MILLI;
> > +		*val2 = st->chip->precision_bits;
> > +		return IIO_VAL_FRACTIONAL_LOG2;
> > +	}
> > +
> > +	*val = st->vref_uv / 256;
>
> This is a bit non obvious.
> A comment on this scaling might be good to have.
> Particularly the lack of / MILLI
> (I think that's a bug?)

Yes I think that should be:
``
*val = st->vref_uv / MILLI;
*val2 = 8;
return IIO_VAL_FRACTIONAL_LOG2;
``

So I guess that requires a V4. I will address the other comments there.

Thanks for your time,

-- 
Esteban "Skallwar" Blanc
BayLibre