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

[Marcelo Schmitt <marcelo.schmitt1@gmail.com>]

...
> 
> > +#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