linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 3/3] iio:dac:ad5686: Add AD5671R/75R/94/94R/95R/96/96R support
@ 2018-03-30 13:56 Stefan Popa
  2018-04-06 15:32 ` Jonathan Cameron
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Stefan Popa @ 2018-03-30 13:56 UTC (permalink / raw)
  To: lars, Michael.Hennerich, jic23
  Cc: knaack.h, pmeerw, davem, gregkh, mchehab, linus.walleij, rdunlap,
	Ismail.Kose, lukas, mike.looijmans, fabrice.gasnier, linux-pm,
	linux-iio, linux-kernel, stefan.popa

The AD5694/AD5694R/AD5695R/AD5696/AD5696R are a family of 4 channel DAC
s with 12-bit, 14-bit and 16-bit precision respectively. The devices have
either no built-in reference, or built-in 2.5V reference.

The AD5671R/AD5675R are similar, except that they have 8 instead of 4
channels.

These devices are similar to AD5672R/AD5676/AD5676R and
AD5684/AD5684R/AD5684/AD5685R/AD5686/AD5686R, except that they use i2c
instead of spi. In order to add support in the previously existing driver,
three modules were created. One baseline module that has all the
chip-logic and then one module for SPI and one for I2C.

Datasheets:
http://www.analog.com/media/en/technical-documentation/data-sheets/AD5671R_5675R.pdf
http://www.analog.com/media/en/technical-documentation/data-sheets/AD5696R_5695R_5694R.pdf

Signed-off-by: Stefan Popa <stefan.popa@analog.com>
---
 MAINTAINERS                  |   8 ++
 drivers/iio/dac/Kconfig      |  27 ++++--
 drivers/iio/dac/ad5686-spi.c |  93 ++++++++++++++++++
 drivers/iio/dac/ad5686.c     | 222 +++++++++++++------------------------------
 drivers/iio/dac/ad5686.h     | 114 ++++++++++++++++++++++
 drivers/iio/dac/ad5696-i2c.c |  98 +++++++++++++++++++
 6 files changed, 400 insertions(+), 162 deletions(-)
 create mode 100644 drivers/iio/dac/ad5686-spi.c
 create mode 100644 drivers/iio/dac/ad5686.h
 create mode 100644 drivers/iio/dac/ad5696-i2c.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 473ac00..002cb01 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -791,6 +791,14 @@ M:	Michael Hanselmann <linux-kernel@hansmi.ch>
 S:	Supported
 F:	drivers/macintosh/ams/
 
+ANALOG DEVICES INC AD5686 DRIVER
+M:	Stefan Popa <stefan.popa@analog.com>
+L:	linux-pm@vger.kernel.org
+W:	http://ez.analog.com/community/linux-device-drivers
+S:	Supported
+F:	drivers/iio/dac/ad5686*
+F:	drivers/iio/dac/ad5696*
+
 ANALOG DEVICES INC AD9389B DRIVER
 M:	Hans Verkuil <hans.verkuil@cisco.com>
 L:	linux-media@vger.kernel.org
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 965d5c0..dab0b8a 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -131,15 +131,30 @@ config LTC2632
 	  module will be called ltc2632.
 
 config AD5686
-	tristate "Analog Devices AD5686R/AD5685R/AD5684R DAC SPI driver"
+	tristate
+
+config AD5686_SPI
+	tristate "Analog Devices AD5686 and similar multi-channel DACs (SPI)"
 	depends on SPI
+	select AD5686
 	help
-	  Say yes here to build support for Analog Devices AD5686R, AD5685R,
-	  AD5684R, AD5791 Voltage Output Digital to
-	  Analog Converter.
+	Say yes here to build support for Analog Devices AD5672R, AD5676,
+	AD5676R, AD5684, AD5684R, AD5684R, AD5685R, AD5686, AD5686R.
+	Voltage Output Digital to Analog Converter.
 
-	  To compile this driver as a module, choose M here: the
-	  module will be called ad5686.
+	To compile this driver as a module, choose M here: the
+	module will be called ad5686.
+
+config AD5696_I2C
+	tristate "Analog Devices AD5696 and similar multi-channel DACs (I2C)"
+	depends on I2C
+	select AD5686
+	help
+	Say yes here to build support for Analog Devices AD5671R, AD5675R,
+	AD5694, AD5694R, AD5695R, AD5696, AD5696R Voltage Output Digital to
+	Analog Converter.
+	To compile this driver as a module, choose M here: the module will be
+	called ad5696.
 
 config AD5755
 	tristate "Analog Devices AD5755/AD5755-1/AD5757/AD5735/AD5737 DAC driver"
diff --git a/drivers/iio/dac/ad5686-spi.c b/drivers/iio/dac/ad5686-spi.c
new file mode 100644
index 0000000..d68579b
--- /dev/null
+++ b/drivers/iio/dac/ad5686-spi.c
@@ -0,0 +1,93 @@
+/*
+ * AD5672R, AD5676, AD5676R, AD5684, AD5684R, AD5684R, AD5685R, AD5686, AD5686R
+ * Digital to analog converters driver
+ *
+ * Copyright 2018 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2.
+ */
+
+#include "ad5686.h"
+
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+
+static int ad5686_spi_write(struct ad5686_state *st,
+			    u8 cmd, u8 addr, u16 val)
+{
+	struct spi_device *spi = to_spi_device(st->dev);
+
+	st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) |
+				      AD5686_ADDR(addr) |
+				      val);
+
+	return spi_write(spi, &st->data[0].d8[1], 3);
+}
+
+static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
+{
+	struct spi_transfer t[] = {
+		{
+			.tx_buf = &st->data[0].d8[1],
+			.len = 3,
+			.cs_change = 1,
+		}, {
+			.tx_buf = &st->data[1].d8[1],
+			.rx_buf = &st->data[2].d8[1],
+			.len = 3,
+		},
+	};
+	struct spi_device *spi = to_spi_device(st->dev);
+	int ret;
+
+	st->data[0].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_READBACK_ENABLE) |
+				      AD5686_ADDR(addr));
+	st->data[1].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_NOOP));
+
+	ret = spi_sync_transfer(spi, t, ARRAY_SIZE(t));
+	if (ret < 0)
+		return ret;
+
+	return be32_to_cpu(st->data[2].d32);
+}
+
+static int ad5686_spi_probe(struct spi_device *spi)
+{
+	const struct spi_device_id *id = spi_get_device_id(spi);
+
+	return ad5686_probe(&spi->dev, id->driver_data, id->name,
+			    ad5686_spi_write, ad5686_spi_read);
+}
+
+static int ad5686_spi_remove(struct spi_device *spi)
+{
+	return ad5686_remove(&spi->dev);
+}
+
+static const struct spi_device_id ad5686_spi_id[] = {
+	{"ad5672r", ID_AD5672R},
+	{"ad5676", ID_AD5676},
+	{"ad5676r", ID_AD5676R},
+	{"ad5684", ID_AD5684},
+	{"ad5684r", ID_AD5684R},
+	{"ad5685r", ID_AD5685R},
+	{"ad5686", ID_AD5686},
+	{"ad5686r", ID_AD5686R},
+	{}
+};
+MODULE_DEVICE_TABLE(spi, ad5686_spi_id);
+
+static struct spi_driver ad5686_spi_driver = {
+	.driver = {
+		.name = "ad5686",
+	},
+	.probe = ad5686_spi_probe,
+	.remove = ad5686_spi_remove,
+	.id_table = ad5686_spi_id,
+};
+
+module_spi_driver(ad5686_spi_driver);
+
+MODULE_AUTHOR("Stefan Popa <stefan.popa@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD5686 and similar multi-channel DACs");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index 5fb0179..e4ea0dc 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -11,7 +11,6 @@
 #include <linux/device.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
-#include <linux/spi/spi.h>
 #include <linux/slab.h>
 #include <linux/sysfs.h>
 #include <linux/regulator/consumer.h>
@@ -19,128 +18,28 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
-#define AD5686_ADDR(x)				((x) << 16)
-#define AD5686_CMD(x)				((x) << 20)
-
-#define AD5686_ADDR_DAC(chan)			(0x1 << (chan))
-#define AD5686_ADDR_ALL_DAC			0xF
-
-#define AD5686_CMD_NOOP				0x0
-#define AD5686_CMD_WRITE_INPUT_N		0x1
-#define AD5686_CMD_UPDATE_DAC_N			0x2
-#define AD5686_CMD_WRITE_INPUT_N_UPDATE_N	0x3
-#define AD5686_CMD_POWERDOWN_DAC		0x4
-#define AD5686_CMD_LDAC_MASK			0x5
-#define AD5686_CMD_RESET			0x6
-#define AD5686_CMD_INTERNAL_REFER_SETUP		0x7
-#define AD5686_CMD_DAISY_CHAIN_ENABLE		0x8
-#define AD5686_CMD_READBACK_ENABLE		0x9
-
-#define AD5686_LDAC_PWRDN_NONE			0x0
-#define AD5686_LDAC_PWRDN_1K			0x1
-#define AD5686_LDAC_PWRDN_100K			0x2
-#define AD5686_LDAC_PWRDN_3STATE		0x3
-
-/**
- * struct ad5686_chip_info - chip specific information
- * @int_vref_mv:	AD5620/40/60: the internal reference voltage
- * @num_channels:	number of channels
- * @channel:		channel specification
-*/
-
-struct ad5686_chip_info {
-	u16				int_vref_mv;
-	unsigned int			num_channels;
-	struct iio_chan_spec		*channels;
-};
-
-/**
- * struct ad5446_state - driver instance specific data
- * @spi:		spi_device
- * @chip_info:		chip model specific constants, available modes etc
- * @reg:		supply regulator
- * @vref_mv:		actual reference voltage used
- * @pwr_down_mask:	power down mask
- * @pwr_down_mode:	current power down mode
- * @data:		spi transfer buffers
- */
+#include "ad5686.h"
 
-struct ad5686_state {
-	struct spi_device		*spi;
-	const struct ad5686_chip_info	*chip_info;
-	struct regulator		*reg;
-	unsigned short			vref_mv;
-	unsigned			pwr_down_mask;
-	unsigned			pwr_down_mode;
-	/*
-	 * DMA (thus cache coherency maintenance) requires the
-	 * transfer buffers to live in their own cache lines.
-	 */
-
-	union {
-		__be32 d32;
-		u8 d8[4];
-	} data[3] ____cacheline_aligned;
+static const char * const ad5686_powerdown_modes[] = {
+	"1kohm_to_gnd",
+	"100kohm_to_gnd",
+	"three_state"
 };
 
-/**
- * ad5686_supported_device_ids:
- */
-
-enum ad5686_supported_device_ids {
-	ID_AD5672R,
-	ID_AD5676,
-	ID_AD5676R,
-	ID_AD5684,
-	ID_AD5684R,
-	ID_AD5685R,
-	ID_AD5686,
-	ID_AD5686R
-};
-static int ad5686_spi_write(struct ad5686_state *st,
-			     u8 cmd, u8 addr, u16 val, u8 shift)
+static int ad5686_write(struct ad5686_state *st,
+			u8 cmd, u8 addr, u16 val, u8 shift)
 {
 	val <<= shift;
 
-	st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) |
-			      AD5686_ADDR(addr) |
-			      val);
-
-	return spi_write(st->spi, &st->data[0].d8[1], 3);
+	return st->write(st, cmd, addr, val);
 }
 
-static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
+static int ad5686_read(struct ad5686_state *st, u8 addr)
 {
-	struct spi_transfer t[] = {
-		{
-			.tx_buf = &st->data[0].d8[1],
-			.len = 3,
-			.cs_change = 1,
-		}, {
-			.tx_buf = &st->data[1].d8[1],
-			.rx_buf = &st->data[2].d8[1],
-			.len = 3,
-		},
-	};
-	int ret;
 
-	st->data[0].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_READBACK_ENABLE) |
-			      AD5686_ADDR(addr));
-	st->data[1].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_NOOP));
-
-	ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
-	if (ret < 0)
-		return ret;
-
-	return be32_to_cpu(st->data[2].d32);
+	return st->read(st, addr);
 }
 
-static const char * const ad5686_powerdown_modes[] = {
-	"1kohm_to_gnd",
-	"100kohm_to_gnd",
-	"three_state"
-};
-
 static int ad5686_get_powerdown_mode(struct iio_dev *indio_dev,
 				     const struct iio_chan_spec *chan)
 {
@@ -196,8 +95,8 @@ static ssize_t ad5686_write_dac_powerdown(struct iio_dev *indio_dev,
 	else
 		st->pwr_down_mask &= ~(0x3 << (chan->channel * 2));
 
-	ret = ad5686_spi_write(st, AD5686_CMD_POWERDOWN_DAC, 0,
-			       st->pwr_down_mask & st->pwr_down_mode, 0);
+	ret = ad5686_write(st, AD5686_CMD_POWERDOWN_DAC, 0,
+			   st->pwr_down_mask & st->pwr_down_mode, 0);
 
 	return ret ? ret : len;
 }
@@ -214,7 +113,7 @@ static int ad5686_read_raw(struct iio_dev *indio_dev,
 	switch (m) {
 	case IIO_CHAN_INFO_RAW:
 		mutex_lock(&indio_dev->mlock);
-		ret = ad5686_spi_read(st, chan->address);
+		ret = ad5686_read(st, chan->address);
 		mutex_unlock(&indio_dev->mlock);
 		if (ret < 0)
 			return ret;
@@ -243,11 +142,11 @@ static int ad5686_write_raw(struct iio_dev *indio_dev,
 			return -EINVAL;
 
 		mutex_lock(&indio_dev->mlock);
-		ret = ad5686_spi_write(st,
-				 AD5686_CMD_WRITE_INPUT_N_UPDATE_N,
-				 chan->address,
-				 val,
-				 chan->scan_type.shift);
+		ret = ad5686_write(st,
+				   AD5686_CMD_WRITE_INPUT_N_UPDATE_N,
+				   chan->address,
+				   val,
+				   chan->scan_type.shift);
 		mutex_unlock(&indio_dev->mlock);
 		break;
 	default:
@@ -318,11 +217,21 @@ DECLARE_AD5686_CHANNELS(ad5685r_channels, 14, 2);
 DECLARE_AD5686_CHANNELS(ad5686_channels, 16, 0);
 
 static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
+	[ID_AD5671R] = {
+		.channels = ad5672_channels,
+		.int_vref_mv = 2500,
+		.num_channels = 8,
+	},
 	[ID_AD5672R] = {
 		.channels = ad5672_channels,
 		.int_vref_mv = 2500,
 		.num_channels = 8,
 	},
+	[ID_AD5675R] = {
+		.channels = ad5676_channels,
+		.int_vref_mv = 2500,
+		.num_channels = 8,
+	},
 	[ID_AD5676] = {
 		.channels = ad5676_channels,
 		.num_channels = 8,
@@ -355,22 +264,47 @@ static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
 		.int_vref_mv = 2500,
 		.num_channels = 4,
 	},
+	[ID_AD5694] = {
+		.channels = ad5684_channels,
+		.num_channels = 4,
+	},
+	[ID_AD5694R] = {
+		.channels = ad5684_channels,
+		.int_vref_mv = 2500,
+		.num_channels = 4,
+	},
+	[ID_AD5696] = {
+		.channels = ad5686_channels,
+		.num_channels = 4,
+	},
+	[ID_AD5696R] = {
+		.channels = ad5686_channels,
+		.int_vref_mv = 2500,
+		.num_channels = 4,
+	},
 };
 
-static int ad5686_probe(struct spi_device *spi)
+int ad5686_probe(struct device *dev,
+		 enum ad5686_supported_device_ids chip_type,
+		 const char *name, ad5686_write_func write,
+		 ad5686_read_func read)
 {
 	struct ad5686_state *st;
 	struct iio_dev *indio_dev;
 	int ret, voltage_uv = 0;
 
-	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
 	if (indio_dev == NULL)
 		return  -ENOMEM;
 
 	st = iio_priv(indio_dev);
-	spi_set_drvdata(spi, indio_dev);
+	dev_set_drvdata(dev, indio_dev);
+
+	st->dev = dev;
+	st->write = write;
+	st->read = read;
 
-	st->reg = devm_regulator_get_optional(&spi->dev, "vcc");
+	st->reg = devm_regulator_get_optional(dev, "vcc");
 	if (!IS_ERR(st->reg)) {
 		ret = regulator_enable(st->reg);
 		if (ret)
@@ -383,28 +317,25 @@ static int ad5686_probe(struct spi_device *spi)
 		voltage_uv = ret;
 	}
 
-	st->chip_info =
-		&ad5686_chip_info_tbl[spi_get_device_id(spi)->driver_data];
+	st->chip_info = &ad5686_chip_info_tbl[chip_type];
 
 	if (voltage_uv)
 		st->vref_mv = voltage_uv / 1000;
 	else
 		st->vref_mv = st->chip_info->int_vref_mv;
 
-	st->spi = spi;
-
 	/* Set all the power down mode for all channels to 1K pulldown */
 	st->pwr_down_mode = 0x55;
 
-	indio_dev->dev.parent = &spi->dev;
-	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->dev.parent = dev;
+	indio_dev->name = name;
 	indio_dev->info = &ad5686_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = st->chip_info->channels;
 	indio_dev->num_channels = st->chip_info->num_channels;
 
-	ret = ad5686_spi_write(st, AD5686_CMD_INTERNAL_REFER_SETUP, 0,
-				!!voltage_uv, 0);
+	ret = ad5686_write(st, AD5686_CMD_INTERNAL_REFER_SETUP,
+			   0, !!voltage_uv, 0);
 	if (ret)
 		goto error_disable_reg;
 
@@ -419,10 +350,11 @@ static int ad5686_probe(struct spi_device *spi)
 		regulator_disable(st->reg);
 	return ret;
 }
+EXPORT_SYMBOL_GPL(ad5686_probe);
 
-static int ad5686_remove(struct spi_device *spi)
+int ad5686_remove(struct device *dev)
 {
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5686_state *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
@@ -431,29 +363,7 @@ static int ad5686_remove(struct spi_device *spi)
 
 	return 0;
 }
-
-static const struct spi_device_id ad5686_id[] = {
-	{"ad5672r", ID_AD5672R},
-	{"ad5676", ID_AD5676},
-	{"ad5676r", ID_AD5676R},
-	{"ad5684", ID_AD5684},
-	{"ad5684r", ID_AD5684R},
-	{"ad5685r", ID_AD5685R},
-	{"ad5686", ID_AD5686},
-	{"ad5686r", ID_AD5686R},
-	{}
-};
-MODULE_DEVICE_TABLE(spi, ad5686_id);
-
-static struct spi_driver ad5686_driver = {
-	.driver = {
-		   .name = "ad5686",
-		   },
-	.probe = ad5686_probe,
-	.remove = ad5686_remove,
-	.id_table = ad5686_id,
-};
-module_spi_driver(ad5686_driver);
+EXPORT_SYMBOL_GPL(ad5686_remove);
 
 MODULE_AUTHOR("Michael Hennerich <hennerich@blackfin.uclinux.org>");
 MODULE_DESCRIPTION("Analog Devices AD5686/85/84 DAC");
diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
new file mode 100644
index 0000000..b2b0110
--- /dev/null
+++ b/drivers/iio/dac/ad5686.h
@@ -0,0 +1,114 @@
+#ifndef __DRIVERS_IIO_DAC_AD5686_H__
+#define __DRIVERS_IIO_DAC_AD5686_H__
+
+#include <linux/types.h>
+#include <linux/cache.h>
+#include <linux/mutex.h>
+#include <linux/kernel.h>
+
+#define AD5686_ADDR(x)				((x) << 16)
+#define AD5686_CMD(x)				((x) << 20)
+
+#define AD5686_ADDR_DAC(chan)			(0x1 << (chan))
+#define AD5686_ADDR_ALL_DAC			0xF
+
+#define AD5686_CMD_NOOP				0x0
+#define AD5686_CMD_WRITE_INPUT_N		0x1
+#define AD5686_CMD_UPDATE_DAC_N			0x2
+#define AD5686_CMD_WRITE_INPUT_N_UPDATE_N	0x3
+#define AD5686_CMD_POWERDOWN_DAC		0x4
+#define AD5686_CMD_LDAC_MASK			0x5
+#define AD5686_CMD_RESET			0x6
+#define AD5686_CMD_INTERNAL_REFER_SETUP		0x7
+#define AD5686_CMD_DAISY_CHAIN_ENABLE		0x8
+#define AD5686_CMD_READBACK_ENABLE		0x9
+
+#define AD5686_LDAC_PWRDN_NONE			0x0
+#define AD5686_LDAC_PWRDN_1K			0x1
+#define AD5686_LDAC_PWRDN_100K			0x2
+#define AD5686_LDAC_PWRDN_3STATE		0x3
+
+/**
+ * ad5686_supported_device_ids:
+ */
+enum ad5686_supported_device_ids {
+	ID_AD5671R,
+	ID_AD5672R,
+	ID_AD5675R,
+	ID_AD5676,
+	ID_AD5676R,
+	ID_AD5684,
+	ID_AD5684R,
+	ID_AD5685R,
+	ID_AD5686,
+	ID_AD5686R,
+	ID_AD5694,
+	ID_AD5694R,
+	ID_AD5695R,
+	ID_AD5696,
+	ID_AD5696R,
+};
+
+struct ad5686_state;
+
+typedef int (*ad5686_write_func)(struct ad5686_state *st,
+				 u8 cmd, u8 addr, u16 val);
+
+typedef int (*ad5686_read_func)(struct ad5686_state *st, u8 addr);
+
+/**
+ * struct ad5686_chip_info - chip specific information
+ * @int_vref_mv:	AD5620/40/60: the internal reference voltage
+ * @num_channels:	number of channels
+ * @channel:		channel specification
+ */
+
+struct ad5686_chip_info {
+	u16				int_vref_mv;
+	unsigned int			num_channels;
+	struct iio_chan_spec		*channels;
+};
+
+/**
+ * struct ad5446_state - driver instance specific data
+ * @spi:		spi_device
+ * @chip_info:		chip model specific constants, available modes etc
+ * @reg:		supply regulator
+ * @vref_mv:		actual reference voltage used
+ * @pwr_down_mask:	power down mask
+ * @pwr_down_mode:	current power down mode
+ * @data:		spi transfer buffers
+ */
+
+struct ad5686_state {
+	struct device			*dev;
+	const struct ad5686_chip_info	*chip_info;
+	struct regulator		*reg;
+	unsigned short			vref_mv;
+	unsigned int			pwr_down_mask;
+	unsigned int			pwr_down_mode;
+	ad5686_write_func		write;
+	ad5686_read_func		read;
+
+	/*
+	 * DMA (thus cache coherency maintenance) requires the
+	 * transfer buffers to live in their own cache lines.
+	 */
+
+	union {
+		__be32 d32;
+		__be16 d16;
+		u8 d8[4];
+	} data[3] ____cacheline_aligned;
+};
+
+
+int ad5686_probe(struct device *dev,
+		 enum ad5686_supported_device_ids chip_type,
+		 const char *name, ad5686_write_func write,
+		 ad5686_read_func read);
+
+int ad5686_remove(struct device *dev);
+
+
+#endif /* __DRIVERS_IIO_DAC_AD5686_H__ */
diff --git a/drivers/iio/dac/ad5696-i2c.c b/drivers/iio/dac/ad5696-i2c.c
new file mode 100644
index 0000000..c9187de
--- /dev/null
+++ b/drivers/iio/dac/ad5696-i2c.c
@@ -0,0 +1,98 @@
+/*
+ * AD5671R, AD5675R, AD5694, AD5694R, AD5695R, AD5696, AD5696R
+ * Digital to analog converters driver
+ *
+ * Copyright 2018 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2.
+ */
+
+#include "ad5686.h"
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+
+static int ad5686_i2c_read(struct ad5686_state *st, u8 addr)
+{
+	struct i2c_client *i2c = to_i2c_client(st->dev);
+	struct i2c_msg msg[2] = {
+		{
+			.addr = i2c->addr,
+			.flags = i2c->flags,
+			.len = 3,
+			.buf = &st->data[0].d8[1],
+		},
+		{
+			.addr = i2c->addr,
+			.flags = i2c->flags | I2C_M_RD,
+			.len = 2,
+			.buf = (char *)&st->data[0].d16,
+		},
+	};
+	int ret;
+
+	st->data[0].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_NOOP) |
+				      AD5686_ADDR(addr) |
+				      0x00);
+
+	ret = i2c_transfer(i2c->adapter, msg, 2);
+	if (ret < 0)
+		return ret;
+
+	return be16_to_cpu(st->data[0].d16);
+}
+
+static int ad5686_i2c_write(struct ad5686_state *st,
+			    u8 cmd, u8 addr, u16 val)
+{
+	struct i2c_client *i2c = to_i2c_client(st->dev);
+	int ret;
+
+	st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) | AD5686_ADDR(addr)
+				      | val);
+
+	ret = i2c_master_send(i2c, &st->data[0].d8[1], 3);
+	if (ret < 0)
+		return ret;
+
+	return (ret != 3) ? -EIO : 0;
+}
+
+static int ad5686_i2c_probe(struct i2c_client *i2c,
+			    const struct i2c_device_id *id)
+{
+	return ad5686_probe(&i2c->dev, id->driver_data, id->name,
+			    ad5686_i2c_write, ad5686_i2c_read);
+}
+
+static int ad5686_i2c_remove(struct i2c_client *i2c)
+{
+	return ad5686_remove(&i2c->dev);
+}
+
+static const struct i2c_device_id ad5686_i2c_id[] = {
+	{"ad5671r", ID_AD5671R},
+	{"ad5675r", ID_AD5675R},
+	{"ad5694", ID_AD5694},
+	{"ad5694r", ID_AD5694R},
+	{"ad5695r", ID_AD5695R},
+	{"ad5696", ID_AD5696},
+	{"ad5696r", ID_AD5696R},
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, i2c_device_id);
+
+static struct i2c_driver ad5686_i2c_driver = {
+	.driver = {
+		.name = "ad5696",
+	},
+	.probe = ad5686_i2c_probe,
+	.remove = ad5686_i2c_remove,
+	.id_table = ad5686_i2c_id,
+};
+
+module_i2c_driver(ad5686_i2c_driver);
+
+MODULE_AUTHOR("Stefan Popa <stefan.popa@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD5686 and similar multi-channel DACs");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

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

* Re: [PATCH 3/3] iio:dac:ad5686: Add AD5671R/75R/94/94R/95R/96/96R support
  2018-03-30 13:56 [PATCH 3/3] iio:dac:ad5686: Add AD5671R/75R/94/94R/95R/96/96R support Stefan Popa
@ 2018-04-06 15:32 ` Jonathan Cameron
  2018-04-10 15:58 ` [PATCH v2 5/6] iio:dac:ad5686: Refactor the driver Stefan Popa
  2018-04-10 15:58 ` [PATCH v2 6/6] iio:dac:ad5686: Add AD5671R/75R/94/94R/95R/96/96R support Stefan Popa
  2 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2018-04-06 15:32 UTC (permalink / raw)
  To: Stefan Popa
  Cc: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, davem, gregkh,
	mchehab, linus.walleij, rdunlap, Ismail.Kose, lukas,
	mike.looijmans, fabrice.gasnier, linux-pm, linux-iio,
	linux-kernel

On Fri, 30 Mar 2018 16:56:49 +0300
Stefan Popa <stefan.popa@analog.com> wrote:

> The AD5694/AD5694R/AD5695R/AD5696/AD5696R are a family of 4 channel DAC
> s with 12-bit, 14-bit and 16-bit precision respectively. The devices have
> either no built-in reference, or built-in 2.5V reference.
> 
> The AD5671R/AD5675R are similar, except that they have 8 instead of 4
> channels.
> 
> These devices are similar to AD5672R/AD5676/AD5676R and
> AD5684/AD5684R/AD5684/AD5685R/AD5686/AD5686R, except that they use i2c
> instead of spi. In order to add support in the previously existing driver,
> three modules were created. One baseline module that has all the
> chip-logic and then one module for SPI and one for I2C.
> 
> Datasheets:
> http://www.analog.com/media/en/technical-documentation/data-sheets/AD5671R_5675R.pdf
> http://www.analog.com/media/en/technical-documentation/data-sheets/AD5696R_5695R_5694R.pdf
> 
> Signed-off-by: Stefan Popa <stefan.popa@analog.com>
Should be multiple patches.  First refactor then follow up with the new stuff.

Comments inline.

> ---
>  MAINTAINERS                  |   8 ++
>  drivers/iio/dac/Kconfig      |  27 ++++--
>  drivers/iio/dac/ad5686-spi.c |  93 ++++++++++++++++++
>  drivers/iio/dac/ad5686.c     | 222 +++++++++++++------------------------------
>  drivers/iio/dac/ad5686.h     | 114 ++++++++++++++++++++++
>  drivers/iio/dac/ad5696-i2c.c |  98 +++++++++++++++++++
>  6 files changed, 400 insertions(+), 162 deletions(-)
>  create mode 100644 drivers/iio/dac/ad5686-spi.c
>  create mode 100644 drivers/iio/dac/ad5686.h
>  create mode 100644 drivers/iio/dac/ad5696-i2c.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 473ac00..002cb01 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -791,6 +791,14 @@ M:	Michael Hanselmann <linux-kernel@hansmi.ch>
>  S:	Supported
>  F:	drivers/macintosh/ams/
>  
> +ANALOG DEVICES INC AD5686 DRIVER
> +M:	Stefan Popa <stefan.popa@analog.com>
> +L:	linux-pm@vger.kernel.org
> +W:	http://ez.analog.com/community/linux-device-drivers
> +S:	Supported
> +F:	drivers/iio/dac/ad5686*
> +F:	drivers/iio/dac/ad5696*
> +
>  ANALOG DEVICES INC AD9389B DRIVER
>  M:	Hans Verkuil <hans.verkuil@cisco.com>
>  L:	linux-media@vger.kernel.org
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 965d5c0..dab0b8a 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -131,15 +131,30 @@ config LTC2632
>  	  module will be called ltc2632.
>  
>  config AD5686
> -	tristate "Analog Devices AD5686R/AD5685R/AD5684R DAC SPI driver"
> +	tristate
> +
> +config AD5686_SPI
> +	tristate "Analog Devices AD5686 and similar multi-channel DACs (SPI)"
>  	depends on SPI
> +	select AD5686
>  	help
> -	  Say yes here to build support for Analog Devices AD5686R, AD5685R,
> -	  AD5684R, AD5791 Voltage Output Digital to
> -	  Analog Converter.
> +	Say yes here to build support for Analog Devices AD5672R, AD5676,
> +	AD5676R, AD5684, AD5684R, AD5684R, AD5685R, AD5686, AD5686R.
> +	Voltage Output Digital to Analog Converter.
>  
> -	  To compile this driver as a module, choose M here: the
> -	  module will be called ad5686.
> +	To compile this driver as a module, choose M here: the
> +	module will be called ad5686.
> +
> +config AD5696_I2C
> +	tristate "Analog Devices AD5696 and similar multi-channel DACs (I2C)"
> +	depends on I2C
> +	select AD5686
> +	help
> +	Say yes here to build support for Analog Devices AD5671R, AD5675R,
> +	AD5694, AD5694R, AD5695R, AD5696, AD5696R Voltage Output Digital to
> +	Analog Converter.
> +	To compile this driver as a module, choose M here: the module will be
> +	called ad5696.
>  
>  config AD5755
>  	tristate "Analog Devices AD5755/AD5755-1/AD5757/AD5735/AD5737 DAC driver"
> diff --git a/drivers/iio/dac/ad5686-spi.c b/drivers/iio/dac/ad5686-spi.c
> new file mode 100644
> index 0000000..d68579b
> --- /dev/null
> +++ b/drivers/iio/dac/ad5686-spi.c
> @@ -0,0 +1,93 @@
> +/*
> + * AD5672R, AD5676, AD5676R, AD5684, AD5684R, AD5684R, AD5685R, AD5686, AD5686R
> + * Digital to analog converters driver
> + *
> + * Copyright 2018 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#include "ad5686.h"
> +
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +
> +static int ad5686_spi_write(struct ad5686_state *st,
> +			    u8 cmd, u8 addr, u16 val)
> +{
> +	struct spi_device *spi = to_spi_device(st->dev);
> +
> +	st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) |
> +				      AD5686_ADDR(addr) |
> +				      val);
> +
> +	return spi_write(spi, &st->data[0].d8[1], 3);
> +}
> +
> +static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
> +{
> +	struct spi_transfer t[] = {
> +		{
> +			.tx_buf = &st->data[0].d8[1],
> +			.len = 3,
> +			.cs_change = 1,
> +		}, {
> +			.tx_buf = &st->data[1].d8[1],
> +			.rx_buf = &st->data[2].d8[1],
> +			.len = 3,
> +		},
> +	};
> +	struct spi_device *spi = to_spi_device(st->dev);
> +	int ret;
> +
> +	st->data[0].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_READBACK_ENABLE) |
> +				      AD5686_ADDR(addr));
> +	st->data[1].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_NOOP));
> +
> +	ret = spi_sync_transfer(spi, t, ARRAY_SIZE(t));
> +	if (ret < 0)
> +		return ret;
> +
> +	return be32_to_cpu(st->data[2].d32);
> +}
> +
> +static int ad5686_spi_probe(struct spi_device *spi)
> +{
> +	const struct spi_device_id *id = spi_get_device_id(spi);
> +
> +	return ad5686_probe(&spi->dev, id->driver_data, id->name,
> +			    ad5686_spi_write, ad5686_spi_read);
> +}
> +
> +static int ad5686_spi_remove(struct spi_device *spi)
> +{
> +	return ad5686_remove(&spi->dev);
> +}
> +
> +static const struct spi_device_id ad5686_spi_id[] = {
> +	{"ad5672r", ID_AD5672R},
> +	{"ad5676", ID_AD5676},
> +	{"ad5676r", ID_AD5676R},
> +	{"ad5684", ID_AD5684},
> +	{"ad5684r", ID_AD5684R},
> +	{"ad5685r", ID_AD5685R},
> +	{"ad5686", ID_AD5686},
> +	{"ad5686r", ID_AD5686R},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(spi, ad5686_spi_id);
> +
> +static struct spi_driver ad5686_spi_driver = {
> +	.driver = {
> +		.name = "ad5686",
> +	},
> +	.probe = ad5686_spi_probe,
> +	.remove = ad5686_spi_remove,
> +	.id_table = ad5686_spi_id,
> +};
> +
> +module_spi_driver(ad5686_spi_driver);
> +
> +MODULE_AUTHOR("Stefan Popa <stefan.popa@analog.com>");
> +MODULE_DESCRIPTION("Analog Devices AD5686 and similar multi-channel DACs");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
> index 5fb0179..e4ea0dc 100644
> --- a/drivers/iio/dac/ad5686.c
> +++ b/drivers/iio/dac/ad5686.c
> @@ -11,7 +11,6 @@
>  #include <linux/device.h>
>  #include <linux/module.h>
>  #include <linux/kernel.h>
> -#include <linux/spi/spi.h>
>  #include <linux/slab.h>
>  #include <linux/sysfs.h>
>  #include <linux/regulator/consumer.h>
> @@ -19,128 +18,28 @@
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  
> -#define AD5686_ADDR(x)				((x) << 16)
> -#define AD5686_CMD(x)				((x) << 20)
> -
> -#define AD5686_ADDR_DAC(chan)			(0x1 << (chan))
> -#define AD5686_ADDR_ALL_DAC			0xF
> -
> -#define AD5686_CMD_NOOP				0x0
> -#define AD5686_CMD_WRITE_INPUT_N		0x1
> -#define AD5686_CMD_UPDATE_DAC_N			0x2
> -#define AD5686_CMD_WRITE_INPUT_N_UPDATE_N	0x3
> -#define AD5686_CMD_POWERDOWN_DAC		0x4
> -#define AD5686_CMD_LDAC_MASK			0x5
> -#define AD5686_CMD_RESET			0x6
> -#define AD5686_CMD_INTERNAL_REFER_SETUP		0x7
> -#define AD5686_CMD_DAISY_CHAIN_ENABLE		0x8
> -#define AD5686_CMD_READBACK_ENABLE		0x9
> -
> -#define AD5686_LDAC_PWRDN_NONE			0x0
> -#define AD5686_LDAC_PWRDN_1K			0x1
> -#define AD5686_LDAC_PWRDN_100K			0x2
> -#define AD5686_LDAC_PWRDN_3STATE		0x3
> -
> -/**
> - * struct ad5686_chip_info - chip specific information
> - * @int_vref_mv:	AD5620/40/60: the internal reference voltage
> - * @num_channels:	number of channels
> - * @channel:		channel specification
> -*/
> -
> -struct ad5686_chip_info {
> -	u16				int_vref_mv;
> -	unsigned int			num_channels;
> -	struct iio_chan_spec		*channels;
> -};
> -
> -/**
> - * struct ad5446_state - driver instance specific data
> - * @spi:		spi_device
> - * @chip_info:		chip model specific constants, available modes etc
> - * @reg:		supply regulator
> - * @vref_mv:		actual reference voltage used
> - * @pwr_down_mask:	power down mask
> - * @pwr_down_mode:	current power down mode
> - * @data:		spi transfer buffers
> - */
> +#include "ad5686.h"
>  
> -struct ad5686_state {
> -	struct spi_device		*spi;
> -	const struct ad5686_chip_info	*chip_info;
> -	struct regulator		*reg;
> -	unsigned short			vref_mv;
> -	unsigned			pwr_down_mask;
> -	unsigned			pwr_down_mode;
> -	/*
> -	 * DMA (thus cache coherency maintenance) requires the
> -	 * transfer buffers to live in their own cache lines.
> -	 */
> -
> -	union {
> -		__be32 d32;
> -		u8 d8[4];
> -	} data[3] ____cacheline_aligned;
> +static const char * const ad5686_powerdown_modes[] = {
> +	"1kohm_to_gnd",
> +	"100kohm_to_gnd",
> +	"three_state"
>  };
>  
> -/**
> - * ad5686_supported_device_ids:
> - */
> -
> -enum ad5686_supported_device_ids {
> -	ID_AD5672R,
> -	ID_AD5676,
> -	ID_AD5676R,
> -	ID_AD5684,
> -	ID_AD5684R,
> -	ID_AD5685R,
> -	ID_AD5686,
> -	ID_AD5686R
> -};
> -static int ad5686_spi_write(struct ad5686_state *st,
> -			     u8 cmd, u8 addr, u16 val, u8 shift)
> +static int ad5686_write(struct ad5686_state *st,
> +			u8 cmd, u8 addr, u16 val, u8 shift)
>  {
>  	val <<= shift;

Given the shift is only applied in one location in the code (and that
one is pretty obvious), I would push the shift inline then
drop the wrapper around st->write in favour of calling
st->write directly.

>  
> -	st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) |
> -			      AD5686_ADDR(addr) |
> -			      val);
> -
> -	return spi_write(st->spi, &st->data[0].d8[1], 3);
> +	return st->write(st, cmd, addr, val);
>  }
>  
> -static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
> +static int ad5686_read(struct ad5686_state *st, u8 addr)
>  {
> -	struct spi_transfer t[] = {
> -		{
> -			.tx_buf = &st->data[0].d8[1],
> -			.len = 3,
> -			.cs_change = 1,
> -		}, {
> -			.tx_buf = &st->data[1].d8[1],
> -			.rx_buf = &st->data[2].d8[1],
> -			.len = 3,
> -		},
> -	};
> -	int ret;
>  
> -	st->data[0].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_READBACK_ENABLE) |
> -			      AD5686_ADDR(addr));
> -	st->data[1].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_NOOP));
> -
> -	ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
> -	if (ret < 0)
> -		return ret;
> -
> -	return be32_to_cpu(st->data[2].d32);
This wrapper adds very little, just call st->read directly.

> +	return st->read(st, addr);
>  }
>  
> -static const char * const ad5686_powerdown_modes[] = {
> -	"1kohm_to_gnd",
> -	"100kohm_to_gnd",
> -	"three_state"
> -};
> -
>  static int ad5686_get_powerdown_mode(struct iio_dev *indio_dev,
>  				     const struct iio_chan_spec *chan)
>  {
> @@ -196,8 +95,8 @@ static ssize_t ad5686_write_dac_powerdown(struct iio_dev *indio_dev,
>  	else
>  		st->pwr_down_mask &= ~(0x3 << (chan->channel * 2));
>  
> -	ret = ad5686_spi_write(st, AD5686_CMD_POWERDOWN_DAC, 0,
> -			       st->pwr_down_mask & st->pwr_down_mode, 0);
> +	ret = ad5686_write(st, AD5686_CMD_POWERDOWN_DAC, 0,
> +			   st->pwr_down_mask & st->pwr_down_mode, 0);
>  
>  	return ret ? ret : len;
>  }
> @@ -214,7 +113,7 @@ static int ad5686_read_raw(struct iio_dev *indio_dev,
>  	switch (m) {
>  	case IIO_CHAN_INFO_RAW:
>  		mutex_lock(&indio_dev->mlock);
> -		ret = ad5686_spi_read(st, chan->address);
> +		ret = ad5686_read(st, chan->address);
>  		mutex_unlock(&indio_dev->mlock);
>  		if (ret < 0)
>  			return ret;
> @@ -243,11 +142,11 @@ static int ad5686_write_raw(struct iio_dev *indio_dev,
>  			return -EINVAL;
>  
>  		mutex_lock(&indio_dev->mlock);
> -		ret = ad5686_spi_write(st,
> -				 AD5686_CMD_WRITE_INPUT_N_UPDATE_N,
> -				 chan->address,
> -				 val,
> -				 chan->scan_type.shift);
> +		ret = ad5686_write(st,
> +				   AD5686_CMD_WRITE_INPUT_N_UPDATE_N,
> +				   chan->address,
> +				   val,
> +				   chan->scan_type.shift);
>  		mutex_unlock(&indio_dev->mlock);
>  		break;
>  	default:
> @@ -318,11 +217,21 @@ DECLARE_AD5686_CHANNELS(ad5685r_channels, 14, 2);
>  DECLARE_AD5686_CHANNELS(ad5686_channels, 16, 0);
>  
>  static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
> +	[ID_AD5671R] = {
> +		.channels = ad5672_channels,
> +		.int_vref_mv = 2500,
> +		.num_channels = 8,
> +	},
>  	[ID_AD5672R] = {
>  		.channels = ad5672_channels,
>  		.int_vref_mv = 2500,
>  		.num_channels = 8,
>  	},
> +	[ID_AD5675R] = {
> +		.channels = ad5676_channels,
> +		.int_vref_mv = 2500,
> +		.num_channels = 8,
> +	},
>  	[ID_AD5676] = {
>  		.channels = ad5676_channels,
>  		.num_channels = 8,
> @@ -355,22 +264,47 @@ static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
>  		.int_vref_mv = 2500,
>  		.num_channels = 4,
>  	},
> +	[ID_AD5694] = {
> +		.channels = ad5684_channels,
> +		.num_channels = 4,
> +	},
> +	[ID_AD5694R] = {
> +		.channels = ad5684_channels,
> +		.int_vref_mv = 2500,
> +		.num_channels = 4,
> +	},
> +	[ID_AD5696] = {
> +		.channels = ad5686_channels,
> +		.num_channels = 4,
> +	},
> +	[ID_AD5696R] = {
> +		.channels = ad5686_channels,
> +		.int_vref_mv = 2500,
> +		.num_channels = 4,
> +	},
>  };
>  
> -static int ad5686_probe(struct spi_device *spi)
> +int ad5686_probe(struct device *dev,
> +		 enum ad5686_supported_device_ids chip_type,
> +		 const char *name, ad5686_write_func write,
> +		 ad5686_read_func read)
>  {
>  	struct ad5686_state *st;
>  	struct iio_dev *indio_dev;
>  	int ret, voltage_uv = 0;
>  
> -	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
>  	if (indio_dev == NULL)
>  		return  -ENOMEM;
>  
>  	st = iio_priv(indio_dev);
> -	spi_set_drvdata(spi, indio_dev);
> +	dev_set_drvdata(dev, indio_dev);
> +
> +	st->dev = dev;
> +	st->write = write;
> +	st->read = read;
>  
> -	st->reg = devm_regulator_get_optional(&spi->dev, "vcc");
> +	st->reg = devm_regulator_get_optional(dev, "vcc");
>  	if (!IS_ERR(st->reg)) {
>  		ret = regulator_enable(st->reg);
>  		if (ret)
> @@ -383,28 +317,25 @@ static int ad5686_probe(struct spi_device *spi)
>  		voltage_uv = ret;
>  	}
>  
> -	st->chip_info =
> -		&ad5686_chip_info_tbl[spi_get_device_id(spi)->driver_data];
> +	st->chip_info = &ad5686_chip_info_tbl[chip_type];
>  
>  	if (voltage_uv)
>  		st->vref_mv = voltage_uv / 1000;
>  	else
>  		st->vref_mv = st->chip_info->int_vref_mv;
>  
> -	st->spi = spi;
> -
>  	/* Set all the power down mode for all channels to 1K pulldown */
>  	st->pwr_down_mode = 0x55;
>  
> -	indio_dev->dev.parent = &spi->dev;
> -	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->dev.parent = dev;
> +	indio_dev->name = name;
>  	indio_dev->info = &ad5686_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	indio_dev->channels = st->chip_info->channels;
>  	indio_dev->num_channels = st->chip_info->num_channels;
>  
> -	ret = ad5686_spi_write(st, AD5686_CMD_INTERNAL_REFER_SETUP, 0,
> -				!!voltage_uv, 0);
> +	ret = ad5686_write(st, AD5686_CMD_INTERNAL_REFER_SETUP,
> +			   0, !!voltage_uv, 0);
>  	if (ret)
>  		goto error_disable_reg;
>  
> @@ -419,10 +350,11 @@ static int ad5686_probe(struct spi_device *spi)
>  		regulator_disable(st->reg);
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL(ad5686_probe);
>  
> -static int ad5686_remove(struct spi_device *spi)
> +int ad5686_remove(struct device *dev)
>  {
> -	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5686_state *st = iio_priv(indio_dev);
>  
>  	iio_device_unregister(indio_dev);
> @@ -431,29 +363,7 @@ static int ad5686_remove(struct spi_device *spi)
>  
>  	return 0;
>  }
> -
> -static const struct spi_device_id ad5686_id[] = {
> -	{"ad5672r", ID_AD5672R},
> -	{"ad5676", ID_AD5676},
> -	{"ad5676r", ID_AD5676R},
> -	{"ad5684", ID_AD5684},
> -	{"ad5684r", ID_AD5684R},
> -	{"ad5685r", ID_AD5685R},
> -	{"ad5686", ID_AD5686},
> -	{"ad5686r", ID_AD5686R},
> -	{}
> -};
> -MODULE_DEVICE_TABLE(spi, ad5686_id);
> -
> -static struct spi_driver ad5686_driver = {
> -	.driver = {
> -		   .name = "ad5686",
> -		   },
> -	.probe = ad5686_probe,
> -	.remove = ad5686_remove,
> -	.id_table = ad5686_id,
> -};
> -module_spi_driver(ad5686_driver);
> +EXPORT_SYMBOL_GPL(ad5686_remove);
>  
>  MODULE_AUTHOR("Michael Hennerich <hennerich@blackfin.uclinux.org>");
>  MODULE_DESCRIPTION("Analog Devices AD5686/85/84 DAC");
> diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
> new file mode 100644
> index 0000000..b2b0110
> --- /dev/null
> +++ b/drivers/iio/dac/ad5686.h
> @@ -0,0 +1,114 @@
> +#ifndef __DRIVERS_IIO_DAC_AD5686_H__
> +#define __DRIVERS_IIO_DAC_AD5686_H__
> +
> +#include <linux/types.h>
> +#include <linux/cache.h>
> +#include <linux/mutex.h>
> +#include <linux/kernel.h>
> +
> +#define AD5686_ADDR(x)				((x) << 16)
> +#define AD5686_CMD(x)				((x) << 20)
> +
> +#define AD5686_ADDR_DAC(chan)			(0x1 << (chan))
> +#define AD5686_ADDR_ALL_DAC			0xF
> +
> +#define AD5686_CMD_NOOP				0x0
> +#define AD5686_CMD_WRITE_INPUT_N		0x1
> +#define AD5686_CMD_UPDATE_DAC_N			0x2
> +#define AD5686_CMD_WRITE_INPUT_N_UPDATE_N	0x3
> +#define AD5686_CMD_POWERDOWN_DAC		0x4
> +#define AD5686_CMD_LDAC_MASK			0x5
> +#define AD5686_CMD_RESET			0x6
> +#define AD5686_CMD_INTERNAL_REFER_SETUP		0x7
> +#define AD5686_CMD_DAISY_CHAIN_ENABLE		0x8
> +#define AD5686_CMD_READBACK_ENABLE		0x9
> +
> +#define AD5686_LDAC_PWRDN_NONE			0x0
> +#define AD5686_LDAC_PWRDN_1K			0x1
> +#define AD5686_LDAC_PWRDN_100K			0x2
> +#define AD5686_LDAC_PWRDN_3STATE		0x3
> +
> +/**
> + * ad5686_supported_device_ids:
> + */
> +enum ad5686_supported_device_ids {
> +	ID_AD5671R,
> +	ID_AD5672R,
> +	ID_AD5675R,
> +	ID_AD5676,
> +	ID_AD5676R,
> +	ID_AD5684,
> +	ID_AD5684R,
> +	ID_AD5685R,
> +	ID_AD5686,
> +	ID_AD5686R,
> +	ID_AD5694,
> +	ID_AD5694R,
> +	ID_AD5695R,
> +	ID_AD5696,
> +	ID_AD5696R,
> +};
> +
> +struct ad5686_state;
> +
> +typedef int (*ad5686_write_func)(struct ad5686_state *st,
> +				 u8 cmd, u8 addr, u16 val);
> +
> +typedef int (*ad5686_read_func)(struct ad5686_state *st, u8 addr);
> +
> +/**
> + * struct ad5686_chip_info - chip specific information
> + * @int_vref_mv:	AD5620/40/60: the internal reference voltage
> + * @num_channels:	number of channels
> + * @channel:		channel specification
> + */
> +
> +struct ad5686_chip_info {
> +	u16				int_vref_mv;
> +	unsigned int			num_channels;
> +	struct iio_chan_spec		*channels;
> +};
> +
> +/**
> + * struct ad5446_state - driver instance specific data
> + * @spi:		spi_device
> + * @chip_info:		chip model specific constants, available modes etc
> + * @reg:		supply regulator
> + * @vref_mv:		actual reference voltage used
> + * @pwr_down_mask:	power down mask
> + * @pwr_down_mode:	current power down mode
> + * @data:		spi transfer buffers
> + */
> +
> +struct ad5686_state {
> +	struct device			*dev;
> +	const struct ad5686_chip_info	*chip_info;
> +	struct regulator		*reg;
> +	unsigned short			vref_mv;
> +	unsigned int			pwr_down_mask;
> +	unsigned int			pwr_down_mode;
> +	ad5686_write_func		write;
> +	ad5686_read_func		read;
> +
> +	/*
> +	 * DMA (thus cache coherency maintenance) requires the
> +	 * transfer buffers to live in their own cache lines.
> +	 */
> +
> +	union {
> +		__be32 d32;
> +		__be16 d16;
> +		u8 d8[4];
> +	} data[3] ____cacheline_aligned;
> +};
> +
> +
> +int ad5686_probe(struct device *dev,
> +		 enum ad5686_supported_device_ids chip_type,
> +		 const char *name, ad5686_write_func write,
> +		 ad5686_read_func read);
> +
> +int ad5686_remove(struct device *dev);
> +
> +
> +#endif /* __DRIVERS_IIO_DAC_AD5686_H__ */
> diff --git a/drivers/iio/dac/ad5696-i2c.c b/drivers/iio/dac/ad5696-i2c.c
> new file mode 100644
> index 0000000..c9187de
> --- /dev/null
> +++ b/drivers/iio/dac/ad5696-i2c.c
> @@ -0,0 +1,98 @@
> +/*
> + * AD5671R, AD5675R, AD5694, AD5694R, AD5695R, AD5696, AD5696R
> + * Digital to analog converters driver
> + *
> + * Copyright 2018 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#include "ad5686.h"
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +
> +static int ad5686_i2c_read(struct ad5686_state *st, u8 addr)
> +{
> +	struct i2c_client *i2c = to_i2c_client(st->dev);
> +	struct i2c_msg msg[2] = {
> +		{
> +			.addr = i2c->addr,
> +			.flags = i2c->flags,
> +			.len = 3,
> +			.buf = &st->data[0].d8[1],
> +		},
> +		{
> +			.addr = i2c->addr,
> +			.flags = i2c->flags | I2C_M_RD,
> +			.len = 2,
> +			.buf = (char *)&st->data[0].d16,
> +		},
> +	};
> +	int ret;
> +
> +	st->data[0].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_NOOP) |
> +				      AD5686_ADDR(addr) |
> +				      0x00);
> +
> +	ret = i2c_transfer(i2c->adapter, msg, 2);
> +	if (ret < 0)
> +		return ret;
> +
> +	return be16_to_cpu(st->data[0].d16);
> +}
> +
> +static int ad5686_i2c_write(struct ad5686_state *st,
> +			    u8 cmd, u8 addr, u16 val)
> +{
> +	struct i2c_client *i2c = to_i2c_client(st->dev);
> +	int ret;
> +
> +	st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) | AD5686_ADDR(addr)
> +				      | val);
> +
> +	ret = i2c_master_send(i2c, &st->data[0].d8[1], 3);
> +	if (ret < 0)
> +		return ret;
> +
> +	return (ret != 3) ? -EIO : 0;
> +}
> +
> +static int ad5686_i2c_probe(struct i2c_client *i2c,
> +			    const struct i2c_device_id *id)
> +{
> +	return ad5686_probe(&i2c->dev, id->driver_data, id->name,
> +			    ad5686_i2c_write, ad5686_i2c_read);
> +}
> +
> +static int ad5686_i2c_remove(struct i2c_client *i2c)
> +{
> +	return ad5686_remove(&i2c->dev);
> +}
> +
> +static const struct i2c_device_id ad5686_i2c_id[] = {
> +	{"ad5671r", ID_AD5671R},
> +	{"ad5675r", ID_AD5675R},
> +	{"ad5694", ID_AD5694},
> +	{"ad5694r", ID_AD5694R},
> +	{"ad5695r", ID_AD5695R},
> +	{"ad5696", ID_AD5696},
> +	{"ad5696r", ID_AD5696R},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, i2c_device_id);
> +
> +static struct i2c_driver ad5686_i2c_driver = {
> +	.driver = {
> +		.name = "ad5696",
> +	},
> +	.probe = ad5686_i2c_probe,
> +	.remove = ad5686_i2c_remove,
> +	.id_table = ad5686_i2c_id,
> +};
> +
> +module_i2c_driver(ad5686_i2c_driver);
> +
> +MODULE_AUTHOR("Stefan Popa <stefan.popa@analog.com>");
> +MODULE_DESCRIPTION("Analog Devices AD5686 and similar multi-channel DACs");
> +MODULE_LICENSE("GPL v2");

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

* [PATCH v2 5/6] iio:dac:ad5686: Refactor the driver
  2018-03-30 13:56 [PATCH 3/3] iio:dac:ad5686: Add AD5671R/75R/94/94R/95R/96/96R support Stefan Popa
  2018-04-06 15:32 ` Jonathan Cameron
@ 2018-04-10 15:58 ` Stefan Popa
  2018-04-10 15:58 ` [PATCH v2 6/6] iio:dac:ad5686: Add AD5671R/75R/94/94R/95R/96/96R support Stefan Popa
  2 siblings, 0 replies; 11+ messages in thread
From: Stefan Popa @ 2018-04-10 15:58 UTC (permalink / raw)
  To: jic23, Michael.Hennerich, lars
  Cc: knaack.h, pmeerw, davem, mchehab, gregkh, linus.walleij, akpm,
	rdunlap, Ismail.Kose, lukas, fabrice.gasnier, mike.looijmans,
	pombredanne, linux-iio, linux-kernel, stefan.popa

In this patch restructures the existing ad5686 driver by adding a module
for SPI and a header file, while the baseline module deals with the
chip-logic.

This is a necessary step, as this driver should support in the future
similar devices which differ only in the type of interface used (I2C
instead of SPI).

Signed-off-by: Stefan Popa <stefan.popa@analog.com>
---
Changes in v2:
	- Refactored the patch
	- Use st->write directly instead of the ad5686_write() wrapper
	- Use st->read directly instead of the ad5686_read() wrapper

 MAINTAINERS                  |   7 ++
 drivers/iio/dac/Kconfig      |  17 ++--
 drivers/iio/dac/Makefile     |   1 +
 drivers/iio/dac/ad5686-spi.c |  93 +++++++++++++++++++++
 drivers/iio/dac/ad5686.c     | 190 +++++++------------------------------------
 drivers/iio/dac/ad5686.h     | 107 ++++++++++++++++++++++++
 6 files changed, 248 insertions(+), 167 deletions(-)
 create mode 100644 drivers/iio/dac/ad5686-spi.c
 create mode 100644 drivers/iio/dac/ad5686.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 473ac00..637e62d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -791,6 +791,13 @@ M:	Michael Hanselmann <linux-kernel@hansmi.ch>
 S:	Supported
 F:	drivers/macintosh/ams/
 
+ANALOG DEVICES INC AD5686 DRIVER
+M:	Stefan Popa <stefan.popa@analog.com>
+L:	linux-pm@vger.kernel.org
+W:	http://ez.analog.com/community/linux-device-drivers
+S:	Supported
+F:	drivers/iio/dac/ad5686*
+
 ANALOG DEVICES INC AD9389B DRIVER
 M:	Hans Verkuil <hans.verkuil@cisco.com>
 L:	linux-media@vger.kernel.org
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 965d5c0..a00fc45 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -131,15 +131,20 @@ config LTC2632
 	  module will be called ltc2632.
 
 config AD5686
-	tristate "Analog Devices AD5686R/AD5685R/AD5684R DAC SPI driver"
+	tristate
+
+config AD5686_SPI
+	tristate "Analog Devices AD5686 and similar multi-channel DACs (SPI)"
 	depends on SPI
+	select AD5686
 	help
-	  Say yes here to build support for Analog Devices AD5686R, AD5685R,
-	  AD5684R, AD5791 Voltage Output Digital to
-	  Analog Converter.
+	Say yes here to build support for Analog Devices AD5672R, AD5676,
+	AD5676R, AD5684, AD5684R, AD5684R, AD5685R, AD5686, AD5686R.
+	Voltage Output Digital to Analog Converter.
+
+	To compile this driver as a module, choose M here: the
+	module will be called ad5686.
 
-	  To compile this driver as a module, choose M here: the
-	  module will be called ad5686.
 
 config AD5755
 	tristate "Analog Devices AD5755/AD5755-1/AD5757/AD5735/AD5737 DAC driver"
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 81e710e..07db92e 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_AD5761) += ad5761.o
 obj-$(CONFIG_AD5764) += ad5764.o
 obj-$(CONFIG_AD5791) += ad5791.o
 obj-$(CONFIG_AD5686) += ad5686.o
+obj-$(CONFIG_AD5686_SPI) += ad5686-spi.o
 obj-$(CONFIG_AD7303) += ad7303.o
 obj-$(CONFIG_AD8801) += ad8801.o
 obj-$(CONFIG_CIO_DAC) += cio-dac.o
diff --git a/drivers/iio/dac/ad5686-spi.c b/drivers/iio/dac/ad5686-spi.c
new file mode 100644
index 0000000..d68579b
--- /dev/null
+++ b/drivers/iio/dac/ad5686-spi.c
@@ -0,0 +1,93 @@
+/*
+ * AD5672R, AD5676, AD5676R, AD5684, AD5684R, AD5684R, AD5685R, AD5686, AD5686R
+ * Digital to analog converters driver
+ *
+ * Copyright 2018 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2.
+ */
+
+#include "ad5686.h"
+
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+
+static int ad5686_spi_write(struct ad5686_state *st,
+			    u8 cmd, u8 addr, u16 val)
+{
+	struct spi_device *spi = to_spi_device(st->dev);
+
+	st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) |
+				      AD5686_ADDR(addr) |
+				      val);
+
+	return spi_write(spi, &st->data[0].d8[1], 3);
+}
+
+static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
+{
+	struct spi_transfer t[] = {
+		{
+			.tx_buf = &st->data[0].d8[1],
+			.len = 3,
+			.cs_change = 1,
+		}, {
+			.tx_buf = &st->data[1].d8[1],
+			.rx_buf = &st->data[2].d8[1],
+			.len = 3,
+		},
+	};
+	struct spi_device *spi = to_spi_device(st->dev);
+	int ret;
+
+	st->data[0].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_READBACK_ENABLE) |
+				      AD5686_ADDR(addr));
+	st->data[1].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_NOOP));
+
+	ret = spi_sync_transfer(spi, t, ARRAY_SIZE(t));
+	if (ret < 0)
+		return ret;
+
+	return be32_to_cpu(st->data[2].d32);
+}
+
+static int ad5686_spi_probe(struct spi_device *spi)
+{
+	const struct spi_device_id *id = spi_get_device_id(spi);
+
+	return ad5686_probe(&spi->dev, id->driver_data, id->name,
+			    ad5686_spi_write, ad5686_spi_read);
+}
+
+static int ad5686_spi_remove(struct spi_device *spi)
+{
+	return ad5686_remove(&spi->dev);
+}
+
+static const struct spi_device_id ad5686_spi_id[] = {
+	{"ad5672r", ID_AD5672R},
+	{"ad5676", ID_AD5676},
+	{"ad5676r", ID_AD5676R},
+	{"ad5684", ID_AD5684},
+	{"ad5684r", ID_AD5684R},
+	{"ad5685r", ID_AD5685R},
+	{"ad5686", ID_AD5686},
+	{"ad5686r", ID_AD5686R},
+	{}
+};
+MODULE_DEVICE_TABLE(spi, ad5686_spi_id);
+
+static struct spi_driver ad5686_spi_driver = {
+	.driver = {
+		.name = "ad5686",
+	},
+	.probe = ad5686_spi_probe,
+	.remove = ad5686_spi_remove,
+	.id_table = ad5686_spi_id,
+};
+
+module_spi_driver(ad5686_spi_driver);
+
+MODULE_AUTHOR("Stefan Popa <stefan.popa@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD5686 and similar multi-channel DACs");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index 5fb0179..95de8b8 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -11,7 +11,6 @@
 #include <linux/device.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
-#include <linux/spi/spi.h>
 #include <linux/slab.h>
 #include <linux/sysfs.h>
 #include <linux/regulator/consumer.h>
@@ -19,121 +18,7 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
-#define AD5686_ADDR(x)				((x) << 16)
-#define AD5686_CMD(x)				((x) << 20)
-
-#define AD5686_ADDR_DAC(chan)			(0x1 << (chan))
-#define AD5686_ADDR_ALL_DAC			0xF
-
-#define AD5686_CMD_NOOP				0x0
-#define AD5686_CMD_WRITE_INPUT_N		0x1
-#define AD5686_CMD_UPDATE_DAC_N			0x2
-#define AD5686_CMD_WRITE_INPUT_N_UPDATE_N	0x3
-#define AD5686_CMD_POWERDOWN_DAC		0x4
-#define AD5686_CMD_LDAC_MASK			0x5
-#define AD5686_CMD_RESET			0x6
-#define AD5686_CMD_INTERNAL_REFER_SETUP		0x7
-#define AD5686_CMD_DAISY_CHAIN_ENABLE		0x8
-#define AD5686_CMD_READBACK_ENABLE		0x9
-
-#define AD5686_LDAC_PWRDN_NONE			0x0
-#define AD5686_LDAC_PWRDN_1K			0x1
-#define AD5686_LDAC_PWRDN_100K			0x2
-#define AD5686_LDAC_PWRDN_3STATE		0x3
-
-/**
- * struct ad5686_chip_info - chip specific information
- * @int_vref_mv:	AD5620/40/60: the internal reference voltage
- * @num_channels:	number of channels
- * @channel:		channel specification
-*/
-
-struct ad5686_chip_info {
-	u16				int_vref_mv;
-	unsigned int			num_channels;
-	struct iio_chan_spec		*channels;
-};
-
-/**
- * struct ad5446_state - driver instance specific data
- * @spi:		spi_device
- * @chip_info:		chip model specific constants, available modes etc
- * @reg:		supply regulator
- * @vref_mv:		actual reference voltage used
- * @pwr_down_mask:	power down mask
- * @pwr_down_mode:	current power down mode
- * @data:		spi transfer buffers
- */
-
-struct ad5686_state {
-	struct spi_device		*spi;
-	const struct ad5686_chip_info	*chip_info;
-	struct regulator		*reg;
-	unsigned short			vref_mv;
-	unsigned			pwr_down_mask;
-	unsigned			pwr_down_mode;
-	/*
-	 * DMA (thus cache coherency maintenance) requires the
-	 * transfer buffers to live in their own cache lines.
-	 */
-
-	union {
-		__be32 d32;
-		u8 d8[4];
-	} data[3] ____cacheline_aligned;
-};
-
-/**
- * ad5686_supported_device_ids:
- */
-
-enum ad5686_supported_device_ids {
-	ID_AD5672R,
-	ID_AD5676,
-	ID_AD5676R,
-	ID_AD5684,
-	ID_AD5684R,
-	ID_AD5685R,
-	ID_AD5686,
-	ID_AD5686R
-};
-static int ad5686_spi_write(struct ad5686_state *st,
-			     u8 cmd, u8 addr, u16 val, u8 shift)
-{
-	val <<= shift;
-
-	st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) |
-			      AD5686_ADDR(addr) |
-			      val);
-
-	return spi_write(st->spi, &st->data[0].d8[1], 3);
-}
-
-static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
-{
-	struct spi_transfer t[] = {
-		{
-			.tx_buf = &st->data[0].d8[1],
-			.len = 3,
-			.cs_change = 1,
-		}, {
-			.tx_buf = &st->data[1].d8[1],
-			.rx_buf = &st->data[2].d8[1],
-			.len = 3,
-		},
-	};
-	int ret;
-
-	st->data[0].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_READBACK_ENABLE) |
-			      AD5686_ADDR(addr));
-	st->data[1].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_NOOP));
-
-	ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
-	if (ret < 0)
-		return ret;
-
-	return be32_to_cpu(st->data[2].d32);
-}
+#include "ad5686.h"
 
 static const char * const ad5686_powerdown_modes[] = {
 	"1kohm_to_gnd",
@@ -196,8 +81,9 @@ static ssize_t ad5686_write_dac_powerdown(struct iio_dev *indio_dev,
 	else
 		st->pwr_down_mask &= ~(0x3 << (chan->channel * 2));
 
-	ret = ad5686_spi_write(st, AD5686_CMD_POWERDOWN_DAC, 0,
-			       st->pwr_down_mask & st->pwr_down_mode, 0);
+	ret = st->write(st, AD5686_CMD_POWERDOWN_DAC, 0,
+			st->pwr_down_mask & st->pwr_down_mode);
+
 
 	return ret ? ret : len;
 }
@@ -214,7 +100,7 @@ static int ad5686_read_raw(struct iio_dev *indio_dev,
 	switch (m) {
 	case IIO_CHAN_INFO_RAW:
 		mutex_lock(&indio_dev->mlock);
-		ret = ad5686_spi_read(st, chan->address);
+		ret = st->read(st, chan->address);
 		mutex_unlock(&indio_dev->mlock);
 		if (ret < 0)
 			return ret;
@@ -243,11 +129,10 @@ static int ad5686_write_raw(struct iio_dev *indio_dev,
 			return -EINVAL;
 
 		mutex_lock(&indio_dev->mlock);
-		ret = ad5686_spi_write(st,
-				 AD5686_CMD_WRITE_INPUT_N_UPDATE_N,
-				 chan->address,
-				 val,
-				 chan->scan_type.shift);
+		ret = st->write(st,
+				AD5686_CMD_WRITE_INPUT_N_UPDATE_N,
+				chan->address,
+				val << chan->scan_type.shift);
 		mutex_unlock(&indio_dev->mlock);
 		break;
 	default:
@@ -357,20 +242,27 @@ static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
 	},
 };
 
-static int ad5686_probe(struct spi_device *spi)
+int ad5686_probe(struct device *dev,
+		 enum ad5686_supported_device_ids chip_type,
+		 const char *name, ad5686_write_func write,
+		 ad5686_read_func read)
 {
 	struct ad5686_state *st;
 	struct iio_dev *indio_dev;
 	int ret, voltage_uv = 0;
 
-	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
 	if (indio_dev == NULL)
 		return  -ENOMEM;
 
 	st = iio_priv(indio_dev);
-	spi_set_drvdata(spi, indio_dev);
+	dev_set_drvdata(dev, indio_dev);
+
+	st->dev = dev;
+	st->write = write;
+	st->read = read;
 
-	st->reg = devm_regulator_get_optional(&spi->dev, "vcc");
+	st->reg = devm_regulator_get_optional(dev, "vcc");
 	if (!IS_ERR(st->reg)) {
 		ret = regulator_enable(st->reg);
 		if (ret)
@@ -383,28 +275,25 @@ static int ad5686_probe(struct spi_device *spi)
 		voltage_uv = ret;
 	}
 
-	st->chip_info =
-		&ad5686_chip_info_tbl[spi_get_device_id(spi)->driver_data];
+	st->chip_info = &ad5686_chip_info_tbl[chip_type];
 
 	if (voltage_uv)
 		st->vref_mv = voltage_uv / 1000;
 	else
 		st->vref_mv = st->chip_info->int_vref_mv;
 
-	st->spi = spi;
-
 	/* Set all the power down mode for all channels to 1K pulldown */
 	st->pwr_down_mode = 0x55;
 
-	indio_dev->dev.parent = &spi->dev;
-	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->dev.parent = dev;
+	indio_dev->name = name;
 	indio_dev->info = &ad5686_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = st->chip_info->channels;
 	indio_dev->num_channels = st->chip_info->num_channels;
 
-	ret = ad5686_spi_write(st, AD5686_CMD_INTERNAL_REFER_SETUP, 0,
-				!!voltage_uv, 0);
+	ret = st->write(st, AD5686_CMD_INTERNAL_REFER_SETUP,
+			0, !!voltage_uv);
 	if (ret)
 		goto error_disable_reg;
 
@@ -419,10 +308,11 @@ static int ad5686_probe(struct spi_device *spi)
 		regulator_disable(st->reg);
 	return ret;
 }
+EXPORT_SYMBOL_GPL(ad5686_probe);
 
-static int ad5686_remove(struct spi_device *spi)
+int ad5686_remove(struct device *dev)
 {
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5686_state *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
@@ -431,29 +321,7 @@ static int ad5686_remove(struct spi_device *spi)
 
 	return 0;
 }
-
-static const struct spi_device_id ad5686_id[] = {
-	{"ad5672r", ID_AD5672R},
-	{"ad5676", ID_AD5676},
-	{"ad5676r", ID_AD5676R},
-	{"ad5684", ID_AD5684},
-	{"ad5684r", ID_AD5684R},
-	{"ad5685r", ID_AD5685R},
-	{"ad5686", ID_AD5686},
-	{"ad5686r", ID_AD5686R},
-	{}
-};
-MODULE_DEVICE_TABLE(spi, ad5686_id);
-
-static struct spi_driver ad5686_driver = {
-	.driver = {
-		   .name = "ad5686",
-		   },
-	.probe = ad5686_probe,
-	.remove = ad5686_remove,
-	.id_table = ad5686_id,
-};
-module_spi_driver(ad5686_driver);
+EXPORT_SYMBOL_GPL(ad5686_remove);
 
 MODULE_AUTHOR("Michael Hennerich <hennerich@blackfin.uclinux.org>");
 MODULE_DESCRIPTION("Analog Devices AD5686/85/84 DAC");
diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
new file mode 100644
index 0000000..e6423af
--- /dev/null
+++ b/drivers/iio/dac/ad5686.h
@@ -0,0 +1,107 @@
+#ifndef __DRIVERS_IIO_DAC_AD5686_H__
+#define __DRIVERS_IIO_DAC_AD5686_H__
+
+#include <linux/types.h>
+#include <linux/cache.h>
+#include <linux/mutex.h>
+#include <linux/kernel.h>
+
+#define AD5686_ADDR(x)				((x) << 16)
+#define AD5686_CMD(x)				((x) << 20)
+
+#define AD5686_ADDR_DAC(chan)			(0x1 << (chan))
+#define AD5686_ADDR_ALL_DAC			0xF
+
+#define AD5686_CMD_NOOP				0x0
+#define AD5686_CMD_WRITE_INPUT_N		0x1
+#define AD5686_CMD_UPDATE_DAC_N			0x2
+#define AD5686_CMD_WRITE_INPUT_N_UPDATE_N	0x3
+#define AD5686_CMD_POWERDOWN_DAC		0x4
+#define AD5686_CMD_LDAC_MASK			0x5
+#define AD5686_CMD_RESET			0x6
+#define AD5686_CMD_INTERNAL_REFER_SETUP		0x7
+#define AD5686_CMD_DAISY_CHAIN_ENABLE		0x8
+#define AD5686_CMD_READBACK_ENABLE		0x9
+
+#define AD5686_LDAC_PWRDN_NONE			0x0
+#define AD5686_LDAC_PWRDN_1K			0x1
+#define AD5686_LDAC_PWRDN_100K			0x2
+#define AD5686_LDAC_PWRDN_3STATE		0x3
+
+/**
+ * ad5686_supported_device_ids:
+ */
+enum ad5686_supported_device_ids {
+	ID_AD5672R,
+	ID_AD5676,
+	ID_AD5676R,
+	ID_AD5684,
+	ID_AD5684R,
+	ID_AD5685R,
+	ID_AD5686,
+	ID_AD5686R,
+};
+
+struct ad5686_state;
+
+typedef int (*ad5686_write_func)(struct ad5686_state *st,
+				 u8 cmd, u8 addr, u16 val);
+
+typedef int (*ad5686_read_func)(struct ad5686_state *st, u8 addr);
+
+/**
+ * struct ad5686_chip_info - chip specific information
+ * @int_vref_mv:	AD5620/40/60: the internal reference voltage
+ * @num_channels:	number of channels
+ * @channel:		channel specification
+ */
+
+struct ad5686_chip_info {
+	u16				int_vref_mv;
+	unsigned int			num_channels;
+	struct iio_chan_spec		*channels;
+};
+
+/**
+ * struct ad5446_state - driver instance specific data
+ * @spi:		spi_device
+ * @chip_info:		chip model specific constants, available modes etc
+ * @reg:		supply regulator
+ * @vref_mv:		actual reference voltage used
+ * @pwr_down_mask:	power down mask
+ * @pwr_down_mode:	current power down mode
+ * @data:		spi transfer buffers
+ */
+
+struct ad5686_state {
+	struct device			*dev;
+	const struct ad5686_chip_info	*chip_info;
+	struct regulator		*reg;
+	unsigned short			vref_mv;
+	unsigned int			pwr_down_mask;
+	unsigned int			pwr_down_mode;
+	ad5686_write_func		write;
+	ad5686_read_func		read;
+
+	/*
+	 * DMA (thus cache coherency maintenance) requires the
+	 * transfer buffers to live in their own cache lines.
+	 */
+
+	union {
+		__be32 d32;
+		__be16 d16;
+		u8 d8[4];
+	} data[3] ____cacheline_aligned;
+};
+
+
+int ad5686_probe(struct device *dev,
+		 enum ad5686_supported_device_ids chip_type,
+		 const char *name, ad5686_write_func write,
+		 ad5686_read_func read);
+
+int ad5686_remove(struct device *dev);
+
+
+#endif /* __DRIVERS_IIO_DAC_AD5686_H__ */
-- 
2.7.4

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

* [PATCH v2 6/6] iio:dac:ad5686: Add AD5671R/75R/94/94R/95R/96/96R support
  2018-03-30 13:56 [PATCH 3/3] iio:dac:ad5686: Add AD5671R/75R/94/94R/95R/96/96R support Stefan Popa
  2018-04-06 15:32 ` Jonathan Cameron
  2018-04-10 15:58 ` [PATCH v2 5/6] iio:dac:ad5686: Refactor the driver Stefan Popa
@ 2018-04-10 15:58 ` Stefan Popa
  2018-04-10 16:19   ` Randy Dunlap
                     ` (3 more replies)
  2 siblings, 4 replies; 11+ messages in thread
From: Stefan Popa @ 2018-04-10 15:58 UTC (permalink / raw)
  To: jic23, Michael.Hennerich, lars
  Cc: knaack.h, pmeerw, davem, mchehab, gregkh, linus.walleij, akpm,
	rdunlap, Ismail.Kose, lukas, fabrice.gasnier, mike.looijmans,
	kstewart, linux-iio, linux-kernel, stefan.popa

The AD5694/AD5694R/AD5695R/AD5696/AD5696R are a family of 4 channel DACs
with 12-bit, 14-bit and 16-bit precision respectively. The devices have
either no built-in reference, or built-in 2.5V reference.

The AD5671R/AD5675R are similar, except that they have 8 instead of 4
channels.

These devices are similar to AD5672R/AD5676/AD5676R and
AD5684/AD5684R/AD5684/AD5685R/AD5686/AD5686R, except that they use i2c
instead of spi.

Datasheets:
http://www.analog.com/media/en/technical-documentation/data-sheets/AD5671R_5675R.pdf
http://www.analog.com/media/en/technical-documentation/data-sheets/AD5696R_5695R_5694R.pdf

Signed-off-by: Stefan Popa <stefan.popa@analog.com>
---
Changes in v2:
	- Refactored the patch

 MAINTAINERS                  |  1 +
 drivers/iio/dac/Kconfig      | 10 +++++
 drivers/iio/dac/Makefile     |  1 +
 drivers/iio/dac/ad5686.c     | 28 +++++++++++++
 drivers/iio/dac/ad5686.h     |  7 ++++
 drivers/iio/dac/ad5696-i2c.c | 98 ++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 145 insertions(+)
 create mode 100644 drivers/iio/dac/ad5696-i2c.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 637e62d..002cb01 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -797,6 +797,7 @@ L:	linux-pm@vger.kernel.org
 W:	http://ez.analog.com/community/linux-device-drivers
 S:	Supported
 F:	drivers/iio/dac/ad5686*
+F:	drivers/iio/dac/ad5696*
 
 ANALOG DEVICES INC AD9389B DRIVER
 M:	Hans Verkuil <hans.verkuil@cisco.com>
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index a00fc45..dab0b8a 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -145,6 +145,16 @@ config AD5686_SPI
 	To compile this driver as a module, choose M here: the
 	module will be called ad5686.
 
+config AD5696_I2C
+	tristate "Analog Devices AD5696 and similar multi-channel DACs (I2C)"
+	depends on I2C
+	select AD5686
+	help
+	Say yes here to build support for Analog Devices AD5671R, AD5675R,
+	AD5694, AD5694R, AD5695R, AD5696, AD5696R Voltage Output Digital to
+	Analog Converter.
+	To compile this driver as a module, choose M here: the module will be
+	called ad5696.
 
 config AD5755
 	tristate "Analog Devices AD5755/AD5755-1/AD5757/AD5735/AD5737 DAC driver"
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 07db92e..4397e21 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_AD5764) += ad5764.o
 obj-$(CONFIG_AD5791) += ad5791.o
 obj-$(CONFIG_AD5686) += ad5686.o
 obj-$(CONFIG_AD5686_SPI) += ad5686-spi.o
+obj-$(CONFIG_AD5696_I2C) += ad5696-i2c.o
 obj-$(CONFIG_AD7303) += ad7303.o
 obj-$(CONFIG_AD8801) += ad8801.o
 obj-$(CONFIG_CIO_DAC) += cio-dac.o
diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index 95de8b8..596e1c9 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -203,11 +203,21 @@ DECLARE_AD5686_CHANNELS(ad5685r_channels, 14, 2);
 DECLARE_AD5686_CHANNELS(ad5686_channels, 16, 0);
 
 static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
+	[ID_AD5671R] = {
+		.channels = ad5672_channels,
+		.int_vref_mv = 2500,
+		.num_channels = 8,
+	},
 	[ID_AD5672R] = {
 		.channels = ad5672_channels,
 		.int_vref_mv = 2500,
 		.num_channels = 8,
 	},
+	[ID_AD5675R] = {
+		.channels = ad5676_channels,
+		.int_vref_mv = 2500,
+		.num_channels = 8,
+	},
 	[ID_AD5676] = {
 		.channels = ad5676_channels,
 		.num_channels = 8,
@@ -240,6 +250,24 @@ static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
 		.int_vref_mv = 2500,
 		.num_channels = 4,
 	},
+	[ID_AD5694] = {
+		.channels = ad5684_channels,
+		.num_channels = 4,
+	},
+	[ID_AD5694R] = {
+		.channels = ad5684_channels,
+		.int_vref_mv = 2500,
+		.num_channels = 4,
+	},
+	[ID_AD5696] = {
+		.channels = ad5686_channels,
+		.num_channels = 4,
+	},
+	[ID_AD5696R] = {
+		.channels = ad5686_channels,
+		.int_vref_mv = 2500,
+		.num_channels = 4,
+	},
 };
 
 int ad5686_probe(struct device *dev,
diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
index e6423af..b2b0110 100644
--- a/drivers/iio/dac/ad5686.h
+++ b/drivers/iio/dac/ad5686.h
@@ -32,7 +32,9 @@
  * ad5686_supported_device_ids:
  */
 enum ad5686_supported_device_ids {
+	ID_AD5671R,
 	ID_AD5672R,
+	ID_AD5675R,
 	ID_AD5676,
 	ID_AD5676R,
 	ID_AD5684,
@@ -40,6 +42,11 @@ enum ad5686_supported_device_ids {
 	ID_AD5685R,
 	ID_AD5686,
 	ID_AD5686R,
+	ID_AD5694,
+	ID_AD5694R,
+	ID_AD5695R,
+	ID_AD5696,
+	ID_AD5696R,
 };
 
 struct ad5686_state;
diff --git a/drivers/iio/dac/ad5696-i2c.c b/drivers/iio/dac/ad5696-i2c.c
new file mode 100644
index 0000000..c9187de
--- /dev/null
+++ b/drivers/iio/dac/ad5696-i2c.c
@@ -0,0 +1,98 @@
+/*
+ * AD5671R, AD5675R, AD5694, AD5694R, AD5695R, AD5696, AD5696R
+ * Digital to analog converters driver
+ *
+ * Copyright 2018 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2.
+ */
+
+#include "ad5686.h"
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+
+static int ad5686_i2c_read(struct ad5686_state *st, u8 addr)
+{
+	struct i2c_client *i2c = to_i2c_client(st->dev);
+	struct i2c_msg msg[2] = {
+		{
+			.addr = i2c->addr,
+			.flags = i2c->flags,
+			.len = 3,
+			.buf = &st->data[0].d8[1],
+		},
+		{
+			.addr = i2c->addr,
+			.flags = i2c->flags | I2C_M_RD,
+			.len = 2,
+			.buf = (char *)&st->data[0].d16,
+		},
+	};
+	int ret;
+
+	st->data[0].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_NOOP) |
+				      AD5686_ADDR(addr) |
+				      0x00);
+
+	ret = i2c_transfer(i2c->adapter, msg, 2);
+	if (ret < 0)
+		return ret;
+
+	return be16_to_cpu(st->data[0].d16);
+}
+
+static int ad5686_i2c_write(struct ad5686_state *st,
+			    u8 cmd, u8 addr, u16 val)
+{
+	struct i2c_client *i2c = to_i2c_client(st->dev);
+	int ret;
+
+	st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) | AD5686_ADDR(addr)
+				      | val);
+
+	ret = i2c_master_send(i2c, &st->data[0].d8[1], 3);
+	if (ret < 0)
+		return ret;
+
+	return (ret != 3) ? -EIO : 0;
+}
+
+static int ad5686_i2c_probe(struct i2c_client *i2c,
+			    const struct i2c_device_id *id)
+{
+	return ad5686_probe(&i2c->dev, id->driver_data, id->name,
+			    ad5686_i2c_write, ad5686_i2c_read);
+}
+
+static int ad5686_i2c_remove(struct i2c_client *i2c)
+{
+	return ad5686_remove(&i2c->dev);
+}
+
+static const struct i2c_device_id ad5686_i2c_id[] = {
+	{"ad5671r", ID_AD5671R},
+	{"ad5675r", ID_AD5675R},
+	{"ad5694", ID_AD5694},
+	{"ad5694r", ID_AD5694R},
+	{"ad5695r", ID_AD5695R},
+	{"ad5696", ID_AD5696},
+	{"ad5696r", ID_AD5696R},
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, i2c_device_id);
+
+static struct i2c_driver ad5686_i2c_driver = {
+	.driver = {
+		.name = "ad5696",
+	},
+	.probe = ad5686_i2c_probe,
+	.remove = ad5686_i2c_remove,
+	.id_table = ad5686_i2c_id,
+};
+
+module_i2c_driver(ad5686_i2c_driver);
+
+MODULE_AUTHOR("Stefan Popa <stefan.popa@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD5686 and similar multi-channel DACs");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

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

* Re: [PATCH v2 6/6] iio:dac:ad5686: Add AD5671R/75R/94/94R/95R/96/96R support
  2018-04-10 15:58 ` [PATCH v2 6/6] iio:dac:ad5686: Add AD5671R/75R/94/94R/95R/96/96R support Stefan Popa
@ 2018-04-10 16:19   ` Randy Dunlap
  2018-04-11 11:52   ` [PATCH v3 2/7] iio:dac:ad5686: Change license description Stefan Popa
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Randy Dunlap @ 2018-04-10 16:19 UTC (permalink / raw)
  To: Stefan Popa, jic23, Michael.Hennerich, lars
  Cc: knaack.h, pmeerw, davem, mchehab, gregkh, linus.walleij, akpm,
	Ismail.Kose, lukas, fabrice.gasnier, mike.looijmans, kstewart,
	linux-iio, linux-kernel

On 04/10/18 08:58, Stefan Popa wrote:
> The AD5694/AD5694R/AD5695R/AD5696/AD5696R are a family of 4 channel DACs
> with 12-bit, 14-bit and 16-bit precision respectively. The devices have
> either no built-in reference, or built-in 2.5V reference.
> 
> The AD5671R/AD5675R are similar, except that they have 8 instead of 4
> channels.
> 
> These devices are similar to AD5672R/AD5676/AD5676R and
> AD5684/AD5684R/AD5684/AD5685R/AD5686/AD5686R, except that they use i2c
> instead of spi.
> 
> Datasheets:
> http://www.analog.com/media/en/technical-documentation/data-sheets/AD5671R_5675R.pdf
> http://www.analog.com/media/en/technical-documentation/data-sheets/AD5696R_5695R_5694R.pdf
> 
> Signed-off-by: Stefan Popa <stefan.popa@analog.com>
> ---
> Changes in v2:
> 	- Refactored the patch
> 
>  MAINTAINERS                  |  1 +
>  drivers/iio/dac/Kconfig      | 10 +++++
>  drivers/iio/dac/Makefile     |  1 +
>  drivers/iio/dac/ad5686.c     | 28 +++++++++++++
>  drivers/iio/dac/ad5686.h     |  7 ++++
>  drivers/iio/dac/ad5696-i2c.c | 98 ++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 145 insertions(+)
>  create mode 100644 drivers/iio/dac/ad5696-i2c.c
> 
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index a00fc45..dab0b8a 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -145,6 +145,16 @@ config AD5686_SPI
>  	To compile this driver as a module, choose M here: the
>  	module will be called ad5686.
>  
> +config AD5696_I2C
> +	tristate "Analog Devices AD5696 and similar multi-channel DACs (I2C)"
> +	depends on I2C
> +	select AD5686
> +	help
> +	Say yes here to build support for Analog Devices AD5671R, AD5675R,
> +	AD5694, AD5694R, AD5695R, AD5696, AD5696R Voltage Output Digital to
> +	Analog Converter.
> +	To compile this driver as a module, choose M here: the module will be
> +	called ad5696.

The help text (following the "help" line) should be indented 2 additional spaces,
according to Documentation/process/coding-style.rst.

(for patches 5 & 6)

>  config AD5755
>  	tristate "Analog Devices AD5755/AD5755-1/AD5757/AD5735/AD5737 DAC driver"


-- 
~Randy

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

* [PATCH v3 2/7] iio:dac:ad5686: Change license description
  2018-04-10 15:58 ` [PATCH v2 6/6] iio:dac:ad5686: Add AD5671R/75R/94/94R/95R/96/96R support Stefan Popa
  2018-04-10 16:19   ` Randy Dunlap
@ 2018-04-11 11:52   ` Stefan Popa
  2018-04-15 18:13     ` Jonathan Cameron
  2018-04-11 11:53   ` [PATCH v3 6/7] iio:dac:ad5686: Refactor the driver Stefan Popa
  2018-04-11 11:53   ` [PATCH v3 7/7] iio:dac:ad5686: Add AD5671R/75R/94/94R/95R/96/96R support Stefan Popa
  3 siblings, 1 reply; 11+ messages in thread
From: Stefan Popa @ 2018-04-11 11:52 UTC (permalink / raw)
  To: jic23, Michael.Hennerich, lars
  Cc: knaack.h, pmeerw, linux-iio, linux-kernel, stefan.popa

Add GPLv2+ SPDX identifier and remove license notice to keep the whole
purpose of using an SPDx id.

Signed-off-by: Stefan Popa <stefan.popa@analog.com>
---
Changes in v3:
	- Created a new patch to use the SPDx identifier

 drivers/iio/dac/ad5686.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index f7f975c..e328513 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -1,9 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0+
 /*
  * AD5686R, AD5685R, AD5684R Digital to analog converters  driver
  *
  * Copyright 2011 Analog Devices Inc.
- *
- * Licensed under the GPL-2.
  */
 
 #include <linux/interrupt.h>
-- 
2.7.4

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

* [PATCH v3 6/7] iio:dac:ad5686: Refactor the driver
  2018-04-10 15:58 ` [PATCH v2 6/6] iio:dac:ad5686: Add AD5671R/75R/94/94R/95R/96/96R support Stefan Popa
  2018-04-10 16:19   ` Randy Dunlap
  2018-04-11 11:52   ` [PATCH v3 2/7] iio:dac:ad5686: Change license description Stefan Popa
@ 2018-04-11 11:53   ` Stefan Popa
  2018-04-15 18:37     ` Jonathan Cameron
  2018-04-11 11:53   ` [PATCH v3 7/7] iio:dac:ad5686: Add AD5671R/75R/94/94R/95R/96/96R support Stefan Popa
  3 siblings, 1 reply; 11+ messages in thread
From: Stefan Popa @ 2018-04-11 11:53 UTC (permalink / raw)
  To: jic23, Michael.Hennerich, lars
  Cc: knaack.h, pmeerw, davem, mchehab, gregkh, linus.walleij, akpm,
	rdunlap, Ismail.Kose, lukas, fabrice.gasnier, mike.looijmans,
	pombredanne, linux-iio, linux-kernel, stefan.popa

In this patch restructures the existing ad5686 driver by adding a module
for SPI and a header file, while the baseline module deals with the
chip-logic.

This is a necessary step, as this driver should support in the future
similar devices which differ only in the type of interface used (I2C
instead of SPI).

Signed-off-by: Stefan Popa <stefan.popa@analog.com>
---
Changes in v2:
	- Refactored the patch
	- Use st->write directly instead of the ad5686_write() wrapper
	- Use st->read directly instead of the ad5686_read() wrapper
Changes in v3:
	- Indented the the help text from the Konfig file with 2
	  additional spaces.
	- Changed the license description to use an SPDX tag.

 MAINTAINERS                  |   7 ++
 drivers/iio/dac/Kconfig      |  13 ++-
 drivers/iio/dac/Makefile     |   1 +
 drivers/iio/dac/ad5686-spi.c |  92 +++++++++++++++++++++
 drivers/iio/dac/ad5686.c     | 190 +++++++------------------------------------
 drivers/iio/dac/ad5686.h     | 114 ++++++++++++++++++++++++++
 6 files changed, 252 insertions(+), 165 deletions(-)
 create mode 100644 drivers/iio/dac/ad5686-spi.c
 create mode 100644 drivers/iio/dac/ad5686.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 473ac00..637e62d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -791,6 +791,13 @@ M:	Michael Hanselmann <linux-kernel@hansmi.ch>
 S:	Supported
 F:	drivers/macintosh/ams/
 
+ANALOG DEVICES INC AD5686 DRIVER
+M:	Stefan Popa <stefan.popa@analog.com>
+L:	linux-pm@vger.kernel.org
+W:	http://ez.analog.com/community/linux-device-drivers
+S:	Supported
+F:	drivers/iio/dac/ad5686*
+
 ANALOG DEVICES INC AD9389B DRIVER
 M:	Hans Verkuil <hans.verkuil@cisco.com>
 L:	linux-media@vger.kernel.org
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 965d5c0..7a81f1e 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -131,16 +131,21 @@ config LTC2632
 	  module will be called ltc2632.
 
 config AD5686
-	tristate "Analog Devices AD5686R/AD5685R/AD5684R DAC SPI driver"
+	tristate
+
+config AD5686_SPI
+	tristate "Analog Devices AD5686 and similar multi-channel DACs (SPI)"
 	depends on SPI
+	select AD5686
 	help
-	  Say yes here to build support for Analog Devices AD5686R, AD5685R,
-	  AD5684R, AD5791 Voltage Output Digital to
-	  Analog Converter.
+	  Say yes here to build support for Analog Devices AD5672R, AD5676,
+	  AD5676R, AD5684, AD5684R, AD5684R, AD5685R, AD5686, AD5686R.
+	  Voltage Output Digital to Analog Converter.
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad5686.
 
+
 config AD5755
 	tristate "Analog Devices AD5755/AD5755-1/AD5757/AD5735/AD5737 DAC driver"
 	depends on SPI_MASTER
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 81e710e..07db92e 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_AD5761) += ad5761.o
 obj-$(CONFIG_AD5764) += ad5764.o
 obj-$(CONFIG_AD5791) += ad5791.o
 obj-$(CONFIG_AD5686) += ad5686.o
+obj-$(CONFIG_AD5686_SPI) += ad5686-spi.o
 obj-$(CONFIG_AD7303) += ad7303.o
 obj-$(CONFIG_AD8801) += ad8801.o
 obj-$(CONFIG_CIO_DAC) += cio-dac.o
diff --git a/drivers/iio/dac/ad5686-spi.c b/drivers/iio/dac/ad5686-spi.c
new file mode 100644
index 0000000..1cc807b
--- /dev/null
+++ b/drivers/iio/dac/ad5686-spi.c
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * AD5672R, AD5676, AD5676R, AD5684, AD5684R, AD5684R, AD5685R, AD5686, AD5686R
+ * Digital to analog converters driver
+ *
+ * Copyright 2018 Analog Devices Inc.
+ */
+
+#include "ad5686.h"
+
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+
+static int ad5686_spi_write(struct ad5686_state *st,
+			    u8 cmd, u8 addr, u16 val)
+{
+	struct spi_device *spi = to_spi_device(st->dev);
+
+	st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) |
+				      AD5686_ADDR(addr) |
+				      val);
+
+	return spi_write(spi, &st->data[0].d8[1], 3);
+}
+
+static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
+{
+	struct spi_transfer t[] = {
+		{
+			.tx_buf = &st->data[0].d8[1],
+			.len = 3,
+			.cs_change = 1,
+		}, {
+			.tx_buf = &st->data[1].d8[1],
+			.rx_buf = &st->data[2].d8[1],
+			.len = 3,
+		},
+	};
+	struct spi_device *spi = to_spi_device(st->dev);
+	int ret;
+
+	st->data[0].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_READBACK_ENABLE) |
+				      AD5686_ADDR(addr));
+	st->data[1].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_NOOP));
+
+	ret = spi_sync_transfer(spi, t, ARRAY_SIZE(t));
+	if (ret < 0)
+		return ret;
+
+	return be32_to_cpu(st->data[2].d32);
+}
+
+static int ad5686_spi_probe(struct spi_device *spi)
+{
+	const struct spi_device_id *id = spi_get_device_id(spi);
+
+	return ad5686_probe(&spi->dev, id->driver_data, id->name,
+			    ad5686_spi_write, ad5686_spi_read);
+}
+
+static int ad5686_spi_remove(struct spi_device *spi)
+{
+	return ad5686_remove(&spi->dev);
+}
+
+static const struct spi_device_id ad5686_spi_id[] = {
+	{"ad5672r", ID_AD5672R},
+	{"ad5676", ID_AD5676},
+	{"ad5676r", ID_AD5676R},
+	{"ad5684", ID_AD5684},
+	{"ad5684r", ID_AD5684R},
+	{"ad5685r", ID_AD5685R},
+	{"ad5686", ID_AD5686},
+	{"ad5686r", ID_AD5686R},
+	{}
+};
+MODULE_DEVICE_TABLE(spi, ad5686_spi_id);
+
+static struct spi_driver ad5686_spi_driver = {
+	.driver = {
+		.name = "ad5686",
+	},
+	.probe = ad5686_spi_probe,
+	.remove = ad5686_spi_remove,
+	.id_table = ad5686_spi_id,
+};
+
+module_spi_driver(ad5686_spi_driver);
+
+MODULE_AUTHOR("Stefan Popa <stefan.popa@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD5686 and similar multi-channel DACs");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index 54f67d5..79abff5 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -10,7 +10,6 @@
 #include <linux/device.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
-#include <linux/spi/spi.h>
 #include <linux/slab.h>
 #include <linux/sysfs.h>
 #include <linux/regulator/consumer.h>
@@ -18,121 +17,7 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
-#define AD5686_ADDR(x)				((x) << 16)
-#define AD5686_CMD(x)				((x) << 20)
-
-#define AD5686_ADDR_DAC(chan)			(0x1 << (chan))
-#define AD5686_ADDR_ALL_DAC			0xF
-
-#define AD5686_CMD_NOOP				0x0
-#define AD5686_CMD_WRITE_INPUT_N		0x1
-#define AD5686_CMD_UPDATE_DAC_N			0x2
-#define AD5686_CMD_WRITE_INPUT_N_UPDATE_N	0x3
-#define AD5686_CMD_POWERDOWN_DAC		0x4
-#define AD5686_CMD_LDAC_MASK			0x5
-#define AD5686_CMD_RESET			0x6
-#define AD5686_CMD_INTERNAL_REFER_SETUP		0x7
-#define AD5686_CMD_DAISY_CHAIN_ENABLE		0x8
-#define AD5686_CMD_READBACK_ENABLE		0x9
-
-#define AD5686_LDAC_PWRDN_NONE			0x0
-#define AD5686_LDAC_PWRDN_1K			0x1
-#define AD5686_LDAC_PWRDN_100K			0x2
-#define AD5686_LDAC_PWRDN_3STATE		0x3
-
-/**
- * struct ad5686_chip_info - chip specific information
- * @int_vref_mv:	AD5620/40/60: the internal reference voltage
- * @num_channels:	number of channels
- * @channel:		channel specification
-*/
-
-struct ad5686_chip_info {
-	u16				int_vref_mv;
-	unsigned int			num_channels;
-	struct iio_chan_spec		*channels;
-};
-
-/**
- * struct ad5446_state - driver instance specific data
- * @spi:		spi_device
- * @chip_info:		chip model specific constants, available modes etc
- * @reg:		supply regulator
- * @vref_mv:		actual reference voltage used
- * @pwr_down_mask:	power down mask
- * @pwr_down_mode:	current power down mode
- * @data:		spi transfer buffers
- */
-
-struct ad5686_state {
-	struct spi_device		*spi;
-	const struct ad5686_chip_info	*chip_info;
-	struct regulator		*reg;
-	unsigned short			vref_mv;
-	unsigned			pwr_down_mask;
-	unsigned			pwr_down_mode;
-	/*
-	 * DMA (thus cache coherency maintenance) requires the
-	 * transfer buffers to live in their own cache lines.
-	 */
-
-	union {
-		__be32 d32;
-		u8 d8[4];
-	} data[3] ____cacheline_aligned;
-};
-
-/**
- * ad5686_supported_device_ids:
- */
-
-enum ad5686_supported_device_ids {
-	ID_AD5672R,
-	ID_AD5676,
-	ID_AD5676R,
-	ID_AD5684,
-	ID_AD5684R,
-	ID_AD5685R,
-	ID_AD5686,
-	ID_AD5686R
-};
-static int ad5686_spi_write(struct ad5686_state *st,
-			     u8 cmd, u8 addr, u16 val, u8 shift)
-{
-	val <<= shift;
-
-	st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) |
-			      AD5686_ADDR(addr) |
-			      val);
-
-	return spi_write(st->spi, &st->data[0].d8[1], 3);
-}
-
-static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
-{
-	struct spi_transfer t[] = {
-		{
-			.tx_buf = &st->data[0].d8[1],
-			.len = 3,
-			.cs_change = 1,
-		}, {
-			.tx_buf = &st->data[1].d8[1],
-			.rx_buf = &st->data[2].d8[1],
-			.len = 3,
-		},
-	};
-	int ret;
-
-	st->data[0].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_READBACK_ENABLE) |
-			      AD5686_ADDR(addr));
-	st->data[1].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_NOOP));
-
-	ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
-	if (ret < 0)
-		return ret;
-
-	return be32_to_cpu(st->data[2].d32);
-}
+#include "ad5686.h"
 
 static const char * const ad5686_powerdown_modes[] = {
 	"1kohm_to_gnd",
@@ -195,8 +80,9 @@ static ssize_t ad5686_write_dac_powerdown(struct iio_dev *indio_dev,
 	else
 		st->pwr_down_mask &= ~(0x3 << (chan->channel * 2));
 
-	ret = ad5686_spi_write(st, AD5686_CMD_POWERDOWN_DAC, 0,
-			       st->pwr_down_mask & st->pwr_down_mode, 0);
+	ret = st->write(st, AD5686_CMD_POWERDOWN_DAC, 0,
+			st->pwr_down_mask & st->pwr_down_mode);
+
 
 	return ret ? ret : len;
 }
@@ -213,7 +99,7 @@ static int ad5686_read_raw(struct iio_dev *indio_dev,
 	switch (m) {
 	case IIO_CHAN_INFO_RAW:
 		mutex_lock(&indio_dev->mlock);
-		ret = ad5686_spi_read(st, chan->address);
+		ret = st->read(st, chan->address);
 		mutex_unlock(&indio_dev->mlock);
 		if (ret < 0)
 			return ret;
@@ -242,11 +128,10 @@ static int ad5686_write_raw(struct iio_dev *indio_dev,
 			return -EINVAL;
 
 		mutex_lock(&indio_dev->mlock);
-		ret = ad5686_spi_write(st,
-				 AD5686_CMD_WRITE_INPUT_N_UPDATE_N,
-				 chan->address,
-				 val,
-				 chan->scan_type.shift);
+		ret = st->write(st,
+				AD5686_CMD_WRITE_INPUT_N_UPDATE_N,
+				chan->address,
+				val << chan->scan_type.shift);
 		mutex_unlock(&indio_dev->mlock);
 		break;
 	default:
@@ -356,20 +241,27 @@ static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
 	},
 };
 
-static int ad5686_probe(struct spi_device *spi)
+int ad5686_probe(struct device *dev,
+		 enum ad5686_supported_device_ids chip_type,
+		 const char *name, ad5686_write_func write,
+		 ad5686_read_func read)
 {
 	struct ad5686_state *st;
 	struct iio_dev *indio_dev;
 	int ret, voltage_uv = 0;
 
-	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
 	if (indio_dev == NULL)
 		return  -ENOMEM;
 
 	st = iio_priv(indio_dev);
-	spi_set_drvdata(spi, indio_dev);
+	dev_set_drvdata(dev, indio_dev);
+
+	st->dev = dev;
+	st->write = write;
+	st->read = read;
 
-	st->reg = devm_regulator_get_optional(&spi->dev, "vcc");
+	st->reg = devm_regulator_get_optional(dev, "vcc");
 	if (!IS_ERR(st->reg)) {
 		ret = regulator_enable(st->reg);
 		if (ret)
@@ -382,28 +274,25 @@ static int ad5686_probe(struct spi_device *spi)
 		voltage_uv = ret;
 	}
 
-	st->chip_info =
-		&ad5686_chip_info_tbl[spi_get_device_id(spi)->driver_data];
+	st->chip_info = &ad5686_chip_info_tbl[chip_type];
 
 	if (voltage_uv)
 		st->vref_mv = voltage_uv / 1000;
 	else
 		st->vref_mv = st->chip_info->int_vref_mv;
 
-	st->spi = spi;
-
 	/* Set all the power down mode for all channels to 1K pulldown */
 	st->pwr_down_mode = 0x55;
 
-	indio_dev->dev.parent = &spi->dev;
-	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->dev.parent = dev;
+	indio_dev->name = name;
 	indio_dev->info = &ad5686_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = st->chip_info->channels;
 	indio_dev->num_channels = st->chip_info->num_channels;
 
-	ret = ad5686_spi_write(st, AD5686_CMD_INTERNAL_REFER_SETUP, 0,
-				!!voltage_uv, 0);
+	ret = st->write(st, AD5686_CMD_INTERNAL_REFER_SETUP,
+			0, !!voltage_uv);
 	if (ret)
 		goto error_disable_reg;
 
@@ -418,10 +307,11 @@ static int ad5686_probe(struct spi_device *spi)
 		regulator_disable(st->reg);
 	return ret;
 }
+EXPORT_SYMBOL_GPL(ad5686_probe);
 
-static int ad5686_remove(struct spi_device *spi)
+int ad5686_remove(struct device *dev)
 {
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5686_state *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
@@ -430,29 +320,7 @@ static int ad5686_remove(struct spi_device *spi)
 
 	return 0;
 }
-
-static const struct spi_device_id ad5686_id[] = {
-	{"ad5672r", ID_AD5672R},
-	{"ad5676", ID_AD5676},
-	{"ad5676r", ID_AD5676R},
-	{"ad5684", ID_AD5684},
-	{"ad5684r", ID_AD5684R},
-	{"ad5685r", ID_AD5685R},
-	{"ad5686", ID_AD5686},
-	{"ad5686r", ID_AD5686R},
-	{}
-};
-MODULE_DEVICE_TABLE(spi, ad5686_id);
-
-static struct spi_driver ad5686_driver = {
-	.driver = {
-		   .name = "ad5686",
-		   },
-	.probe = ad5686_probe,
-	.remove = ad5686_remove,
-	.id_table = ad5686_id,
-};
-module_spi_driver(ad5686_driver);
+EXPORT_SYMBOL_GPL(ad5686_remove);
 
 MODULE_AUTHOR("Michael Hennerich <hennerich@blackfin.uclinux.org>");
 MODULE_DESCRIPTION("Analog Devices AD5686/85/84 DAC");
diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
new file mode 100644
index 0000000..c8e1565
--- /dev/null
+++ b/drivers/iio/dac/ad5686.h
@@ -0,0 +1,114 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * This file is part of AD5686 DAC driver
+ *
+ * Copyright 2018 Analog Devices Inc.
+ */
+
+#ifndef __DRIVERS_IIO_DAC_AD5686_H__
+#define __DRIVERS_IIO_DAC_AD5686_H__
+
+#include <linux/types.h>
+#include <linux/cache.h>
+#include <linux/mutex.h>
+#include <linux/kernel.h>
+
+#define AD5686_ADDR(x)				((x) << 16)
+#define AD5686_CMD(x)				((x) << 20)
+
+#define AD5686_ADDR_DAC(chan)			(0x1 << (chan))
+#define AD5686_ADDR_ALL_DAC			0xF
+
+#define AD5686_CMD_NOOP				0x0
+#define AD5686_CMD_WRITE_INPUT_N		0x1
+#define AD5686_CMD_UPDATE_DAC_N			0x2
+#define AD5686_CMD_WRITE_INPUT_N_UPDATE_N	0x3
+#define AD5686_CMD_POWERDOWN_DAC		0x4
+#define AD5686_CMD_LDAC_MASK			0x5
+#define AD5686_CMD_RESET			0x6
+#define AD5686_CMD_INTERNAL_REFER_SETUP		0x7
+#define AD5686_CMD_DAISY_CHAIN_ENABLE		0x8
+#define AD5686_CMD_READBACK_ENABLE		0x9
+
+#define AD5686_LDAC_PWRDN_NONE			0x0
+#define AD5686_LDAC_PWRDN_1K			0x1
+#define AD5686_LDAC_PWRDN_100K			0x2
+#define AD5686_LDAC_PWRDN_3STATE		0x3
+
+/**
+ * ad5686_supported_device_ids:
+ */
+enum ad5686_supported_device_ids {
+	ID_AD5672R,
+	ID_AD5676,
+	ID_AD5676R,
+	ID_AD5684,
+	ID_AD5684R,
+	ID_AD5685R,
+	ID_AD5686,
+	ID_AD5686R,
+};
+
+struct ad5686_state;
+
+typedef int (*ad5686_write_func)(struct ad5686_state *st,
+				 u8 cmd, u8 addr, u16 val);
+
+typedef int (*ad5686_read_func)(struct ad5686_state *st, u8 addr);
+
+/**
+ * struct ad5686_chip_info - chip specific information
+ * @int_vref_mv:	AD5620/40/60: the internal reference voltage
+ * @num_channels:	number of channels
+ * @channel:		channel specification
+ */
+
+struct ad5686_chip_info {
+	u16				int_vref_mv;
+	unsigned int			num_channels;
+	struct iio_chan_spec		*channels;
+};
+
+/**
+ * struct ad5446_state - driver instance specific data
+ * @spi:		spi_device
+ * @chip_info:		chip model specific constants, available modes etc
+ * @reg:		supply regulator
+ * @vref_mv:		actual reference voltage used
+ * @pwr_down_mask:	power down mask
+ * @pwr_down_mode:	current power down mode
+ * @data:		spi transfer buffers
+ */
+
+struct ad5686_state {
+	struct device			*dev;
+	const struct ad5686_chip_info	*chip_info;
+	struct regulator		*reg;
+	unsigned short			vref_mv;
+	unsigned int			pwr_down_mask;
+	unsigned int			pwr_down_mode;
+	ad5686_write_func		write;
+	ad5686_read_func		read;
+
+	/*
+	 * DMA (thus cache coherency maintenance) requires the
+	 * transfer buffers to live in their own cache lines.
+	 */
+
+	union {
+		__be32 d32;
+		__be16 d16;
+		u8 d8[4];
+	} data[3] ____cacheline_aligned;
+};
+
+
+int ad5686_probe(struct device *dev,
+		 enum ad5686_supported_device_ids chip_type,
+		 const char *name, ad5686_write_func write,
+		 ad5686_read_func read);
+
+int ad5686_remove(struct device *dev);
+
+
+#endif /* __DRIVERS_IIO_DAC_AD5686_H__ */
-- 
2.7.4

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

* [PATCH v3 7/7] iio:dac:ad5686: Add AD5671R/75R/94/94R/95R/96/96R support
  2018-04-10 15:58 ` [PATCH v2 6/6] iio:dac:ad5686: Add AD5671R/75R/94/94R/95R/96/96R support Stefan Popa
                     ` (2 preceding siblings ...)
  2018-04-11 11:53   ` [PATCH v3 6/7] iio:dac:ad5686: Refactor the driver Stefan Popa
@ 2018-04-11 11:53   ` Stefan Popa
  2018-04-15 19:09     ` Jonathan Cameron
  3 siblings, 1 reply; 11+ messages in thread
From: Stefan Popa @ 2018-04-11 11:53 UTC (permalink / raw)
  To: jic23, Michael.Hennerich, lars
  Cc: knaack.h, pmeerw, davem, mchehab, gregkh, linus.walleij, akpm,
	rdunlap, Ismail.Kose, lukas, fabrice.gasnier, mike.looijmans,
	kstewart, linux-iio, linux-kernel, stefan.popa

The AD5694/AD5694R/AD5695R/AD5696/AD5696R are a family of 4 channel DACs
with 12-bit, 14-bit and 16-bit precision respectively. The devices have
either no built-in reference, or built-in 2.5V reference.

The AD5671R/AD5675R are similar, except that they have 8 instead of 4
channels.

These devices are similar to AD5672R/AD5676/AD5676R and
AD5684/AD5684R/AD5684/AD5685R/AD5686/AD5686R, except that they use i2c
instead of spi.

Datasheets:
http://www.analog.com/media/en/technical-documentation/data-sheets/AD5671R_5675R.pdf
http://www.analog.com/media/en/technical-documentation/data-sheets/AD5696R_5695R_5694R.pdf

Signed-off-by: Stefan Popa <stefan.popa@analog.com>
---
Changes in v2:
	- Refactored the patch
Changes in v3:
	- Indented the the help text from the Konfig file with 2
	  additional spaces.
	- Changed the license description to use an SPDX tag.

 MAINTAINERS                  |  1 +
 drivers/iio/dac/Kconfig      | 10 +++++
 drivers/iio/dac/Makefile     |  1 +
 drivers/iio/dac/ad5686.c     | 28 +++++++++++++
 drivers/iio/dac/ad5686.h     |  7 ++++
 drivers/iio/dac/ad5696-i2c.c | 97 ++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 144 insertions(+)
 create mode 100644 drivers/iio/dac/ad5696-i2c.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 637e62d..002cb01 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -797,6 +797,7 @@ L:	linux-pm@vger.kernel.org
 W:	http://ez.analog.com/community/linux-device-drivers
 S:	Supported
 F:	drivers/iio/dac/ad5686*
+F:	drivers/iio/dac/ad5696*
 
 ANALOG DEVICES INC AD9389B DRIVER
 M:	Hans Verkuil <hans.verkuil@cisco.com>
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 7a81f1e..3ff8a32 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -145,6 +145,16 @@ config AD5686_SPI
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad5686.
 
+config AD5696_I2C
+	tristate "Analog Devices AD5696 and similar multi-channel DACs (I2C)"
+	depends on I2C
+	select AD5686
+	help
+	  Say yes here to build support for Analog Devices AD5671R, AD5675R,
+	  AD5694, AD5694R, AD5695R, AD5696, AD5696R Voltage Output Digital to
+	  Analog Converter.
+	  To compile this driver as a module, choose M here: the module will be
+	  called ad5696.
 
 config AD5755
 	tristate "Analog Devices AD5755/AD5755-1/AD5757/AD5735/AD5737 DAC driver"
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 07db92e..4397e21 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_AD5764) += ad5764.o
 obj-$(CONFIG_AD5791) += ad5791.o
 obj-$(CONFIG_AD5686) += ad5686.o
 obj-$(CONFIG_AD5686_SPI) += ad5686-spi.o
+obj-$(CONFIG_AD5696_I2C) += ad5696-i2c.o
 obj-$(CONFIG_AD7303) += ad7303.o
 obj-$(CONFIG_AD8801) += ad8801.o
 obj-$(CONFIG_CIO_DAC) += cio-dac.o
diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index 79abff5..89c5f08 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -202,11 +202,21 @@ DECLARE_AD5686_CHANNELS(ad5685r_channels, 14, 2);
 DECLARE_AD5686_CHANNELS(ad5686_channels, 16, 0);
 
 static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
+	[ID_AD5671R] = {
+		.channels = ad5672_channels,
+		.int_vref_mv = 2500,
+		.num_channels = 8,
+	},
 	[ID_AD5672R] = {
 		.channels = ad5672_channels,
 		.int_vref_mv = 2500,
 		.num_channels = 8,
 	},
+	[ID_AD5675R] = {
+		.channels = ad5676_channels,
+		.int_vref_mv = 2500,
+		.num_channels = 8,
+	},
 	[ID_AD5676] = {
 		.channels = ad5676_channels,
 		.num_channels = 8,
@@ -239,6 +249,24 @@ static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
 		.int_vref_mv = 2500,
 		.num_channels = 4,
 	},
+	[ID_AD5694] = {
+		.channels = ad5684_channels,
+		.num_channels = 4,
+	},
+	[ID_AD5694R] = {
+		.channels = ad5684_channels,
+		.int_vref_mv = 2500,
+		.num_channels = 4,
+	},
+	[ID_AD5696] = {
+		.channels = ad5686_channels,
+		.num_channels = 4,
+	},
+	[ID_AD5696R] = {
+		.channels = ad5686_channels,
+		.int_vref_mv = 2500,
+		.num_channels = 4,
+	},
 };
 
 int ad5686_probe(struct device *dev,
diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
index c8e1565..05f0ce9 100644
--- a/drivers/iio/dac/ad5686.h
+++ b/drivers/iio/dac/ad5686.h
@@ -39,7 +39,9 @@
  * ad5686_supported_device_ids:
  */
 enum ad5686_supported_device_ids {
+	ID_AD5671R,
 	ID_AD5672R,
+	ID_AD5675R,
 	ID_AD5676,
 	ID_AD5676R,
 	ID_AD5684,
@@ -47,6 +49,11 @@ enum ad5686_supported_device_ids {
 	ID_AD5685R,
 	ID_AD5686,
 	ID_AD5686R,
+	ID_AD5694,
+	ID_AD5694R,
+	ID_AD5695R,
+	ID_AD5696,
+	ID_AD5696R,
 };
 
 struct ad5686_state;
diff --git a/drivers/iio/dac/ad5696-i2c.c b/drivers/iio/dac/ad5696-i2c.c
new file mode 100644
index 0000000..088cb36
--- /dev/null
+++ b/drivers/iio/dac/ad5696-i2c.c
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * AD5671R, AD5675R, AD5694, AD5694R, AD5695R, AD5696, AD5696R
+ * Digital to analog converters driver
+ *
+ * Copyright 2018 Analog Devices Inc.
+ */
+
+#include "ad5686.h"
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+
+static int ad5686_i2c_read(struct ad5686_state *st, u8 addr)
+{
+	struct i2c_client *i2c = to_i2c_client(st->dev);
+	struct i2c_msg msg[2] = {
+		{
+			.addr = i2c->addr,
+			.flags = i2c->flags,
+			.len = 3,
+			.buf = &st->data[0].d8[1],
+		},
+		{
+			.addr = i2c->addr,
+			.flags = i2c->flags | I2C_M_RD,
+			.len = 2,
+			.buf = (char *)&st->data[0].d16,
+		},
+	};
+	int ret;
+
+	st->data[0].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_NOOP) |
+				      AD5686_ADDR(addr) |
+				      0x00);
+
+	ret = i2c_transfer(i2c->adapter, msg, 2);
+	if (ret < 0)
+		return ret;
+
+	return be16_to_cpu(st->data[0].d16);
+}
+
+static int ad5686_i2c_write(struct ad5686_state *st,
+			    u8 cmd, u8 addr, u16 val)
+{
+	struct i2c_client *i2c = to_i2c_client(st->dev);
+	int ret;
+
+	st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) | AD5686_ADDR(addr)
+				      | val);
+
+	ret = i2c_master_send(i2c, &st->data[0].d8[1], 3);
+	if (ret < 0)
+		return ret;
+
+	return (ret != 3) ? -EIO : 0;
+}
+
+static int ad5686_i2c_probe(struct i2c_client *i2c,
+			    const struct i2c_device_id *id)
+{
+	return ad5686_probe(&i2c->dev, id->driver_data, id->name,
+			    ad5686_i2c_write, ad5686_i2c_read);
+}
+
+static int ad5686_i2c_remove(struct i2c_client *i2c)
+{
+	return ad5686_remove(&i2c->dev);
+}
+
+static const struct i2c_device_id ad5686_i2c_id[] = {
+	{"ad5671r", ID_AD5671R},
+	{"ad5675r", ID_AD5675R},
+	{"ad5694", ID_AD5694},
+	{"ad5694r", ID_AD5694R},
+	{"ad5695r", ID_AD5695R},
+	{"ad5696", ID_AD5696},
+	{"ad5696r", ID_AD5696R},
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, i2c_device_id);
+
+static struct i2c_driver ad5686_i2c_driver = {
+	.driver = {
+		.name = "ad5696",
+	},
+	.probe = ad5686_i2c_probe,
+	.remove = ad5686_i2c_remove,
+	.id_table = ad5686_i2c_id,
+};
+
+module_i2c_driver(ad5686_i2c_driver);
+
+MODULE_AUTHOR("Stefan Popa <stefan.popa@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD5686 and similar multi-channel DACs");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

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

* Re: [PATCH v3 2/7] iio:dac:ad5686: Change license description
  2018-04-11 11:52   ` [PATCH v3 2/7] iio:dac:ad5686: Change license description Stefan Popa
@ 2018-04-15 18:13     ` Jonathan Cameron
  0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2018-04-15 18:13 UTC (permalink / raw)
  To: Stefan Popa
  Cc: Michael.Hennerich, lars, knaack.h, pmeerw, linux-iio, linux-kernel

On Wed, 11 Apr 2018 14:52:12 +0300
Stefan Popa <stefan.popa@analog.com> wrote:

> Add GPLv2+ SPDX identifier and remove license notice to keep the whole
> purpose of using an SPDx id.
> 
> Signed-off-by: Stefan Popa <stefan.popa@analog.com>
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> ---
> Changes in v3:
> 	- Created a new patch to use the SPDx identifier
> 
>  drivers/iio/dac/ad5686.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
> index f7f975c..e328513 100644
> --- a/drivers/iio/dac/ad5686.c
> +++ b/drivers/iio/dac/ad5686.c
> @@ -1,9 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0+
>  /*
>   * AD5686R, AD5685R, AD5684R Digital to analog converters  driver
>   *
>   * Copyright 2011 Analog Devices Inc.
> - *
> - * Licensed under the GPL-2.
>   */
>  
>  #include <linux/interrupt.h>

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

* Re: [PATCH v3 6/7] iio:dac:ad5686: Refactor the driver
  2018-04-11 11:53   ` [PATCH v3 6/7] iio:dac:ad5686: Refactor the driver Stefan Popa
@ 2018-04-15 18:37     ` Jonathan Cameron
  0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2018-04-15 18:37 UTC (permalink / raw)
  To: Stefan Popa
  Cc: Michael.Hennerich, lars, knaack.h, pmeerw, davem, mchehab,
	gregkh, linus.walleij, akpm, rdunlap, Ismail.Kose, lukas,
	fabrice.gasnier, mike.looijmans, pombredanne, linux-iio,
	linux-kernel

On Wed, 11 Apr 2018 14:53:17 +0300
Stefan Popa <stefan.popa@analog.com> wrote:

> In this patch restructures the existing ad5686 driver by adding a module
> for SPI and a header file, while the baseline module deals with the
> chip-logic.
> 
> This is a necessary step, as this driver should support in the future
> similar devices which differ only in the type of interface used (I2C
> instead of SPI).
> 
> Signed-off-by: Stefan Popa <stefan.popa@analog.com>
Applied to the togreg branch of iio.git (carrying forward the change
I made earlier).  Again please check I didn't mess it up.

Jonathan

> ---
> Changes in v2:
> 	- Refactored the patch
> 	- Use st->write directly instead of the ad5686_write() wrapper
> 	- Use st->read directly instead of the ad5686_read() wrapper
> Changes in v3:
> 	- Indented the the help text from the Konfig file with 2
> 	  additional spaces.
> 	- Changed the license description to use an SPDX tag.
> 
>  MAINTAINERS                  |   7 ++
>  drivers/iio/dac/Kconfig      |  13 ++-
>  drivers/iio/dac/Makefile     |   1 +
>  drivers/iio/dac/ad5686-spi.c |  92 +++++++++++++++++++++
>  drivers/iio/dac/ad5686.c     | 190 +++++++------------------------------------
>  drivers/iio/dac/ad5686.h     | 114 ++++++++++++++++++++++++++
>  6 files changed, 252 insertions(+), 165 deletions(-)
>  create mode 100644 drivers/iio/dac/ad5686-spi.c
>  create mode 100644 drivers/iio/dac/ad5686.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 473ac00..637e62d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -791,6 +791,13 @@ M:	Michael Hanselmann <linux-kernel@hansmi.ch>
>  S:	Supported
>  F:	drivers/macintosh/ams/
>  
> +ANALOG DEVICES INC AD5686 DRIVER
> +M:	Stefan Popa <stefan.popa@analog.com>
> +L:	linux-pm@vger.kernel.org
> +W:	http://ez.analog.com/community/linux-device-drivers
> +S:	Supported
> +F:	drivers/iio/dac/ad5686*
> +
>  ANALOG DEVICES INC AD9389B DRIVER
>  M:	Hans Verkuil <hans.verkuil@cisco.com>
>  L:	linux-media@vger.kernel.org
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 965d5c0..7a81f1e 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -131,16 +131,21 @@ config LTC2632
>  	  module will be called ltc2632.
>  
>  config AD5686
> -	tristate "Analog Devices AD5686R/AD5685R/AD5684R DAC SPI driver"
> +	tristate
> +
> +config AD5686_SPI
> +	tristate "Analog Devices AD5686 and similar multi-channel DACs (SPI)"
>  	depends on SPI
> +	select AD5686
>  	help
> -	  Say yes here to build support for Analog Devices AD5686R, AD5685R,
> -	  AD5684R, AD5791 Voltage Output Digital to
> -	  Analog Converter.
> +	  Say yes here to build support for Analog Devices AD5672R, AD5676,
> +	  AD5676R, AD5684, AD5684R, AD5684R, AD5685R, AD5686, AD5686R.
> +	  Voltage Output Digital to Analog Converter.
>  
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ad5686.
>  
> +
>  config AD5755
>  	tristate "Analog Devices AD5755/AD5755-1/AD5757/AD5735/AD5737 DAC driver"
>  	depends on SPI_MASTER
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 81e710e..07db92e 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_AD5761) += ad5761.o
>  obj-$(CONFIG_AD5764) += ad5764.o
>  obj-$(CONFIG_AD5791) += ad5791.o
>  obj-$(CONFIG_AD5686) += ad5686.o
> +obj-$(CONFIG_AD5686_SPI) += ad5686-spi.o
>  obj-$(CONFIG_AD7303) += ad7303.o
>  obj-$(CONFIG_AD8801) += ad8801.o
>  obj-$(CONFIG_CIO_DAC) += cio-dac.o
> diff --git a/drivers/iio/dac/ad5686-spi.c b/drivers/iio/dac/ad5686-spi.c
> new file mode 100644
> index 0000000..1cc807b
> --- /dev/null
> +++ b/drivers/iio/dac/ad5686-spi.c
> @@ -0,0 +1,92 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * AD5672R, AD5676, AD5676R, AD5684, AD5684R, AD5684R, AD5685R, AD5686, AD5686R
> + * Digital to analog converters driver
> + *
> + * Copyright 2018 Analog Devices Inc.
> + */
> +
> +#include "ad5686.h"
> +
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +
> +static int ad5686_spi_write(struct ad5686_state *st,
> +			    u8 cmd, u8 addr, u16 val)
> +{
> +	struct spi_device *spi = to_spi_device(st->dev);
> +
> +	st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) |
> +				      AD5686_ADDR(addr) |
> +				      val);
> +
> +	return spi_write(spi, &st->data[0].d8[1], 3);
> +}
> +
> +static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
> +{
> +	struct spi_transfer t[] = {
> +		{
> +			.tx_buf = &st->data[0].d8[1],
> +			.len = 3,
> +			.cs_change = 1,
> +		}, {
> +			.tx_buf = &st->data[1].d8[1],
> +			.rx_buf = &st->data[2].d8[1],
> +			.len = 3,
> +		},
> +	};
> +	struct spi_device *spi = to_spi_device(st->dev);
> +	int ret;
> +
> +	st->data[0].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_READBACK_ENABLE) |
> +				      AD5686_ADDR(addr));
> +	st->data[1].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_NOOP));
> +
> +	ret = spi_sync_transfer(spi, t, ARRAY_SIZE(t));
> +	if (ret < 0)
> +		return ret;
> +
> +	return be32_to_cpu(st->data[2].d32);
> +}
> +
> +static int ad5686_spi_probe(struct spi_device *spi)
> +{
> +	const struct spi_device_id *id = spi_get_device_id(spi);
> +
> +	return ad5686_probe(&spi->dev, id->driver_data, id->name,
> +			    ad5686_spi_write, ad5686_spi_read);
> +}
> +
> +static int ad5686_spi_remove(struct spi_device *spi)
> +{
> +	return ad5686_remove(&spi->dev);
> +}
> +
> +static const struct spi_device_id ad5686_spi_id[] = {
> +	{"ad5672r", ID_AD5672R},
> +	{"ad5676", ID_AD5676},
> +	{"ad5676r", ID_AD5676R},
> +	{"ad5684", ID_AD5684},
> +	{"ad5684r", ID_AD5684R},
> +	{"ad5685r", ID_AD5685R},
> +	{"ad5686", ID_AD5686},
> +	{"ad5686r", ID_AD5686R},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(spi, ad5686_spi_id);
> +
> +static struct spi_driver ad5686_spi_driver = {
> +	.driver = {
> +		.name = "ad5686",
> +	},
> +	.probe = ad5686_spi_probe,
> +	.remove = ad5686_spi_remove,
> +	.id_table = ad5686_spi_id,
> +};
> +
> +module_spi_driver(ad5686_spi_driver);
> +
> +MODULE_AUTHOR("Stefan Popa <stefan.popa@analog.com>");
> +MODULE_DESCRIPTION("Analog Devices AD5686 and similar multi-channel DACs");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
> index 54f67d5..79abff5 100644
> --- a/drivers/iio/dac/ad5686.c
> +++ b/drivers/iio/dac/ad5686.c
> @@ -10,7 +10,6 @@
>  #include <linux/device.h>
>  #include <linux/module.h>
>  #include <linux/kernel.h>
> -#include <linux/spi/spi.h>
>  #include <linux/slab.h>
>  #include <linux/sysfs.h>
>  #include <linux/regulator/consumer.h>
> @@ -18,121 +17,7 @@
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  
> -#define AD5686_ADDR(x)				((x) << 16)
> -#define AD5686_CMD(x)				((x) << 20)
> -
> -#define AD5686_ADDR_DAC(chan)			(0x1 << (chan))
> -#define AD5686_ADDR_ALL_DAC			0xF
> -
> -#define AD5686_CMD_NOOP				0x0
> -#define AD5686_CMD_WRITE_INPUT_N		0x1
> -#define AD5686_CMD_UPDATE_DAC_N			0x2
> -#define AD5686_CMD_WRITE_INPUT_N_UPDATE_N	0x3
> -#define AD5686_CMD_POWERDOWN_DAC		0x4
> -#define AD5686_CMD_LDAC_MASK			0x5
> -#define AD5686_CMD_RESET			0x6
> -#define AD5686_CMD_INTERNAL_REFER_SETUP		0x7
> -#define AD5686_CMD_DAISY_CHAIN_ENABLE		0x8
> -#define AD5686_CMD_READBACK_ENABLE		0x9
> -
> -#define AD5686_LDAC_PWRDN_NONE			0x0
> -#define AD5686_LDAC_PWRDN_1K			0x1
> -#define AD5686_LDAC_PWRDN_100K			0x2
> -#define AD5686_LDAC_PWRDN_3STATE		0x3
> -
> -/**
> - * struct ad5686_chip_info - chip specific information
> - * @int_vref_mv:	AD5620/40/60: the internal reference voltage
> - * @num_channels:	number of channels
> - * @channel:		channel specification
> -*/
> -
> -struct ad5686_chip_info {
> -	u16				int_vref_mv;
> -	unsigned int			num_channels;
> -	struct iio_chan_spec		*channels;
> -};
> -
> -/**
> - * struct ad5446_state - driver instance specific data
> - * @spi:		spi_device
> - * @chip_info:		chip model specific constants, available modes etc
> - * @reg:		supply regulator
> - * @vref_mv:		actual reference voltage used
> - * @pwr_down_mask:	power down mask
> - * @pwr_down_mode:	current power down mode
> - * @data:		spi transfer buffers
> - */
> -
> -struct ad5686_state {
> -	struct spi_device		*spi;
> -	const struct ad5686_chip_info	*chip_info;
> -	struct regulator		*reg;
> -	unsigned short			vref_mv;
> -	unsigned			pwr_down_mask;
> -	unsigned			pwr_down_mode;
> -	/*
> -	 * DMA (thus cache coherency maintenance) requires the
> -	 * transfer buffers to live in their own cache lines.
> -	 */
> -
> -	union {
> -		__be32 d32;
> -		u8 d8[4];
> -	} data[3] ____cacheline_aligned;
> -};
> -
> -/**
> - * ad5686_supported_device_ids:
> - */
> -
> -enum ad5686_supported_device_ids {
> -	ID_AD5672R,
> -	ID_AD5676,
> -	ID_AD5676R,
> -	ID_AD5684,
> -	ID_AD5684R,
> -	ID_AD5685R,
> -	ID_AD5686,
> -	ID_AD5686R
> -};
> -static int ad5686_spi_write(struct ad5686_state *st,
> -			     u8 cmd, u8 addr, u16 val, u8 shift)
> -{
> -	val <<= shift;
> -
> -	st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) |
> -			      AD5686_ADDR(addr) |
> -			      val);
> -
> -	return spi_write(st->spi, &st->data[0].d8[1], 3);
> -}
> -
> -static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
> -{
> -	struct spi_transfer t[] = {
> -		{
> -			.tx_buf = &st->data[0].d8[1],
> -			.len = 3,
> -			.cs_change = 1,
> -		}, {
> -			.tx_buf = &st->data[1].d8[1],
> -			.rx_buf = &st->data[2].d8[1],
> -			.len = 3,
> -		},
> -	};
> -	int ret;
> -
> -	st->data[0].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_READBACK_ENABLE) |
> -			      AD5686_ADDR(addr));
> -	st->data[1].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_NOOP));
> -
> -	ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
> -	if (ret < 0)
> -		return ret;
> -
> -	return be32_to_cpu(st->data[2].d32);
> -}
> +#include "ad5686.h"
>  
>  static const char * const ad5686_powerdown_modes[] = {
>  	"1kohm_to_gnd",
> @@ -195,8 +80,9 @@ static ssize_t ad5686_write_dac_powerdown(struct iio_dev *indio_dev,
>  	else
>  		st->pwr_down_mask &= ~(0x3 << (chan->channel * 2));
>  
> -	ret = ad5686_spi_write(st, AD5686_CMD_POWERDOWN_DAC, 0,
> -			       st->pwr_down_mask & st->pwr_down_mode, 0);
> +	ret = st->write(st, AD5686_CMD_POWERDOWN_DAC, 0,
> +			st->pwr_down_mask & st->pwr_down_mode);
> +
>  
>  	return ret ? ret : len;
>  }
> @@ -213,7 +99,7 @@ static int ad5686_read_raw(struct iio_dev *indio_dev,
>  	switch (m) {
>  	case IIO_CHAN_INFO_RAW:
>  		mutex_lock(&indio_dev->mlock);
> -		ret = ad5686_spi_read(st, chan->address);
> +		ret = st->read(st, chan->address);
>  		mutex_unlock(&indio_dev->mlock);
>  		if (ret < 0)
>  			return ret;
> @@ -242,11 +128,10 @@ static int ad5686_write_raw(struct iio_dev *indio_dev,
>  			return -EINVAL;
>  
>  		mutex_lock(&indio_dev->mlock);
> -		ret = ad5686_spi_write(st,
> -				 AD5686_CMD_WRITE_INPUT_N_UPDATE_N,
> -				 chan->address,
> -				 val,
> -				 chan->scan_type.shift);
> +		ret = st->write(st,
> +				AD5686_CMD_WRITE_INPUT_N_UPDATE_N,
> +				chan->address,
> +				val << chan->scan_type.shift);
>  		mutex_unlock(&indio_dev->mlock);
>  		break;
>  	default:
> @@ -356,20 +241,27 @@ static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
>  	},
>  };
>  
> -static int ad5686_probe(struct spi_device *spi)
> +int ad5686_probe(struct device *dev,
> +		 enum ad5686_supported_device_ids chip_type,
> +		 const char *name, ad5686_write_func write,
> +		 ad5686_read_func read)
>  {
>  	struct ad5686_state *st;
>  	struct iio_dev *indio_dev;
>  	int ret, voltage_uv = 0;
>  
> -	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
>  	if (indio_dev == NULL)
>  		return  -ENOMEM;
>  
>  	st = iio_priv(indio_dev);
> -	spi_set_drvdata(spi, indio_dev);
> +	dev_set_drvdata(dev, indio_dev);
> +
> +	st->dev = dev;
> +	st->write = write;
> +	st->read = read;
>  
> -	st->reg = devm_regulator_get_optional(&spi->dev, "vcc");
> +	st->reg = devm_regulator_get_optional(dev, "vcc");
>  	if (!IS_ERR(st->reg)) {
>  		ret = regulator_enable(st->reg);
>  		if (ret)
> @@ -382,28 +274,25 @@ static int ad5686_probe(struct spi_device *spi)
>  		voltage_uv = ret;
>  	}
>  
> -	st->chip_info =
> -		&ad5686_chip_info_tbl[spi_get_device_id(spi)->driver_data];
> +	st->chip_info = &ad5686_chip_info_tbl[chip_type];
>  
>  	if (voltage_uv)
>  		st->vref_mv = voltage_uv / 1000;
>  	else
>  		st->vref_mv = st->chip_info->int_vref_mv;
>  
> -	st->spi = spi;
> -
>  	/* Set all the power down mode for all channels to 1K pulldown */
>  	st->pwr_down_mode = 0x55;
>  
> -	indio_dev->dev.parent = &spi->dev;
> -	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->dev.parent = dev;
> +	indio_dev->name = name;
>  	indio_dev->info = &ad5686_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	indio_dev->channels = st->chip_info->channels;
>  	indio_dev->num_channels = st->chip_info->num_channels;
>  
> -	ret = ad5686_spi_write(st, AD5686_CMD_INTERNAL_REFER_SETUP, 0,
> -				!!voltage_uv, 0);
> +	ret = st->write(st, AD5686_CMD_INTERNAL_REFER_SETUP,
> +			0, !!voltage_uv);
>  	if (ret)
>  		goto error_disable_reg;
>  
> @@ -418,10 +307,11 @@ static int ad5686_probe(struct spi_device *spi)
>  		regulator_disable(st->reg);
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL(ad5686_probe);
>  
> -static int ad5686_remove(struct spi_device *spi)
> +int ad5686_remove(struct device *dev)
>  {
> -	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5686_state *st = iio_priv(indio_dev);
>  
>  	iio_device_unregister(indio_dev);
> @@ -430,29 +320,7 @@ static int ad5686_remove(struct spi_device *spi)
>  
>  	return 0;
>  }
> -
> -static const struct spi_device_id ad5686_id[] = {
> -	{"ad5672r", ID_AD5672R},
> -	{"ad5676", ID_AD5676},
> -	{"ad5676r", ID_AD5676R},
> -	{"ad5684", ID_AD5684},
> -	{"ad5684r", ID_AD5684R},
> -	{"ad5685r", ID_AD5685R},
> -	{"ad5686", ID_AD5686},
> -	{"ad5686r", ID_AD5686R},
> -	{}
> -};
> -MODULE_DEVICE_TABLE(spi, ad5686_id);
> -
> -static struct spi_driver ad5686_driver = {
> -	.driver = {
> -		   .name = "ad5686",
> -		   },
> -	.probe = ad5686_probe,
> -	.remove = ad5686_remove,
> -	.id_table = ad5686_id,
> -};
> -module_spi_driver(ad5686_driver);
> +EXPORT_SYMBOL_GPL(ad5686_remove);
>  
>  MODULE_AUTHOR("Michael Hennerich <hennerich@blackfin.uclinux.org>");
>  MODULE_DESCRIPTION("Analog Devices AD5686/85/84 DAC");
> diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
> new file mode 100644
> index 0000000..c8e1565
> --- /dev/null
> +++ b/drivers/iio/dac/ad5686.h
> @@ -0,0 +1,114 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * This file is part of AD5686 DAC driver
> + *
> + * Copyright 2018 Analog Devices Inc.
> + */
> +
> +#ifndef __DRIVERS_IIO_DAC_AD5686_H__
> +#define __DRIVERS_IIO_DAC_AD5686_H__
> +
> +#include <linux/types.h>
> +#include <linux/cache.h>
> +#include <linux/mutex.h>
> +#include <linux/kernel.h>
> +
> +#define AD5686_ADDR(x)				((x) << 16)
> +#define AD5686_CMD(x)				((x) << 20)
> +
> +#define AD5686_ADDR_DAC(chan)			(0x1 << (chan))
> +#define AD5686_ADDR_ALL_DAC			0xF
> +
> +#define AD5686_CMD_NOOP				0x0
> +#define AD5686_CMD_WRITE_INPUT_N		0x1
> +#define AD5686_CMD_UPDATE_DAC_N			0x2
> +#define AD5686_CMD_WRITE_INPUT_N_UPDATE_N	0x3
> +#define AD5686_CMD_POWERDOWN_DAC		0x4
> +#define AD5686_CMD_LDAC_MASK			0x5
> +#define AD5686_CMD_RESET			0x6
> +#define AD5686_CMD_INTERNAL_REFER_SETUP		0x7
> +#define AD5686_CMD_DAISY_CHAIN_ENABLE		0x8
> +#define AD5686_CMD_READBACK_ENABLE		0x9
> +
> +#define AD5686_LDAC_PWRDN_NONE			0x0
> +#define AD5686_LDAC_PWRDN_1K			0x1
> +#define AD5686_LDAC_PWRDN_100K			0x2
> +#define AD5686_LDAC_PWRDN_3STATE		0x3
> +
> +/**
> + * ad5686_supported_device_ids:
> + */
> +enum ad5686_supported_device_ids {
> +	ID_AD5672R,
> +	ID_AD5676,
> +	ID_AD5676R,
> +	ID_AD5684,
> +	ID_AD5684R,
> +	ID_AD5685R,
> +	ID_AD5686,
> +	ID_AD5686R,
> +};
> +
> +struct ad5686_state;
> +
> +typedef int (*ad5686_write_func)(struct ad5686_state *st,
> +				 u8 cmd, u8 addr, u16 val);
> +
> +typedef int (*ad5686_read_func)(struct ad5686_state *st, u8 addr);
> +
> +/**
> + * struct ad5686_chip_info - chip specific information
> + * @int_vref_mv:	AD5620/40/60: the internal reference voltage
> + * @num_channels:	number of channels
> + * @channel:		channel specification
> + */
> +
> +struct ad5686_chip_info {
> +	u16				int_vref_mv;
> +	unsigned int			num_channels;
> +	struct iio_chan_spec		*channels;
> +};
> +
> +/**
> + * struct ad5446_state - driver instance specific data
> + * @spi:		spi_device
> + * @chip_info:		chip model specific constants, available modes etc
> + * @reg:		supply regulator
> + * @vref_mv:		actual reference voltage used
> + * @pwr_down_mask:	power down mask
> + * @pwr_down_mode:	current power down mode
> + * @data:		spi transfer buffers
> + */
> +
> +struct ad5686_state {
> +	struct device			*dev;
> +	const struct ad5686_chip_info	*chip_info;
> +	struct regulator		*reg;
> +	unsigned short			vref_mv;
> +	unsigned int			pwr_down_mask;
> +	unsigned int			pwr_down_mode;
> +	ad5686_write_func		write;
> +	ad5686_read_func		read;
> +
> +	/*
> +	 * DMA (thus cache coherency maintenance) requires the
> +	 * transfer buffers to live in their own cache lines.
> +	 */
> +
> +	union {
> +		__be32 d32;
> +		__be16 d16;
> +		u8 d8[4];
> +	} data[3] ____cacheline_aligned;
> +};
> +
> +
> +int ad5686_probe(struct device *dev,
> +		 enum ad5686_supported_device_ids chip_type,
> +		 const char *name, ad5686_write_func write,
> +		 ad5686_read_func read);
> +
> +int ad5686_remove(struct device *dev);
> +
> +
> +#endif /* __DRIVERS_IIO_DAC_AD5686_H__ */

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

* Re: [PATCH v3 7/7] iio:dac:ad5686: Add AD5671R/75R/94/94R/95R/96/96R support
  2018-04-11 11:53   ` [PATCH v3 7/7] iio:dac:ad5686: Add AD5671R/75R/94/94R/95R/96/96R support Stefan Popa
@ 2018-04-15 19:09     ` Jonathan Cameron
  0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2018-04-15 19:09 UTC (permalink / raw)
  To: Stefan Popa
  Cc: Michael.Hennerich, lars, knaack.h, pmeerw, davem, mchehab,
	gregkh, linus.walleij, akpm, rdunlap, Ismail.Kose, lukas,
	fabrice.gasnier, mike.looijmans, kstewart, linux-iio,
	linux-kernel

On Wed, 11 Apr 2018 14:53:39 +0300
Stefan Popa <stefan.popa@analog.com> wrote:

> The AD5694/AD5694R/AD5695R/AD5696/AD5696R are a family of 4 channel DACs
> with 12-bit, 14-bit and 16-bit precision respectively. The devices have
> either no built-in reference, or built-in 2.5V reference.
> 
> The AD5671R/AD5675R are similar, except that they have 8 instead of 4
> channels.
> 
> These devices are similar to AD5672R/AD5676/AD5676R and
> AD5684/AD5684R/AD5684/AD5685R/AD5686/AD5686R, except that they use i2c
> instead of spi.
> 
> Datasheets:
> http://www.analog.com/media/en/technical-documentation/data-sheets/AD5671R_5675R.pdf
> http://www.analog.com/media/en/technical-documentation/data-sheets/AD5696R_5695R_5694R.pdf
> 
> Signed-off-by: Stefan Popa <stefan.popa@analog.com>
There is one bug inline that stops it building.  I've fixed up and
applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> ---
> Changes in v2:
> 	- Refactored the patch
> Changes in v3:
> 	- Indented the the help text from the Konfig file with 2
> 	  additional spaces.
> 	- Changed the license description to use an SPDX tag.
> 
>  MAINTAINERS                  |  1 +
>  drivers/iio/dac/Kconfig      | 10 +++++
>  drivers/iio/dac/Makefile     |  1 +
>  drivers/iio/dac/ad5686.c     | 28 +++++++++++++
>  drivers/iio/dac/ad5686.h     |  7 ++++
>  drivers/iio/dac/ad5696-i2c.c | 97 ++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 144 insertions(+)
>  create mode 100644 drivers/iio/dac/ad5696-i2c.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 637e62d..002cb01 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -797,6 +797,7 @@ L:	linux-pm@vger.kernel.org
>  W:	http://ez.analog.com/community/linux-device-drivers
>  S:	Supported
>  F:	drivers/iio/dac/ad5686*
> +F:	drivers/iio/dac/ad5696*
>  
>  ANALOG DEVICES INC AD9389B DRIVER
>  M:	Hans Verkuil <hans.verkuil@cisco.com>
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 7a81f1e..3ff8a32 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -145,6 +145,16 @@ config AD5686_SPI
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ad5686.
>  
> +config AD5696_I2C
> +	tristate "Analog Devices AD5696 and similar multi-channel DACs (I2C)"
> +	depends on I2C
> +	select AD5686
> +	help
> +	  Say yes here to build support for Analog Devices AD5671R, AD5675R,
> +	  AD5694, AD5694R, AD5695R, AD5696, AD5696R Voltage Output Digital to
> +	  Analog Converter.
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called ad5696.
>  
>  config AD5755
>  	tristate "Analog Devices AD5755/AD5755-1/AD5757/AD5735/AD5737 DAC driver"
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 07db92e..4397e21 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_AD5764) += ad5764.o
>  obj-$(CONFIG_AD5791) += ad5791.o
>  obj-$(CONFIG_AD5686) += ad5686.o
>  obj-$(CONFIG_AD5686_SPI) += ad5686-spi.o
> +obj-$(CONFIG_AD5696_I2C) += ad5696-i2c.o
>  obj-$(CONFIG_AD7303) += ad7303.o
>  obj-$(CONFIG_AD8801) += ad8801.o
>  obj-$(CONFIG_CIO_DAC) += cio-dac.o
> diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
> index 79abff5..89c5f08 100644
> --- a/drivers/iio/dac/ad5686.c
> +++ b/drivers/iio/dac/ad5686.c
> @@ -202,11 +202,21 @@ DECLARE_AD5686_CHANNELS(ad5685r_channels, 14, 2);
>  DECLARE_AD5686_CHANNELS(ad5686_channels, 16, 0);
>  
>  static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
> +	[ID_AD5671R] = {
> +		.channels = ad5672_channels,
> +		.int_vref_mv = 2500,
> +		.num_channels = 8,
> +	},
>  	[ID_AD5672R] = {
>  		.channels = ad5672_channels,
>  		.int_vref_mv = 2500,
>  		.num_channels = 8,
>  	},
> +	[ID_AD5675R] = {
> +		.channels = ad5676_channels,
> +		.int_vref_mv = 2500,
> +		.num_channels = 8,
> +	},
>  	[ID_AD5676] = {
>  		.channels = ad5676_channels,
>  		.num_channels = 8,
> @@ -239,6 +249,24 @@ static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
>  		.int_vref_mv = 2500,
>  		.num_channels = 4,
>  	},
> +	[ID_AD5694] = {
> +		.channels = ad5684_channels,
> +		.num_channels = 4,
> +	},
> +	[ID_AD5694R] = {
> +		.channels = ad5684_channels,
> +		.int_vref_mv = 2500,
> +		.num_channels = 4,
> +	},
> +	[ID_AD5696] = {
> +		.channels = ad5686_channels,
> +		.num_channels = 4,
> +	},
> +	[ID_AD5696R] = {
> +		.channels = ad5686_channels,
> +		.int_vref_mv = 2500,
> +		.num_channels = 4,
> +	},
>  };
>  
>  int ad5686_probe(struct device *dev,
> diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
> index c8e1565..05f0ce9 100644
> --- a/drivers/iio/dac/ad5686.h
> +++ b/drivers/iio/dac/ad5686.h
> @@ -39,7 +39,9 @@
>   * ad5686_supported_device_ids:
>   */
>  enum ad5686_supported_device_ids {
> +	ID_AD5671R,
>  	ID_AD5672R,
> +	ID_AD5675R,
>  	ID_AD5676,
>  	ID_AD5676R,
>  	ID_AD5684,
> @@ -47,6 +49,11 @@ enum ad5686_supported_device_ids {
>  	ID_AD5685R,
>  	ID_AD5686,
>  	ID_AD5686R,
> +	ID_AD5694,
> +	ID_AD5694R,
> +	ID_AD5695R,
> +	ID_AD5696,
> +	ID_AD5696R,
>  };
>  
>  struct ad5686_state;
> diff --git a/drivers/iio/dac/ad5696-i2c.c b/drivers/iio/dac/ad5696-i2c.c
> new file mode 100644
> index 0000000..088cb36
> --- /dev/null
> +++ b/drivers/iio/dac/ad5696-i2c.c
> @@ -0,0 +1,97 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * AD5671R, AD5675R, AD5694, AD5694R, AD5695R, AD5696, AD5696R
> + * Digital to analog converters driver
> + *
> + * Copyright 2018 Analog Devices Inc.
> + */
> +
> +#include "ad5686.h"
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +
> +static int ad5686_i2c_read(struct ad5686_state *st, u8 addr)
> +{
> +	struct i2c_client *i2c = to_i2c_client(st->dev);
> +	struct i2c_msg msg[2] = {
> +		{
> +			.addr = i2c->addr,
> +			.flags = i2c->flags,
> +			.len = 3,
> +			.buf = &st->data[0].d8[1],
> +		},
> +		{
> +			.addr = i2c->addr,
> +			.flags = i2c->flags | I2C_M_RD,
> +			.len = 2,
> +			.buf = (char *)&st->data[0].d16,
> +		},
> +	};
> +	int ret;
> +
> +	st->data[0].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_NOOP) |
> +				      AD5686_ADDR(addr) |
> +				      0x00);
> +
> +	ret = i2c_transfer(i2c->adapter, msg, 2);
> +	if (ret < 0)
> +		return ret;
> +
> +	return be16_to_cpu(st->data[0].d16);
> +}
> +
> +static int ad5686_i2c_write(struct ad5686_state *st,
> +			    u8 cmd, u8 addr, u16 val)
> +{
> +	struct i2c_client *i2c = to_i2c_client(st->dev);
> +	int ret;
> +
> +	st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) | AD5686_ADDR(addr)
> +				      | val);
> +
> +	ret = i2c_master_send(i2c, &st->data[0].d8[1], 3);
> +	if (ret < 0)
> +		return ret;
> +
> +	return (ret != 3) ? -EIO : 0;
> +}
> +
> +static int ad5686_i2c_probe(struct i2c_client *i2c,
> +			    const struct i2c_device_id *id)
> +{
> +	return ad5686_probe(&i2c->dev, id->driver_data, id->name,
> +			    ad5686_i2c_write, ad5686_i2c_read);
> +}
> +
> +static int ad5686_i2c_remove(struct i2c_client *i2c)
> +{
> +	return ad5686_remove(&i2c->dev);
> +}
> +
> +static const struct i2c_device_id ad5686_i2c_id[] = {
> +	{"ad5671r", ID_AD5671R},
> +	{"ad5675r", ID_AD5675R},
> +	{"ad5694", ID_AD5694},
> +	{"ad5694r", ID_AD5694R},
> +	{"ad5695r", ID_AD5695R},
> +	{"ad5696", ID_AD5696},
> +	{"ad5696r", ID_AD5696R},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, i2c_device_id);
This worries me a little as it won't build...  You've
specified the type, rather than the instance.

Anyhow, I've fixed that.

Jonathan

> +
> +static struct i2c_driver ad5686_i2c_driver = {
> +	.driver = {
> +		.name = "ad5696",
> +	},
> +	.probe = ad5686_i2c_probe,
> +	.remove = ad5686_i2c_remove,
> +	.id_table = ad5686_i2c_id,
> +};
> +
> +module_i2c_driver(ad5686_i2c_driver);
> +
> +MODULE_AUTHOR("Stefan Popa <stefan.popa@analog.com>");
> +MODULE_DESCRIPTION("Analog Devices AD5686 and similar multi-channel DACs");
> +MODULE_LICENSE("GPL v2");

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

end of thread, other threads:[~2018-04-15 19:09 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-30 13:56 [PATCH 3/3] iio:dac:ad5686: Add AD5671R/75R/94/94R/95R/96/96R support Stefan Popa
2018-04-06 15:32 ` Jonathan Cameron
2018-04-10 15:58 ` [PATCH v2 5/6] iio:dac:ad5686: Refactor the driver Stefan Popa
2018-04-10 15:58 ` [PATCH v2 6/6] iio:dac:ad5686: Add AD5671R/75R/94/94R/95R/96/96R support Stefan Popa
2018-04-10 16:19   ` Randy Dunlap
2018-04-11 11:52   ` [PATCH v3 2/7] iio:dac:ad5686: Change license description Stefan Popa
2018-04-15 18:13     ` Jonathan Cameron
2018-04-11 11:53   ` [PATCH v3 6/7] iio:dac:ad5686: Refactor the driver Stefan Popa
2018-04-15 18:37     ` Jonathan Cameron
2018-04-11 11:53   ` [PATCH v3 7/7] iio:dac:ad5686: Add AD5671R/75R/94/94R/95R/96/96R support Stefan Popa
2018-04-15 19:09     ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).