Skip to contentSkip to conversation

iio: adc: New driver for TI ADS7950 chips

PATCH Superseded David Lechner 4 participants 6 messages Nov 20, 2016 – Nov 24, 2016 IIO

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

David Lechner

author
#0 lore raw

PATCH v2 1/1 iio: adc: New driver for TI ADS7950 chips

This adds a new driver for the TI ADS7950 family of ADC chips. These communicate using SPI and come in 8/10/12-bit and 4/8/12/16 channel varieties.

Signed-off-by David Lechner <[email protected]>

---

v2 changes:

* Got rid of XX wildcards - using ADS7950 everywhere
* Fixed some macro parentheses issues
* Added TI_ prefix to macros to match ti_ prefixes used elsewhere
* Added space in rx_buf for holding timestamp
* Use iio_device_claim_direct_mode() and spi_message_init_with_transfers() helper functions
* Don't use dev_info() at end of probe
* Minor spelling and code style fixes

 drivers/iio/adc/Kconfig      |  13 ++
 drivers/iio/adc/Makefile     |   1 +
 drivers/iio/adc/ti-ads7950.c | 488 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 502 insertions(+)
 create mode 100644 drivers/iio/adc/ti-ads7950.c
drivers/iio/adc/Kconfig +13 -0
index 6bbee0b..26b32f0 100644
@@ -527,6 +527,19 @@ config TI_ADS1015
 	  This driver can also be built as a module. If so, the module will be
 	  called ti-ads1015.
 
+config TI_ADS7950
+	tristate "Texas Instruments ADS7950 ADC driver"
+	depends on SPI
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	help
+	  Say yes here to build support for Texas Instruments ADS7950, ADS7951,
+	  ADS7952, ADS7953, ADS7954, ADS7955, ADS7956, ADS7957, ADS7958, ADS7959.
+	  ADS7960, ADS7961.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ti-ads7950.
+
 config TI_ADS8688
 	tristate "Texas Instruments ADS8688"
 	depends on SPI && OF
drivers/iio/adc/Makefile +1 -0
index 9391217..1966801 100644
@@ -49,6 +49,7 @@ obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
 obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
 obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o
 obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
+obj-$(CONFIG_TI_ADS7950) += ti-ads7950.o
 obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
 obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
 obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
drivers/iio/adc/ti-ads7950.c new file +488 -0

488 lines not on this page · raw message

Signature · 1 line
2.7.4
author
#1 lore raw
Quoted 24 lines · David Lechner

This adds a new driver for the TI ADS7950 family of ADC chips. These communicate using SPI and come in 8/10/12-bit and 4/8/12/16 channel varieties.

Signed-off-by David Lechner <[email protected]>

---

v2 changes:

* Got rid of XX wildcards - using ADS7950 everywhere
* Fixed some macro parentheses issues
* Added TI_ prefix to macros to match ti_ prefixes used elsewhere
* Added space in rx_buf for holding timestamp
* Use iio_device_claim_direct_mode() and spi_message_init_with_transfers() helper functions
* Don't use dev_info() at end of probe
* Minor spelling and code style fixes

 drivers/iio/adc/Kconfig      |  13 ++
 drivers/iio/adc/Makefile     |   1 +
 drivers/iio/adc/ti-ads7950.c | 488 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 502 insertions(+)
 create mode 100644 drivers/iio/adc/ti-ads7950.c

...

Quoted 5 lines
drivers/iio/adc/ti-ads7950.c new file +0 -0
new file mode 100644
index 0000000..d0b76bd

...

drivers/iio/adc/ti-ads7950.c line 276

Quoted 8 lines
drivers/iio/adc/ti-ads7950.c new file +8 -0
+static irqreturn_t ti_ads7950_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct ti_ads7950_state *st = iio_priv(indio_dev);
+	int b_sent;
+
+	b_sent = spi_sync(st->spi, &st->ring_msg);

hmm, I copied this from another driver, but spi_sync() in IRQ handler does not sound like a good idea (spi_sync() can sleep). I will replace it with spi_async().

Quoted 11 lines
drivers/iio/adc/ti-ads7950.c new file +11 -0
+	if (b_sent)
+		goto done;
+
+	iio_push_to_buffers_with_timestamp(indio_dev, st->rx_buf,
+					   iio_get_time_ns(indio_dev));
+
+done:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}

...

Quoted 13 lines
drivers/iio/adc/ti-ads7950.c new file +13 -0
+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;

drivers/iio/adc/ti-ads7950.c line 341

Quoted 10 lines
drivers/iio/adc/ti-ads7950.c new file +10 -0
+
+		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?

Quoted 15 lines
drivers/iio/adc/ti-ads7950.c new file +15 -0
+
+		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;
+}
#2 lore raw
Quoted 23 lines
Quoted 23 lines
drivers/iio/adc/ti-ads7950.c new file +22 -0
+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;
+
Quoted 8 lines

drivers/iio/adc/ti-ads7950.c line 341

Quoted 1 line
drivers/iio/adc/ti-ads7950.c new file +1 -0
+		*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

Quoted 15 lines
Quoted 15 lines
drivers/iio/adc/ti-ads7950.c new file +15 -0
+
+		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;
+}
Signature · 2 lines
Peter Meerwald-Stadler
+43-664-2444418 (mobile)
#3 lore raw
Quoted 30 lines · Peter Meerwald-Stadler
Quoted 18 lines
Quoted 18 lines
drivers/iio/adc/ti-ads7950.c new file +17 -0
+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)
Quoted 8 lines

drivers/iio/adc/ti-ads7950.c line 341

Quoted 6 lines
drivers/iio/adc/ti-ads7950.c new file +6 -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

Quoted 1 line

this chip. The 8- and 10-bit versions still return a 12-bit number

where the

Quoted 1 line

last 4 or 2 bits are always 0. Should I be shifting the 12-bit value

Quoted 8 lines

here

Quoted 1 line

based on the chip being used so that *val is 0-255 for 8-bit and

0-1023 for

Quoted 1 line

10-bit? Or should this be *really* raw and not even use

TI_ADS7950_EXTRACT()

Quoted 1 line

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'd slightly prefer the doing nothing route but don't really care as both are valid uses of the ABI.

Jonathan

Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
drivers/iio/adc/ti-ads7950.c new file +15 -0
+
+		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;
+}
Signature · 1 line
Sent from my Android device with K-9 Mail. Please excuse my brevity.
author
#4 lore raw
Quoted 35 lines · Jonathan Cameron
Quoted 32 lines · Peter Meerwald-Stadler
Quoted 18 lines
Quoted 18 lines
drivers/iio/adc/ti-ads7950.c new file +17 -0
+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)
Quoted 8 lines

drivers/iio/adc/ti-ads7950.c line 341

Quoted 6 lines
drivers/iio/adc/ti-ads7950.c new file +6 -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

Quoted 1 line

this chip. The 8- and 10-bit versions still return a 12-bit number

where the

Quoted 1 line

last 4 or 2 bits are always 0. Should I be shifting the 12-bit value

here

Quoted 1 line

based on the chip being used so that *val is 0-255 for 8-bit and

Quoted 8 lines
Quoted 6 lines

0-1023 for

Quoted 1 line

10-bit? Or should this be *really* raw and not even use

TI_ADS7950_EXTRACT()

Quoted 1 line

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.

Quoted 21 lines

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

Jonathan

Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
drivers/iio/adc/ti-ads7950.c new file +15 -0
+
+		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;
+}
maintainer
#5 lore raw
In reply to David Lechner
Quoted 36 lines · David Lechner
Quoted 24 lines · David Lechner

This adds a new driver for the TI ADS7950 family of ADC chips. These communicate using SPI and come in 8/10/12-bit and 4/8/12/16 channel varieties.

Signed-off-by David Lechner <[email protected]>

---

v2 changes:

* Got rid of XX wildcards - using ADS7950 everywhere
* Fixed some macro parentheses issues
* Added TI_ prefix to macros to match ti_ prefixes used elsewhere
* Added space in rx_buf for holding timestamp
* Use iio_device_claim_direct_mode() and spi_message_init_with_transfers() helper functions
* Don't use dev_info() at end of probe
* Minor spelling and code style fixes

 drivers/iio/adc/Kconfig      |  13 ++
 drivers/iio/adc/Makefile     |   1 +
 drivers/iio/adc/ti-ads7950.c | 488 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 502 insertions(+)
 create mode 100644 drivers/iio/adc/ti-ads7950.c

...

Quoted 5 lines
drivers/iio/adc/ti-ads7950.c new file +0 -0
new file mode 100644
index 0000000..d0b76bd

...

Quoted 10 lines

drivers/iio/adc/ti-ads7950.c line 276

Quoted 8 lines
drivers/iio/adc/ti-ads7950.c new file +8 -0
+static irqreturn_t ti_ads7950_trigger_handler(int irq, void *p)
+{
+    struct iio_poll_func *pf = p;
+    struct iio_dev *indio_dev = pf->indio_dev;
+    struct ti_ads7950_state *st = iio_priv(indio_dev);
+    int b_sent;
+
+    b_sent = spi_sync(st->spi, &st->ring_msg);

hmm, I copied this from another driver, but spi_sync() in IRQ handler does not sound like a good idea (spi_sync() can sleep). I will replace it with spi_async().

Umm.. spi_async doesn't make any sense here...

Key thing is that here you are in a threaded interrupt so sleeping is fine and here I'd imagine it leads to simpler code.

It's a bit old but https://lwn.net/Articles/302043/ has a good description.

Quoted 67 lines
Quoted 13 lines
drivers/iio/adc/ti-ads7950.c new file +11 -0
+    if (b_sent)
+        goto done;
+
+    iio_push_to_buffers_with_timestamp(indio_dev, st->rx_buf,
+                       iio_get_time_ns(indio_dev));
+
+done:
+    iio_trigger_notify_done(indio_dev->trig);
+
+    return IRQ_HANDLED;
+}

...

Quoted 13 lines
drivers/iio/adc/ti-ads7950.c new file +13 -0
+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;

drivers/iio/adc/ti-ads7950.c line 341

Quoted 10 lines
drivers/iio/adc/ti-ads7950.c new file +10 -0
+
+        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?

Quoted 15 lines
drivers/iio/adc/ti-ads7950.c new file +15 -0
+
+        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;
+}
Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html