All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] iio: adc: add ADC12130/ADC12132/ADC12138 ADC driver
@ 2016-07-18 15:00 ` Akinobu Mita
  0 siblings, 0 replies; 10+ messages in thread
From: Akinobu Mita @ 2016-07-18 15:00 UTC (permalink / raw)
  To: linux-iio-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Akinobu Mita, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald

This adds Texas Instruments' ADC12130/ADC12132/ADC12138 12-bit plus
sign ADC driver.  I have tested with the ADC12138.  The ADC12130 and
ADC12132 are not tested but these are similar to ADC12138 except that
the mode programming instruction is a bit different.

Signed-off-by: Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Hartmut Knaack <knaack.h-Mmb7MZpHnFY@public.gmane.org>
Cc: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
Cc: Peter Meerwald <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>
---
 .../devicetree/bindings/iio/adc/ti-adc12138.txt    |  32 ++
 drivers/iio/adc/Kconfig                            |  12 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/ti-adc12138.c                      | 541 +++++++++++++++++++++
 4 files changed, 586 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
 create mode 100644 drivers/iio/adc/ti-adc12138.c

diff --git a/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt b/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
new file mode 100644
index 0000000..3a11d2a
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
@@ -0,0 +1,32 @@
+* Texas Instruments' ADC12130/ADC12132/ADC12138
+
+Required properties:
+ - compatible: Should be one of
+	* "ti,adc12130"
+	* "ti,adc12132"
+	* "ti,adc12138"
+ - reg: SPI chip select number for the device
+ - interrupts: Should contain interrupt for EOC (end of conversion)
+ - clocks: phandle to conversion clock input
+ - spi-max-frequency: Definision as per
+	Documentation/devicetree/bindings/spi/spi-bus.txt
+ - vref-p-supply: The regulator supply for positive analog voltage reference
+
+Optional properties:
+ - vref-n-supply: The regulator supply for negative analog voltage reference
+	If not specified, this is assumed to be analog ground.
+ - acquisition-time: The number of conversion clock periods for the S/H's
+	acquisition time.  Should be one of 6, 10, 18, 34.  If not specified,
+	default value of 10 is used.
+
+Example:
+adc@0 {
+	compatible = "ti,adc12138";
+	reg = <0>;
+	interrupts = <28 IRQ_TYPE_EDGE_RISING>;
+	interrupt-parent = <&gpio1>;
+	clocks = <&cclk>;
+	vref-p-supply = <&ldo4_reg>;
+	spi-max-frequency = <5000000>;
+	acquisition-time = <6>;
+};
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index fa0ff29..0783291 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -414,6 +414,18 @@ config TI_ADC0832
 	  This driver can also be built as a module. If so, the module will be
 	  called ti-adc0832.
 
+config TI_ADC12138
+	tristate "Texas Instruments ADC12130/ADC12132/ADC12138"
+	depends on SPI
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	help
+	  If you say yes here you get support for Texas Instruments ADC12130,
+	  ADC12132and ADC12138 chips.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called ti-adc12138.
+
 config TI_ADC128S052
 	tristate "Texas Instruments ADC128S052/ADC122S021/ADC124S021"
 	depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 599714b..3ae4ae3 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
 obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
 obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
 obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
+obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
 obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
 obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
 obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
diff --git a/drivers/iio/adc/ti-adc12138.c b/drivers/iio/adc/ti-adc12138.c
new file mode 100644
index 0000000..e7bd986
--- /dev/null
+++ b/drivers/iio/adc/ti-adc12138.c
@@ -0,0 +1,541 @@
+/*
+ * ADC12130/ADC12132/ADC12138 12-bit plus sign ADC driver
+ *
+ * Copyright (c) 2016 Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+ *
+ * This file is subject to the terms and conditions of version 2 of
+ * the GNU General Public License.  See the file COPYING in the main
+ * directory of this archive for more details.
+ *
+ * Datasheet: http://www.ti.com/lit/ds/symlink/adc12138.pdf
+ */
+
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/completion.h>
+#include <linux/clk.h>
+#include <linux/spi/spi.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/regulator/consumer.h>
+
+#define ADC12138_MODE_AUTO_CAL			0x08
+#define ADC12138_MODE_READ_STATUS		0x0c
+#define ADC12138_MODE_ACQUISITION_TIME_6	0x0e
+#define ADC12138_MODE_ACQUISITION_TIME_10	0x4e
+#define ADC12138_MODE_ACQUISITION_TIME_18	0x8e
+#define ADC12138_MODE_ACQUISITION_TIME_34	0xce
+
+#define ADC12138_STATUS_CAL			BIT(6)
+
+enum {
+	adc12130,
+	adc12132,
+	adc12138,
+};
+
+struct adc12138 {
+	struct spi_device *spi;
+	unsigned int id;
+	/* conversion clock */
+	struct clk *cclk;
+	/* positive analog voltage reference */
+	struct regulator *vref_p;
+	/* negative analog voltage reference */
+	struct regulator *vref_n;
+	struct mutex lock;
+	struct completion complete;
+	unsigned int acquisition_time;
+
+	u8 tx_buf[2] ____cacheline_aligned;
+	u8 rx_buf[2];
+};
+
+#define ADC12138_VOLTAGE_CHANNEL(chan)					\
+	{								\
+		.type = IIO_VOLTAGE,					\
+		.indexed = 1,						\
+		.channel = chan,					\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE)	\
+					| BIT(IIO_CHAN_INFO_OFFSET),	\
+		.scan_index = chan,					\
+		.scan_type = {						\
+			.sign = 's',					\
+			.realbits = 13,					\
+			.storagebits = 16,				\
+			.shift = 3,					\
+			.endianness = IIO_BE,				\
+		},							\
+	}
+
+#define ADC12138_VOLTAGE_CHANNEL_DIFF(chan1, chan2, si_offset)		\
+	{								\
+		.type = IIO_VOLTAGE,					\
+		.indexed = 1,						\
+		.channel = (chan1),					\
+		.channel2 = (chan2),					\
+		.differential = 1,					\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE)	\
+					| BIT(IIO_CHAN_INFO_OFFSET),	\
+		.scan_index = (chan1) + (si_offset),			\
+		.scan_type = {						\
+			.sign = 's',					\
+			.realbits = 13,					\
+			.storagebits = 16,				\
+			.shift = 3,					\
+			.endianness = IIO_BE,				\
+		},							\
+	}
+
+static const struct iio_chan_spec adc12132_channels[] = {
+	ADC12138_VOLTAGE_CHANNEL(0),
+	ADC12138_VOLTAGE_CHANNEL(1),
+	ADC12138_VOLTAGE_CHANNEL_DIFF(0, 1, 2),
+	ADC12138_VOLTAGE_CHANNEL_DIFF(1, 0, 2),
+	IIO_CHAN_SOFT_TIMESTAMP(4),
+};
+
+static const struct iio_chan_spec adc12138_channels[] = {
+	ADC12138_VOLTAGE_CHANNEL(0),
+	ADC12138_VOLTAGE_CHANNEL(1),
+	ADC12138_VOLTAGE_CHANNEL(2),
+	ADC12138_VOLTAGE_CHANNEL(3),
+	ADC12138_VOLTAGE_CHANNEL(4),
+	ADC12138_VOLTAGE_CHANNEL(5),
+	ADC12138_VOLTAGE_CHANNEL(6),
+	ADC12138_VOLTAGE_CHANNEL(7),
+	ADC12138_VOLTAGE_CHANNEL_DIFF(0, 1, 8),
+	ADC12138_VOLTAGE_CHANNEL_DIFF(1, 0, 8),
+	ADC12138_VOLTAGE_CHANNEL_DIFF(2, 3, 8),
+	ADC12138_VOLTAGE_CHANNEL_DIFF(3, 2, 8),
+	ADC12138_VOLTAGE_CHANNEL_DIFF(4, 5, 8),
+	ADC12138_VOLTAGE_CHANNEL_DIFF(5, 4, 8),
+	ADC12138_VOLTAGE_CHANNEL_DIFF(6, 7, 8),
+	ADC12138_VOLTAGE_CHANNEL_DIFF(7, 6, 8),
+	IIO_CHAN_SOFT_TIMESTAMP(16),
+};
+
+static int adc12138_mode_programming(struct adc12138 *adc, u8 mode,
+				     void *rx_buf, int len)
+{
+	struct spi_transfer xfer = {
+		.tx_buf = adc->tx_buf,
+		.rx_buf = adc->rx_buf,
+		.len = len,
+	};
+	int ret;
+
+	/* Skip disused bits for ADC12130 and ADC12132 */
+	if (adc->id != adc12138)
+		mode = (mode & 0xc0) | ((mode & 0x0f) << 2);
+
+	adc->tx_buf[0] = mode;
+
+	ret = spi_sync_transfer(adc->spi, &xfer, 1);
+	if (ret)
+		return ret;
+
+	memcpy(rx_buf, adc->rx_buf, len);
+
+	return 0;
+}
+
+static int adc12138_read_status(struct adc12138 *adc)
+{
+	u8 rx_buf[2];
+	int ret;
+
+	ret = adc12138_mode_programming(adc, ADC12138_MODE_READ_STATUS,
+					rx_buf, 2);
+	if (ret)
+		return ret;
+
+	return (rx_buf[0] << 1) | (rx_buf[1] >> 7);
+}
+
+static int __adc12138_start_conv(struct adc12138 *adc,
+				 struct iio_chan_spec const *channel,
+				 void *data, int len)
+
+{
+	const u8 ch_to_mux[] = { 0, 4, 1, 5, 2, 6, 3, 7 };
+	u8 mode = (ch_to_mux[channel->channel] << 4) |
+		  (channel->differential ? 0 : 0x80);
+
+	return adc12138_mode_programming(adc, mode, data, len);
+}
+
+static int adc12138_start_conv(struct adc12138 *adc,
+			       struct iio_chan_spec const *channel)
+{
+	u8 trash;
+
+	return __adc12138_start_conv(adc, channel, &trash, 1);
+}
+
+static int adc12138_start_and_read_conv(struct adc12138 *adc,
+					struct iio_chan_spec const *channel,
+					__be16 *data)
+{
+	return __adc12138_start_conv(adc, channel, data, 2);
+}
+
+static int adc12138_read_conv_data(struct adc12138 *adc, __be16 *value)
+{
+	/* Issue a read status instruction and read previous conversion data */
+	return adc12138_mode_programming(adc, ADC12138_MODE_READ_STATUS,
+					 value, sizeof(*value));
+}
+
+static int adc12138_wait_eoc(struct adc12138 *adc, unsigned long timeout)
+{
+	if (!wait_for_completion_timeout(&adc->complete, timeout))
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static int adc12138_adc_conversion(struct adc12138 *adc,
+				   struct iio_chan_spec const *channel,
+				   __be16 *value)
+{
+	int ret;
+
+	reinit_completion(&adc->complete);
+
+	ret = adc12138_start_conv(adc, channel);
+	if (ret)
+		return ret;
+
+	ret = adc12138_wait_eoc(adc, msecs_to_jiffies(100));
+	if (ret)
+		return ret;
+
+	return adc12138_read_conv_data(adc, value);
+}
+
+static int adc12138_read_raw(struct iio_dev *iio,
+			     struct iio_chan_spec const *channel, int *value,
+			     int *shift, long mask)
+{
+	struct adc12138 *adc = iio_priv(iio);
+	int ret;
+	__be16 data;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&adc->lock);
+		ret = adc12138_adc_conversion(adc, channel, &data);
+		mutex_unlock(&adc->lock);
+		if (ret)
+			return ret;
+
+		*value = sign_extend32(be16_to_cpu(data) >> 3, 12);
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		ret = regulator_get_voltage(adc->vref_p);
+		if (ret < 0)
+			return ret;
+		*value = ret;
+
+		if (!IS_ERR(adc->vref_n)) {
+			ret = regulator_get_voltage(adc->vref_n);
+			if (ret < 0)
+				return ret;
+			*value -= ret;
+		}
+
+		/* convert regulator output voltage to mV */
+		*value /= 1000;
+		*shift = channel->scan_type.realbits - 1;
+
+		return IIO_VAL_FRACTIONAL_LOG2;
+	case IIO_CHAN_INFO_OFFSET:
+		if (!IS_ERR(adc->vref_n)) {
+			*value = regulator_get_voltage(adc->vref_n);
+			if (*value < 0)
+				return *value;
+		} else {
+			*value = 0;
+		}
+
+		/* convert regulator output voltage to mV */
+		*value /= 1000;
+
+		return IIO_VAL_INT;
+	}
+
+	return -EINVAL;
+}
+
+static const struct iio_info adc12138_info = {
+	.read_raw = adc12138_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static int adc12138_init(struct adc12138 *adc)
+{
+	int ret;
+	int status;
+	u8 mode;
+	u8 trash;
+
+	reinit_completion(&adc->complete);
+
+	ret = adc12138_mode_programming(adc, ADC12138_MODE_AUTO_CAL, &trash, 1);
+	if (ret)
+		return ret;
+
+	/* data output at this time has no significance */
+	status = adc12138_read_status(adc);
+	if (status < 0)
+		return status;
+
+	adc12138_wait_eoc(adc, msecs_to_jiffies(100));
+
+	status = adc12138_read_status(adc);
+	if (status & ADC12138_STATUS_CAL) {
+		dev_warn(&adc->spi->dev,
+			"Auto Cal sequence is still in progress: %#x\n",
+			status);
+		return -EIO;
+	}
+
+	switch (adc->acquisition_time) {
+	case 6:
+		mode = ADC12138_MODE_ACQUISITION_TIME_6;
+		break;
+	case 10:
+		mode = ADC12138_MODE_ACQUISITION_TIME_10;
+		break;
+	case 18:
+		mode = ADC12138_MODE_ACQUISITION_TIME_18;
+		break;
+	case 34:
+		mode = ADC12138_MODE_ACQUISITION_TIME_34;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return adc12138_mode_programming(adc, mode, &trash, 1);
+}
+
+static irqreturn_t adc12138_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct adc12138 *adc = iio_priv(indio_dev);
+	__be16 data[20] = { };
+	__be16 trash;
+	int ret;
+	int scan_index;
+	int i = 0;
+
+	mutex_lock(&adc->lock);
+
+	for_each_set_bit(scan_index, indio_dev->active_scan_mask,
+			 indio_dev->masklength) {
+		const struct iio_chan_spec *scan_chan =
+				&indio_dev->channels[scan_index];
+
+		reinit_completion(&adc->complete);
+
+		ret = adc12138_start_and_read_conv(adc, scan_chan,
+						   i ? &data[i - 1] : &trash);
+		if (ret) {
+			dev_warn(&adc->spi->dev,
+				 "failed to start conversion\n");
+			goto out;
+		}
+
+		ret = adc12138_wait_eoc(adc, msecs_to_jiffies(100));
+		if (ret) {
+			dev_warn(&adc->spi->dev, "wait eoc timeout\n");
+			goto out;
+		}
+
+		i++;
+	}
+
+	if (i) {
+		ret = adc12138_read_conv_data(adc, &data[i - 1]);
+		if (ret) {
+			dev_warn(&adc->spi->dev,
+				 "failed to get conversion data\n");
+			goto out;
+		}
+	}
+
+	iio_push_to_buffers_with_timestamp(indio_dev, data, iio_get_time_ns());
+out:
+	mutex_unlock(&adc->lock);
+
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t adc12138_eoc_handler(int irq, void *p)
+{
+	struct iio_dev *indio_dev = p;
+	struct adc12138 *adc = iio_priv(indio_dev);
+
+	complete(&adc->complete);
+
+	return IRQ_HANDLED;
+}
+
+static int adc12138_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct adc12138 *adc;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	adc = iio_priv(indio_dev);
+	adc->spi = spi;
+	adc->id = spi_get_device_id(spi)->driver_data;
+	mutex_init(&adc->lock);
+	init_completion(&adc->complete);
+
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->info = &adc12138_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	switch (adc->id) {
+	case adc12130:
+	case adc12132:
+		indio_dev->channels = adc12132_channels;
+		indio_dev->num_channels = ARRAY_SIZE(adc12132_channels);
+		break;
+	case adc12138:
+		indio_dev->channels = adc12138_channels;
+		indio_dev->num_channels = ARRAY_SIZE(adc12138_channels);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32(spi->dev.of_node, "acquisition-time",
+				   &adc->acquisition_time);
+	if (ret)
+		adc->acquisition_time = 10;
+
+	adc->cclk = devm_clk_get(&spi->dev, NULL);
+	if (IS_ERR(adc->cclk))
+		return PTR_ERR(adc->cclk);
+
+	adc->vref_p = devm_regulator_get(&spi->dev, "vref-p");
+	if (IS_ERR(adc->vref_p))
+		return PTR_ERR(adc->vref_p);
+
+	adc->vref_n = devm_regulator_get_optional(&spi->dev, "vref-n");
+
+	ret = devm_request_irq(&spi->dev, spi->irq, adc12138_eoc_handler,
+			       IRQF_TRIGGER_RISING, indio_dev->name, indio_dev);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(adc->cclk);
+	if (ret)
+		return ret;
+
+	ret = regulator_enable(adc->vref_p);
+	if (ret)
+		goto error1;
+
+	if (!IS_ERR(adc->vref_n)) {
+		ret = regulator_enable(adc->vref_n);
+		if (ret)
+			goto error2;
+	}
+
+	ret = adc12138_init(adc);
+	if (ret)
+		goto error3;
+
+	spi_set_drvdata(spi, indio_dev);
+
+	ret = iio_triggered_buffer_setup(indio_dev, NULL,
+					 adc12138_trigger_handler, NULL);
+	if (ret)
+		goto error3;
+
+	ret = iio_device_register(indio_dev);
+	if (ret)
+		goto error4;
+
+	return 0;
+error4:
+	iio_triggered_buffer_cleanup(indio_dev);
+error3:
+	if (!IS_ERR(adc->vref_n))
+		regulator_disable(adc->vref_n);
+error2:
+	regulator_disable(adc->vref_p);
+error1:
+	clk_disable_unprepare(adc->cclk);
+
+	return ret;
+}
+
+static int adc12138_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct adc12138 *adc = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+	iio_triggered_buffer_cleanup(indio_dev);
+	if (!IS_ERR(adc->vref_n))
+		regulator_disable(adc->vref_n);
+	regulator_disable(adc->vref_p);
+	clk_disable_unprepare(adc->cclk);
+
+	return 0;
+}
+
+#ifdef CONFIG_OF
+
+static const struct of_device_id adc12138_dt_ids[] = {
+	{ .compatible = "ti,adc12130", },
+	{ .compatible = "ti,adc12132", },
+	{ .compatible = "ti,adc12138", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, adc12138_dt_ids);
+
+#endif
+
+static const struct spi_device_id adc12138_id[] = {
+	{ "adc12130", adc12130 },
+	{ "adc12132", adc12132 },
+	{ "adc12138", adc12138 },
+	{}
+};
+MODULE_DEVICE_TABLE(spi, adc12138_id);
+
+static struct spi_driver adc12138_driver = {
+	.driver = {
+		.name = "adc12138",
+		.of_match_table = of_match_ptr(adc12138_dt_ids),
+	},
+	.probe = adc12138_probe,
+	.remove = adc12138_remove,
+	.id_table = adc12138_id,
+};
+module_spi_driver(adc12138_driver);
+
+MODULE_AUTHOR("Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
+MODULE_DESCRIPTION("ADC12130/ADC12132/ADC12138 driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH] iio: adc: add ADC12130/ADC12132/ADC12138 ADC driver
@ 2016-07-18 15:00 ` Akinobu Mita
  0 siblings, 0 replies; 10+ messages in thread
From: Akinobu Mita @ 2016-07-18 15:00 UTC (permalink / raw)
  To: linux-iio, devicetree
  Cc: Akinobu Mita, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald

This adds Texas Instruments' ADC12130/ADC12132/ADC12138 12-bit plus
sign ADC driver.  I have tested with the ADC12138.  The ADC12130 and
ADC12132 are not tested but these are similar to ADC12138 except that
the mode programming instruction is a bit different.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>
Cc: Hartmut Knaack <knaack.h@gmx.de>
Cc: Lars-Peter Clausen <lars@metafoo.de>
Cc: Peter Meerwald <pmeerw@pmeerw.net>
---
 .../devicetree/bindings/iio/adc/ti-adc12138.txt    |  32 ++
 drivers/iio/adc/Kconfig                            |  12 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/ti-adc12138.c                      | 541 +++++++++++++++++++++
 4 files changed, 586 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
 create mode 100644 drivers/iio/adc/ti-adc12138.c

diff --git a/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt b/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
new file mode 100644
index 0000000..3a11d2a
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
@@ -0,0 +1,32 @@
+* Texas Instruments' ADC12130/ADC12132/ADC12138
+
+Required properties:
+ - compatible: Should be one of
+	* "ti,adc12130"
+	* "ti,adc12132"
+	* "ti,adc12138"
+ - reg: SPI chip select number for the device
+ - interrupts: Should contain interrupt for EOC (end of conversion)
+ - clocks: phandle to conversion clock input
+ - spi-max-frequency: Definision as per
+	Documentation/devicetree/bindings/spi/spi-bus.txt
+ - vref-p-supply: The regulator supply for positive analog voltage reference
+
+Optional properties:
+ - vref-n-supply: The regulator supply for negative analog voltage reference
+	If not specified, this is assumed to be analog ground.
+ - acquisition-time: The number of conversion clock periods for the S/H's
+	acquisition time.  Should be one of 6, 10, 18, 34.  If not specified,
+	default value of 10 is used.
+
+Example:
+adc@0 {
+	compatible = "ti,adc12138";
+	reg = <0>;
+	interrupts = <28 IRQ_TYPE_EDGE_RISING>;
+	interrupt-parent = <&gpio1>;
+	clocks = <&cclk>;
+	vref-p-supply = <&ldo4_reg>;
+	spi-max-frequency = <5000000>;
+	acquisition-time = <6>;
+};
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index fa0ff29..0783291 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -414,6 +414,18 @@ config TI_ADC0832
 	  This driver can also be built as a module. If so, the module will be
 	  called ti-adc0832.
 
+config TI_ADC12138
+	tristate "Texas Instruments ADC12130/ADC12132/ADC12138"
+	depends on SPI
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	help
+	  If you say yes here you get support for Texas Instruments ADC12130,
+	  ADC12132and ADC12138 chips.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called ti-adc12138.
+
 config TI_ADC128S052
 	tristate "Texas Instruments ADC128S052/ADC122S021/ADC124S021"
 	depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 599714b..3ae4ae3 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
 obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
 obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
 obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
+obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
 obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
 obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
 obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
diff --git a/drivers/iio/adc/ti-adc12138.c b/drivers/iio/adc/ti-adc12138.c
new file mode 100644
index 0000000..e7bd986
--- /dev/null
+++ b/drivers/iio/adc/ti-adc12138.c
@@ -0,0 +1,541 @@
+/*
+ * ADC12130/ADC12132/ADC12138 12-bit plus sign ADC driver
+ *
+ * Copyright (c) 2016 Akinobu Mita <akinobu.mita@gmail.com>
+ *
+ * This file is subject to the terms and conditions of version 2 of
+ * the GNU General Public License.  See the file COPYING in the main
+ * directory of this archive for more details.
+ *
+ * Datasheet: http://www.ti.com/lit/ds/symlink/adc12138.pdf
+ */
+
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/completion.h>
+#include <linux/clk.h>
+#include <linux/spi/spi.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/regulator/consumer.h>
+
+#define ADC12138_MODE_AUTO_CAL			0x08
+#define ADC12138_MODE_READ_STATUS		0x0c
+#define ADC12138_MODE_ACQUISITION_TIME_6	0x0e
+#define ADC12138_MODE_ACQUISITION_TIME_10	0x4e
+#define ADC12138_MODE_ACQUISITION_TIME_18	0x8e
+#define ADC12138_MODE_ACQUISITION_TIME_34	0xce
+
+#define ADC12138_STATUS_CAL			BIT(6)
+
+enum {
+	adc12130,
+	adc12132,
+	adc12138,
+};
+
+struct adc12138 {
+	struct spi_device *spi;
+	unsigned int id;
+	/* conversion clock */
+	struct clk *cclk;
+	/* positive analog voltage reference */
+	struct regulator *vref_p;
+	/* negative analog voltage reference */
+	struct regulator *vref_n;
+	struct mutex lock;
+	struct completion complete;
+	unsigned int acquisition_time;
+
+	u8 tx_buf[2] ____cacheline_aligned;
+	u8 rx_buf[2];
+};
+
+#define ADC12138_VOLTAGE_CHANNEL(chan)					\
+	{								\
+		.type = IIO_VOLTAGE,					\
+		.indexed = 1,						\
+		.channel = chan,					\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE)	\
+					| BIT(IIO_CHAN_INFO_OFFSET),	\
+		.scan_index = chan,					\
+		.scan_type = {						\
+			.sign = 's',					\
+			.realbits = 13,					\
+			.storagebits = 16,				\
+			.shift = 3,					\
+			.endianness = IIO_BE,				\
+		},							\
+	}
+
+#define ADC12138_VOLTAGE_CHANNEL_DIFF(chan1, chan2, si_offset)		\
+	{								\
+		.type = IIO_VOLTAGE,					\
+		.indexed = 1,						\
+		.channel = (chan1),					\
+		.channel2 = (chan2),					\
+		.differential = 1,					\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE)	\
+					| BIT(IIO_CHAN_INFO_OFFSET),	\
+		.scan_index = (chan1) + (si_offset),			\
+		.scan_type = {						\
+			.sign = 's',					\
+			.realbits = 13,					\
+			.storagebits = 16,				\
+			.shift = 3,					\
+			.endianness = IIO_BE,				\
+		},							\
+	}
+
+static const struct iio_chan_spec adc12132_channels[] = {
+	ADC12138_VOLTAGE_CHANNEL(0),
+	ADC12138_VOLTAGE_CHANNEL(1),
+	ADC12138_VOLTAGE_CHANNEL_DIFF(0, 1, 2),
+	ADC12138_VOLTAGE_CHANNEL_DIFF(1, 0, 2),
+	IIO_CHAN_SOFT_TIMESTAMP(4),
+};
+
+static const struct iio_chan_spec adc12138_channels[] = {
+	ADC12138_VOLTAGE_CHANNEL(0),
+	ADC12138_VOLTAGE_CHANNEL(1),
+	ADC12138_VOLTAGE_CHANNEL(2),
+	ADC12138_VOLTAGE_CHANNEL(3),
+	ADC12138_VOLTAGE_CHANNEL(4),
+	ADC12138_VOLTAGE_CHANNEL(5),
+	ADC12138_VOLTAGE_CHANNEL(6),
+	ADC12138_VOLTAGE_CHANNEL(7),
+	ADC12138_VOLTAGE_CHANNEL_DIFF(0, 1, 8),
+	ADC12138_VOLTAGE_CHANNEL_DIFF(1, 0, 8),
+	ADC12138_VOLTAGE_CHANNEL_DIFF(2, 3, 8),
+	ADC12138_VOLTAGE_CHANNEL_DIFF(3, 2, 8),
+	ADC12138_VOLTAGE_CHANNEL_DIFF(4, 5, 8),
+	ADC12138_VOLTAGE_CHANNEL_DIFF(5, 4, 8),
+	ADC12138_VOLTAGE_CHANNEL_DIFF(6, 7, 8),
+	ADC12138_VOLTAGE_CHANNEL_DIFF(7, 6, 8),
+	IIO_CHAN_SOFT_TIMESTAMP(16),
+};
+
+static int adc12138_mode_programming(struct adc12138 *adc, u8 mode,
+				     void *rx_buf, int len)
+{
+	struct spi_transfer xfer = {
+		.tx_buf = adc->tx_buf,
+		.rx_buf = adc->rx_buf,
+		.len = len,
+	};
+	int ret;
+
+	/* Skip disused bits for ADC12130 and ADC12132 */
+	if (adc->id != adc12138)
+		mode = (mode & 0xc0) | ((mode & 0x0f) << 2);
+
+	adc->tx_buf[0] = mode;
+
+	ret = spi_sync_transfer(adc->spi, &xfer, 1);
+	if (ret)
+		return ret;
+
+	memcpy(rx_buf, adc->rx_buf, len);
+
+	return 0;
+}
+
+static int adc12138_read_status(struct adc12138 *adc)
+{
+	u8 rx_buf[2];
+	int ret;
+
+	ret = adc12138_mode_programming(adc, ADC12138_MODE_READ_STATUS,
+					rx_buf, 2);
+	if (ret)
+		return ret;
+
+	return (rx_buf[0] << 1) | (rx_buf[1] >> 7);
+}
+
+static int __adc12138_start_conv(struct adc12138 *adc,
+				 struct iio_chan_spec const *channel,
+				 void *data, int len)
+
+{
+	const u8 ch_to_mux[] = { 0, 4, 1, 5, 2, 6, 3, 7 };
+	u8 mode = (ch_to_mux[channel->channel] << 4) |
+		  (channel->differential ? 0 : 0x80);
+
+	return adc12138_mode_programming(adc, mode, data, len);
+}
+
+static int adc12138_start_conv(struct adc12138 *adc,
+			       struct iio_chan_spec const *channel)
+{
+	u8 trash;
+
+	return __adc12138_start_conv(adc, channel, &trash, 1);
+}
+
+static int adc12138_start_and_read_conv(struct adc12138 *adc,
+					struct iio_chan_spec const *channel,
+					__be16 *data)
+{
+	return __adc12138_start_conv(adc, channel, data, 2);
+}
+
+static int adc12138_read_conv_data(struct adc12138 *adc, __be16 *value)
+{
+	/* Issue a read status instruction and read previous conversion data */
+	return adc12138_mode_programming(adc, ADC12138_MODE_READ_STATUS,
+					 value, sizeof(*value));
+}
+
+static int adc12138_wait_eoc(struct adc12138 *adc, unsigned long timeout)
+{
+	if (!wait_for_completion_timeout(&adc->complete, timeout))
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static int adc12138_adc_conversion(struct adc12138 *adc,
+				   struct iio_chan_spec const *channel,
+				   __be16 *value)
+{
+	int ret;
+
+	reinit_completion(&adc->complete);
+
+	ret = adc12138_start_conv(adc, channel);
+	if (ret)
+		return ret;
+
+	ret = adc12138_wait_eoc(adc, msecs_to_jiffies(100));
+	if (ret)
+		return ret;
+
+	return adc12138_read_conv_data(adc, value);
+}
+
+static int adc12138_read_raw(struct iio_dev *iio,
+			     struct iio_chan_spec const *channel, int *value,
+			     int *shift, long mask)
+{
+	struct adc12138 *adc = iio_priv(iio);
+	int ret;
+	__be16 data;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&adc->lock);
+		ret = adc12138_adc_conversion(adc, channel, &data);
+		mutex_unlock(&adc->lock);
+		if (ret)
+			return ret;
+
+		*value = sign_extend32(be16_to_cpu(data) >> 3, 12);
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		ret = regulator_get_voltage(adc->vref_p);
+		if (ret < 0)
+			return ret;
+		*value = ret;
+
+		if (!IS_ERR(adc->vref_n)) {
+			ret = regulator_get_voltage(adc->vref_n);
+			if (ret < 0)
+				return ret;
+			*value -= ret;
+		}
+
+		/* convert regulator output voltage to mV */
+		*value /= 1000;
+		*shift = channel->scan_type.realbits - 1;
+
+		return IIO_VAL_FRACTIONAL_LOG2;
+	case IIO_CHAN_INFO_OFFSET:
+		if (!IS_ERR(adc->vref_n)) {
+			*value = regulator_get_voltage(adc->vref_n);
+			if (*value < 0)
+				return *value;
+		} else {
+			*value = 0;
+		}
+
+		/* convert regulator output voltage to mV */
+		*value /= 1000;
+
+		return IIO_VAL_INT;
+	}
+
+	return -EINVAL;
+}
+
+static const struct iio_info adc12138_info = {
+	.read_raw = adc12138_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static int adc12138_init(struct adc12138 *adc)
+{
+	int ret;
+	int status;
+	u8 mode;
+	u8 trash;
+
+	reinit_completion(&adc->complete);
+
+	ret = adc12138_mode_programming(adc, ADC12138_MODE_AUTO_CAL, &trash, 1);
+	if (ret)
+		return ret;
+
+	/* data output at this time has no significance */
+	status = adc12138_read_status(adc);
+	if (status < 0)
+		return status;
+
+	adc12138_wait_eoc(adc, msecs_to_jiffies(100));
+
+	status = adc12138_read_status(adc);
+	if (status & ADC12138_STATUS_CAL) {
+		dev_warn(&adc->spi->dev,
+			"Auto Cal sequence is still in progress: %#x\n",
+			status);
+		return -EIO;
+	}
+
+	switch (adc->acquisition_time) {
+	case 6:
+		mode = ADC12138_MODE_ACQUISITION_TIME_6;
+		break;
+	case 10:
+		mode = ADC12138_MODE_ACQUISITION_TIME_10;
+		break;
+	case 18:
+		mode = ADC12138_MODE_ACQUISITION_TIME_18;
+		break;
+	case 34:
+		mode = ADC12138_MODE_ACQUISITION_TIME_34;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return adc12138_mode_programming(adc, mode, &trash, 1);
+}
+
+static irqreturn_t adc12138_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct adc12138 *adc = iio_priv(indio_dev);
+	__be16 data[20] = { };
+	__be16 trash;
+	int ret;
+	int scan_index;
+	int i = 0;
+
+	mutex_lock(&adc->lock);
+
+	for_each_set_bit(scan_index, indio_dev->active_scan_mask,
+			 indio_dev->masklength) {
+		const struct iio_chan_spec *scan_chan =
+				&indio_dev->channels[scan_index];
+
+		reinit_completion(&adc->complete);
+
+		ret = adc12138_start_and_read_conv(adc, scan_chan,
+						   i ? &data[i - 1] : &trash);
+		if (ret) {
+			dev_warn(&adc->spi->dev,
+				 "failed to start conversion\n");
+			goto out;
+		}
+
+		ret = adc12138_wait_eoc(adc, msecs_to_jiffies(100));
+		if (ret) {
+			dev_warn(&adc->spi->dev, "wait eoc timeout\n");
+			goto out;
+		}
+
+		i++;
+	}
+
+	if (i) {
+		ret = adc12138_read_conv_data(adc, &data[i - 1]);
+		if (ret) {
+			dev_warn(&adc->spi->dev,
+				 "failed to get conversion data\n");
+			goto out;
+		}
+	}
+
+	iio_push_to_buffers_with_timestamp(indio_dev, data, iio_get_time_ns());
+out:
+	mutex_unlock(&adc->lock);
+
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t adc12138_eoc_handler(int irq, void *p)
+{
+	struct iio_dev *indio_dev = p;
+	struct adc12138 *adc = iio_priv(indio_dev);
+
+	complete(&adc->complete);
+
+	return IRQ_HANDLED;
+}
+
+static int adc12138_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct adc12138 *adc;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	adc = iio_priv(indio_dev);
+	adc->spi = spi;
+	adc->id = spi_get_device_id(spi)->driver_data;
+	mutex_init(&adc->lock);
+	init_completion(&adc->complete);
+
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->info = &adc12138_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	switch (adc->id) {
+	case adc12130:
+	case adc12132:
+		indio_dev->channels = adc12132_channels;
+		indio_dev->num_channels = ARRAY_SIZE(adc12132_channels);
+		break;
+	case adc12138:
+		indio_dev->channels = adc12138_channels;
+		indio_dev->num_channels = ARRAY_SIZE(adc12138_channels);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32(spi->dev.of_node, "acquisition-time",
+				   &adc->acquisition_time);
+	if (ret)
+		adc->acquisition_time = 10;
+
+	adc->cclk = devm_clk_get(&spi->dev, NULL);
+	if (IS_ERR(adc->cclk))
+		return PTR_ERR(adc->cclk);
+
+	adc->vref_p = devm_regulator_get(&spi->dev, "vref-p");
+	if (IS_ERR(adc->vref_p))
+		return PTR_ERR(adc->vref_p);
+
+	adc->vref_n = devm_regulator_get_optional(&spi->dev, "vref-n");
+
+	ret = devm_request_irq(&spi->dev, spi->irq, adc12138_eoc_handler,
+			       IRQF_TRIGGER_RISING, indio_dev->name, indio_dev);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(adc->cclk);
+	if (ret)
+		return ret;
+
+	ret = regulator_enable(adc->vref_p);
+	if (ret)
+		goto error1;
+
+	if (!IS_ERR(adc->vref_n)) {
+		ret = regulator_enable(adc->vref_n);
+		if (ret)
+			goto error2;
+	}
+
+	ret = adc12138_init(adc);
+	if (ret)
+		goto error3;
+
+	spi_set_drvdata(spi, indio_dev);
+
+	ret = iio_triggered_buffer_setup(indio_dev, NULL,
+					 adc12138_trigger_handler, NULL);
+	if (ret)
+		goto error3;
+
+	ret = iio_device_register(indio_dev);
+	if (ret)
+		goto error4;
+
+	return 0;
+error4:
+	iio_triggered_buffer_cleanup(indio_dev);
+error3:
+	if (!IS_ERR(adc->vref_n))
+		regulator_disable(adc->vref_n);
+error2:
+	regulator_disable(adc->vref_p);
+error1:
+	clk_disable_unprepare(adc->cclk);
+
+	return ret;
+}
+
+static int adc12138_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct adc12138 *adc = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+	iio_triggered_buffer_cleanup(indio_dev);
+	if (!IS_ERR(adc->vref_n))
+		regulator_disable(adc->vref_n);
+	regulator_disable(adc->vref_p);
+	clk_disable_unprepare(adc->cclk);
+
+	return 0;
+}
+
+#ifdef CONFIG_OF
+
+static const struct of_device_id adc12138_dt_ids[] = {
+	{ .compatible = "ti,adc12130", },
+	{ .compatible = "ti,adc12132", },
+	{ .compatible = "ti,adc12138", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, adc12138_dt_ids);
+
+#endif
+
+static const struct spi_device_id adc12138_id[] = {
+	{ "adc12130", adc12130 },
+	{ "adc12132", adc12132 },
+	{ "adc12138", adc12138 },
+	{}
+};
+MODULE_DEVICE_TABLE(spi, adc12138_id);
+
+static struct spi_driver adc12138_driver = {
+	.driver = {
+		.name = "adc12138",
+		.of_match_table = of_match_ptr(adc12138_dt_ids),
+	},
+	.probe = adc12138_probe,
+	.remove = adc12138_remove,
+	.id_table = adc12138_id,
+};
+module_spi_driver(adc12138_driver);
+
+MODULE_AUTHOR("Akinobu Mita <akinobu.mita@gmail.com>");
+MODULE_DESCRIPTION("ADC12130/ADC12132/ADC12138 driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] iio: adc: add ADC12130/ADC12132/ADC12138 ADC driver
  2016-07-18 15:00 ` Akinobu Mita
@ 2016-07-18 22:02     ` Peter Meerwald-Stadler
  -1 siblings, 0 replies; 10+ messages in thread
From: Peter Meerwald-Stadler @ 2016-07-18 22:02 UTC (permalink / raw)
  To: Akinobu Mita
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron,
	Hartmut Knaack, Lars-Peter Clausen

[-- Attachment #1: Type: TEXT/PLAIN, Size: 18479 bytes --]


> This adds Texas Instruments' ADC12130/ADC12132/ADC12138 12-bit plus
> sign ADC driver.  I have tested with the ADC12138.  The ADC12130 and
> ADC12132 are not tested but these are similar to ADC12138 except that
> the mode programming instruction is a bit different.

minor comments below

> Signed-off-by: Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Hartmut Knaack <knaack.h-Mmb7MZpHnFY@public.gmane.org>
> Cc: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
> Cc: Peter Meerwald <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>
> ---
>  .../devicetree/bindings/iio/adc/ti-adc12138.txt    |  32 ++
>  drivers/iio/adc/Kconfig                            |  12 +
>  drivers/iio/adc/Makefile                           |   1 +
>  drivers/iio/adc/ti-adc12138.c                      | 541 +++++++++++++++++++++
>  4 files changed, 586 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
>  create mode 100644 drivers/iio/adc/ti-adc12138.c
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt b/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
> new file mode 100644
> index 0000000..3a11d2a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
> @@ -0,0 +1,32 @@
> +* Texas Instruments' ADC12130/ADC12132/ADC12138
> +
> +Required properties:
> + - compatible: Should be one of
> +	* "ti,adc12130"
> +	* "ti,adc12132"
> +	* "ti,adc12138"
> + - reg: SPI chip select number for the device
> + - interrupts: Should contain interrupt for EOC (end of conversion)
> + - clocks: phandle to conversion clock input
> + - spi-max-frequency: Definision as per
> +	Documentation/devicetree/bindings/spi/spi-bus.txt
> + - vref-p-supply: The regulator supply for positive analog voltage reference
> +
> +Optional properties:
> + - vref-n-supply: The regulator supply for negative analog voltage reference
> +	If not specified, this is assumed to be analog ground.
> + - acquisition-time: The number of conversion clock periods for the S/H's
> +	acquisition time.  Should be one of 6, 10, 18, 34.  If not specified,
> +	default value of 10 is used.
> +
> +Example:
> +adc@0 {
> +	compatible = "ti,adc12138";
> +	reg = <0>;
> +	interrupts = <28 IRQ_TYPE_EDGE_RISING>;
> +	interrupt-parent = <&gpio1>;
> +	clocks = <&cclk>;
> +	vref-p-supply = <&ldo4_reg>;
> +	spi-max-frequency = <5000000>;
> +	acquisition-time = <6>;
> +};
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index fa0ff29..0783291 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -414,6 +414,18 @@ config TI_ADC0832
>  	  This driver can also be built as a module. If so, the module will be
>  	  called ti-adc0832.
>  
> +config TI_ADC12138
> +	tristate "Texas Instruments ADC12130/ADC12132/ADC12138"
> +	depends on SPI
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
> +	help
> +	  If you say yes here you get support for Texas Instruments ADC12130,
> +	  ADC12132and ADC12138 chips.

space before 'and' missing

> +
> +	  This driver can also be built as a module. If so, the module will be
> +	  called ti-adc12138.
> +
>  config TI_ADC128S052
>  	tristate "Texas Instruments ADC128S052/ADC122S021/ADC124S021"
>  	depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 599714b..3ae4ae3 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -40,6 +40,7 @@ obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
>  obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
>  obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>  obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
> +obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
>  obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
>  obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
>  obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
> diff --git a/drivers/iio/adc/ti-adc12138.c b/drivers/iio/adc/ti-adc12138.c
> new file mode 100644
> index 0000000..e7bd986
> --- /dev/null
> +++ b/drivers/iio/adc/ti-adc12138.c
> @@ -0,0 +1,541 @@
> +/*
> + * ADC12130/ADC12132/ADC12138 12-bit plus sign ADC driver
> + *
> + * Copyright (c) 2016 Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License.  See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * Datasheet: http://www.ti.com/lit/ds/symlink/adc12138.pdf
> + */
> +
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/completion.h>
> +#include <linux/clk.h>
> +#include <linux/spi/spi.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/regulator/consumer.h>
> +
> +#define ADC12138_MODE_AUTO_CAL			0x08
> +#define ADC12138_MODE_READ_STATUS		0x0c
> +#define ADC12138_MODE_ACQUISITION_TIME_6	0x0e
> +#define ADC12138_MODE_ACQUISITION_TIME_10	0x4e
> +#define ADC12138_MODE_ACQUISITION_TIME_18	0x8e
> +#define ADC12138_MODE_ACQUISITION_TIME_34	0xce
> +
> +#define ADC12138_STATUS_CAL			BIT(6)
> +
> +enum {
> +	adc12130,
> +	adc12132,
> +	adc12138,
> +};
> +
> +struct adc12138 {
> +	struct spi_device *spi;
> +	unsigned int id;
> +	/* conversion clock */
> +	struct clk *cclk;
> +	/* positive analog voltage reference */
> +	struct regulator *vref_p;
> +	/* negative analog voltage reference */
> +	struct regulator *vref_n;
> +	struct mutex lock;
> +	struct completion complete;
> +	unsigned int acquisition_time;

unit? ms? µs?

> +
> +	u8 tx_buf[2] ____cacheline_aligned;
> +	u8 rx_buf[2];
> +};
> +
> +#define ADC12138_VOLTAGE_CHANNEL(chan)					\
> +	{								\
> +		.type = IIO_VOLTAGE,					\
> +		.indexed = 1,						\
> +		.channel = chan,					\
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE)	\
> +					| BIT(IIO_CHAN_INFO_OFFSET),	\
> +		.scan_index = chan,					\
> +		.scan_type = {						\
> +			.sign = 's',					\
> +			.realbits = 13,					\
> +			.storagebits = 16,				\
> +			.shift = 3,					\
> +			.endianness = IIO_BE,				\
> +		},							\
> +	}
> +
> +#define ADC12138_VOLTAGE_CHANNEL_DIFF(chan1, chan2, si_offset)		\
> +	{								\
> +		.type = IIO_VOLTAGE,					\
> +		.indexed = 1,						\
> +		.channel = (chan1),					\
> +		.channel2 = (chan2),					\
> +		.differential = 1,					\
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE)	\
> +					| BIT(IIO_CHAN_INFO_OFFSET),	\
> +		.scan_index = (chan1) + (si_offset),			\
> +		.scan_type = {						\
> +			.sign = 's',					\
> +			.realbits = 13,					\
> +			.storagebits = 16,				\
> +			.shift = 3,					\
> +			.endianness = IIO_BE,				\
> +		},							\
> +	}
> +
> +static const struct iio_chan_spec adc12132_channels[] = {
> +	ADC12138_VOLTAGE_CHANNEL(0),
> +	ADC12138_VOLTAGE_CHANNEL(1),
> +	ADC12138_VOLTAGE_CHANNEL_DIFF(0, 1, 2),
> +	ADC12138_VOLTAGE_CHANNEL_DIFF(1, 0, 2),
> +	IIO_CHAN_SOFT_TIMESTAMP(4),
> +};
> +
> +static const struct iio_chan_spec adc12138_channels[] = {
> +	ADC12138_VOLTAGE_CHANNEL(0),
> +	ADC12138_VOLTAGE_CHANNEL(1),
> +	ADC12138_VOLTAGE_CHANNEL(2),
> +	ADC12138_VOLTAGE_CHANNEL(3),
> +	ADC12138_VOLTAGE_CHANNEL(4),
> +	ADC12138_VOLTAGE_CHANNEL(5),
> +	ADC12138_VOLTAGE_CHANNEL(6),
> +	ADC12138_VOLTAGE_CHANNEL(7),
> +	ADC12138_VOLTAGE_CHANNEL_DIFF(0, 1, 8),
> +	ADC12138_VOLTAGE_CHANNEL_DIFF(1, 0, 8),
> +	ADC12138_VOLTAGE_CHANNEL_DIFF(2, 3, 8),
> +	ADC12138_VOLTAGE_CHANNEL_DIFF(3, 2, 8),
> +	ADC12138_VOLTAGE_CHANNEL_DIFF(4, 5, 8),
> +	ADC12138_VOLTAGE_CHANNEL_DIFF(5, 4, 8),
> +	ADC12138_VOLTAGE_CHANNEL_DIFF(6, 7, 8),
> +	ADC12138_VOLTAGE_CHANNEL_DIFF(7, 6, 8),
> +	IIO_CHAN_SOFT_TIMESTAMP(16),
> +};
> +
> +static int adc12138_mode_programming(struct adc12138 *adc, u8 mode,
> +				     void *rx_buf, int len)
> +{
> +	struct spi_transfer xfer = {
> +		.tx_buf = adc->tx_buf,
> +		.rx_buf = adc->rx_buf,
> +		.len = len,
> +	};
> +	int ret;
> +
> +	/* Skip disused bits for ADC12130 and ADC12132 */

probably 'unused' instead of 'disused'

> +	if (adc->id != adc12138)
> +		mode = (mode & 0xc0) | ((mode & 0x0f) << 2);
> +
> +	adc->tx_buf[0] = mode;
> +
> +	ret = spi_sync_transfer(adc->spi, &xfer, 1);
> +	if (ret)
> +		return ret;
> +
> +	memcpy(rx_buf, adc->rx_buf, len);
> +
> +	return 0;
> +}
> +
> +static int adc12138_read_status(struct adc12138 *adc)
> +{
> +	u8 rx_buf[2];
> +	int ret;
> +
> +	ret = adc12138_mode_programming(adc, ADC12138_MODE_READ_STATUS,
> +					rx_buf, 2);
> +	if (ret)
> +		return ret;
> +
> +	return (rx_buf[0] << 1) | (rx_buf[1] >> 7);
> +}
> +
> +static int __adc12138_start_conv(struct adc12138 *adc,
> +				 struct iio_chan_spec const *channel,
> +				 void *data, int len)
> +
> +{
> +	const u8 ch_to_mux[] = { 0, 4, 1, 5, 2, 6, 3, 7 };
> +	u8 mode = (ch_to_mux[channel->channel] << 4) |
> +		  (channel->differential ? 0 : 0x80);
> +
> +	return adc12138_mode_programming(adc, mode, data, len);
> +}
> +
> +static int adc12138_start_conv(struct adc12138 *adc,
> +			       struct iio_chan_spec const *channel)
> +{
> +	u8 trash;
> +
> +	return __adc12138_start_conv(adc, channel, &trash, 1);
> +}
> +
> +static int adc12138_start_and_read_conv(struct adc12138 *adc,
> +					struct iio_chan_spec const *channel,
> +					__be16 *data)
> +{
> +	return __adc12138_start_conv(adc, channel, data, 2);
> +}
> +
> +static int adc12138_read_conv_data(struct adc12138 *adc, __be16 *value)
> +{
> +	/* Issue a read status instruction and read previous conversion data */
> +	return adc12138_mode_programming(adc, ADC12138_MODE_READ_STATUS,
> +					 value, sizeof(*value));
> +}
> +
> +static int adc12138_wait_eoc(struct adc12138 *adc, unsigned long timeout)
> +{
> +	if (!wait_for_completion_timeout(&adc->complete, timeout))
> +		return -ETIMEDOUT;
> +
> +	return 0;
> +}
> +
> +static int adc12138_adc_conversion(struct adc12138 *adc,
> +				   struct iio_chan_spec const *channel,
> +				   __be16 *value)
> +{
> +	int ret;
> +
> +	reinit_completion(&adc->complete);
> +
> +	ret = adc12138_start_conv(adc, channel);
> +	if (ret)
> +		return ret;
> +
> +	ret = adc12138_wait_eoc(adc, msecs_to_jiffies(100));
> +	if (ret)
> +		return ret;
> +
> +	return adc12138_read_conv_data(adc, value);
> +}
> +
> +static int adc12138_read_raw(struct iio_dev *iio,
> +			     struct iio_chan_spec const *channel, int *value,
> +			     int *shift, long mask)
> +{
> +	struct adc12138 *adc = iio_priv(iio);
> +	int ret;
> +	__be16 data;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:

what if buffered reads have been set up? probably use 
iio_device_claim_direct_mode()

> +		mutex_lock(&adc->lock);
> +		ret = adc12138_adc_conversion(adc, channel, &data);
> +		mutex_unlock(&adc->lock);
> +		if (ret)
> +			return ret;
> +
> +		*value = sign_extend32(be16_to_cpu(data) >> 3, 12);
> +
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		ret = regulator_get_voltage(adc->vref_p);
> +		if (ret < 0)
> +			return ret;
> +		*value = ret;
> +
> +		if (!IS_ERR(adc->vref_n)) {
> +			ret = regulator_get_voltage(adc->vref_n);
> +			if (ret < 0)
> +				return ret;
> +			*value -= ret;
> +		}
> +
> +		/* convert regulator output voltage to mV */
> +		*value /= 1000;
> +		*shift = channel->scan_type.realbits - 1;
> +
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	case IIO_CHAN_INFO_OFFSET:
> +		if (!IS_ERR(adc->vref_n)) {
> +			*value = regulator_get_voltage(adc->vref_n);
> +			if (*value < 0)
> +				return *value;
> +		} else {
> +			*value = 0;
> +		}
> +
> +		/* convert regulator output voltage to mV */
> +		*value /= 1000;
> +
> +		return IIO_VAL_INT;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct iio_info adc12138_info = {
> +	.read_raw = adc12138_read_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static int adc12138_init(struct adc12138 *adc)
> +{
> +	int ret;
> +	int status;
> +	u8 mode;
> +	u8 trash;
> +
> +	reinit_completion(&adc->complete);
> +
> +	ret = adc12138_mode_programming(adc, ADC12138_MODE_AUTO_CAL, &trash, 1);
> +	if (ret)
> +		return ret;
> +
> +	/* data output at this time has no significance */
> +	status = adc12138_read_status(adc);
> +	if (status < 0)
> +		return status;
> +
> +	adc12138_wait_eoc(adc, msecs_to_jiffies(100));
> +
> +	status = adc12138_read_status(adc);
> +	if (status & ADC12138_STATUS_CAL) {
> +		dev_warn(&adc->spi->dev,
> +			"Auto Cal sequence is still in progress: %#x\n",
> +			status);
> +		return -EIO;
> +	}
> +
> +	switch (adc->acquisition_time) {
> +	case 6:
> +		mode = ADC12138_MODE_ACQUISITION_TIME_6;
> +		break;
> +	case 10:
> +		mode = ADC12138_MODE_ACQUISITION_TIME_10;
> +		break;
> +	case 18:
> +		mode = ADC12138_MODE_ACQUISITION_TIME_18;
> +		break;
> +	case 34:
> +		mode = ADC12138_MODE_ACQUISITION_TIME_34;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return adc12138_mode_programming(adc, mode, &trash, 1);
> +}
> +
> +static irqreturn_t adc12138_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct adc12138 *adc = iio_priv(indio_dev);
> +	__be16 data[20] = { };

why { } here?
why 20?

> +	__be16 trash;
> +	int ret;
> +	int scan_index;
> +	int i = 0;
> +
> +	mutex_lock(&adc->lock);
> +
> +	for_each_set_bit(scan_index, indio_dev->active_scan_mask,
> +			 indio_dev->masklength) {
> +		const struct iio_chan_spec *scan_chan =
> +				&indio_dev->channels[scan_index];
> +
> +		reinit_completion(&adc->complete);
> +
> +		ret = adc12138_start_and_read_conv(adc, scan_chan,
> +						   i ? &data[i - 1] : &trash);
> +		if (ret) {
> +			dev_warn(&adc->spi->dev,
> +				 "failed to start conversion\n");
> +			goto out;
> +		}
> +
> +		ret = adc12138_wait_eoc(adc, msecs_to_jiffies(100));
> +		if (ret) {
> +			dev_warn(&adc->spi->dev, "wait eoc timeout\n");
> +			goto out;
> +		}
> +
> +		i++;
> +	}
> +
> +	if (i) {
> +		ret = adc12138_read_conv_data(adc, &data[i - 1]);
> +		if (ret) {
> +			dev_warn(&adc->spi->dev,
> +				 "failed to get conversion data\n");
> +			goto out;
> +		}
> +	}
> +
> +	iio_push_to_buffers_with_timestamp(indio_dev, data, iio_get_time_ns());
> +out:
> +	mutex_unlock(&adc->lock);
> +
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t adc12138_eoc_handler(int irq, void *p)
> +{
> +	struct iio_dev *indio_dev = p;
> +	struct adc12138 *adc = iio_priv(indio_dev);
> +
> +	complete(&adc->complete);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int adc12138_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct adc12138 *adc;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	adc = iio_priv(indio_dev);
> +	adc->spi = spi;
> +	adc->id = spi_get_device_id(spi)->driver_data;
> +	mutex_init(&adc->lock);
> +	init_completion(&adc->complete);
> +
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->info = &adc12138_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	switch (adc->id) {
> +	case adc12130:
> +	case adc12132:
> +		indio_dev->channels = adc12132_channels;
> +		indio_dev->num_channels = ARRAY_SIZE(adc12132_channels);
> +		break;
> +	case adc12138:
> +		indio_dev->channels = adc12138_channels;
> +		indio_dev->num_channels = ARRAY_SIZE(adc12138_channels);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	ret = of_property_read_u32(spi->dev.of_node, "acquisition-time",
> +				   &adc->acquisition_time);
> +	if (ret)
> +		adc->acquisition_time = 10;
> +
> +	adc->cclk = devm_clk_get(&spi->dev, NULL);
> +	if (IS_ERR(adc->cclk))
> +		return PTR_ERR(adc->cclk);
> +
> +	adc->vref_p = devm_regulator_get(&spi->dev, "vref-p");
> +	if (IS_ERR(adc->vref_p))
> +		return PTR_ERR(adc->vref_p);
> +
> +	adc->vref_n = devm_regulator_get_optional(&spi->dev, "vref-n");
> +
> +	ret = devm_request_irq(&spi->dev, spi->irq, adc12138_eoc_handler,
> +			       IRQF_TRIGGER_RISING, indio_dev->name, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(adc->cclk);
> +	if (ret)
> +		return ret;
> +
> +	ret = regulator_enable(adc->vref_p);
> +	if (ret)
> +		goto error1;
> +
> +	if (!IS_ERR(adc->vref_n)) {
> +		ret = regulator_enable(adc->vref_n);
> +		if (ret)
> +			goto error2;
> +	}
> +
> +	ret = adc12138_init(adc);
> +	if (ret)
> +		goto error3;
> +
> +	spi_set_drvdata(spi, indio_dev);
> +
> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> +					 adc12138_trigger_handler, NULL);
> +	if (ret)
> +		goto error3;
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret)
> +		goto error4;
> +
> +	return 0;
> +error4:
> +	iio_triggered_buffer_cleanup(indio_dev);
> +error3:
> +	if (!IS_ERR(adc->vref_n))
> +		regulator_disable(adc->vref_n);
> +error2:
> +	regulator_disable(adc->vref_p);
> +error1:
> +	clk_disable_unprepare(adc->cclk);
> +
> +	return ret;
> +}
> +
> +static int adc12138_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct adc12138 *adc = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +	iio_triggered_buffer_cleanup(indio_dev);
> +	if (!IS_ERR(adc->vref_n))
> +		regulator_disable(adc->vref_n);
> +	regulator_disable(adc->vref_p);
> +	clk_disable_unprepare(adc->cclk);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +
> +static const struct of_device_id adc12138_dt_ids[] = {
> +	{ .compatible = "ti,adc12130", },
> +	{ .compatible = "ti,adc12132", },
> +	{ .compatible = "ti,adc12138", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, adc12138_dt_ids);
> +
> +#endif
> +
> +static const struct spi_device_id adc12138_id[] = {
> +	{ "adc12130", adc12130 },
> +	{ "adc12132", adc12132 },
> +	{ "adc12138", adc12138 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(spi, adc12138_id);
> +
> +static struct spi_driver adc12138_driver = {
> +	.driver = {
> +		.name = "adc12138",
> +		.of_match_table = of_match_ptr(adc12138_dt_ids),
> +	},
> +	.probe = adc12138_probe,
> +	.remove = adc12138_remove,
> +	.id_table = adc12138_id,
> +};
> +module_spi_driver(adc12138_driver);
> +
> +MODULE_AUTHOR("Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
> +MODULE_DESCRIPTION("ADC12130/ADC12132/ADC12138 driver");
> +MODULE_LICENSE("GPL v2");
> 

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] iio: adc: add ADC12130/ADC12132/ADC12138 ADC driver
@ 2016-07-18 22:02     ` Peter Meerwald-Stadler
  0 siblings, 0 replies; 10+ messages in thread
From: Peter Meerwald-Stadler @ 2016-07-18 22:02 UTC (permalink / raw)
  To: Akinobu Mita
  Cc: linux-iio, devicetree, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen

[-- Attachment #1: Type: TEXT/PLAIN, Size: 18280 bytes --]


> This adds Texas Instruments' ADC12130/ADC12132/ADC12138 12-bit plus
> sign ADC driver.  I have tested with the ADC12138.  The ADC12130 and
> ADC12132 are not tested but these are similar to ADC12138 except that
> the mode programming instruction is a bit different.

minor comments below

> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: Jonathan Cameron <jic23@kernel.org>
> Cc: Hartmut Knaack <knaack.h@gmx.de>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Peter Meerwald <pmeerw@pmeerw.net>
> ---
>  .../devicetree/bindings/iio/adc/ti-adc12138.txt    |  32 ++
>  drivers/iio/adc/Kconfig                            |  12 +
>  drivers/iio/adc/Makefile                           |   1 +
>  drivers/iio/adc/ti-adc12138.c                      | 541 +++++++++++++++++++++
>  4 files changed, 586 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
>  create mode 100644 drivers/iio/adc/ti-adc12138.c
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt b/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
> new file mode 100644
> index 0000000..3a11d2a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
> @@ -0,0 +1,32 @@
> +* Texas Instruments' ADC12130/ADC12132/ADC12138
> +
> +Required properties:
> + - compatible: Should be one of
> +	* "ti,adc12130"
> +	* "ti,adc12132"
> +	* "ti,adc12138"
> + - reg: SPI chip select number for the device
> + - interrupts: Should contain interrupt for EOC (end of conversion)
> + - clocks: phandle to conversion clock input
> + - spi-max-frequency: Definision as per
> +	Documentation/devicetree/bindings/spi/spi-bus.txt
> + - vref-p-supply: The regulator supply for positive analog voltage reference
> +
> +Optional properties:
> + - vref-n-supply: The regulator supply for negative analog voltage reference
> +	If not specified, this is assumed to be analog ground.
> + - acquisition-time: The number of conversion clock periods for the S/H's
> +	acquisition time.  Should be one of 6, 10, 18, 34.  If not specified,
> +	default value of 10 is used.
> +
> +Example:
> +adc@0 {
> +	compatible = "ti,adc12138";
> +	reg = <0>;
> +	interrupts = <28 IRQ_TYPE_EDGE_RISING>;
> +	interrupt-parent = <&gpio1>;
> +	clocks = <&cclk>;
> +	vref-p-supply = <&ldo4_reg>;
> +	spi-max-frequency = <5000000>;
> +	acquisition-time = <6>;
> +};
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index fa0ff29..0783291 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -414,6 +414,18 @@ config TI_ADC0832
>  	  This driver can also be built as a module. If so, the module will be
>  	  called ti-adc0832.
>  
> +config TI_ADC12138
> +	tristate "Texas Instruments ADC12130/ADC12132/ADC12138"
> +	depends on SPI
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
> +	help
> +	  If you say yes here you get support for Texas Instruments ADC12130,
> +	  ADC12132and ADC12138 chips.

space before 'and' missing

> +
> +	  This driver can also be built as a module. If so, the module will be
> +	  called ti-adc12138.
> +
>  config TI_ADC128S052
>  	tristate "Texas Instruments ADC128S052/ADC122S021/ADC124S021"
>  	depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 599714b..3ae4ae3 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -40,6 +40,7 @@ obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
>  obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
>  obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>  obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
> +obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
>  obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
>  obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
>  obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
> diff --git a/drivers/iio/adc/ti-adc12138.c b/drivers/iio/adc/ti-adc12138.c
> new file mode 100644
> index 0000000..e7bd986
> --- /dev/null
> +++ b/drivers/iio/adc/ti-adc12138.c
> @@ -0,0 +1,541 @@
> +/*
> + * ADC12130/ADC12132/ADC12138 12-bit plus sign ADC driver
> + *
> + * Copyright (c) 2016 Akinobu Mita <akinobu.mita@gmail.com>
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License.  See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * Datasheet: http://www.ti.com/lit/ds/symlink/adc12138.pdf
> + */
> +
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/completion.h>
> +#include <linux/clk.h>
> +#include <linux/spi/spi.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/regulator/consumer.h>
> +
> +#define ADC12138_MODE_AUTO_CAL			0x08
> +#define ADC12138_MODE_READ_STATUS		0x0c
> +#define ADC12138_MODE_ACQUISITION_TIME_6	0x0e
> +#define ADC12138_MODE_ACQUISITION_TIME_10	0x4e
> +#define ADC12138_MODE_ACQUISITION_TIME_18	0x8e
> +#define ADC12138_MODE_ACQUISITION_TIME_34	0xce
> +
> +#define ADC12138_STATUS_CAL			BIT(6)
> +
> +enum {
> +	adc12130,
> +	adc12132,
> +	adc12138,
> +};
> +
> +struct adc12138 {
> +	struct spi_device *spi;
> +	unsigned int id;
> +	/* conversion clock */
> +	struct clk *cclk;
> +	/* positive analog voltage reference */
> +	struct regulator *vref_p;
> +	/* negative analog voltage reference */
> +	struct regulator *vref_n;
> +	struct mutex lock;
> +	struct completion complete;
> +	unsigned int acquisition_time;

unit? ms? µs?

> +
> +	u8 tx_buf[2] ____cacheline_aligned;
> +	u8 rx_buf[2];
> +};
> +
> +#define ADC12138_VOLTAGE_CHANNEL(chan)					\
> +	{								\
> +		.type = IIO_VOLTAGE,					\
> +		.indexed = 1,						\
> +		.channel = chan,					\
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE)	\
> +					| BIT(IIO_CHAN_INFO_OFFSET),	\
> +		.scan_index = chan,					\
> +		.scan_type = {						\
> +			.sign = 's',					\
> +			.realbits = 13,					\
> +			.storagebits = 16,				\
> +			.shift = 3,					\
> +			.endianness = IIO_BE,				\
> +		},							\
> +	}
> +
> +#define ADC12138_VOLTAGE_CHANNEL_DIFF(chan1, chan2, si_offset)		\
> +	{								\
> +		.type = IIO_VOLTAGE,					\
> +		.indexed = 1,						\
> +		.channel = (chan1),					\
> +		.channel2 = (chan2),					\
> +		.differential = 1,					\
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE)	\
> +					| BIT(IIO_CHAN_INFO_OFFSET),	\
> +		.scan_index = (chan1) + (si_offset),			\
> +		.scan_type = {						\
> +			.sign = 's',					\
> +			.realbits = 13,					\
> +			.storagebits = 16,				\
> +			.shift = 3,					\
> +			.endianness = IIO_BE,				\
> +		},							\
> +	}
> +
> +static const struct iio_chan_spec adc12132_channels[] = {
> +	ADC12138_VOLTAGE_CHANNEL(0),
> +	ADC12138_VOLTAGE_CHANNEL(1),
> +	ADC12138_VOLTAGE_CHANNEL_DIFF(0, 1, 2),
> +	ADC12138_VOLTAGE_CHANNEL_DIFF(1, 0, 2),
> +	IIO_CHAN_SOFT_TIMESTAMP(4),
> +};
> +
> +static const struct iio_chan_spec adc12138_channels[] = {
> +	ADC12138_VOLTAGE_CHANNEL(0),
> +	ADC12138_VOLTAGE_CHANNEL(1),
> +	ADC12138_VOLTAGE_CHANNEL(2),
> +	ADC12138_VOLTAGE_CHANNEL(3),
> +	ADC12138_VOLTAGE_CHANNEL(4),
> +	ADC12138_VOLTAGE_CHANNEL(5),
> +	ADC12138_VOLTAGE_CHANNEL(6),
> +	ADC12138_VOLTAGE_CHANNEL(7),
> +	ADC12138_VOLTAGE_CHANNEL_DIFF(0, 1, 8),
> +	ADC12138_VOLTAGE_CHANNEL_DIFF(1, 0, 8),
> +	ADC12138_VOLTAGE_CHANNEL_DIFF(2, 3, 8),
> +	ADC12138_VOLTAGE_CHANNEL_DIFF(3, 2, 8),
> +	ADC12138_VOLTAGE_CHANNEL_DIFF(4, 5, 8),
> +	ADC12138_VOLTAGE_CHANNEL_DIFF(5, 4, 8),
> +	ADC12138_VOLTAGE_CHANNEL_DIFF(6, 7, 8),
> +	ADC12138_VOLTAGE_CHANNEL_DIFF(7, 6, 8),
> +	IIO_CHAN_SOFT_TIMESTAMP(16),
> +};
> +
> +static int adc12138_mode_programming(struct adc12138 *adc, u8 mode,
> +				     void *rx_buf, int len)
> +{
> +	struct spi_transfer xfer = {
> +		.tx_buf = adc->tx_buf,
> +		.rx_buf = adc->rx_buf,
> +		.len = len,
> +	};
> +	int ret;
> +
> +	/* Skip disused bits for ADC12130 and ADC12132 */

probably 'unused' instead of 'disused'

> +	if (adc->id != adc12138)
> +		mode = (mode & 0xc0) | ((mode & 0x0f) << 2);
> +
> +	adc->tx_buf[0] = mode;
> +
> +	ret = spi_sync_transfer(adc->spi, &xfer, 1);
> +	if (ret)
> +		return ret;
> +
> +	memcpy(rx_buf, adc->rx_buf, len);
> +
> +	return 0;
> +}
> +
> +static int adc12138_read_status(struct adc12138 *adc)
> +{
> +	u8 rx_buf[2];
> +	int ret;
> +
> +	ret = adc12138_mode_programming(adc, ADC12138_MODE_READ_STATUS,
> +					rx_buf, 2);
> +	if (ret)
> +		return ret;
> +
> +	return (rx_buf[0] << 1) | (rx_buf[1] >> 7);
> +}
> +
> +static int __adc12138_start_conv(struct adc12138 *adc,
> +				 struct iio_chan_spec const *channel,
> +				 void *data, int len)
> +
> +{
> +	const u8 ch_to_mux[] = { 0, 4, 1, 5, 2, 6, 3, 7 };
> +	u8 mode = (ch_to_mux[channel->channel] << 4) |
> +		  (channel->differential ? 0 : 0x80);
> +
> +	return adc12138_mode_programming(adc, mode, data, len);
> +}
> +
> +static int adc12138_start_conv(struct adc12138 *adc,
> +			       struct iio_chan_spec const *channel)
> +{
> +	u8 trash;
> +
> +	return __adc12138_start_conv(adc, channel, &trash, 1);
> +}
> +
> +static int adc12138_start_and_read_conv(struct adc12138 *adc,
> +					struct iio_chan_spec const *channel,
> +					__be16 *data)
> +{
> +	return __adc12138_start_conv(adc, channel, data, 2);
> +}
> +
> +static int adc12138_read_conv_data(struct adc12138 *adc, __be16 *value)
> +{
> +	/* Issue a read status instruction and read previous conversion data */
> +	return adc12138_mode_programming(adc, ADC12138_MODE_READ_STATUS,
> +					 value, sizeof(*value));
> +}
> +
> +static int adc12138_wait_eoc(struct adc12138 *adc, unsigned long timeout)
> +{
> +	if (!wait_for_completion_timeout(&adc->complete, timeout))
> +		return -ETIMEDOUT;
> +
> +	return 0;
> +}
> +
> +static int adc12138_adc_conversion(struct adc12138 *adc,
> +				   struct iio_chan_spec const *channel,
> +				   __be16 *value)
> +{
> +	int ret;
> +
> +	reinit_completion(&adc->complete);
> +
> +	ret = adc12138_start_conv(adc, channel);
> +	if (ret)
> +		return ret;
> +
> +	ret = adc12138_wait_eoc(adc, msecs_to_jiffies(100));
> +	if (ret)
> +		return ret;
> +
> +	return adc12138_read_conv_data(adc, value);
> +}
> +
> +static int adc12138_read_raw(struct iio_dev *iio,
> +			     struct iio_chan_spec const *channel, int *value,
> +			     int *shift, long mask)
> +{
> +	struct adc12138 *adc = iio_priv(iio);
> +	int ret;
> +	__be16 data;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:

what if buffered reads have been set up? probably use 
iio_device_claim_direct_mode()

> +		mutex_lock(&adc->lock);
> +		ret = adc12138_adc_conversion(adc, channel, &data);
> +		mutex_unlock(&adc->lock);
> +		if (ret)
> +			return ret;
> +
> +		*value = sign_extend32(be16_to_cpu(data) >> 3, 12);
> +
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		ret = regulator_get_voltage(adc->vref_p);
> +		if (ret < 0)
> +			return ret;
> +		*value = ret;
> +
> +		if (!IS_ERR(adc->vref_n)) {
> +			ret = regulator_get_voltage(adc->vref_n);
> +			if (ret < 0)
> +				return ret;
> +			*value -= ret;
> +		}
> +
> +		/* convert regulator output voltage to mV */
> +		*value /= 1000;
> +		*shift = channel->scan_type.realbits - 1;
> +
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	case IIO_CHAN_INFO_OFFSET:
> +		if (!IS_ERR(adc->vref_n)) {
> +			*value = regulator_get_voltage(adc->vref_n);
> +			if (*value < 0)
> +				return *value;
> +		} else {
> +			*value = 0;
> +		}
> +
> +		/* convert regulator output voltage to mV */
> +		*value /= 1000;
> +
> +		return IIO_VAL_INT;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct iio_info adc12138_info = {
> +	.read_raw = adc12138_read_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static int adc12138_init(struct adc12138 *adc)
> +{
> +	int ret;
> +	int status;
> +	u8 mode;
> +	u8 trash;
> +
> +	reinit_completion(&adc->complete);
> +
> +	ret = adc12138_mode_programming(adc, ADC12138_MODE_AUTO_CAL, &trash, 1);
> +	if (ret)
> +		return ret;
> +
> +	/* data output at this time has no significance */
> +	status = adc12138_read_status(adc);
> +	if (status < 0)
> +		return status;
> +
> +	adc12138_wait_eoc(adc, msecs_to_jiffies(100));
> +
> +	status = adc12138_read_status(adc);
> +	if (status & ADC12138_STATUS_CAL) {
> +		dev_warn(&adc->spi->dev,
> +			"Auto Cal sequence is still in progress: %#x\n",
> +			status);
> +		return -EIO;
> +	}
> +
> +	switch (adc->acquisition_time) {
> +	case 6:
> +		mode = ADC12138_MODE_ACQUISITION_TIME_6;
> +		break;
> +	case 10:
> +		mode = ADC12138_MODE_ACQUISITION_TIME_10;
> +		break;
> +	case 18:
> +		mode = ADC12138_MODE_ACQUISITION_TIME_18;
> +		break;
> +	case 34:
> +		mode = ADC12138_MODE_ACQUISITION_TIME_34;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return adc12138_mode_programming(adc, mode, &trash, 1);
> +}
> +
> +static irqreturn_t adc12138_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct adc12138 *adc = iio_priv(indio_dev);
> +	__be16 data[20] = { };

why { } here?
why 20?

> +	__be16 trash;
> +	int ret;
> +	int scan_index;
> +	int i = 0;
> +
> +	mutex_lock(&adc->lock);
> +
> +	for_each_set_bit(scan_index, indio_dev->active_scan_mask,
> +			 indio_dev->masklength) {
> +		const struct iio_chan_spec *scan_chan =
> +				&indio_dev->channels[scan_index];
> +
> +		reinit_completion(&adc->complete);
> +
> +		ret = adc12138_start_and_read_conv(adc, scan_chan,
> +						   i ? &data[i - 1] : &trash);
> +		if (ret) {
> +			dev_warn(&adc->spi->dev,
> +				 "failed to start conversion\n");
> +			goto out;
> +		}
> +
> +		ret = adc12138_wait_eoc(adc, msecs_to_jiffies(100));
> +		if (ret) {
> +			dev_warn(&adc->spi->dev, "wait eoc timeout\n");
> +			goto out;
> +		}
> +
> +		i++;
> +	}
> +
> +	if (i) {
> +		ret = adc12138_read_conv_data(adc, &data[i - 1]);
> +		if (ret) {
> +			dev_warn(&adc->spi->dev,
> +				 "failed to get conversion data\n");
> +			goto out;
> +		}
> +	}
> +
> +	iio_push_to_buffers_with_timestamp(indio_dev, data, iio_get_time_ns());
> +out:
> +	mutex_unlock(&adc->lock);
> +
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t adc12138_eoc_handler(int irq, void *p)
> +{
> +	struct iio_dev *indio_dev = p;
> +	struct adc12138 *adc = iio_priv(indio_dev);
> +
> +	complete(&adc->complete);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int adc12138_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct adc12138 *adc;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	adc = iio_priv(indio_dev);
> +	adc->spi = spi;
> +	adc->id = spi_get_device_id(spi)->driver_data;
> +	mutex_init(&adc->lock);
> +	init_completion(&adc->complete);
> +
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->info = &adc12138_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	switch (adc->id) {
> +	case adc12130:
> +	case adc12132:
> +		indio_dev->channels = adc12132_channels;
> +		indio_dev->num_channels = ARRAY_SIZE(adc12132_channels);
> +		break;
> +	case adc12138:
> +		indio_dev->channels = adc12138_channels;
> +		indio_dev->num_channels = ARRAY_SIZE(adc12138_channels);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	ret = of_property_read_u32(spi->dev.of_node, "acquisition-time",
> +				   &adc->acquisition_time);
> +	if (ret)
> +		adc->acquisition_time = 10;
> +
> +	adc->cclk = devm_clk_get(&spi->dev, NULL);
> +	if (IS_ERR(adc->cclk))
> +		return PTR_ERR(adc->cclk);
> +
> +	adc->vref_p = devm_regulator_get(&spi->dev, "vref-p");
> +	if (IS_ERR(adc->vref_p))
> +		return PTR_ERR(adc->vref_p);
> +
> +	adc->vref_n = devm_regulator_get_optional(&spi->dev, "vref-n");
> +
> +	ret = devm_request_irq(&spi->dev, spi->irq, adc12138_eoc_handler,
> +			       IRQF_TRIGGER_RISING, indio_dev->name, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(adc->cclk);
> +	if (ret)
> +		return ret;
> +
> +	ret = regulator_enable(adc->vref_p);
> +	if (ret)
> +		goto error1;
> +
> +	if (!IS_ERR(adc->vref_n)) {
> +		ret = regulator_enable(adc->vref_n);
> +		if (ret)
> +			goto error2;
> +	}
> +
> +	ret = adc12138_init(adc);
> +	if (ret)
> +		goto error3;
> +
> +	spi_set_drvdata(spi, indio_dev);
> +
> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> +					 adc12138_trigger_handler, NULL);
> +	if (ret)
> +		goto error3;
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret)
> +		goto error4;
> +
> +	return 0;
> +error4:
> +	iio_triggered_buffer_cleanup(indio_dev);
> +error3:
> +	if (!IS_ERR(adc->vref_n))
> +		regulator_disable(adc->vref_n);
> +error2:
> +	regulator_disable(adc->vref_p);
> +error1:
> +	clk_disable_unprepare(adc->cclk);
> +
> +	return ret;
> +}
> +
> +static int adc12138_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct adc12138 *adc = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +	iio_triggered_buffer_cleanup(indio_dev);
> +	if (!IS_ERR(adc->vref_n))
> +		regulator_disable(adc->vref_n);
> +	regulator_disable(adc->vref_p);
> +	clk_disable_unprepare(adc->cclk);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +
> +static const struct of_device_id adc12138_dt_ids[] = {
> +	{ .compatible = "ti,adc12130", },
> +	{ .compatible = "ti,adc12132", },
> +	{ .compatible = "ti,adc12138", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, adc12138_dt_ids);
> +
> +#endif
> +
> +static const struct spi_device_id adc12138_id[] = {
> +	{ "adc12130", adc12130 },
> +	{ "adc12132", adc12132 },
> +	{ "adc12138", adc12138 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(spi, adc12138_id);
> +
> +static struct spi_driver adc12138_driver = {
> +	.driver = {
> +		.name = "adc12138",
> +		.of_match_table = of_match_ptr(adc12138_dt_ids),
> +	},
> +	.probe = adc12138_probe,
> +	.remove = adc12138_remove,
> +	.id_table = adc12138_id,
> +};
> +module_spi_driver(adc12138_driver);
> +
> +MODULE_AUTHOR("Akinobu Mita <akinobu.mita@gmail.com>");
> +MODULE_DESCRIPTION("ADC12130/ADC12132/ADC12138 driver");
> +MODULE_LICENSE("GPL v2");
> 

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] iio: adc: add ADC12130/ADC12132/ADC12138 ADC driver
  2016-07-18 22:02     ` Peter Meerwald-Stadler
@ 2016-07-19 16:24         ` Akinobu Mita
  -1 siblings, 0 replies; 10+ messages in thread
From: Akinobu Mita @ 2016-07-19 16:24 UTC (permalink / raw)
  To: Peter Meerwald-Stadler
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, open list:OPEN FIRMWARE AND...,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen

2016-07-19 7:02 GMT+09:00 Peter Meerwald-Stadler <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>:
>
>> This adds Texas Instruments' ADC12130/ADC12132/ADC12138 12-bit plus
>> sign ADC driver.  I have tested with the ADC12138.  The ADC12130 and
>> ADC12132 are not tested but these are similar to ADC12138 except that
>> the mode programming instruction is a bit different.
>
> minor comments below
>
>> Signed-off-by: Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Cc: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Cc: Hartmut Knaack <knaack.h-Mmb7MZpHnFY@public.gmane.org>
>> Cc: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
>> Cc: Peter Meerwald <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>
>> ---
>>  .../devicetree/bindings/iio/adc/ti-adc12138.txt    |  32 ++
>>  drivers/iio/adc/Kconfig                            |  12 +
>>  drivers/iio/adc/Makefile                           |   1 +
>>  drivers/iio/adc/ti-adc12138.c                      | 541 +++++++++++++++++++++
>>  4 files changed, 586 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
>>  create mode 100644 drivers/iio/adc/ti-adc12138.c
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt b/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
>> new file mode 100644
>> index 0000000..3a11d2a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
>> @@ -0,0 +1,32 @@
>> +* Texas Instruments' ADC12130/ADC12132/ADC12138
>> +
>> +Required properties:
>> + - compatible: Should be one of
>> +     * "ti,adc12130"
>> +     * "ti,adc12132"
>> +     * "ti,adc12138"
>> + - reg: SPI chip select number for the device
>> + - interrupts: Should contain interrupt for EOC (end of conversion)
>> + - clocks: phandle to conversion clock input
>> + - spi-max-frequency: Definision as per
>> +     Documentation/devicetree/bindings/spi/spi-bus.txt
>> + - vref-p-supply: The regulator supply for positive analog voltage reference
>> +
>> +Optional properties:
>> + - vref-n-supply: The regulator supply for negative analog voltage reference
>> +     If not specified, this is assumed to be analog ground.
>> + - acquisition-time: The number of conversion clock periods for the S/H's
>> +     acquisition time.  Should be one of 6, 10, 18, 34.  If not specified,
>> +     default value of 10 is used.
>> +
>> +Example:
>> +adc@0 {
>> +     compatible = "ti,adc12138";
>> +     reg = <0>;
>> +     interrupts = <28 IRQ_TYPE_EDGE_RISING>;
>> +     interrupt-parent = <&gpio1>;
>> +     clocks = <&cclk>;
>> +     vref-p-supply = <&ldo4_reg>;
>> +     spi-max-frequency = <5000000>;
>> +     acquisition-time = <6>;
>> +};
>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>> index fa0ff29..0783291 100644
>> --- a/drivers/iio/adc/Kconfig
>> +++ b/drivers/iio/adc/Kconfig
>> @@ -414,6 +414,18 @@ config TI_ADC0832
>>         This driver can also be built as a module. If so, the module will be
>>         called ti-adc0832.
>>
>> +config TI_ADC12138
>> +     tristate "Texas Instruments ADC12130/ADC12132/ADC12138"
>> +     depends on SPI
>> +     select IIO_BUFFER
>> +     select IIO_TRIGGERED_BUFFER
>> +     help
>> +       If you say yes here you get support for Texas Instruments ADC12130,
>> +       ADC12132and ADC12138 chips.
>
> space before 'and' missing

Ugh.

>> +
>> +       This driver can also be built as a module. If so, the module will be
>> +       called ti-adc12138.
>> +
>>  config TI_ADC128S052
>>       tristate "Texas Instruments ADC128S052/ADC122S021/ADC124S021"
>>       depends on SPI
>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>> index 599714b..3ae4ae3 100644
>> --- a/drivers/iio/adc/Makefile
>> +++ b/drivers/iio/adc/Makefile
>> @@ -40,6 +40,7 @@ obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
>>  obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
>>  obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>>  obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
>> +obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
>>  obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
>>  obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
>>  obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
>> diff --git a/drivers/iio/adc/ti-adc12138.c b/drivers/iio/adc/ti-adc12138.c
>> new file mode 100644
>> index 0000000..e7bd986
>> --- /dev/null
>> +++ b/drivers/iio/adc/ti-adc12138.c
>> @@ -0,0 +1,541 @@
>> +/*
>> + * ADC12130/ADC12132/ADC12138 12-bit plus sign ADC driver
>> + *
>> + * Copyright (c) 2016 Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> + *
>> + * This file is subject to the terms and conditions of version 2 of
>> + * the GNU General Public License.  See the file COPYING in the main
>> + * directory of this archive for more details.
>> + *
>> + * Datasheet: http://www.ti.com/lit/ds/symlink/adc12138.pdf
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/completion.h>
>> +#include <linux/clk.h>
>> +#include <linux/spi/spi.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/buffer.h>
>> +#include <linux/iio/trigger.h>
>> +#include <linux/iio/triggered_buffer.h>
>> +#include <linux/iio/trigger_consumer.h>
>> +#include <linux/regulator/consumer.h>
>> +
>> +#define ADC12138_MODE_AUTO_CAL                       0x08
>> +#define ADC12138_MODE_READ_STATUS            0x0c
>> +#define ADC12138_MODE_ACQUISITION_TIME_6     0x0e
>> +#define ADC12138_MODE_ACQUISITION_TIME_10    0x4e
>> +#define ADC12138_MODE_ACQUISITION_TIME_18    0x8e
>> +#define ADC12138_MODE_ACQUISITION_TIME_34    0xce
>> +
>> +#define ADC12138_STATUS_CAL                  BIT(6)
>> +
>> +enum {
>> +     adc12130,
>> +     adc12132,
>> +     adc12138,
>> +};
>> +
>> +struct adc12138 {
>> +     struct spi_device *spi;
>> +     unsigned int id;
>> +     /* conversion clock */
>> +     struct clk *cclk;
>> +     /* positive analog voltage reference */
>> +     struct regulator *vref_p;
>> +     /* negative analog voltage reference */
>> +     struct regulator *vref_n;
>> +     struct mutex lock;
>> +     struct completion complete;
>> +     unsigned int acquisition_time;
>
> unit? ms? µs?

This is specified by the number of conversion clock (cclk) periods.
I'll add a comment about it.

>> +
>> +     u8 tx_buf[2] ____cacheline_aligned;
>> +     u8 rx_buf[2];
>> +};
>> +
>> +#define ADC12138_VOLTAGE_CHANNEL(chan)                                       \
>> +     {                                                               \
>> +             .type = IIO_VOLTAGE,                                    \
>> +             .indexed = 1,                                           \
>> +             .channel = chan,                                        \
>> +             .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),           \
>> +             .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE)    \
>> +                                     | BIT(IIO_CHAN_INFO_OFFSET),    \
>> +             .scan_index = chan,                                     \
>> +             .scan_type = {                                          \
>> +                     .sign = 's',                                    \
>> +                     .realbits = 13,                                 \
>> +                     .storagebits = 16,                              \
>> +                     .shift = 3,                                     \
>> +                     .endianness = IIO_BE,                           \
>> +             },                                                      \
>> +     }
>> +
>> +#define ADC12138_VOLTAGE_CHANNEL_DIFF(chan1, chan2, si_offset)               \
>> +     {                                                               \
>> +             .type = IIO_VOLTAGE,                                    \
>> +             .indexed = 1,                                           \
>> +             .channel = (chan1),                                     \
>> +             .channel2 = (chan2),                                    \
>> +             .differential = 1,                                      \
>> +             .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),           \
>> +             .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE)    \
>> +                                     | BIT(IIO_CHAN_INFO_OFFSET),    \
>> +             .scan_index = (chan1) + (si_offset),                    \
>> +             .scan_type = {                                          \
>> +                     .sign = 's',                                    \
>> +                     .realbits = 13,                                 \
>> +                     .storagebits = 16,                              \
>> +                     .shift = 3,                                     \
>> +                     .endianness = IIO_BE,                           \
>> +             },                                                      \
>> +     }
>> +
>> +static const struct iio_chan_spec adc12132_channels[] = {
>> +     ADC12138_VOLTAGE_CHANNEL(0),
>> +     ADC12138_VOLTAGE_CHANNEL(1),
>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(0, 1, 2),
>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(1, 0, 2),
>> +     IIO_CHAN_SOFT_TIMESTAMP(4),
>> +};
>> +
>> +static const struct iio_chan_spec adc12138_channels[] = {
>> +     ADC12138_VOLTAGE_CHANNEL(0),
>> +     ADC12138_VOLTAGE_CHANNEL(1),
>> +     ADC12138_VOLTAGE_CHANNEL(2),
>> +     ADC12138_VOLTAGE_CHANNEL(3),
>> +     ADC12138_VOLTAGE_CHANNEL(4),
>> +     ADC12138_VOLTAGE_CHANNEL(5),
>> +     ADC12138_VOLTAGE_CHANNEL(6),
>> +     ADC12138_VOLTAGE_CHANNEL(7),
>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(0, 1, 8),
>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(1, 0, 8),
>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(2, 3, 8),
>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(3, 2, 8),
>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(4, 5, 8),
>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(5, 4, 8),
>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(6, 7, 8),
>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(7, 6, 8),
>> +     IIO_CHAN_SOFT_TIMESTAMP(16),
>> +};
>> +
>> +static int adc12138_mode_programming(struct adc12138 *adc, u8 mode,
>> +                                  void *rx_buf, int len)
>> +{
>> +     struct spi_transfer xfer = {
>> +             .tx_buf = adc->tx_buf,
>> +             .rx_buf = adc->rx_buf,
>> +             .len = len,
>> +     };
>> +     int ret;
>> +
>> +     /* Skip disused bits for ADC12130 and ADC12132 */
>
> probably 'unused' instead of 'disused'

OK.

>> +     if (adc->id != adc12138)
>> +             mode = (mode & 0xc0) | ((mode & 0x0f) << 2);
>> +
>> +     adc->tx_buf[0] = mode;
>> +
>> +     ret = spi_sync_transfer(adc->spi, &xfer, 1);
>> +     if (ret)
>> +             return ret;
>> +
>> +     memcpy(rx_buf, adc->rx_buf, len);
>> +
>> +     return 0;
>> +}
>> +
>> +static int adc12138_read_status(struct adc12138 *adc)
>> +{
>> +     u8 rx_buf[2];
>> +     int ret;
>> +
>> +     ret = adc12138_mode_programming(adc, ADC12138_MODE_READ_STATUS,
>> +                                     rx_buf, 2);
>> +     if (ret)
>> +             return ret;
>> +
>> +     return (rx_buf[0] << 1) | (rx_buf[1] >> 7);
>> +}
>> +
>> +static int __adc12138_start_conv(struct adc12138 *adc,
>> +                              struct iio_chan_spec const *channel,
>> +                              void *data, int len)
>> +
>> +{
>> +     const u8 ch_to_mux[] = { 0, 4, 1, 5, 2, 6, 3, 7 };
>> +     u8 mode = (ch_to_mux[channel->channel] << 4) |
>> +               (channel->differential ? 0 : 0x80);
>> +
>> +     return adc12138_mode_programming(adc, mode, data, len);
>> +}
>> +
>> +static int adc12138_start_conv(struct adc12138 *adc,
>> +                            struct iio_chan_spec const *channel)
>> +{
>> +     u8 trash;
>> +
>> +     return __adc12138_start_conv(adc, channel, &trash, 1);
>> +}
>> +
>> +static int adc12138_start_and_read_conv(struct adc12138 *adc,
>> +                                     struct iio_chan_spec const *channel,
>> +                                     __be16 *data)
>> +{
>> +     return __adc12138_start_conv(adc, channel, data, 2);
>> +}
>> +
>> +static int adc12138_read_conv_data(struct adc12138 *adc, __be16 *value)
>> +{
>> +     /* Issue a read status instruction and read previous conversion data */
>> +     return adc12138_mode_programming(adc, ADC12138_MODE_READ_STATUS,
>> +                                      value, sizeof(*value));
>> +}
>> +
>> +static int adc12138_wait_eoc(struct adc12138 *adc, unsigned long timeout)
>> +{
>> +     if (!wait_for_completion_timeout(&adc->complete, timeout))
>> +             return -ETIMEDOUT;
>> +
>> +     return 0;
>> +}
>> +
>> +static int adc12138_adc_conversion(struct adc12138 *adc,
>> +                                struct iio_chan_spec const *channel,
>> +                                __be16 *value)
>> +{
>> +     int ret;
>> +
>> +     reinit_completion(&adc->complete);
>> +
>> +     ret = adc12138_start_conv(adc, channel);
>> +     if (ret)
>> +             return ret;
>> +
>> +     ret = adc12138_wait_eoc(adc, msecs_to_jiffies(100));
>> +     if (ret)
>> +             return ret;
>> +
>> +     return adc12138_read_conv_data(adc, value);
>> +}
>> +
>> +static int adc12138_read_raw(struct iio_dev *iio,
>> +                          struct iio_chan_spec const *channel, int *value,
>> +                          int *shift, long mask)
>> +{
>> +     struct adc12138 *adc = iio_priv(iio);
>> +     int ret;
>> +     __be16 data;
>> +
>> +     switch (mask) {
>> +     case IIO_CHAN_INFO_RAW:
>
> what if buffered reads have been set up? probably use
> iio_device_claim_direct_mode()

Maybe it is safe because the ADC data is always requested and retrieved
while holding adc12138.lock from adc12138_read_raw() and
adc12138_trigger_handler().  Is it better to restrict direct mode and
return -EBUSY while buffer mode is enabled?

(BTW, I realized that I can use iio_dev->mlock instead of adc12138.lock)

>> +             mutex_lock(&adc->lock);
>> +             ret = adc12138_adc_conversion(adc, channel, &data);
>> +             mutex_unlock(&adc->lock);
>> +             if (ret)
>> +                     return ret;
>> +
>> +             *value = sign_extend32(be16_to_cpu(data) >> 3, 12);
>> +
>> +             return IIO_VAL_INT;
>> +     case IIO_CHAN_INFO_SCALE:
>> +             ret = regulator_get_voltage(adc->vref_p);
>> +             if (ret < 0)
>> +                     return ret;
>> +             *value = ret;
>> +
>> +             if (!IS_ERR(adc->vref_n)) {
>> +                     ret = regulator_get_voltage(adc->vref_n);
>> +                     if (ret < 0)
>> +                             return ret;
>> +                     *value -= ret;
>> +             }
>> +
>> +             /* convert regulator output voltage to mV */
>> +             *value /= 1000;
>> +             *shift = channel->scan_type.realbits - 1;
>> +
>> +             return IIO_VAL_FRACTIONAL_LOG2;
>> +     case IIO_CHAN_INFO_OFFSET:
>> +             if (!IS_ERR(adc->vref_n)) {
>> +                     *value = regulator_get_voltage(adc->vref_n);
>> +                     if (*value < 0)
>> +                             return *value;
>> +             } else {
>> +                     *value = 0;
>> +             }
>> +
>> +             /* convert regulator output voltage to mV */
>> +             *value /= 1000;
>> +
>> +             return IIO_VAL_INT;
>> +     }
>> +
>> +     return -EINVAL;
>> +}
>> +
>> +static const struct iio_info adc12138_info = {
>> +     .read_raw = adc12138_read_raw,
>> +     .driver_module = THIS_MODULE,
>> +};
>> +
>> +static int adc12138_init(struct adc12138 *adc)
>> +{
>> +     int ret;
>> +     int status;
>> +     u8 mode;
>> +     u8 trash;
>> +
>> +     reinit_completion(&adc->complete);
>> +
>> +     ret = adc12138_mode_programming(adc, ADC12138_MODE_AUTO_CAL, &trash, 1);
>> +     if (ret)
>> +             return ret;
>> +
>> +     /* data output at this time has no significance */
>> +     status = adc12138_read_status(adc);
>> +     if (status < 0)
>> +             return status;
>> +
>> +     adc12138_wait_eoc(adc, msecs_to_jiffies(100));
>> +
>> +     status = adc12138_read_status(adc);
>> +     if (status & ADC12138_STATUS_CAL) {
>> +             dev_warn(&adc->spi->dev,
>> +                     "Auto Cal sequence is still in progress: %#x\n",
>> +                     status);
>> +             return -EIO;
>> +     }
>> +
>> +     switch (adc->acquisition_time) {
>> +     case 6:
>> +             mode = ADC12138_MODE_ACQUISITION_TIME_6;
>> +             break;
>> +     case 10:
>> +             mode = ADC12138_MODE_ACQUISITION_TIME_10;
>> +             break;
>> +     case 18:
>> +             mode = ADC12138_MODE_ACQUISITION_TIME_18;
>> +             break;
>> +     case 34:
>> +             mode = ADC12138_MODE_ACQUISITION_TIME_34;
>> +             break;
>> +     default:
>> +             return -EINVAL;
>> +     }
>> +
>> +     return adc12138_mode_programming(adc, mode, &trash, 1);
>> +}
>> +
>> +static irqreturn_t adc12138_trigger_handler(int irq, void *p)
>> +{
>> +     struct iio_poll_func *pf = p;
>> +     struct iio_dev *indio_dev = pf->indio_dev;
>> +     struct adc12138 *adc = iio_priv(indio_dev);
>> +     __be16 data[20] = { };
>
> why { } here?

To initialize with zero and avoids exposing kernel stack to userspace
through padding entries.  I could use memset here.

> why 20?

When all scan elements are enabled, the sample data requires 40 bytes
(2 bytes ADC data * 16 channels + 8 bytes timestamp).

I'll add comments about these.

>> +     __be16 trash;
>> +     int ret;
>> +     int scan_index;
>> +     int i = 0;
>> +
>> +     mutex_lock(&adc->lock);
>> +
>> +     for_each_set_bit(scan_index, indio_dev->active_scan_mask,
>> +                      indio_dev->masklength) {
>> +             const struct iio_chan_spec *scan_chan =
>> +                             &indio_dev->channels[scan_index];
>> +
>> +             reinit_completion(&adc->complete);
>> +
>> +             ret = adc12138_start_and_read_conv(adc, scan_chan,
>> +                                                i ? &data[i - 1] : &trash);
>> +             if (ret) {
>> +                     dev_warn(&adc->spi->dev,
>> +                              "failed to start conversion\n");
>> +                     goto out;
>> +             }
>> +
>> +             ret = adc12138_wait_eoc(adc, msecs_to_jiffies(100));
>> +             if (ret) {
>> +                     dev_warn(&adc->spi->dev, "wait eoc timeout\n");
>> +                     goto out;
>> +             }
>> +
>> +             i++;
>> +     }
>> +
>> +     if (i) {
>> +             ret = adc12138_read_conv_data(adc, &data[i - 1]);
>> +             if (ret) {
>> +                     dev_warn(&adc->spi->dev,
>> +                              "failed to get conversion data\n");
>> +                     goto out;
>> +             }
>> +     }
>> +
>> +     iio_push_to_buffers_with_timestamp(indio_dev, data, iio_get_time_ns());
>> +out:
>> +     mutex_unlock(&adc->lock);
>> +
>> +     iio_trigger_notify_done(indio_dev->trig);
>> +
>> +     return IRQ_HANDLED;
>> +}

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] iio: adc: add ADC12130/ADC12132/ADC12138 ADC driver
@ 2016-07-19 16:24         ` Akinobu Mita
  0 siblings, 0 replies; 10+ messages in thread
From: Akinobu Mita @ 2016-07-19 16:24 UTC (permalink / raw)
  To: Peter Meerwald-Stadler
  Cc: linux-iio, open list:OPEN FIRMWARE AND...,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen

2016-07-19 7:02 GMT+09:00 Peter Meerwald-Stadler <pmeerw@pmeerw.net>:
>
>> This adds Texas Instruments' ADC12130/ADC12132/ADC12138 12-bit plus
>> sign ADC driver.  I have tested with the ADC12138.  The ADC12130 and
>> ADC12132 are not tested but these are similar to ADC12138 except that
>> the mode programming instruction is a bit different.
>
> minor comments below
>
>> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
>> Cc: Jonathan Cameron <jic23@kernel.org>
>> Cc: Hartmut Knaack <knaack.h@gmx.de>
>> Cc: Lars-Peter Clausen <lars@metafoo.de>
>> Cc: Peter Meerwald <pmeerw@pmeerw.net>
>> ---
>>  .../devicetree/bindings/iio/adc/ti-adc12138.txt    |  32 ++
>>  drivers/iio/adc/Kconfig                            |  12 +
>>  drivers/iio/adc/Makefile                           |   1 +
>>  drivers/iio/adc/ti-adc12138.c                      | 541 +++++++++++++++++++++
>>  4 files changed, 586 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
>>  create mode 100644 drivers/iio/adc/ti-adc12138.c
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt b/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
>> new file mode 100644
>> index 0000000..3a11d2a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
>> @@ -0,0 +1,32 @@
>> +* Texas Instruments' ADC12130/ADC12132/ADC12138
>> +
>> +Required properties:
>> + - compatible: Should be one of
>> +     * "ti,adc12130"
>> +     * "ti,adc12132"
>> +     * "ti,adc12138"
>> + - reg: SPI chip select number for the device
>> + - interrupts: Should contain interrupt for EOC (end of conversion)
>> + - clocks: phandle to conversion clock input
>> + - spi-max-frequency: Definision as per
>> +     Documentation/devicetree/bindings/spi/spi-bus.txt
>> + - vref-p-supply: The regulator supply for positive analog voltage reference
>> +
>> +Optional properties:
>> + - vref-n-supply: The regulator supply for negative analog voltage reference
>> +     If not specified, this is assumed to be analog ground.
>> + - acquisition-time: The number of conversion clock periods for the S/H's
>> +     acquisition time.  Should be one of 6, 10, 18, 34.  If not specified,
>> +     default value of 10 is used.
>> +
>> +Example:
>> +adc@0 {
>> +     compatible = "ti,adc12138";
>> +     reg = <0>;
>> +     interrupts = <28 IRQ_TYPE_EDGE_RISING>;
>> +     interrupt-parent = <&gpio1>;
>> +     clocks = <&cclk>;
>> +     vref-p-supply = <&ldo4_reg>;
>> +     spi-max-frequency = <5000000>;
>> +     acquisition-time = <6>;
>> +};
>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>> index fa0ff29..0783291 100644
>> --- a/drivers/iio/adc/Kconfig
>> +++ b/drivers/iio/adc/Kconfig
>> @@ -414,6 +414,18 @@ config TI_ADC0832
>>         This driver can also be built as a module. If so, the module will be
>>         called ti-adc0832.
>>
>> +config TI_ADC12138
>> +     tristate "Texas Instruments ADC12130/ADC12132/ADC12138"
>> +     depends on SPI
>> +     select IIO_BUFFER
>> +     select IIO_TRIGGERED_BUFFER
>> +     help
>> +       If you say yes here you get support for Texas Instruments ADC12130,
>> +       ADC12132and ADC12138 chips.
>
> space before 'and' missing

Ugh.

>> +
>> +       This driver can also be built as a module. If so, the module will be
>> +       called ti-adc12138.
>> +
>>  config TI_ADC128S052
>>       tristate "Texas Instruments ADC128S052/ADC122S021/ADC124S021"
>>       depends on SPI
>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>> index 599714b..3ae4ae3 100644
>> --- a/drivers/iio/adc/Makefile
>> +++ b/drivers/iio/adc/Makefile
>> @@ -40,6 +40,7 @@ obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
>>  obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
>>  obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>>  obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
>> +obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
>>  obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
>>  obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
>>  obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
>> diff --git a/drivers/iio/adc/ti-adc12138.c b/drivers/iio/adc/ti-adc12138.c
>> new file mode 100644
>> index 0000000..e7bd986
>> --- /dev/null
>> +++ b/drivers/iio/adc/ti-adc12138.c
>> @@ -0,0 +1,541 @@
>> +/*
>> + * ADC12130/ADC12132/ADC12138 12-bit plus sign ADC driver
>> + *
>> + * Copyright (c) 2016 Akinobu Mita <akinobu.mita@gmail.com>
>> + *
>> + * This file is subject to the terms and conditions of version 2 of
>> + * the GNU General Public License.  See the file COPYING in the main
>> + * directory of this archive for more details.
>> + *
>> + * Datasheet: http://www.ti.com/lit/ds/symlink/adc12138.pdf
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/completion.h>
>> +#include <linux/clk.h>
>> +#include <linux/spi/spi.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/buffer.h>
>> +#include <linux/iio/trigger.h>
>> +#include <linux/iio/triggered_buffer.h>
>> +#include <linux/iio/trigger_consumer.h>
>> +#include <linux/regulator/consumer.h>
>> +
>> +#define ADC12138_MODE_AUTO_CAL                       0x08
>> +#define ADC12138_MODE_READ_STATUS            0x0c
>> +#define ADC12138_MODE_ACQUISITION_TIME_6     0x0e
>> +#define ADC12138_MODE_ACQUISITION_TIME_10    0x4e
>> +#define ADC12138_MODE_ACQUISITION_TIME_18    0x8e
>> +#define ADC12138_MODE_ACQUISITION_TIME_34    0xce
>> +
>> +#define ADC12138_STATUS_CAL                  BIT(6)
>> +
>> +enum {
>> +     adc12130,
>> +     adc12132,
>> +     adc12138,
>> +};
>> +
>> +struct adc12138 {
>> +     struct spi_device *spi;
>> +     unsigned int id;
>> +     /* conversion clock */
>> +     struct clk *cclk;
>> +     /* positive analog voltage reference */
>> +     struct regulator *vref_p;
>> +     /* negative analog voltage reference */
>> +     struct regulator *vref_n;
>> +     struct mutex lock;
>> +     struct completion complete;
>> +     unsigned int acquisition_time;
>
> unit? ms? µs?

This is specified by the number of conversion clock (cclk) periods.
I'll add a comment about it.

>> +
>> +     u8 tx_buf[2] ____cacheline_aligned;
>> +     u8 rx_buf[2];
>> +};
>> +
>> +#define ADC12138_VOLTAGE_CHANNEL(chan)                                       \
>> +     {                                                               \
>> +             .type = IIO_VOLTAGE,                                    \
>> +             .indexed = 1,                                           \
>> +             .channel = chan,                                        \
>> +             .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),           \
>> +             .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE)    \
>> +                                     | BIT(IIO_CHAN_INFO_OFFSET),    \
>> +             .scan_index = chan,                                     \
>> +             .scan_type = {                                          \
>> +                     .sign = 's',                                    \
>> +                     .realbits = 13,                                 \
>> +                     .storagebits = 16,                              \
>> +                     .shift = 3,                                     \
>> +                     .endianness = IIO_BE,                           \
>> +             },                                                      \
>> +     }
>> +
>> +#define ADC12138_VOLTAGE_CHANNEL_DIFF(chan1, chan2, si_offset)               \
>> +     {                                                               \
>> +             .type = IIO_VOLTAGE,                                    \
>> +             .indexed = 1,                                           \
>> +             .channel = (chan1),                                     \
>> +             .channel2 = (chan2),                                    \
>> +             .differential = 1,                                      \
>> +             .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),           \
>> +             .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE)    \
>> +                                     | BIT(IIO_CHAN_INFO_OFFSET),    \
>> +             .scan_index = (chan1) + (si_offset),                    \
>> +             .scan_type = {                                          \
>> +                     .sign = 's',                                    \
>> +                     .realbits = 13,                                 \
>> +                     .storagebits = 16,                              \
>> +                     .shift = 3,                                     \
>> +                     .endianness = IIO_BE,                           \
>> +             },                                                      \
>> +     }
>> +
>> +static const struct iio_chan_spec adc12132_channels[] = {
>> +     ADC12138_VOLTAGE_CHANNEL(0),
>> +     ADC12138_VOLTAGE_CHANNEL(1),
>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(0, 1, 2),
>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(1, 0, 2),
>> +     IIO_CHAN_SOFT_TIMESTAMP(4),
>> +};
>> +
>> +static const struct iio_chan_spec adc12138_channels[] = {
>> +     ADC12138_VOLTAGE_CHANNEL(0),
>> +     ADC12138_VOLTAGE_CHANNEL(1),
>> +     ADC12138_VOLTAGE_CHANNEL(2),
>> +     ADC12138_VOLTAGE_CHANNEL(3),
>> +     ADC12138_VOLTAGE_CHANNEL(4),
>> +     ADC12138_VOLTAGE_CHANNEL(5),
>> +     ADC12138_VOLTAGE_CHANNEL(6),
>> +     ADC12138_VOLTAGE_CHANNEL(7),
>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(0, 1, 8),
>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(1, 0, 8),
>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(2, 3, 8),
>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(3, 2, 8),
>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(4, 5, 8),
>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(5, 4, 8),
>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(6, 7, 8),
>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(7, 6, 8),
>> +     IIO_CHAN_SOFT_TIMESTAMP(16),
>> +};
>> +
>> +static int adc12138_mode_programming(struct adc12138 *adc, u8 mode,
>> +                                  void *rx_buf, int len)
>> +{
>> +     struct spi_transfer xfer = {
>> +             .tx_buf = adc->tx_buf,
>> +             .rx_buf = adc->rx_buf,
>> +             .len = len,
>> +     };
>> +     int ret;
>> +
>> +     /* Skip disused bits for ADC12130 and ADC12132 */
>
> probably 'unused' instead of 'disused'

OK.

>> +     if (adc->id != adc12138)
>> +             mode = (mode & 0xc0) | ((mode & 0x0f) << 2);
>> +
>> +     adc->tx_buf[0] = mode;
>> +
>> +     ret = spi_sync_transfer(adc->spi, &xfer, 1);
>> +     if (ret)
>> +             return ret;
>> +
>> +     memcpy(rx_buf, adc->rx_buf, len);
>> +
>> +     return 0;
>> +}
>> +
>> +static int adc12138_read_status(struct adc12138 *adc)
>> +{
>> +     u8 rx_buf[2];
>> +     int ret;
>> +
>> +     ret = adc12138_mode_programming(adc, ADC12138_MODE_READ_STATUS,
>> +                                     rx_buf, 2);
>> +     if (ret)
>> +             return ret;
>> +
>> +     return (rx_buf[0] << 1) | (rx_buf[1] >> 7);
>> +}
>> +
>> +static int __adc12138_start_conv(struct adc12138 *adc,
>> +                              struct iio_chan_spec const *channel,
>> +                              void *data, int len)
>> +
>> +{
>> +     const u8 ch_to_mux[] = { 0, 4, 1, 5, 2, 6, 3, 7 };
>> +     u8 mode = (ch_to_mux[channel->channel] << 4) |
>> +               (channel->differential ? 0 : 0x80);
>> +
>> +     return adc12138_mode_programming(adc, mode, data, len);
>> +}
>> +
>> +static int adc12138_start_conv(struct adc12138 *adc,
>> +                            struct iio_chan_spec const *channel)
>> +{
>> +     u8 trash;
>> +
>> +     return __adc12138_start_conv(adc, channel, &trash, 1);
>> +}
>> +
>> +static int adc12138_start_and_read_conv(struct adc12138 *adc,
>> +                                     struct iio_chan_spec const *channel,
>> +                                     __be16 *data)
>> +{
>> +     return __adc12138_start_conv(adc, channel, data, 2);
>> +}
>> +
>> +static int adc12138_read_conv_data(struct adc12138 *adc, __be16 *value)
>> +{
>> +     /* Issue a read status instruction and read previous conversion data */
>> +     return adc12138_mode_programming(adc, ADC12138_MODE_READ_STATUS,
>> +                                      value, sizeof(*value));
>> +}
>> +
>> +static int adc12138_wait_eoc(struct adc12138 *adc, unsigned long timeout)
>> +{
>> +     if (!wait_for_completion_timeout(&adc->complete, timeout))
>> +             return -ETIMEDOUT;
>> +
>> +     return 0;
>> +}
>> +
>> +static int adc12138_adc_conversion(struct adc12138 *adc,
>> +                                struct iio_chan_spec const *channel,
>> +                                __be16 *value)
>> +{
>> +     int ret;
>> +
>> +     reinit_completion(&adc->complete);
>> +
>> +     ret = adc12138_start_conv(adc, channel);
>> +     if (ret)
>> +             return ret;
>> +
>> +     ret = adc12138_wait_eoc(adc, msecs_to_jiffies(100));
>> +     if (ret)
>> +             return ret;
>> +
>> +     return adc12138_read_conv_data(adc, value);
>> +}
>> +
>> +static int adc12138_read_raw(struct iio_dev *iio,
>> +                          struct iio_chan_spec const *channel, int *value,
>> +                          int *shift, long mask)
>> +{
>> +     struct adc12138 *adc = iio_priv(iio);
>> +     int ret;
>> +     __be16 data;
>> +
>> +     switch (mask) {
>> +     case IIO_CHAN_INFO_RAW:
>
> what if buffered reads have been set up? probably use
> iio_device_claim_direct_mode()

Maybe it is safe because the ADC data is always requested and retrieved
while holding adc12138.lock from adc12138_read_raw() and
adc12138_trigger_handler().  Is it better to restrict direct mode and
return -EBUSY while buffer mode is enabled?

(BTW, I realized that I can use iio_dev->mlock instead of adc12138.lock)

>> +             mutex_lock(&adc->lock);
>> +             ret = adc12138_adc_conversion(adc, channel, &data);
>> +             mutex_unlock(&adc->lock);
>> +             if (ret)
>> +                     return ret;
>> +
>> +             *value = sign_extend32(be16_to_cpu(data) >> 3, 12);
>> +
>> +             return IIO_VAL_INT;
>> +     case IIO_CHAN_INFO_SCALE:
>> +             ret = regulator_get_voltage(adc->vref_p);
>> +             if (ret < 0)
>> +                     return ret;
>> +             *value = ret;
>> +
>> +             if (!IS_ERR(adc->vref_n)) {
>> +                     ret = regulator_get_voltage(adc->vref_n);
>> +                     if (ret < 0)
>> +                             return ret;
>> +                     *value -= ret;
>> +             }
>> +
>> +             /* convert regulator output voltage to mV */
>> +             *value /= 1000;
>> +             *shift = channel->scan_type.realbits - 1;
>> +
>> +             return IIO_VAL_FRACTIONAL_LOG2;
>> +     case IIO_CHAN_INFO_OFFSET:
>> +             if (!IS_ERR(adc->vref_n)) {
>> +                     *value = regulator_get_voltage(adc->vref_n);
>> +                     if (*value < 0)
>> +                             return *value;
>> +             } else {
>> +                     *value = 0;
>> +             }
>> +
>> +             /* convert regulator output voltage to mV */
>> +             *value /= 1000;
>> +
>> +             return IIO_VAL_INT;
>> +     }
>> +
>> +     return -EINVAL;
>> +}
>> +
>> +static const struct iio_info adc12138_info = {
>> +     .read_raw = adc12138_read_raw,
>> +     .driver_module = THIS_MODULE,
>> +};
>> +
>> +static int adc12138_init(struct adc12138 *adc)
>> +{
>> +     int ret;
>> +     int status;
>> +     u8 mode;
>> +     u8 trash;
>> +
>> +     reinit_completion(&adc->complete);
>> +
>> +     ret = adc12138_mode_programming(adc, ADC12138_MODE_AUTO_CAL, &trash, 1);
>> +     if (ret)
>> +             return ret;
>> +
>> +     /* data output at this time has no significance */
>> +     status = adc12138_read_status(adc);
>> +     if (status < 0)
>> +             return status;
>> +
>> +     adc12138_wait_eoc(adc, msecs_to_jiffies(100));
>> +
>> +     status = adc12138_read_status(adc);
>> +     if (status & ADC12138_STATUS_CAL) {
>> +             dev_warn(&adc->spi->dev,
>> +                     "Auto Cal sequence is still in progress: %#x\n",
>> +                     status);
>> +             return -EIO;
>> +     }
>> +
>> +     switch (adc->acquisition_time) {
>> +     case 6:
>> +             mode = ADC12138_MODE_ACQUISITION_TIME_6;
>> +             break;
>> +     case 10:
>> +             mode = ADC12138_MODE_ACQUISITION_TIME_10;
>> +             break;
>> +     case 18:
>> +             mode = ADC12138_MODE_ACQUISITION_TIME_18;
>> +             break;
>> +     case 34:
>> +             mode = ADC12138_MODE_ACQUISITION_TIME_34;
>> +             break;
>> +     default:
>> +             return -EINVAL;
>> +     }
>> +
>> +     return adc12138_mode_programming(adc, mode, &trash, 1);
>> +}
>> +
>> +static irqreturn_t adc12138_trigger_handler(int irq, void *p)
>> +{
>> +     struct iio_poll_func *pf = p;
>> +     struct iio_dev *indio_dev = pf->indio_dev;
>> +     struct adc12138 *adc = iio_priv(indio_dev);
>> +     __be16 data[20] = { };
>
> why { } here?

To initialize with zero and avoids exposing kernel stack to userspace
through padding entries.  I could use memset here.

> why 20?

When all scan elements are enabled, the sample data requires 40 bytes
(2 bytes ADC data * 16 channels + 8 bytes timestamp).

I'll add comments about these.

>> +     __be16 trash;
>> +     int ret;
>> +     int scan_index;
>> +     int i = 0;
>> +
>> +     mutex_lock(&adc->lock);
>> +
>> +     for_each_set_bit(scan_index, indio_dev->active_scan_mask,
>> +                      indio_dev->masklength) {
>> +             const struct iio_chan_spec *scan_chan =
>> +                             &indio_dev->channels[scan_index];
>> +
>> +             reinit_completion(&adc->complete);
>> +
>> +             ret = adc12138_start_and_read_conv(adc, scan_chan,
>> +                                                i ? &data[i - 1] : &trash);
>> +             if (ret) {
>> +                     dev_warn(&adc->spi->dev,
>> +                              "failed to start conversion\n");
>> +                     goto out;
>> +             }
>> +
>> +             ret = adc12138_wait_eoc(adc, msecs_to_jiffies(100));
>> +             if (ret) {
>> +                     dev_warn(&adc->spi->dev, "wait eoc timeout\n");
>> +                     goto out;
>> +             }
>> +
>> +             i++;
>> +     }
>> +
>> +     if (i) {
>> +             ret = adc12138_read_conv_data(adc, &data[i - 1]);
>> +             if (ret) {
>> +                     dev_warn(&adc->spi->dev,
>> +                              "failed to get conversion data\n");
>> +                     goto out;
>> +             }
>> +     }
>> +
>> +     iio_push_to_buffers_with_timestamp(indio_dev, data, iio_get_time_ns());
>> +out:
>> +     mutex_unlock(&adc->lock);
>> +
>> +     iio_trigger_notify_done(indio_dev->trig);
>> +
>> +     return IRQ_HANDLED;
>> +}

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] iio: adc: add ADC12130/ADC12132/ADC12138 ADC driver
  2016-07-18 15:00 ` Akinobu Mita
@ 2016-07-20  1:11     ` Rob Herring
  -1 siblings, 0 replies; 10+ messages in thread
From: Rob Herring @ 2016-07-20  1:11 UTC (permalink / raw)
  To: Akinobu Mita
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron,
	Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald

On Tue, Jul 19, 2016 at 12:00:29AM +0900, Akinobu Mita wrote:
> This adds Texas Instruments' ADC12130/ADC12132/ADC12138 12-bit plus
> sign ADC driver.  I have tested with the ADC12138.  The ADC12130 and
> ADC12132 are not tested but these are similar to ADC12138 except that
> the mode programming instruction is a bit different.
> 
> Signed-off-by: Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Hartmut Knaack <knaack.h-Mmb7MZpHnFY@public.gmane.org>
> Cc: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
> Cc: Peter Meerwald <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>
> ---
>  .../devicetree/bindings/iio/adc/ti-adc12138.txt    |  32 ++

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

>  drivers/iio/adc/Kconfig                            |  12 +
>  drivers/iio/adc/Makefile                           |   1 +
>  drivers/iio/adc/ti-adc12138.c                      | 541 +++++++++++++++++++++
>  4 files changed, 586 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
>  create mode 100644 drivers/iio/adc/ti-adc12138.c

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] iio: adc: add ADC12130/ADC12132/ADC12138 ADC driver
@ 2016-07-20  1:11     ` Rob Herring
  0 siblings, 0 replies; 10+ messages in thread
From: Rob Herring @ 2016-07-20  1:11 UTC (permalink / raw)
  To: Akinobu Mita
  Cc: linux-iio, devicetree, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald

On Tue, Jul 19, 2016 at 12:00:29AM +0900, Akinobu Mita wrote:
> This adds Texas Instruments' ADC12130/ADC12132/ADC12138 12-bit plus
> sign ADC driver.  I have tested with the ADC12138.  The ADC12130 and
> ADC12132 are not tested but these are similar to ADC12138 except that
> the mode programming instruction is a bit different.
> 
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: Jonathan Cameron <jic23@kernel.org>
> Cc: Hartmut Knaack <knaack.h@gmx.de>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Peter Meerwald <pmeerw@pmeerw.net>
> ---
>  .../devicetree/bindings/iio/adc/ti-adc12138.txt    |  32 ++

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/iio/adc/Kconfig                            |  12 +
>  drivers/iio/adc/Makefile                           |   1 +
>  drivers/iio/adc/ti-adc12138.c                      | 541 +++++++++++++++++++++
>  4 files changed, 586 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
>  create mode 100644 drivers/iio/adc/ti-adc12138.c

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] iio: adc: add ADC12130/ADC12132/ADC12138 ADC driver
  2016-07-19 16:24         ` Akinobu Mita
@ 2016-07-20 15:05             ` Jonathan Cameron
  -1 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2016-07-20 15:05 UTC (permalink / raw)
  To: Akinobu Mita, Peter Meerwald-Stadler
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, open list:OPEN FIRMWARE AND...,
	Hartmut Knaack, Lars-Peter Clausen

On 19/07/16 17:24, Akinobu Mita wrote:
> 2016-07-19 7:02 GMT+09:00 Peter Meerwald-Stadler <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>:
>>
>>> This adds Texas Instruments' ADC12130/ADC12132/ADC12138 12-bit plus
>>> sign ADC driver.  I have tested with the ADC12138.  The ADC12130 and
>>> ADC12132 are not tested but these are similar to ADC12138 except that
>>> the mode programming instruction is a bit different.
>>
>> minor comments below
>>
>>> Signed-off-by: Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> Cc: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>> Cc: Hartmut Knaack <knaack.h-Mmb7MZpHnFY@public.gmane.org>
>>> Cc: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
>>> Cc: Peter Meerwald <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>
>>> ---
>>>  .../devicetree/bindings/iio/adc/ti-adc12138.txt    |  32 ++
>>>  drivers/iio/adc/Kconfig                            |  12 +
>>>  drivers/iio/adc/Makefile                           |   1 +
>>>  drivers/iio/adc/ti-adc12138.c                      | 541 +++++++++++++++++++++
>>>  4 files changed, 586 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
>>>  create mode 100644 drivers/iio/adc/ti-adc12138.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt b/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
>>> new file mode 100644
>>> index 0000000..3a11d2a
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
>>> @@ -0,0 +1,32 @@
>>> +* Texas Instruments' ADC12130/ADC12132/ADC12138
>>> +
>>> +Required properties:
>>> + - compatible: Should be one of
>>> +     * "ti,adc12130"
>>> +     * "ti,adc12132"
>>> +     * "ti,adc12138"
>>> + - reg: SPI chip select number for the device
>>> + - interrupts: Should contain interrupt for EOC (end of conversion)
>>> + - clocks: phandle to conversion clock input
>>> + - spi-max-frequency: Definision as per
>>> +     Documentation/devicetree/bindings/spi/spi-bus.txt
>>> + - vref-p-supply: The regulator supply for positive analog voltage reference
>>> +
>>> +Optional properties:
>>> + - vref-n-supply: The regulator supply for negative analog voltage reference
>>> +     If not specified, this is assumed to be analog ground.
>>> + - acquisition-time: The number of conversion clock periods for the S/H's
>>> +     acquisition time.  Should be one of 6, 10, 18, 34.  If not specified,
>>> +     default value of 10 is used.
>>> +
>>> +Example:
>>> +adc@0 {
>>> +     compatible = "ti,adc12138";
>>> +     reg = <0>;
>>> +     interrupts = <28 IRQ_TYPE_EDGE_RISING>;
>>> +     interrupt-parent = <&gpio1>;
>>> +     clocks = <&cclk>;
>>> +     vref-p-supply = <&ldo4_reg>;
>>> +     spi-max-frequency = <5000000>;
>>> +     acquisition-time = <6>;
>>> +};
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index fa0ff29..0783291 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -414,6 +414,18 @@ config TI_ADC0832
>>>         This driver can also be built as a module. If so, the module will be
>>>         called ti-adc0832.
>>>
>>> +config TI_ADC12138
>>> +     tristate "Texas Instruments ADC12130/ADC12132/ADC12138"
>>> +     depends on SPI
>>> +     select IIO_BUFFER
>>> +     select IIO_TRIGGERED_BUFFER
>>> +     help
>>> +       If you say yes here you get support for Texas Instruments ADC12130,
>>> +       ADC12132and ADC12138 chips.
>>
>> space before 'and' missing
> 
> Ugh.
> 
>>> +
>>> +       This driver can also be built as a module. If so, the module will be
>>> +       called ti-adc12138.
>>> +
>>>  config TI_ADC128S052
>>>       tristate "Texas Instruments ADC128S052/ADC122S021/ADC124S021"
>>>       depends on SPI
>>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>>> index 599714b..3ae4ae3 100644
>>> --- a/drivers/iio/adc/Makefile
>>> +++ b/drivers/iio/adc/Makefile
>>> @@ -40,6 +40,7 @@ obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
>>>  obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
>>>  obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>>>  obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
>>> +obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
>>>  obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
>>>  obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
>>>  obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
>>> diff --git a/drivers/iio/adc/ti-adc12138.c b/drivers/iio/adc/ti-adc12138.c
>>> new file mode 100644
>>> index 0000000..e7bd986
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/ti-adc12138.c
>>> @@ -0,0 +1,541 @@
>>> +/*
>>> + * ADC12130/ADC12132/ADC12138 12-bit plus sign ADC driver
>>> + *
>>> + * Copyright (c) 2016 Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> + *
>>> + * This file is subject to the terms and conditions of version 2 of
>>> + * the GNU General Public License.  See the file COPYING in the main
>>> + * directory of this archive for more details.
>>> + *
>>> + * Datasheet: http://www.ti.com/lit/ds/symlink/adc12138.pdf
>>> + */
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/completion.h>
>>> +#include <linux/clk.h>
>>> +#include <linux/spi/spi.h>
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/iio/buffer.h>
>>> +#include <linux/iio/trigger.h>
>>> +#include <linux/iio/triggered_buffer.h>
>>> +#include <linux/iio/trigger_consumer.h>
>>> +#include <linux/regulator/consumer.h>
>>> +
>>> +#define ADC12138_MODE_AUTO_CAL                       0x08
>>> +#define ADC12138_MODE_READ_STATUS            0x0c
>>> +#define ADC12138_MODE_ACQUISITION_TIME_6     0x0e
>>> +#define ADC12138_MODE_ACQUISITION_TIME_10    0x4e
>>> +#define ADC12138_MODE_ACQUISITION_TIME_18    0x8e
>>> +#define ADC12138_MODE_ACQUISITION_TIME_34    0xce
>>> +
>>> +#define ADC12138_STATUS_CAL                  BIT(6)
>>> +
>>> +enum {
>>> +     adc12130,
>>> +     adc12132,
>>> +     adc12138,
>>> +};
>>> +
>>> +struct adc12138 {
>>> +     struct spi_device *spi;
>>> +     unsigned int id;
>>> +     /* conversion clock */
>>> +     struct clk *cclk;
>>> +     /* positive analog voltage reference */
>>> +     struct regulator *vref_p;
>>> +     /* negative analog voltage reference */
>>> +     struct regulator *vref_n;
>>> +     struct mutex lock;
>>> +     struct completion complete;
>>> +     unsigned int acquisition_time;
>>
>> unit? ms? µs?
> 
> This is specified by the number of conversion clock (cclk) periods.
> I'll add a comment about it.
> 
>>> +
>>> +     u8 tx_buf[2] ____cacheline_aligned;
>>> +     u8 rx_buf[2];
>>> +};
>>> +
>>> +#define ADC12138_VOLTAGE_CHANNEL(chan)                                       \
>>> +     {                                                               \
>>> +             .type = IIO_VOLTAGE,                                    \
>>> +             .indexed = 1,                                           \
>>> +             .channel = chan,                                        \
>>> +             .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),           \
>>> +             .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE)    \
>>> +                                     | BIT(IIO_CHAN_INFO_OFFSET),    \
>>> +             .scan_index = chan,                                     \
>>> +             .scan_type = {                                          \
>>> +                     .sign = 's',                                    \
>>> +                     .realbits = 13,                                 \
>>> +                     .storagebits = 16,                              \
>>> +                     .shift = 3,                                     \
>>> +                     .endianness = IIO_BE,                           \
>>> +             },                                                      \
>>> +     }
>>> +
>>> +#define ADC12138_VOLTAGE_CHANNEL_DIFF(chan1, chan2, si_offset)               \
>>> +     {                                                               \
>>> +             .type = IIO_VOLTAGE,                                    \
>>> +             .indexed = 1,                                           \
>>> +             .channel = (chan1),                                     \
>>> +             .channel2 = (chan2),                                    \
>>> +             .differential = 1,                                      \
>>> +             .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),           \
>>> +             .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE)    \
>>> +                                     | BIT(IIO_CHAN_INFO_OFFSET),    \
>>> +             .scan_index = (chan1) + (si_offset),                    \
>>> +             .scan_type = {                                          \
>>> +                     .sign = 's',                                    \
>>> +                     .realbits = 13,                                 \
>>> +                     .storagebits = 16,                              \
>>> +                     .shift = 3,                                     \
>>> +                     .endianness = IIO_BE,                           \
>>> +             },                                                      \
>>> +     }
>>> +
>>> +static const struct iio_chan_spec adc12132_channels[] = {
>>> +     ADC12138_VOLTAGE_CHANNEL(0),
>>> +     ADC12138_VOLTAGE_CHANNEL(1),
>>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(0, 1, 2),
>>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(1, 0, 2),
>>> +     IIO_CHAN_SOFT_TIMESTAMP(4),
>>> +};
>>> +
>>> +static const struct iio_chan_spec adc12138_channels[] = {
>>> +     ADC12138_VOLTAGE_CHANNEL(0),
>>> +     ADC12138_VOLTAGE_CHANNEL(1),
>>> +     ADC12138_VOLTAGE_CHANNEL(2),
>>> +     ADC12138_VOLTAGE_CHANNEL(3),
>>> +     ADC12138_VOLTAGE_CHANNEL(4),
>>> +     ADC12138_VOLTAGE_CHANNEL(5),
>>> +     ADC12138_VOLTAGE_CHANNEL(6),
>>> +     ADC12138_VOLTAGE_CHANNEL(7),
>>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(0, 1, 8),
>>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(1, 0, 8),
>>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(2, 3, 8),
>>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(3, 2, 8),
>>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(4, 5, 8),
>>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(5, 4, 8),
>>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(6, 7, 8),
>>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(7, 6, 8),
>>> +     IIO_CHAN_SOFT_TIMESTAMP(16),
>>> +};
>>> +
>>> +static int adc12138_mode_programming(struct adc12138 *adc, u8 mode,
>>> +                                  void *rx_buf, int len)
>>> +{
>>> +     struct spi_transfer xfer = {
>>> +             .tx_buf = adc->tx_buf,
>>> +             .rx_buf = adc->rx_buf,
>>> +             .len = len,
>>> +     };
>>> +     int ret;
>>> +
>>> +     /* Skip disused bits for ADC12130 and ADC12132 */
>>
>> probably 'unused' instead of 'disused'
> 
> OK.
> 
>>> +     if (adc->id != adc12138)
>>> +             mode = (mode & 0xc0) | ((mode & 0x0f) << 2);
>>> +
>>> +     adc->tx_buf[0] = mode;
>>> +
>>> +     ret = spi_sync_transfer(adc->spi, &xfer, 1);
>>> +     if (ret)
>>> +             return ret;
>>> +
>>> +     memcpy(rx_buf, adc->rx_buf, len);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int adc12138_read_status(struct adc12138 *adc)
>>> +{
>>> +     u8 rx_buf[2];
>>> +     int ret;
>>> +
>>> +     ret = adc12138_mode_programming(adc, ADC12138_MODE_READ_STATUS,
>>> +                                     rx_buf, 2);
>>> +     if (ret)
>>> +             return ret;
>>> +
>>> +     return (rx_buf[0] << 1) | (rx_buf[1] >> 7);
>>> +}
>>> +
>>> +static int __adc12138_start_conv(struct adc12138 *adc,
>>> +                              struct iio_chan_spec const *channel,
>>> +                              void *data, int len)
>>> +
>>> +{
>>> +     const u8 ch_to_mux[] = { 0, 4, 1, 5, 2, 6, 3, 7 };
>>> +     u8 mode = (ch_to_mux[channel->channel] << 4) |
>>> +               (channel->differential ? 0 : 0x80);
>>> +
>>> +     return adc12138_mode_programming(adc, mode, data, len);
>>> +}
>>> +
>>> +static int adc12138_start_conv(struct adc12138 *adc,
>>> +                            struct iio_chan_spec const *channel)
>>> +{
>>> +     u8 trash;
>>> +
>>> +     return __adc12138_start_conv(adc, channel, &trash, 1);
>>> +}
>>> +
>>> +static int adc12138_start_and_read_conv(struct adc12138 *adc,
>>> +                                     struct iio_chan_spec const *channel,
>>> +                                     __be16 *data)
>>> +{
>>> +     return __adc12138_start_conv(adc, channel, data, 2);
>>> +}
>>> +
>>> +static int adc12138_read_conv_data(struct adc12138 *adc, __be16 *value)
>>> +{
>>> +     /* Issue a read status instruction and read previous conversion data */
>>> +     return adc12138_mode_programming(adc, ADC12138_MODE_READ_STATUS,
>>> +                                      value, sizeof(*value));
>>> +}
>>> +
>>> +static int adc12138_wait_eoc(struct adc12138 *adc, unsigned long timeout)
>>> +{
>>> +     if (!wait_for_completion_timeout(&adc->complete, timeout))
>>> +             return -ETIMEDOUT;
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int adc12138_adc_conversion(struct adc12138 *adc,
>>> +                                struct iio_chan_spec const *channel,
>>> +                                __be16 *value)
>>> +{
>>> +     int ret;
>>> +
>>> +     reinit_completion(&adc->complete);
>>> +
>>> +     ret = adc12138_start_conv(adc, channel);
>>> +     if (ret)
>>> +             return ret;
>>> +
>>> +     ret = adc12138_wait_eoc(adc, msecs_to_jiffies(100));
>>> +     if (ret)
>>> +             return ret;
>>> +
>>> +     return adc12138_read_conv_data(adc, value);
>>> +}
>>> +
>>> +static int adc12138_read_raw(struct iio_dev *iio,
>>> +                          struct iio_chan_spec const *channel, int *value,
>>> +                          int *shift, long mask)
>>> +{
>>> +     struct adc12138 *adc = iio_priv(iio);
>>> +     int ret;
>>> +     __be16 data;
>>> +
>>> +     switch (mask) {
>>> +     case IIO_CHAN_INFO_RAW:
>>
>> what if buffered reads have been set up? probably use
>> iio_device_claim_direct_mode()
> 
> Maybe it is safe because the ADC data is always requested and retrieved
> while holding adc12138.lock from adc12138_read_raw() and
> adc12138_trigger_handler().  Is it better to restrict direct mode and
> return -EBUSY while buffer mode is enabled?
> 
> (BTW, I realized that I can use iio_dev->mlock instead of adc12138.lock)
Don't use it for anything other than state change locking.  It has that
well focused task within the subsystem - we are trying to clear out other
uses entirely as there have been more than a few bugs around this.

Much better to have a separate lock for protecting resources controlled
directly by the driver itself.
> 
>>> +             mutex_lock(&adc->lock);
>>> +             ret = adc12138_adc_conversion(adc, channel, &data);
>>> +             mutex_unlock(&adc->lock);
>>> +             if (ret)
>>> +                     return ret;
>>> +
>>> +             *value = sign_extend32(be16_to_cpu(data) >> 3, 12);
>>> +
>>> +             return IIO_VAL_INT;
>>> +     case IIO_CHAN_INFO_SCALE:
>>> +             ret = regulator_get_voltage(adc->vref_p);
>>> +             if (ret < 0)
>>> +                     return ret;
>>> +             *value = ret;
>>> +
>>> +             if (!IS_ERR(adc->vref_n)) {
>>> +                     ret = regulator_get_voltage(adc->vref_n);
>>> +                     if (ret < 0)
>>> +                             return ret;
>>> +                     *value -= ret;
>>> +             }
>>> +
>>> +             /* convert regulator output voltage to mV */
>>> +             *value /= 1000;
>>> +             *shift = channel->scan_type.realbits - 1;
>>> +
>>> +             return IIO_VAL_FRACTIONAL_LOG2;
>>> +     case IIO_CHAN_INFO_OFFSET:
>>> +             if (!IS_ERR(adc->vref_n)) {
>>> +                     *value = regulator_get_voltage(adc->vref_n);
>>> +                     if (*value < 0)
>>> +                             return *value;
>>> +             } else {
>>> +                     *value = 0;
>>> +             }
>>> +
>>> +             /* convert regulator output voltage to mV */
>>> +             *value /= 1000;
>>> +
>>> +             return IIO_VAL_INT;
>>> +     }
>>> +
>>> +     return -EINVAL;
>>> +}
>>> +
>>> +static const struct iio_info adc12138_info = {
>>> +     .read_raw = adc12138_read_raw,
>>> +     .driver_module = THIS_MODULE,
>>> +};
>>> +
>>> +static int adc12138_init(struct adc12138 *adc)
>>> +{
>>> +     int ret;
>>> +     int status;
>>> +     u8 mode;
>>> +     u8 trash;
>>> +
>>> +     reinit_completion(&adc->complete);
>>> +
>>> +     ret = adc12138_mode_programming(adc, ADC12138_MODE_AUTO_CAL, &trash, 1);
>>> +     if (ret)
>>> +             return ret;
>>> +
>>> +     /* data output at this time has no significance */
>>> +     status = adc12138_read_status(adc);
>>> +     if (status < 0)
>>> +             return status;
>>> +
>>> +     adc12138_wait_eoc(adc, msecs_to_jiffies(100));
>>> +
>>> +     status = adc12138_read_status(adc);
>>> +     if (status & ADC12138_STATUS_CAL) {
>>> +             dev_warn(&adc->spi->dev,
>>> +                     "Auto Cal sequence is still in progress: %#x\n",
>>> +                     status);
>>> +             return -EIO;
>>> +     }
>>> +
>>> +     switch (adc->acquisition_time) {
>>> +     case 6:
>>> +             mode = ADC12138_MODE_ACQUISITION_TIME_6;
>>> +             break;
>>> +     case 10:
>>> +             mode = ADC12138_MODE_ACQUISITION_TIME_10;
>>> +             break;
>>> +     case 18:
>>> +             mode = ADC12138_MODE_ACQUISITION_TIME_18;
>>> +             break;
>>> +     case 34:
>>> +             mode = ADC12138_MODE_ACQUISITION_TIME_34;
>>> +             break;
>>> +     default:
>>> +             return -EINVAL;
>>> +     }
>>> +
>>> +     return adc12138_mode_programming(adc, mode, &trash, 1);
>>> +}
>>> +
>>> +static irqreturn_t adc12138_trigger_handler(int irq, void *p)
>>> +{
>>> +     struct iio_poll_func *pf = p;
>>> +     struct iio_dev *indio_dev = pf->indio_dev;
>>> +     struct adc12138 *adc = iio_priv(indio_dev);
>>> +     __be16 data[20] = { };
>>
>> why { } here?
> 
> To initialize with zero and avoids exposing kernel stack to userspace
> through padding entries.  I could use memset here.
> 
>> why 20?
> 
> When all scan elements are enabled, the sample data requires 40 bytes
> (2 bytes ADC data * 16 channels + 8 bytes timestamp).
> 
> I'll add comments about these.
> 
>>> +     __be16 trash;
>>> +     int ret;
>>> +     int scan_index;
>>> +     int i = 0;
>>> +
>>> +     mutex_lock(&adc->lock);
>>> +
>>> +     for_each_set_bit(scan_index, indio_dev->active_scan_mask,
>>> +                      indio_dev->masklength) {
>>> +             const struct iio_chan_spec *scan_chan =
>>> +                             &indio_dev->channels[scan_index];
>>> +
>>> +             reinit_completion(&adc->complete);
>>> +
>>> +             ret = adc12138_start_and_read_conv(adc, scan_chan,
>>> +                                                i ? &data[i - 1] : &trash);
>>> +             if (ret) {
>>> +                     dev_warn(&adc->spi->dev,
>>> +                              "failed to start conversion\n");
>>> +                     goto out;
>>> +             }
>>> +
>>> +             ret = adc12138_wait_eoc(adc, msecs_to_jiffies(100));
>>> +             if (ret) {
>>> +                     dev_warn(&adc->spi->dev, "wait eoc timeout\n");
>>> +                     goto out;
>>> +             }
>>> +
>>> +             i++;
>>> +     }
>>> +
>>> +     if (i) {
>>> +             ret = adc12138_read_conv_data(adc, &data[i - 1]);
>>> +             if (ret) {
>>> +                     dev_warn(&adc->spi->dev,
>>> +                              "failed to get conversion data\n");
>>> +                     goto out;
>>> +             }
>>> +     }
>>> +
>>> +     iio_push_to_buffers_with_timestamp(indio_dev, data, iio_get_time_ns());
>>> +out:
>>> +     mutex_unlock(&adc->lock);
>>> +
>>> +     iio_trigger_notify_done(indio_dev->trig);
>>> +
>>> +     return IRQ_HANDLED;
>>> +}
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] iio: adc: add ADC12130/ADC12132/ADC12138 ADC driver
@ 2016-07-20 15:05             ` Jonathan Cameron
  0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2016-07-20 15:05 UTC (permalink / raw)
  To: Akinobu Mita, Peter Meerwald-Stadler
  Cc: linux-iio, open list:OPEN FIRMWARE AND...,
	Hartmut Knaack, Lars-Peter Clausen

On 19/07/16 17:24, Akinobu Mita wrote:
> 2016-07-19 7:02 GMT+09:00 Peter Meerwald-Stadler <pmeerw@pmeerw.net>:
>>
>>> This adds Texas Instruments' ADC12130/ADC12132/ADC12138 12-bit plus
>>> sign ADC driver.  I have tested with the ADC12138.  The ADC12130 and
>>> ADC12132 are not tested but these are similar to ADC12138 except that
>>> the mode programming instruction is a bit different.
>>
>> minor comments below
>>
>>> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
>>> Cc: Jonathan Cameron <jic23@kernel.org>
>>> Cc: Hartmut Knaack <knaack.h@gmx.de>
>>> Cc: Lars-Peter Clausen <lars@metafoo.de>
>>> Cc: Peter Meerwald <pmeerw@pmeerw.net>
>>> ---
>>>  .../devicetree/bindings/iio/adc/ti-adc12138.txt    |  32 ++
>>>  drivers/iio/adc/Kconfig                            |  12 +
>>>  drivers/iio/adc/Makefile                           |   1 +
>>>  drivers/iio/adc/ti-adc12138.c                      | 541 +++++++++++++++++++++
>>>  4 files changed, 586 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
>>>  create mode 100644 drivers/iio/adc/ti-adc12138.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt b/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
>>> new file mode 100644
>>> index 0000000..3a11d2a
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
>>> @@ -0,0 +1,32 @@
>>> +* Texas Instruments' ADC12130/ADC12132/ADC12138
>>> +
>>> +Required properties:
>>> + - compatible: Should be one of
>>> +     * "ti,adc12130"
>>> +     * "ti,adc12132"
>>> +     * "ti,adc12138"
>>> + - reg: SPI chip select number for the device
>>> + - interrupts: Should contain interrupt for EOC (end of conversion)
>>> + - clocks: phandle to conversion clock input
>>> + - spi-max-frequency: Definision as per
>>> +     Documentation/devicetree/bindings/spi/spi-bus.txt
>>> + - vref-p-supply: The regulator supply for positive analog voltage reference
>>> +
>>> +Optional properties:
>>> + - vref-n-supply: The regulator supply for negative analog voltage reference
>>> +     If not specified, this is assumed to be analog ground.
>>> + - acquisition-time: The number of conversion clock periods for the S/H's
>>> +     acquisition time.  Should be one of 6, 10, 18, 34.  If not specified,
>>> +     default value of 10 is used.
>>> +
>>> +Example:
>>> +adc@0 {
>>> +     compatible = "ti,adc12138";
>>> +     reg = <0>;
>>> +     interrupts = <28 IRQ_TYPE_EDGE_RISING>;
>>> +     interrupt-parent = <&gpio1>;
>>> +     clocks = <&cclk>;
>>> +     vref-p-supply = <&ldo4_reg>;
>>> +     spi-max-frequency = <5000000>;
>>> +     acquisition-time = <6>;
>>> +};
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index fa0ff29..0783291 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -414,6 +414,18 @@ config TI_ADC0832
>>>         This driver can also be built as a module. If so, the module will be
>>>         called ti-adc0832.
>>>
>>> +config TI_ADC12138
>>> +     tristate "Texas Instruments ADC12130/ADC12132/ADC12138"
>>> +     depends on SPI
>>> +     select IIO_BUFFER
>>> +     select IIO_TRIGGERED_BUFFER
>>> +     help
>>> +       If you say yes here you get support for Texas Instruments ADC12130,
>>> +       ADC12132and ADC12138 chips.
>>
>> space before 'and' missing
> 
> Ugh.
> 
>>> +
>>> +       This driver can also be built as a module. If so, the module will be
>>> +       called ti-adc12138.
>>> +
>>>  config TI_ADC128S052
>>>       tristate "Texas Instruments ADC128S052/ADC122S021/ADC124S021"
>>>       depends on SPI
>>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>>> index 599714b..3ae4ae3 100644
>>> --- a/drivers/iio/adc/Makefile
>>> +++ b/drivers/iio/adc/Makefile
>>> @@ -40,6 +40,7 @@ obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
>>>  obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
>>>  obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>>>  obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
>>> +obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
>>>  obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
>>>  obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
>>>  obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
>>> diff --git a/drivers/iio/adc/ti-adc12138.c b/drivers/iio/adc/ti-adc12138.c
>>> new file mode 100644
>>> index 0000000..e7bd986
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/ti-adc12138.c
>>> @@ -0,0 +1,541 @@
>>> +/*
>>> + * ADC12130/ADC12132/ADC12138 12-bit plus sign ADC driver
>>> + *
>>> + * Copyright (c) 2016 Akinobu Mita <akinobu.mita@gmail.com>
>>> + *
>>> + * This file is subject to the terms and conditions of version 2 of
>>> + * the GNU General Public License.  See the file COPYING in the main
>>> + * directory of this archive for more details.
>>> + *
>>> + * Datasheet: http://www.ti.com/lit/ds/symlink/adc12138.pdf
>>> + */
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/completion.h>
>>> +#include <linux/clk.h>
>>> +#include <linux/spi/spi.h>
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/iio/buffer.h>
>>> +#include <linux/iio/trigger.h>
>>> +#include <linux/iio/triggered_buffer.h>
>>> +#include <linux/iio/trigger_consumer.h>
>>> +#include <linux/regulator/consumer.h>
>>> +
>>> +#define ADC12138_MODE_AUTO_CAL                       0x08
>>> +#define ADC12138_MODE_READ_STATUS            0x0c
>>> +#define ADC12138_MODE_ACQUISITION_TIME_6     0x0e
>>> +#define ADC12138_MODE_ACQUISITION_TIME_10    0x4e
>>> +#define ADC12138_MODE_ACQUISITION_TIME_18    0x8e
>>> +#define ADC12138_MODE_ACQUISITION_TIME_34    0xce
>>> +
>>> +#define ADC12138_STATUS_CAL                  BIT(6)
>>> +
>>> +enum {
>>> +     adc12130,
>>> +     adc12132,
>>> +     adc12138,
>>> +};
>>> +
>>> +struct adc12138 {
>>> +     struct spi_device *spi;
>>> +     unsigned int id;
>>> +     /* conversion clock */
>>> +     struct clk *cclk;
>>> +     /* positive analog voltage reference */
>>> +     struct regulator *vref_p;
>>> +     /* negative analog voltage reference */
>>> +     struct regulator *vref_n;
>>> +     struct mutex lock;
>>> +     struct completion complete;
>>> +     unsigned int acquisition_time;
>>
>> unit? ms? µs?
> 
> This is specified by the number of conversion clock (cclk) periods.
> I'll add a comment about it.
> 
>>> +
>>> +     u8 tx_buf[2] ____cacheline_aligned;
>>> +     u8 rx_buf[2];
>>> +};
>>> +
>>> +#define ADC12138_VOLTAGE_CHANNEL(chan)                                       \
>>> +     {                                                               \
>>> +             .type = IIO_VOLTAGE,                                    \
>>> +             .indexed = 1,                                           \
>>> +             .channel = chan,                                        \
>>> +             .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),           \
>>> +             .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE)    \
>>> +                                     | BIT(IIO_CHAN_INFO_OFFSET),    \
>>> +             .scan_index = chan,                                     \
>>> +             .scan_type = {                                          \
>>> +                     .sign = 's',                                    \
>>> +                     .realbits = 13,                                 \
>>> +                     .storagebits = 16,                              \
>>> +                     .shift = 3,                                     \
>>> +                     .endianness = IIO_BE,                           \
>>> +             },                                                      \
>>> +     }
>>> +
>>> +#define ADC12138_VOLTAGE_CHANNEL_DIFF(chan1, chan2, si_offset)               \
>>> +     {                                                               \
>>> +             .type = IIO_VOLTAGE,                                    \
>>> +             .indexed = 1,                                           \
>>> +             .channel = (chan1),                                     \
>>> +             .channel2 = (chan2),                                    \
>>> +             .differential = 1,                                      \
>>> +             .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),           \
>>> +             .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE)    \
>>> +                                     | BIT(IIO_CHAN_INFO_OFFSET),    \
>>> +             .scan_index = (chan1) + (si_offset),                    \
>>> +             .scan_type = {                                          \
>>> +                     .sign = 's',                                    \
>>> +                     .realbits = 13,                                 \
>>> +                     .storagebits = 16,                              \
>>> +                     .shift = 3,                                     \
>>> +                     .endianness = IIO_BE,                           \
>>> +             },                                                      \
>>> +     }
>>> +
>>> +static const struct iio_chan_spec adc12132_channels[] = {
>>> +     ADC12138_VOLTAGE_CHANNEL(0),
>>> +     ADC12138_VOLTAGE_CHANNEL(1),
>>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(0, 1, 2),
>>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(1, 0, 2),
>>> +     IIO_CHAN_SOFT_TIMESTAMP(4),
>>> +};
>>> +
>>> +static const struct iio_chan_spec adc12138_channels[] = {
>>> +     ADC12138_VOLTAGE_CHANNEL(0),
>>> +     ADC12138_VOLTAGE_CHANNEL(1),
>>> +     ADC12138_VOLTAGE_CHANNEL(2),
>>> +     ADC12138_VOLTAGE_CHANNEL(3),
>>> +     ADC12138_VOLTAGE_CHANNEL(4),
>>> +     ADC12138_VOLTAGE_CHANNEL(5),
>>> +     ADC12138_VOLTAGE_CHANNEL(6),
>>> +     ADC12138_VOLTAGE_CHANNEL(7),
>>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(0, 1, 8),
>>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(1, 0, 8),
>>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(2, 3, 8),
>>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(3, 2, 8),
>>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(4, 5, 8),
>>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(5, 4, 8),
>>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(6, 7, 8),
>>> +     ADC12138_VOLTAGE_CHANNEL_DIFF(7, 6, 8),
>>> +     IIO_CHAN_SOFT_TIMESTAMP(16),
>>> +};
>>> +
>>> +static int adc12138_mode_programming(struct adc12138 *adc, u8 mode,
>>> +                                  void *rx_buf, int len)
>>> +{
>>> +     struct spi_transfer xfer = {
>>> +             .tx_buf = adc->tx_buf,
>>> +             .rx_buf = adc->rx_buf,
>>> +             .len = len,
>>> +     };
>>> +     int ret;
>>> +
>>> +     /* Skip disused bits for ADC12130 and ADC12132 */
>>
>> probably 'unused' instead of 'disused'
> 
> OK.
> 
>>> +     if (adc->id != adc12138)
>>> +             mode = (mode & 0xc0) | ((mode & 0x0f) << 2);
>>> +
>>> +     adc->tx_buf[0] = mode;
>>> +
>>> +     ret = spi_sync_transfer(adc->spi, &xfer, 1);
>>> +     if (ret)
>>> +             return ret;
>>> +
>>> +     memcpy(rx_buf, adc->rx_buf, len);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int adc12138_read_status(struct adc12138 *adc)
>>> +{
>>> +     u8 rx_buf[2];
>>> +     int ret;
>>> +
>>> +     ret = adc12138_mode_programming(adc, ADC12138_MODE_READ_STATUS,
>>> +                                     rx_buf, 2);
>>> +     if (ret)
>>> +             return ret;
>>> +
>>> +     return (rx_buf[0] << 1) | (rx_buf[1] >> 7);
>>> +}
>>> +
>>> +static int __adc12138_start_conv(struct adc12138 *adc,
>>> +                              struct iio_chan_spec const *channel,
>>> +                              void *data, int len)
>>> +
>>> +{
>>> +     const u8 ch_to_mux[] = { 0, 4, 1, 5, 2, 6, 3, 7 };
>>> +     u8 mode = (ch_to_mux[channel->channel] << 4) |
>>> +               (channel->differential ? 0 : 0x80);
>>> +
>>> +     return adc12138_mode_programming(adc, mode, data, len);
>>> +}
>>> +
>>> +static int adc12138_start_conv(struct adc12138 *adc,
>>> +                            struct iio_chan_spec const *channel)
>>> +{
>>> +     u8 trash;
>>> +
>>> +     return __adc12138_start_conv(adc, channel, &trash, 1);
>>> +}
>>> +
>>> +static int adc12138_start_and_read_conv(struct adc12138 *adc,
>>> +                                     struct iio_chan_spec const *channel,
>>> +                                     __be16 *data)
>>> +{
>>> +     return __adc12138_start_conv(adc, channel, data, 2);
>>> +}
>>> +
>>> +static int adc12138_read_conv_data(struct adc12138 *adc, __be16 *value)
>>> +{
>>> +     /* Issue a read status instruction and read previous conversion data */
>>> +     return adc12138_mode_programming(adc, ADC12138_MODE_READ_STATUS,
>>> +                                      value, sizeof(*value));
>>> +}
>>> +
>>> +static int adc12138_wait_eoc(struct adc12138 *adc, unsigned long timeout)
>>> +{
>>> +     if (!wait_for_completion_timeout(&adc->complete, timeout))
>>> +             return -ETIMEDOUT;
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int adc12138_adc_conversion(struct adc12138 *adc,
>>> +                                struct iio_chan_spec const *channel,
>>> +                                __be16 *value)
>>> +{
>>> +     int ret;
>>> +
>>> +     reinit_completion(&adc->complete);
>>> +
>>> +     ret = adc12138_start_conv(adc, channel);
>>> +     if (ret)
>>> +             return ret;
>>> +
>>> +     ret = adc12138_wait_eoc(adc, msecs_to_jiffies(100));
>>> +     if (ret)
>>> +             return ret;
>>> +
>>> +     return adc12138_read_conv_data(adc, value);
>>> +}
>>> +
>>> +static int adc12138_read_raw(struct iio_dev *iio,
>>> +                          struct iio_chan_spec const *channel, int *value,
>>> +                          int *shift, long mask)
>>> +{
>>> +     struct adc12138 *adc = iio_priv(iio);
>>> +     int ret;
>>> +     __be16 data;
>>> +
>>> +     switch (mask) {
>>> +     case IIO_CHAN_INFO_RAW:
>>
>> what if buffered reads have been set up? probably use
>> iio_device_claim_direct_mode()
> 
> Maybe it is safe because the ADC data is always requested and retrieved
> while holding adc12138.lock from adc12138_read_raw() and
> adc12138_trigger_handler().  Is it better to restrict direct mode and
> return -EBUSY while buffer mode is enabled?
> 
> (BTW, I realized that I can use iio_dev->mlock instead of adc12138.lock)
Don't use it for anything other than state change locking.  It has that
well focused task within the subsystem - we are trying to clear out other
uses entirely as there have been more than a few bugs around this.

Much better to have a separate lock for protecting resources controlled
directly by the driver itself.
> 
>>> +             mutex_lock(&adc->lock);
>>> +             ret = adc12138_adc_conversion(adc, channel, &data);
>>> +             mutex_unlock(&adc->lock);
>>> +             if (ret)
>>> +                     return ret;
>>> +
>>> +             *value = sign_extend32(be16_to_cpu(data) >> 3, 12);
>>> +
>>> +             return IIO_VAL_INT;
>>> +     case IIO_CHAN_INFO_SCALE:
>>> +             ret = regulator_get_voltage(adc->vref_p);
>>> +             if (ret < 0)
>>> +                     return ret;
>>> +             *value = ret;
>>> +
>>> +             if (!IS_ERR(adc->vref_n)) {
>>> +                     ret = regulator_get_voltage(adc->vref_n);
>>> +                     if (ret < 0)
>>> +                             return ret;
>>> +                     *value -= ret;
>>> +             }
>>> +
>>> +             /* convert regulator output voltage to mV */
>>> +             *value /= 1000;
>>> +             *shift = channel->scan_type.realbits - 1;
>>> +
>>> +             return IIO_VAL_FRACTIONAL_LOG2;
>>> +     case IIO_CHAN_INFO_OFFSET:
>>> +             if (!IS_ERR(adc->vref_n)) {
>>> +                     *value = regulator_get_voltage(adc->vref_n);
>>> +                     if (*value < 0)
>>> +                             return *value;
>>> +             } else {
>>> +                     *value = 0;
>>> +             }
>>> +
>>> +             /* convert regulator output voltage to mV */
>>> +             *value /= 1000;
>>> +
>>> +             return IIO_VAL_INT;
>>> +     }
>>> +
>>> +     return -EINVAL;
>>> +}
>>> +
>>> +static const struct iio_info adc12138_info = {
>>> +     .read_raw = adc12138_read_raw,
>>> +     .driver_module = THIS_MODULE,
>>> +};
>>> +
>>> +static int adc12138_init(struct adc12138 *adc)
>>> +{
>>> +     int ret;
>>> +     int status;
>>> +     u8 mode;
>>> +     u8 trash;
>>> +
>>> +     reinit_completion(&adc->complete);
>>> +
>>> +     ret = adc12138_mode_programming(adc, ADC12138_MODE_AUTO_CAL, &trash, 1);
>>> +     if (ret)
>>> +             return ret;
>>> +
>>> +     /* data output at this time has no significance */
>>> +     status = adc12138_read_status(adc);
>>> +     if (status < 0)
>>> +             return status;
>>> +
>>> +     adc12138_wait_eoc(adc, msecs_to_jiffies(100));
>>> +
>>> +     status = adc12138_read_status(adc);
>>> +     if (status & ADC12138_STATUS_CAL) {
>>> +             dev_warn(&adc->spi->dev,
>>> +                     "Auto Cal sequence is still in progress: %#x\n",
>>> +                     status);
>>> +             return -EIO;
>>> +     }
>>> +
>>> +     switch (adc->acquisition_time) {
>>> +     case 6:
>>> +             mode = ADC12138_MODE_ACQUISITION_TIME_6;
>>> +             break;
>>> +     case 10:
>>> +             mode = ADC12138_MODE_ACQUISITION_TIME_10;
>>> +             break;
>>> +     case 18:
>>> +             mode = ADC12138_MODE_ACQUISITION_TIME_18;
>>> +             break;
>>> +     case 34:
>>> +             mode = ADC12138_MODE_ACQUISITION_TIME_34;
>>> +             break;
>>> +     default:
>>> +             return -EINVAL;
>>> +     }
>>> +
>>> +     return adc12138_mode_programming(adc, mode, &trash, 1);
>>> +}
>>> +
>>> +static irqreturn_t adc12138_trigger_handler(int irq, void *p)
>>> +{
>>> +     struct iio_poll_func *pf = p;
>>> +     struct iio_dev *indio_dev = pf->indio_dev;
>>> +     struct adc12138 *adc = iio_priv(indio_dev);
>>> +     __be16 data[20] = { };
>>
>> why { } here?
> 
> To initialize with zero and avoids exposing kernel stack to userspace
> through padding entries.  I could use memset here.
> 
>> why 20?
> 
> When all scan elements are enabled, the sample data requires 40 bytes
> (2 bytes ADC data * 16 channels + 8 bytes timestamp).
> 
> I'll add comments about these.
> 
>>> +     __be16 trash;
>>> +     int ret;
>>> +     int scan_index;
>>> +     int i = 0;
>>> +
>>> +     mutex_lock(&adc->lock);
>>> +
>>> +     for_each_set_bit(scan_index, indio_dev->active_scan_mask,
>>> +                      indio_dev->masklength) {
>>> +             const struct iio_chan_spec *scan_chan =
>>> +                             &indio_dev->channels[scan_index];
>>> +
>>> +             reinit_completion(&adc->complete);
>>> +
>>> +             ret = adc12138_start_and_read_conv(adc, scan_chan,
>>> +                                                i ? &data[i - 1] : &trash);
>>> +             if (ret) {
>>> +                     dev_warn(&adc->spi->dev,
>>> +                              "failed to start conversion\n");
>>> +                     goto out;
>>> +             }
>>> +
>>> +             ret = adc12138_wait_eoc(adc, msecs_to_jiffies(100));
>>> +             if (ret) {
>>> +                     dev_warn(&adc->spi->dev, "wait eoc timeout\n");
>>> +                     goto out;
>>> +             }
>>> +
>>> +             i++;
>>> +     }
>>> +
>>> +     if (i) {
>>> +             ret = adc12138_read_conv_data(adc, &data[i - 1]);
>>> +             if (ret) {
>>> +                     dev_warn(&adc->spi->dev,
>>> +                              "failed to get conversion data\n");
>>> +                     goto out;
>>> +             }
>>> +     }
>>> +
>>> +     iio_push_to_buffers_with_timestamp(indio_dev, data, iio_get_time_ns());
>>> +out:
>>> +     mutex_unlock(&adc->lock);
>>> +
>>> +     iio_trigger_notify_done(indio_dev->trig);
>>> +
>>> +     return IRQ_HANDLED;
>>> +}
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-07-20 15:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-18 15:00 [PATCH] iio: adc: add ADC12130/ADC12132/ADC12138 ADC driver Akinobu Mita
2016-07-18 15:00 ` Akinobu Mita
     [not found] ` <1468854029-22153-1-git-send-email-akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-07-18 22:02   ` Peter Meerwald-Stadler
2016-07-18 22:02     ` Peter Meerwald-Stadler
     [not found]     ` <alpine.DEB.2.02.1607182348340.25263-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>
2016-07-19 16:24       ` Akinobu Mita
2016-07-19 16:24         ` Akinobu Mita
     [not found]         ` <CAC5umyjV0cUG3ovfqDy8nyDZYz_yYTwxnp7ti1T_xvz=FjWSNQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-07-20 15:05           ` Jonathan Cameron
2016-07-20 15:05             ` Jonathan Cameron
2016-07-20  1:11   ` Rob Herring
2016-07-20  1:11     ` Rob Herring

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.