iio: adc: ti-ads7950: allow simultaneous use of buffer and direct mode

PATCH Applied David Lechner 2 participants 5 messages Jul 21, 2018 – Jul 22, 2018 IIO

Share & tools

David Lechner

author

2018-07-21 19:37 UTC #0 lore raw

v2 cover iio: adc: ti-ads7950: allow simultaneous use of buffer and direct mode

v2 changes:
- add new patch to use SPDX-License-Identifier (since we are touching this file anyway)
- don't add struct iio_device *indio_dev as field to private data
- fix indent on return statement

David Lechner (2):
iio: adc: ti-ads7950: use SPDX-License-Identifier
iio: adc: ti-ads7950: allow simultaneous use of buffer and direct mode

 drivers/iio/adc/ti-ads7950.c | 42 ++++++++++++++++--------------------
 1 file changed, 19 insertions(+), 23 deletions(-)

Signature · 1 line

2.17.1

David Lechner

author

2018-07-21 19:38 UTC #1 lore raw

PATCH v2 1/2 iio: adc: ti-ads7950: use SPDX-License-Identifier

This updates the ti-ads7950.c file to use SPDX-License-Identifier instead of more verbose license text.

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

---
 drivers/iio/adc/ti-ads7950.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

drivers/iio/adc/ti-ads7950.c +1 -9

index 0225c1b333ab..f08c4de2bd10 100644
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * Texas Instruments ADS7950 SPI ADC driver
  *

@@ -10,15 +11,6 @@
  * And also on hwmon/ads79xx.c
  * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com/
  *	Nishanth Menon
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation version 2.
- *
- * This program is distributed "as is" WITHOUT ANY WARRANTY of any
- * kind, whether express or implied; without even the implied warranty
- * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
  */
 
 #include <linux/acpi.h>

Signature · 1 line

2.17.1

Jonathan Cameron

maintainer Applied

2018-07-22 07:40 UTC #2 lore raw

Re: [PATCH v2 1/2] iio: adc: ti-ads7950: use SPDX-License-Identifier

Quoted 4 lines · On Sat, 21 Jul 2018 14:37:33 -0500 David Lechner <[email protected]> wrote:

This updates the ti-ads7950.c file to use SPDX-License-Identifier instead of more verbose license text.
Signed-off-by David Lechner <[email protected]>

Applied, thanks,

Jonathan

Quoted 29 lines

---
 drivers/iio/adc/ti-ads7950.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

drivers/iio/adc/ti-ads7950.c +1 -9

index 0225c1b333ab..f08c4de2bd10 100644
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * Texas Instruments ADS7950 SPI ADC driver
  *

@@ -10,15 +11,6 @@
  * And also on hwmon/ads79xx.c
  * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com/
  *	Nishanth Menon
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation version 2.
- *
- * This program is distributed "as is" WITHOUT ANY WARRANTY of any
- * kind, whether express or implied; without even the implied warranty
- * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
  */
 
 #include <linux/acpi.h>

David Lechner

author

2018-07-21 19:37 UTC #3 lore raw

PATCH v2 2/2 iio: adc: ti-ads7950: allow simultaneous use of buffer and direct mode

In reply to David Lechner

This modifies the TI ADS7950 A/DC driver to allow the simultaneous use of both the triggered buffer and reading channels directly (via in- kernel API or sysfs).

The use case for this is on LEGO MINDSTORMS EV3. Two of the voltage channels are used in-kernel by a power supply driver, which reads the values using iio_read_channel_processed(). These channels are only read at a slow rate (<= 1Hz). However, we want to be able to read 12 other channels at the same time using the triggered buffer at a high rate (>= 100Hz).

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

---
 drivers/iio/adc/ti-ads7950.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

drivers/iio/adc/ti-ads7950.c +18 ~~-14~~

index f08c4de2bd10..eae987bef994 100644
@@ -60,6 +60,8 @@ struct ti_ads7950_state {
 	unsigned int		vref_mv;
 
 	unsigned int		settings;
+	__be16			single_tx;
+	__be16			single_rx;
 
 	/*
 	 * DMA (thus cache coherency maintenance) requires the

@@ -287,18 +289,26 @@ static irqreturn_t ti_ads7950_trigger_handler(int irq, void *p)
 	return IRQ_HANDLED;
 }
 
-static int ti_ads7950_scan_direct(struct ti_ads7950_state *st, unsigned int ch)
+static int ti_ads7950_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
 {
+	struct ti_ads7950_state *st = iio_priv(indio_dev);
 	int ret, cmd;
 
+	mutex_lock(&indio_dev->mlock);
+
 	cmd = TI_ADS7950_CR_WRITE | TI_ADS7950_CR_CHAN(ch) | st->settings;
-	st->tx_buf[0] = cpu_to_be16(cmd);
+	st->single_tx = cpu_to_be16(cmd);
 
 	ret = spi_sync(st->spi, &st->scan_single_msg);
 	if (ret)
-		return ret;
+		goto out;
+
+	ret = be16_to_cpu(st->single_rx);
+
+out:
+	mutex_unlock(&indio_dev->mlock);
 
-	return be16_to_cpu(st->rx_buf[0]);
+	return ret;
 }
 
 static int ti_ads7950_get_range(struct ti_ads7950_state *st)

@@ -330,13 +340,7 @@ static int ti_ads7950_read_raw(struct iio_dev *indio_dev,
 
 	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);
+		ret = ti_ads7950_scan_direct(indio_dev, chan->address);
 		if (ret < 0)
 			return ret;

@@ -402,13 +406,13 @@ static int ti_ads7950_probe(struct spi_device *spi)
 	 * was read at the end of the first transfer.
 	 */
 
-	st->scan_single_xfer[0].tx_buf = &st->tx_buf[0];
+	st->scan_single_xfer[0].tx_buf = &st->single_tx;
 	st->scan_single_xfer[0].len = 2;
 	st->scan_single_xfer[0].cs_change = 1;
-	st->scan_single_xfer[1].tx_buf = &st->tx_buf[0];
+	st->scan_single_xfer[1].tx_buf = &st->single_tx;
 	st->scan_single_xfer[1].len = 2;
 	st->scan_single_xfer[1].cs_change = 1;
-	st->scan_single_xfer[2].rx_buf = &st->rx_buf[0];
+	st->scan_single_xfer[2].rx_buf = &st->single_rx;
 	st->scan_single_xfer[2].len = 2;
 
 	spi_message_init_with_transfers(&st->scan_single_msg,

Signature · 1 line

2.17.1

Jonathan Cameron

maintainer

2018-07-22 07:46 UTC #4 lore raw

Quoted 4 lines · On Sat, 21 Jul 2018 14:37:34 -0500 David Lechner <[email protected]> wrote:

This modifies the TI ADS7950 A/DC driver to allow the simultaneous use of both the triggered buffer and reading channels directly (via in- kernel API or sysfs).

Quoted 8 lines

The use case for this is on LEGO MINDSTORMS EV3. Two of the voltage channels are used in-kernel by a power supply driver, which reads the values using iio_read_channel_processed(). These channels are only read at a slow rate (<= 1Hz). However, we want to be able to read 12 other channels at the same time using the triggered buffer at a high rate (>= 100Hz).
Signed-off-by David Lechner <[email protected]>

One slight quirk I'd missed before. The two new buffers are used for dma and as such need to be in a cacheline separate from anything else that might be corrupted. I've just move them down to below the existing buffers which should accomplish this.

Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it.

Thanks,

Jonathan

Quoted 80 lines

---
 drivers/iio/adc/ti-ads7950.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

drivers/iio/adc/ti-ads7950.c +18 ~~-14~~

index f08c4de2bd10..eae987bef994 100644
@@ -60,6 +60,8 @@ struct ti_ads7950_state {
 	unsigned int		vref_mv;
 
 	unsigned int		settings;
+	__be16			single_tx;
+	__be16			single_rx;
 
 	/*
 	 * DMA (thus cache coherency maintenance) requires the

@@ -287,18 +289,26 @@ static irqreturn_t ti_ads7950_trigger_handler(int irq, void *p)
 	return IRQ_HANDLED;
 }
 
-static int ti_ads7950_scan_direct(struct ti_ads7950_state *st, unsigned int ch)
+static int ti_ads7950_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
 {
+	struct ti_ads7950_state *st = iio_priv(indio_dev);
 	int ret, cmd;
 
+	mutex_lock(&indio_dev->mlock);
+
 	cmd = TI_ADS7950_CR_WRITE | TI_ADS7950_CR_CHAN(ch) | st->settings;
-	st->tx_buf[0] = cpu_to_be16(cmd);
+	st->single_tx = cpu_to_be16(cmd);
 
 	ret = spi_sync(st->spi, &st->scan_single_msg);
 	if (ret)
-		return ret;
+		goto out;
+
+	ret = be16_to_cpu(st->single_rx);
+
+out:
+	mutex_unlock(&indio_dev->mlock);
 
-	return be16_to_cpu(st->rx_buf[0]);
+	return ret;
 }
 
 static int ti_ads7950_get_range(struct ti_ads7950_state *st)

@@ -330,13 +340,7 @@ static int ti_ads7950_read_raw(struct iio_dev *indio_dev,
 
 	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);
+		ret = ti_ads7950_scan_direct(indio_dev, chan->address);
 		if (ret < 0)
 			return ret;

@@ -402,13 +406,13 @@ static int ti_ads7950_probe(struct spi_device *spi)
 	 * was read at the end of the first transfer.
 	 */
 
-	st->scan_single_xfer[0].tx_buf = &st->tx_buf[0];
+	st->scan_single_xfer[0].tx_buf = &st->single_tx;
 	st->scan_single_xfer[0].len = 2;
 	st->scan_single_xfer[0].cs_change = 1;
-	st->scan_single_xfer[1].tx_buf = &st->tx_buf[0];
+	st->scan_single_xfer[1].tx_buf = &st->single_tx;
 	st->scan_single_xfer[1].len = 2;
 	st->scan_single_xfer[1].cs_change = 1;
-	st->scan_single_xfer[2].rx_buf = &st->rx_buf[0];
+	st->scan_single_xfer[2].rx_buf = &st->single_rx;
 	st->scan_single_xfer[2].len = 2;
 
 	spi_message_init_with_transfers(&st->scan_single_msg,