Re: [PATCH v2] iio: adc: New driver for TI ADS7950 chips

From: David Lechner
Date: Tue Nov 22 2016 - 13:23:53 EST


On 11/22/2016 01:23 AM, Jonathan Cameron wrote:


On 21 November 2016 22:54:24 GMT+00:00, Peter Meerwald-Stadler <pmeerw@xxxxxxxxxx> wrote:

+static int ti_ads7950_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long m)
+{
+ struct ti_ads7950_state *st = iio_priv(indio_dev);
+ int ret;
+
+ switch (m) {
+ case IIO_CHAN_INFO_RAW:
+
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret < 0)
+ return ret;
+
+ ret = ti_ads7950_scan_direct(st, chan->address);
+ iio_device_release_direct_mode(indio_dev);
+ if (ret < 0)
+ return ret;
+
+ if (chan->address != TI_ADS7950_EXTRACT(ret, 12, 4))
+ return -EIO;
+
+ *val = TI_ADS7950_EXTRACT(ret, 0, 12);

I'm not sure if I am doing this right. There are 8- 10- and 12-bit
versions of
this chip. The 8- and 10-bit versions still return a 12-bit number
where the
last 4 or 2 bits are always 0. Should I be shifting the 12-bit value
here
based on the chip being used so that *val is 0-255 for 8-bit and
0-1023 for
10-bit? Or should this be *really* raw and not even use
TI_ADS7950_EXTRACT()
to mask the channel address bits?

I'd shift and adjust _SCALE so that *val * scale gives mV
It would also be fine to not do anything and let userspace deal with shifting and masking for
buffered data. Non buffered obviously still needs shifting and masking though!

I have sent a v3 patch already that does exactly this. Buffered data is untouched and unbuffered data is now correct with shifting and masking.


I'd slightly prefer the doing nothing route but don't really care as both are valid uses of the ABI.

Jonathan

+
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ ret = ti_ads7950_get_range(st);
+ if (ret < 0)
+ return ret;
+
+ *val = ret;
+ *val2 = chan->scan_type.realbits;
+
+ return IIO_VAL_FRACTIONAL_LOG2;
+ }
+
+ return -EINVAL;
+}