Re: [PATCH v4 6/6] iio: adc: Add support for AD4000

[Jonathan Cameron <jic23@kernel.org>]

> > +struct ad4000_chip_info {
> > +	const char *dev_name;
> > +	struct iio_chan_spec chan_spec;
> > +	struct iio_chan_spec three_w_chan_spec;
> > +};  
> 
> I understand the reason for doing this, but it still seems a bit weird
> to me to have two different sets of specs for the same chip. I guess
> we'll see what Jonathan has to say about this.


It's very common, though for a different reason.

Normally it's for cases where we have events (threshold crossing as similar)
and the interrupt is optional. In those case we have channel specs with
and without the event.  In this case the change is small so maybe
the code to set it up on a copy of the chan spec would be fine.

This is simple though so I'd keep it this way.

> 
> > +
> > +static const struct ad4000_chip_info ad4000_chip_info = {
> > +	.dev_name = "ad4000",
> > +	.chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16, 0),
> > +	.three_w_chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16, 1),
> > +};
> 
> or could just replace all of this this will spi_w8r8() and have
> a one-line function.
Good point. I'd forgotten that existed.  Better still than
spi_write_then_read.

Glad we spotted some of the same things. Sometimes it's weird
and two reviews are entirely unrelated issues throughout!

Jonathan