linux-kernel - Re: [PATCH v2 2/7] iio: adc: Add basic support for AD4170

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: <aCH2a53ClLnhPDrG@debian-BULLSEYE-live-builder-AMD64>
Date: Mon, 12 May 2025 10:23:55 -0300
From: Marcelo Schmitt <marcelo.schmitt1@...il.com>
To: Andy Shevchenko <andy@...nel.org>
Cc: Marcelo Schmitt <marcelo.schmitt@...log.com>, linux-iio@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	Ana-Maria Cusco <ana-maria.cusco@...log.com>, jic23@...nel.org,
	lars@...afoo.de, Michael.Hennerich@...log.com,
	dlechner@...libre.com, nuno.sa@...log.com, robh@...nel.org,
	krzk+dt@...nel.org, conor+dt@...nel.org
Subject: Re: [PATCH v2 2/7] iio: adc: Add basic support for AD4170

...
> 
> > +#define AD4170_EXT_CLOCK_MHZ_MIN			(1 * MEGA)
> > +#define AD4170_EXT_CLOCK_MHZ_MAX			(17 * MEGA)
> 
> Ditto. But are you sure that it's 17 and not 16?

The maximum external clock rates shown in the datasheet are 17 MHz.
The minimum clock low/high times are 27.6 ns, resulting in roughly 18 MHz.
So, the specifications indicate that 17 MHz clock would be okay to operate AD4170.

...
> ...
> 
> > +		nv = (u64)chan_info->input_range_uv * NANO;
> > +		lshift = (pga >> 3 & 1);  /* handle cases 8 and 9 */
> 
> 		lshift = !!(pga & BIT(3));
> 
> ?
> 
> > +		rshift = precision_bits - bipolar + (pga & 0x7) - lshift;
> 
> GENMASK() ?

		rshift = precision_bits - bipolar + (pga & GENMASK(2, 0)) - lshift;
?

> ...
> 
> This was really hard to review and I am sure I missed something.
> 
> Please, split this to 3+ patches (basic + feature1 + feature2 + ...).

Sorry about that. This part is so flexible that even basic ADC channel support
becomes quite complex due to the dynamic setup configurations and possible
combinations of input type (differential / pseudo-diff) and PGA. For v3, I split
calib gain, offset, and samp freq into separate patches. The PGA, though, is
harder to disentangle from the rest of the base driver.

Thanks,
Marcelo