[PATCH V3 1/2] iio:adc:ad7949: Add AD7949 ADC driver family

[PATCH V3 1/2] iio:adc:ad7949: Add AD7949 ADC driver family

[Charles-Antoine Couret]

Compatible with AD7682 and AD7689 chips.
It is a Analog Devices ADC driver 14/16 bits 4/8 channels
with SPI protocol

Datasheet of the device:
http://www.analog.com/media/en/technical-documentation/data-sheets/AD7949.pdf

Signed-off-by: Charles-Antoine Couret <charles-antoine.couret@essensium.com>
---
V1 to V2:
* Use GENMASK
* Little refactoring to define SPI message parameters
* Add ____cacheline_aligned for SPI buffer
* Comment required delays
* Clean switch statements
* Init SPI before iio

V2 to V3:
* Merge message init functions into spi_message_init_with_transfers call
* Add vendor prefix for device tree
* Remove SPI settings overrinding, use info from device tree only

 drivers/iio/adc/Kconfig  |  10 ++
 drivers/iio/adc/Makefile |   1 +
 drivers/iio/adc/ad7949.c | 347 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 358 insertions(+)
 create mode 100644 drivers/iio/adc/ad7949.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 4a754921fb6f..42e66efff6c0 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -116,6 +116,16 @@ config AD7923
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad7923.
 
+config AD7949
+	tristate "Analog Devices AD7949 and similar ADCs driver"
+	depends on SPI
+	help
+	  Say yes here to build support for Analog Devices
+	  AD7949, AD7682, AD7689 8 Channel ADCs.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad7949.
+
 config AD799X
 	tristate "Analog Devices AD799x ADC driver"
 	depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 03db7b578f9c..88804c867aa9 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_AD7766) += ad7766.o
 obj-$(CONFIG_AD7791) += ad7791.o
 obj-$(CONFIG_AD7793) += ad7793.o
 obj-$(CONFIG_AD7887) += ad7887.o
+obj-$(CONFIG_AD7949) += ad7949.o
 obj-$(CONFIG_AD799X) += ad799x.o
 obj-$(CONFIG_ASPEED_ADC) += aspeed_adc.o
 obj-$(CONFIG_AT91_ADC) += at91_adc.o
diff --git a/drivers/iio/adc/ad7949.c b/drivers/iio/adc/ad7949.c
new file mode 100644
index 000000000000..5f93ab0ebac2
--- /dev/null
+++ b/drivers/iio/adc/ad7949.c
@@ -0,0 +1,348 @@
+// SPDX-License-Identifier: GPL-2.0
+/* ad7949.c - Analog Devices ADC driver 14/16 bits 4/8 channels
+ *
+ * Copyright (C) 2018 CMC NV
+ *
+ * http://www.analog.com/media/en/technical-documentation/data-sheets/AD7949.pdf
+ */
+
+#include <linux/delay.h>
+#include <linux/iio/iio.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#define AD7949_MASK_CHANNEL_SEL		GENMASK(9, 7)
+#define AD7949_MASK_TOTAL		GENMASK(13, 0)
+#define AD7949_OFFSET_CHANNEL_SEL	7
+#define AD7949_CFG_READ_BACK		0x1
+#define AD7949_CFG_REG_SIZE_BITS	14
+
+enum {
+	ID_AD7949 = 0,
+	ID_AD7682,
+	ID_AD7689,
+};
+
+struct ad7949_adc_spec {
+	u8 num_channels;
+	u8 resolution;
+};
+
+static const struct ad7949_adc_spec ad7949_adc_spec[] = {
+	[ID_AD7949] = { .num_channels = 8, .resolution = 14 },
+	[ID_AD7682] = { .num_channels = 4, .resolution = 16 },
+	[ID_AD7689] = { .num_channels = 8, .resolution = 16 },
+};
+
+/**
+ * struct ad7949_adc_chip - AD ADC chip
+ * @lock: protects write sequences
+ * @vref: regulator generating Vref
+ * @iio_dev: reference to iio structure
+ * @spi: reference to spi structure
+ * @resolution: resolution of the chip
+ * @cfg: copy of the configuration register
+ * @current_channel: current channel in use
+ * @buffer: buffer to send / receive data to / from device
+ */
+struct ad7949_adc_chip {
+	struct mutex lock;
+	struct regulator *vref;
+	struct iio_dev *indio_dev;
+	struct spi_device *spi;
+	u8 resolution;
+	u16 cfg;
+	unsigned int current_channel;
+	u32 buffer ____cacheline_aligned;
+};
+
+static bool ad7949_spi_cfg_is_read_back(struct ad7949_adc_chip *ad7949_adc)
+{
+	if (!(ad7949_adc->cfg & AD7949_CFG_READ_BACK))
+		return true;
+
+	return false;
+}
+
+static int ad7949_spi_bits_per_word(struct ad7949_adc_chip *ad7949_adc)
+{
+	int ret = ad7949_adc->resolution;
+
+	if (ad7949_spi_cfg_is_read_back(ad7949_adc))
+		ret += AD7949_CFG_REG_SIZE_BITS;
+
+	return ret;
+}
+
+static int ad7949_spi_write_cfg(struct ad7949_adc_chip *ad7949_adc, u16 val,
+				u16 mask)
+{
+	int ret;
+	int bits_per_word = ad7949_spi_bits_per_word(ad7949_adc);
+	int shift = bits_per_word - AD7949_CFG_REG_SIZE_BITS;
+	struct spi_message msg;
+	struct spi_transfer tx[] = {
+		{
+			.tx_buf = &ad7949_adc->buffer,
+			.len = 4,
+			.bits_per_word = bits_per_word,
+		},
+	};
+
+	ad7949_adc->cfg = (val & mask) | (ad7949_adc->cfg & ~mask);
+	ad7949_adc->buffer = ad7949_adc->cfg << shift;
+	spi_message_init_with_transfers(&msg, tx, 1);
+	ret = spi_sync(ad7949_adc->spi, &msg);
+
+	/*
+	 * This delay is to avoid a new request before the required time to
+	 * send a new command to the device
+	 */
+	udelay(2);
+	return ret;
+}
+
+static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val,
+				   unsigned int channel)
+{
+	int ret;
+	int bits_per_word = ad7949_spi_bits_per_word(ad7949_adc);
+	int mask = GENMASK(ad7949_adc->resolution, 0);
+	struct spi_message msg;
+	struct spi_transfer tx[] = {
+		{
+			.rx_buf = &ad7949_adc->buffer,
+			.len = 4,
+			.bits_per_word = bits_per_word,
+		},
+	};
+
+	ret = ad7949_spi_write_cfg(ad7949_adc,
+				   channel << AD7949_OFFSET_CHANNEL_SEL,
+				   AD7949_MASK_CHANNEL_SEL);
+	if (ret)
+		return ret;
+
+	ad7949_adc->buffer = 0;
+	spi_message_init_with_transfers(&msg, tx, 1);
+	ret = spi_sync(ad7949_adc->spi, &msg);
+	if (ret)
+		return ret;
+
+	/*
+	 * This delay is to avoid a new request before the required time to
+	 * send a new command to the device
+	 */
+	udelay(2);
+
+	ad7949_adc->current_channel = channel;
+
+	if (ad7949_spi_cfg_is_read_back(ad7949_adc))
+		*val = (ad7949_adc->buffer >> AD7949_CFG_REG_SIZE_BITS) & mask;
+	else
+		*val = ad7949_adc->buffer & mask;
+
+	return 0;
+}
+
+#define AD7949_ADC_CHANNEL(chan) {				\
+	.type = IIO_VOLTAGE,					\
+	.indexed = 1,						\
+	.channel = (chan),					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
+}
+
+static const struct iio_chan_spec ad7949_adc_channels[] = {
+	AD7949_ADC_CHANNEL(0),
+	AD7949_ADC_CHANNEL(1),
+	AD7949_ADC_CHANNEL(2),
+	AD7949_ADC_CHANNEL(3),
+	AD7949_ADC_CHANNEL(4),
+	AD7949_ADC_CHANNEL(5),
+	AD7949_ADC_CHANNEL(6),
+	AD7949_ADC_CHANNEL(7),
+};
+
+static int ad7949_spi_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct ad7949_adc_chip *ad7949_adc = iio_priv(indio_dev);
+	int ret;
+
+	if (!val)
+		return -EINVAL;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&ad7949_adc->lock);
+		ret = ad7949_spi_read_channel(ad7949_adc, val, chan->channel);
+		mutex_unlock(&ad7949_adc->lock);
+
+		if (ret < 0)
+			return ret;
+
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		ret = regulator_get_voltage(ad7949_adc->vref);
+		if (ret < 0)
+			return ret;
+
+		*val = ret / 5000;
+		return IIO_VAL_INT;
+	}
+
+	return -EINVAL;
+}
+
+static int ad7949_spi_reg_access(struct iio_dev *indio_dev,
+			unsigned int reg, unsigned int writeval,
+			unsigned int *readval)
+{
+	struct ad7949_adc_chip *ad7949_adc = iio_priv(indio_dev);
+	int ret = 0;
+
+	if (readval)
+		*readval = ad7949_adc->cfg;
+	else
+		ret = ad7949_spi_write_cfg(ad7949_adc,
+			writeval & AD7949_MASK_TOTAL, AD7949_MASK_TOTAL);
+
+	return ret;
+}
+
+static const struct iio_info ad7949_spi_info = {
+	.read_raw = ad7949_spi_read_raw,
+	.debugfs_reg_access = ad7949_spi_reg_access,
+};
+
+static int ad7949_spi_init(struct ad7949_adc_chip *ad7949_adc)
+{
+	int ret;
+	int val;
+
+	/* Sequencer disabled, CFG readback disabled, IN0 as default channel */
+	ad7949_adc->current_channel = 0;
+	ret = ad7949_spi_write_cfg(ad7949_adc, 0x3C79, AD7949_MASK_TOTAL);
+
+	/* Do two dummy conversions to apply the first configuration setting.
+	 * Required only after the start up of the device.
+	 */
+	ad7949_spi_read_channel(ad7949_adc, &val, ad7949_adc->current_channel);
+	ad7949_spi_read_channel(ad7949_adc, &val, ad7949_adc->current_channel);
+
+	return ret;
+}
+
+static int ad7949_spi_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	const struct ad7949_adc_spec *spec;
+	struct ad7949_adc_chip *ad7949_adc;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*ad7949_adc));
+	if (!indio_dev) {
+		dev_err(dev, "can not allocate iio device\n");
+		return -ENOMEM;
+	}
+
+	indio_dev->dev.parent = dev;
+	indio_dev->dev.of_node = dev->of_node;
+	indio_dev->info = &ad7949_spi_info;
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = ad7949_adc_channels;
+	spi_set_drvdata(spi, indio_dev);
+
+	ad7949_adc = iio_priv(indio_dev);
+	ad7949_adc->indio_dev = indio_dev;
+	ad7949_adc->spi = spi;
+
+	spec = &ad7949_adc_spec[spi_get_device_id(spi)->driver_data];
+	indio_dev->num_channels = spec->num_channels;
+	ad7949_adc->resolution = spec->resolution;
+
+	ad7949_adc->vref = devm_regulator_get(dev, "vref");
+	if (IS_ERR(ad7949_adc->vref)) {
+		dev_err(dev, "fail to request regulator\n");
+		return PTR_ERR(ad7949_adc->vref);
+	}
+
+	ret = regulator_enable(ad7949_adc->vref);
+	if (ret < 0) {
+		dev_err(dev, "fail to enable regulator\n");
+		goto err_regulator;
+	}
+
+	mutex_init(&ad7949_adc->lock);
+
+	ret = ad7949_spi_init(ad7949_adc);
+	if (ret) {
+		dev_err(dev, "enable to init this device: %d\n", ret);
+		goto err;
+	}
+
+	ret = iio_device_register(indio_dev);
+	if (ret) {
+		dev_err(dev, "fail to register iio device: %d\n", ret);
+		goto err;
+	}
+
+	return 0;
+
+err:
+	mutex_destroy(&ad7949_adc->lock);
+	regulator_disable(ad7949_adc->vref);
+err_regulator:
+	return ret;
+}
+
+static int ad7949_spi_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct ad7949_adc_chip *ad7949_adc = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+	mutex_destroy(&ad7949_adc->lock);
+	regulator_disable(ad7949_adc->vref);
+
+	return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id ad7949_spi_of_id[] = {
+	{ .compatible = "adi,ad7949" },
+	{ .compatible = "adi,ad7682" },
+	{ .compatible = "adi,ad7689" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad7949_spi_of_id);
+#endif
+
+static const struct spi_device_id ad7949_spi_id[] = {
+	{ "ad7949", ID_AD7949  },
+	{ "ad7682", ID_AD7682 },
+	{ "ad7689", ID_AD7689 },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ad7949_spi_id);
+
+static struct spi_driver ad7949_spi_driver = {
+	.driver = {
+		.name		= "ad7949",
+		.of_match_table	= of_match_ptr(ad7949_spi_of_id),
+	},
+	.probe	  = ad7949_spi_probe,
+	.remove   = ad7949_spi_remove,
+	.id_table = ad7949_spi_id,
+};
+module_spi_driver(ad7949_spi_driver);
+
+MODULE_AUTHOR("Charles-Antoine Couret <charles-antoine.couret@essensium.com>");
+MODULE_DESCRIPTION("Analog Devices 14/16-bit 8-channel ADC driver");
+MODULE_LICENSE("GPL v2");
-- 
2.19.1

[PATCH V3 2/2] doc:devicetree: Add AD7949 device tree bindings in documentation

[Charles-Antoine Couret]

Description how to invoke ad7949 driver from device tree file.

Signed-off-by: Charles-Antoine Couret <charles-antoine.couret@essensium.com>
---
V1 to V3:
* Add prefix vendor
* Improve patch description

 .../devicetree/bindings/iio/adc/ad7949.txt     | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ad7949.txt

diff --git a/Documentation/devicetree/bindings/iio/adc/ad7949.txt b/Documentation/devicetree/bindings/iio/adc/ad7949.txt
new file mode 100644
index 000000000000..f1c3af53f5cc
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ad7949.txt
@@ -0,0 +1,18 @@
+* Analog Devices AD7949/AD7682/AD7689
+
+Required properties:
+ - compatible: Should be one of
+	* "adi,ad7949"
+	* "adi,ad7682"
+	* "adi,ad7689"
+ - reg: spi chip select number for the device
+ - vref-supply: The regulator supply for ADC reference voltage
+ - spi-max-frequency: Max SPI frequency to use (< 33000000)
+
+Example:
+adc@0 {
+	compatible = "adi,ad7949";
+	reg = <0>;
+	vref-supply = <&vdd_supply>;
+	spi-max-frequency = <200000>;
+};
-- 
2.19.1

Re: [PATCH V3 2/2] doc:devicetree: Add AD7949 device tree bindings in documentation

[Rob Herring]

On Mon, Oct 22, 2018 at 11:02:43PM +0200, Charles-Antoine Couret wrote:
> Description how to invoke ad7949 driver from device tree file.

Use subject prefixes that match the subsystem (i.e run git log --oneline 
<dir> to see). Something like this would be better:

dt-bindings: iio: adc: Add Analog Devices AD7949, AD7682 and AD7689

> 
> Signed-off-by: Charles-Antoine Couret <charles-antoine.couret@essensium.com>
> ---
> V1 to V3:
> * Add prefix vendor
> * Improve patch description
> 
>  .../devicetree/bindings/iio/adc/ad7949.txt     | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ad7949.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ad7949.txt b/Documentation/devicetree/bindings/iio/adc/ad7949.txt
> new file mode 100644
> index 000000000000..f1c3af53f5cc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ad7949.txt
> @@ -0,0 +1,18 @@
> +* Analog Devices AD7949/AD7682/AD7689
> +
> +Required properties:
> + - compatible: Should be one of
> +	* "adi,ad7949"
> +	* "adi,ad7682"
> +	* "adi,ad7689"
> + - reg: spi chip select number for the device
> + - vref-supply: The regulator supply for ADC reference voltage
> + - spi-max-frequency: Max SPI frequency to use (< 33000000)

This should be optional and only be needed if the frequency needs to be 
limited below what the host and device can support. IOW, a board 
limitation. The host and device drivers already need to know their 
maximum frequencies to validate this value.

> +
> +Example:
> +adc@0 {
> +	compatible = "adi,ad7949";
> +	reg = <0>;
> +	vref-supply = <&vdd_supply>;
> +	spi-max-frequency = <200000>;
> +};
> -- 
> 2.19.1
> 

Re: [PATCH V3 1/2] iio:adc:ad7949: Add AD7949 ADC driver family

[Jonathan Cameron]

On Mon, 22 Oct 2018 23:02:42 +0200
Charles-Antoine Couret <charles-antoine.couret@essensium.com> wrote:

> Compatible with AD7682 and AD7689 chips.
> It is a Analog Devices ADC driver 14/16 bits 4/8 channels
> with SPI protocol
> 
> Datasheet of the device:
> http://www.analog.com/media/en/technical-documentation/data-sheets/AD7949.pdf
> 
> Signed-off-by: Charles-Antoine Couret <charles-antoine.couret@essensium.com>

Hi Charles-Antoine

There are a couple of really small things in here. Given most of them weren't
in my previous list and to save us both time I'll fix them up whilst applying.

Please do check I didn't mess it up however!

Jonathan

> ---
> V1 to V2:
> * Use GENMASK
> * Little refactoring to define SPI message parameters
> * Add ____cacheline_aligned for SPI buffer
> * Comment required delays
> * Clean switch statements
> * Init SPI before iio
> 
> V2 to V3:
> * Merge message init functions into spi_message_init_with_transfers call
> * Add vendor prefix for device tree
> * Remove SPI settings overrinding, use info from device tree only
> 
>  drivers/iio/adc/Kconfig  |  10 ++
>  drivers/iio/adc/Makefile |   1 +
>  drivers/iio/adc/ad7949.c | 347 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 358 insertions(+)
>  create mode 100644 drivers/iio/adc/ad7949.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 4a754921fb6f..42e66efff6c0 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -116,6 +116,16 @@ config AD7923
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ad7923.
>  
> +config AD7949
> +	tristate "Analog Devices AD7949 and similar ADCs driver"
> +	depends on SPI
> +	help
> +	  Say yes here to build support for Analog Devices
> +	  AD7949, AD7682, AD7689 8 Channel ADCs.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ad7949.
> +
>  config AD799X
>  	tristate "Analog Devices AD799x ADC driver"
>  	depends on I2C
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 03db7b578f9c..88804c867aa9 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_AD7766) += ad7766.o
>  obj-$(CONFIG_AD7791) += ad7791.o
>  obj-$(CONFIG_AD7793) += ad7793.o
>  obj-$(CONFIG_AD7887) += ad7887.o
> +obj-$(CONFIG_AD7949) += ad7949.o
>  obj-$(CONFIG_AD799X) += ad799x.o
>  obj-$(CONFIG_ASPEED_ADC) += aspeed_adc.o
>  obj-$(CONFIG_AT91_ADC) += at91_adc.o
> diff --git a/drivers/iio/adc/ad7949.c b/drivers/iio/adc/ad7949.c
> new file mode 100644
> index 000000000000..5f93ab0ebac2
> --- /dev/null
> +++ b/drivers/iio/adc/ad7949.c
> @@ -0,0 +1,348 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* ad7949.c - Analog Devices ADC driver 14/16 bits 4/8 channels
> + *
> + * Copyright (C) 2018 CMC NV
> + *
> + * http://www.analog.com/media/en/technical-documentation/data-sheets/AD7949.pdf
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/iio/iio.h>
> +#include <linux/module.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +
> +#define AD7949_MASK_CHANNEL_SEL		GENMASK(9, 7)
> +#define AD7949_MASK_TOTAL		GENMASK(13, 0)
> +#define AD7949_OFFSET_CHANNEL_SEL	7
> +#define AD7949_CFG_READ_BACK		0x1
> +#define AD7949_CFG_REG_SIZE_BITS	14
> +
> +enum {
> +	ID_AD7949 = 0,
> +	ID_AD7682,
> +	ID_AD7689,
> +};
> +
> +struct ad7949_adc_spec {
> +	u8 num_channels;
> +	u8 resolution;
> +};
> +
> +static const struct ad7949_adc_spec ad7949_adc_spec[] = {
> +	[ID_AD7949] = { .num_channels = 8, .resolution = 14 },
> +	[ID_AD7682] = { .num_channels = 4, .resolution = 16 },
> +	[ID_AD7689] = { .num_channels = 8, .resolution = 16 },
> +};
> +
> +/**
> + * struct ad7949_adc_chip - AD ADC chip
> + * @lock: protects write sequences
> + * @vref: regulator generating Vref
> + * @iio_dev: reference to iio structure
> + * @spi: reference to spi structure
> + * @resolution: resolution of the chip
> + * @cfg: copy of the configuration register
> + * @current_channel: current channel in use
> + * @buffer: buffer to send / receive data to / from device
> + */
> +struct ad7949_adc_chip {
> +	struct mutex lock;
> +	struct regulator *vref;
> +	struct iio_dev *indio_dev;
> +	struct spi_device *spi;
> +	u8 resolution;
> +	u16 cfg;
> +	unsigned int current_channel;
> +	u32 buffer ____cacheline_aligned;
> +};
> +
> +static bool ad7949_spi_cfg_is_read_back(struct ad7949_adc_chip *ad7949_adc)
> +{
> +	if (!(ad7949_adc->cfg & AD7949_CFG_READ_BACK))
> +		return true;
> +
> +	return false;
> +}
> +
> +static int ad7949_spi_bits_per_word(struct ad7949_adc_chip *ad7949_adc)
> +{
> +	int ret = ad7949_adc->resolution;
> +
> +	if (ad7949_spi_cfg_is_read_back(ad7949_adc))
> +		ret += AD7949_CFG_REG_SIZE_BITS;
> +
> +	return ret;
> +}
> +
> +static int ad7949_spi_write_cfg(struct ad7949_adc_chip *ad7949_adc, u16 val,
> +				u16 mask)
> +{
> +	int ret;
> +	int bits_per_word = ad7949_spi_bits_per_word(ad7949_adc);
> +	int shift = bits_per_word - AD7949_CFG_REG_SIZE_BITS;
> +	struct spi_message msg;
> +	struct spi_transfer tx[] = {
> +		{
> +			.tx_buf = &ad7949_adc->buffer,
> +			.len = 4,
> +			.bits_per_word = bits_per_word,
> +		},
> +	};
> +
> +	ad7949_adc->cfg = (val & mask) | (ad7949_adc->cfg & ~mask);
> +	ad7949_adc->buffer = ad7949_adc->cfg << shift;
> +	spi_message_init_with_transfers(&msg, tx, 1);
> +	ret = spi_sync(ad7949_adc->spi, &msg);
> +
> +	/*
> +	 * This delay is to avoid a new request before the required time to
> +	 * send a new command to the device
> +	 */
> +	udelay(2);
> +	return ret;
> +}
> +
> +static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val,
> +				   unsigned int channel)
> +{
> +	int ret;
> +	int bits_per_word = ad7949_spi_bits_per_word(ad7949_adc);
> +	int mask = GENMASK(ad7949_adc->resolution, 0);
> +	struct spi_message msg;
> +	struct spi_transfer tx[] = {
> +		{
> +			.rx_buf = &ad7949_adc->buffer,
> +			.len = 4,
> +			.bits_per_word = bits_per_word,
> +		},
> +	};
> +
> +	ret = ad7949_spi_write_cfg(ad7949_adc,
> +				   channel << AD7949_OFFSET_CHANNEL_SEL,
> +				   AD7949_MASK_CHANNEL_SEL);
> +	if (ret)
> +		return ret;
> +
> +	ad7949_adc->buffer = 0;
> +	spi_message_init_with_transfers(&msg, tx, 1);
> +	ret = spi_sync(ad7949_adc->spi, &msg);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * This delay is to avoid a new request before the required time to
> +	 * send a new command to the device
> +	 */
> +	udelay(2);
> +
> +	ad7949_adc->current_channel = channel;
> +
> +	if (ad7949_spi_cfg_is_read_back(ad7949_adc))
> +		*val = (ad7949_adc->buffer >> AD7949_CFG_REG_SIZE_BITS) & mask;
> +	else
> +		*val = ad7949_adc->buffer & mask;
> +
> +	return 0;
> +}
> +
> +#define AD7949_ADC_CHANNEL(chan) {				\
> +	.type = IIO_VOLTAGE,					\
> +	.indexed = 1,						\
> +	.channel = (chan),					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
> +}
> +
> +static const struct iio_chan_spec ad7949_adc_channels[] = {
> +	AD7949_ADC_CHANNEL(0),
> +	AD7949_ADC_CHANNEL(1),
> +	AD7949_ADC_CHANNEL(2),
> +	AD7949_ADC_CHANNEL(3),
> +	AD7949_ADC_CHANNEL(4),
> +	AD7949_ADC_CHANNEL(5),
> +	AD7949_ADC_CHANNEL(6),
> +	AD7949_ADC_CHANNEL(7),
> +};
> +
> +static int ad7949_spi_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long mask)
> +{
> +	struct ad7949_adc_chip *ad7949_adc = iio_priv(indio_dev);
> +	int ret;
> +
> +	if (!val)
> +		return -EINVAL;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&ad7949_adc->lock);
> +		ret = ad7949_spi_read_channel(ad7949_adc, val, chan->channel);
> +		mutex_unlock(&ad7949_adc->lock);
> +
> +		if (ret < 0)
> +			return ret;
> +
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		ret = regulator_get_voltage(ad7949_adc->vref);
> +		if (ret < 0)
> +			return ret;
> +
> +		*val = ret / 5000;
> +		return IIO_VAL_INT;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int ad7949_spi_reg_access(struct iio_dev *indio_dev,
> +			unsigned int reg, unsigned int writeval,
> +			unsigned int *readval)
> +{
> +	struct ad7949_adc_chip *ad7949_adc = iio_priv(indio_dev);
> +	int ret = 0;
> +
> +	if (readval)
> +		*readval = ad7949_adc->cfg;
> +	else
> +		ret = ad7949_spi_write_cfg(ad7949_adc,
> +			writeval & AD7949_MASK_TOTAL, AD7949_MASK_TOTAL);
> +
> +	return ret;
> +}
> +
> +static const struct iio_info ad7949_spi_info = {
> +	.read_raw = ad7949_spi_read_raw,
> +	.debugfs_reg_access = ad7949_spi_reg_access,
> +};
> +
> +static int ad7949_spi_init(struct ad7949_adc_chip *ad7949_adc)
> +{
> +	int ret;
> +	int val;
> +
> +	/* Sequencer disabled, CFG readback disabled, IN0 as default channel */
> +	ad7949_adc->current_channel = 0;
> +	ret = ad7949_spi_write_cfg(ad7949_adc, 0x3C79, AD7949_MASK_TOTAL);
> +
> +	/* Do two dummy conversions to apply the first configuration setting.
/*
 * Do two...

> +	 * Required only after the start up of the device.
> +	 */
> +	ad7949_spi_read_channel(ad7949_adc, &val, ad7949_adc->current_channel);
I'm not that keen on the absence of error checking on these.  Given they aren't
real I'll let it go though.
> +	ad7949_spi_read_channel(ad7949_adc, &val, ad7949_adc->current_channel);
> +
> +	return ret;
> +}
> +
> +static int ad7949_spi_probe(struct spi_device *spi)
> +{
> +	struct device *dev = &spi->dev;
> +	const struct ad7949_adc_spec *spec;
> +	struct ad7949_adc_chip *ad7949_adc;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*ad7949_adc));
> +	if (!indio_dev) {
> +		dev_err(dev, "can not allocate iio device\n");
> +		return -ENOMEM;
> +	}
> +
> +	indio_dev->dev.parent = dev;
> +	indio_dev->dev.of_node = dev->of_node;
> +	indio_dev->info = &ad7949_spi_info;
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = ad7949_adc_channels;
> +	spi_set_drvdata(spi, indio_dev);
> +
> +	ad7949_adc = iio_priv(indio_dev);
> +	ad7949_adc->indio_dev = indio_dev;
> +	ad7949_adc->spi = spi;
> +
> +	spec = &ad7949_adc_spec[spi_get_device_id(spi)->driver_data];
> +	indio_dev->num_channels = spec->num_channels;
> +	ad7949_adc->resolution = spec->resolution;
> +
> +	ad7949_adc->vref = devm_regulator_get(dev, "vref");
> +	if (IS_ERR(ad7949_adc->vref)) {
> +		dev_err(dev, "fail to request regulator\n");
> +		return PTR_ERR(ad7949_adc->vref);
> +	}
> +
> +	ret = regulator_enable(ad7949_adc->vref);
> +	if (ret < 0) {
> +		dev_err(dev, "fail to enable regulator\n");
> +		goto err_regulator;
> +	}
> +
> +	mutex_init(&ad7949_adc->lock);
> +
> +	ret = ad7949_spi_init(ad7949_adc);
> +	if (ret) {
> +		dev_err(dev, "enable to init this device: %d\n", ret);
> +		goto err;
> +	}
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret) {
> +		dev_err(dev, "fail to register iio device: %d\n", ret);
> +		goto err;
> +	}
> +
> +	return 0;
> +
> +err:
> +	mutex_destroy(&ad7949_adc->lock);
> +	regulator_disable(ad7949_adc->vref);
> +err_regulator:

It's preferred to return directly if there is no cleanup to do.
Makes the code more readable (as you don't have to check the cleanup
paths on a direct return - other than verifying there shouldn't be
any of them).

> +	return ret;
> +}
> +
> +static int ad7949_spi_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct ad7949_adc_chip *ad7949_adc = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +	mutex_destroy(&ad7949_adc->lock);
> +	regulator_disable(ad7949_adc->vref);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id ad7949_spi_of_id[] = {
> +	{ .compatible = "adi,ad7949" },
> +	{ .compatible = "adi,ad7682" },
> +	{ .compatible = "adi,ad7689" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ad7949_spi_of_id);
> +#endif
> +
> +static const struct spi_device_id ad7949_spi_id[] = {
> +	{ "ad7949", ID_AD7949  },
> +	{ "ad7682", ID_AD7682 },
> +	{ "ad7689", ID_AD7689 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(spi, ad7949_spi_id);
> +
> +static struct spi_driver ad7949_spi_driver = {
> +	.driver = {
> +		.name		= "ad7949",
> +		.of_match_table	= of_match_ptr(ad7949_spi_of_id),
Now there is an interesting story behind of_match_ptr.

We used to only be interested in devicetree id tables if
we were running devicetree, then someone had a clever idea of
providing a handy way (for testing) of using ACPI (with a magic
ACPI id) to instantiate device drivers based on their devicetree
pointers.

That only works if we don't define the of_match_table to NULL.

Upshot is that general view is now don't use of_match_ptr certainly
for devices that are likely to be randomly tested on boards (like
this might be).

The same is true of not defining out the table above.


> +	},
> +	.probe	  = ad7949_spi_probe,
> +	.remove   = ad7949_spi_remove,
> +	.id_table = ad7949_spi_id,
> +};
> +module_spi_driver(ad7949_spi_driver);
> +
> +MODULE_AUTHOR("Charles-Antoine Couret <charles-antoine.couret@essensium.com>");
> +MODULE_DESCRIPTION("Analog Devices 14/16-bit 8-channel ADC driver");
> +MODULE_LICENSE("GPL v2");

Re: [PATCH V3 2/2] doc:devicetree: Add AD7949 device tree bindings in documentation

[Jonathan Cameron]

On Tue, 23 Oct 2018 10:43:19 -0500
Rob Herring <robh@kernel.org> wrote:

> On Mon, Oct 22, 2018 at 11:02:43PM +0200, Charles-Antoine Couret wrote:
> > Description how to invoke ad7949 driver from device tree file.  
> 
> Use subject prefixes that match the subsystem (i.e run git log --oneline 
> <dir> to see). Something like this would be better:
> 
> dt-bindings: iio: adc: Add Analog Devices AD7949, AD7682 and AD7689
I've modified this and changed as below in the interests of saving everyone
time that a V4 and it's review would take!
Thanks for the review Rob.

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

Thanks,
Jonathan

> 
> > 
> > Signed-off-by: Charles-Antoine Couret <charles-antoine.couret@essensium.com>
> > ---
> > V1 to V3:
> > * Add prefix vendor
> > * Improve patch description
> > 
> >  .../devicetree/bindings/iio/adc/ad7949.txt     | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/adc/ad7949.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/ad7949.txt b/Documentation/devicetree/bindings/iio/adc/ad7949.txt
> > new file mode 100644
> > index 000000000000..f1c3af53f5cc
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/ad7949.txt
> > @@ -0,0 +1,18 @@
> > +* Analog Devices AD7949/AD7682/AD7689
> > +
> > +Required properties:
> > + - compatible: Should be one of
> > +	* "adi,ad7949"
> > +	* "adi,ad7682"
> > +	* "adi,ad7689"
> > + - reg: spi chip select number for the device
> > + - vref-supply: The regulator supply for ADC reference voltage
> > + - spi-max-frequency: Max SPI frequency to use (< 33000000)  
> 
> This should be optional and only be needed if the frequency needs to be 
> limited below what the host and device can support. IOW, a board 
> limitation. The host and device drivers already need to know their 
> maximum frequencies to validate this value.
I dropped this from the binding as it is generic anyway rather than
device specific.

> 
> > +
> > +Example:
> > +adc@0 {
> > +	compatible = "adi,ad7949";
> > +	reg = <0>;
> > +	vref-supply = <&vdd_supply>;
> > +	spi-max-frequency = <200000>;
> > +};
> > -- 
> > 2.19.1
> >   

Re: [PATCH V3 1/2] iio:adc:ad7949: Add AD7949 ADC driver family

[Couret Charles-Antoine]

Le 28/10/2018 à 16:13, Jonathan Cameron a écrit :
Hi,
> There are a couple of really small things in here. Given most of them 
> weren't
> in my previous list and to save us both time I'll fix them up whilst applying.

Ok.

I don't find anything else, your remarks are relevant from my point of view.

Thank you.

Regards,

Charles-Antoine Couret