* [PATCH v3 0/2] add driver for the ti-adc084s021 chip
@ 2017-05-03 12:01 ` Mårten Lindahl
0 siblings, 0 replies; 14+ messages in thread
From: Mårten Lindahl @ 2017-05-03 12:01 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A
Cc: knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
pmeerw-jW+XmwGofnusTnJN9+BGXg, linux-iio-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
devicetree-u79uwXL29TY76Z2rM5mHXA, Mårten Lindahl
From: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
This adds support for the Texas Instruments ADC084S021 ADC chip.
---
Changes in v3:
- Removed unnecessary comment about channel specification
- Skipped usage of 'address' in iio_chan_spec config macro
- Mask and shift channel readings only for _read_raw function
- Enable/disable regulator in _read_raw function
- Improved setup of ADC channel readings
- Use SPI config of speed_hz and bits_per_word
- Use devm_iio_triggered_buffer_setup and devm_iio_device_register
- Removed error message for failed devm_iio_device_register
- Removed driver _remove callback function
Changes in v2:
- Split dt-bindings and iio/adc into separate patches
- Updated dt-bindings after Robs comments
- Removed most #defines to inlines
- Corrected channel macro
- Removed configuration array with only one item
- Updated func adc084s021_adc_conversion to use be16_to_cpu
- Added IIO_CHAN_INFO_SCALE to func adc084s021_read_raw
- Use iio_device_claim_direct_mode in func adc084s021_read_raw
- Removed documentation for standard driver functions
- Changed retval to ret everywhere
- Removed dynamic alloc for data buffer in trigger handler
- Keeping mutex for all iterations in trigger handler
- Removed usage of events in this driver
- Removed info log in probe
- Use spi_message_init_with_transfers for spi message structs
- Use preenable and postdisable functions for regulator
- Inserted blank line before last return in all functions
Mårten Lindahl (2):
dt-bindings: iio: adc: add driver for the ti-adc084s021 chip
iio: adc: add driver for the ti-adc084s021 chip
.../devicetree/bindings/iio/adc/ti-adc084s021.txt | 19 ++
drivers/iio/adc/Kconfig | 12 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ti-adc084s021.c | 302 +++++++++++++++++++++
4 files changed, 334 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
create mode 100644 drivers/iio/adc/ti-adc084s021.c
--
2.1.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 0/2] add driver for the ti-adc084s021 chip
@ 2017-05-03 12:01 ` Mårten Lindahl
0 siblings, 0 replies; 14+ messages in thread
From: Mårten Lindahl @ 2017-05-03 12:01 UTC (permalink / raw)
To: jic23
Cc: knaack.h, lars, pmeerw, linux-iio, robh+dt, mark.rutland,
devicetree, Mårten Lindahl
From: Mårten Lindahl <martenli@axis.com>
This adds support for the Texas Instruments ADC084S021 ADC chip.
---
Changes in v3:
- Removed unnecessary comment about channel specification
- Skipped usage of 'address' in iio_chan_spec config macro
- Mask and shift channel readings only for _read_raw function
- Enable/disable regulator in _read_raw function
- Improved setup of ADC channel readings
- Use SPI config of speed_hz and bits_per_word
- Use devm_iio_triggered_buffer_setup and devm_iio_device_register
- Removed error message for failed devm_iio_device_register
- Removed driver _remove callback function
Changes in v2:
- Split dt-bindings and iio/adc into separate patches
- Updated dt-bindings after Robs comments
- Removed most #defines to inlines
- Corrected channel macro
- Removed configuration array with only one item
- Updated func adc084s021_adc_conversion to use be16_to_cpu
- Added IIO_CHAN_INFO_SCALE to func adc084s021_read_raw
- Use iio_device_claim_direct_mode in func adc084s021_read_raw
- Removed documentation for standard driver functions
- Changed retval to ret everywhere
- Removed dynamic alloc for data buffer in trigger handler
- Keeping mutex for all iterations in trigger handler
- Removed usage of events in this driver
- Removed info log in probe
- Use spi_message_init_with_transfers for spi message structs
- Use preenable and postdisable functions for regulator
- Inserted blank line before last return in all functions
Mårten Lindahl (2):
dt-bindings: iio: adc: add driver for the ti-adc084s021 chip
iio: adc: add driver for the ti-adc084s021 chip
.../devicetree/bindings/iio/adc/ti-adc084s021.txt | 19 ++
drivers/iio/adc/Kconfig | 12 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ti-adc084s021.c | 302 +++++++++++++++++++++
4 files changed, 334 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
create mode 100644 drivers/iio/adc/ti-adc084s021.c
--
2.1.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 1/2] dt-bindings: iio: adc: add driver for the ti-adc084s021 chip
2017-05-03 12:01 ` Mårten Lindahl
@ 2017-05-03 12:01 ` Mårten Lindahl
-1 siblings, 0 replies; 14+ messages in thread
From: Mårten Lindahl @ 2017-05-03 12:01 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A
Cc: knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
pmeerw-jW+XmwGofnusTnJN9+BGXg, linux-iio-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
devicetree-u79uwXL29TY76Z2rM5mHXA, Mårten Lindahl
From: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
This adds support for the Texas Instruments ADC084S021 ADC chip.
Signed-off-by: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
---
Changes in v3:
- No updates since v2
Changes in v2:
- Updated the 'Required properties' section
- Removed the 'Optional properties' section
.../devicetree/bindings/iio/adc/ti-adc084s021.txt | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
diff --git a/Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt b/Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
new file mode 100644
index 0000000..4259e50
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
@@ -0,0 +1,19 @@
+* Texas Instruments' ADC084S021
+
+Required properties:
+ - compatible : Must be "ti,adc084s021"
+ - reg : SPI chip select number for the device
+ - vref-supply : The regulator supply for ADC reference voltage
+ - spi-cpol : Per spi-bus bindings
+ - spi-cpha : Per spi-bus bindings
+ - spi-max-frequency : Per spi-bus bindings
+
+Example:
+adc@0 {
+ compatible = "ti,adc084s021";
+ reg = <0>;
+ vref-supply = <&adc_vref>;
+ spi-cpol;
+ spi-cpha;
+ spi-max-frequency = <16000000>;
+};
--
2.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 1/2] dt-bindings: iio: adc: add driver for the ti-adc084s021 chip
@ 2017-05-03 12:01 ` Mårten Lindahl
0 siblings, 0 replies; 14+ messages in thread
From: Mårten Lindahl @ 2017-05-03 12:01 UTC (permalink / raw)
To: jic23
Cc: knaack.h, lars, pmeerw, linux-iio, robh+dt, mark.rutland,
devicetree, Mårten Lindahl
From: Mårten Lindahl <martenli@axis.com>
This adds support for the Texas Instruments ADC084S021 ADC chip.
Signed-off-by: Mårten Lindahl <martenli@axis.com>
---
Changes in v3:
- No updates since v2
Changes in v2:
- Updated the 'Required properties' section
- Removed the 'Optional properties' section
.../devicetree/bindings/iio/adc/ti-adc084s021.txt | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
diff --git a/Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt b/Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
new file mode 100644
index 0000000..4259e50
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
@@ -0,0 +1,19 @@
+* Texas Instruments' ADC084S021
+
+Required properties:
+ - compatible : Must be "ti,adc084s021"
+ - reg : SPI chip select number for the device
+ - vref-supply : The regulator supply for ADC reference voltage
+ - spi-cpol : Per spi-bus bindings
+ - spi-cpha : Per spi-bus bindings
+ - spi-max-frequency : Per spi-bus bindings
+
+Example:
+adc@0 {
+ compatible = "ti,adc084s021";
+ reg = <0>;
+ vref-supply = <&adc_vref>;
+ spi-cpol;
+ spi-cpha;
+ spi-max-frequency = <16000000>;
+};
--
2.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 2/2] iio: adc: add driver for the ti-adc084s021 chip
2017-05-03 12:01 ` Mårten Lindahl
@ 2017-05-03 12:01 ` Mårten Lindahl
-1 siblings, 0 replies; 14+ messages in thread
From: Mårten Lindahl @ 2017-05-03 12:01 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A
Cc: knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
pmeerw-jW+XmwGofnusTnJN9+BGXg, linux-iio-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
devicetree-u79uwXL29TY76Z2rM5mHXA, Mårten Lindahl
From: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
This adds support for the Texas Instruments ADC084S021 ADC chip.
Signed-off-by: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
---
Changes in v3:
- Removed unnecessary comment about channel specification
- Skipped usage of 'address' in iio_chan_spec config macro
- Mask and shift channel readings only for _read_raw function
- Enable/disable regulator in _read_raw function
- Improved setup of ADC channel readings
- Use SPI config of speed_hz and bits_per_word
- Use devm_iio_triggered_buffer_setup and devm_iio_device_register
- Removed error message for failed devm_iio_device_register
- Removed driver _remove callback function
Changes in v2:
- Removed most #defines in favor of inlines
- Corrected channel macro
- Removed configuration array with only one item
- Updated func adc084s021_adc_conversion to use be16_to_cpu
- Added IIO_CHAN_INFO_SCALE to func adc084s021_read_raw
- Use iio_device_claim_direct_mode in func adc084s021_read_raw
- Removed documentation for standard driver functions
- Changed retval to ret everywhere
- Removed dynamic alloc for data buffer in trigger handler
- Keeping mutex for all iterations in trigger handler
- Removed usage of events in this driver
- Removed info log in probe
- Use spi_message_init_with_transfers for spi message structs
- Use preenable and postdisable functions for regulator
- Inserted blank line before last return in all functions
drivers/iio/adc/Kconfig | 12 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ti-adc084s021.c | 302 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 315 insertions(+)
create mode 100644 drivers/iio/adc/ti-adc084s021.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index dedae7a..13141e5 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -560,6 +560,18 @@ config TI_ADC0832
This driver can also be built as a module. If so, the module will be
called ti-adc0832.
+config TI_ADC084S021
+ tristate "Texas Instruments ADC084S021"
+ depends on SPI
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
+ help
+ If you say yes here you get support for Texas Instruments ADC084S021
+ chips.
+
+ This driver can also be built as a module. If so, the module will be
+ called ti-adc084s021.
+
config TI_ADC12138
tristate "Texas Instruments ADC12130/ADC12132/ADC12138"
depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index d001262..b1a6158 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_STM32_ADC_CORE) += stm32-adc-core.o
obj-$(CONFIG_STM32_ADC) += stm32-adc.o
obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
+obj-$(CONFIG_TI_ADC084S021) += ti-adc084s021.o
obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o
diff --git a/drivers/iio/adc/ti-adc084s021.c b/drivers/iio/adc/ti-adc084s021.c
new file mode 100644
index 0000000..f2fb0fa
--- /dev/null
+++ b/drivers/iio/adc/ti-adc084s021.c
@@ -0,0 +1,302 @@
+/**
+ * Copyright (C) 2017 Axis Communications AB
+ *
+ * Driver for Texas Instruments' ADC084S021 ADC chip.
+ * Datasheets can be found here:
+ * http://www.ti.com/lit/ds/symlink/adc084s021.pdf
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/err.h>
+#include <linux/spi/spi.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/regulator/consumer.h>
+
+#define ADC084S021_DRIVER_NAME "adc084s021"
+
+struct adc084s021 {
+ struct spi_device *spi;
+ struct spi_message message;
+ struct spi_transfer spi_trans;
+ struct regulator *reg;
+ struct mutex lock;
+ /*
+ * DMA (thus cache coherency maintenance) requires the
+ * transfer buffers to live in their own cache lines.
+ */
+ u16 tx_buf[4] ____cacheline_aligned;
+ __be16 rx_buf[5] ____cacheline_aligned; /* First 16-bits are trash */
+};
+
+#define ADC084S021_VOLTAGE_CHANNEL(num) \
+ { \
+ .type = IIO_VOLTAGE, \
+ .channel = (num), \
+ .indexed = 1, \
+ .scan_index = (num), \
+ .scan_type = { \
+ .sign = 'u', \
+ .realbits = 8, \
+ .storagebits = 16, \
+ .shift = 4, \
+ .endianness = IIO_BE, \
+ }, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
+ }
+
+static const struct iio_chan_spec adc084s021_channels[] = {
+ ADC084S021_VOLTAGE_CHANNEL(0),
+ ADC084S021_VOLTAGE_CHANNEL(1),
+ ADC084S021_VOLTAGE_CHANNEL(2),
+ ADC084S021_VOLTAGE_CHANNEL(3),
+ IIO_CHAN_SOFT_TIMESTAMP(4),
+};
+
+static int adc084s021_power_on(struct adc084s021 *adc)
+{
+ int ret;
+
+ ret = regulator_enable(adc->reg);
+ if (ret) {
+ dev_warn(&adc->spi->dev,
+ "Failed to enable supply voltage\n");
+ }
+
+ return ret;
+}
+
+static int adc084s021_power_off(struct adc084s021 *adc)
+{
+ int ret;
+
+ ret = regulator_disable(adc->reg);
+ if (ret) {
+ dev_warn(&adc->spi->dev,
+ "Failed to disable supply voltage\n");
+ }
+
+ return ret;
+}
+
+/**
+ * Read an ADC channel and return its value.
+ *
+ * @adc: The ADC SPI data.
+ * @data: Buffer for converted data.
+ */
+static int adc084s021_adc_conversion(struct adc084s021 *adc, void *data)
+{
+ int n_words = (adc->spi_trans.len >> 1) - 1; /* Discard first word */
+ int ret, i = 0;
+ u16 *p = data;
+
+ /* Do the transfer */
+ ret = spi_sync(adc->spi, &adc->message);
+ if (ret < 0)
+ return ret;
+
+ for (; i < n_words; i++)
+ *(p + i) = be16_to_cpu(adc->rx_buf[i + 1]);
+
+ return ret;
+}
+
+static int adc084s021_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *channel, int *val,
+ int *val2, long mask)
+{
+ struct adc084s021 *adc = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret < 0)
+ return ret;
+
+ ret = adc084s021_power_on(adc);
+ if (ret) {
+ iio_device_release_direct_mode(indio_dev);
+ return ret;
+ }
+
+ adc->tx_buf[0] = channel->channel << 3;
+ ret = adc084s021_adc_conversion(adc, val);
+ iio_device_release_direct_mode(indio_dev);
+ adc084s021_power_off(adc);
+ if (ret < 0)
+ return ret;
+
+ *val = (*val >> channel->scan_type.shift) & 0xff;
+
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ ret = adc084s021_power_on(adc);
+ if (ret)
+ return ret;
+
+ ret = regulator_get_voltage(adc->reg);
+ adc084s021_power_off(adc);
+ if (ret < 0)
+ return ret;
+
+ *val = ret / 1000;
+
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+}
+
+/**
+ * Read enabled ADC channels and push data to the buffer.
+ *
+ * @irq: The interrupt number (not used).
+ * @pollfunc: Pointer to the poll func.
+ */
+static irqreturn_t adc084s021_buffer_trigger_handler(int irq, void *pollfunc)
+{
+ struct iio_poll_func *pf = pollfunc;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct adc084s021 *adc = iio_priv(indio_dev);
+ __be16 data[8] = {0}; /* 4 * 16-bit words of data + 8 bytes timestamp */
+
+ mutex_lock(&adc->lock);
+
+ if (adc084s021_adc_conversion(adc, &data) < 0)
+ dev_err(&adc->spi->dev, "Failed to read data\n");
+
+ iio_push_to_buffers_with_timestamp(indio_dev, data,
+ iio_get_time_ns(indio_dev));
+ mutex_unlock(&adc->lock);
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
+static int adc084s021_buffer_preenable(struct iio_dev *indio_dev)
+{
+ struct adc084s021 *adc = iio_priv(indio_dev);
+ int scan_index;
+ int i = 0;
+
+ for_each_set_bit(scan_index, indio_dev->active_scan_mask,
+ indio_dev->masklength) {
+ const struct iio_chan_spec *channel =
+ &indio_dev->channels[scan_index];
+ adc->tx_buf[i++] = channel->channel << 3;
+ }
+ adc->spi_trans.len = 2 + (i * sizeof(__be16)); /* Trash + channels */
+
+ return adc084s021_power_on(adc);
+}
+
+static int adc084s021_buffer_postdisable(struct iio_dev *indio_dev)
+{
+ struct adc084s021 *adc = iio_priv(indio_dev);
+
+ adc->spi_trans.len = 4; /* Trash + single channel */
+
+ return adc084s021_power_off(adc);
+}
+
+static const struct iio_info adc084s021_info = {
+ .read_raw = adc084s021_read_raw,
+ .driver_module = THIS_MODULE,
+};
+
+static const struct iio_buffer_setup_ops adc084s021_buffer_setup_ops = {
+ .preenable = adc084s021_buffer_preenable,
+ .postenable = iio_triggered_buffer_postenable,
+ .predisable = iio_triggered_buffer_predisable,
+ .postdisable = adc084s021_buffer_postdisable,
+};
+
+static int adc084s021_probe(struct spi_device *spi)
+{
+ struct iio_dev *indio_dev;
+ struct adc084s021 *adc;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
+ if (!indio_dev) {
+ dev_err(&spi->dev, "Failed to allocate IIO device\n");
+ return -ENOMEM;
+ }
+
+ adc = iio_priv(indio_dev);
+ adc->spi = spi;
+
+ /* Connect the SPI device and the iio dev */
+ spi_set_drvdata(spi, indio_dev);
+
+ /* Initiate the Industrial I/O device */
+ indio_dev->dev.parent = &spi->dev;
+ indio_dev->dev.of_node = spi->dev.of_node;
+ indio_dev->name = spi_get_device_id(spi)->name;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->info = &adc084s021_info;
+ indio_dev->channels = adc084s021_channels;
+ indio_dev->num_channels = ARRAY_SIZE(adc084s021_channels);
+
+ /* Create SPI transfer for channel reads */
+ adc->spi_trans.tx_buf = adc->tx_buf;
+ adc->spi_trans.rx_buf = adc->rx_buf;
+ adc->spi_trans.len = 4; /* Trash + single channel */
+ spi_message_init_with_transfers(&adc->message, &adc->spi_trans, 1);
+
+ adc->reg = devm_regulator_get(&spi->dev, "vref");
+ if (IS_ERR(adc->reg))
+ return PTR_ERR(adc->reg);
+
+ mutex_init(&adc->lock);
+
+ /* Setup triggered buffer with pollfunction */
+ ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
+ adc084s021_buffer_trigger_handler,
+ &adc084s021_buffer_setup_ops);
+ if (ret) {
+ dev_err(&spi->dev, "Failed to setup triggered buffer\n");
+ return ret;
+ }
+
+ ret = devm_iio_device_register(&spi->dev, indio_dev);
+
+ return ret;
+}
+
+static const struct of_device_id adc084s021_of_match[] = {
+ { .compatible = "ti,adc084s021", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, adc084s021_of_match);
+
+static const struct spi_device_id adc084s021_id[] = {
+ { ADC084S021_DRIVER_NAME, 0},
+ {}
+};
+MODULE_DEVICE_TABLE(spi, adc084s021_id);
+
+static struct spi_driver adc084s021_driver = {
+ .driver = {
+ .name = ADC084S021_DRIVER_NAME,
+ .of_match_table = of_match_ptr(adc084s021_of_match),
+ },
+ .probe = adc084s021_probe,
+ .id_table = adc084s021_id,
+};
+module_spi_driver(adc084s021_driver);
+
+MODULE_AUTHOR("Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>");
+MODULE_DESCRIPTION("Texas Instruments ADC084S021");
+MODULE_LICENSE("GPL v2");
+MODULE_VERSION("1.0");
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 related [flat|nested] 14+ messages in thread
* [PATCH v3 2/2] iio: adc: add driver for the ti-adc084s021 chip
@ 2017-05-03 12:01 ` Mårten Lindahl
0 siblings, 0 replies; 14+ messages in thread
From: Mårten Lindahl @ 2017-05-03 12:01 UTC (permalink / raw)
To: jic23
Cc: knaack.h, lars, pmeerw, linux-iio, robh+dt, mark.rutland,
devicetree, Mårten Lindahl
From: Mårten Lindahl <martenli@axis.com>
This adds support for the Texas Instruments ADC084S021 ADC chip.
Signed-off-by: Mårten Lindahl <martenli@axis.com>
---
Changes in v3:
- Removed unnecessary comment about channel specification
- Skipped usage of 'address' in iio_chan_spec config macro
- Mask and shift channel readings only for _read_raw function
- Enable/disable regulator in _read_raw function
- Improved setup of ADC channel readings
- Use SPI config of speed_hz and bits_per_word
- Use devm_iio_triggered_buffer_setup and devm_iio_device_register
- Removed error message for failed devm_iio_device_register
- Removed driver _remove callback function
Changes in v2:
- Removed most #defines in favor of inlines
- Corrected channel macro
- Removed configuration array with only one item
- Updated func adc084s021_adc_conversion to use be16_to_cpu
- Added IIO_CHAN_INFO_SCALE to func adc084s021_read_raw
- Use iio_device_claim_direct_mode in func adc084s021_read_raw
- Removed documentation for standard driver functions
- Changed retval to ret everywhere
- Removed dynamic alloc for data buffer in trigger handler
- Keeping mutex for all iterations in trigger handler
- Removed usage of events in this driver
- Removed info log in probe
- Use spi_message_init_with_transfers for spi message structs
- Use preenable and postdisable functions for regulator
- Inserted blank line before last return in all functions
drivers/iio/adc/Kconfig | 12 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ti-adc084s021.c | 302 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 315 insertions(+)
create mode 100644 drivers/iio/adc/ti-adc084s021.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index dedae7a..13141e5 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -560,6 +560,18 @@ config TI_ADC0832
This driver can also be built as a module. If so, the module will be
called ti-adc0832.
+config TI_ADC084S021
+ tristate "Texas Instruments ADC084S021"
+ depends on SPI
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
+ help
+ If you say yes here you get support for Texas Instruments ADC084S021
+ chips.
+
+ This driver can also be built as a module. If so, the module will be
+ called ti-adc084s021.
+
config TI_ADC12138
tristate "Texas Instruments ADC12130/ADC12132/ADC12138"
depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index d001262..b1a6158 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_STM32_ADC_CORE) += stm32-adc-core.o
obj-$(CONFIG_STM32_ADC) += stm32-adc.o
obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
+obj-$(CONFIG_TI_ADC084S021) += ti-adc084s021.o
obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o
diff --git a/drivers/iio/adc/ti-adc084s021.c b/drivers/iio/adc/ti-adc084s021.c
new file mode 100644
index 0000000..f2fb0fa
--- /dev/null
+++ b/drivers/iio/adc/ti-adc084s021.c
@@ -0,0 +1,302 @@
+/**
+ * Copyright (C) 2017 Axis Communications AB
+ *
+ * Driver for Texas Instruments' ADC084S021 ADC chip.
+ * Datasheets can be found here:
+ * http://www.ti.com/lit/ds/symlink/adc084s021.pdf
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/err.h>
+#include <linux/spi/spi.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/regulator/consumer.h>
+
+#define ADC084S021_DRIVER_NAME "adc084s021"
+
+struct adc084s021 {
+ struct spi_device *spi;
+ struct spi_message message;
+ struct spi_transfer spi_trans;
+ struct regulator *reg;
+ struct mutex lock;
+ /*
+ * DMA (thus cache coherency maintenance) requires the
+ * transfer buffers to live in their own cache lines.
+ */
+ u16 tx_buf[4] ____cacheline_aligned;
+ __be16 rx_buf[5] ____cacheline_aligned; /* First 16-bits are trash */
+};
+
+#define ADC084S021_VOLTAGE_CHANNEL(num) \
+ { \
+ .type = IIO_VOLTAGE, \
+ .channel = (num), \
+ .indexed = 1, \
+ .scan_index = (num), \
+ .scan_type = { \
+ .sign = 'u', \
+ .realbits = 8, \
+ .storagebits = 16, \
+ .shift = 4, \
+ .endianness = IIO_BE, \
+ }, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
+ }
+
+static const struct iio_chan_spec adc084s021_channels[] = {
+ ADC084S021_VOLTAGE_CHANNEL(0),
+ ADC084S021_VOLTAGE_CHANNEL(1),
+ ADC084S021_VOLTAGE_CHANNEL(2),
+ ADC084S021_VOLTAGE_CHANNEL(3),
+ IIO_CHAN_SOFT_TIMESTAMP(4),
+};
+
+static int adc084s021_power_on(struct adc084s021 *adc)
+{
+ int ret;
+
+ ret = regulator_enable(adc->reg);
+ if (ret) {
+ dev_warn(&adc->spi->dev,
+ "Failed to enable supply voltage\n");
+ }
+
+ return ret;
+}
+
+static int adc084s021_power_off(struct adc084s021 *adc)
+{
+ int ret;
+
+ ret = regulator_disable(adc->reg);
+ if (ret) {
+ dev_warn(&adc->spi->dev,
+ "Failed to disable supply voltage\n");
+ }
+
+ return ret;
+}
+
+/**
+ * Read an ADC channel and return its value.
+ *
+ * @adc: The ADC SPI data.
+ * @data: Buffer for converted data.
+ */
+static int adc084s021_adc_conversion(struct adc084s021 *adc, void *data)
+{
+ int n_words = (adc->spi_trans.len >> 1) - 1; /* Discard first word */
+ int ret, i = 0;
+ u16 *p = data;
+
+ /* Do the transfer */
+ ret = spi_sync(adc->spi, &adc->message);
+ if (ret < 0)
+ return ret;
+
+ for (; i < n_words; i++)
+ *(p + i) = be16_to_cpu(adc->rx_buf[i + 1]);
+
+ return ret;
+}
+
+static int adc084s021_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *channel, int *val,
+ int *val2, long mask)
+{
+ struct adc084s021 *adc = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret < 0)
+ return ret;
+
+ ret = adc084s021_power_on(adc);
+ if (ret) {
+ iio_device_release_direct_mode(indio_dev);
+ return ret;
+ }
+
+ adc->tx_buf[0] = channel->channel << 3;
+ ret = adc084s021_adc_conversion(adc, val);
+ iio_device_release_direct_mode(indio_dev);
+ adc084s021_power_off(adc);
+ if (ret < 0)
+ return ret;
+
+ *val = (*val >> channel->scan_type.shift) & 0xff;
+
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ ret = adc084s021_power_on(adc);
+ if (ret)
+ return ret;
+
+ ret = regulator_get_voltage(adc->reg);
+ adc084s021_power_off(adc);
+ if (ret < 0)
+ return ret;
+
+ *val = ret / 1000;
+
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+}
+
+/**
+ * Read enabled ADC channels and push data to the buffer.
+ *
+ * @irq: The interrupt number (not used).
+ * @pollfunc: Pointer to the poll func.
+ */
+static irqreturn_t adc084s021_buffer_trigger_handler(int irq, void *pollfunc)
+{
+ struct iio_poll_func *pf = pollfunc;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct adc084s021 *adc = iio_priv(indio_dev);
+ __be16 data[8] = {0}; /* 4 * 16-bit words of data + 8 bytes timestamp */
+
+ mutex_lock(&adc->lock);
+
+ if (adc084s021_adc_conversion(adc, &data) < 0)
+ dev_err(&adc->spi->dev, "Failed to read data\n");
+
+ iio_push_to_buffers_with_timestamp(indio_dev, data,
+ iio_get_time_ns(indio_dev));
+ mutex_unlock(&adc->lock);
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
+static int adc084s021_buffer_preenable(struct iio_dev *indio_dev)
+{
+ struct adc084s021 *adc = iio_priv(indio_dev);
+ int scan_index;
+ int i = 0;
+
+ for_each_set_bit(scan_index, indio_dev->active_scan_mask,
+ indio_dev->masklength) {
+ const struct iio_chan_spec *channel =
+ &indio_dev->channels[scan_index];
+ adc->tx_buf[i++] = channel->channel << 3;
+ }
+ adc->spi_trans.len = 2 + (i * sizeof(__be16)); /* Trash + channels */
+
+ return adc084s021_power_on(adc);
+}
+
+static int adc084s021_buffer_postdisable(struct iio_dev *indio_dev)
+{
+ struct adc084s021 *adc = iio_priv(indio_dev);
+
+ adc->spi_trans.len = 4; /* Trash + single channel */
+
+ return adc084s021_power_off(adc);
+}
+
+static const struct iio_info adc084s021_info = {
+ .read_raw = adc084s021_read_raw,
+ .driver_module = THIS_MODULE,
+};
+
+static const struct iio_buffer_setup_ops adc084s021_buffer_setup_ops = {
+ .preenable = adc084s021_buffer_preenable,
+ .postenable = iio_triggered_buffer_postenable,
+ .predisable = iio_triggered_buffer_predisable,
+ .postdisable = adc084s021_buffer_postdisable,
+};
+
+static int adc084s021_probe(struct spi_device *spi)
+{
+ struct iio_dev *indio_dev;
+ struct adc084s021 *adc;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
+ if (!indio_dev) {
+ dev_err(&spi->dev, "Failed to allocate IIO device\n");
+ return -ENOMEM;
+ }
+
+ adc = iio_priv(indio_dev);
+ adc->spi = spi;
+
+ /* Connect the SPI device and the iio dev */
+ spi_set_drvdata(spi, indio_dev);
+
+ /* Initiate the Industrial I/O device */
+ indio_dev->dev.parent = &spi->dev;
+ indio_dev->dev.of_node = spi->dev.of_node;
+ indio_dev->name = spi_get_device_id(spi)->name;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->info = &adc084s021_info;
+ indio_dev->channels = adc084s021_channels;
+ indio_dev->num_channels = ARRAY_SIZE(adc084s021_channels);
+
+ /* Create SPI transfer for channel reads */
+ adc->spi_trans.tx_buf = adc->tx_buf;
+ adc->spi_trans.rx_buf = adc->rx_buf;
+ adc->spi_trans.len = 4; /* Trash + single channel */
+ spi_message_init_with_transfers(&adc->message, &adc->spi_trans, 1);
+
+ adc->reg = devm_regulator_get(&spi->dev, "vref");
+ if (IS_ERR(adc->reg))
+ return PTR_ERR(adc->reg);
+
+ mutex_init(&adc->lock);
+
+ /* Setup triggered buffer with pollfunction */
+ ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
+ adc084s021_buffer_trigger_handler,
+ &adc084s021_buffer_setup_ops);
+ if (ret) {
+ dev_err(&spi->dev, "Failed to setup triggered buffer\n");
+ return ret;
+ }
+
+ ret = devm_iio_device_register(&spi->dev, indio_dev);
+
+ return ret;
+}
+
+static const struct of_device_id adc084s021_of_match[] = {
+ { .compatible = "ti,adc084s021", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, adc084s021_of_match);
+
+static const struct spi_device_id adc084s021_id[] = {
+ { ADC084S021_DRIVER_NAME, 0},
+ {}
+};
+MODULE_DEVICE_TABLE(spi, adc084s021_id);
+
+static struct spi_driver adc084s021_driver = {
+ .driver = {
+ .name = ADC084S021_DRIVER_NAME,
+ .of_match_table = of_match_ptr(adc084s021_of_match),
+ },
+ .probe = adc084s021_probe,
+ .id_table = adc084s021_id,
+};
+module_spi_driver(adc084s021_driver);
+
+MODULE_AUTHOR("Mårten Lindahl <martenli@axis.com>");
+MODULE_DESCRIPTION("Texas Instruments ADC084S021");
+MODULE_LICENSE("GPL v2");
+MODULE_VERSION("1.0");
--
2.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] iio: adc: add driver for the ti-adc084s021 chip
2017-05-03 12:01 ` Mårten Lindahl
@ 2017-05-07 14:21 ` Jonathan Cameron
-1 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2017-05-07 14:21 UTC (permalink / raw)
To: Mårten Lindahl
Cc: knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
pmeerw-jW+XmwGofnusTnJN9+BGXg, linux-iio-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
devicetree-u79uwXL29TY76Z2rM5mHXA, Mårten Lindahl
On 03/05/17 13:01, Mårten Lindahl wrote:
> From: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
>
> This adds support for the Texas Instruments ADC084S021 ADC chip.
>
> Signed-off-by: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
A few more minor bits and pieces.
Jonathan
> ---
> Changes in v3:
> - Removed unnecessary comment about channel specification
> - Skipped usage of 'address' in iio_chan_spec config macro
> - Mask and shift channel readings only for _read_raw function
> - Enable/disable regulator in _read_raw function
> - Improved setup of ADC channel readings
> - Use SPI config of speed_hz and bits_per_word
> - Use devm_iio_triggered_buffer_setup and devm_iio_device_register
> - Removed error message for failed devm_iio_device_register
> - Removed driver _remove callback function
>
> Changes in v2:
> - Removed most #defines in favor of inlines
> - Corrected channel macro
> - Removed configuration array with only one item
> - Updated func adc084s021_adc_conversion to use be16_to_cpu
> - Added IIO_CHAN_INFO_SCALE to func adc084s021_read_raw
> - Use iio_device_claim_direct_mode in func adc084s021_read_raw
> - Removed documentation for standard driver functions
> - Changed retval to ret everywhere
> - Removed dynamic alloc for data buffer in trigger handler
> - Keeping mutex for all iterations in trigger handler
> - Removed usage of events in this driver
> - Removed info log in probe
> - Use spi_message_init_with_transfers for spi message structs
> - Use preenable and postdisable functions for regulator
> - Inserted blank line before last return in all functions
>
> drivers/iio/adc/Kconfig | 12 ++
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ti-adc084s021.c | 302 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 315 insertions(+)
> create mode 100644 drivers/iio/adc/ti-adc084s021.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index dedae7a..13141e5 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -560,6 +560,18 @@ config TI_ADC0832
> This driver can also be built as a module. If so, the module will be
> called ti-adc0832.
>
> +config TI_ADC084S021
> + tristate "Texas Instruments ADC084S021"
> + depends on SPI
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + help
> + If you say yes here you get support for Texas Instruments ADC084S021
> + chips.
> +
> + This driver can also be built as a module. If so, the module will be
> + called ti-adc084s021.
> +
> config TI_ADC12138
> tristate "Texas Instruments ADC12130/ADC12132/ADC12138"
> depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index d001262..b1a6158 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -51,6 +51,7 @@ obj-$(CONFIG_STM32_ADC_CORE) += stm32-adc-core.o
> obj-$(CONFIG_STM32_ADC) += stm32-adc.o
> obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
> obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
> +obj-$(CONFIG_TI_ADC084S021) += ti-adc084s021.o
> obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
> obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
> obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o
> diff --git a/drivers/iio/adc/ti-adc084s021.c b/drivers/iio/adc/ti-adc084s021.c
> new file mode 100644
> index 0000000..f2fb0fa
> --- /dev/null
> +++ b/drivers/iio/adc/ti-adc084s021.c
> @@ -0,0 +1,302 @@
> +/**
> + * Copyright (C) 2017 Axis Communications AB
> + *
> + * Driver for Texas Instruments' ADC084S021 ADC chip.
> + * Datasheets can be found here:
> + * http://www.ti.com/lit/ds/symlink/adc084s021.pdf
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/spi/spi.h>
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/regulator/consumer.h>
> +
> +#define ADC084S021_DRIVER_NAME "adc084s021"
> +
> +struct adc084s021 {
> + struct spi_device *spi;
> + struct spi_message message;
> + struct spi_transfer spi_trans;
> + struct regulator *reg;
> + struct mutex lock;
> + /*
> + * DMA (thus cache coherency maintenance) requires the
> + * transfer buffers to live in their own cache lines.
> + */
> + u16 tx_buf[4] ____cacheline_aligned;
> + __be16 rx_buf[5] ____cacheline_aligned; /* First 16-bits are trash */
You only need to do this for tx_buf as then rx_buf will at worst end up
in the same cacheline as it. Fairly implausible that this would cause
issues given they will be read before a transfer and written after.
Doing it twice results in potential waste of say 64 bytes depending on
the cacheline length.
> +};
> +
> +#define ADC084S021_VOLTAGE_CHANNEL(num) \
> + { \
> + .type = IIO_VOLTAGE, \
> + .channel = (num), \
> + .indexed = 1, \
> + .scan_index = (num), \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = 8, \
> + .storagebits = 16, \
> + .shift = 4, \
> + .endianness = IIO_BE, \
They aren't at the moment as you are doing the endian conversion in kernel.
Drop that in the push to buffers path.
> + }, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
> + }
> +
> +static const struct iio_chan_spec adc084s021_channels[] = {
> + ADC084S021_VOLTAGE_CHANNEL(0),
> + ADC084S021_VOLTAGE_CHANNEL(1),
> + ADC084S021_VOLTAGE_CHANNEL(2),
> + ADC084S021_VOLTAGE_CHANNEL(3),
> + IIO_CHAN_SOFT_TIMESTAMP(4),
> +};
> +
> +static int adc084s021_power_on(struct adc084s021 *adc)
> +{
> + int ret;
> +
> + ret = regulator_enable(adc->reg);
> + if (ret) {
> + dev_warn(&adc->spi->dev,
> + "Failed to enable supply voltage\n");
> + }
> +
> + return ret;
These two little functions do seem a little unnecessary.
A regulator fail should be pretty unlikely so I'd be tempted
to just have the regulator_enable / disable inline instead
of off in these functions.
> +}
> +
> +static int adc084s021_power_off(struct adc084s021 *adc)
> +{
> + int ret;
> +
> + ret = regulator_disable(adc->reg);
> + if (ret) {
> + dev_warn(&adc->spi->dev,
> + "Failed to disable supply voltage\n");
> + }
> +
> + return ret;
> +}
> +
> +/**
> + * Read an ADC channel and return its value.
> + *
> + * @adc: The ADC SPI data.
> + * @data: Buffer for converted data.
> + */
> +static int adc084s021_adc_conversion(struct adc084s021 *adc, void *data)
> +{
> + int n_words = (adc->spi_trans.len >> 1) - 1; /* Discard first word */
> + int ret, i = 0;
> + u16 *p = data;
> +
> + /* Do the transfer */
> + ret = spi_sync(adc->spi, &adc->message);
> + if (ret < 0)
> + return ret;
> +
> + for (; i < n_words; i++)
> + *(p + i) = be16_to_cpu(adc->rx_buf[i + 1]);
Should only do the endian conversion in kernel if not
using it for buffered output. In buffered output we can
save a few cycles by leaving it up to userspace.
> +
> + return ret;
> +}
> +
> +static int adc084s021_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *channel, int *val,
> + int *val2, long mask)
> +{
> + struct adc084s021 *adc = iio_priv(indio_dev);
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = iio_device_claim_direct_mode(indio_dev);
> + if (ret < 0)
> + return ret;
> +
> + ret = adc084s021_power_on(adc);
> + if (ret) {
> + iio_device_release_direct_mode(indio_dev);
> + return ret;
> + }
> +
> + adc->tx_buf[0] = channel->channel << 3;
> + ret = adc084s021_adc_conversion(adc, val);
> + iio_device_release_direct_mode(indio_dev);
> + adc084s021_power_off(adc);
> + if (ret < 0)
> + return ret;
> +
> + *val = (*val >> channel->scan_type.shift) & 0xff;
> +
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + ret = adc084s021_power_on(adc);
> + if (ret)
> + return ret;
> +
> + ret = regulator_get_voltage(adc->reg);
> + adc084s021_power_off(adc);
> + if (ret < 0)
> + return ret;
> +
> + *val = ret / 1000;
> +
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +/**
> + * Read enabled ADC channels and push data to the buffer.
> + *
> + * @irq: The interrupt number (not used).
> + * @pollfunc: Pointer to the poll func.
> + */
> +static irqreturn_t adc084s021_buffer_trigger_handler(int irq, void *pollfunc)
> +{
> + struct iio_poll_func *pf = pollfunc;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct adc084s021 *adc = iio_priv(indio_dev);
> + __be16 data[8] = {0}; /* 4 * 16-bit words of data + 8 bytes timestamp */
> +
> + mutex_lock(&adc->lock);
> +
> + if (adc084s021_adc_conversion(adc, &data) < 0)
> + dev_err(&adc->spi->dev, "Failed to read data\n");
hmm. Not sure I'm keen on pushing garbage to the buffer.
I might fix this up as well... Depends on what else there is ;)
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, data,
> + iio_get_time_ns(indio_dev));
> + mutex_unlock(&adc->lock);
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int adc084s021_buffer_preenable(struct iio_dev *indio_dev)
> +{
> + struct adc084s021 *adc = iio_priv(indio_dev);
> + int scan_index;
> + int i = 0;
> +
> + for_each_set_bit(scan_index, indio_dev->active_scan_mask,
> + indio_dev->masklength) {
> + const struct iio_chan_spec *channel =
> + &indio_dev->channels[scan_index];
> + adc->tx_buf[i++] = channel->channel << 3;
> + }
> + adc->spi_trans.len = 2 + (i * sizeof(__be16)); /* Trash + channels */
> +
> + return adc084s021_power_on(adc);
> +}
> +
> +static int adc084s021_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> + struct adc084s021 *adc = iio_priv(indio_dev);
> +
> + adc->spi_trans.len = 4; /* Trash + single channel */
> +
> + return adc084s021_power_off(adc);
> +}
> +
> +static const struct iio_info adc084s021_info = {
> + .read_raw = adc084s021_read_raw,
> + .driver_module = THIS_MODULE,
> +};
> +
> +static const struct iio_buffer_setup_ops adc084s021_buffer_setup_ops = {
> + .preenable = adc084s021_buffer_preenable,
> + .postenable = iio_triggered_buffer_postenable,
> + .predisable = iio_triggered_buffer_predisable,
> + .postdisable = adc084s021_buffer_postdisable,
> +};
> +
> +static int adc084s021_probe(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev;
> + struct adc084s021 *adc;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
> + if (!indio_dev) {
> + dev_err(&spi->dev, "Failed to allocate IIO device\n");
> + return -ENOMEM;
> + }
> +
> + adc = iio_priv(indio_dev);
> + adc->spi = spi;
> +
> + /* Connect the SPI device and the iio dev */
> + spi_set_drvdata(spi, indio_dev);
> +
> + /* Initiate the Industrial I/O device */
> + indio_dev->dev.parent = &spi->dev;
> + indio_dev->dev.of_node = spi->dev.of_node;
> + indio_dev->name = spi_get_device_id(spi)->name;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->info = &adc084s021_info;
> + indio_dev->channels = adc084s021_channels;
> + indio_dev->num_channels = ARRAY_SIZE(adc084s021_channels);
> +
> + /* Create SPI transfer for channel reads */
> + adc->spi_trans.tx_buf = adc->tx_buf;
> + adc->spi_trans.rx_buf = adc->rx_buf;
> + adc->spi_trans.len = 4; /* Trash + single channel */
> + spi_message_init_with_transfers(&adc->message, &adc->spi_trans, 1);
> +
> + adc->reg = devm_regulator_get(&spi->dev, "vref");
> + if (IS_ERR(adc->reg))
> + return PTR_ERR(adc->reg);
> +
> + mutex_init(&adc->lock);
> +
> + /* Setup triggered buffer with pollfunction */
> + ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
> + adc084s021_buffer_trigger_handler,
> + &adc084s021_buffer_setup_ops);
> + if (ret) {
> + dev_err(&spi->dev, "Failed to setup triggered buffer\n");
> + return ret;
> + }
> +
> + ret = devm_iio_device_register(&spi->dev, indio_dev);
> +
> + return ret;
return devm_iio_deivce_register(...
If this all there is I'll fix it up.
> +}
> +
> +static const struct of_device_id adc084s021_of_match[] = {
> + { .compatible = "ti,adc084s021", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, adc084s021_of_match);
> +
> +static const struct spi_device_id adc084s021_id[] = {
> + { ADC084S021_DRIVER_NAME, 0},
> + {}
> +};
> +MODULE_DEVICE_TABLE(spi, adc084s021_id);
> +
> +static struct spi_driver adc084s021_driver = {
> + .driver = {
> + .name = ADC084S021_DRIVER_NAME,
> + .of_match_table = of_match_ptr(adc084s021_of_match),
> + },
> + .probe = adc084s021_probe,
> + .id_table = adc084s021_id,
> +};
> +module_spi_driver(adc084s021_driver);
> +
> +MODULE_AUTHOR("Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>");
> +MODULE_DESCRIPTION("Texas Instruments ADC084S021");
> +MODULE_LICENSE("GPL v2");
> +MODULE_VERSION("1.0");
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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] 14+ messages in thread
* Re: [PATCH v3 2/2] iio: adc: add driver for the ti-adc084s021 chip
@ 2017-05-07 14:21 ` Jonathan Cameron
0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2017-05-07 14:21 UTC (permalink / raw)
To: Mårten Lindahl
Cc: knaack.h, lars, pmeerw, linux-iio, robh+dt, mark.rutland,
devicetree, Mårten Lindahl
On 03/05/17 13:01, Mårten Lindahl wrote:
> From: Mårten Lindahl <martenli@axis.com>
>
> This adds support for the Texas Instruments ADC084S021 ADC chip.
>
> Signed-off-by: Mårten Lindahl <martenli@axis.com>
A few more minor bits and pieces.
Jonathan
> ---
> Changes in v3:
> - Removed unnecessary comment about channel specification
> - Skipped usage of 'address' in iio_chan_spec config macro
> - Mask and shift channel readings only for _read_raw function
> - Enable/disable regulator in _read_raw function
> - Improved setup of ADC channel readings
> - Use SPI config of speed_hz and bits_per_word
> - Use devm_iio_triggered_buffer_setup and devm_iio_device_register
> - Removed error message for failed devm_iio_device_register
> - Removed driver _remove callback function
>
> Changes in v2:
> - Removed most #defines in favor of inlines
> - Corrected channel macro
> - Removed configuration array with only one item
> - Updated func adc084s021_adc_conversion to use be16_to_cpu
> - Added IIO_CHAN_INFO_SCALE to func adc084s021_read_raw
> - Use iio_device_claim_direct_mode in func adc084s021_read_raw
> - Removed documentation for standard driver functions
> - Changed retval to ret everywhere
> - Removed dynamic alloc for data buffer in trigger handler
> - Keeping mutex for all iterations in trigger handler
> - Removed usage of events in this driver
> - Removed info log in probe
> - Use spi_message_init_with_transfers for spi message structs
> - Use preenable and postdisable functions for regulator
> - Inserted blank line before last return in all functions
>
> drivers/iio/adc/Kconfig | 12 ++
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ti-adc084s021.c | 302 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 315 insertions(+)
> create mode 100644 drivers/iio/adc/ti-adc084s021.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index dedae7a..13141e5 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -560,6 +560,18 @@ config TI_ADC0832
> This driver can also be built as a module. If so, the module will be
> called ti-adc0832.
>
> +config TI_ADC084S021
> + tristate "Texas Instruments ADC084S021"
> + depends on SPI
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + help
> + If you say yes here you get support for Texas Instruments ADC084S021
> + chips.
> +
> + This driver can also be built as a module. If so, the module will be
> + called ti-adc084s021.
> +
> config TI_ADC12138
> tristate "Texas Instruments ADC12130/ADC12132/ADC12138"
> depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index d001262..b1a6158 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -51,6 +51,7 @@ obj-$(CONFIG_STM32_ADC_CORE) += stm32-adc-core.o
> obj-$(CONFIG_STM32_ADC) += stm32-adc.o
> obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
> obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
> +obj-$(CONFIG_TI_ADC084S021) += ti-adc084s021.o
> obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
> obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
> obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o
> diff --git a/drivers/iio/adc/ti-adc084s021.c b/drivers/iio/adc/ti-adc084s021.c
> new file mode 100644
> index 0000000..f2fb0fa
> --- /dev/null
> +++ b/drivers/iio/adc/ti-adc084s021.c
> @@ -0,0 +1,302 @@
> +/**
> + * Copyright (C) 2017 Axis Communications AB
> + *
> + * Driver for Texas Instruments' ADC084S021 ADC chip.
> + * Datasheets can be found here:
> + * http://www.ti.com/lit/ds/symlink/adc084s021.pdf
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/spi/spi.h>
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/regulator/consumer.h>
> +
> +#define ADC084S021_DRIVER_NAME "adc084s021"
> +
> +struct adc084s021 {
> + struct spi_device *spi;
> + struct spi_message message;
> + struct spi_transfer spi_trans;
> + struct regulator *reg;
> + struct mutex lock;
> + /*
> + * DMA (thus cache coherency maintenance) requires the
> + * transfer buffers to live in their own cache lines.
> + */
> + u16 tx_buf[4] ____cacheline_aligned;
> + __be16 rx_buf[5] ____cacheline_aligned; /* First 16-bits are trash */
You only need to do this for tx_buf as then rx_buf will at worst end up
in the same cacheline as it. Fairly implausible that this would cause
issues given they will be read before a transfer and written after.
Doing it twice results in potential waste of say 64 bytes depending on
the cacheline length.
> +};
> +
> +#define ADC084S021_VOLTAGE_CHANNEL(num) \
> + { \
> + .type = IIO_VOLTAGE, \
> + .channel = (num), \
> + .indexed = 1, \
> + .scan_index = (num), \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = 8, \
> + .storagebits = 16, \
> + .shift = 4, \
> + .endianness = IIO_BE, \
They aren't at the moment as you are doing the endian conversion in kernel.
Drop that in the push to buffers path.
> + }, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
> + }
> +
> +static const struct iio_chan_spec adc084s021_channels[] = {
> + ADC084S021_VOLTAGE_CHANNEL(0),
> + ADC084S021_VOLTAGE_CHANNEL(1),
> + ADC084S021_VOLTAGE_CHANNEL(2),
> + ADC084S021_VOLTAGE_CHANNEL(3),
> + IIO_CHAN_SOFT_TIMESTAMP(4),
> +};
> +
> +static int adc084s021_power_on(struct adc084s021 *adc)
> +{
> + int ret;
> +
> + ret = regulator_enable(adc->reg);
> + if (ret) {
> + dev_warn(&adc->spi->dev,
> + "Failed to enable supply voltage\n");
> + }
> +
> + return ret;
These two little functions do seem a little unnecessary.
A regulator fail should be pretty unlikely so I'd be tempted
to just have the regulator_enable / disable inline instead
of off in these functions.
> +}
> +
> +static int adc084s021_power_off(struct adc084s021 *adc)
> +{
> + int ret;
> +
> + ret = regulator_disable(adc->reg);
> + if (ret) {
> + dev_warn(&adc->spi->dev,
> + "Failed to disable supply voltage\n");
> + }
> +
> + return ret;
> +}
> +
> +/**
> + * Read an ADC channel and return its value.
> + *
> + * @adc: The ADC SPI data.
> + * @data: Buffer for converted data.
> + */
> +static int adc084s021_adc_conversion(struct adc084s021 *adc, void *data)
> +{
> + int n_words = (adc->spi_trans.len >> 1) - 1; /* Discard first word */
> + int ret, i = 0;
> + u16 *p = data;
> +
> + /* Do the transfer */
> + ret = spi_sync(adc->spi, &adc->message);
> + if (ret < 0)
> + return ret;
> +
> + for (; i < n_words; i++)
> + *(p + i) = be16_to_cpu(adc->rx_buf[i + 1]);
Should only do the endian conversion in kernel if not
using it for buffered output. In buffered output we can
save a few cycles by leaving it up to userspace.
> +
> + return ret;
> +}
> +
> +static int adc084s021_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *channel, int *val,
> + int *val2, long mask)
> +{
> + struct adc084s021 *adc = iio_priv(indio_dev);
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = iio_device_claim_direct_mode(indio_dev);
> + if (ret < 0)
> + return ret;
> +
> + ret = adc084s021_power_on(adc);
> + if (ret) {
> + iio_device_release_direct_mode(indio_dev);
> + return ret;
> + }
> +
> + adc->tx_buf[0] = channel->channel << 3;
> + ret = adc084s021_adc_conversion(adc, val);
> + iio_device_release_direct_mode(indio_dev);
> + adc084s021_power_off(adc);
> + if (ret < 0)
> + return ret;
> +
> + *val = (*val >> channel->scan_type.shift) & 0xff;
> +
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + ret = adc084s021_power_on(adc);
> + if (ret)
> + return ret;
> +
> + ret = regulator_get_voltage(adc->reg);
> + adc084s021_power_off(adc);
> + if (ret < 0)
> + return ret;
> +
> + *val = ret / 1000;
> +
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +/**
> + * Read enabled ADC channels and push data to the buffer.
> + *
> + * @irq: The interrupt number (not used).
> + * @pollfunc: Pointer to the poll func.
> + */
> +static irqreturn_t adc084s021_buffer_trigger_handler(int irq, void *pollfunc)
> +{
> + struct iio_poll_func *pf = pollfunc;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct adc084s021 *adc = iio_priv(indio_dev);
> + __be16 data[8] = {0}; /* 4 * 16-bit words of data + 8 bytes timestamp */
> +
> + mutex_lock(&adc->lock);
> +
> + if (adc084s021_adc_conversion(adc, &data) < 0)
> + dev_err(&adc->spi->dev, "Failed to read data\n");
hmm. Not sure I'm keen on pushing garbage to the buffer.
I might fix this up as well... Depends on what else there is ;)
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, data,
> + iio_get_time_ns(indio_dev));
> + mutex_unlock(&adc->lock);
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int adc084s021_buffer_preenable(struct iio_dev *indio_dev)
> +{
> + struct adc084s021 *adc = iio_priv(indio_dev);
> + int scan_index;
> + int i = 0;
> +
> + for_each_set_bit(scan_index, indio_dev->active_scan_mask,
> + indio_dev->masklength) {
> + const struct iio_chan_spec *channel =
> + &indio_dev->channels[scan_index];
> + adc->tx_buf[i++] = channel->channel << 3;
> + }
> + adc->spi_trans.len = 2 + (i * sizeof(__be16)); /* Trash + channels */
> +
> + return adc084s021_power_on(adc);
> +}
> +
> +static int adc084s021_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> + struct adc084s021 *adc = iio_priv(indio_dev);
> +
> + adc->spi_trans.len = 4; /* Trash + single channel */
> +
> + return adc084s021_power_off(adc);
> +}
> +
> +static const struct iio_info adc084s021_info = {
> + .read_raw = adc084s021_read_raw,
> + .driver_module = THIS_MODULE,
> +};
> +
> +static const struct iio_buffer_setup_ops adc084s021_buffer_setup_ops = {
> + .preenable = adc084s021_buffer_preenable,
> + .postenable = iio_triggered_buffer_postenable,
> + .predisable = iio_triggered_buffer_predisable,
> + .postdisable = adc084s021_buffer_postdisable,
> +};
> +
> +static int adc084s021_probe(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev;
> + struct adc084s021 *adc;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
> + if (!indio_dev) {
> + dev_err(&spi->dev, "Failed to allocate IIO device\n");
> + return -ENOMEM;
> + }
> +
> + adc = iio_priv(indio_dev);
> + adc->spi = spi;
> +
> + /* Connect the SPI device and the iio dev */
> + spi_set_drvdata(spi, indio_dev);
> +
> + /* Initiate the Industrial I/O device */
> + indio_dev->dev.parent = &spi->dev;
> + indio_dev->dev.of_node = spi->dev.of_node;
> + indio_dev->name = spi_get_device_id(spi)->name;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->info = &adc084s021_info;
> + indio_dev->channels = adc084s021_channels;
> + indio_dev->num_channels = ARRAY_SIZE(adc084s021_channels);
> +
> + /* Create SPI transfer for channel reads */
> + adc->spi_trans.tx_buf = adc->tx_buf;
> + adc->spi_trans.rx_buf = adc->rx_buf;
> + adc->spi_trans.len = 4; /* Trash + single channel */
> + spi_message_init_with_transfers(&adc->message, &adc->spi_trans, 1);
> +
> + adc->reg = devm_regulator_get(&spi->dev, "vref");
> + if (IS_ERR(adc->reg))
> + return PTR_ERR(adc->reg);
> +
> + mutex_init(&adc->lock);
> +
> + /* Setup triggered buffer with pollfunction */
> + ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
> + adc084s021_buffer_trigger_handler,
> + &adc084s021_buffer_setup_ops);
> + if (ret) {
> + dev_err(&spi->dev, "Failed to setup triggered buffer\n");
> + return ret;
> + }
> +
> + ret = devm_iio_device_register(&spi->dev, indio_dev);
> +
> + return ret;
return devm_iio_deivce_register(...
If this all there is I'll fix it up.
> +}
> +
> +static const struct of_device_id adc084s021_of_match[] = {
> + { .compatible = "ti,adc084s021", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, adc084s021_of_match);
> +
> +static const struct spi_device_id adc084s021_id[] = {
> + { ADC084S021_DRIVER_NAME, 0},
> + {}
> +};
> +MODULE_DEVICE_TABLE(spi, adc084s021_id);
> +
> +static struct spi_driver adc084s021_driver = {
> + .driver = {
> + .name = ADC084S021_DRIVER_NAME,
> + .of_match_table = of_match_ptr(adc084s021_of_match),
> + },
> + .probe = adc084s021_probe,
> + .id_table = adc084s021_id,
> +};
> +module_spi_driver(adc084s021_driver);
> +
> +MODULE_AUTHOR("Mårten Lindahl <martenli@axis.com>");
> +MODULE_DESCRIPTION("Texas Instruments ADC084S021");
> +MODULE_LICENSE("GPL v2");
> +MODULE_VERSION("1.0");
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: adc: add driver for the ti-adc084s021 chip
2017-05-03 12:01 ` Mårten Lindahl
@ 2017-05-08 16:25 ` Rob Herring
-1 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2017-05-08 16:25 UTC (permalink / raw)
To: Mårten Lindahl
Cc: jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
linux-iio-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8,
devicetree-u79uwXL29TY76Z2rM5mHXA, Mårten Lindahl
On Wed, May 03, 2017 at 02:01:44PM +0200, Mårten Lindahl wrote:
> From: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
>
> This adds support for the Texas Instruments ADC084S021 ADC chip.
>
> Signed-off-by: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
> ---
> Changes in v3:
> - No updates since v2
>
> Changes in v2:
> - Updated the 'Required properties' section
> - Removed the 'Optional properties' section
>
> .../devicetree/bindings/iio/adc/ti-adc084s021.txt | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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] 14+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: adc: add driver for the ti-adc084s021 chip
@ 2017-05-08 16:25 ` Rob Herring
0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2017-05-08 16:25 UTC (permalink / raw)
To: Mårten Lindahl
Cc: jic23, knaack.h, lars, pmeerw, linux-iio, mark.rutland,
devicetree, Mårten Lindahl
On Wed, May 03, 2017 at 02:01:44PM +0200, Mårten Lindahl wrote:
> From: Mårten Lindahl <martenli@axis.com>
>
> This adds support for the Texas Instruments ADC084S021 ADC chip.
>
> Signed-off-by: Mårten Lindahl <martenli@axis.com>
> ---
> Changes in v3:
> - No updates since v2
>
> Changes in v2:
> - Updated the 'Required properties' section
> - Removed the 'Optional properties' section
>
> .../devicetree/bindings/iio/adc/ti-adc084s021.txt | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] iio: adc: add driver for the ti-adc084s021 chip
2017-05-07 14:21 ` Jonathan Cameron
@ 2017-05-09 15:17 ` Mårten Lindahl
-1 siblings, 0 replies; 14+ messages in thread
From: Mårten Lindahl @ 2017-05-09 15:17 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Mårten Lindahl, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
linux-iio-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA
On Sun, 2017-05-07 at 15:21 +0100, Jonathan Cameron wrote:
> On 03/05/17 13:01, Mårten Lindahl wrote:
> > From: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
> >
> > This adds support for the Texas Instruments ADC084S021 ADC chip.
> >
> > Signed-off-by: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
> A few more minor bits and pieces.
>
> Jonathan
Hi Jonathan!
I made v4. Please read my comments below. Could there potentially be
garbage in the buffer?
Thanks,
Mårten
> > ---
> > Changes in v3:
> > - Removed unnecessary comment about channel specification
> > - Skipped usage of 'address' in iio_chan_spec config macro
> > - Mask and shift channel readings only for _read_raw function
> > - Enable/disable regulator in _read_raw function
> > - Improved setup of ADC channel readings
> > - Use SPI config of speed_hz and bits_per_word
> > - Use devm_iio_triggered_buffer_setup and devm_iio_device_register
> > - Removed error message for failed devm_iio_device_register
> > - Removed driver _remove callback function
> >
> > Changes in v2:
> > - Removed most #defines in favor of inlines
> > - Corrected channel macro
> > - Removed configuration array with only one item
> > - Updated func adc084s021_adc_conversion to use be16_to_cpu
> > - Added IIO_CHAN_INFO_SCALE to func adc084s021_read_raw
> > - Use iio_device_claim_direct_mode in func adc084s021_read_raw
> > - Removed documentation for standard driver functions
> > - Changed retval to ret everywhere
> > - Removed dynamic alloc for data buffer in trigger handler
> > - Keeping mutex for all iterations in trigger handler
> > - Removed usage of events in this driver
> > - Removed info log in probe
> > - Use spi_message_init_with_transfers for spi message structs
> > - Use preenable and postdisable functions for regulator
> > - Inserted blank line before last return in all functions
> >
> > drivers/iio/adc/Kconfig | 12 ++
> > drivers/iio/adc/Makefile | 1 +
> > drivers/iio/adc/ti-adc084s021.c | 302 ++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 315 insertions(+)
> > create mode 100644 drivers/iio/adc/ti-adc084s021.c
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index dedae7a..13141e5 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -560,6 +560,18 @@ config TI_ADC0832
> > This driver can also be built as a module. If so, the module will be
> > called ti-adc0832.
> >
> > +config TI_ADC084S021
> > + tristate "Texas Instruments ADC084S021"
> > + depends on SPI
> > + select IIO_BUFFER
> > + select IIO_TRIGGERED_BUFFER
> > + help
> > + If you say yes here you get support for Texas Instruments ADC084S021
> > + chips.
> > +
> > + This driver can also be built as a module. If so, the module will be
> > + called ti-adc084s021.
> > +
> > config TI_ADC12138
> > tristate "Texas Instruments ADC12130/ADC12132/ADC12138"
> > depends on SPI
> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > index d001262..b1a6158 100644
> > --- a/drivers/iio/adc/Makefile
> > +++ b/drivers/iio/adc/Makefile
> > @@ -51,6 +51,7 @@ obj-$(CONFIG_STM32_ADC_CORE) += stm32-adc-core.o
> > obj-$(CONFIG_STM32_ADC) += stm32-adc.o
> > obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
> > obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
> > +obj-$(CONFIG_TI_ADC084S021) += ti-adc084s021.o
> > obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
> > obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
> > obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o
> > diff --git a/drivers/iio/adc/ti-adc084s021.c b/drivers/iio/adc/ti-adc084s021.c
> > new file mode 100644
> > index 0000000..f2fb0fa
> > --- /dev/null
> > +++ b/drivers/iio/adc/ti-adc084s021.c
> > @@ -0,0 +1,302 @@
> > +/**
> > + * Copyright (C) 2017 Axis Communications AB
> > + *
> > + * Driver for Texas Instruments' ADC084S021 ADC chip.
> > + * Datasheets can be found here:
> > + * http://www.ti.com/lit/ds/symlink/adc084s021.pdf
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/err.h>
> > +#include <linux/spi/spi.h>
> > +#include <linux/module.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/buffer.h>
> > +#include <linux/iio/triggered_buffer.h>
> > +#include <linux/iio/trigger_consumer.h>
> > +#include <linux/regulator/consumer.h>
> > +
> > +#define ADC084S021_DRIVER_NAME "adc084s021"
> > +
> > +struct adc084s021 {
> > + struct spi_device *spi;
> > + struct spi_message message;
> > + struct spi_transfer spi_trans;
> > + struct regulator *reg;
> > + struct mutex lock;
> > + /*
> > + * DMA (thus cache coherency maintenance) requires the
> > + * transfer buffers to live in their own cache lines.
> > + */
> > + u16 tx_buf[4] ____cacheline_aligned;
> > + __be16 rx_buf[5] ____cacheline_aligned; /* First 16-bits are trash */
> You only need to do this for tx_buf as then rx_buf will at worst end up
> in the same cacheline as it. Fairly implausible that this would cause
> issues given they will be read before a transfer and written after.
> Doing it twice results in potential waste of say 64 bytes depending on
> the cacheline length.
Fixed and verified with pahole in v4.
> > +};
> > +
> > +#define ADC084S021_VOLTAGE_CHANNEL(num) \
> > + { \
> > + .type = IIO_VOLTAGE, \
> > + .channel = (num), \
> > + .indexed = 1, \
> > + .scan_index = (num), \
> > + .scan_type = { \
> > + .sign = 'u', \
> > + .realbits = 8, \
> > + .storagebits = 16, \
> > + .shift = 4, \
> > + .endianness = IIO_BE, \
> They aren't at the moment as you are doing the endian conversion in kernel.
> Drop that in the push to buffers path.
Ok, fixed in "push to buffers path" in v4.
> > + }, \
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
> > + }
> > +
> > +static const struct iio_chan_spec adc084s021_channels[] = {
> > + ADC084S021_VOLTAGE_CHANNEL(0),
> > + ADC084S021_VOLTAGE_CHANNEL(1),
> > + ADC084S021_VOLTAGE_CHANNEL(2),
> > + ADC084S021_VOLTAGE_CHANNEL(3),
> > + IIO_CHAN_SOFT_TIMESTAMP(4),
> > +};
> > +
> > +static int adc084s021_power_on(struct adc084s021 *adc)
> > +{
> > + int ret;
> > +
> > + ret = regulator_enable(adc->reg);
> > + if (ret) {
> > + dev_warn(&adc->spi->dev,
> > + "Failed to enable supply voltage\n");
> > + }
> > +
> > + return ret;
> These two little functions do seem a little unnecessary.
> A regulator fail should be pretty unlikely so I'd be tempted
> to just have the regulator_enable / disable inline instead
> of off in these functions.
Fixed in v4.
> > +}
> > +
> > +static int adc084s021_power_off(struct adc084s021 *adc)
> > +{
> > + int ret;
> > +
> > + ret = regulator_disable(adc->reg);
> > + if (ret) {
> > + dev_warn(&adc->spi->dev,
> > + "Failed to disable supply voltage\n");
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * Read an ADC channel and return its value.
> > + *
> > + * @adc: The ADC SPI data.
> > + * @data: Buffer for converted data.
> > + */
> > +static int adc084s021_adc_conversion(struct adc084s021 *adc, void *data)
> > +{
> > + int n_words = (adc->spi_trans.len >> 1) - 1; /* Discard first word */
> > + int ret, i = 0;
> > + u16 *p = data;
> > +
> > + /* Do the transfer */
> > + ret = spi_sync(adc->spi, &adc->message);
> > + if (ret < 0)
> > + return ret;
> > +
> > + for (; i < n_words; i++)
> > + *(p + i) = be16_to_cpu(adc->rx_buf[i + 1]);
> Should only do the endian conversion in kernel if not
> using it for buffered output. In buffered output we can
> save a few cycles by leaving it up to userspace.
Ok, fixed in v4.
> > +
> > + return ret;
> > +}
> > +
> > +static int adc084s021_read_raw(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *channel, int *val,
> > + int *val2, long mask)
> > +{
> > + struct adc084s021 *adc = iio_priv(indio_dev);
> > + int ret;
> > +
> > + switch (mask) {
> > + case IIO_CHAN_INFO_RAW:
> > + ret = iio_device_claim_direct_mode(indio_dev);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = adc084s021_power_on(adc);
> > + if (ret) {
> > + iio_device_release_direct_mode(indio_dev);
> > + return ret;
> > + }
> > +
> > + adc->tx_buf[0] = channel->channel << 3;
> > + ret = adc084s021_adc_conversion(adc, val);
> > + iio_device_release_direct_mode(indio_dev);
> > + adc084s021_power_off(adc);
> > + if (ret < 0)
> > + return ret;
> > +
> > + *val = (*val >> channel->scan_type.shift) & 0xff;
> > +
> > + return IIO_VAL_INT;
> > + case IIO_CHAN_INFO_SCALE:
> > + ret = adc084s021_power_on(adc);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regulator_get_voltage(adc->reg);
> > + adc084s021_power_off(adc);
> > + if (ret < 0)
> > + return ret;
> > +
> > + *val = ret / 1000;
> > +
> > + return IIO_VAL_INT;
> > + default:
> > + return -EINVAL;
> > + }
> > +}
> > +
> > +/**
> > + * Read enabled ADC channels and push data to the buffer.
> > + *
> > + * @irq: The interrupt number (not used).
> > + * @pollfunc: Pointer to the poll func.
> > + */
> > +static irqreturn_t adc084s021_buffer_trigger_handler(int irq, void *pollfunc)
> > +{
> > + struct iio_poll_func *pf = pollfunc;
> > + struct iio_dev *indio_dev = pf->indio_dev;
> > + struct adc084s021 *adc = iio_priv(indio_dev);
> > + __be16 data[8] = {0}; /* 4 * 16-bit words of data + 8 bytes timestamp */
> > +
> > + mutex_lock(&adc->lock);
> > +
> > + if (adc084s021_adc_conversion(adc, &data) < 0)
> > + dev_err(&adc->spi->dev, "Failed to read data\n");
> hmm. Not sure I'm keen on pushing garbage to the buffer.
> I might fix this up as well... Depends on what else there is ;)
Since the __be16 data[8] array is initialized to 0, and the
adc084s021_adc_conversion function error returns before any data has
been copied to the array it should always contain either zeroes or valid
data as I see it. Would it be wrong to push zeroes?
> > +
> > + iio_push_to_buffers_with_timestamp(indio_dev, data,
> > + iio_get_time_ns(indio_dev));
> > + mutex_unlock(&adc->lock);
> > + iio_trigger_notify_done(indio_dev->trig);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int adc084s021_buffer_preenable(struct iio_dev *indio_dev)
> > +{
> > + struct adc084s021 *adc = iio_priv(indio_dev);
> > + int scan_index;
> > + int i = 0;
> > +
> > + for_each_set_bit(scan_index, indio_dev->active_scan_mask,
> > + indio_dev->masklength) {
> > + const struct iio_chan_spec *channel =
> > + &indio_dev->channels[scan_index];
> > + adc->tx_buf[i++] = channel->channel << 3;
> > + }
> > + adc->spi_trans.len = 2 + (i * sizeof(__be16)); /* Trash + channels */
> > +
> > + return adc084s021_power_on(adc);
> > +}
> > +
> > +static int adc084s021_buffer_postdisable(struct iio_dev *indio_dev)
> > +{
> > + struct adc084s021 *adc = iio_priv(indio_dev);
> > +
> > + adc->spi_trans.len = 4; /* Trash + single channel */
> > +
> > + return adc084s021_power_off(adc);
> > +}
> > +
> > +static const struct iio_info adc084s021_info = {
> > + .read_raw = adc084s021_read_raw,
> > + .driver_module = THIS_MODULE,
> > +};
> > +
> > +static const struct iio_buffer_setup_ops adc084s021_buffer_setup_ops = {
> > + .preenable = adc084s021_buffer_preenable,
> > + .postenable = iio_triggered_buffer_postenable,
> > + .predisable = iio_triggered_buffer_predisable,
> > + .postdisable = adc084s021_buffer_postdisable,
> > +};
> > +
> > +static int adc084s021_probe(struct spi_device *spi)
> > +{
> > + struct iio_dev *indio_dev;
> > + struct adc084s021 *adc;
> > + int ret;
> > +
> > + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
> > + if (!indio_dev) {
> > + dev_err(&spi->dev, "Failed to allocate IIO device\n");
> > + return -ENOMEM;
> > + }
> > +
> > + adc = iio_priv(indio_dev);
> > + adc->spi = spi;
> > +
> > + /* Connect the SPI device and the iio dev */
> > + spi_set_drvdata(spi, indio_dev);
> > +
> > + /* Initiate the Industrial I/O device */
> > + indio_dev->dev.parent = &spi->dev;
> > + indio_dev->dev.of_node = spi->dev.of_node;
> > + indio_dev->name = spi_get_device_id(spi)->name;
> > + indio_dev->modes = INDIO_DIRECT_MODE;
> > + indio_dev->info = &adc084s021_info;
> > + indio_dev->channels = adc084s021_channels;
> > + indio_dev->num_channels = ARRAY_SIZE(adc084s021_channels);
> > +
> > + /* Create SPI transfer for channel reads */
> > + adc->spi_trans.tx_buf = adc->tx_buf;
> > + adc->spi_trans.rx_buf = adc->rx_buf;
> > + adc->spi_trans.len = 4; /* Trash + single channel */
> > + spi_message_init_with_transfers(&adc->message, &adc->spi_trans, 1);
> > +
> > + adc->reg = devm_regulator_get(&spi->dev, "vref");
> > + if (IS_ERR(adc->reg))
> > + return PTR_ERR(adc->reg);
> > +
> > + mutex_init(&adc->lock);
> > +
> > + /* Setup triggered buffer with pollfunction */
> > + ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
> > + adc084s021_buffer_trigger_handler,
> > + &adc084s021_buffer_setup_ops);
> > + if (ret) {
> > + dev_err(&spi->dev, "Failed to setup triggered buffer\n");
> > + return ret;
> > + }
> > +
> > + ret = devm_iio_device_register(&spi->dev, indio_dev);
> > +
> > + return ret;
> return devm_iio_deivce_register(...
Fixed in v4.
>
> If this all there is I'll fix it up.
Ok, so I'll send no more patches after v4 then?
> > +}
> > +
> > +static const struct of_device_id adc084s021_of_match[] = {
> > + { .compatible = "ti,adc084s021", },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, adc084s021_of_match);
> > +
> > +static const struct spi_device_id adc084s021_id[] = {
> > + { ADC084S021_DRIVER_NAME, 0},
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(spi, adc084s021_id);
> > +
> > +static struct spi_driver adc084s021_driver = {
> > + .driver = {
> > + .name = ADC084S021_DRIVER_NAME,
> > + .of_match_table = of_match_ptr(adc084s021_of_match),
> > + },
> > + .probe = adc084s021_probe,
> > + .id_table = adc084s021_id,
> > +};
> > +module_spi_driver(adc084s021_driver);
> > +
> > +MODULE_AUTHOR("Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>");
> > +MODULE_DESCRIPTION("Texas Instruments ADC084S021");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_VERSION("1.0");
> >
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] iio: adc: add driver for the ti-adc084s021 chip
@ 2017-05-09 15:17 ` Mårten Lindahl
0 siblings, 0 replies; 14+ messages in thread
From: Mårten Lindahl @ 2017-05-09 15:17 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Mårten Lindahl, knaack.h, lars, pmeerw, linux-iio, robh+dt,
mark.rutland, devicetree
On Sun, 2017-05-07 at 15:21 +0100, Jonathan Cameron wrote:
> On 03/05/17 13:01, Mårten Lindahl wrote:
> > From: Mårten Lindahl <martenli@axis.com>
> >
> > This adds support for the Texas Instruments ADC084S021 ADC chip.
> >
> > Signed-off-by: Mårten Lindahl <martenli@axis.com>
> A few more minor bits and pieces.
>
> Jonathan
Hi Jonathan!
I made v4. Please read my comments below. Could there potentially be
garbage in the buffer?
Thanks,
Mårten
> > ---
> > Changes in v3:
> > - Removed unnecessary comment about channel specification
> > - Skipped usage of 'address' in iio_chan_spec config macro
> > - Mask and shift channel readings only for _read_raw function
> > - Enable/disable regulator in _read_raw function
> > - Improved setup of ADC channel readings
> > - Use SPI config of speed_hz and bits_per_word
> > - Use devm_iio_triggered_buffer_setup and devm_iio_device_register
> > - Removed error message for failed devm_iio_device_register
> > - Removed driver _remove callback function
> >
> > Changes in v2:
> > - Removed most #defines in favor of inlines
> > - Corrected channel macro
> > - Removed configuration array with only one item
> > - Updated func adc084s021_adc_conversion to use be16_to_cpu
> > - Added IIO_CHAN_INFO_SCALE to func adc084s021_read_raw
> > - Use iio_device_claim_direct_mode in func adc084s021_read_raw
> > - Removed documentation for standard driver functions
> > - Changed retval to ret everywhere
> > - Removed dynamic alloc for data buffer in trigger handler
> > - Keeping mutex for all iterations in trigger handler
> > - Removed usage of events in this driver
> > - Removed info log in probe
> > - Use spi_message_init_with_transfers for spi message structs
> > - Use preenable and postdisable functions for regulator
> > - Inserted blank line before last return in all functions
> >
> > drivers/iio/adc/Kconfig | 12 ++
> > drivers/iio/adc/Makefile | 1 +
> > drivers/iio/adc/ti-adc084s021.c | 302 ++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 315 insertions(+)
> > create mode 100644 drivers/iio/adc/ti-adc084s021.c
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index dedae7a..13141e5 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -560,6 +560,18 @@ config TI_ADC0832
> > This driver can also be built as a module. If so, the module will be
> > called ti-adc0832.
> >
> > +config TI_ADC084S021
> > + tristate "Texas Instruments ADC084S021"
> > + depends on SPI
> > + select IIO_BUFFER
> > + select IIO_TRIGGERED_BUFFER
> > + help
> > + If you say yes here you get support for Texas Instruments ADC084S021
> > + chips.
> > +
> > + This driver can also be built as a module. If so, the module will be
> > + called ti-adc084s021.
> > +
> > config TI_ADC12138
> > tristate "Texas Instruments ADC12130/ADC12132/ADC12138"
> > depends on SPI
> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > index d001262..b1a6158 100644
> > --- a/drivers/iio/adc/Makefile
> > +++ b/drivers/iio/adc/Makefile
> > @@ -51,6 +51,7 @@ obj-$(CONFIG_STM32_ADC_CORE) += stm32-adc-core.o
> > obj-$(CONFIG_STM32_ADC) += stm32-adc.o
> > obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
> > obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
> > +obj-$(CONFIG_TI_ADC084S021) += ti-adc084s021.o
> > obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
> > obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
> > obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o
> > diff --git a/drivers/iio/adc/ti-adc084s021.c b/drivers/iio/adc/ti-adc084s021.c
> > new file mode 100644
> > index 0000000..f2fb0fa
> > --- /dev/null
> > +++ b/drivers/iio/adc/ti-adc084s021.c
> > @@ -0,0 +1,302 @@
> > +/**
> > + * Copyright (C) 2017 Axis Communications AB
> > + *
> > + * Driver for Texas Instruments' ADC084S021 ADC chip.
> > + * Datasheets can be found here:
> > + * http://www.ti.com/lit/ds/symlink/adc084s021.pdf
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/err.h>
> > +#include <linux/spi/spi.h>
> > +#include <linux/module.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/buffer.h>
> > +#include <linux/iio/triggered_buffer.h>
> > +#include <linux/iio/trigger_consumer.h>
> > +#include <linux/regulator/consumer.h>
> > +
> > +#define ADC084S021_DRIVER_NAME "adc084s021"
> > +
> > +struct adc084s021 {
> > + struct spi_device *spi;
> > + struct spi_message message;
> > + struct spi_transfer spi_trans;
> > + struct regulator *reg;
> > + struct mutex lock;
> > + /*
> > + * DMA (thus cache coherency maintenance) requires the
> > + * transfer buffers to live in their own cache lines.
> > + */
> > + u16 tx_buf[4] ____cacheline_aligned;
> > + __be16 rx_buf[5] ____cacheline_aligned; /* First 16-bits are trash */
> You only need to do this for tx_buf as then rx_buf will at worst end up
> in the same cacheline as it. Fairly implausible that this would cause
> issues given they will be read before a transfer and written after.
> Doing it twice results in potential waste of say 64 bytes depending on
> the cacheline length.
Fixed and verified with pahole in v4.
> > +};
> > +
> > +#define ADC084S021_VOLTAGE_CHANNEL(num) \
> > + { \
> > + .type = IIO_VOLTAGE, \
> > + .channel = (num), \
> > + .indexed = 1, \
> > + .scan_index = (num), \
> > + .scan_type = { \
> > + .sign = 'u', \
> > + .realbits = 8, \
> > + .storagebits = 16, \
> > + .shift = 4, \
> > + .endianness = IIO_BE, \
> They aren't at the moment as you are doing the endian conversion in kernel.
> Drop that in the push to buffers path.
Ok, fixed in "push to buffers path" in v4.
> > + }, \
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
> > + }
> > +
> > +static const struct iio_chan_spec adc084s021_channels[] = {
> > + ADC084S021_VOLTAGE_CHANNEL(0),
> > + ADC084S021_VOLTAGE_CHANNEL(1),
> > + ADC084S021_VOLTAGE_CHANNEL(2),
> > + ADC084S021_VOLTAGE_CHANNEL(3),
> > + IIO_CHAN_SOFT_TIMESTAMP(4),
> > +};
> > +
> > +static int adc084s021_power_on(struct adc084s021 *adc)
> > +{
> > + int ret;
> > +
> > + ret = regulator_enable(adc->reg);
> > + if (ret) {
> > + dev_warn(&adc->spi->dev,
> > + "Failed to enable supply voltage\n");
> > + }
> > +
> > + return ret;
> These two little functions do seem a little unnecessary.
> A regulator fail should be pretty unlikely so I'd be tempted
> to just have the regulator_enable / disable inline instead
> of off in these functions.
Fixed in v4.
> > +}
> > +
> > +static int adc084s021_power_off(struct adc084s021 *adc)
> > +{
> > + int ret;
> > +
> > + ret = regulator_disable(adc->reg);
> > + if (ret) {
> > + dev_warn(&adc->spi->dev,
> > + "Failed to disable supply voltage\n");
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * Read an ADC channel and return its value.
> > + *
> > + * @adc: The ADC SPI data.
> > + * @data: Buffer for converted data.
> > + */
> > +static int adc084s021_adc_conversion(struct adc084s021 *adc, void *data)
> > +{
> > + int n_words = (adc->spi_trans.len >> 1) - 1; /* Discard first word */
> > + int ret, i = 0;
> > + u16 *p = data;
> > +
> > + /* Do the transfer */
> > + ret = spi_sync(adc->spi, &adc->message);
> > + if (ret < 0)
> > + return ret;
> > +
> > + for (; i < n_words; i++)
> > + *(p + i) = be16_to_cpu(adc->rx_buf[i + 1]);
> Should only do the endian conversion in kernel if not
> using it for buffered output. In buffered output we can
> save a few cycles by leaving it up to userspace.
Ok, fixed in v4.
> > +
> > + return ret;
> > +}
> > +
> > +static int adc084s021_read_raw(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *channel, int *val,
> > + int *val2, long mask)
> > +{
> > + struct adc084s021 *adc = iio_priv(indio_dev);
> > + int ret;
> > +
> > + switch (mask) {
> > + case IIO_CHAN_INFO_RAW:
> > + ret = iio_device_claim_direct_mode(indio_dev);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = adc084s021_power_on(adc);
> > + if (ret) {
> > + iio_device_release_direct_mode(indio_dev);
> > + return ret;
> > + }
> > +
> > + adc->tx_buf[0] = channel->channel << 3;
> > + ret = adc084s021_adc_conversion(adc, val);
> > + iio_device_release_direct_mode(indio_dev);
> > + adc084s021_power_off(adc);
> > + if (ret < 0)
> > + return ret;
> > +
> > + *val = (*val >> channel->scan_type.shift) & 0xff;
> > +
> > + return IIO_VAL_INT;
> > + case IIO_CHAN_INFO_SCALE:
> > + ret = adc084s021_power_on(adc);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regulator_get_voltage(adc->reg);
> > + adc084s021_power_off(adc);
> > + if (ret < 0)
> > + return ret;
> > +
> > + *val = ret / 1000;
> > +
> > + return IIO_VAL_INT;
> > + default:
> > + return -EINVAL;
> > + }
> > +}
> > +
> > +/**
> > + * Read enabled ADC channels and push data to the buffer.
> > + *
> > + * @irq: The interrupt number (not used).
> > + * @pollfunc: Pointer to the poll func.
> > + */
> > +static irqreturn_t adc084s021_buffer_trigger_handler(int irq, void *pollfunc)
> > +{
> > + struct iio_poll_func *pf = pollfunc;
> > + struct iio_dev *indio_dev = pf->indio_dev;
> > + struct adc084s021 *adc = iio_priv(indio_dev);
> > + __be16 data[8] = {0}; /* 4 * 16-bit words of data + 8 bytes timestamp */
> > +
> > + mutex_lock(&adc->lock);
> > +
> > + if (adc084s021_adc_conversion(adc, &data) < 0)
> > + dev_err(&adc->spi->dev, "Failed to read data\n");
> hmm. Not sure I'm keen on pushing garbage to the buffer.
> I might fix this up as well... Depends on what else there is ;)
Since the __be16 data[8] array is initialized to 0, and the
adc084s021_adc_conversion function error returns before any data has
been copied to the array it should always contain either zeroes or valid
data as I see it. Would it be wrong to push zeroes?
> > +
> > + iio_push_to_buffers_with_timestamp(indio_dev, data,
> > + iio_get_time_ns(indio_dev));
> > + mutex_unlock(&adc->lock);
> > + iio_trigger_notify_done(indio_dev->trig);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int adc084s021_buffer_preenable(struct iio_dev *indio_dev)
> > +{
> > + struct adc084s021 *adc = iio_priv(indio_dev);
> > + int scan_index;
> > + int i = 0;
> > +
> > + for_each_set_bit(scan_index, indio_dev->active_scan_mask,
> > + indio_dev->masklength) {
> > + const struct iio_chan_spec *channel =
> > + &indio_dev->channels[scan_index];
> > + adc->tx_buf[i++] = channel->channel << 3;
> > + }
> > + adc->spi_trans.len = 2 + (i * sizeof(__be16)); /* Trash + channels */
> > +
> > + return adc084s021_power_on(adc);
> > +}
> > +
> > +static int adc084s021_buffer_postdisable(struct iio_dev *indio_dev)
> > +{
> > + struct adc084s021 *adc = iio_priv(indio_dev);
> > +
> > + adc->spi_trans.len = 4; /* Trash + single channel */
> > +
> > + return adc084s021_power_off(adc);
> > +}
> > +
> > +static const struct iio_info adc084s021_info = {
> > + .read_raw = adc084s021_read_raw,
> > + .driver_module = THIS_MODULE,
> > +};
> > +
> > +static const struct iio_buffer_setup_ops adc084s021_buffer_setup_ops = {
> > + .preenable = adc084s021_buffer_preenable,
> > + .postenable = iio_triggered_buffer_postenable,
> > + .predisable = iio_triggered_buffer_predisable,
> > + .postdisable = adc084s021_buffer_postdisable,
> > +};
> > +
> > +static int adc084s021_probe(struct spi_device *spi)
> > +{
> > + struct iio_dev *indio_dev;
> > + struct adc084s021 *adc;
> > + int ret;
> > +
> > + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
> > + if (!indio_dev) {
> > + dev_err(&spi->dev, "Failed to allocate IIO device\n");
> > + return -ENOMEM;
> > + }
> > +
> > + adc = iio_priv(indio_dev);
> > + adc->spi = spi;
> > +
> > + /* Connect the SPI device and the iio dev */
> > + spi_set_drvdata(spi, indio_dev);
> > +
> > + /* Initiate the Industrial I/O device */
> > + indio_dev->dev.parent = &spi->dev;
> > + indio_dev->dev.of_node = spi->dev.of_node;
> > + indio_dev->name = spi_get_device_id(spi)->name;
> > + indio_dev->modes = INDIO_DIRECT_MODE;
> > + indio_dev->info = &adc084s021_info;
> > + indio_dev->channels = adc084s021_channels;
> > + indio_dev->num_channels = ARRAY_SIZE(adc084s021_channels);
> > +
> > + /* Create SPI transfer for channel reads */
> > + adc->spi_trans.tx_buf = adc->tx_buf;
> > + adc->spi_trans.rx_buf = adc->rx_buf;
> > + adc->spi_trans.len = 4; /* Trash + single channel */
> > + spi_message_init_with_transfers(&adc->message, &adc->spi_trans, 1);
> > +
> > + adc->reg = devm_regulator_get(&spi->dev, "vref");
> > + if (IS_ERR(adc->reg))
> > + return PTR_ERR(adc->reg);
> > +
> > + mutex_init(&adc->lock);
> > +
> > + /* Setup triggered buffer with pollfunction */
> > + ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
> > + adc084s021_buffer_trigger_handler,
> > + &adc084s021_buffer_setup_ops);
> > + if (ret) {
> > + dev_err(&spi->dev, "Failed to setup triggered buffer\n");
> > + return ret;
> > + }
> > +
> > + ret = devm_iio_device_register(&spi->dev, indio_dev);
> > +
> > + return ret;
> return devm_iio_deivce_register(...
Fixed in v4.
>
> If this all there is I'll fix it up.
Ok, so I'll send no more patches after v4 then?
> > +}
> > +
> > +static const struct of_device_id adc084s021_of_match[] = {
> > + { .compatible = "ti,adc084s021", },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, adc084s021_of_match);
> > +
> > +static const struct spi_device_id adc084s021_id[] = {
> > + { ADC084S021_DRIVER_NAME, 0},
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(spi, adc084s021_id);
> > +
> > +static struct spi_driver adc084s021_driver = {
> > + .driver = {
> > + .name = ADC084S021_DRIVER_NAME,
> > + .of_match_table = of_match_ptr(adc084s021_of_match),
> > + },
> > + .probe = adc084s021_probe,
> > + .id_table = adc084s021_id,
> > +};
> > +module_spi_driver(adc084s021_driver);
> > +
> > +MODULE_AUTHOR("Mårten Lindahl <martenli@axis.com>");
> > +MODULE_DESCRIPTION("Texas Instruments ADC084S021");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_VERSION("1.0");
> >
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] iio: adc: add driver for the ti-adc084s021 chip
2017-05-09 15:17 ` Mårten Lindahl
@ 2017-05-14 15:37 ` Jonathan Cameron
-1 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2017-05-14 15:37 UTC (permalink / raw)
To: Mårten Lindahl
Cc: Mårten Lindahl, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
linux-iio-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA
On 09/05/17 16:17, Mårten Lindahl wrote:
> On Sun, 2017-05-07 at 15:21 +0100, Jonathan Cameron wrote:
>> On 03/05/17 13:01, Mårten Lindahl wrote:
>>> From: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
>>>
>>> This adds support for the Texas Instruments ADC084S021 ADC chip.
>>>
>>> Signed-off-by: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
>> A few more minor bits and pieces.
>>
>> Jonathan
> Hi Jonathan!
> I made v4. Please read my comments below. Could there potentially be
> garbage in the buffer?
> Thanks,
> Mårten
>>> ---
>>> Changes in v3:
>>> - Removed unnecessary comment about channel specification
>>> - Skipped usage of 'address' in iio_chan_spec config macro
>>> - Mask and shift channel readings only for _read_raw function
>>> - Enable/disable regulator in _read_raw function
>>> - Improved setup of ADC channel readings
>>> - Use SPI config of speed_hz and bits_per_word
>>> - Use devm_iio_triggered_buffer_setup and devm_iio_device_register
>>> - Removed error message for failed devm_iio_device_register
>>> - Removed driver _remove callback function
>>>
>>> Changes in v2:
>>> - Removed most #defines in favor of inlines
>>> - Corrected channel macro
>>> - Removed configuration array with only one item
>>> - Updated func adc084s021_adc_conversion to use be16_to_cpu
>>> - Added IIO_CHAN_INFO_SCALE to func adc084s021_read_raw
>>> - Use iio_device_claim_direct_mode in func adc084s021_read_raw
>>> - Removed documentation for standard driver functions
>>> - Changed retval to ret everywhere
>>> - Removed dynamic alloc for data buffer in trigger handler
>>> - Keeping mutex for all iterations in trigger handler
>>> - Removed usage of events in this driver
>>> - Removed info log in probe
>>> - Use spi_message_init_with_transfers for spi message structs
>>> - Use preenable and postdisable functions for regulator
>>> - Inserted blank line before last return in all functions
>>>
>>> drivers/iio/adc/Kconfig | 12 ++
>>> drivers/iio/adc/Makefile | 1 +
>>> drivers/iio/adc/ti-adc084s021.c | 302 ++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 315 insertions(+)
>>> create mode 100644 drivers/iio/adc/ti-adc084s021.c
>>>
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index dedae7a..13141e5 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -560,6 +560,18 @@ config TI_ADC0832
>>> This driver can also be built as a module. If so, the module will be
>>> called ti-adc0832.
>>>
>>> +config TI_ADC084S021
>>> + tristate "Texas Instruments ADC084S021"
>>> + depends on SPI
>>> + select IIO_BUFFER
>>> + select IIO_TRIGGERED_BUFFER
>>> + help
>>> + If you say yes here you get support for Texas Instruments ADC084S021
>>> + chips.
>>> +
>>> + This driver can also be built as a module. If so, the module will be
>>> + called ti-adc084s021.
>>> +
>>> config TI_ADC12138
>>> tristate "Texas Instruments ADC12130/ADC12132/ADC12138"
>>> depends on SPI
>>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>>> index d001262..b1a6158 100644
>>> --- a/drivers/iio/adc/Makefile
>>> +++ b/drivers/iio/adc/Makefile
>>> @@ -51,6 +51,7 @@ obj-$(CONFIG_STM32_ADC_CORE) += stm32-adc-core.o
>>> obj-$(CONFIG_STM32_ADC) += stm32-adc.o
>>> obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>>> obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
>>> +obj-$(CONFIG_TI_ADC084S021) += ti-adc084s021.o
>>> obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
>>> obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
>>> obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o
>>> diff --git a/drivers/iio/adc/ti-adc084s021.c b/drivers/iio/adc/ti-adc084s021.c
>>> new file mode 100644
>>> index 0000000..f2fb0fa
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/ti-adc084s021.c
>>> @@ -0,0 +1,302 @@
>>> +/**
>>> + * Copyright (C) 2017 Axis Communications AB
>>> + *
>>> + * Driver for Texas Instruments' ADC084S021 ADC chip.
>>> + * Datasheets can be found here:
>>> + * http://www.ti.com/lit/ds/symlink/adc084s021.pdf
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>> +#include <linux/err.h>
>>> +#include <linux/spi/spi.h>
>>> +#include <linux/module.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/iio/buffer.h>
>>> +#include <linux/iio/triggered_buffer.h>
>>> +#include <linux/iio/trigger_consumer.h>
>>> +#include <linux/regulator/consumer.h>
>>> +
>>> +#define ADC084S021_DRIVER_NAME "adc084s021"
>>> +
>>> +struct adc084s021 {
>>> + struct spi_device *spi;
>>> + struct spi_message message;
>>> + struct spi_transfer spi_trans;
>>> + struct regulator *reg;
>>> + struct mutex lock;
>>> + /*
>>> + * DMA (thus cache coherency maintenance) requires the
>>> + * transfer buffers to live in their own cache lines.
>>> + */
>>> + u16 tx_buf[4] ____cacheline_aligned;
>>> + __be16 rx_buf[5] ____cacheline_aligned; /* First 16-bits are trash */
>> You only need to do this for tx_buf as then rx_buf will at worst end up
>> in the same cacheline as it. Fairly implausible that this would cause
>> issues given they will be read before a transfer and written after.
>> Doing it twice results in potential waste of say 64 bytes depending on
>> the cacheline length.
> Fixed and verified with pahole in v4.
>>> +};
>>> +
>>> +#define ADC084S021_VOLTAGE_CHANNEL(num) \
>>> + { \
>>> + .type = IIO_VOLTAGE, \
>>> + .channel = (num), \
>>> + .indexed = 1, \
>>> + .scan_index = (num), \
>>> + .scan_type = { \
>>> + .sign = 'u', \
>>> + .realbits = 8, \
>>> + .storagebits = 16, \
>>> + .shift = 4, \
>>> + .endianness = IIO_BE, \
>> They aren't at the moment as you are doing the endian conversion in kernel.
>> Drop that in the push to buffers path.
> Ok, fixed in "push to buffers path" in v4.
>>> + }, \
>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
>>> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
>>> + }
>>> +
>>> +static const struct iio_chan_spec adc084s021_channels[] = {
>>> + ADC084S021_VOLTAGE_CHANNEL(0),
>>> + ADC084S021_VOLTAGE_CHANNEL(1),
>>> + ADC084S021_VOLTAGE_CHANNEL(2),
>>> + ADC084S021_VOLTAGE_CHANNEL(3),
>>> + IIO_CHAN_SOFT_TIMESTAMP(4),
>>> +};
>>> +
>>> +static int adc084s021_power_on(struct adc084s021 *adc)
>>> +{
>>> + int ret;
>>> +
>>> + ret = regulator_enable(adc->reg);
>>> + if (ret) {
>>> + dev_warn(&adc->spi->dev,
>>> + "Failed to enable supply voltage\n");
>>> + }
>>> +
>>> + return ret;
>> These two little functions do seem a little unnecessary.
>> A regulator fail should be pretty unlikely so I'd be tempted
>> to just have the regulator_enable / disable inline instead
>> of off in these functions.
> Fixed in v4.
>>> +}
>>> +
>>> +static int adc084s021_power_off(struct adc084s021 *adc)
>>> +{
>>> + int ret;
>>> +
>>> + ret = regulator_disable(adc->reg);
>>> + if (ret) {
>>> + dev_warn(&adc->spi->dev,
>>> + "Failed to disable supply voltage\n");
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +/**
>>> + * Read an ADC channel and return its value.
>>> + *
>>> + * @adc: The ADC SPI data.
>>> + * @data: Buffer for converted data.
>>> + */
>>> +static int adc084s021_adc_conversion(struct adc084s021 *adc, void *data)
>>> +{
>>> + int n_words = (adc->spi_trans.len >> 1) - 1; /* Discard first word */
>>> + int ret, i = 0;
>>> + u16 *p = data;
>>> +
>>> + /* Do the transfer */
>>> + ret = spi_sync(adc->spi, &adc->message);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + for (; i < n_words; i++)
>>> + *(p + i) = be16_to_cpu(adc->rx_buf[i + 1]);
>> Should only do the endian conversion in kernel if not
>> using it for buffered output. In buffered output we can
>> save a few cycles by leaving it up to userspace.
> Ok, fixed in v4.
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int adc084s021_read_raw(struct iio_dev *indio_dev,
>>> + struct iio_chan_spec const *channel, int *val,
>>> + int *val2, long mask)
>>> +{
>>> + struct adc084s021 *adc = iio_priv(indio_dev);
>>> + int ret;
>>> +
>>> + switch (mask) {
>>> + case IIO_CHAN_INFO_RAW:
>>> + ret = iio_device_claim_direct_mode(indio_dev);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + ret = adc084s021_power_on(adc);
>>> + if (ret) {
>>> + iio_device_release_direct_mode(indio_dev);
>>> + return ret;
>>> + }
>>> +
>>> + adc->tx_buf[0] = channel->channel << 3;
>>> + ret = adc084s021_adc_conversion(adc, val);
>>> + iio_device_release_direct_mode(indio_dev);
>>> + adc084s021_power_off(adc);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + *val = (*val >> channel->scan_type.shift) & 0xff;
>>> +
>>> + return IIO_VAL_INT;
>>> + case IIO_CHAN_INFO_SCALE:
>>> + ret = adc084s021_power_on(adc);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = regulator_get_voltage(adc->reg);
>>> + adc084s021_power_off(adc);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + *val = ret / 1000;
>>> +
>>> + return IIO_VAL_INT;
>>> + default:
>>> + return -EINVAL;
>>> + }
>>> +}
>>> +
>>> +/**
>>> + * Read enabled ADC channels and push data to the buffer.
>>> + *
>>> + * @irq: The interrupt number (not used).
>>> + * @pollfunc: Pointer to the poll func.
>>> + */
>>> +static irqreturn_t adc084s021_buffer_trigger_handler(int irq, void *pollfunc)
>>> +{
>>> + struct iio_poll_func *pf = pollfunc;
>>> + struct iio_dev *indio_dev = pf->indio_dev;
>>> + struct adc084s021 *adc = iio_priv(indio_dev);
>>> + __be16 data[8] = {0}; /* 4 * 16-bit words of data + 8 bytes timestamp */
>>> +
>>> + mutex_lock(&adc->lock);
>>> +
>>> + if (adc084s021_adc_conversion(adc, &data) < 0)
>>> + dev_err(&adc->spi->dev, "Failed to read data\n");
>> hmm. Not sure I'm keen on pushing garbage to the buffer.
>> I might fix this up as well... Depends on what else there is ;)
> Since the __be16 data[8] array is initialized to 0, and the
> adc084s021_adc_conversion function error returns before any data has
> been copied to the array it should always contain either zeroes or valid
> data as I see it. Would it be wrong to push zeroes?
Yes from the point of view that userspace might think it was actual
data. Not critical in an error path though I suppose.
>>> +
>>> + iio_push_to_buffers_with_timestamp(indio_dev, data,
>>> + iio_get_time_ns(indio_dev));
>>> + mutex_unlock(&adc->lock);
>>> + iio_trigger_notify_done(indio_dev->trig);
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +static int adc084s021_buffer_preenable(struct iio_dev *indio_dev)
>>> +{
>>> + struct adc084s021 *adc = iio_priv(indio_dev);
>>> + int scan_index;
>>> + int i = 0;
>>> +
>>> + for_each_set_bit(scan_index, indio_dev->active_scan_mask,
>>> + indio_dev->masklength) {
>>> + const struct iio_chan_spec *channel =
>>> + &indio_dev->channels[scan_index];
>>> + adc->tx_buf[i++] = channel->channel << 3;
>>> + }
>>> + adc->spi_trans.len = 2 + (i * sizeof(__be16)); /* Trash + channels */
>>> +
>>> + return adc084s021_power_on(adc);
>>> +}
>>> +
>>> +static int adc084s021_buffer_postdisable(struct iio_dev *indio_dev)
>>> +{
>>> + struct adc084s021 *adc = iio_priv(indio_dev);
>>> +
>>> + adc->spi_trans.len = 4; /* Trash + single channel */
>>> +
>>> + return adc084s021_power_off(adc);
>>> +}
>>> +
>>> +static const struct iio_info adc084s021_info = {
>>> + .read_raw = adc084s021_read_raw,
>>> + .driver_module = THIS_MODULE,
>>> +};
>>> +
>>> +static const struct iio_buffer_setup_ops adc084s021_buffer_setup_ops = {
>>> + .preenable = adc084s021_buffer_preenable,
>>> + .postenable = iio_triggered_buffer_postenable,
>>> + .predisable = iio_triggered_buffer_predisable,
>>> + .postdisable = adc084s021_buffer_postdisable,
>>> +};
>>> +
>>> +static int adc084s021_probe(struct spi_device *spi)
>>> +{
>>> + struct iio_dev *indio_dev;
>>> + struct adc084s021 *adc;
>>> + int ret;
>>> +
>>> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
>>> + if (!indio_dev) {
>>> + dev_err(&spi->dev, "Failed to allocate IIO device\n");
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + adc = iio_priv(indio_dev);
>>> + adc->spi = spi;
>>> +
>>> + /* Connect the SPI device and the iio dev */
>>> + spi_set_drvdata(spi, indio_dev);
>>> +
>>> + /* Initiate the Industrial I/O device */
>>> + indio_dev->dev.parent = &spi->dev;
>>> + indio_dev->dev.of_node = spi->dev.of_node;
>>> + indio_dev->name = spi_get_device_id(spi)->name;
>>> + indio_dev->modes = INDIO_DIRECT_MODE;
>>> + indio_dev->info = &adc084s021_info;
>>> + indio_dev->channels = adc084s021_channels;
>>> + indio_dev->num_channels = ARRAY_SIZE(adc084s021_channels);
>>> +
>>> + /* Create SPI transfer for channel reads */
>>> + adc->spi_trans.tx_buf = adc->tx_buf;
>>> + adc->spi_trans.rx_buf = adc->rx_buf;
>>> + adc->spi_trans.len = 4; /* Trash + single channel */
>>> + spi_message_init_with_transfers(&adc->message, &adc->spi_trans, 1);
>>> +
>>> + adc->reg = devm_regulator_get(&spi->dev, "vref");
>>> + if (IS_ERR(adc->reg))
>>> + return PTR_ERR(adc->reg);
>>> +
>>> + mutex_init(&adc->lock);
>>> +
>>> + /* Setup triggered buffer with pollfunction */
>>> + ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
>>> + adc084s021_buffer_trigger_handler,
>>> + &adc084s021_buffer_setup_ops);
>>> + if (ret) {
>>> + dev_err(&spi->dev, "Failed to setup triggered buffer\n");
>>> + return ret;
>>> + }
>>> +
>>> + ret = devm_iio_device_register(&spi->dev, indio_dev);
>>> +
>>> + return ret;
>> return devm_iio_deivce_register(...
> Fixed in v4.
>>
>> If this all there is I'll fix it up.
> Ok, so I'll send no more patches after v4 then?
>>> +}
>>> +
>>> +static const struct of_device_id adc084s021_of_match[] = {
>>> + { .compatible = "ti,adc084s021", },
>>> + {},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, adc084s021_of_match);
>>> +
>>> +static const struct spi_device_id adc084s021_id[] = {
>>> + { ADC084S021_DRIVER_NAME, 0},
>>> + {}
>>> +};
>>> +MODULE_DEVICE_TABLE(spi, adc084s021_id);
>>> +
>>> +static struct spi_driver adc084s021_driver = {
>>> + .driver = {
>>> + .name = ADC084S021_DRIVER_NAME,
>>> + .of_match_table = of_match_ptr(adc084s021_of_match),
>>> + },
>>> + .probe = adc084s021_probe,
>>> + .id_table = adc084s021_id,
>>> +};
>>> +module_spi_driver(adc084s021_driver);
>>> +
>>> +MODULE_AUTHOR("Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>");
>>> +MODULE_DESCRIPTION("Texas Instruments ADC084S021");
>>> +MODULE_LICENSE("GPL v2");
>>> +MODULE_VERSION("1.0");
>>>
>>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] iio: adc: add driver for the ti-adc084s021 chip
@ 2017-05-14 15:37 ` Jonathan Cameron
0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2017-05-14 15:37 UTC (permalink / raw)
To: Mårten Lindahl
Cc: Mårten Lindahl, knaack.h, lars, pmeerw, linux-iio, robh+dt,
mark.rutland, devicetree
On 09/05/17 16:17, Mårten Lindahl wrote:
> On Sun, 2017-05-07 at 15:21 +0100, Jonathan Cameron wrote:
>> On 03/05/17 13:01, Mårten Lindahl wrote:
>>> From: Mårten Lindahl <martenli@axis.com>
>>>
>>> This adds support for the Texas Instruments ADC084S021 ADC chip.
>>>
>>> Signed-off-by: Mårten Lindahl <martenli@axis.com>
>> A few more minor bits and pieces.
>>
>> Jonathan
> Hi Jonathan!
> I made v4. Please read my comments below. Could there potentially be
> garbage in the buffer?
> Thanks,
> Mårten
>>> ---
>>> Changes in v3:
>>> - Removed unnecessary comment about channel specification
>>> - Skipped usage of 'address' in iio_chan_spec config macro
>>> - Mask and shift channel readings only for _read_raw function
>>> - Enable/disable regulator in _read_raw function
>>> - Improved setup of ADC channel readings
>>> - Use SPI config of speed_hz and bits_per_word
>>> - Use devm_iio_triggered_buffer_setup and devm_iio_device_register
>>> - Removed error message for failed devm_iio_device_register
>>> - Removed driver _remove callback function
>>>
>>> Changes in v2:
>>> - Removed most #defines in favor of inlines
>>> - Corrected channel macro
>>> - Removed configuration array with only one item
>>> - Updated func adc084s021_adc_conversion to use be16_to_cpu
>>> - Added IIO_CHAN_INFO_SCALE to func adc084s021_read_raw
>>> - Use iio_device_claim_direct_mode in func adc084s021_read_raw
>>> - Removed documentation for standard driver functions
>>> - Changed retval to ret everywhere
>>> - Removed dynamic alloc for data buffer in trigger handler
>>> - Keeping mutex for all iterations in trigger handler
>>> - Removed usage of events in this driver
>>> - Removed info log in probe
>>> - Use spi_message_init_with_transfers for spi message structs
>>> - Use preenable and postdisable functions for regulator
>>> - Inserted blank line before last return in all functions
>>>
>>> drivers/iio/adc/Kconfig | 12 ++
>>> drivers/iio/adc/Makefile | 1 +
>>> drivers/iio/adc/ti-adc084s021.c | 302 ++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 315 insertions(+)
>>> create mode 100644 drivers/iio/adc/ti-adc084s021.c
>>>
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index dedae7a..13141e5 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -560,6 +560,18 @@ config TI_ADC0832
>>> This driver can also be built as a module. If so, the module will be
>>> called ti-adc0832.
>>>
>>> +config TI_ADC084S021
>>> + tristate "Texas Instruments ADC084S021"
>>> + depends on SPI
>>> + select IIO_BUFFER
>>> + select IIO_TRIGGERED_BUFFER
>>> + help
>>> + If you say yes here you get support for Texas Instruments ADC084S021
>>> + chips.
>>> +
>>> + This driver can also be built as a module. If so, the module will be
>>> + called ti-adc084s021.
>>> +
>>> config TI_ADC12138
>>> tristate "Texas Instruments ADC12130/ADC12132/ADC12138"
>>> depends on SPI
>>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>>> index d001262..b1a6158 100644
>>> --- a/drivers/iio/adc/Makefile
>>> +++ b/drivers/iio/adc/Makefile
>>> @@ -51,6 +51,7 @@ obj-$(CONFIG_STM32_ADC_CORE) += stm32-adc-core.o
>>> obj-$(CONFIG_STM32_ADC) += stm32-adc.o
>>> obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>>> obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
>>> +obj-$(CONFIG_TI_ADC084S021) += ti-adc084s021.o
>>> obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
>>> obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
>>> obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o
>>> diff --git a/drivers/iio/adc/ti-adc084s021.c b/drivers/iio/adc/ti-adc084s021.c
>>> new file mode 100644
>>> index 0000000..f2fb0fa
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/ti-adc084s021.c
>>> @@ -0,0 +1,302 @@
>>> +/**
>>> + * Copyright (C) 2017 Axis Communications AB
>>> + *
>>> + * Driver for Texas Instruments' ADC084S021 ADC chip.
>>> + * Datasheets can be found here:
>>> + * http://www.ti.com/lit/ds/symlink/adc084s021.pdf
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>> +#include <linux/err.h>
>>> +#include <linux/spi/spi.h>
>>> +#include <linux/module.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/iio/buffer.h>
>>> +#include <linux/iio/triggered_buffer.h>
>>> +#include <linux/iio/trigger_consumer.h>
>>> +#include <linux/regulator/consumer.h>
>>> +
>>> +#define ADC084S021_DRIVER_NAME "adc084s021"
>>> +
>>> +struct adc084s021 {
>>> + struct spi_device *spi;
>>> + struct spi_message message;
>>> + struct spi_transfer spi_trans;
>>> + struct regulator *reg;
>>> + struct mutex lock;
>>> + /*
>>> + * DMA (thus cache coherency maintenance) requires the
>>> + * transfer buffers to live in their own cache lines.
>>> + */
>>> + u16 tx_buf[4] ____cacheline_aligned;
>>> + __be16 rx_buf[5] ____cacheline_aligned; /* First 16-bits are trash */
>> You only need to do this for tx_buf as then rx_buf will at worst end up
>> in the same cacheline as it. Fairly implausible that this would cause
>> issues given they will be read before a transfer and written after.
>> Doing it twice results in potential waste of say 64 bytes depending on
>> the cacheline length.
> Fixed and verified with pahole in v4.
>>> +};
>>> +
>>> +#define ADC084S021_VOLTAGE_CHANNEL(num) \
>>> + { \
>>> + .type = IIO_VOLTAGE, \
>>> + .channel = (num), \
>>> + .indexed = 1, \
>>> + .scan_index = (num), \
>>> + .scan_type = { \
>>> + .sign = 'u', \
>>> + .realbits = 8, \
>>> + .storagebits = 16, \
>>> + .shift = 4, \
>>> + .endianness = IIO_BE, \
>> They aren't at the moment as you are doing the endian conversion in kernel.
>> Drop that in the push to buffers path.
> Ok, fixed in "push to buffers path" in v4.
>>> + }, \
>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
>>> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
>>> + }
>>> +
>>> +static const struct iio_chan_spec adc084s021_channels[] = {
>>> + ADC084S021_VOLTAGE_CHANNEL(0),
>>> + ADC084S021_VOLTAGE_CHANNEL(1),
>>> + ADC084S021_VOLTAGE_CHANNEL(2),
>>> + ADC084S021_VOLTAGE_CHANNEL(3),
>>> + IIO_CHAN_SOFT_TIMESTAMP(4),
>>> +};
>>> +
>>> +static int adc084s021_power_on(struct adc084s021 *adc)
>>> +{
>>> + int ret;
>>> +
>>> + ret = regulator_enable(adc->reg);
>>> + if (ret) {
>>> + dev_warn(&adc->spi->dev,
>>> + "Failed to enable supply voltage\n");
>>> + }
>>> +
>>> + return ret;
>> These two little functions do seem a little unnecessary.
>> A regulator fail should be pretty unlikely so I'd be tempted
>> to just have the regulator_enable / disable inline instead
>> of off in these functions.
> Fixed in v4.
>>> +}
>>> +
>>> +static int adc084s021_power_off(struct adc084s021 *adc)
>>> +{
>>> + int ret;
>>> +
>>> + ret = regulator_disable(adc->reg);
>>> + if (ret) {
>>> + dev_warn(&adc->spi->dev,
>>> + "Failed to disable supply voltage\n");
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +/**
>>> + * Read an ADC channel and return its value.
>>> + *
>>> + * @adc: The ADC SPI data.
>>> + * @data: Buffer for converted data.
>>> + */
>>> +static int adc084s021_adc_conversion(struct adc084s021 *adc, void *data)
>>> +{
>>> + int n_words = (adc->spi_trans.len >> 1) - 1; /* Discard first word */
>>> + int ret, i = 0;
>>> + u16 *p = data;
>>> +
>>> + /* Do the transfer */
>>> + ret = spi_sync(adc->spi, &adc->message);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + for (; i < n_words; i++)
>>> + *(p + i) = be16_to_cpu(adc->rx_buf[i + 1]);
>> Should only do the endian conversion in kernel if not
>> using it for buffered output. In buffered output we can
>> save a few cycles by leaving it up to userspace.
> Ok, fixed in v4.
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int adc084s021_read_raw(struct iio_dev *indio_dev,
>>> + struct iio_chan_spec const *channel, int *val,
>>> + int *val2, long mask)
>>> +{
>>> + struct adc084s021 *adc = iio_priv(indio_dev);
>>> + int ret;
>>> +
>>> + switch (mask) {
>>> + case IIO_CHAN_INFO_RAW:
>>> + ret = iio_device_claim_direct_mode(indio_dev);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + ret = adc084s021_power_on(adc);
>>> + if (ret) {
>>> + iio_device_release_direct_mode(indio_dev);
>>> + return ret;
>>> + }
>>> +
>>> + adc->tx_buf[0] = channel->channel << 3;
>>> + ret = adc084s021_adc_conversion(adc, val);
>>> + iio_device_release_direct_mode(indio_dev);
>>> + adc084s021_power_off(adc);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + *val = (*val >> channel->scan_type.shift) & 0xff;
>>> +
>>> + return IIO_VAL_INT;
>>> + case IIO_CHAN_INFO_SCALE:
>>> + ret = adc084s021_power_on(adc);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = regulator_get_voltage(adc->reg);
>>> + adc084s021_power_off(adc);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + *val = ret / 1000;
>>> +
>>> + return IIO_VAL_INT;
>>> + default:
>>> + return -EINVAL;
>>> + }
>>> +}
>>> +
>>> +/**
>>> + * Read enabled ADC channels and push data to the buffer.
>>> + *
>>> + * @irq: The interrupt number (not used).
>>> + * @pollfunc: Pointer to the poll func.
>>> + */
>>> +static irqreturn_t adc084s021_buffer_trigger_handler(int irq, void *pollfunc)
>>> +{
>>> + struct iio_poll_func *pf = pollfunc;
>>> + struct iio_dev *indio_dev = pf->indio_dev;
>>> + struct adc084s021 *adc = iio_priv(indio_dev);
>>> + __be16 data[8] = {0}; /* 4 * 16-bit words of data + 8 bytes timestamp */
>>> +
>>> + mutex_lock(&adc->lock);
>>> +
>>> + if (adc084s021_adc_conversion(adc, &data) < 0)
>>> + dev_err(&adc->spi->dev, "Failed to read data\n");
>> hmm. Not sure I'm keen on pushing garbage to the buffer.
>> I might fix this up as well... Depends on what else there is ;)
> Since the __be16 data[8] array is initialized to 0, and the
> adc084s021_adc_conversion function error returns before any data has
> been copied to the array it should always contain either zeroes or valid
> data as I see it. Would it be wrong to push zeroes?
Yes from the point of view that userspace might think it was actual
data. Not critical in an error path though I suppose.
>>> +
>>> + iio_push_to_buffers_with_timestamp(indio_dev, data,
>>> + iio_get_time_ns(indio_dev));
>>> + mutex_unlock(&adc->lock);
>>> + iio_trigger_notify_done(indio_dev->trig);
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +static int adc084s021_buffer_preenable(struct iio_dev *indio_dev)
>>> +{
>>> + struct adc084s021 *adc = iio_priv(indio_dev);
>>> + int scan_index;
>>> + int i = 0;
>>> +
>>> + for_each_set_bit(scan_index, indio_dev->active_scan_mask,
>>> + indio_dev->masklength) {
>>> + const struct iio_chan_spec *channel =
>>> + &indio_dev->channels[scan_index];
>>> + adc->tx_buf[i++] = channel->channel << 3;
>>> + }
>>> + adc->spi_trans.len = 2 + (i * sizeof(__be16)); /* Trash + channels */
>>> +
>>> + return adc084s021_power_on(adc);
>>> +}
>>> +
>>> +static int adc084s021_buffer_postdisable(struct iio_dev *indio_dev)
>>> +{
>>> + struct adc084s021 *adc = iio_priv(indio_dev);
>>> +
>>> + adc->spi_trans.len = 4; /* Trash + single channel */
>>> +
>>> + return adc084s021_power_off(adc);
>>> +}
>>> +
>>> +static const struct iio_info adc084s021_info = {
>>> + .read_raw = adc084s021_read_raw,
>>> + .driver_module = THIS_MODULE,
>>> +};
>>> +
>>> +static const struct iio_buffer_setup_ops adc084s021_buffer_setup_ops = {
>>> + .preenable = adc084s021_buffer_preenable,
>>> + .postenable = iio_triggered_buffer_postenable,
>>> + .predisable = iio_triggered_buffer_predisable,
>>> + .postdisable = adc084s021_buffer_postdisable,
>>> +};
>>> +
>>> +static int adc084s021_probe(struct spi_device *spi)
>>> +{
>>> + struct iio_dev *indio_dev;
>>> + struct adc084s021 *adc;
>>> + int ret;
>>> +
>>> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
>>> + if (!indio_dev) {
>>> + dev_err(&spi->dev, "Failed to allocate IIO device\n");
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + adc = iio_priv(indio_dev);
>>> + adc->spi = spi;
>>> +
>>> + /* Connect the SPI device and the iio dev */
>>> + spi_set_drvdata(spi, indio_dev);
>>> +
>>> + /* Initiate the Industrial I/O device */
>>> + indio_dev->dev.parent = &spi->dev;
>>> + indio_dev->dev.of_node = spi->dev.of_node;
>>> + indio_dev->name = spi_get_device_id(spi)->name;
>>> + indio_dev->modes = INDIO_DIRECT_MODE;
>>> + indio_dev->info = &adc084s021_info;
>>> + indio_dev->channels = adc084s021_channels;
>>> + indio_dev->num_channels = ARRAY_SIZE(adc084s021_channels);
>>> +
>>> + /* Create SPI transfer for channel reads */
>>> + adc->spi_trans.tx_buf = adc->tx_buf;
>>> + adc->spi_trans.rx_buf = adc->rx_buf;
>>> + adc->spi_trans.len = 4; /* Trash + single channel */
>>> + spi_message_init_with_transfers(&adc->message, &adc->spi_trans, 1);
>>> +
>>> + adc->reg = devm_regulator_get(&spi->dev, "vref");
>>> + if (IS_ERR(adc->reg))
>>> + return PTR_ERR(adc->reg);
>>> +
>>> + mutex_init(&adc->lock);
>>> +
>>> + /* Setup triggered buffer with pollfunction */
>>> + ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
>>> + adc084s021_buffer_trigger_handler,
>>> + &adc084s021_buffer_setup_ops);
>>> + if (ret) {
>>> + dev_err(&spi->dev, "Failed to setup triggered buffer\n");
>>> + return ret;
>>> + }
>>> +
>>> + ret = devm_iio_device_register(&spi->dev, indio_dev);
>>> +
>>> + return ret;
>> return devm_iio_deivce_register(...
> Fixed in v4.
>>
>> If this all there is I'll fix it up.
> Ok, so I'll send no more patches after v4 then?
>>> +}
>>> +
>>> +static const struct of_device_id adc084s021_of_match[] = {
>>> + { .compatible = "ti,adc084s021", },
>>> + {},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, adc084s021_of_match);
>>> +
>>> +static const struct spi_device_id adc084s021_id[] = {
>>> + { ADC084S021_DRIVER_NAME, 0},
>>> + {}
>>> +};
>>> +MODULE_DEVICE_TABLE(spi, adc084s021_id);
>>> +
>>> +static struct spi_driver adc084s021_driver = {
>>> + .driver = {
>>> + .name = ADC084S021_DRIVER_NAME,
>>> + .of_match_table = of_match_ptr(adc084s021_of_match),
>>> + },
>>> + .probe = adc084s021_probe,
>>> + .id_table = adc084s021_id,
>>> +};
>>> +module_spi_driver(adc084s021_driver);
>>> +
>>> +MODULE_AUTHOR("Mårten Lindahl <martenli@axis.com>");
>>> +MODULE_DESCRIPTION("Texas Instruments ADC084S021");
>>> +MODULE_LICENSE("GPL v2");
>>> +MODULE_VERSION("1.0");
>>>
>>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-05-14 15:37 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-03 12:01 [PATCH v3 0/2] add driver for the ti-adc084s021 chip Mårten Lindahl
2017-05-03 12:01 ` Mårten Lindahl
[not found] ` <1493812905-28904-1-git-send-email-marten.lindahl-VrBV9hrLPhE@public.gmane.org>
2017-05-03 12:01 ` [PATCH v3 1/2] dt-bindings: iio: adc: " Mårten Lindahl
2017-05-03 12:01 ` Mårten Lindahl
[not found] ` <1493812905-28904-2-git-send-email-marten.lindahl-VrBV9hrLPhE@public.gmane.org>
2017-05-08 16:25 ` Rob Herring
2017-05-08 16:25 ` Rob Herring
2017-05-03 12:01 ` [PATCH v3 2/2] " Mårten Lindahl
2017-05-03 12:01 ` Mårten Lindahl
[not found] ` <1493812905-28904-3-git-send-email-marten.lindahl-VrBV9hrLPhE@public.gmane.org>
2017-05-07 14:21 ` Jonathan Cameron
2017-05-07 14:21 ` Jonathan Cameron
[not found] ` <fb177b86-3e28-5051-67cf-bcc81ae0e116-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2017-05-09 15:17 ` Mårten Lindahl
2017-05-09 15:17 ` Mårten Lindahl
[not found] ` <1494343072.18621.13.camel-VrBV9hrLPhE@public.gmane.org>
2017-05-14 15:37 ` Jonathan Cameron
2017-05-14 15:37 ` Jonathan Cameron
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.