2016-11-11 06:35:46

by Eva Rachel Retuya

[permalink] [raw]
Subject: [PATCH 0/2] staging: iio: ad7606: move driver out of staging

Address the last remaining TODO [1] for this driver and move it from staging
into mainline.

[1] https://marc.info/?l=linux-iio&m=147689684332118&w=2

Eva Rachel Retuya (2):
staging: iio: ad7606: replace range/range_available with corresponding
scale
staging: iio: ad7606: move out of staging

drivers/iio/adc/Kconfig | 34 +++
drivers/iio/adc/Makefile | 3 +
drivers/iio/adc/ad7606.c | 519 +++++++++++++++++++++++++++++++++
drivers/iio/adc/ad7606.h | 78 +++++
drivers/iio/adc/ad7606_par.c | 112 ++++++++
drivers/iio/adc/ad7606_spi.c | 78 +++++
drivers/staging/iio/adc/Kconfig | 34 ---
drivers/staging/iio/adc/Makefile | 4 -
drivers/staging/iio/adc/ad7606.c | 543 -----------------------------------
drivers/staging/iio/adc/ad7606.h | 78 -----
drivers/staging/iio/adc/ad7606_par.c | 112 --------
drivers/staging/iio/adc/ad7606_spi.c | 78 -----
12 files changed, 824 insertions(+), 849 deletions(-)
create mode 100644 drivers/iio/adc/ad7606.c
create mode 100644 drivers/iio/adc/ad7606.h
create mode 100644 drivers/iio/adc/ad7606_par.c
create mode 100644 drivers/iio/adc/ad7606_spi.c
delete mode 100644 drivers/staging/iio/adc/ad7606.c
delete mode 100644 drivers/staging/iio/adc/ad7606.h
delete mode 100644 drivers/staging/iio/adc/ad7606_par.c
delete mode 100644 drivers/staging/iio/adc/ad7606_spi.c

--
2.7.4


2016-11-11 06:35:51

by Eva Rachel Retuya

[permalink] [raw]
Subject: [PATCH 1/2] staging: iio: ad7606: replace range/range_available with corresponding scale

Eliminate the non-standard attribute in_voltage_range and move its
functionality under the scale attribute. read_raw() has been taken care
of previously so only write_raw() is handled here.

Additionally, rename the attribute in_voltage_range_available into
in_voltage_scale_available.

Suggested-by: Lars-Peter Clausen <[email protected]>
Signed-off-by: Eva Rachel Retuya <[email protected]>
---
drivers/staging/iio/adc/ad7606.c | 56 ++++++++++++----------------------------
1 file changed, 16 insertions(+), 40 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 4531908..cceb18c 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -161,42 +161,7 @@ static int ad7606_read_raw(struct iio_dev *indio_dev,
return -EINVAL;
}

-static ssize_t ad7606_show_range(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct iio_dev *indio_dev = dev_to_iio_dev(dev);
- struct ad7606_state *st = iio_priv(indio_dev);
-
- return sprintf(buf, "%u\n", st->range);
-}
-
-static ssize_t ad7606_store_range(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
-{
- struct iio_dev *indio_dev = dev_to_iio_dev(dev);
- struct ad7606_state *st = iio_priv(indio_dev);
- unsigned long lval;
- int ret;
-
- ret = kstrtoul(buf, 10, &lval);
- if (ret)
- return ret;
-
- if (!(lval == 5000 || lval == 10000))
- return -EINVAL;
-
- mutex_lock(&indio_dev->mlock);
- gpiod_set_value(st->gpio_range, lval == 10000);
- st->range = lval;
- mutex_unlock(&indio_dev->mlock);
-
- return count;
-}
-
-static IIO_DEVICE_ATTR(in_voltage_range, S_IRUGO | S_IWUSR,
- ad7606_show_range, ad7606_store_range, 0);
-static IIO_CONST_ATTR(in_voltage_range_available, "5000 10000");
+static IIO_CONST_ATTR(in_voltage_scale_available, "5000 10000");

static int ad7606_oversampling_get_index(unsigned int val)
{
@@ -221,6 +186,19 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
int ret;

switch (mask) {
+ case IIO_CHAN_INFO_SCALE:
+ if (val2)
+ return -EINVAL;
+
+ if (!(val == 5000 || val == 10000))
+ return -EINVAL;
+
+ mutex_lock(&indio_dev->mlock);
+ gpiod_set_value(st->gpio_range, val == 10000);
+ st->range = val;
+ mutex_unlock(&indio_dev->mlock);
+
+ return 0;
case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
if (val2)
return -EINVAL;
@@ -247,8 +225,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
static IIO_CONST_ATTR(oversampling_ratio_available, "1 2 4 8 16 32 64");

static struct attribute *ad7606_attributes_os_and_range[] = {
- &iio_dev_attr_in_voltage_range.dev_attr.attr,
- &iio_const_attr_in_voltage_range_available.dev_attr.attr,
+ &iio_const_attr_in_voltage_scale_available.dev_attr.attr,
&iio_const_attr_oversampling_ratio_available.dev_attr.attr,
NULL,
};
@@ -267,8 +244,7 @@ static const struct attribute_group ad7606_attribute_group_os = {
};

static struct attribute *ad7606_attributes_range[] = {
- &iio_dev_attr_in_voltage_range.dev_attr.attr,
- &iio_const_attr_in_voltage_range_available.dev_attr.attr,
+ &iio_const_attr_in_voltage_scale_available.dev_attr.attr,
NULL,
};

--
2.7.4

2016-11-11 06:36:01

by Eva Rachel Retuya

[permalink] [raw]
Subject: [PATCH 2/2] staging: iio: ad7606: move out of staging

Move the ad7606 driver from staging/iio/adc to iio/adc. Also, update the
corresponding Makefile and Kconfig associated with the change.

Signed-off-by: Eva Rachel Retuya <[email protected]>
---
drivers/iio/adc/Kconfig | 34 +++
drivers/iio/adc/Makefile | 3 +
drivers/iio/adc/ad7606.c | 519 +++++++++++++++++++++++++++++++++++
drivers/iio/adc/ad7606.h | 78 ++++++
drivers/iio/adc/ad7606_par.c | 112 ++++++++
drivers/iio/adc/ad7606_spi.c | 78 ++++++
drivers/staging/iio/adc/Kconfig | 34 ---
drivers/staging/iio/adc/Makefile | 4 -
drivers/staging/iio/adc/ad7606.c | 519 -----------------------------------
drivers/staging/iio/adc/ad7606.h | 78 ------
drivers/staging/iio/adc/ad7606_par.c | 112 --------
drivers/staging/iio/adc/ad7606_spi.c | 78 ------
12 files changed, 824 insertions(+), 825 deletions(-)
create mode 100644 drivers/iio/adc/ad7606.c
create mode 100644 drivers/iio/adc/ad7606.h
create mode 100644 drivers/iio/adc/ad7606_par.c
create mode 100644 drivers/iio/adc/ad7606_spi.c
delete mode 100644 drivers/staging/iio/adc/ad7606.c
delete mode 100644 drivers/staging/iio/adc/ad7606.h
delete mode 100644 drivers/staging/iio/adc/ad7606_par.c
delete mode 100644 drivers/staging/iio/adc/ad7606_spi.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index c3fe98d..742db8f 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -58,6 +58,40 @@ config AD7476
To compile this driver as a module, choose M here: the
module will be called ad7476.

+config AD7606
+ tristate "Analog Devices AD7606 ADC driver"
+ depends on GPIOLIB || COMPILE_TEST
+ depends on HAS_IOMEM
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
+ help
+ Say yes here to build support for Analog Devices:
+ ad7606, ad7606-6, ad7606-4 analog to digital converters (ADC).
+
+ To compile this driver as a module, choose M here: the
+ module will be called ad7606.
+
+config AD7606_IFACE_PARALLEL
+ tristate "parallel interface support"
+ depends on AD7606
+ help
+ Say yes here to include parallel interface support on the AD7606
+ ADC driver.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ad7606_parallel.
+
+config AD7606_IFACE_SPI
+ tristate "spi interface support"
+ depends on AD7606
+ depends on SPI
+ help
+ Say yes here to include parallel interface support on the AD7606
+ ADC driver.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ad7606_spi.
+
config AD7766
tristate "Analog Devices AD7766/AD7767 ADC driver"
depends on SPI_MASTER
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 96894b3..d06d3a9 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -9,6 +9,9 @@ obj-$(CONFIG_AD7291) += ad7291.o
obj-$(CONFIG_AD7298) += ad7298.o
obj-$(CONFIG_AD7923) += ad7923.o
obj-$(CONFIG_AD7476) += ad7476.o
+obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o
+obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o
+obj-$(CONFIG_AD7606) += ad7606.o
obj-$(CONFIG_AD7766) += ad7766.o
obj-$(CONFIG_AD7791) += ad7791.o
obj-$(CONFIG_AD7793) += ad7793.o
diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
new file mode 100644
index 0000000..cceb18c
--- /dev/null
+++ b/drivers/iio/adc/ad7606.c
@@ -0,0 +1,519 @@
+/*
+ * AD7606 SPI ADC driver
+ *
+ * Copyright 2011 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+#include <linux/regulator/consumer.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/delay.h>
+#include <linux/sched.h>
+#include <linux/module.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+
+#include "ad7606.h"
+
+static int ad7606_reset(struct ad7606_state *st)
+{
+ if (st->gpio_reset) {
+ gpiod_set_value(st->gpio_reset, 1);
+ ndelay(100); /* t_reset >= 100ns */
+ gpiod_set_value(st->gpio_reset, 0);
+ return 0;
+ }
+
+ return -ENODEV;
+}
+
+static int ad7606_read_samples(struct ad7606_state *st)
+{
+ unsigned int num = st->chip_info->num_channels;
+ u16 *data = st->data;
+ int ret;
+
+ /*
+ * The frstdata signal is set to high while and after reading the sample
+ * of the first channel and low for all other channels. This can be used
+ * to check that the incoming data is correctly aligned. During normal
+ * operation the data should never become unaligned, but some glitch or
+ * electrostatic discharge might cause an extra read or clock cycle.
+ * Monitoring the frstdata signal allows to recover from such failure
+ * situations.
+ */
+
+ if (st->gpio_frstdata) {
+ ret = st->bops->read_block(st->dev, 1, data);
+ if (ret)
+ return ret;
+
+ if (!gpiod_get_value(st->gpio_frstdata)) {
+ ad7606_reset(st);
+ return -EIO;
+ }
+
+ data++;
+ num--;
+ }
+
+ return st->bops->read_block(st->dev, num, data);
+}
+
+static irqreturn_t ad7606_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct ad7606_state *st = iio_priv(pf->indio_dev);
+
+ gpiod_set_value(st->gpio_convst, 1);
+
+ return IRQ_HANDLED;
+}
+
+/**
+ * ad7606_poll_bh_to_ring() bh of trigger launched polling to ring buffer
+ * @work_s: the work struct through which this was scheduled
+ *
+ * Currently there is no option in this driver to disable the saving of
+ * timestamps within the ring.
+ * I think the one copy of this at a time was to avoid problems if the
+ * trigger was set far too high and the reads then locked up the computer.
+ **/
+static void ad7606_poll_bh_to_ring(struct work_struct *work_s)
+{
+ struct ad7606_state *st = container_of(work_s, struct ad7606_state,
+ poll_work);
+ struct iio_dev *indio_dev = iio_priv_to_dev(st);
+ int ret;
+
+ ret = ad7606_read_samples(st);
+ if (ret == 0)
+ iio_push_to_buffers_with_timestamp(indio_dev, st->data,
+ iio_get_time_ns(indio_dev));
+
+ gpiod_set_value(st->gpio_convst, 0);
+ iio_trigger_notify_done(indio_dev->trig);
+}
+
+static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
+{
+ struct ad7606_state *st = iio_priv(indio_dev);
+ int ret;
+
+ st->done = false;
+ gpiod_set_value(st->gpio_convst, 1);
+
+ ret = wait_event_interruptible(st->wq_data_avail, st->done);
+ if (ret)
+ goto error_ret;
+
+ ret = ad7606_read_samples(st);
+ if (ret == 0)
+ ret = st->data[ch];
+
+error_ret:
+ gpiod_set_value(st->gpio_convst, 0);
+
+ return ret;
+}
+
+static int ad7606_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val,
+ int *val2,
+ long m)
+{
+ int ret;
+ struct ad7606_state *st = iio_priv(indio_dev);
+
+ switch (m) {
+ case IIO_CHAN_INFO_RAW:
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret)
+ return ret;
+
+ ret = ad7606_scan_direct(indio_dev, chan->address);
+ iio_device_release_direct_mode(indio_dev);
+
+ if (ret < 0)
+ return ret;
+ *val = (short)ret;
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ *val = st->range * 2;
+ *val2 = st->chip_info->channels[0].scan_type.realbits;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+ *val = st->oversampling;
+ return IIO_VAL_INT;
+ }
+ return -EINVAL;
+}
+
+static IIO_CONST_ATTR(in_voltage_scale_available, "5000 10000");
+
+static int ad7606_oversampling_get_index(unsigned int val)
+{
+ unsigned char supported[] = {1, 2, 4, 8, 16, 32, 64};
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(supported); i++)
+ if (val == supported[i])
+ return i;
+
+ return -EINVAL;
+}
+
+static int ad7606_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val,
+ int val2,
+ long mask)
+{
+ struct ad7606_state *st = iio_priv(indio_dev);
+ int values[3];
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_SCALE:
+ if (val2)
+ return -EINVAL;
+
+ if (!(val == 5000 || val == 10000))
+ return -EINVAL;
+
+ mutex_lock(&indio_dev->mlock);
+ gpiod_set_value(st->gpio_range, val == 10000);
+ st->range = val;
+ mutex_unlock(&indio_dev->mlock);
+
+ return 0;
+ case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+ if (val2)
+ return -EINVAL;
+ ret = ad7606_oversampling_get_index(val);
+ if (ret < 0)
+ return ret;
+
+ values[0] = (ret >> 0) & 1;
+ values[1] = (ret >> 1) & 1;
+ values[2] = (ret >> 2) & 1;
+
+ mutex_lock(&indio_dev->mlock);
+ gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
+ values);
+ st->oversampling = val;
+ mutex_unlock(&indio_dev->mlock);
+
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
+static IIO_CONST_ATTR(oversampling_ratio_available, "1 2 4 8 16 32 64");
+
+static struct attribute *ad7606_attributes_os_and_range[] = {
+ &iio_const_attr_in_voltage_scale_available.dev_attr.attr,
+ &iio_const_attr_oversampling_ratio_available.dev_attr.attr,
+ NULL,
+};
+
+static const struct attribute_group ad7606_attribute_group_os_and_range = {
+ .attrs = ad7606_attributes_os_and_range,
+};
+
+static struct attribute *ad7606_attributes_os[] = {
+ &iio_const_attr_oversampling_ratio_available.dev_attr.attr,
+ NULL,
+};
+
+static const struct attribute_group ad7606_attribute_group_os = {
+ .attrs = ad7606_attributes_os,
+};
+
+static struct attribute *ad7606_attributes_range[] = {
+ &iio_const_attr_in_voltage_scale_available.dev_attr.attr,
+ NULL,
+};
+
+static const struct attribute_group ad7606_attribute_group_range = {
+ .attrs = ad7606_attributes_range,
+};
+
+#define AD7606_CHANNEL(num) \
+ { \
+ .type = IIO_VOLTAGE, \
+ .indexed = 1, \
+ .channel = num, \
+ .address = num, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
+ .info_mask_shared_by_all = \
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
+ .scan_index = num, \
+ .scan_type = { \
+ .sign = 's', \
+ .realbits = 16, \
+ .storagebits = 16, \
+ .endianness = IIO_CPU, \
+ }, \
+ }
+
+static const struct iio_chan_spec ad7606_channels[] = {
+ IIO_CHAN_SOFT_TIMESTAMP(8),
+ AD7606_CHANNEL(0),
+ AD7606_CHANNEL(1),
+ AD7606_CHANNEL(2),
+ AD7606_CHANNEL(3),
+ AD7606_CHANNEL(4),
+ AD7606_CHANNEL(5),
+ AD7606_CHANNEL(6),
+ AD7606_CHANNEL(7),
+};
+
+static const struct ad7606_chip_info ad7606_chip_info_tbl[] = {
+ /*
+ * More devices added in future
+ */
+ [ID_AD7606_8] = {
+ .channels = ad7606_channels,
+ .num_channels = 9,
+ },
+ [ID_AD7606_6] = {
+ .channels = ad7606_channels,
+ .num_channels = 7,
+ },
+ [ID_AD7606_4] = {
+ .channels = ad7606_channels,
+ .num_channels = 5,
+ },
+};
+
+static int ad7606_request_gpios(struct ad7606_state *st)
+{
+ struct device *dev = st->dev;
+
+ st->gpio_convst = devm_gpiod_get(dev, "conversion-start",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(st->gpio_convst))
+ return PTR_ERR(st->gpio_convst);
+
+ st->gpio_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+ if (IS_ERR(st->gpio_reset))
+ return PTR_ERR(st->gpio_reset);
+
+ st->gpio_range = devm_gpiod_get_optional(dev, "range", GPIOD_OUT_LOW);
+ if (IS_ERR(st->gpio_range))
+ return PTR_ERR(st->gpio_range);
+
+ st->gpio_standby = devm_gpiod_get_optional(dev, "standby",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(st->gpio_standby))
+ return PTR_ERR(st->gpio_standby);
+
+ st->gpio_frstdata = devm_gpiod_get_optional(dev, "first-data",
+ GPIOD_IN);
+ if (IS_ERR(st->gpio_frstdata))
+ return PTR_ERR(st->gpio_frstdata);
+
+ st->gpio_os = devm_gpiod_get_array_optional(dev, "oversampling-ratio",
+ GPIOD_OUT_LOW);
+ return PTR_ERR_OR_ZERO(st->gpio_os);
+}
+
+/**
+ * Interrupt handler
+ */
+static irqreturn_t ad7606_interrupt(int irq, void *dev_id)
+{
+ struct iio_dev *indio_dev = dev_id;
+ struct ad7606_state *st = iio_priv(indio_dev);
+
+ if (iio_buffer_enabled(indio_dev)) {
+ schedule_work(&st->poll_work);
+ } else {
+ st->done = true;
+ wake_up_interruptible(&st->wq_data_avail);
+ }
+
+ return IRQ_HANDLED;
+};
+
+static const struct iio_info ad7606_info_no_os_or_range = {
+ .driver_module = THIS_MODULE,
+ .read_raw = &ad7606_read_raw,
+};
+
+static const struct iio_info ad7606_info_os_and_range = {
+ .driver_module = THIS_MODULE,
+ .read_raw = &ad7606_read_raw,
+ .write_raw = &ad7606_write_raw,
+ .attrs = &ad7606_attribute_group_os_and_range,
+};
+
+static const struct iio_info ad7606_info_os = {
+ .driver_module = THIS_MODULE,
+ .read_raw = &ad7606_read_raw,
+ .write_raw = &ad7606_write_raw,
+ .attrs = &ad7606_attribute_group_os,
+};
+
+static const struct iio_info ad7606_info_range = {
+ .driver_module = THIS_MODULE,
+ .read_raw = &ad7606_read_raw,
+ .attrs = &ad7606_attribute_group_range,
+};
+
+int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
+ const char *name, unsigned int id,
+ const struct ad7606_bus_ops *bops)
+{
+ struct ad7606_state *st;
+ int ret;
+ struct iio_dev *indio_dev;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ st = iio_priv(indio_dev);
+
+ st->dev = dev;
+ st->bops = bops;
+ st->base_address = base_address;
+ st->range = 5000;
+ st->oversampling = 1;
+ INIT_WORK(&st->poll_work, &ad7606_poll_bh_to_ring);
+
+ st->reg = devm_regulator_get(dev, "avcc");
+ if (IS_ERR(st->reg))
+ return PTR_ERR(st->reg);
+
+ ret = regulator_enable(st->reg);
+ if (ret) {
+ dev_err(dev, "Failed to enable specified AVcc supply\n");
+ return ret;
+ }
+
+ ret = ad7606_request_gpios(st);
+ if (ret)
+ goto error_disable_reg;
+
+ st->chip_info = &ad7606_chip_info_tbl[id];
+
+ indio_dev->dev.parent = dev;
+ if (st->gpio_os) {
+ if (st->gpio_range)
+ indio_dev->info = &ad7606_info_os_and_range;
+ else
+ indio_dev->info = &ad7606_info_os;
+ } else {
+ if (st->gpio_range)
+ indio_dev->info = &ad7606_info_range;
+ else
+ indio_dev->info = &ad7606_info_no_os_or_range;
+ }
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->name = name;
+ indio_dev->channels = st->chip_info->channels;
+ indio_dev->num_channels = st->chip_info->num_channels;
+
+ init_waitqueue_head(&st->wq_data_avail);
+
+ ret = ad7606_reset(st);
+ if (ret)
+ dev_warn(st->dev, "failed to RESET: no RESET GPIO specified\n");
+
+ ret = request_irq(irq, ad7606_interrupt, IRQF_TRIGGER_FALLING, name,
+ indio_dev);
+ if (ret)
+ goto error_disable_reg;
+
+ ret = iio_triggered_buffer_setup(indio_dev, &ad7606_trigger_handler,
+ NULL, NULL);
+ if (ret)
+ goto error_free_irq;
+
+ ret = iio_device_register(indio_dev);
+ if (ret)
+ goto error_unregister_ring;
+
+ dev_set_drvdata(dev, indio_dev);
+
+ return 0;
+error_unregister_ring:
+ iio_triggered_buffer_cleanup(indio_dev);
+
+error_free_irq:
+ free_irq(irq, indio_dev);
+
+error_disable_reg:
+ regulator_disable(st->reg);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(ad7606_probe);
+
+int ad7606_remove(struct device *dev, int irq)
+{
+ struct iio_dev *indio_dev = dev_get_drvdata(dev);
+ struct ad7606_state *st = iio_priv(indio_dev);
+
+ iio_device_unregister(indio_dev);
+ iio_triggered_buffer_cleanup(indio_dev);
+
+ free_irq(irq, indio_dev);
+ regulator_disable(st->reg);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ad7606_remove);
+
+#ifdef CONFIG_PM_SLEEP
+
+static int ad7606_suspend(struct device *dev)
+{
+ struct iio_dev *indio_dev = dev_get_drvdata(dev);
+ struct ad7606_state *st = iio_priv(indio_dev);
+
+ if (st->gpio_standby) {
+ gpiod_set_value(st->gpio_range, 1);
+ gpiod_set_value(st->gpio_standby, 0);
+ }
+
+ return 0;
+}
+
+static int ad7606_resume(struct device *dev)
+{
+ struct iio_dev *indio_dev = dev_get_drvdata(dev);
+ struct ad7606_state *st = iio_priv(indio_dev);
+
+ if (st->gpio_standby) {
+ gpiod_set_value(st->gpio_range, st->range == 10000);
+ gpiod_set_value(st->gpio_standby, 1);
+ ad7606_reset(st);
+ }
+
+ return 0;
+}
+
+SIMPLE_DEV_PM_OPS(ad7606_pm_ops, ad7606_suspend, ad7606_resume);
+EXPORT_SYMBOL_GPL(ad7606_pm_ops);
+
+#endif
+
+MODULE_AUTHOR("Michael Hennerich <[email protected]>");
+MODULE_DESCRIPTION("Analog Devices AD7606 ADC");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
new file mode 100644
index 0000000..746f955
--- /dev/null
+++ b/drivers/iio/adc/ad7606.h
@@ -0,0 +1,78 @@
+/*
+ * AD7606 ADC driver
+ *
+ * Copyright 2011 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2.
+ */
+
+#ifndef IIO_ADC_AD7606_H_
+#define IIO_ADC_AD7606_H_
+
+/**
+ * struct ad7606_chip_info - chip specific information
+ * @name: identification string for chip
+ * @channels: channel specification
+ * @num_channels: number of channels
+ */
+
+struct ad7606_chip_info {
+ const struct iio_chan_spec *channels;
+ unsigned int num_channels;
+};
+
+/**
+ * struct ad7606_state - driver instance specific data
+ */
+
+struct ad7606_state {
+ struct device *dev;
+ const struct ad7606_chip_info *chip_info;
+ struct regulator *reg;
+ struct work_struct poll_work;
+ wait_queue_head_t wq_data_avail;
+ const struct ad7606_bus_ops *bops;
+ unsigned int range;
+ unsigned int oversampling;
+ bool done;
+ void __iomem *base_address;
+
+ struct gpio_desc *gpio_convst;
+ struct gpio_desc *gpio_reset;
+ struct gpio_desc *gpio_range;
+ struct gpio_desc *gpio_standby;
+ struct gpio_desc *gpio_frstdata;
+ struct gpio_descs *gpio_os;
+
+ /*
+ * DMA (thus cache coherency maintenance) requires the
+ * transfer buffers to live in their own cache lines.
+ * 8 * 16-bit samples + 64-bit timestamp
+ */
+ unsigned short data[12] ____cacheline_aligned;
+};
+
+struct ad7606_bus_ops {
+ /* more methods added in future? */
+ int (*read_block)(struct device *, int, void *);
+};
+
+int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
+ const char *name, unsigned int id,
+ const struct ad7606_bus_ops *bops);
+int ad7606_remove(struct device *dev, int irq);
+
+enum ad7606_supported_device_ids {
+ ID_AD7606_8,
+ ID_AD7606_6,
+ ID_AD7606_4
+};
+
+#ifdef CONFIG_PM_SLEEP
+extern const struct dev_pm_ops ad7606_pm_ops;
+#define AD7606_PM_OPS (&ad7606_pm_ops)
+#else
+#define AD7606_PM_OPS NULL
+#endif
+
+#endif /* IIO_ADC_AD7606_H_ */
diff --git a/drivers/iio/adc/ad7606_par.c b/drivers/iio/adc/ad7606_par.c
new file mode 100644
index 0000000..cd6c410c
--- /dev/null
+++ b/drivers/iio/adc/ad7606_par.c
@@ -0,0 +1,112 @@
+/*
+ * AD7606 Parallel Interface ADC driver
+ *
+ * Copyright 2011 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+#include <linux/err.h>
+#include <linux/io.h>
+
+#include <linux/iio/iio.h>
+#include "ad7606.h"
+
+static int ad7606_par16_read_block(struct device *dev,
+ int count, void *buf)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+ struct ad7606_state *st = iio_priv(indio_dev);
+
+ insw((unsigned long)st->base_address, buf, count);
+
+ return 0;
+}
+
+static const struct ad7606_bus_ops ad7606_par16_bops = {
+ .read_block = ad7606_par16_read_block,
+};
+
+static int ad7606_par8_read_block(struct device *dev,
+ int count, void *buf)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+ struct ad7606_state *st = iio_priv(indio_dev);
+
+ insb((unsigned long)st->base_address, buf, count * 2);
+
+ return 0;
+}
+
+static const struct ad7606_bus_ops ad7606_par8_bops = {
+ .read_block = ad7606_par8_read_block,
+};
+
+static int ad7606_par_probe(struct platform_device *pdev)
+{
+ const struct platform_device_id *id = platform_get_device_id(pdev);
+ struct resource *res;
+ void __iomem *addr;
+ resource_size_t remap_size;
+ int irq;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(&pdev->dev, "no irq\n");
+ return -ENODEV;
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ addr = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(addr))
+ return PTR_ERR(addr);
+
+ remap_size = resource_size(res);
+
+ return ad7606_probe(&pdev->dev, irq, addr,
+ id->name, id->driver_data,
+ remap_size > 1 ? &ad7606_par16_bops :
+ &ad7606_par8_bops);
+}
+
+static int ad7606_par_remove(struct platform_device *pdev)
+{
+ return ad7606_remove(&pdev->dev, platform_get_irq(pdev, 0));
+}
+
+static const struct platform_device_id ad7606_driver_ids[] = {
+ {
+ .name = "ad7606-8",
+ .driver_data = ID_AD7606_8,
+ }, {
+ .name = "ad7606-6",
+ .driver_data = ID_AD7606_6,
+ }, {
+ .name = "ad7606-4",
+ .driver_data = ID_AD7606_4,
+ },
+ { }
+};
+
+MODULE_DEVICE_TABLE(platform, ad7606_driver_ids);
+
+static struct platform_driver ad7606_driver = {
+ .probe = ad7606_par_probe,
+ .remove = ad7606_par_remove,
+ .id_table = ad7606_driver_ids,
+ .driver = {
+ .name = "ad7606",
+ .pm = AD7606_PM_OPS,
+ },
+};
+
+module_platform_driver(ad7606_driver);
+
+MODULE_AUTHOR("Michael Hennerich <[email protected]>");
+MODULE_DESCRIPTION("Analog Devices AD7606 ADC");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c
new file mode 100644
index 0000000..c9b1f266
--- /dev/null
+++ b/drivers/iio/adc/ad7606_spi.c
@@ -0,0 +1,78 @@
+/*
+ * AD7606 SPI ADC driver
+ *
+ * Copyright 2011 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2.
+ */
+
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+#include <linux/types.h>
+#include <linux/err.h>
+
+#include <linux/iio/iio.h>
+#include "ad7606.h"
+
+#define MAX_SPI_FREQ_HZ 23500000 /* VDRIVE above 4.75 V */
+
+static int ad7606_spi_read_block(struct device *dev,
+ int count, void *buf)
+{
+ struct spi_device *spi = to_spi_device(dev);
+ int i, ret;
+ unsigned short *data = buf;
+ __be16 *bdata = buf;
+
+ ret = spi_read(spi, buf, count * 2);
+ if (ret < 0) {
+ dev_err(&spi->dev, "SPI read error\n");
+ return ret;
+ }
+
+ for (i = 0; i < count; i++)
+ data[i] = be16_to_cpu(bdata[i]);
+
+ return 0;
+}
+
+static const struct ad7606_bus_ops ad7606_spi_bops = {
+ .read_block = ad7606_spi_read_block,
+};
+
+static int ad7606_spi_probe(struct spi_device *spi)
+{
+ const struct spi_device_id *id = spi_get_device_id(spi);
+
+ return ad7606_probe(&spi->dev, spi->irq, NULL,
+ id->name, id->driver_data,
+ &ad7606_spi_bops);
+}
+
+static int ad7606_spi_remove(struct spi_device *spi)
+{
+ return ad7606_remove(&spi->dev, spi->irq);
+}
+
+static const struct spi_device_id ad7606_id[] = {
+ {"ad7606-8", ID_AD7606_8},
+ {"ad7606-6", ID_AD7606_6},
+ {"ad7606-4", ID_AD7606_4},
+ {}
+};
+MODULE_DEVICE_TABLE(spi, ad7606_id);
+
+static struct spi_driver ad7606_driver = {
+ .driver = {
+ .name = "ad7606",
+ .pm = AD7606_PM_OPS,
+ },
+ .probe = ad7606_spi_probe,
+ .remove = ad7606_spi_remove,
+ .id_table = ad7606_id,
+};
+module_spi_driver(ad7606_driver);
+
+MODULE_AUTHOR("Michael Hennerich <[email protected]>");
+MODULE_DESCRIPTION("Analog Devices AD7606 ADC");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig
index deff899..995867b 100644
--- a/drivers/staging/iio/adc/Kconfig
+++ b/drivers/staging/iio/adc/Kconfig
@@ -3,40 +3,6 @@
#
menu "Analog to digital converters"

-config AD7606
- tristate "Analog Devices AD7606 ADC driver"
- depends on GPIOLIB || COMPILE_TEST
- depends on HAS_IOMEM
- select IIO_BUFFER
- select IIO_TRIGGERED_BUFFER
- help
- Say yes here to build support for Analog Devices:
- ad7606, ad7606-6, ad7606-4 analog to digital converters (ADC).
-
- To compile this driver as a module, choose M here: the
- module will be called ad7606.
-
-config AD7606_IFACE_PARALLEL
- tristate "parallel interface support"
- depends on AD7606
- help
- Say yes here to include parallel interface support on the AD7606
- ADC driver.
-
- To compile this driver as a module, choose M here: the
- module will be called ad7606_parallel.
-
-config AD7606_IFACE_SPI
- tristate "spi interface support"
- depends on AD7606
- depends on SPI
- help
- Say yes here to include parallel interface support on the AD7606
- ADC driver.
-
- To compile this driver as a module, choose M here: the
- module will be called ad7606_spi.
-
config AD7780
tristate "Analog Devices AD7780 and similar ADCs driver"
depends on SPI
diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc/Makefile
index ac09485..549032b 100644
--- a/drivers/staging/iio/adc/Makefile
+++ b/drivers/staging/iio/adc/Makefile
@@ -2,10 +2,6 @@
# Makefile for industrial I/O ADC drivers
#

-obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o
-obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o
-obj-$(CONFIG_AD7606) += ad7606.o
-
obj-$(CONFIG_AD7780) += ad7780.o
obj-$(CONFIG_AD7816) += ad7816.o
obj-$(CONFIG_AD7192) += ad7192.o
diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
deleted file mode 100644
index cceb18c..0000000
--- a/drivers/staging/iio/adc/ad7606.c
+++ /dev/null
@@ -1,519 +0,0 @@
-/*
- * AD7606 SPI ADC driver
- *
- * Copyright 2011 Analog Devices Inc.
- *
- * Licensed under the GPL-2.
- */
-
-#include <linux/interrupt.h>
-#include <linux/device.h>
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/sysfs.h>
-#include <linux/regulator/consumer.h>
-#include <linux/err.h>
-#include <linux/gpio/consumer.h>
-#include <linux/delay.h>
-#include <linux/sched.h>
-#include <linux/module.h>
-
-#include <linux/iio/iio.h>
-#include <linux/iio/sysfs.h>
-#include <linux/iio/buffer.h>
-#include <linux/iio/trigger_consumer.h>
-#include <linux/iio/triggered_buffer.h>
-
-#include "ad7606.h"
-
-static int ad7606_reset(struct ad7606_state *st)
-{
- if (st->gpio_reset) {
- gpiod_set_value(st->gpio_reset, 1);
- ndelay(100); /* t_reset >= 100ns */
- gpiod_set_value(st->gpio_reset, 0);
- return 0;
- }
-
- return -ENODEV;
-}
-
-static int ad7606_read_samples(struct ad7606_state *st)
-{
- unsigned int num = st->chip_info->num_channels;
- u16 *data = st->data;
- int ret;
-
- /*
- * The frstdata signal is set to high while and after reading the sample
- * of the first channel and low for all other channels. This can be used
- * to check that the incoming data is correctly aligned. During normal
- * operation the data should never become unaligned, but some glitch or
- * electrostatic discharge might cause an extra read or clock cycle.
- * Monitoring the frstdata signal allows to recover from such failure
- * situations.
- */
-
- if (st->gpio_frstdata) {
- ret = st->bops->read_block(st->dev, 1, data);
- if (ret)
- return ret;
-
- if (!gpiod_get_value(st->gpio_frstdata)) {
- ad7606_reset(st);
- return -EIO;
- }
-
- data++;
- num--;
- }
-
- return st->bops->read_block(st->dev, num, data);
-}
-
-static irqreturn_t ad7606_trigger_handler(int irq, void *p)
-{
- struct iio_poll_func *pf = p;
- struct ad7606_state *st = iio_priv(pf->indio_dev);
-
- gpiod_set_value(st->gpio_convst, 1);
-
- return IRQ_HANDLED;
-}
-
-/**
- * ad7606_poll_bh_to_ring() bh of trigger launched polling to ring buffer
- * @work_s: the work struct through which this was scheduled
- *
- * Currently there is no option in this driver to disable the saving of
- * timestamps within the ring.
- * I think the one copy of this at a time was to avoid problems if the
- * trigger was set far too high and the reads then locked up the computer.
- **/
-static void ad7606_poll_bh_to_ring(struct work_struct *work_s)
-{
- struct ad7606_state *st = container_of(work_s, struct ad7606_state,
- poll_work);
- struct iio_dev *indio_dev = iio_priv_to_dev(st);
- int ret;
-
- ret = ad7606_read_samples(st);
- if (ret == 0)
- iio_push_to_buffers_with_timestamp(indio_dev, st->data,
- iio_get_time_ns(indio_dev));
-
- gpiod_set_value(st->gpio_convst, 0);
- iio_trigger_notify_done(indio_dev->trig);
-}
-
-static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
-{
- struct ad7606_state *st = iio_priv(indio_dev);
- int ret;
-
- st->done = false;
- gpiod_set_value(st->gpio_convst, 1);
-
- ret = wait_event_interruptible(st->wq_data_avail, st->done);
- if (ret)
- goto error_ret;
-
- ret = ad7606_read_samples(st);
- if (ret == 0)
- ret = st->data[ch];
-
-error_ret:
- gpiod_set_value(st->gpio_convst, 0);
-
- return ret;
-}
-
-static int ad7606_read_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int *val,
- int *val2,
- long m)
-{
- int ret;
- struct ad7606_state *st = iio_priv(indio_dev);
-
- switch (m) {
- case IIO_CHAN_INFO_RAW:
- ret = iio_device_claim_direct_mode(indio_dev);
- if (ret)
- return ret;
-
- ret = ad7606_scan_direct(indio_dev, chan->address);
- iio_device_release_direct_mode(indio_dev);
-
- if (ret < 0)
- return ret;
- *val = (short)ret;
- return IIO_VAL_INT;
- case IIO_CHAN_INFO_SCALE:
- *val = st->range * 2;
- *val2 = st->chip_info->channels[0].scan_type.realbits;
- return IIO_VAL_FRACTIONAL_LOG2;
- case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
- *val = st->oversampling;
- return IIO_VAL_INT;
- }
- return -EINVAL;
-}
-
-static IIO_CONST_ATTR(in_voltage_scale_available, "5000 10000");
-
-static int ad7606_oversampling_get_index(unsigned int val)
-{
- unsigned char supported[] = {1, 2, 4, 8, 16, 32, 64};
- int i;
-
- for (i = 0; i < ARRAY_SIZE(supported); i++)
- if (val == supported[i])
- return i;
-
- return -EINVAL;
-}
-
-static int ad7606_write_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int val,
- int val2,
- long mask)
-{
- struct ad7606_state *st = iio_priv(indio_dev);
- int values[3];
- int ret;
-
- switch (mask) {
- case IIO_CHAN_INFO_SCALE:
- if (val2)
- return -EINVAL;
-
- if (!(val == 5000 || val == 10000))
- return -EINVAL;
-
- mutex_lock(&indio_dev->mlock);
- gpiod_set_value(st->gpio_range, val == 10000);
- st->range = val;
- mutex_unlock(&indio_dev->mlock);
-
- return 0;
- case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
- if (val2)
- return -EINVAL;
- ret = ad7606_oversampling_get_index(val);
- if (ret < 0)
- return ret;
-
- values[0] = (ret >> 0) & 1;
- values[1] = (ret >> 1) & 1;
- values[2] = (ret >> 2) & 1;
-
- mutex_lock(&indio_dev->mlock);
- gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
- values);
- st->oversampling = val;
- mutex_unlock(&indio_dev->mlock);
-
- return 0;
- default:
- return -EINVAL;
- }
-}
-
-static IIO_CONST_ATTR(oversampling_ratio_available, "1 2 4 8 16 32 64");
-
-static struct attribute *ad7606_attributes_os_and_range[] = {
- &iio_const_attr_in_voltage_scale_available.dev_attr.attr,
- &iio_const_attr_oversampling_ratio_available.dev_attr.attr,
- NULL,
-};
-
-static const struct attribute_group ad7606_attribute_group_os_and_range = {
- .attrs = ad7606_attributes_os_and_range,
-};
-
-static struct attribute *ad7606_attributes_os[] = {
- &iio_const_attr_oversampling_ratio_available.dev_attr.attr,
- NULL,
-};
-
-static const struct attribute_group ad7606_attribute_group_os = {
- .attrs = ad7606_attributes_os,
-};
-
-static struct attribute *ad7606_attributes_range[] = {
- &iio_const_attr_in_voltage_scale_available.dev_attr.attr,
- NULL,
-};
-
-static const struct attribute_group ad7606_attribute_group_range = {
- .attrs = ad7606_attributes_range,
-};
-
-#define AD7606_CHANNEL(num) \
- { \
- .type = IIO_VOLTAGE, \
- .indexed = 1, \
- .channel = num, \
- .address = num, \
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
- .info_mask_shared_by_all = \
- BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
- .scan_index = num, \
- .scan_type = { \
- .sign = 's', \
- .realbits = 16, \
- .storagebits = 16, \
- .endianness = IIO_CPU, \
- }, \
- }
-
-static const struct iio_chan_spec ad7606_channels[] = {
- IIO_CHAN_SOFT_TIMESTAMP(8),
- AD7606_CHANNEL(0),
- AD7606_CHANNEL(1),
- AD7606_CHANNEL(2),
- AD7606_CHANNEL(3),
- AD7606_CHANNEL(4),
- AD7606_CHANNEL(5),
- AD7606_CHANNEL(6),
- AD7606_CHANNEL(7),
-};
-
-static const struct ad7606_chip_info ad7606_chip_info_tbl[] = {
- /*
- * More devices added in future
- */
- [ID_AD7606_8] = {
- .channels = ad7606_channels,
- .num_channels = 9,
- },
- [ID_AD7606_6] = {
- .channels = ad7606_channels,
- .num_channels = 7,
- },
- [ID_AD7606_4] = {
- .channels = ad7606_channels,
- .num_channels = 5,
- },
-};
-
-static int ad7606_request_gpios(struct ad7606_state *st)
-{
- struct device *dev = st->dev;
-
- st->gpio_convst = devm_gpiod_get(dev, "conversion-start",
- GPIOD_OUT_LOW);
- if (IS_ERR(st->gpio_convst))
- return PTR_ERR(st->gpio_convst);
-
- st->gpio_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
- if (IS_ERR(st->gpio_reset))
- return PTR_ERR(st->gpio_reset);
-
- st->gpio_range = devm_gpiod_get_optional(dev, "range", GPIOD_OUT_LOW);
- if (IS_ERR(st->gpio_range))
- return PTR_ERR(st->gpio_range);
-
- st->gpio_standby = devm_gpiod_get_optional(dev, "standby",
- GPIOD_OUT_HIGH);
- if (IS_ERR(st->gpio_standby))
- return PTR_ERR(st->gpio_standby);
-
- st->gpio_frstdata = devm_gpiod_get_optional(dev, "first-data",
- GPIOD_IN);
- if (IS_ERR(st->gpio_frstdata))
- return PTR_ERR(st->gpio_frstdata);
-
- st->gpio_os = devm_gpiod_get_array_optional(dev, "oversampling-ratio",
- GPIOD_OUT_LOW);
- return PTR_ERR_OR_ZERO(st->gpio_os);
-}
-
-/**
- * Interrupt handler
- */
-static irqreturn_t ad7606_interrupt(int irq, void *dev_id)
-{
- struct iio_dev *indio_dev = dev_id;
- struct ad7606_state *st = iio_priv(indio_dev);
-
- if (iio_buffer_enabled(indio_dev)) {
- schedule_work(&st->poll_work);
- } else {
- st->done = true;
- wake_up_interruptible(&st->wq_data_avail);
- }
-
- return IRQ_HANDLED;
-};
-
-static const struct iio_info ad7606_info_no_os_or_range = {
- .driver_module = THIS_MODULE,
- .read_raw = &ad7606_read_raw,
-};
-
-static const struct iio_info ad7606_info_os_and_range = {
- .driver_module = THIS_MODULE,
- .read_raw = &ad7606_read_raw,
- .write_raw = &ad7606_write_raw,
- .attrs = &ad7606_attribute_group_os_and_range,
-};
-
-static const struct iio_info ad7606_info_os = {
- .driver_module = THIS_MODULE,
- .read_raw = &ad7606_read_raw,
- .write_raw = &ad7606_write_raw,
- .attrs = &ad7606_attribute_group_os,
-};
-
-static const struct iio_info ad7606_info_range = {
- .driver_module = THIS_MODULE,
- .read_raw = &ad7606_read_raw,
- .attrs = &ad7606_attribute_group_range,
-};
-
-int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
- const char *name, unsigned int id,
- const struct ad7606_bus_ops *bops)
-{
- struct ad7606_state *st;
- int ret;
- struct iio_dev *indio_dev;
-
- indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
- if (!indio_dev)
- return -ENOMEM;
-
- st = iio_priv(indio_dev);
-
- st->dev = dev;
- st->bops = bops;
- st->base_address = base_address;
- st->range = 5000;
- st->oversampling = 1;
- INIT_WORK(&st->poll_work, &ad7606_poll_bh_to_ring);
-
- st->reg = devm_regulator_get(dev, "avcc");
- if (IS_ERR(st->reg))
- return PTR_ERR(st->reg);
-
- ret = regulator_enable(st->reg);
- if (ret) {
- dev_err(dev, "Failed to enable specified AVcc supply\n");
- return ret;
- }
-
- ret = ad7606_request_gpios(st);
- if (ret)
- goto error_disable_reg;
-
- st->chip_info = &ad7606_chip_info_tbl[id];
-
- indio_dev->dev.parent = dev;
- if (st->gpio_os) {
- if (st->gpio_range)
- indio_dev->info = &ad7606_info_os_and_range;
- else
- indio_dev->info = &ad7606_info_os;
- } else {
- if (st->gpio_range)
- indio_dev->info = &ad7606_info_range;
- else
- indio_dev->info = &ad7606_info_no_os_or_range;
- }
- indio_dev->modes = INDIO_DIRECT_MODE;
- indio_dev->name = name;
- indio_dev->channels = st->chip_info->channels;
- indio_dev->num_channels = st->chip_info->num_channels;
-
- init_waitqueue_head(&st->wq_data_avail);
-
- ret = ad7606_reset(st);
- if (ret)
- dev_warn(st->dev, "failed to RESET: no RESET GPIO specified\n");
-
- ret = request_irq(irq, ad7606_interrupt, IRQF_TRIGGER_FALLING, name,
- indio_dev);
- if (ret)
- goto error_disable_reg;
-
- ret = iio_triggered_buffer_setup(indio_dev, &ad7606_trigger_handler,
- NULL, NULL);
- if (ret)
- goto error_free_irq;
-
- ret = iio_device_register(indio_dev);
- if (ret)
- goto error_unregister_ring;
-
- dev_set_drvdata(dev, indio_dev);
-
- return 0;
-error_unregister_ring:
- iio_triggered_buffer_cleanup(indio_dev);
-
-error_free_irq:
- free_irq(irq, indio_dev);
-
-error_disable_reg:
- regulator_disable(st->reg);
- return ret;
-}
-EXPORT_SYMBOL_GPL(ad7606_probe);
-
-int ad7606_remove(struct device *dev, int irq)
-{
- struct iio_dev *indio_dev = dev_get_drvdata(dev);
- struct ad7606_state *st = iio_priv(indio_dev);
-
- iio_device_unregister(indio_dev);
- iio_triggered_buffer_cleanup(indio_dev);
-
- free_irq(irq, indio_dev);
- regulator_disable(st->reg);
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(ad7606_remove);
-
-#ifdef CONFIG_PM_SLEEP
-
-static int ad7606_suspend(struct device *dev)
-{
- struct iio_dev *indio_dev = dev_get_drvdata(dev);
- struct ad7606_state *st = iio_priv(indio_dev);
-
- if (st->gpio_standby) {
- gpiod_set_value(st->gpio_range, 1);
- gpiod_set_value(st->gpio_standby, 0);
- }
-
- return 0;
-}
-
-static int ad7606_resume(struct device *dev)
-{
- struct iio_dev *indio_dev = dev_get_drvdata(dev);
- struct ad7606_state *st = iio_priv(indio_dev);
-
- if (st->gpio_standby) {
- gpiod_set_value(st->gpio_range, st->range == 10000);
- gpiod_set_value(st->gpio_standby, 1);
- ad7606_reset(st);
- }
-
- return 0;
-}
-
-SIMPLE_DEV_PM_OPS(ad7606_pm_ops, ad7606_suspend, ad7606_resume);
-EXPORT_SYMBOL_GPL(ad7606_pm_ops);
-
-#endif
-
-MODULE_AUTHOR("Michael Hennerich <[email protected]>");
-MODULE_DESCRIPTION("Analog Devices AD7606 ADC");
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
deleted file mode 100644
index 746f955..0000000
--- a/drivers/staging/iio/adc/ad7606.h
+++ /dev/null
@@ -1,78 +0,0 @@
-/*
- * AD7606 ADC driver
- *
- * Copyright 2011 Analog Devices Inc.
- *
- * Licensed under the GPL-2.
- */
-
-#ifndef IIO_ADC_AD7606_H_
-#define IIO_ADC_AD7606_H_
-
-/**
- * struct ad7606_chip_info - chip specific information
- * @name: identification string for chip
- * @channels: channel specification
- * @num_channels: number of channels
- */
-
-struct ad7606_chip_info {
- const struct iio_chan_spec *channels;
- unsigned int num_channels;
-};
-
-/**
- * struct ad7606_state - driver instance specific data
- */
-
-struct ad7606_state {
- struct device *dev;
- const struct ad7606_chip_info *chip_info;
- struct regulator *reg;
- struct work_struct poll_work;
- wait_queue_head_t wq_data_avail;
- const struct ad7606_bus_ops *bops;
- unsigned int range;
- unsigned int oversampling;
- bool done;
- void __iomem *base_address;
-
- struct gpio_desc *gpio_convst;
- struct gpio_desc *gpio_reset;
- struct gpio_desc *gpio_range;
- struct gpio_desc *gpio_standby;
- struct gpio_desc *gpio_frstdata;
- struct gpio_descs *gpio_os;
-
- /*
- * DMA (thus cache coherency maintenance) requires the
- * transfer buffers to live in their own cache lines.
- * 8 * 16-bit samples + 64-bit timestamp
- */
- unsigned short data[12] ____cacheline_aligned;
-};
-
-struct ad7606_bus_ops {
- /* more methods added in future? */
- int (*read_block)(struct device *, int, void *);
-};
-
-int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
- const char *name, unsigned int id,
- const struct ad7606_bus_ops *bops);
-int ad7606_remove(struct device *dev, int irq);
-
-enum ad7606_supported_device_ids {
- ID_AD7606_8,
- ID_AD7606_6,
- ID_AD7606_4
-};
-
-#ifdef CONFIG_PM_SLEEP
-extern const struct dev_pm_ops ad7606_pm_ops;
-#define AD7606_PM_OPS (&ad7606_pm_ops)
-#else
-#define AD7606_PM_OPS NULL
-#endif
-
-#endif /* IIO_ADC_AD7606_H_ */
diff --git a/drivers/staging/iio/adc/ad7606_par.c b/drivers/staging/iio/adc/ad7606_par.c
deleted file mode 100644
index cd6c410c..0000000
--- a/drivers/staging/iio/adc/ad7606_par.c
+++ /dev/null
@@ -1,112 +0,0 @@
-/*
- * AD7606 Parallel Interface ADC driver
- *
- * Copyright 2011 Analog Devices Inc.
- *
- * Licensed under the GPL-2.
- */
-
-#include <linux/module.h>
-#include <linux/platform_device.h>
-#include <linux/types.h>
-#include <linux/err.h>
-#include <linux/io.h>
-
-#include <linux/iio/iio.h>
-#include "ad7606.h"
-
-static int ad7606_par16_read_block(struct device *dev,
- int count, void *buf)
-{
- struct platform_device *pdev = to_platform_device(dev);
- struct iio_dev *indio_dev = platform_get_drvdata(pdev);
- struct ad7606_state *st = iio_priv(indio_dev);
-
- insw((unsigned long)st->base_address, buf, count);
-
- return 0;
-}
-
-static const struct ad7606_bus_ops ad7606_par16_bops = {
- .read_block = ad7606_par16_read_block,
-};
-
-static int ad7606_par8_read_block(struct device *dev,
- int count, void *buf)
-{
- struct platform_device *pdev = to_platform_device(dev);
- struct iio_dev *indio_dev = platform_get_drvdata(pdev);
- struct ad7606_state *st = iio_priv(indio_dev);
-
- insb((unsigned long)st->base_address, buf, count * 2);
-
- return 0;
-}
-
-static const struct ad7606_bus_ops ad7606_par8_bops = {
- .read_block = ad7606_par8_read_block,
-};
-
-static int ad7606_par_probe(struct platform_device *pdev)
-{
- const struct platform_device_id *id = platform_get_device_id(pdev);
- struct resource *res;
- void __iomem *addr;
- resource_size_t remap_size;
- int irq;
-
- irq = platform_get_irq(pdev, 0);
- if (irq < 0) {
- dev_err(&pdev->dev, "no irq\n");
- return -ENODEV;
- }
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- addr = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(addr))
- return PTR_ERR(addr);
-
- remap_size = resource_size(res);
-
- return ad7606_probe(&pdev->dev, irq, addr,
- id->name, id->driver_data,
- remap_size > 1 ? &ad7606_par16_bops :
- &ad7606_par8_bops);
-}
-
-static int ad7606_par_remove(struct platform_device *pdev)
-{
- return ad7606_remove(&pdev->dev, platform_get_irq(pdev, 0));
-}
-
-static const struct platform_device_id ad7606_driver_ids[] = {
- {
- .name = "ad7606-8",
- .driver_data = ID_AD7606_8,
- }, {
- .name = "ad7606-6",
- .driver_data = ID_AD7606_6,
- }, {
- .name = "ad7606-4",
- .driver_data = ID_AD7606_4,
- },
- { }
-};
-
-MODULE_DEVICE_TABLE(platform, ad7606_driver_ids);
-
-static struct platform_driver ad7606_driver = {
- .probe = ad7606_par_probe,
- .remove = ad7606_par_remove,
- .id_table = ad7606_driver_ids,
- .driver = {
- .name = "ad7606",
- .pm = AD7606_PM_OPS,
- },
-};
-
-module_platform_driver(ad7606_driver);
-
-MODULE_AUTHOR("Michael Hennerich <[email protected]>");
-MODULE_DESCRIPTION("Analog Devices AD7606 ADC");
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/staging/iio/adc/ad7606_spi.c b/drivers/staging/iio/adc/ad7606_spi.c
deleted file mode 100644
index c9b1f266..0000000
--- a/drivers/staging/iio/adc/ad7606_spi.c
+++ /dev/null
@@ -1,78 +0,0 @@
-/*
- * AD7606 SPI ADC driver
- *
- * Copyright 2011 Analog Devices Inc.
- *
- * Licensed under the GPL-2.
- */
-
-#include <linux/module.h>
-#include <linux/spi/spi.h>
-#include <linux/types.h>
-#include <linux/err.h>
-
-#include <linux/iio/iio.h>
-#include "ad7606.h"
-
-#define MAX_SPI_FREQ_HZ 23500000 /* VDRIVE above 4.75 V */
-
-static int ad7606_spi_read_block(struct device *dev,
- int count, void *buf)
-{
- struct spi_device *spi = to_spi_device(dev);
- int i, ret;
- unsigned short *data = buf;
- __be16 *bdata = buf;
-
- ret = spi_read(spi, buf, count * 2);
- if (ret < 0) {
- dev_err(&spi->dev, "SPI read error\n");
- return ret;
- }
-
- for (i = 0; i < count; i++)
- data[i] = be16_to_cpu(bdata[i]);
-
- return 0;
-}
-
-static const struct ad7606_bus_ops ad7606_spi_bops = {
- .read_block = ad7606_spi_read_block,
-};
-
-static int ad7606_spi_probe(struct spi_device *spi)
-{
- const struct spi_device_id *id = spi_get_device_id(spi);
-
- return ad7606_probe(&spi->dev, spi->irq, NULL,
- id->name, id->driver_data,
- &ad7606_spi_bops);
-}
-
-static int ad7606_spi_remove(struct spi_device *spi)
-{
- return ad7606_remove(&spi->dev, spi->irq);
-}
-
-static const struct spi_device_id ad7606_id[] = {
- {"ad7606-8", ID_AD7606_8},
- {"ad7606-6", ID_AD7606_6},
- {"ad7606-4", ID_AD7606_4},
- {}
-};
-MODULE_DEVICE_TABLE(spi, ad7606_id);
-
-static struct spi_driver ad7606_driver = {
- .driver = {
- .name = "ad7606",
- .pm = AD7606_PM_OPS,
- },
- .probe = ad7606_spi_probe,
- .remove = ad7606_spi_remove,
- .id_table = ad7606_id,
-};
-module_spi_driver(ad7606_driver);
-
-MODULE_AUTHOR("Michael Hennerich <[email protected]>");
-MODULE_DESCRIPTION("Analog Devices AD7606 ADC");
-MODULE_LICENSE("GPL v2");
--
2.7.4

2016-11-11 14:18:43

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: iio: ad7606: replace range/range_available with corresponding scale

On 11/11/2016 07:34 AM, Eva Rachel Retuya wrote:
> Eliminate the non-standard attribute in_voltage_range and move its
> functionality under the scale attribute. read_raw() has been taken care
> of previously so only write_raw() is handled here.
>
> Additionally, rename the attribute in_voltage_range_available into
> in_voltage_scale_available.
>
> Suggested-by: Lars-Peter Clausen <[email protected]>
> Signed-off-by: Eva Rachel Retuya <[email protected]>

Hi,

Thanks for the patch. Unfortunately this is not quite this straight forward.

The scale is what you multiply the raw with to get the value in the standard
IIO unit. Range as implemented in this driver is the maximum output voltage.

To get the scale we need to look at the transfer function of the ADC [1].
The transfer function tells us that 1 LSB is 305uV for the 10V range and
152uV for the 5V range.

More specifically this is $RANGE*2/2**16 (times two since the ADC is bipolar).

Since the default unit for IIO is mV for voltages we need to multiply this
by 1000.

The other thing we need to handle is the case where the RANGE pin is not
connected to a GPIO but either hardwired to 1 or 0. Which we need to handle
somehow.

[1]
http://www.analog.com/media/en/technical-documentation/data-sheets/AD7606_7606-6_7606-4.pdf#page=23
(right bottom)

> ---
> drivers/staging/iio/adc/ad7606.c | 56 ++++++++++++----------------------------
> 1 file changed, 16 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
> index 4531908..cceb18c 100644
> --- a/drivers/staging/iio/adc/ad7606.c
> +++ b/drivers/staging/iio/adc/ad7606.c
> @@ -161,42 +161,7 @@ static int ad7606_read_raw(struct iio_dev *indio_dev,
> return -EINVAL;
> }
>
> -static ssize_t ad7606_show_range(struct device *dev,
> - struct device_attribute *attr, char *buf)
> -{
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ad7606_state *st = iio_priv(indio_dev);
> -
> - return sprintf(buf, "%u\n", st->range);
> -}
> -
> -static ssize_t ad7606_store_range(struct device *dev,
> - struct device_attribute *attr,
> - const char *buf, size_t count)
> -{
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ad7606_state *st = iio_priv(indio_dev);
> - unsigned long lval;
> - int ret;
> -
> - ret = kstrtoul(buf, 10, &lval);
> - if (ret)
> - return ret;
> -
> - if (!(lval == 5000 || lval == 10000))
> - return -EINVAL;
> -
> - mutex_lock(&indio_dev->mlock);
> - gpiod_set_value(st->gpio_range, lval == 10000);
> - st->range = lval;
> - mutex_unlock(&indio_dev->mlock);
> -
> - return count;
> -}
> -
> -static IIO_DEVICE_ATTR(in_voltage_range, S_IRUGO | S_IWUSR,
> - ad7606_show_range, ad7606_store_range, 0);
> -static IIO_CONST_ATTR(in_voltage_range_available, "5000 10000");
> +static IIO_CONST_ATTR(in_voltage_scale_available, "5000 10000");
>
> static int ad7606_oversampling_get_index(unsigned int val)
> {
> @@ -221,6 +186,19 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
> int ret;
>
> switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + if (val2)
> + return -EINVAL;
> +
> + if (!(val == 5000 || val == 10000))
> + return -EINVAL;
> +
> + mutex_lock(&indio_dev->mlock);
> + gpiod_set_value(st->gpio_range, val == 10000);
> + st->range = val;
> + mutex_unlock(&indio_dev->mlock);
> +
> + return 0;
> case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> if (val2)
> return -EINVAL;
> @@ -247,8 +225,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
> static IIO_CONST_ATTR(oversampling_ratio_available, "1 2 4 8 16 32 64");
>
> static struct attribute *ad7606_attributes_os_and_range[] = {
> - &iio_dev_attr_in_voltage_range.dev_attr.attr,
> - &iio_const_attr_in_voltage_range_available.dev_attr.attr,
> + &iio_const_attr_in_voltage_scale_available.dev_attr.attr,
> &iio_const_attr_oversampling_ratio_available.dev_attr.attr,
> NULL,
> };
> @@ -267,8 +244,7 @@ static const struct attribute_group ad7606_attribute_group_os = {
> };
>
> static struct attribute *ad7606_attributes_range[] = {
> - &iio_dev_attr_in_voltage_range.dev_attr.attr,
> - &iio_const_attr_in_voltage_range_available.dev_attr.attr,
> + &iio_const_attr_in_voltage_scale_available.dev_attr.attr,
> NULL,
> };
>
>

2016-11-11 14:22:14

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH 2/2] staging: iio: ad7606: move out of staging

On 11/11/2016 07:34 AM, Eva Rachel Retuya wrote:
> Move the ad7606 driver from staging/iio/adc to iio/adc. Also, update the
> corresponding Makefile and Kconfig associated with the change.

This is obviously OK, but when you generate a patch that moves files use
`git format-patch -M ...`. This will generate a more compact patch.

2016-11-11 16:11:16

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] staging: iio: ad7606: move out of staging

Hi Eva,

[auto build test WARNING on iio/togreg]
[also build test WARNING on next-20161111]
[cannot apply to v4.9-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Eva-Rachel-Retuya/staging-iio-ad7606-move-driver-out-of-staging/20161111-143836
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sh

All warnings (new ones prefixed by >>):

drivers/iio/adc/ad7606_par.c: In function 'ad7606_par16_read_block':
>> drivers/iio/adc/ad7606_par.c:23:23: warning: unused variable 'st' [-Wunused-variable]
struct ad7606_state *st = iio_priv(indio_dev);
^~
drivers/iio/adc/ad7606_par.c: In function 'ad7606_par8_read_block':
drivers/iio/adc/ad7606_par.c:39:23: warning: unused variable 'st' [-Wunused-variable]
struct ad7606_state *st = iio_priv(indio_dev);
^~

vim +/st +23 drivers/iio/adc/ad7606_par.c

b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 7 */
b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 8
b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 9 #include <linux/module.h>
b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 10 #include <linux/platform_device.h>
b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 11 #include <linux/types.h>
b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 12 #include <linux/err.h>
b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 13 #include <linux/io.h>
b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 14
06458e27 drivers/staging/iio/adc/ad7606_par.c Jonathan Cameron 2012-04-25 15 #include <linux/iio/iio.h>
b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 16 #include "ad7606.h"
b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 17
b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 18 static int ad7606_par16_read_block(struct device *dev,
b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 19 int count, void *buf)
b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 20 {
b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 21 struct platform_device *pdev = to_platform_device(dev);
e61181d0 drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-05-18 22 struct iio_dev *indio_dev = platform_get_drvdata(pdev);
e61181d0 drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-05-18 @23 struct ad7606_state *st = iio_priv(indio_dev);
b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 24
b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 25 insw((unsigned long)st->base_address, buf, count);
b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 26
b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 27 return 0;
b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 28 }
b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 29
b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 30 static const struct ad7606_bus_ops ad7606_par16_bops = {
b9618c0c drivers/staging/iio/adc/ad7606_par.c Michael Hennerich 2011-02-22 31 .read_block = ad7606_par16_read_block,

:::::: The code at line 23 was first introduced by commit
:::::: e61181d0a3e6788d57de9c1ae305d1c6f5fabade staging:iio:adc:ad7606: Use private data space from iio_allocate_device

:::::: TO: Michael Hennerich <[email protected]>
:::::: CC: Greg Kroah-Hartman <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (4.29 kB)
.config.gz (41.26 kB)
Download all attachments

2016-11-12 14:25:01

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: iio: ad7606: replace range/range_available with corresponding scale

On 11/11/16 14:18, Lars-Peter Clausen wrote:
> On 11/11/2016 07:34 AM, Eva Rachel Retuya wrote:
>> Eliminate the non-standard attribute in_voltage_range and move its
>> functionality under the scale attribute. read_raw() has been taken care
>> of previously so only write_raw() is handled here.
>>
>> Additionally, rename the attribute in_voltage_range_available into
>> in_voltage_scale_available.
>>
>> Suggested-by: Lars-Peter Clausen <[email protected]>
>> Signed-off-by: Eva Rachel Retuya <[email protected]>
>
> Hi,
>
> Thanks for the patch. Unfortunately this is not quite this straight forward.
>
> The scale is what you multiply the raw with to get the value in the standard
> IIO unit. Range as implemented in this driver is the maximum output voltage.
>
> To get the scale we need to look at the transfer function of the ADC [1].
> The transfer function tells us that 1 LSB is 305uV for the 10V range and
> 152uV for the 5V range.
>
> More specifically this is $RANGE*2/2**16 (times two since the ADC is bipolar).
>
> Since the default unit for IIO is mV for voltages we need to multiply this
> by 1000.
>
> The other thing we need to handle is the case where the RANGE pin is not
> connected to a GPIO but either hardwired to 1 or 0. Which we need to handle
> somehow.
Is it just me who thought, we need a fixed GPI like a fixed regulator?
Would allow this sort of fixed wiring to be simply defined.

Linus, worth exploring?

I doubt this will be the last case of this particular problem
(not exactly unusual to hard wire control lines like these as which range
makes sense is often a feature of the device).

Would be a pain to have to add code to every driver to cover the fixed
case.
>
> [1]
> http://www.analog.com/media/en/technical-documentation/data-sheets/AD7606_7606-6_7606-4.pdf#page=23
> (right bottom)
>
>> ---
>> drivers/staging/iio/adc/ad7606.c | 56 ++++++++++++----------------------------
>> 1 file changed, 16 insertions(+), 40 deletions(-)
>>
>> diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
>> index 4531908..cceb18c 100644
>> --- a/drivers/staging/iio/adc/ad7606.c
>> +++ b/drivers/staging/iio/adc/ad7606.c
>> @@ -161,42 +161,7 @@ static int ad7606_read_raw(struct iio_dev *indio_dev,
>> return -EINVAL;
>> }
>>
>> -static ssize_t ad7606_show_range(struct device *dev,
>> - struct device_attribute *attr, char *buf)
>> -{
>> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>> - struct ad7606_state *st = iio_priv(indio_dev);
>> -
>> - return sprintf(buf, "%u\n", st->range);
>> -}
>> -
>> -static ssize_t ad7606_store_range(struct device *dev,
>> - struct device_attribute *attr,
>> - const char *buf, size_t count)
>> -{
>> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>> - struct ad7606_state *st = iio_priv(indio_dev);
>> - unsigned long lval;
>> - int ret;
>> -
>> - ret = kstrtoul(buf, 10, &lval);
>> - if (ret)
>> - return ret;
>> -
>> - if (!(lval == 5000 || lval == 10000))
>> - return -EINVAL;
>> -
>> - mutex_lock(&indio_dev->mlock);
>> - gpiod_set_value(st->gpio_range, lval == 10000);
>> - st->range = lval;
>> - mutex_unlock(&indio_dev->mlock);
>> -
>> - return count;
>> -}
>> -
>> -static IIO_DEVICE_ATTR(in_voltage_range, S_IRUGO | S_IWUSR,
>> - ad7606_show_range, ad7606_store_range, 0);
>> -static IIO_CONST_ATTR(in_voltage_range_available, "5000 10000");
>> +static IIO_CONST_ATTR(in_voltage_scale_available, "5000 10000");
>>
>> static int ad7606_oversampling_get_index(unsigned int val)
>> {
>> @@ -221,6 +186,19 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
>> int ret;
>>
>> switch (mask) {
>> + case IIO_CHAN_INFO_SCALE:
>> + if (val2)
>> + return -EINVAL;
>> +
>> + if (!(val == 5000 || val == 10000))
>> + return -EINVAL;
>> +
>> + mutex_lock(&indio_dev->mlock);
>> + gpiod_set_value(st->gpio_range, val == 10000);
>> + st->range = val;
>> + mutex_unlock(&indio_dev->mlock);
>> +
>> + return 0;
>> case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
>> if (val2)
>> return -EINVAL;
>> @@ -247,8 +225,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
>> static IIO_CONST_ATTR(oversampling_ratio_available, "1 2 4 8 16 32 64");
>>
>> static struct attribute *ad7606_attributes_os_and_range[] = {
>> - &iio_dev_attr_in_voltage_range.dev_attr.attr,
>> - &iio_const_attr_in_voltage_range_available.dev_attr.attr,
>> + &iio_const_attr_in_voltage_scale_available.dev_attr.attr,
>> &iio_const_attr_oversampling_ratio_available.dev_attr.attr,
>> NULL,
>> };
>> @@ -267,8 +244,7 @@ static const struct attribute_group ad7606_attribute_group_os = {
>> };
>>
>> static struct attribute *ad7606_attributes_range[] = {
>> - &iio_dev_attr_in_voltage_range.dev_attr.attr,
>> - &iio_const_attr_in_voltage_range_available.dev_attr.attr,
>> + &iio_const_attr_in_voltage_scale_available.dev_attr.attr,
>> NULL,
>> };
>>
>>
>

2016-11-12 14:26:55

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 2/2] staging: iio: ad7606: move out of staging

On 11/11/16 14:22, Lars-Peter Clausen wrote:
> On 11/11/2016 07:34 AM, Eva Rachel Retuya wrote:
>> Move the ad7606 driver from staging/iio/adc to iio/adc. Also, update the
>> corresponding Makefile and Kconfig associated with the change.
>
> This is obviously OK, but when you generate a patch that moves files use
> `git format-patch -M ...`. This will generate a more compact patch.
>
I tend to make an exception for staging graduation patches.
The mere posting of the whole driver by not using the move detection
makes for really easy review of whether we have forgotten something
else that needs cleanup before the graduation.

Most of the time -M is much more sensible!

Jonathan

2016-11-12 14:32:52

by Eva Rachel Retuya

[permalink] [raw]
Subject: Re: [PATCH 2/2] staging: iio: ad7606: move out of staging

On Sat, Nov 12, 2016 at 02:26:51PM +0000, Jonathan Cameron wrote:
> On 11/11/16 14:22, Lars-Peter Clausen wrote:
> > On 11/11/2016 07:34 AM, Eva Rachel Retuya wrote:
> >> Move the ad7606 driver from staging/iio/adc to iio/adc. Also, update the
> >> corresponding Makefile and Kconfig associated with the change.
> >
> > This is obviously OK, but when you generate a patch that moves files use
> > `git format-patch -M ...`. This will generate a more compact patch.
> >
> I tend to make an exception for staging graduation patches.
> The mere posting of the whole driver by not using the move detection
> makes for really easy review of whether we have forgotten something
> else that needs cleanup before the graduation.
>
> Most of the time -M is much more sensible!
>
> Jonathan

Should I use this -M on version 2 or still go with the previous method?

Thanks,
Eva

2016-11-12 14:40:40

by Eva Rachel Retuya

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: iio: ad7606: replace range/range_available with corresponding scale

Hello,

Thanks for explaining it. Now I understand better why read_raw is
formatted in that manner. I have some questions in-line:

On Fri, Nov 11, 2016 at 03:18:37PM +0100, Lars-Peter Clausen wrote:
> On 11/11/2016 07:34 AM, Eva Rachel Retuya wrote:
> > Eliminate the non-standard attribute in_voltage_range and move its
> > functionality under the scale attribute. read_raw() has been taken care
> > of previously so only write_raw() is handled here.
> >
> > Additionally, rename the attribute in_voltage_range_available into
> > in_voltage_scale_available.
> >
> > Suggested-by: Lars-Peter Clausen <[email protected]>
> > Signed-off-by: Eva Rachel Retuya <[email protected]>
>
> Hi,
>
> Thanks for the patch. Unfortunately this is not quite this straight forward.
>
> The scale is what you multiply the raw with to get the value in the standard
> IIO unit. Range as implemented in this driver is the maximum output voltage.
>
> To get the scale we need to look at the transfer function of the ADC [1].
> The transfer function tells us that 1 LSB is 305uV for the 10V range and
> 152uV for the 5V range.

Instead of 10000 and 5000, these will be the values for the
scale_available attribute?

>
> More specifically this is $RANGE*2/2**16 (times two since the ADC is bipolar).
>
> Since the default unit for IIO is mV for voltages we need to multiply this
> by 1000.
>
> The other thing we need to handle is the case where the RANGE pin is not
> connected to a GPIO but either hardwired to 1 or 0. Which we need to handle
> somehow.

Will this be on the same patch or a separate one? Can you please give a
hint on how to check if it is hardwired?

Eva

>
> [1]
> http://www.analog.com/media/en/technical-documentation/data-sheets/AD7606_7606-6_7606-4.pdf#page=23
> (right bottom)
>
> > ---
> > drivers/staging/iio/adc/ad7606.c | 56 ++++++++++++----------------------------
> > 1 file changed, 16 insertions(+), 40 deletions(-)
> >
> > diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
> > index 4531908..cceb18c 100644
> > --- a/drivers/staging/iio/adc/ad7606.c
> > +++ b/drivers/staging/iio/adc/ad7606.c
> > @@ -161,42 +161,7 @@ static int ad7606_read_raw(struct iio_dev *indio_dev,
> > return -EINVAL;
> > }
> >
> > -static ssize_t ad7606_show_range(struct device *dev,
> > - struct device_attribute *attr, char *buf)
> > -{
> > - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > - struct ad7606_state *st = iio_priv(indio_dev);
> > -
> > - return sprintf(buf, "%u\n", st->range);
> > -}
> > -
> > -static ssize_t ad7606_store_range(struct device *dev,
> > - struct device_attribute *attr,
> > - const char *buf, size_t count)
> > -{
> > - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > - struct ad7606_state *st = iio_priv(indio_dev);
> > - unsigned long lval;
> > - int ret;
> > -
> > - ret = kstrtoul(buf, 10, &lval);
> > - if (ret)
> > - return ret;
> > -
> > - if (!(lval == 5000 || lval == 10000))
> > - return -EINVAL;
> > -
> > - mutex_lock(&indio_dev->mlock);
> > - gpiod_set_value(st->gpio_range, lval == 10000);
> > - st->range = lval;
> > - mutex_unlock(&indio_dev->mlock);
> > -
> > - return count;
> > -}
> > -
> > -static IIO_DEVICE_ATTR(in_voltage_range, S_IRUGO | S_IWUSR,
> > - ad7606_show_range, ad7606_store_range, 0);
> > -static IIO_CONST_ATTR(in_voltage_range_available, "5000 10000");
> > +static IIO_CONST_ATTR(in_voltage_scale_available, "5000 10000");
> >
> > static int ad7606_oversampling_get_index(unsigned int val)
> > {
> > @@ -221,6 +186,19 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
> > int ret;
> >
> > switch (mask) {
> > + case IIO_CHAN_INFO_SCALE:
> > + if (val2)
> > + return -EINVAL;
> > +
> > + if (!(val == 5000 || val == 10000))
> > + return -EINVAL;
> > +
> > + mutex_lock(&indio_dev->mlock);
> > + gpiod_set_value(st->gpio_range, val == 10000);
> > + st->range = val;
> > + mutex_unlock(&indio_dev->mlock);
> > +
> > + return 0;
> > case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> > if (val2)
> > return -EINVAL;
> > @@ -247,8 +225,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
> > static IIO_CONST_ATTR(oversampling_ratio_available, "1 2 4 8 16 32 64");
> >
> > static struct attribute *ad7606_attributes_os_and_range[] = {
> > - &iio_dev_attr_in_voltage_range.dev_attr.attr,
> > - &iio_const_attr_in_voltage_range_available.dev_attr.attr,
> > + &iio_const_attr_in_voltage_scale_available.dev_attr.attr,
> > &iio_const_attr_oversampling_ratio_available.dev_attr.attr,
> > NULL,
> > };
> > @@ -267,8 +244,7 @@ static const struct attribute_group ad7606_attribute_group_os = {
> > };
> >
> > static struct attribute *ad7606_attributes_range[] = {
> > - &iio_dev_attr_in_voltage_range.dev_attr.attr,
> > - &iio_const_attr_in_voltage_range_available.dev_attr.attr,
> > + &iio_const_attr_in_voltage_scale_available.dev_attr.attr,
> > NULL,
> > };
> >
> >
>

2016-11-12 14:42:14

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 2/2] staging: iio: ad7606: move out of staging

On 12/11/16 14:32, Eva Rachel Retuya wrote:
> On Sat, Nov 12, 2016 at 02:26:51PM +0000, Jonathan Cameron wrote:
>> On 11/11/16 14:22, Lars-Peter Clausen wrote:
>>> On 11/11/2016 07:34 AM, Eva Rachel Retuya wrote:
>>>> Move the ad7606 driver from staging/iio/adc to iio/adc. Also, update the
>>>> corresponding Makefile and Kconfig associated with the change.
>>>
>>> This is obviously OK, but when you generate a patch that moves files use
>>> `git format-patch -M ...`. This will generate a more compact patch.
>>>
>> I tend to make an exception for staging graduation patches.
>> The mere posting of the whole driver by not using the move detection
>> makes for really easy review of whether we have forgotten something
>> else that needs cleanup before the graduation.
>>
>> Most of the time -M is much more sensible!
>>
>> Jonathan
>
> Should I use this -M on version 2 or still go with the previous method?
>
> Thanks,
> Eva
>
Go without the no -M option and face the ire of Lars ;)

J

2016-11-14 10:25:32

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: iio: ad7606: replace range/range_available with corresponding scale

On 11/12/2016 03:22 PM, Eva Rachel Retuya wrote:
> Hello,
>
> Thanks for explaining it. Now I understand better why read_raw is
> formatted in that manner. I have some questions in-line:
>
> On Fri, Nov 11, 2016 at 03:18:37PM +0100, Lars-Peter Clausen wrote:
>> On 11/11/2016 07:34 AM, Eva Rachel Retuya wrote:
>>> Eliminate the non-standard attribute in_voltage_range and move its
>>> functionality under the scale attribute. read_raw() has been taken care
>>> of previously so only write_raw() is handled here.
>>>
>>> Additionally, rename the attribute in_voltage_range_available into
>>> in_voltage_scale_available.
>>>
>>> Suggested-by: Lars-Peter Clausen <[email protected]>
>>> Signed-off-by: Eva Rachel Retuya <[email protected]>
>>
>> Hi,
>>
>> Thanks for the patch. Unfortunately this is not quite this straight forward.
>>
>> The scale is what you multiply the raw with to get the value in the standard
>> IIO unit. Range as implemented in this driver is the maximum output voltage.
>>
>> To get the scale we need to look at the transfer function of the ADC [1].
>> The transfer function tells us that 1 LSB is 305uV for the 10V range and
>> 152uV for the 5V range.
>
> Instead of 10000 and 5000, these will be the values for the
> scale_available attribute?

Yes. But these are slightly truncated values, we should try to minimize the
rounding error introduced by those scales by making them as accurate as
possible. The exact values are 5/2**16 and 2.5/2**16. Neither of which we
can express accurately. But we should use at least 6 digits and also round
to the closest rather than truncating.

>
>>
>> More specifically this is $RANGE*2/2**16 (times two since the ADC is bipolar).
>>
>> Since the default unit for IIO is mV for voltages we need to multiply this
>> by 1000.
>>
>> The other thing we need to handle is the case where the RANGE pin is not
>> connected to a GPIO but either hardwired to 1 or 0. Which we need to handle
>> somehow.
>
> Will this be on the same patch or a separate one? Can you please give a
> hint on how to check if it is hardwired?

The current driver checks if the GPIO is set and if it is not it does not
register the range attribute. As a first step we could mimic this behavior,
but long term we should find a way to discover the setting of the hardwired
pin and report the scale accordingly.

2016-11-14 10:30:58

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: iio: ad7606: replace range/range_available with corresponding scale

On 11/12/2016 03:24 PM, Jonathan Cameron wrote:
> On 11/11/16 14:18, Lars-Peter Clausen wrote:
>> On 11/11/2016 07:34 AM, Eva Rachel Retuya wrote:
>>> Eliminate the non-standard attribute in_voltage_range and move its
>>> functionality under the scale attribute. read_raw() has been taken care
>>> of previously so only write_raw() is handled here.
>>>
>>> Additionally, rename the attribute in_voltage_range_available into
>>> in_voltage_scale_available.
>>>
>>> Suggested-by: Lars-Peter Clausen <[email protected]>
>>> Signed-off-by: Eva Rachel Retuya <[email protected]>
>>
>> Hi,
>>
>> Thanks for the patch. Unfortunately this is not quite this straight forward.
>>
>> The scale is what you multiply the raw with to get the value in the standard
>> IIO unit. Range as implemented in this driver is the maximum output voltage.
>>
>> To get the scale we need to look at the transfer function of the ADC [1].
>> The transfer function tells us that 1 LSB is 305uV for the 10V range and
>> 152uV for the 5V range.
>>
>> More specifically this is $RANGE*2/2**16 (times two since the ADC is bipolar).
>>
>> Since the default unit for IIO is mV for voltages we need to multiply this
>> by 1000.
>>
>> The other thing we need to handle is the case where the RANGE pin is not
>> connected to a GPIO but either hardwired to 1 or 0. Which we need to handle
>> somehow.
> Is it just me who thought, we need a fixed GPI like a fixed regulator?
> Would allow this sort of fixed wiring to be simply defined.
>
> Linus, worth exploring?
>
> I doubt this will be the last case of this particular problem
> (not exactly unusual to hard wire control lines like these as which range
> makes sense is often a feature of the device).
>
> Would be a pain to have to add code to every driver to cover the fixed
> case.

We still have to add code to every driver to cover the fixed case since the
mode of operation is inherently different. But it would be nice to have a
coherent way of doing so with a standardized interface rather than having
every device come up with its own code and bindings.

This could either be handled directly by the GPIO API or by a small set of
helper functions on top of it.

I think the most important part for now is to agree on a standard binding to
describe hardwired GPIOs. We can still rework the kernel API later on, but
the DT bindings will be set in stone.

2016-11-14 16:59:10

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: iio: ad7606: replace range/range_available with corresponding scale

On Sat, Nov 12, 2016 at 3:24 PM, Jonathan Cameron <[email protected]> wrote:

> Is it just me who thought, we need a fixed GPI like a fixed regulator?
> Would allow this sort of fixed wiring to be simply defined.
>
> Linus, worth exploring?

So if fixed regulator is for a voltage provider, this would be
pretty much the inverse: deciding for a voltage range by switching
a GPIO.

No previous experience with that, it should be outside of the
GPIO subsystem for sure...

Yours,
Linus Walleij

2016-11-14 17:26:28

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: iio: ad7606: replace range/range_available with corresponding scale



On 14 November 2016 10:30:50 GMT+00:00, Lars-Peter Clausen <[email protected]> wrote:
>On 11/12/2016 03:24 PM, Jonathan Cameron wrote:
>> On 11/11/16 14:18, Lars-Peter Clausen wrote:
>>> On 11/11/2016 07:34 AM, Eva Rachel Retuya wrote:
>>>> Eliminate the non-standard attribute in_voltage_range and move its
>>>> functionality under the scale attribute. read_raw() has been taken
>care
>>>> of previously so only write_raw() is handled here.
>>>>
>>>> Additionally, rename the attribute in_voltage_range_available into
>>>> in_voltage_scale_available.
>>>>
>>>> Suggested-by: Lars-Peter Clausen <[email protected]>
>>>> Signed-off-by: Eva Rachel Retuya <[email protected]>
>>>
>>> Hi,
>>>
>>> Thanks for the patch. Unfortunately this is not quite this straight
>forward.
>>>
>>> The scale is what you multiply the raw with to get the value in the
>standard
>>> IIO unit. Range as implemented in this driver is the maximum output
>voltage.
>>>
>>> To get the scale we need to look at the transfer function of the ADC
>[1].
>>> The transfer function tells us that 1 LSB is 305uV for the 10V range
>and
>>> 152uV for the 5V range.
>>>
>>> More specifically this is $RANGE*2/2**16 (times two since the ADC is
>bipolar).
>>>
>>> Since the default unit for IIO is mV for voltages we need to
>multiply this
>>> by 1000.
>>>
>>> The other thing we need to handle is the case where the RANGE pin is
>not
>>> connected to a GPIO but either hardwired to 1 or 0. Which we need to
>handle
>>> somehow.
>> Is it just me who thought, we need a fixed GPI like a fixed
>regulator?
>> Would allow this sort of fixed wiring to be simply defined.
>>
>> Linus, worth exploring?
>>
>> I doubt this will be the last case of this particular problem
>> (not exactly unusual to hard wire control lines like these as which
>range
>> makes sense is often a feature of the device).
>>
>> Would be a pain to have to add code to every driver to cover the
>fixed
>> case.
>
>We still have to add code to every driver to cover the fixed case since
>the
>mode of operation is inherently different. But it would be nice to have
>a
>coherent way of doing so with a standardized interface rather than
>having
>every device come up with its own code and bindings.
Agreed. Not totally obvious how to do it but definitely don't want to work around it by custom bindings all over the place.

A simple is_gpo_fixed or similar would get the driver the info it needs. Could do it with errors
but that would perhaps be ugly!
The other flags related to type of input/output can be emulated so there doesn't seem to be anything similar...
>
>This could either be handled directly by the GPIO API or by a small set
>of
>helper functions on top of it.
>
>I think the most important part for now is to agree on a standard
>binding to
>describe hardwired GPIOs. We can still rework the kernel API later on,
>but
>the DT bindings will be set in stone.
Agreed. Some magic gpio description or a fake gpio chip similar to fixed regs?

Jonathan
>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

2016-11-14 18:53:40

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: iio: ad7606: replace range/range_available with corresponding scale

On 11/14/2016 05:58 PM, Linus Walleij wrote:
> On Sat, Nov 12, 2016 at 3:24 PM, Jonathan Cameron <[email protected]> wrote:
>
>> Is it just me who thought, we need a fixed GPI like a fixed regulator?
>> Would allow this sort of fixed wiring to be simply defined.
>>
>> Linus, worth exploring?
>
> So if fixed regulator is for a voltage provider, this would be
> pretty much the inverse: deciding for a voltage range by switching
> a GPIO.

It's about figuring out the setting of a "GPIO" that can't be changed from
software.

Devices sometimes, instead of a configuration bus like I2C or SPI, use
simple input pins, that can either be set to high or low, to allow software
the state of the device. The GPIO API is typically used to configure these pins.

This works fine as long as the pin is connected to a GPIO. But sometimes the
system designer decides that a settings does not need to be configurable, in
this case the pin will be tied to logic low or high directly on the PCB
without any GPIO controller being involved.

Sometimes a driver wants to know how the pin is wired up so it can report to
userspace this part runs in the following mode and the mode can't be
changed. In a sense it is like a reverse GPIO hog.

Considering that this is a common usecase the question was how this can be
implemented in a driver independent way to avoid code duplication and
slightly different variations of what is effectively the same DT/ACPI binding.

E.g. lets say for a configurable pin you use

range-gpio = <&gpio ...>;

and for a static pin

range-gpio-fixed = <1>;

Or something similar.

2016-11-14 22:15:13

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: iio: ad7606: replace range/range_available with corresponding scale



On 14 November 2016 18:53:28 GMT+00:00, Lars-Peter Clausen <[email protected]> wrote:
>On 11/14/2016 05:58 PM, Linus Walleij wrote:
>> On Sat, Nov 12, 2016 at 3:24 PM, Jonathan Cameron <[email protected]>
>wrote:
>>
>>> Is it just me who thought, we need a fixed GPI like a fixed
>regulator?
Probably didn't help clarity that I described it as an input pin whereas it's kind of like having an
output pin whose state you can't change...

>>> Would allow this sort of fixed wiring to be simply defined.
>>>
>>> Linus, worth exploring?
>>
>> So if fixed regulator is for a voltage provider, this would be
>> pretty much the inverse: deciding for a voltage range by switching
>> a GPIO.
>
>It's about figuring out the setting of a "GPIO" that can't be changed
>from
>software.
>
>Devices sometimes, instead of a configuration bus like I2C or SPI, use
>simple input pins, that can either be set to high or low, to allow
>software
>the state of the device. The GPIO API is typically used to configure
>these pins.
>
>This works fine as long as the pin is connected to a GPIO. But
>sometimes the
>system designer decides that a settings does not need to be
>configurable, in
>this case the pin will be tied to logic low or high directly on the PCB
>without any GPIO controller being involved.
>
>Sometimes a driver wants to know how the pin is wired up so it can
>report to
>userspace this part runs in the following mode and the mode can't be
>changed. In a sense it is like a reverse GPIO hog.
>
>Considering that this is a common usecase the question was how this can
>be
>implemented in a driver independent way to avoid code duplication and
>slightly different variations of what is effectively the same DT/ACPI
>binding.
>
>E.g. lets say for a configurable pin you use
>
> range-gpio = <&gpio ...>;
>
>and for a static pin
>
> range-gpio-fixed = <1>;
>
>Or something similar.
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

2016-11-14 23:12:58

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: iio: ad7606: replace range/range_available with corresponding scale

On Mon, Nov 14, 2016 at 7:53 PM, Lars-Peter Clausen <[email protected]> wrote:

> It's about figuring out the setting of a "GPIO" that can't be changed from
> software.
>
> Devices sometimes, instead of a configuration bus like I2C or SPI, use
> simple input pins, that can either be set to high or low, to allow software
> the state of the device. The GPIO API is typically used to configure these pins.
>
> This works fine as long as the pin is connected to a GPIO. But sometimes the
> system designer decides that a settings does not need to be configurable, in
> this case the pin will be tied to logic low or high directly on the PCB
> without any GPIO controller being involved.
>
> Sometimes a driver wants to know how the pin is wired up so it can report to
> userspace this part runs in the following mode and the mode can't be
> changed. In a sense it is like a reverse GPIO hog.
>
> Considering that this is a common usecase the question was how this can be
> implemented in a driver independent way to avoid code duplication and
> slightly different variations of what is effectively the same DT/ACPI binding.
>
> E.g. lets say for a configurable pin you use
>
> range-gpio = <&gpio ...>;
>
> and for a static pin
>
> range-gpio-fixed = <1>;
>
> Or something similar.

Aha I understand.

Usually I feel we need not shoehorn stuff into GPIO because it is convenient,
it might be best to leave the GPIO optional and if it is not there, look for
a custom attribute that represents the "hogging" to 0/1. I think trying
to extend GPIO bindings to cover it is overgeneralization, instead go
for a local binding for this kind of devices.

But mainly it is a question to the DT bindings maintainers.

Yours,
Linus Walleij

2016-11-19 12:32:47

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: iio: ad7606: replace range/range_available with corresponding scale

On 14/11/16 23:12, Linus Walleij wrote:
> On Mon, Nov 14, 2016 at 7:53 PM, Lars-Peter Clausen <[email protected]> wrote:
>
>> It's about figuring out the setting of a "GPIO" that can't be changed from
>> software.
>>
>> Devices sometimes, instead of a configuration bus like I2C or SPI, use
>> simple input pins, that can either be set to high or low, to allow software
>> the state of the device. The GPIO API is typically used to configure these pins.
>>
>> This works fine as long as the pin is connected to a GPIO. But sometimes the
>> system designer decides that a settings does not need to be configurable, in
>> this case the pin will be tied to logic low or high directly on the PCB
>> without any GPIO controller being involved.
>>
>> Sometimes a driver wants to know how the pin is wired up so it can report to
>> userspace this part runs in the following mode and the mode can't be
>> changed. In a sense it is like a reverse GPIO hog.
>>
>> Considering that this is a common usecase the question was how this can be
>> implemented in a driver independent way to avoid code duplication and
>> slightly different variations of what is effectively the same DT/ACPI binding.
>>
>> E.g. lets say for a configurable pin you use
>>
>> range-gpio = <&gpio ...>;
>>
>> and for a static pin
>>
>> range-gpio-fixed = <1>;
>>
>> Or something similar.
>
> Aha I understand.
>
> Usually I feel we need not shoehorn stuff into GPIO because it is convenient,
> it might be best to leave the GPIO optional and if it is not there, look for
> a custom attribute that represents the "hogging" to 0/1. I think trying
> to extend GPIO bindings to cover it is overgeneralization, instead go
> for a local binding for this kind of devices.
>
> But mainly it is a question to the DT bindings maintainers.
That's a reasonable approach, but I'd certainly like to see a generic binding
to describe it. It's a pretty common situation.

Seems more likely we'll get a device tree maintainer response if we cc them ;)

So Mark, Rob and thoughts on this?

Thanks,

Jonathan
>
> Yours,
> Linus Walleij
>