2019-10-29 19:39:36

by Beniamin Bia

[permalink] [raw]
Subject: [PATCH v2 1/4] iio: adc: Add support for AD7091R5 ADC

From: Paul Cercueil <[email protected]>

AD7091 is 4-Channel, I2C, Ultra Low Power,12-Bit ADC.

Datasheet:
Link: https://www.analog.com/media/en/technical-documentation/data-sheets/ad7091r-5.pdf

Signed-off-by: Paul Cercueil <[email protected]>
Co-developed-by: Beniamin Bia <[email protected]>
Signed-off-by: Beniamin Bia <[email protected]>
---
Changes in v2:
-blank lines removed
-prefix added to macros
-comments rework
-error checking syntax changed
-iio mutex replaced by a mutex
-device remove function was removed and later replaced by devm_add_action
-regmap include removed from header

drivers/iio/adc/Kconfig | 7 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ad7091r-base.c | 264 +++++++++++++++++++++++++++++++++
drivers/iio/adc/ad7091r-base.h | 25 ++++
drivers/iio/adc/ad7091r5.c | 102 +++++++++++++
5 files changed, 399 insertions(+)
create mode 100644 drivers/iio/adc/ad7091r-base.c
create mode 100644 drivers/iio/adc/ad7091r-base.h
create mode 100644 drivers/iio/adc/ad7091r5.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 7e3286265a38..80b1b9551749 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -22,6 +22,13 @@ config AD7124
To compile this driver as a module, choose M here: the module will be
called ad7124.

+config AD7091R5
+ tristate "Analog Devices AD7091R5 ADC Driver"
+ depends on I2C
+ select REGMAP_I2C
+ help
+ Say yes here to build support for Analog Devices AD7091R-5 ADC.
+
config AD7266
tristate "Analog Devices AD7265/AD7266 ADC driver"
depends on SPI_MASTER
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index ef9cc485fb67..55e44735aaac 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -6,6 +6,7 @@
# When adding new entries keep the list in alphabetical order
obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
obj-$(CONFIG_AD7124) += ad7124.o
+obj-$(CONFIG_AD7091R5) += ad7091r5.o ad7091r-base.o
obj-$(CONFIG_AD7266) += ad7266.o
obj-$(CONFIG_AD7291) += ad7291.o
obj-$(CONFIG_AD7298) += ad7298.o
diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
new file mode 100644
index 000000000000..c2500f614d54
--- /dev/null
+++ b/drivers/iio/adc/ad7091r-base.c
@@ -0,0 +1,264 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AD7091RX Analog to Digital converter driver
+ *
+ * Copyright 2014-2019 Analog Devices Inc.
+ */
+
+#include <linux/bitops.h>
+#include <linux/iio/events.h>
+#include <linux/iio/iio.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#include "ad7091r-base.h"
+
+#define AD7091R_REG_RESULT 0
+#define AD7091R_REG_CHANNEL 1
+#define AD7091R_REG_CONF 2
+#define AD7091R_REG_ALERT 3
+#define AD7091R_REG_CH_LOW_LIMIT(ch) ((ch) * 3 + 4)
+#define AD7091R_REG_CH_HIGH_LIMIT(ch) ((ch) * 3 + 5)
+#define AD7091R_REG_CH_HYSTERESIS(ch) ((ch) * 3 + 6)
+
+/* AD7091R_REG_RESULT */
+#define AD7091R_REG_RESULT_CH_ID(x) (((x) >> 13) & 0x3)
+#define AD7091R_REG_RESULT_CONV_RESULT(x) ((x) & 0xfff)
+
+/* AD7091R_REG_CONF */
+#define AD7091R_REG_CONF_AUTO BIT(8)
+#define AD7091R_REG_CONF_CMD BIT(10)
+
+#define AD7091R_REG_CONF_MODE_MASK \
+ (AD7091R_REG_CONF_AUTO | AD7091R_REG_CONF_CMD)
+
+enum ad7091r_mode {
+ AD7091R_MODE_SAMPLE,
+ AD7091R_MODE_COMMAND,
+ AD7091R_MODE_AUTOCYCLE,
+};
+
+struct ad7091r_state {
+ struct device *dev;
+ struct regmap *map;
+ const struct ad7091r_chip_info *chip_info;
+ enum ad7091r_mode mode;
+ struct mutex lock;
+};
+
+static int ad7091r_set_mode(struct ad7091r_state *st, enum ad7091r_mode mode)
+{
+ int ret;
+
+ switch (mode) {
+ case AD7091R_MODE_SAMPLE:
+ ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
+ AD7091R_REG_CONF_MODE_MASK, 0);
+ break;
+ case AD7091R_MODE_COMMAND:
+ ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
+ AD7091R_REG_CONF_MODE_MASK,
+ AD7091R_REG_CONF_CMD);
+ break;
+ case AD7091R_MODE_AUTOCYCLE:
+ ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
+ AD7091R_REG_CONF_MODE_MASK,
+ AD7091R_REG_CONF_AUTO);
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ if (ret)
+ return ret;
+
+ st->mode = mode;
+
+ return ret;
+}
+
+static int ad7091r_set_channel(struct ad7091r_state *st, unsigned int channel)
+{
+ unsigned int foo;
+ int ret;
+
+ /* AD7091R_REG_CHANNEL specified which channels to be converted */
+ ret = regmap_write(st->map, AD7091R_REG_CHANNEL,
+ BIT(channel) | (BIT(channel) << 8));
+ if (ret)
+ return ret;
+
+ /*
+ * There is a latency of one conversion before the channel conversion
+ * sequence is updated
+ */
+ return regmap_read(st->map, AD7091R_REG_RESULT, &foo);
+}
+
+static int ad7091r_read_one(struct iio_dev *iio_dev,
+ unsigned int channel, unsigned int *read_val)
+{
+ struct ad7091r_state *st = iio_priv(iio_dev);
+ unsigned int val;
+ int ret;
+
+ ret = ad7091r_set_channel(st, channel);
+ if (ret)
+ return ret;
+
+ ret = regmap_read(st->map, AD7091R_REG_RESULT, &val);
+ if (ret)
+ return ret;
+
+ if (AD7091R_REG_RESULT_CH_ID(val) != channel)
+ return -EIO;
+
+ *read_val = AD7091R_REG_RESULT_CONV_RESULT(val);
+
+ return 0;
+}
+
+static int ad7091r_read_raw(struct iio_dev *iio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long m)
+{
+ struct ad7091r_state *st = iio_priv(iio_dev);
+ unsigned int read_val;
+ int ret;
+
+ mutex_lock(&st->lock);
+
+ switch (m) {
+ case IIO_CHAN_INFO_RAW:
+ if (st->mode != AD7091R_MODE_COMMAND) {
+ ret = -EBUSY;
+ goto unlock;
+ }
+
+ ret = ad7091r_read_one(iio_dev, chan->channel, &read_val);
+ if (ret)
+ goto unlock;
+
+ *val = read_val;
+ ret = IIO_VAL_INT;
+ break;
+
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+unlock:
+ mutex_unlock(&st->lock);
+ return ret;
+}
+
+static const struct iio_info ad7091r_info = {
+ .read_raw = ad7091r_read_raw,
+};
+
+static irqreturn_t ad7091r_event_handler(int irq, void *private)
+{
+ struct ad7091r_state *st = (struct ad7091r_state *) private;
+ struct iio_dev *iio_dev = dev_get_drvdata(st->dev);
+ unsigned int i, read_val;
+ int ret;
+ s64 timestamp = iio_get_time_ns(iio_dev);
+
+ ret = regmap_read(st->map, AD7091R_REG_ALERT, &read_val);
+ if (ret)
+ return IRQ_HANDLED;
+
+ for (i = 0; i < st->chip_info->num_channels; i++) {
+ if (read_val & BIT(i * 2))
+ iio_push_event(iio_dev,
+ IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE, i,
+ IIO_EV_TYPE_THRESH,
+ IIO_EV_DIR_RISING), timestamp);
+ if (read_val & BIT(i * 2 + 1))
+ iio_push_event(iio_dev,
+ IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE, i,
+ IIO_EV_TYPE_THRESH,
+ IIO_EV_DIR_FALLING), timestamp);
+ }
+
+ return IRQ_HANDLED;
+}
+
+int ad7091r_probe(struct device *dev, const char *name,
+ const struct ad7091r_chip_info *chip_info,
+ struct regmap *map, int irq)
+{
+ struct iio_dev *iio_dev;
+ struct ad7091r_state *st;
+ int ret;
+
+ iio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+ if (!iio_dev)
+ return -ENOMEM;
+
+ st = iio_priv(iio_dev);
+ st->dev = dev;
+ st->chip_info = chip_info;
+ st->map = map;
+
+ iio_dev->dev.parent = dev;
+ iio_dev->name = name;
+ iio_dev->info = &ad7091r_info;
+ iio_dev->modes = INDIO_DIRECT_MODE;
+
+ iio_dev->num_channels = chip_info->num_channels;
+ iio_dev->channels = chip_info->channels;
+
+ if (irq) {
+ ret = devm_request_threaded_irq(dev, irq, NULL,
+ ad7091r_event_handler,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT, name, st);
+ if (ret)
+ return ret;
+ }
+
+ /* Use command mode by default to convert only desired channels*/
+ ret = ad7091r_set_mode(st, AD7091R_MODE_COMMAND);
+ if (ret < 0)
+ return ret;
+
+ return iio_device_register(iio_dev);
+}
+EXPORT_SYMBOL_GPL(ad7091r_probe);
+
+static bool ad7091r_writeable_reg(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case AD7091R_REG_RESULT:
+ case AD7091R_REG_ALERT:
+ return false;
+ default:
+ return true;
+ }
+}
+
+static bool ad7091r_volatile_reg(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case AD7091R_REG_RESULT:
+ case AD7091R_REG_ALERT:
+ return true;
+ default:
+ return false;
+ }
+}
+
+const struct regmap_config ad7091r_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 16,
+ .writeable_reg = ad7091r_writeable_reg,
+ .volatile_reg = ad7091r_volatile_reg,
+};
+EXPORT_SYMBOL_GPL(ad7091r_regmap_config);
+
+MODULE_AUTHOR("Beniamin Bia <[email protected]>");
+MODULE_DESCRIPTION("Analog Devices AD7091Rx multi-channel converters");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/adc/ad7091r-base.h b/drivers/iio/adc/ad7091r-base.h
new file mode 100644
index 000000000000..5f1147735953
--- /dev/null
+++ b/drivers/iio/adc/ad7091r-base.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * AD7091RX Analog to Digital converter driver
+ *
+ * Copyright 2014-2019 Analog Devices Inc.
+ */
+
+#ifndef __DRIVERS_IIO_ADC_AD7091R_BASE_H__
+#define __DRIVERS_IIO_ADC_AD7091R_BASE_H__
+
+struct device;
+struct ad7091r_state;
+
+struct ad7091r_chip_info {
+ unsigned int num_channels;
+ const struct iio_chan_spec *channels;
+};
+
+extern const struct regmap_config ad7091r_regmap_config;
+
+int ad7091r_probe(struct device *dev, const char *name,
+ const struct ad7091r_chip_info *chip_info,
+ struct regmap *map, int irq);
+
+#endif /* __DRIVERS_IIO_ADC_AD7091R5_BASE_H__ */
diff --git a/drivers/iio/adc/ad7091r5.c b/drivers/iio/adc/ad7091r5.c
new file mode 100644
index 000000000000..2c44dd02ba1a
--- /dev/null
+++ b/drivers/iio/adc/ad7091r5.c
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AD7091R5 Analog to Digital converter driver
+ *
+ * Copyright 2014-2019 Analog Devices Inc.
+ */
+
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#include "ad7091r-base.h"
+
+static const struct iio_event_spec ad7091r5_events[] = {
+ {
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_RISING,
+ .mask_separate = BIT(IIO_EV_INFO_VALUE) |
+ BIT(IIO_EV_INFO_ENABLE),
+ },
+ {
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_FALLING,
+ .mask_separate = BIT(IIO_EV_INFO_VALUE) |
+ BIT(IIO_EV_INFO_ENABLE),
+ },
+ {
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_EITHER,
+ .mask_separate = BIT(IIO_EV_INFO_HYSTERESIS),
+ },
+};
+
+#define AD7091R_CHANNEL(idx, bits, ev, num_ev) { \
+ .type = IIO_VOLTAGE, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .indexed = 1, \
+ .channel = idx, \
+ .event_spec = ev, \
+ .num_event_specs = num_ev, \
+}
+static const struct iio_chan_spec ad7091r5_channels_irq[] = {
+ AD7091R_CHANNEL(0, 12, ad7091r5_events, ARRAY_SIZE(ad7091r5_events)),
+ AD7091R_CHANNEL(1, 12, ad7091r5_events, ARRAY_SIZE(ad7091r5_events)),
+ AD7091R_CHANNEL(2, 12, ad7091r5_events, ARRAY_SIZE(ad7091r5_events)),
+ AD7091R_CHANNEL(3, 12, ad7091r5_events, ARRAY_SIZE(ad7091r5_events)),
+};
+
+static const struct iio_chan_spec ad7091r5_channels_noirq[] = {
+ AD7091R_CHANNEL(0, 12, NULL, 0),
+ AD7091R_CHANNEL(1, 12, NULL, 0),
+ AD7091R_CHANNEL(2, 12, NULL, 0),
+ AD7091R_CHANNEL(3, 12, NULL, 0),
+};
+#undef AD7091R_CHANNEL
+
+static const struct ad7091r_chip_info ad7091r5_chip_info_irq = {
+ .channels = ad7091r5_channels_irq,
+ .num_channels = ARRAY_SIZE(ad7091r5_channels_irq),
+};
+
+static const struct ad7091r_chip_info ad7091r5_chip_info_noirq = {
+ .channels = ad7091r5_channels_noirq,
+ .num_channels = ARRAY_SIZE(ad7091r5_channels_noirq),
+};
+
+static int ad7091r5_i2c_probe(struct i2c_client *i2c,
+ const struct i2c_device_id *id)
+{
+ const struct ad7091r_chip_info *chip_info;
+ struct regmap *map = devm_regmap_init_i2c(i2c, &ad7091r_regmap_config);
+
+ if (IS_ERR(map))
+ return PTR_ERR(map);
+
+ if (i2c->irq)
+ chip_info = &ad7091r5_chip_info_irq;
+ else
+ chip_info = &ad7091r5_chip_info_noirq;
+
+ return ad7091r_probe(&i2c->dev, id->name, chip_info, map, i2c->irq);
+}
+
+static const struct i2c_device_id ad7091r5_i2c_ids[] = {
+ {"ad7091r5", 0},
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, ad7091r5_i2c_ids);
+
+static struct i2c_driver ad7091r5_driver = {
+ .driver = {
+ .name = "ad7091r5",
+ },
+ .probe = ad7091r5_i2c_probe,
+ .id_table = ad7091r5_i2c_ids,
+};
+module_i2c_driver(ad7091r5_driver);
+
+MODULE_AUTHOR("Beniamin Bia <[email protected]>");
+MODULE_DESCRIPTION("Analog Devices AD7091R5 multi-channel ADC driver");
+MODULE_LICENSE("GPL v2");
--
2.17.1


2019-10-29 19:39:52

by Beniamin Bia

[permalink] [raw]
Subject: [PATCH v2 2/4] iio: adc: ad7091r5: Add scale and external VREF support

From: Paul Cercueil <[email protected]>

The scale can now be obtained with the "in_voltage_scale" file.
By default, the scale returned corresponds to the internal VREF of 2.5V.

It is possible to use an external VREF (through the REFIN/REFOUT pin of
the chip), by passing a regulator to the driver. The scale will then be
calculated according to the voltage reported by the regulator.

Signed-off-by: Paul Cercueil <[email protected]>
Co-developed-by: Beniamin Bia <[email protected]>
Signed-off-by: Beniamin Bia <[email protected]>
---
Changes in v2:
-device remove function replaced by devm_add_action_or_reset
drivers/iio/adc/ad7091r-base.c | 41 ++++++++++++++++++++++++++++++++++
drivers/iio/adc/ad7091r-base.h | 1 +
drivers/iio/adc/ad7091r5.c | 5 +++++
3 files changed, 47 insertions(+)

diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
index c2500f614d54..fda1112e7376 100644
--- a/drivers/iio/adc/ad7091r-base.c
+++ b/drivers/iio/adc/ad7091r-base.c
@@ -11,6 +11,7 @@
#include <linux/interrupt.h>
#include <linux/module.h>
#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>

#include "ad7091r-base.h"

@@ -42,6 +43,7 @@ enum ad7091r_mode {
struct ad7091r_state {
struct device *dev;
struct regmap *map;
+ struct regulator *reg;
const struct ad7091r_chip_info *chip_info;
enum ad7091r_mode mode;
struct mutex lock;
@@ -145,6 +147,21 @@ static int ad7091r_read_raw(struct iio_dev *iio_dev,
ret = IIO_VAL_INT;
break;

+ case IIO_CHAN_INFO_SCALE:
+ if (st->reg) {
+ ret = regulator_get_voltage(st->reg);
+ if (ret < 0)
+ goto unlock;
+
+ *val = ret / 1000;
+ } else {
+ *val = st->chip_info->vref_mV;
+ }
+
+ *val2 = chan->scan_type.realbits;
+ ret = IIO_VAL_FRACTIONAL_LOG2;
+ break;
+
default:
ret = -EINVAL;
break;
@@ -187,6 +204,16 @@ static irqreturn_t ad7091r_event_handler(int irq, void *private)
return IRQ_HANDLED;
}

+void ad7091r_remove(void *data)
+{
+ struct ad7091r_state *st;
+
+ st = (struct ad7091r_state *)data;
+
+ if (st->reg)
+ regulator_disable(st->reg);
+}
+
int ad7091r_probe(struct device *dev, const char *name,
const struct ad7091r_chip_info *chip_info,
struct regmap *map, int irq)
@@ -220,6 +247,20 @@ int ad7091r_probe(struct device *dev, const char *name,
return ret;
}

+ st->reg = devm_regulator_get_optional(dev, "vref");
+ if (IS_ERR(st->reg)) {
+ if (PTR_ERR(st->reg) == EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
+ st->reg = NULL;
+ } else {
+ ret = regulator_enable(st->reg);
+ if (ret)
+ return ret;
+ }
+
+ devm_add_action_or_reset(dev, ad7091r_remove, st);
+
/* Use command mode by default to convert only desired channels*/
ret = ad7091r_set_mode(st, AD7091R_MODE_COMMAND);
if (ret < 0)
diff --git a/drivers/iio/adc/ad7091r-base.h b/drivers/iio/adc/ad7091r-base.h
index 5f1147735953..76b76968d071 100644
--- a/drivers/iio/adc/ad7091r-base.h
+++ b/drivers/iio/adc/ad7091r-base.h
@@ -14,6 +14,7 @@ struct ad7091r_state;
struct ad7091r_chip_info {
unsigned int num_channels;
const struct iio_chan_spec *channels;
+ unsigned int vref_mV;
};

extern const struct regmap_config ad7091r_regmap_config;
diff --git a/drivers/iio/adc/ad7091r5.c b/drivers/iio/adc/ad7091r5.c
index 2c44dd02ba1a..7ee4a781cc01 100644
--- a/drivers/iio/adc/ad7091r5.c
+++ b/drivers/iio/adc/ad7091r5.c
@@ -35,10 +35,13 @@ static const struct iio_event_spec ad7091r5_events[] = {
#define AD7091R_CHANNEL(idx, bits, ev, num_ev) { \
.type = IIO_VOLTAGE, \
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
.indexed = 1, \
.channel = idx, \
.event_spec = ev, \
.num_event_specs = num_ev, \
+ .scan_type.storagebits = 16, \
+ .scan_type.realbits = bits, \
}
static const struct iio_chan_spec ad7091r5_channels_irq[] = {
AD7091R_CHANNEL(0, 12, ad7091r5_events, ARRAY_SIZE(ad7091r5_events)),
@@ -58,11 +61,13 @@ static const struct iio_chan_spec ad7091r5_channels_noirq[] = {
static const struct ad7091r_chip_info ad7091r5_chip_info_irq = {
.channels = ad7091r5_channels_irq,
.num_channels = ARRAY_SIZE(ad7091r5_channels_irq),
+ .vref_mV = 2500,
};

static const struct ad7091r_chip_info ad7091r5_chip_info_noirq = {
.channels = ad7091r5_channels_noirq,
.num_channels = ARRAY_SIZE(ad7091r5_channels_noirq),
+ .vref_mV = 2500,
};

static int ad7091r5_i2c_probe(struct i2c_client *i2c,
--
2.17.1

2019-10-29 19:42:05

by Beniamin Bia

[permalink] [raw]
Subject: [PATCH v2 4/4] MAINTAINERS: add entry for AD7091R5 driver

Add Beniamin Bia as a maintainer for AD7091R5 ADC.

Signed-off-by: Beniamin Bia <[email protected]>
---
Changes in v2:
-nothing changed
MAINTAINERS | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2e01d0f0b0e5..7f1e4b88688f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -893,6 +893,14 @@ S: Supported
F: drivers/iio/dac/ad5758.c
F: Documentation/devicetree/bindings/iio/dac/ad5758.txt

+ANALOG DEVICES INC AD7091R5 DRIVER
+M: Beniamin Bia <[email protected]>
+L: [email protected]
+W: http://ez.analog.com/community/linux-device-drivers
+S: Supported
+F: drivers/iio/adc/ad7091r5.c
+F: Documentation/devicetree/bindings/iio/adc/adi,ad7091r5.yaml
+
ANALOG DEVICES INC AD7124 DRIVER
M: Stefan Popa <[email protected]>
L: [email protected]
--
2.17.1

2019-10-29 20:03:30

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] iio: adc: Add support for AD7091R5 ADC

It looks like you're going to have to respin the patchset so I'm adding
my nits even though it's a bit late.


> +static int ad7091r_set_mode(struct ad7091r_state *st, enum ad7091r_mode mode)
> +{
> + int ret;
> +
> + switch (mode) {
> + case AD7091R_MODE_SAMPLE:
> + ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
> + AD7091R_REG_CONF_MODE_MASK, 0);
> + break;
> + case AD7091R_MODE_COMMAND:
> + ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
> + AD7091R_REG_CONF_MODE_MASK,
> + AD7091R_REG_CONF_CMD);
> + break;
> + case AD7091R_MODE_AUTOCYCLE:
> + ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
> + AD7091R_REG_CONF_MODE_MASK,
> + AD7091R_REG_CONF_AUTO);
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }

This would look even better as:

switch (mode) {
case AD7091R_MODE_SAMPLE:
conf = 0;
break;
case AD7091R_MODE_COMMAND:
conf = AD7091R_REG_CONF_CMD;
break;
case AD7091R_MODE_AUTOCYCLE:
conf = AD7091R_REG_CONF_AUTO;
break;
default:
return -EINVAL;
}

ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
AD7091R_REG_CONF_MODE_MASK, conf);
if (ret)
return ret;

st->mode = mode;

return 0;

> +int ad7091r_probe(struct device *dev, const char *name,
> + const struct ad7091r_chip_info *chip_info,
> + struct regmap *map, int irq)
> +{
> + struct iio_dev *iio_dev;
> + struct ad7091r_state *st;
> + int ret;
> +
> + iio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> + if (!iio_dev)
> + return -ENOMEM;
> +
> + st = iio_priv(iio_dev);
> + st->dev = dev;
> + st->chip_info = chip_info;
> + st->map = map;
> +
> + iio_dev->dev.parent = dev;
> + iio_dev->name = name;
> + iio_dev->info = &ad7091r_info;
> + iio_dev->modes = INDIO_DIRECT_MODE;
> +
> + iio_dev->num_channels = chip_info->num_channels;
> + iio_dev->channels = chip_info->channels;
> +
> + if (irq) {
> + ret = devm_request_threaded_irq(dev, irq, NULL,
> + ad7091r_event_handler,
> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, name, st);
> + if (ret)
> + return ret;
> + }
> +
> + /* Use command mode by default to convert only desired channels*/
> + ret = ad7091r_set_mode(st, AD7091R_MODE_COMMAND);
> + if (ret < 0)

if (ret) {

> + return ret;
> +
> + return iio_device_register(iio_dev);
> +}
> +EXPORT_SYMBOL_GPL(ad7091r_probe);

[ snip ]

> +#include <linux/i2c.h>
> +#include <linux/iio/iio.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +
> +#include "ad7091r-base.h"
> +
> +static const struct iio_event_spec ad7091r5_events[] = {
> + {
> + .type = IIO_EV_TYPE_THRESH,
> + .dir = IIO_EV_DIR_RISING,
> + .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> + BIT(IIO_EV_INFO_ENABLE),

This would be more clear if it were aligned like so:

.mask_separate = BIT(IIO_EV_INFO_VALUE) |
BIT(IIO_EV_INFO_ENABLE),


> + },
> + {
> + .type = IIO_EV_TYPE_THRESH,
> + .dir = IIO_EV_DIR_FALLING,
> + .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> + BIT(IIO_EV_INFO_ENABLE),

.mask_separate = BIT(IIO_EV_INFO_VALUE) |
BIT(IIO_EV_INFO_ENABLE),

> + },
> + {
> + .type = IIO_EV_TYPE_THRESH,
> + .dir = IIO_EV_DIR_EITHER,
> + .mask_separate = BIT(IIO_EV_INFO_HYSTERESIS),
> + },
> +};
> +

regards,
dan carpenter

2019-10-31 10:58:28

by kernel test robot

[permalink] [raw]
Subject: [RFC PATCH] iio: adc: ad7091r5: ad7091r_remove() can be static


Fixes: 608229aeaa1f ("iio: adc: ad7091r5: Add scale and external VREF support")
Signed-off-by: kbuild test robot <[email protected]>
---
ad7091r-base.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
index fda1112e73760..dc42735680c83 100644
--- a/drivers/iio/adc/ad7091r-base.c
+++ b/drivers/iio/adc/ad7091r-base.c
@@ -204,7 +204,7 @@ static irqreturn_t ad7091r_event_handler(int irq, void *private)
return IRQ_HANDLED;
}

-void ad7091r_remove(void *data)
+static void ad7091r_remove(void *data)
{
struct ad7091r_state *st;

2019-10-31 11:00:47

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] iio: adc: ad7091r5: Add scale and external VREF support

Hi Beniamin,

I love your patch! Perhaps something to improve:

[auto build test WARNING on iio/togreg]
[also build test WARNING on v5.4-rc5 next-20191030]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Beniamin-Bia/iio-adc-Add-support-for-AD7091R5-ADC/20191031-100114
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>


sparse warnings: (new ones prefixed by >>)

>> drivers/iio/adc/ad7091r-base.c:207:6: sparse: sparse: symbol 'ad7091r_remove' was not declared. Should it be static?

Please review and possibly fold the followup patch.

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

2019-11-25 11:36:00

by Alexandru Ardelean

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] iio: adc: Add support for AD7091R5 ADC

On Tue, 2019-10-29 at 18:29 +0200, Beniamin Bia wrote:
> [External]
>
> From: Paul Cercueil <[email protected]>
>
> AD7091 is 4-Channel, I2C, Ultra Low Power,12-Bit ADC.
>
> Datasheet:
> Link:
> https://www.analog.com/media/en/technical-documentation/data-sheets/ad7091r-5.pdf
>
> Signed-off-by: Paul Cercueil <[email protected]>
> Co-developed-by: Beniamin Bia <[email protected]>
> Signed-off-by: Beniamin Bia <[email protected]>
> ---
> Changes in v2:
> -blank lines removed
> -prefix added to macros
> -comments rework
> -error checking syntax changed
> -iio mutex replaced by a mutex
> -device remove function was removed and later replaced by devm_add_action
> -regmap include removed from header
>
> drivers/iio/adc/Kconfig | 7 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ad7091r-base.c | 264 +++++++++++++++++++++++++++++++++
> drivers/iio/adc/ad7091r-base.h | 25 ++++
> drivers/iio/adc/ad7091r5.c | 102 +++++++++++++
> 5 files changed, 399 insertions(+)
> create mode 100644 drivers/iio/adc/ad7091r-base.c
> create mode 100644 drivers/iio/adc/ad7091r-base.h
> create mode 100644 drivers/iio/adc/ad7091r5.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 7e3286265a38..80b1b9551749 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -22,6 +22,13 @@ config AD7124
> To compile this driver as a module, choose M here: the module
> will be
> called ad7124.
>
> +config AD7091R5
> + tristate "Analog Devices AD7091R5 ADC Driver"
> + depends on I2C
> + select REGMAP_I2C
> + help
> + Say yes here to build support for Analog Devices AD7091R-5 ADC.
> +

Sorry for the lateness here.
Is it too late to mention to put this before the AD7124 driver?
Same question for Makefile.

This is to keep things alphabetically sorted.

Thanks
Alex

> config AD7266
> tristate "Analog Devices AD7265/AD7266 ADC driver"
> depends on SPI_MASTER
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index ef9cc485fb67..55e44735aaac 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -6,6 +6,7 @@
> # When adding new entries keep the list in alphabetical order
> obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
> obj-$(CONFIG_AD7124) += ad7124.o
> +obj-$(CONFIG_AD7091R5) += ad7091r5.o ad7091r-base.o
> obj-$(CONFIG_AD7266) += ad7266.o
> obj-$(CONFIG_AD7291) += ad7291.o
> obj-$(CONFIG_AD7298) += ad7298.o
> diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-
> base.c
> new file mode 100644
> index 000000000000..c2500f614d54
> --- /dev/null
> +++ b/drivers/iio/adc/ad7091r-base.c
> @@ -0,0 +1,264 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * AD7091RX Analog to Digital converter driver
> + *
> + * Copyright 2014-2019 Analog Devices Inc.
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/iio/events.h>
> +#include <linux/iio/iio.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +
> +#include "ad7091r-base.h"
> +
> +#define AD7091R_REG_RESULT 0
> +#define AD7091R_REG_CHANNEL 1
> +#define AD7091R_REG_CONF 2
> +#define AD7091R_REG_ALERT 3
> +#define AD7091R_REG_CH_LOW_LIMIT(ch) ((ch) * 3 + 4)
> +#define AD7091R_REG_CH_HIGH_LIMIT(ch) ((ch) * 3 + 5)
> +#define AD7091R_REG_CH_HYSTERESIS(ch) ((ch) * 3 + 6)
> +
> +/* AD7091R_REG_RESULT */
> +#define AD7091R_REG_RESULT_CH_ID(x) (((x) >> 13) & 0x3)
> +#define AD7091R_REG_RESULT_CONV_RESULT(x) ((x) & 0xfff)
> +
> +/* AD7091R_REG_CONF */
> +#define AD7091R_REG_CONF_AUTO BIT(8)
> +#define AD7091R_REG_CONF_CMD BIT(10)
> +
> +#define AD7091R_REG_CONF_MODE_MASK \
> + (AD7091R_REG_CONF_AUTO | AD7091R_REG_CONF_CMD)
> +
> +enum ad7091r_mode {
> + AD7091R_MODE_SAMPLE,
> + AD7091R_MODE_COMMAND,
> + AD7091R_MODE_AUTOCYCLE,
> +};
> +
> +struct ad7091r_state {
> + struct device *dev;
> + struct regmap *map;
> + const struct ad7091r_chip_info *chip_info;
> + enum ad7091r_mode mode;
> + struct mutex lock;
> +};
> +
> +static int ad7091r_set_mode(struct ad7091r_state *st, enum ad7091r_mode
> mode)
> +{
> + int ret;
> +
> + switch (mode) {
> + case AD7091R_MODE_SAMPLE:
> + ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
> + AD7091R_REG_CONF_MODE_MASK, 0);
> + break;
> + case AD7091R_MODE_COMMAND:
> + ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
> + AD7091R_REG_CONF_MODE_MASK,
> + AD7091R_REG_CONF_CMD);
> + break;
> + case AD7091R_MODE_AUTOCYCLE:
> + ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
> + AD7091R_REG_CONF_MODE_MASK,
> + AD7091R_REG_CONF_AUTO);
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> +
> + if (ret)
> + return ret;
> +
> + st->mode = mode;
> +
> + return ret;
> +}
> +
> +static int ad7091r_set_channel(struct ad7091r_state *st, unsigned int
> channel)
> +{
> + unsigned int foo;
> + int ret;
> +
> + /* AD7091R_REG_CHANNEL specified which channels to be converted */
> + ret = regmap_write(st->map, AD7091R_REG_CHANNEL,
> + BIT(channel) | (BIT(channel) << 8));
> + if (ret)
> + return ret;
> +
> + /*
> + * There is a latency of one conversion before the channel
> conversion
> + * sequence is updated
> + */
> + return regmap_read(st->map, AD7091R_REG_RESULT, &foo);
> +}
> +
> +static int ad7091r_read_one(struct iio_dev *iio_dev,
> + unsigned int channel, unsigned int *read_val)
> +{
> + struct ad7091r_state *st = iio_priv(iio_dev);
> + unsigned int val;
> + int ret;
> +
> + ret = ad7091r_set_channel(st, channel);
> + if (ret)
> + return ret;
> +
> + ret = regmap_read(st->map, AD7091R_REG_RESULT, &val);
> + if (ret)
> + return ret;
> +
> + if (AD7091R_REG_RESULT_CH_ID(val) != channel)
> + return -EIO;
> +
> + *read_val = AD7091R_REG_RESULT_CONV_RESULT(val);
> +
> + return 0;
> +}
> +
> +static int ad7091r_read_raw(struct iio_dev *iio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long m)
> +{
> + struct ad7091r_state *st = iio_priv(iio_dev);
> + unsigned int read_val;
> + int ret;
> +
> + mutex_lock(&st->lock);
> +
> + switch (m) {
> + case IIO_CHAN_INFO_RAW:
> + if (st->mode != AD7091R_MODE_COMMAND) {
> + ret = -EBUSY;
> + goto unlock;
> + }
> +
> + ret = ad7091r_read_one(iio_dev, chan->channel, &read_val);
> + if (ret)
> + goto unlock;
> +
> + *val = read_val;
> + ret = IIO_VAL_INT;
> + break;
> +
> + default:
> + ret = -EINVAL;
> + break;
> + }
> +
> +unlock:
> + mutex_unlock(&st->lock);
> + return ret;
> +}
> +
> +static const struct iio_info ad7091r_info = {
> + .read_raw = ad7091r_read_raw,
> +};
> +
> +static irqreturn_t ad7091r_event_handler(int irq, void *private)
> +{
> + struct ad7091r_state *st = (struct ad7091r_state *) private;
> + struct iio_dev *iio_dev = dev_get_drvdata(st->dev);
> + unsigned int i, read_val;
> + int ret;
> + s64 timestamp = iio_get_time_ns(iio_dev);
> +
> + ret = regmap_read(st->map, AD7091R_REG_ALERT, &read_val);
> + if (ret)
> + return IRQ_HANDLED;
> +
> + for (i = 0; i < st->chip_info->num_channels; i++) {
> + if (read_val & BIT(i * 2))
> + iio_push_event(iio_dev,
> + IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE,
> i,
> + IIO_EV_TYPE_THRESH,
> + IIO_EV_DIR_RISING),
> timestamp);
> + if (read_val & BIT(i * 2 + 1))
> + iio_push_event(iio_dev,
> + IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE,
> i,
> + IIO_EV_TYPE_THRESH,
> + IIO_EV_DIR_FALLING),
> timestamp);
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +int ad7091r_probe(struct device *dev, const char *name,
> + const struct ad7091r_chip_info *chip_info,
> + struct regmap *map, int irq)
> +{
> + struct iio_dev *iio_dev;
> + struct ad7091r_state *st;
> + int ret;
> +
> + iio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> + if (!iio_dev)
> + return -ENOMEM;
> +
> + st = iio_priv(iio_dev);
> + st->dev = dev;
> + st->chip_info = chip_info;
> + st->map = map;
> +
> + iio_dev->dev.parent = dev;
> + iio_dev->name = name;
> + iio_dev->info = &ad7091r_info;
> + iio_dev->modes = INDIO_DIRECT_MODE;
> +
> + iio_dev->num_channels = chip_info->num_channels;
> + iio_dev->channels = chip_info->channels;
> +
> + if (irq) {
> + ret = devm_request_threaded_irq(dev, irq, NULL,
> + ad7091r_event_handler,
> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, name,
> st);
> + if (ret)
> + return ret;
> + }
> +
> + /* Use command mode by default to convert only desired channels*/
> + ret = ad7091r_set_mode(st, AD7091R_MODE_COMMAND);
> + if (ret < 0)
> + return ret;
> +
> + return iio_device_register(iio_dev);
> +}
> +EXPORT_SYMBOL_GPL(ad7091r_probe);
> +
> +static bool ad7091r_writeable_reg(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case AD7091R_REG_RESULT:
> + case AD7091R_REG_ALERT:
> + return false;
> + default:
> + return true;
> + }
> +}
> +
> +static bool ad7091r_volatile_reg(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case AD7091R_REG_RESULT:
> + case AD7091R_REG_ALERT:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +const struct regmap_config ad7091r_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 16,
> + .writeable_reg = ad7091r_writeable_reg,
> + .volatile_reg = ad7091r_volatile_reg,
> +};
> +EXPORT_SYMBOL_GPL(ad7091r_regmap_config);
> +
> +MODULE_AUTHOR("Beniamin Bia <[email protected]>");
> +MODULE_DESCRIPTION("Analog Devices AD7091Rx multi-channel converters");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/adc/ad7091r-base.h b/drivers/iio/adc/ad7091r-
> base.h
> new file mode 100644
> index 000000000000..5f1147735953
> --- /dev/null
> +++ b/drivers/iio/adc/ad7091r-base.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * AD7091RX Analog to Digital converter driver
> + *
> + * Copyright 2014-2019 Analog Devices Inc.
> + */
> +
> +#ifndef __DRIVERS_IIO_ADC_AD7091R_BASE_H__
> +#define __DRIVERS_IIO_ADC_AD7091R_BASE_H__
> +
> +struct device;
> +struct ad7091r_state;
> +
> +struct ad7091r_chip_info {
> + unsigned int num_channels;
> + const struct iio_chan_spec *channels;
> +};
> +
> +extern const struct regmap_config ad7091r_regmap_config;
> +
> +int ad7091r_probe(struct device *dev, const char *name,
> + const struct ad7091r_chip_info *chip_info,
> + struct regmap *map, int irq);
> +
> +#endif /* __DRIVERS_IIO_ADC_AD7091R5_BASE_H__ */
> diff --git a/drivers/iio/adc/ad7091r5.c b/drivers/iio/adc/ad7091r5.c
> new file mode 100644
> index 000000000000..2c44dd02ba1a
> --- /dev/null
> +++ b/drivers/iio/adc/ad7091r5.c
> @@ -0,0 +1,102 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * AD7091R5 Analog to Digital converter driver
> + *
> + * Copyright 2014-2019 Analog Devices Inc.
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/iio/iio.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +
> +#include "ad7091r-base.h"
> +
> +static const struct iio_event_spec ad7091r5_events[] = {
> + {
> + .type = IIO_EV_TYPE_THRESH,
> + .dir = IIO_EV_DIR_RISING,
> + .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> + BIT(IIO_EV_INFO_ENABLE),
> + },
> + {
> + .type = IIO_EV_TYPE_THRESH,
> + .dir = IIO_EV_DIR_FALLING,
> + .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> + BIT(IIO_EV_INFO_ENABLE),
> + },
> + {
> + .type = IIO_EV_TYPE_THRESH,
> + .dir = IIO_EV_DIR_EITHER,
> + .mask_separate = BIT(IIO_EV_INFO_HYSTERESIS),
> + },
> +};
> +
> +#define AD7091R_CHANNEL(idx, bits, ev, num_ev) { \
> + .type = IIO_VOLTAGE, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .indexed = 1, \
> + .channel = idx, \
> + .event_spec = ev, \
> + .num_event_specs = num_ev, \
> +}
> +static const struct iio_chan_spec ad7091r5_channels_irq[] = {
> + AD7091R_CHANNEL(0, 12, ad7091r5_events,
> ARRAY_SIZE(ad7091r5_events)),
> + AD7091R_CHANNEL(1, 12, ad7091r5_events,
> ARRAY_SIZE(ad7091r5_events)),
> + AD7091R_CHANNEL(2, 12, ad7091r5_events,
> ARRAY_SIZE(ad7091r5_events)),
> + AD7091R_CHANNEL(3, 12, ad7091r5_events,
> ARRAY_SIZE(ad7091r5_events)),
> +};
> +
> +static const struct iio_chan_spec ad7091r5_channels_noirq[] = {
> + AD7091R_CHANNEL(0, 12, NULL, 0),
> + AD7091R_CHANNEL(1, 12, NULL, 0),
> + AD7091R_CHANNEL(2, 12, NULL, 0),
> + AD7091R_CHANNEL(3, 12, NULL, 0),
> +};
> +#undef AD7091R_CHANNEL
> +
> +static const struct ad7091r_chip_info ad7091r5_chip_info_irq = {
> + .channels = ad7091r5_channels_irq,
> + .num_channels = ARRAY_SIZE(ad7091r5_channels_irq),
> +};
> +
> +static const struct ad7091r_chip_info ad7091r5_chip_info_noirq = {
> + .channels = ad7091r5_channels_noirq,
> + .num_channels = ARRAY_SIZE(ad7091r5_channels_noirq),
> +};
> +
> +static int ad7091r5_i2c_probe(struct i2c_client *i2c,
> + const struct i2c_device_id *id)
> +{
> + const struct ad7091r_chip_info *chip_info;
> + struct regmap *map = devm_regmap_init_i2c(i2c,
> &ad7091r_regmap_config);
> +
> + if (IS_ERR(map))
> + return PTR_ERR(map);
> +
> + if (i2c->irq)
> + chip_info = &ad7091r5_chip_info_irq;
> + else
> + chip_info = &ad7091r5_chip_info_noirq;
> +
> + return ad7091r_probe(&i2c->dev, id->name, chip_info, map, i2c-
> >irq);
> +}
> +
> +static const struct i2c_device_id ad7091r5_i2c_ids[] = {
> + {"ad7091r5", 0},
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, ad7091r5_i2c_ids);
> +
> +static struct i2c_driver ad7091r5_driver = {
> + .driver = {
> + .name = "ad7091r5",
> + },
> + .probe = ad7091r5_i2c_probe,
> + .id_table = ad7091r5_i2c_ids,
> +};
> +module_i2c_driver(ad7091r5_driver);
> +
> +MODULE_AUTHOR("Beniamin Bia <[email protected]>");
> +MODULE_DESCRIPTION("Analog Devices AD7091R5 multi-channel ADC driver");
> +MODULE_LICENSE("GPL v2");

2019-12-01 10:17:04

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] iio: adc: Add support for AD7091R5 ADC

On Mon, 25 Nov 2019 11:26:27 +0000
"Ardelean, Alexandru" <[email protected]> wrote:

> On Tue, 2019-10-29 at 18:29 +0200, Beniamin Bia wrote:
> > [External]
> >
> > From: Paul Cercueil <[email protected]>
> >
> > AD7091 is 4-Channel, I2C, Ultra Low Power,12-Bit ADC.
> >
> > Datasheet:
> > Link:
> > https://www.analog.com/media/en/technical-documentation/data-sheets/ad7091r-5.pdf
> >
> > Signed-off-by: Paul Cercueil <[email protected]>
> > Co-developed-by: Beniamin Bia <[email protected]>
> > Signed-off-by: Beniamin Bia <[email protected]>
> > ---
> > Changes in v2:
> > -blank lines removed
> > -prefix added to macros
> > -comments rework
> > -error checking syntax changed
> > -iio mutex replaced by a mutex
> > -device remove function was removed and later replaced by devm_add_action
> > -regmap include removed from header
> >
> > drivers/iio/adc/Kconfig | 7 +
> > drivers/iio/adc/Makefile | 1 +
> > drivers/iio/adc/ad7091r-base.c | 264 +++++++++++++++++++++++++++++++++
> > drivers/iio/adc/ad7091r-base.h | 25 ++++
> > drivers/iio/adc/ad7091r5.c | 102 +++++++++++++
> > 5 files changed, 399 insertions(+)
> > create mode 100644 drivers/iio/adc/ad7091r-base.c
> > create mode 100644 drivers/iio/adc/ad7091r-base.h
> > create mode 100644 drivers/iio/adc/ad7091r5.c
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 7e3286265a38..80b1b9551749 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -22,6 +22,13 @@ config AD7124
> > To compile this driver as a module, choose M here: the module
> > will be
> > called ad7124.
> >
> > +config AD7091R5
> > + tristate "Analog Devices AD7091R5 ADC Driver"
> > + depends on I2C
> > + select REGMAP_I2C
> > + help
> > + Say yes here to build support for Analog Devices AD7091R-5 ADC.
> > +
>
> Sorry for the lateness here.
> Is it too late to mention to put this before the AD7124 driver?
> Same question for Makefile.

Good spot, but at this stage please send a follow up patch
tidying the ordering up.

Thanks,

Jonathan

>
> This is to keep things alphabetically sorted.
>
> Thanks
> Alex
>
> > config AD7266
> > tristate "Analog Devices AD7265/AD7266 ADC driver"
> > depends on SPI_MASTER
> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > index ef9cc485fb67..55e44735aaac 100644
> > --- a/drivers/iio/adc/Makefile
> > +++ b/drivers/iio/adc/Makefile
> > @@ -6,6 +6,7 @@
> > # When adding new entries keep the list in alphabetical order
> > obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
> > obj-$(CONFIG_AD7124) += ad7124.o
> > +obj-$(CONFIG_AD7091R5) += ad7091r5.o ad7091r-base.o
> > obj-$(CONFIG_AD7266) += ad7266.o
> > obj-$(CONFIG_AD7291) += ad7291.o
> > obj-$(CONFIG_AD7298) += ad7298.o
> > diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-
> > base.c
> > new file mode 100644
> > index 000000000000..c2500f614d54
> > --- /dev/null
> > +++ b/drivers/iio/adc/ad7091r-base.c
> > @@ -0,0 +1,264 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * AD7091RX Analog to Digital converter driver
> > + *
> > + * Copyright 2014-2019 Analog Devices Inc.
> > + */
> > +
> > +#include <linux/bitops.h>
> > +#include <linux/iio/events.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/regmap.h>
> > +
> > +#include "ad7091r-base.h"
> > +
> > +#define AD7091R_REG_RESULT 0
> > +#define AD7091R_REG_CHANNEL 1
> > +#define AD7091R_REG_CONF 2
> > +#define AD7091R_REG_ALERT 3
> > +#define AD7091R_REG_CH_LOW_LIMIT(ch) ((ch) * 3 + 4)
> > +#define AD7091R_REG_CH_HIGH_LIMIT(ch) ((ch) * 3 + 5)
> > +#define AD7091R_REG_CH_HYSTERESIS(ch) ((ch) * 3 + 6)
> > +
> > +/* AD7091R_REG_RESULT */
> > +#define AD7091R_REG_RESULT_CH_ID(x) (((x) >> 13) & 0x3)
> > +#define AD7091R_REG_RESULT_CONV_RESULT(x) ((x) & 0xfff)
> > +
> > +/* AD7091R_REG_CONF */
> > +#define AD7091R_REG_CONF_AUTO BIT(8)
> > +#define AD7091R_REG_CONF_CMD BIT(10)
> > +
> > +#define AD7091R_REG_CONF_MODE_MASK \
> > + (AD7091R_REG_CONF_AUTO | AD7091R_REG_CONF_CMD)
> > +
> > +enum ad7091r_mode {
> > + AD7091R_MODE_SAMPLE,
> > + AD7091R_MODE_COMMAND,
> > + AD7091R_MODE_AUTOCYCLE,
> > +};
> > +
> > +struct ad7091r_state {
> > + struct device *dev;
> > + struct regmap *map;
> > + const struct ad7091r_chip_info *chip_info;
> > + enum ad7091r_mode mode;
> > + struct mutex lock;
> > +};
> > +
> > +static int ad7091r_set_mode(struct ad7091r_state *st, enum ad7091r_mode
> > mode)
> > +{
> > + int ret;
> > +
> > + switch (mode) {
> > + case AD7091R_MODE_SAMPLE:
> > + ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
> > + AD7091R_REG_CONF_MODE_MASK, 0);
> > + break;
> > + case AD7091R_MODE_COMMAND:
> > + ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
> > + AD7091R_REG_CONF_MODE_MASK,
> > + AD7091R_REG_CONF_CMD);
> > + break;
> > + case AD7091R_MODE_AUTOCYCLE:
> > + ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
> > + AD7091R_REG_CONF_MODE_MASK,
> > + AD7091R_REG_CONF_AUTO);
> > + break;
> > + default:
> > + ret = -EINVAL;
> > + break;
> > + }
> > +
> > + if (ret)
> > + return ret;
> > +
> > + st->mode = mode;
> > +
> > + return ret;
> > +}
> > +
> > +static int ad7091r_set_channel(struct ad7091r_state *st, unsigned int
> > channel)
> > +{
> > + unsigned int foo;
> > + int ret;
> > +
> > + /* AD7091R_REG_CHANNEL specified which channels to be converted */
> > + ret = regmap_write(st->map, AD7091R_REG_CHANNEL,
> > + BIT(channel) | (BIT(channel) << 8));
> > + if (ret)
> > + return ret;
> > +
> > + /*
> > + * There is a latency of one conversion before the channel
> > conversion
> > + * sequence is updated
> > + */
> > + return regmap_read(st->map, AD7091R_REG_RESULT, &foo);
> > +}
> > +
> > +static int ad7091r_read_one(struct iio_dev *iio_dev,
> > + unsigned int channel, unsigned int *read_val)
> > +{
> > + struct ad7091r_state *st = iio_priv(iio_dev);
> > + unsigned int val;
> > + int ret;
> > +
> > + ret = ad7091r_set_channel(st, channel);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_read(st->map, AD7091R_REG_RESULT, &val);
> > + if (ret)
> > + return ret;
> > +
> > + if (AD7091R_REG_RESULT_CH_ID(val) != channel)
> > + return -EIO;
> > +
> > + *read_val = AD7091R_REG_RESULT_CONV_RESULT(val);
> > +
> > + return 0;
> > +}
> > +
> > +static int ad7091r_read_raw(struct iio_dev *iio_dev,
> > + struct iio_chan_spec const *chan,
> > + int *val, int *val2, long m)
> > +{
> > + struct ad7091r_state *st = iio_priv(iio_dev);
> > + unsigned int read_val;
> > + int ret;
> > +
> > + mutex_lock(&st->lock);
> > +
> > + switch (m) {
> > + case IIO_CHAN_INFO_RAW:
> > + if (st->mode != AD7091R_MODE_COMMAND) {
> > + ret = -EBUSY;
> > + goto unlock;
> > + }
> > +
> > + ret = ad7091r_read_one(iio_dev, chan->channel, &read_val);
> > + if (ret)
> > + goto unlock;
> > +
> > + *val = read_val;
> > + ret = IIO_VAL_INT;
> > + break;
> > +
> > + default:
> > + ret = -EINVAL;
> > + break;
> > + }
> > +
> > +unlock:
> > + mutex_unlock(&st->lock);
> > + return ret;
> > +}
> > +
> > +static const struct iio_info ad7091r_info = {
> > + .read_raw = ad7091r_read_raw,
> > +};
> > +
> > +static irqreturn_t ad7091r_event_handler(int irq, void *private)
> > +{
> > + struct ad7091r_state *st = (struct ad7091r_state *) private;
> > + struct iio_dev *iio_dev = dev_get_drvdata(st->dev);
> > + unsigned int i, read_val;
> > + int ret;
> > + s64 timestamp = iio_get_time_ns(iio_dev);
> > +
> > + ret = regmap_read(st->map, AD7091R_REG_ALERT, &read_val);
> > + if (ret)
> > + return IRQ_HANDLED;
> > +
> > + for (i = 0; i < st->chip_info->num_channels; i++) {
> > + if (read_val & BIT(i * 2))
> > + iio_push_event(iio_dev,
> > + IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE,
> > i,
> > + IIO_EV_TYPE_THRESH,
> > + IIO_EV_DIR_RISING),
> > timestamp);
> > + if (read_val & BIT(i * 2 + 1))
> > + iio_push_event(iio_dev,
> > + IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE,
> > i,
> > + IIO_EV_TYPE_THRESH,
> > + IIO_EV_DIR_FALLING),
> > timestamp);
> > + }
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +int ad7091r_probe(struct device *dev, const char *name,
> > + const struct ad7091r_chip_info *chip_info,
> > + struct regmap *map, int irq)
> > +{
> > + struct iio_dev *iio_dev;
> > + struct ad7091r_state *st;
> > + int ret;
> > +
> > + iio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> > + if (!iio_dev)
> > + return -ENOMEM;
> > +
> > + st = iio_priv(iio_dev);
> > + st->dev = dev;
> > + st->chip_info = chip_info;
> > + st->map = map;
> > +
> > + iio_dev->dev.parent = dev;
> > + iio_dev->name = name;
> > + iio_dev->info = &ad7091r_info;
> > + iio_dev->modes = INDIO_DIRECT_MODE;
> > +
> > + iio_dev->num_channels = chip_info->num_channels;
> > + iio_dev->channels = chip_info->channels;
> > +
> > + if (irq) {
> > + ret = devm_request_threaded_irq(dev, irq, NULL,
> > + ad7091r_event_handler,
> > + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, name,
> > st);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + /* Use command mode by default to convert only desired channels*/
> > + ret = ad7091r_set_mode(st, AD7091R_MODE_COMMAND);
> > + if (ret < 0)
> > + return ret;
> > +
> > + return iio_device_register(iio_dev);
> > +}
> > +EXPORT_SYMBOL_GPL(ad7091r_probe);
> > +
> > +static bool ad7091r_writeable_reg(struct device *dev, unsigned int reg)
> > +{
> > + switch (reg) {
> > + case AD7091R_REG_RESULT:
> > + case AD7091R_REG_ALERT:
> > + return false;
> > + default:
> > + return true;
> > + }
> > +}
> > +
> > +static bool ad7091r_volatile_reg(struct device *dev, unsigned int reg)
> > +{
> > + switch (reg) {
> > + case AD7091R_REG_RESULT:
> > + case AD7091R_REG_ALERT:
> > + return true;
> > + default:
> > + return false;
> > + }
> > +}
> > +
> > +const struct regmap_config ad7091r_regmap_config = {
> > + .reg_bits = 8,
> > + .val_bits = 16,
> > + .writeable_reg = ad7091r_writeable_reg,
> > + .volatile_reg = ad7091r_volatile_reg,
> > +};
> > +EXPORT_SYMBOL_GPL(ad7091r_regmap_config);
> > +
> > +MODULE_AUTHOR("Beniamin Bia <[email protected]>");
> > +MODULE_DESCRIPTION("Analog Devices AD7091Rx multi-channel converters");
> > +MODULE_LICENSE("GPL v2");
> > diff --git a/drivers/iio/adc/ad7091r-base.h b/drivers/iio/adc/ad7091r-
> > base.h
> > new file mode 100644
> > index 000000000000..5f1147735953
> > --- /dev/null
> > +++ b/drivers/iio/adc/ad7091r-base.h
> > @@ -0,0 +1,25 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * AD7091RX Analog to Digital converter driver
> > + *
> > + * Copyright 2014-2019 Analog Devices Inc.
> > + */
> > +
> > +#ifndef __DRIVERS_IIO_ADC_AD7091R_BASE_H__
> > +#define __DRIVERS_IIO_ADC_AD7091R_BASE_H__
> > +
> > +struct device;
> > +struct ad7091r_state;
> > +
> > +struct ad7091r_chip_info {
> > + unsigned int num_channels;
> > + const struct iio_chan_spec *channels;
> > +};
> > +
> > +extern const struct regmap_config ad7091r_regmap_config;
> > +
> > +int ad7091r_probe(struct device *dev, const char *name,
> > + const struct ad7091r_chip_info *chip_info,
> > + struct regmap *map, int irq);
> > +
> > +#endif /* __DRIVERS_IIO_ADC_AD7091R5_BASE_H__ */
> > diff --git a/drivers/iio/adc/ad7091r5.c b/drivers/iio/adc/ad7091r5.c
> > new file mode 100644
> > index 000000000000..2c44dd02ba1a
> > --- /dev/null
> > +++ b/drivers/iio/adc/ad7091r5.c
> > @@ -0,0 +1,102 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * AD7091R5 Analog to Digital converter driver
> > + *
> > + * Copyright 2014-2019 Analog Devices Inc.
> > + */
> > +
> > +#include <linux/i2c.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/module.h>
> > +#include <linux/regmap.h>
> > +
> > +#include "ad7091r-base.h"
> > +
> > +static const struct iio_event_spec ad7091r5_events[] = {
> > + {
> > + .type = IIO_EV_TYPE_THRESH,
> > + .dir = IIO_EV_DIR_RISING,
> > + .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> > + BIT(IIO_EV_INFO_ENABLE),
> > + },
> > + {
> > + .type = IIO_EV_TYPE_THRESH,
> > + .dir = IIO_EV_DIR_FALLING,
> > + .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> > + BIT(IIO_EV_INFO_ENABLE),
> > + },
> > + {
> > + .type = IIO_EV_TYPE_THRESH,
> > + .dir = IIO_EV_DIR_EITHER,
> > + .mask_separate = BIT(IIO_EV_INFO_HYSTERESIS),
> > + },
> > +};
> > +
> > +#define AD7091R_CHANNEL(idx, bits, ev, num_ev) { \
> > + .type = IIO_VOLTAGE, \
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> > + .indexed = 1, \
> > + .channel = idx, \
> > + .event_spec = ev, \
> > + .num_event_specs = num_ev, \
> > +}
> > +static const struct iio_chan_spec ad7091r5_channels_irq[] = {
> > + AD7091R_CHANNEL(0, 12, ad7091r5_events,
> > ARRAY_SIZE(ad7091r5_events)),
> > + AD7091R_CHANNEL(1, 12, ad7091r5_events,
> > ARRAY_SIZE(ad7091r5_events)),
> > + AD7091R_CHANNEL(2, 12, ad7091r5_events,
> > ARRAY_SIZE(ad7091r5_events)),
> > + AD7091R_CHANNEL(3, 12, ad7091r5_events,
> > ARRAY_SIZE(ad7091r5_events)),
> > +};
> > +
> > +static const struct iio_chan_spec ad7091r5_channels_noirq[] = {
> > + AD7091R_CHANNEL(0, 12, NULL, 0),
> > + AD7091R_CHANNEL(1, 12, NULL, 0),
> > + AD7091R_CHANNEL(2, 12, NULL, 0),
> > + AD7091R_CHANNEL(3, 12, NULL, 0),
> > +};
> > +#undef AD7091R_CHANNEL
> > +
> > +static const struct ad7091r_chip_info ad7091r5_chip_info_irq = {
> > + .channels = ad7091r5_channels_irq,
> > + .num_channels = ARRAY_SIZE(ad7091r5_channels_irq),
> > +};
> > +
> > +static const struct ad7091r_chip_info ad7091r5_chip_info_noirq = {
> > + .channels = ad7091r5_channels_noirq,
> > + .num_channels = ARRAY_SIZE(ad7091r5_channels_noirq),
> > +};
> > +
> > +static int ad7091r5_i2c_probe(struct i2c_client *i2c,
> > + const struct i2c_device_id *id)
> > +{
> > + const struct ad7091r_chip_info *chip_info;
> > + struct regmap *map = devm_regmap_init_i2c(i2c,
> > &ad7091r_regmap_config);
> > +
> > + if (IS_ERR(map))
> > + return PTR_ERR(map);
> > +
> > + if (i2c->irq)
> > + chip_info = &ad7091r5_chip_info_irq;
> > + else
> > + chip_info = &ad7091r5_chip_info_noirq;
> > +
> > + return ad7091r_probe(&i2c->dev, id->name, chip_info, map, i2c-
> > >irq);
> > +}
> > +
> > +static const struct i2c_device_id ad7091r5_i2c_ids[] = {
> > + {"ad7091r5", 0},
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(i2c, ad7091r5_i2c_ids);
> > +
> > +static struct i2c_driver ad7091r5_driver = {
> > + .driver = {
> > + .name = "ad7091r5",
> > + },
> > + .probe = ad7091r5_i2c_probe,
> > + .id_table = ad7091r5_i2c_ids,
> > +};
> > +module_i2c_driver(ad7091r5_driver);
> > +
> > +MODULE_AUTHOR("Beniamin Bia <[email protected]>");
> > +MODULE_DESCRIPTION("Analog Devices AD7091R5 multi-channel ADC driver");
> > +MODULE_LICENSE("GPL v2");