public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] iio: add ad5761 DAC driver
@ 2015-12-16 13:27 Ricardo Ribalda Delgado
  2015-12-19 16:31 ` Jonathan Cameron
  2015-12-19 17:06 ` Peter Meerwald-Stadler
  0 siblings, 2 replies; 8+ messages in thread
From: Ricardo Ribalda Delgado @ 2015-12-16 13:27 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Hartmut Knaack, Peter Meerwald, linux-iio, linux-kernel
  Cc: Ricardo Ribalda Delgado

ad5761 is a 1-channel DAC with configurable output range.
The driver uses the regulator interface for its voltage ref.

It shares its register layout with ad5761r, ad5721 and ad5721r.

Differences:
ad5761* are 16 bit, ad5721* are 12 bits.
ad57*1r have an internal reference.

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 CREDITS                  |   1 +
 drivers/iio/dac/Kconfig  |  10 ++
 drivers/iio/dac/Makefile |   1 +
 drivers/iio/dac/ad5761.c | 425 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 437 insertions(+)
 create mode 100644 drivers/iio/dac/ad5761.c

diff --git a/CREDITS b/CREDITS
index 8207cc62ee9d..44ea433d70a1 100644
--- a/CREDITS
+++ b/CREDITS
@@ -3035,6 +3035,7 @@ D: PLX USB338x driver
 D: PCA9634 driver
 D: Option GTM671WFS
 D: Fintek F81216A
+D: AD5761 iio driver
 D: Various kernel hacks
 S: Qtechnology A/S
 S: Valby Langgade 142
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index e701e28fb1cd..4caedd671360 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -111,6 +111,16 @@ config AD5755
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad5755.
 
+config AD5761
+	tristate "Analog Devices AD5761/61R/21/21R DAC driver"
+	depends on SPI_MASTER
+	help
+	  Say yes here to build support for Analog Devices AD5761, AD5761R, AD5721,
+	  AD5721R Digital to Analog Converter.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad5761.
+
 config AD5764
 	tristate "Analog Devices AD5764/64R/44/44R DAC driver"
 	depends on SPI_MASTER
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 63ae05633e0c..cb525b53fc7b 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_AD5504) += ad5504.o
 obj-$(CONFIG_AD5446) += ad5446.o
 obj-$(CONFIG_AD5449) += ad5449.o
 obj-$(CONFIG_AD5755) += ad5755.o
+obj-$(CONFIG_AD5761) += ad5761.o
 obj-$(CONFIG_AD5764) += ad5764.o
 obj-$(CONFIG_AD5791) += ad5791.o
 obj-$(CONFIG_AD5686) += ad5686.o
diff --git a/drivers/iio/dac/ad5761.c b/drivers/iio/dac/ad5761.c
new file mode 100644
index 000000000000..b4eac59ef81f
--- /dev/null
+++ b/drivers/iio/dac/ad5761.c
@@ -0,0 +1,425 @@
+/*
+ * AD5721, AD5721R, AD5761, AD5761R, Voltage Output Digital to Analog Converter
+ *
+ * Copyright 2015 Qtechnology A/S
+ *
+ * Licensed under the GPL-2.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+#include <linux/bitops.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/regulator/consumer.h>
+
+#define AD5761_ADDR(addr)		((addr&0xf) << 16)
+#define AD5761_ADDR_NOOP		0x0
+#define AD5761_ADDR_DAC_WRITE		0x3
+#define AD5761_ADDR_CTRL_WRITE_REG	0x4
+#define AD5761_ADDR_SW_DATA_RESET	0x7
+#define AD5761_ADDR_DAC_READ		0xb
+#define AD5761_ADDR_CTRL_READ_REG	0xc
+#define AD5761_ADDR_SW_FULL_RESET	0xf
+
+#define AD5761_CTRL_USE_INTVREF		BIT(5)
+#define AD5761_CTRL_ETS			BIT(6)
+
+/**
+ * struct ad5761_chip_info - chip specific information
+ * @int_vref:	Value of the internal reference voltage in mV - 0 if external
+ *		reference voltage is used
+ * @channel	channel specification
+*/
+
+struct ad5761_chip_info {
+	unsigned long int_vref;
+	const struct iio_chan_spec channel;
+};
+
+struct ad5761_range_params {
+	int m;
+	int c;
+};
+
+/**
+ * ad5761_supported_device_ids:
+ */
+
+enum ad5761_supported_device_ids {
+	ID_AD5721,
+	ID_AD5721R,
+	ID_AD5761,
+	ID_AD5761R,
+};
+
+/**
+ * struct ad5761_state - driver instance specific data
+ * @spi:		spi_device
+ * @chip_info:		chip model specific constants
+ * @vref_mv:		actual reference voltage used
+ */
+struct ad5761_state {
+	struct spi_device		*spi;
+	struct regulator		*vref_reg;
+
+	bool use_intref;
+	int vref;
+	int range;
+
+	/*
+	 * 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;
+};
+
+enum ad5761_range_ids {
+	MODE_M10V_10V,
+	MODE_0V_10V,
+	MODE_M5V_5V,
+	MODE_0V_5V,
+	MODE_M2V5_7V5,
+	MODE_M3V_3V,
+	MODE_0V_16V,
+	MODE_0V_20V,
+};
+
+static const struct ad5761_range_params ad5761_range_params[] = {
+	[MODE_M10V_10V] = {
+		.m = 80,
+		.c = 40,
+	},
+	[MODE_0V_10V] = {
+		.m = 40,
+		.c = 0,
+	},
+	[MODE_M5V_5V] = {
+		.m = 40,
+		.c = 20,
+	},
+	[MODE_0V_5V] = {
+		.m = 20,
+		.c = 0,
+	},
+	[MODE_M2V5_7V5] = {
+		.m = 40,
+		.c = 10,
+	},
+	[MODE_M3V_3V] = {
+		.m = 24,
+		.c = 12,
+	},
+	[MODE_0V_16V] = {
+		.m = 64,
+		.c = 0,
+	},
+	[MODE_0V_20V] = {
+		.m = 80,
+		.c = 0,
+	},
+};
+
+static const char * const ad5761_ranges[] = {
+	[MODE_M10V_10V] = "-10V_10V",
+	[MODE_0V_10V] = "0V_10V",
+	[MODE_M5V_5V] = "-5V_5V",
+	[MODE_0V_5V] = "0V_5V",
+	[MODE_M2V5_7V5] = "-2V5_7V5",
+	[MODE_M3V_3V] = "-3V_3V",
+	[MODE_0V_16V] = "0V_16V",
+	[MODE_0V_20V] = "0V_20V",
+};
+
+static int ad5761_spi_write(struct ad5761_state *st, u8 addr, u16 val)
+{
+	st->data[0].d32 = cpu_to_be32(AD5761_ADDR(addr) | val);
+
+	return spi_write(st->spi, &st->data[0].d8[1], 3);
+}
+
+static int ad5761_spi_read(struct ad5761_state *st, u8 addr, u16 *val)
+{
+	int ret;
+	struct spi_transfer xfers[] = {
+		{
+			.tx_buf = &st->data[0].d8[1],
+			.bits_per_word = 8,
+			.len = 3,
+			.cs_change = true,
+		}, {
+			.tx_buf = &st->data[1].d8[1],
+			.rx_buf = &st->data[2].d8[1],
+			.bits_per_word = 8,
+			.len = 3,
+		},
+	};
+
+	st->data[0].d32 = cpu_to_be32(AD5761_ADDR(addr));
+	st->data[1].d32 = cpu_to_be32(AD5761_ADDR(AD5761_ADDR_NOOP));
+
+	ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
+
+	*val = be32_to_cpu(st->data[2].d32);
+
+	return ret;
+}
+
+static int ad5761_spi_set_range(struct ad5761_state *st, int range)
+{
+	u16 aux;
+	int ret;
+
+	aux = range;
+	aux |= AD5761_CTRL_ETS;
+	if (st->use_intref)
+		aux |= AD5761_CTRL_USE_INTVREF;
+	ret = ad5761_spi_write(st, AD5761_ADDR_CTRL_WRITE_REG, aux);
+	if (ret)
+		return ret;
+	ret = ad5761_spi_write(st, AD5761_ADDR_SW_DATA_RESET, 0);
+	if (ret)
+		return ret;
+
+	st->range = range;
+	return 0;
+}
+
+static int ad5761_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val,
+			   int *val2,
+			   long m)
+{
+	struct ad5761_state *st = iio_priv(indio_dev);
+	int ret;
+	u16 aux;
+
+	switch (m) {
+	case IIO_CHAN_INFO_RAW:
+		ret = ad5761_spi_read(st, AD5761_ADDR_DAC_READ, &aux);
+		if (ret)
+			return ret;
+		*val = aux >> chan->scan_type.shift;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = st->vref * ad5761_range_params[st->range].m;
+		*val /= 10;
+		*val2 = chan->scan_type.realbits;
+		return IIO_VAL_FRACTIONAL_LOG2;
+	case IIO_CHAN_INFO_OFFSET:
+		*val = -(1<<chan->scan_type.realbits);
+		*val *=	ad5761_range_params[st->range].c;
+		*val /=	ad5761_range_params[st->range].m;
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+
+}
+
+static int ad5761_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val,
+			    int val2,
+			    long mask)
+{
+	struct ad5761_state *st = iio_priv(indio_dev);
+	u16 aux;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		aux =  val << chan->scan_type.shift;
+		return ad5761_spi_write(st, AD5761_ADDR_DAC_WRITE, aux);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad5761_set_range(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, unsigned int range)
+{
+	struct ad5761_state *st = iio_priv(indio_dev);
+
+	return ad5761_spi_set_range(st, range);
+}
+
+static int ad5761_get_range(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan)
+{
+	struct ad5761_state *st = iio_priv(indio_dev);
+	u16 aux;
+	int ret;
+
+	ret = ad5761_spi_read(st, AD5761_ADDR_CTRL_READ_REG, &aux);
+	if (ret)
+		return ret;
+
+	return aux & 0x7;
+}
+
+static const struct iio_info ad5761_info = {
+	.read_raw = &ad5761_read_raw,
+	.write_raw = &ad5761_write_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static const struct iio_enum ad5761_range_enum = {
+	.items = ad5761_ranges,
+	.num_items = ARRAY_SIZE(ad5761_ranges),
+	.get = ad5761_get_range,
+	.set = ad5761_set_range,
+};
+
+static struct iio_chan_spec_ext_info ad5761_ext_info[] = {
+	IIO_ENUM("range", IIO_SHARED_BY_TYPE,
+		 &ad5761_range_enum),
+	IIO_ENUM_AVAILABLE("range", &ad5761_range_enum),
+	{ },
+};
+
+#define AD5761_CHAN(_bits) {				\
+	.type = IIO_VOLTAGE,				\
+	.output = 1,					\
+	.indexed = 1,					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
+		BIT(IIO_CHAN_INFO_OFFSET),		\
+	.scan_type = {					\
+		.sign = 'u',				\
+		.realbits = (_bits),			\
+		.storagebits = 16,			\
+		.shift = 16 - (_bits),			\
+	},						\
+	.ext_info = ad5761_ext_info,			\
+}
+
+static const struct ad5761_chip_info ad5761_chip_infos[] = {
+	[ID_AD5721] = {
+		.int_vref = 0,
+		.channel = AD5761_CHAN(12),
+	},
+	[ID_AD5721R] = {
+		.int_vref = 2500,
+		.channel = AD5761_CHAN(12),
+	},
+	[ID_AD5761] = {
+		.int_vref = 0,
+		.channel = AD5761_CHAN(16),
+	},
+	[ID_AD5761R] = {
+		.int_vref = 2500,
+		.channel = AD5761_CHAN(16),
+	},
+};
+
+static void ad5761_get_vref(struct ad5761_state *st)
+{
+	int ret;
+
+
+	st->vref_reg = devm_regulator_get(&st->spi->dev, "vref");
+	if (IS_ERR(st->vref_reg))
+		return;
+
+	ret = regulator_enable(st->vref_reg);
+	if (ret) {
+		dev_warn(&st->spi->dev, "Failed to enable vref. Using internal");
+		return;
+	}
+
+	ret = regulator_get_voltage(st->vref_reg);
+	if (ret < 0) {
+		regulator_disable(st->vref_reg);
+		dev_warn(&st->spi->dev,
+			 "Failed to get vref value. Using internal");
+		return;
+	}
+
+	if (ret < 2000000 || ret > 3000000) {
+		regulator_disable(st->vref_reg);
+		dev_warn(&st->spi->dev,
+				"Invalid external vref value. Using internal");
+		return;
+	}
+
+	st->vref = ret / 1000;
+	st->use_intref = false;
+}
+
+static int ad5761_probe(struct spi_device *spi)
+{
+	struct iio_dev *iio_dev;
+	struct ad5761_state *st;
+	int ret;
+	const struct ad5761_chip_info *chip_info =
+		&ad5761_chip_infos[spi_get_device_id(spi)->driver_data];
+
+	iio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!iio_dev)
+		return -ENOMEM;
+	st = iio_priv(iio_dev);
+
+	st->spi = spi;
+	spi_set_drvdata(spi, iio_dev);
+
+	st->use_intref = true;
+	st->vref = chip_info->int_vref;
+	ad5761_get_vref(st);
+	if (st->use_intref && !chip_info->int_vref) {
+		dev_err(&spi->dev, "Missing vref, cannot continue");
+		return -EIO;
+	}
+
+	ad5761_spi_set_range(st, MODE_0V_5V);
+
+	iio_dev->dev.parent = &spi->dev;
+	iio_dev->info = &ad5761_info;
+	iio_dev->modes = INDIO_DIRECT_MODE;
+	iio_dev->channels = &chip_info->channel;
+	iio_dev->num_channels = 1;
+	iio_dev->name = spi_get_device_id(st->spi)->name;
+	ret = iio_device_register(iio_dev);
+	if (ret && !IS_ERR(st->vref_reg))
+		regulator_disable(st->vref_reg);
+
+	return ret;
+}
+
+static int ad5761_remove(struct spi_device *spi)
+{
+	struct iio_dev *iio_dev = spi_get_drvdata(spi);
+	struct ad5761_state *st = iio_priv(iio_dev);
+
+	if (!IS_ERR(st->vref_reg))
+		regulator_disable(st->vref_reg);
+	iio_device_unregister(iio_dev);
+	return 0;
+}
+
+static const struct spi_device_id ad5761_id[] = {
+	{"ad5721", ID_AD5721},
+	{"ad5721r", ID_AD5721R},
+	{"ad5761", ID_AD5761},
+	{"ad5761r", ID_AD5761R},
+	{}
+};
+MODULE_DEVICE_TABLE(spi, ad5761_id);
+
+static struct spi_driver ad5761_driver = {
+	.driver = {
+		   .name = "ad5761",
+		   .owner = THIS_MODULE,
+		   },
+	.probe = ad5761_probe,
+	.remove = ad5761_remove,
+	.id_table = ad5761_id,
+};
+module_spi_driver(ad5761_driver);
+
+MODULE_AUTHOR("Ricardo Ribalda <ricardo.ribalda@gmail.com>");
+MODULE_DESCRIPTION("Analog Devices AD5721, AD5721R, AD5761, AD5761R driver");
+MODULE_LICENSE("GPL v2");
-- 
2.6.2


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

* Re: [PATCH] iio: add ad5761 DAC driver
  2015-12-16 13:27 [PATCH] iio: add ad5761 DAC driver Ricardo Ribalda Delgado
@ 2015-12-19 16:31 ` Jonathan Cameron
  2015-12-20 11:19   ` Ricardo Ribalda Delgado
  2015-12-19 17:06 ` Peter Meerwald-Stadler
  1 sibling, 1 reply; 8+ messages in thread
From: Jonathan Cameron @ 2015-12-19 16:31 UTC (permalink / raw)
  To: Ricardo Ribalda Delgado, Lars-Peter Clausen, Michael Hennerich,
	Hartmut Knaack, Peter Meerwald, linux-iio, linux-kernel

On 16/12/15 13:27, Ricardo Ribalda Delgado wrote:
> ad5761 is a 1-channel DAC with configurable output range.
> The driver uses the regulator interface for its voltage ref.
> 
> It shares its register layout with ad5761r, ad5721 and ad5721r.
> 
> Differences:
> ad5761* are 16 bit, ad5721* are 12 bits.
> ad57*1r have an internal reference.
> 
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
Mostly fine, but the range stuff is not standard ABI.  Whilst it is
rather fiddly to fit it to standard ABI it shouldn't be too bad.

A few other bits inline.

Jonathan
> ---
>  CREDITS                  |   1 +
>  drivers/iio/dac/Kconfig  |  10 ++
>  drivers/iio/dac/Makefile |   1 +
>  drivers/iio/dac/ad5761.c | 425 +++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 437 insertions(+)
>  create mode 100644 drivers/iio/dac/ad5761.c
> 
> diff --git a/CREDITS b/CREDITS
> index 8207cc62ee9d..44ea433d70a1 100644
> --- a/CREDITS
> +++ b/CREDITS
> @@ -3035,6 +3035,7 @@ D: PLX USB338x driver
>  D: PCA9634 driver
>  D: Option GTM671WFS
>  D: Fintek F81216A
> +D: AD5761 iio driver
>  D: Various kernel hacks
>  S: Qtechnology A/S
>  S: Valby Langgade 142
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index e701e28fb1cd..4caedd671360 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -111,6 +111,16 @@ config AD5755
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ad5755.
>  
> +config AD5761
> +	tristate "Analog Devices AD5761/61R/21/21R DAC driver"
> +	depends on SPI_MASTER
> +	help
> +	  Say yes here to build support for Analog Devices AD5761, AD5761R, AD5721,
> +	  AD5721R Digital to Analog Converter.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ad5761.
> +
>  config AD5764
>  	tristate "Analog Devices AD5764/64R/44/44R DAC driver"
>  	depends on SPI_MASTER
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 63ae05633e0c..cb525b53fc7b 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_AD5504) += ad5504.o
>  obj-$(CONFIG_AD5446) += ad5446.o
>  obj-$(CONFIG_AD5449) += ad5449.o
>  obj-$(CONFIG_AD5755) += ad5755.o
> +obj-$(CONFIG_AD5761) += ad5761.o
>  obj-$(CONFIG_AD5764) += ad5764.o
>  obj-$(CONFIG_AD5791) += ad5791.o
>  obj-$(CONFIG_AD5686) += ad5686.o
> diff --git a/drivers/iio/dac/ad5761.c b/drivers/iio/dac/ad5761.c
> new file mode 100644
> index 000000000000..b4eac59ef81f
> --- /dev/null
> +++ b/drivers/iio/dac/ad5761.c
> @@ -0,0 +1,425 @@
> +/*
> + * AD5721, AD5721R, AD5761, AD5761R, Voltage Output Digital to Analog Converter
> + *
> + * Copyright 2015 Qtechnology A/S
> + *
> + * Licensed under the GPL-2.
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/bitops.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/regulator/consumer.h>
> +
> +#define AD5761_ADDR(addr)		((addr&0xf) << 16)
> +#define AD5761_ADDR_NOOP		0x0
> +#define AD5761_ADDR_DAC_WRITE		0x3
> +#define AD5761_ADDR_CTRL_WRITE_REG	0x4
> +#define AD5761_ADDR_SW_DATA_RESET	0x7
> +#define AD5761_ADDR_DAC_READ		0xb
> +#define AD5761_ADDR_CTRL_READ_REG	0xc
> +#define AD5761_ADDR_SW_FULL_RESET	0xf
> +
> +#define AD5761_CTRL_USE_INTVREF		BIT(5)
> +#define AD5761_CTRL_ETS			BIT(6)
> +
> +/**
> + * struct ad5761_chip_info - chip specific information
> + * @int_vref:	Value of the internal reference voltage in mV - 0 if external
> + *		reference voltage is used
> + * @channel	channel specification
> +*/
> +
> +struct ad5761_chip_info {
> +	unsigned long int_vref;
> +	const struct iio_chan_spec channel;
> +};
> +
> +struct ad5761_range_params {
> +	int m;
> +	int c;
> +};
> +
> +/**
> + * ad5761_supported_device_ids:
Single line comment syntax please.
> + */
> +
> +enum ad5761_supported_device_ids {
> +	ID_AD5721,
> +	ID_AD5721R,
> +	ID_AD5761,
> +	ID_AD5761R,
> +};
> +
> +/**
> + * struct ad5761_state - driver instance specific data
> + * @spi:		spi_device
> + * @chip_info:		chip model specific constants
> + * @vref_mv:		actual reference voltage used
> + */
> +struct ad5761_state {
> +	struct spi_device		*spi;
> +	struct regulator		*vref_reg;
> +
> +	bool use_intref;
> +	int vref;
> +	int range;
> +
> +	/*
> +	 * 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;
> +};
> +
> +enum ad5761_range_ids {
> +	MODE_M10V_10V,
> +	MODE_0V_10V,
> +	MODE_M5V_5V,
> +	MODE_0V_5V,
> +	MODE_M2V5_7V5,
> +	MODE_M3V_3V,
> +	MODE_0V_16V,
> +	MODE_0V_20V,
> +};
> +
> +static const struct ad5761_range_params ad5761_range_params[] = {
> +	[MODE_M10V_10V] = {
> +		.m = 80,
> +		.c = 40,
> +	},
> +	[MODE_0V_10V] = {
> +		.m = 40,
> +		.c = 0,
> +	},
> +	[MODE_M5V_5V] = {
> +		.m = 40,
> +		.c = 20,
> +	},
> +	[MODE_0V_5V] = {
> +		.m = 20,
> +		.c = 0,
> +	},
> +	[MODE_M2V5_7V5] = {
> +		.m = 40,
> +		.c = 10,
> +	},
> +	[MODE_M3V_3V] = {
> +		.m = 24,
> +		.c = 12,
> +	},
> +	[MODE_0V_16V] = {
> +		.m = 64,
> +		.c = 0,
> +	},
> +	[MODE_0V_20V] = {
> +		.m = 80,
> +		.c = 0,
> +	},
> +};
> +
> +static const char * const ad5761_ranges[] = {
> +	[MODE_M10V_10V] = "-10V_10V",
> +	[MODE_0V_10V] = "0V_10V",
> +	[MODE_M5V_5V] = "-5V_5V",
> +	[MODE_0V_5V] = "0V_5V",
> +	[MODE_M2V5_7V5] = "-2V5_7V5",
> +	[MODE_M3V_3V] = "-3V_3V",
> +	[MODE_0V_16V] = "0V_16V",
> +	[MODE_0V_20V] = "0V_20V",
> +};
This is totally non standard ABI.  See below.
> +
> +static int ad5761_spi_write(struct ad5761_state *st, u8 addr, u16 val)
> +{
> +	st->data[0].d32 = cpu_to_be32(AD5761_ADDR(addr) | val);
> +
> +	return spi_write(st->spi, &st->data[0].d8[1], 3);
> +}
> +
> +static int ad5761_spi_read(struct ad5761_state *st, u8 addr, u16 *val)
> +{
> +	int ret;
> +	struct spi_transfer xfers[] = {
> +		{
> +			.tx_buf = &st->data[0].d8[1],
> +			.bits_per_word = 8,
> +			.len = 3,
> +			.cs_change = true,
> +		}, {
> +			.tx_buf = &st->data[1].d8[1],
> +			.rx_buf = &st->data[2].d8[1],
> +			.bits_per_word = 8,
> +			.len = 3,
> +		},
> +	};
> +
> +	st->data[0].d32 = cpu_to_be32(AD5761_ADDR(addr));
> +	st->data[1].d32 = cpu_to_be32(AD5761_ADDR(AD5761_ADDR_NOOP));
> +
> +	ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
> +
> +	*val = be32_to_cpu(st->data[2].d32);
> +
> +	return ret;
> +}
> +
> +static int ad5761_spi_set_range(struct ad5761_state *st, int range)
> +{
> +	u16 aux;
> +	int ret;
> +
> +	aux = range;
> +	aux |= AD5761_CTRL_ETS;
> +	if (st->use_intref)
> +		aux |= AD5761_CTRL_USE_INTVREF;
A few blank lines in here would make this a touch more readable.
> +	ret = ad5761_spi_write(st, AD5761_ADDR_CTRL_WRITE_REG, aux);
> +	if (ret)
> +		return ret;
blank line here.
> +	ret = ad5761_spi_write(st, AD5761_ADDR_SW_DATA_RESET, 0);
> +	if (ret)
> +		return ret;
> +
> +	st->range = range;
blank line here.
> +	return 0;
> +}
> +
> +static int ad5761_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val,
> +			   int *val2,
> +			   long m)
> +{
> +	struct ad5761_state *st = iio_priv(indio_dev);
> +	int ret;
> +	u16 aux;
> +
> +	switch (m) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = ad5761_spi_read(st, AD5761_ADDR_DAC_READ, &aux);
> +		if (ret)
> +			return ret;
> +		*val = aux >> chan->scan_type.shift;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = st->vref * ad5761_range_params[st->range].m;
> +		*val /= 10;
> +		*val2 = chan->scan_type.realbits;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = -(1<<chan->scan_type.realbits);
> +		*val *=	ad5761_range_params[st->range].c;
> +		*val /=	ad5761_range_params[st->range].m;
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +}
> +
> +static int ad5761_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int val,
> +			    int val2,
> +			    long mask)
> +{
> +	struct ad5761_state *st = iio_priv(indio_dev);
> +	u16 aux;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		aux =  val << chan->scan_type.shift;
> +		return ad5761_spi_write(st, AD5761_ADDR_DAC_WRITE, aux);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ad5761_set_range(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan, unsigned int range)
> +{
> +	struct ad5761_state *st = iio_priv(indio_dev);
> +
> +	return ad5761_spi_set_range(st, range);
> +}
> +
> +static int ad5761_get_range(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan)
> +{
> +	struct ad5761_state *st = iio_priv(indio_dev);
> +	u16 aux;
> +	int ret;
> +
> +	ret = ad5761_spi_read(st, AD5761_ADDR_CTRL_READ_REG, &aux);
> +	if (ret)
> +		return ret;
> +
> +	return aux & 0x7;
> +}
> +
> +static const struct iio_info ad5761_info = {
> +	.read_raw = &ad5761_read_raw,
> +	.write_raw = &ad5761_write_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static const struct iio_enum ad5761_range_enum = {
> +	.items = ad5761_ranges,
> +	.num_items = ARRAY_SIZE(ad5761_ranges),
> +	.get = ad5761_get_range,
> +	.set = ad5761_set_range,
> +};
> +
> +static struct iio_chan_spec_ext_info ad5761_ext_info[] = {
> +	IIO_ENUM("range", IIO_SHARED_BY_TYPE,
> +		 &ad5761_range_enum),
> +	IIO_ENUM_AVAILABLE("range", &ad5761_range_enum),
> +	{ },
> +};
Range isn't actually specified in the ABI docs.
Documenation\ABI\testing\sysfs-bus-iio*
The control interface for this is normally scale rather than range
(we had to pick one of the two and that is the way it fell out)  Usually
hardware designers care about range, but userspace programs are often
most directly interested in scale factors that need to be applied.
(and that was my most rediculously over generalized statement for the day ;)

I can see this is rather complex here given the random looking collection
of associated scales and offsets.  You would have to have _available
attributes to say what offsets are available at a given scale I think.
Also we'd have to then define a precedence order in the docs for the
two attributes (worth doing to make it obvious what to do when this
sort of setup arises).

> +
> +#define AD5761_CHAN(_bits) {				\
> +	.type = IIO_VOLTAGE,				\
> +	.output = 1,					\
> +	.indexed = 1,					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
> +		BIT(IIO_CHAN_INFO_OFFSET),		\
> +	.scan_type = {					\
> +		.sign = 'u',				\
> +		.realbits = (_bits),			\
> +		.storagebits = 16,			\
> +		.shift = 16 - (_bits),			\
> +	},						\
> +	.ext_info = ad5761_ext_info,			\
> +}
> +
> +static const struct ad5761_chip_info ad5761_chip_infos[] = {
> +	[ID_AD5721] = {
> +		.int_vref = 0,
> +		.channel = AD5761_CHAN(12),
> +	},
> +	[ID_AD5721R] = {
> +		.int_vref = 2500,
> +		.channel = AD5761_CHAN(12),
> +	},
> +	[ID_AD5761] = {
> +		.int_vref = 0,
> +		.channel = AD5761_CHAN(16),
> +	},
> +	[ID_AD5761R] = {
> +		.int_vref = 2500,
> +		.channel = AD5761_CHAN(16),
> +	},
> +};
> +
> +static void ad5761_get_vref(struct ad5761_state *st)
> +{
> +	int ret;
> +
> +
Single blank line only please.
> +	st->vref_reg = devm_regulator_get(&st->spi->dev, "vref");
> +	if (IS_ERR(st->vref_reg))
Don't eat errors that may be informative.
If it's optional, then use the get_optional calls, though care will be
needed as it's only sometimes optional here..
> +		return;
> +
> +	ret = regulator_enable(st->vref_reg);
> +	if (ret) {
> +		dev_warn(&st->spi->dev, "Failed to enable vref. Using internal");
> +		return;
> +	}
> +
> +	ret = regulator_get_voltage(st->vref_reg);
> +	if (ret < 0) {
> +		regulator_disable(st->vref_reg);
> +		dev_warn(&st->spi->dev,
> +			 "Failed to get vref value. Using internal");
> +		return;
> +	}
> +
> +	if (ret < 2000000 || ret > 3000000) {
> +		regulator_disable(st->vref_reg);
> +		dev_warn(&st->spi->dev,
> +				"Invalid external vref value. Using internal");
> +		return;
> +	}
> +
> +	st->vref = ret / 1000;
> +	st->use_intref = false;
> +}
> +
> +static int ad5761_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *iio_dev;
> +	struct ad5761_state *st;
> +	int ret;
> +	const struct ad5761_chip_info *chip_info =
> +		&ad5761_chip_infos[spi_get_device_id(spi)->driver_data];
> +
> +	iio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!iio_dev)
> +		return -ENOMEM;
> +	st = iio_priv(iio_dev);
> +
> +	st->spi = spi;
> +	spi_set_drvdata(spi, iio_dev);
> +
> +	st->use_intref = true;
> +	st->vref = chip_info->int_vref;
> +	ad5761_get_vref(st);
> +	if (st->use_intref && !chip_info->int_vref) {
> +		dev_err(&spi->dev, "Missing vref, cannot continue");
> +		return -EIO;
> +	}
> +
> +	ad5761_spi_set_range(st, MODE_0V_5V);
> +
> +	iio_dev->dev.parent = &spi->dev;
> +	iio_dev->info = &ad5761_info;
> +	iio_dev->modes = INDIO_DIRECT_MODE;
> +	iio_dev->channels = &chip_info->channel;
> +	iio_dev->num_channels = 1;
> +	iio_dev->name = spi_get_device_id(st->spi)->name;
> +	ret = iio_device_register(iio_dev);
> +	if (ret && !IS_ERR(st->vref_reg))

I would ever so slightly prefer to have a matched funcitonto get_vref
(release_vref perhaps) so that it's obvious what this disable is
'undoing'.  At first glance, I wondered where it was being enabled!

> +		regulator_disable(st->vref_reg);
> +
> +	return ret;
> +}
> +
> +static int ad5761_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *iio_dev = spi_get_drvdata(spi);
> +	struct ad5761_state *st = iio_priv(iio_dev);
> +
> +	if (!IS_ERR(st->vref_reg))
> +		regulator_disable(st->vref_reg);
> +	iio_device_unregister(iio_dev);
Nitpick.  Blank line before simple returns like this slightly
improves readability.

> +	return 0;
> +}
> +
> +static const struct spi_device_id ad5761_id[] = {
> +	{"ad5721", ID_AD5721},
> +	{"ad5721r", ID_AD5721R},
> +	{"ad5761", ID_AD5761},
> +	{"ad5761r", ID_AD5761R},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(spi, ad5761_id);
> +
> +static struct spi_driver ad5761_driver = {
> +	.driver = {
> +		   .name = "ad5761",
> +		   .owner = THIS_MODULE,
> +		   },
> +	.probe = ad5761_probe,
> +	.remove = ad5761_remove,
> +	.id_table = ad5761_id,
> +};
> +module_spi_driver(ad5761_driver);
> +
> +MODULE_AUTHOR("Ricardo Ribalda <ricardo.ribalda@gmail.com>");
> +MODULE_DESCRIPTION("Analog Devices AD5721, AD5721R, AD5761, AD5761R driver");
> +MODULE_LICENSE("GPL v2");
> 


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

* Re: [PATCH] iio: add ad5761 DAC driver
  2015-12-16 13:27 [PATCH] iio: add ad5761 DAC driver Ricardo Ribalda Delgado
  2015-12-19 16:31 ` Jonathan Cameron
@ 2015-12-19 17:06 ` Peter Meerwald-Stadler
  2015-12-20 11:24   ` Ricardo Ribalda Delgado
  1 sibling, 1 reply; 8+ messages in thread
From: Peter Meerwald-Stadler @ 2015-12-19 17:06 UTC (permalink / raw)
  To: Ricardo Ribalda Delgado
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Hartmut Knaack, linux-iio, linux-kernel


> ad5761 is a 1-channel DAC with configurable output range.
> The driver uses the regulator interface for its voltage ref.

some nitpicking below

> It shares its register layout with ad5761r, ad5721 and ad5721r.
> 
> Differences:
> ad5761* are 16 bit, ad5721* are 12 bits.
> ad57*1r have an internal reference.
> 
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
>  CREDITS                  |   1 +
>  drivers/iio/dac/Kconfig  |  10 ++
>  drivers/iio/dac/Makefile |   1 +
>  drivers/iio/dac/ad5761.c | 425 +++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 437 insertions(+)
>  create mode 100644 drivers/iio/dac/ad5761.c
> 
> diff --git a/CREDITS b/CREDITS
> index 8207cc62ee9d..44ea433d70a1 100644
> --- a/CREDITS
> +++ b/CREDITS
> @@ -3035,6 +3035,7 @@ D: PLX USB338x driver
>  D: PCA9634 driver
>  D: Option GTM671WFS
>  D: Fintek F81216A
> +D: AD5761 iio driver
>  D: Various kernel hacks
>  S: Qtechnology A/S
>  S: Valby Langgade 142
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index e701e28fb1cd..4caedd671360 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -111,6 +111,16 @@ config AD5755
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ad5755.
>  
> +config AD5761
> +	tristate "Analog Devices AD5761/61R/21/21R DAC driver"
> +	depends on SPI_MASTER
> +	help
> +	  Say yes here to build support for Analog Devices AD5761, AD5761R, AD5721,
> +	  AD5721R Digital to Analog Converter.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ad5761.
> +
>  config AD5764
>  	tristate "Analog Devices AD5764/64R/44/44R DAC driver"
>  	depends on SPI_MASTER
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 63ae05633e0c..cb525b53fc7b 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_AD5504) += ad5504.o
>  obj-$(CONFIG_AD5446) += ad5446.o
>  obj-$(CONFIG_AD5449) += ad5449.o
>  obj-$(CONFIG_AD5755) += ad5755.o
> +obj-$(CONFIG_AD5761) += ad5761.o
>  obj-$(CONFIG_AD5764) += ad5764.o
>  obj-$(CONFIG_AD5791) += ad5791.o
>  obj-$(CONFIG_AD5686) += ad5686.o
> diff --git a/drivers/iio/dac/ad5761.c b/drivers/iio/dac/ad5761.c
> new file mode 100644
> index 000000000000..b4eac59ef81f
> --- /dev/null
> +++ b/drivers/iio/dac/ad5761.c
> @@ -0,0 +1,425 @@
> +/*
> + * AD5721, AD5721R, AD5761, AD5761R, Voltage Output Digital to Analog Converter
> + *
> + * Copyright 2015 Qtechnology A/S
> + *
> + * Licensed under the GPL-2.
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/bitops.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/regulator/consumer.h>
> +
> +#define AD5761_ADDR(addr)		((addr&0xf) << 16)

maybe add whitespace around &

> +#define AD5761_ADDR_NOOP		0x0
> +#define AD5761_ADDR_DAC_WRITE		0x3
> +#define AD5761_ADDR_CTRL_WRITE_REG	0x4
> +#define AD5761_ADDR_SW_DATA_RESET	0x7
> +#define AD5761_ADDR_DAC_READ		0xb
> +#define AD5761_ADDR_CTRL_READ_REG	0xc
> +#define AD5761_ADDR_SW_FULL_RESET	0xf
> +
> +#define AD5761_CTRL_USE_INTVREF		BIT(5)
> +#define AD5761_CTRL_ETS			BIT(6)
> +
> +/**
> + * struct ad5761_chip_info - chip specific information
> + * @int_vref:	Value of the internal reference voltage in mV - 0 if external
> + *		reference voltage is used
> + * @channel	channel specification

@channel:

> +*/
> +
> +struct ad5761_chip_info {
> +	unsigned long int_vref;
> +	const struct iio_chan_spec channel;
> +};
> +
> +struct ad5761_range_params {
> +	int m;
> +	int c;
> +};
> +
> +/**
> + * ad5761_supported_device_ids:
> + */
> +
> +enum ad5761_supported_device_ids {
> +	ID_AD5721,
> +	ID_AD5721R,
> +	ID_AD5761,
> +	ID_AD5761R,
> +};
> +
> +/**
> + * struct ad5761_state - driver instance specific data
> + * @spi:		spi_device
> + * @chip_info:		chip model specific constants
> + * @vref_mv:		actual reference voltage used

there is no chip_info and no vref_mv in ad5761_state

vref_reg, use_intref, vref, range, data exist but have no explanation

> + */
> +struct ad5761_state {
> +	struct spi_device		*spi;
> +	struct regulator		*vref_reg;
> +
> +	bool use_intref;
> +	int vref;
> +	int range;
> +
> +	/*
> +	 * 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;
> +};
> +
> +enum ad5761_range_ids {
> +	MODE_M10V_10V,
> +	MODE_0V_10V,
> +	MODE_M5V_5V,
> +	MODE_0V_5V,
> +	MODE_M2V5_7V5,
> +	MODE_M3V_3V,
> +	MODE_0V_16V,
> +	MODE_0V_20V,
> +};
> +
> +static const struct ad5761_range_params ad5761_range_params[] = {
> +	[MODE_M10V_10V] = {
> +		.m = 80,
> +		.c = 40,
> +	},
> +	[MODE_0V_10V] = {
> +		.m = 40,
> +		.c = 0,
> +	},
> +	[MODE_M5V_5V] = {
> +		.m = 40,
> +		.c = 20,
> +	},
> +	[MODE_0V_5V] = {
> +		.m = 20,
> +		.c = 0,
> +	},
> +	[MODE_M2V5_7V5] = {
> +		.m = 40,
> +		.c = 10,
> +	},
> +	[MODE_M3V_3V] = {
> +		.m = 24,
> +		.c = 12,
> +	},
> +	[MODE_0V_16V] = {
> +		.m = 64,
> +		.c = 0,
> +	},
> +	[MODE_0V_20V] = {
> +		.m = 80,
> +		.c = 0,
> +	},
> +};
> +
> +static const char * const ad5761_ranges[] = {
> +	[MODE_M10V_10V] = "-10V_10V",
> +	[MODE_0V_10V] = "0V_10V",
> +	[MODE_M5V_5V] = "-5V_5V",
> +	[MODE_0V_5V] = "0V_5V",
> +	[MODE_M2V5_7V5] = "-2V5_7V5",
> +	[MODE_M3V_3V] = "-3V_3V",
> +	[MODE_0V_16V] = "0V_16V",
> +	[MODE_0V_20V] = "0V_20V",
> +};
> +
> +static int ad5761_spi_write(struct ad5761_state *st, u8 addr, u16 val)
> +{
> +	st->data[0].d32 = cpu_to_be32(AD5761_ADDR(addr) | val);
> +
> +	return spi_write(st->spi, &st->data[0].d8[1], 3);
> +}
> +
> +static int ad5761_spi_read(struct ad5761_state *st, u8 addr, u16 *val)
> +{
> +	int ret;
> +	struct spi_transfer xfers[] = {
> +		{
> +			.tx_buf = &st->data[0].d8[1],
> +			.bits_per_word = 8,
> +			.len = 3,
> +			.cs_change = true,
> +		}, {
> +			.tx_buf = &st->data[1].d8[1],
> +			.rx_buf = &st->data[2].d8[1],
> +			.bits_per_word = 8,
> +			.len = 3,
> +		},
> +	};
> +
> +	st->data[0].d32 = cpu_to_be32(AD5761_ADDR(addr));
> +	st->data[1].d32 = cpu_to_be32(AD5761_ADDR(AD5761_ADDR_NOOP));
> +
> +	ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
> +
> +	*val = be32_to_cpu(st->data[2].d32);
> +
> +	return ret;
> +}
> +
> +static int ad5761_spi_set_range(struct ad5761_state *st, int range)
> +{
> +	u16 aux;
> +	int ret;
> +
> +	aux = range;
> +	aux |= AD5761_CTRL_ETS;
> +	if (st->use_intref)
> +		aux |= AD5761_CTRL_USE_INTVREF;
> +	ret = ad5761_spi_write(st, AD5761_ADDR_CTRL_WRITE_REG, aux);
> +	if (ret)
> +		return ret;
> +	ret = ad5761_spi_write(st, AD5761_ADDR_SW_DATA_RESET, 0);
> +	if (ret)
> +		return ret;
> +
> +	st->range = range;
> +	return 0;
> +}
> +
> +static int ad5761_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val,
> +			   int *val2,
> +			   long m)

often 'mask' is used instead of m

> +{
> +	struct ad5761_state *st = iio_priv(indio_dev);
> +	int ret;
> +	u16 aux;
> +
> +	switch (m) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = ad5761_spi_read(st, AD5761_ADDR_DAC_READ, &aux);
> +		if (ret)
> +			return ret;
> +		*val = aux >> chan->scan_type.shift;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = st->vref * ad5761_range_params[st->range].m;
> +		*val /= 10;
> +		*val2 = chan->scan_type.realbits;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = -(1<<chan->scan_type.realbits);

whitespace around <<

> +		*val *=	ad5761_range_params[st->range].c;
> +		*val /=	ad5761_range_params[st->range].m;
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +

delete newline

> +}
> +
> +static int ad5761_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int val,
> +			    int val2,
> +			    long mask)
> +{
> +	struct ad5761_state *st = iio_priv(indio_dev);
> +	u16 aux;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		aux =  val << chan->scan_type.shift;

delete extra space before val

> +		return ad5761_spi_write(st, AD5761_ADDR_DAC_WRITE, aux);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ad5761_set_range(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan, unsigned int range)
> +{
> +	struct ad5761_state *st = iio_priv(indio_dev);
> +
> +	return ad5761_spi_set_range(st, range);
> +}
> +
> +static int ad5761_get_range(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan)
> +{
> +	struct ad5761_state *st = iio_priv(indio_dev);
> +	u16 aux;
> +	int ret;
> +
> +	ret = ad5761_spi_read(st, AD5761_ADDR_CTRL_READ_REG, &aux);
> +	if (ret)
> +		return ret;
> +
> +	return aux & 0x7;
> +}
> +
> +static const struct iio_info ad5761_info = {
> +	.read_raw = &ad5761_read_raw,
> +	.write_raw = &ad5761_write_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static const struct iio_enum ad5761_range_enum = {
> +	.items = ad5761_ranges,
> +	.num_items = ARRAY_SIZE(ad5761_ranges),
> +	.get = ad5761_get_range,
> +	.set = ad5761_set_range,
> +};
> +
> +static struct iio_chan_spec_ext_info ad5761_ext_info[] = {
> +	IIO_ENUM("range", IIO_SHARED_BY_TYPE,
> +		 &ad5761_range_enum),
> +	IIO_ENUM_AVAILABLE("range", &ad5761_range_enum),
> +	{ },
> +};
> +
> +#define AD5761_CHAN(_bits) {				\
> +	.type = IIO_VOLTAGE,				\
> +	.output = 1,					\
> +	.indexed = 1,					\

since there is only one channel, .indexed could be 0 (or not set)

> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
> +		BIT(IIO_CHAN_INFO_OFFSET),		\
> +	.scan_type = {					\

the driver does not support buffered mode, not sure if scan_type should be 
used to store the number of bits; I think it would be cleaner to put this 
in chip_info

> +		.sign = 'u',				\
> +		.realbits = (_bits),			\
> +		.storagebits = 16,			\
> +		.shift = 16 - (_bits),			\
> +	},						\
> +	.ext_info = ad5761_ext_info,			\
> +}
> +
> +static const struct ad5761_chip_info ad5761_chip_infos[] = {
> +	[ID_AD5721] = {
> +		.int_vref = 0,
> +		.channel = AD5761_CHAN(12),
> +	},
> +	[ID_AD5721R] = {
> +		.int_vref = 2500,
> +		.channel = AD5761_CHAN(12),
> +	},
> +	[ID_AD5761] = {
> +		.int_vref = 0,
> +		.channel = AD5761_CHAN(16),
> +	},
> +	[ID_AD5761R] = {
> +		.int_vref = 2500,
> +		.channel = AD5761_CHAN(16),
> +	},
> +};
> +
> +static void ad5761_get_vref(struct ad5761_state *st)
> +{
> +	int ret;
> +

delete extra newline

> +
> +	st->vref_reg = devm_regulator_get(&st->spi->dev, "vref");
> +	if (IS_ERR(st->vref_reg))
> +		return;
> +
> +	ret = regulator_enable(st->vref_reg);
> +	if (ret) {
> +		dev_warn(&st->spi->dev, "Failed to enable vref. Using internal");

put \n at the end of the message (here and elsewhere)

> +		return;
> +	}
> +
> +	ret = regulator_get_voltage(st->vref_reg);
> +	if (ret < 0) {
> +		regulator_disable(st->vref_reg);
> +		dev_warn(&st->spi->dev,
> +			 "Failed to get vref value. Using internal");
> +		return;
> +	}
> +
> +	if (ret < 2000000 || ret > 3000000) {
> +		regulator_disable(st->vref_reg);
> +		dev_warn(&st->spi->dev,
> +				"Invalid external vref value. Using internal");
> +		return;
> +	}
> +
> +	st->vref = ret / 1000;
> +	st->use_intref = false;
> +}
> +
> +static int ad5761_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *iio_dev;
> +	struct ad5761_state *st;
> +	int ret;
> +	const struct ad5761_chip_info *chip_info =
> +		&ad5761_chip_infos[spi_get_device_id(spi)->driver_data];
> +
> +	iio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!iio_dev)
> +		return -ENOMEM;
> +	st = iio_priv(iio_dev);
> +
> +	st->spi = spi;
> +	spi_set_drvdata(spi, iio_dev);
> +
> +	st->use_intref = true;
> +	st->vref = chip_info->int_vref;
> +	ad5761_get_vref(st);

this is ugly, I'd prefer to return something from _get_vref() instead of 
first setting a could of variable and then checking a couple of variables

> +	if (st->use_intref && !chip_info->int_vref) {
> +		dev_err(&spi->dev, "Missing vref, cannot continue");
> +		return -EIO;
> +	}
> +
> +	ad5761_spi_set_range(st, MODE_0V_5V);

this can fail, retval not checked

> +
> +	iio_dev->dev.parent = &spi->dev;
> +	iio_dev->info = &ad5761_info;
> +	iio_dev->modes = INDIO_DIRECT_MODE;
> +	iio_dev->channels = &chip_info->channel;
> +	iio_dev->num_channels = 1;
> +	iio_dev->name = spi_get_device_id(st->spi)->name;
> +	ret = iio_device_register(iio_dev);
> +	if (ret && !IS_ERR(st->vref_reg))
> +		regulator_disable(st->vref_reg);

maybe vref_reg is disabled twice, here and in get_vref()

> +
> +	return ret;
> +}
> +
> +static int ad5761_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *iio_dev = spi_get_drvdata(spi);
> +	struct ad5761_state *st = iio_priv(iio_dev);
> +
> +	if (!IS_ERR(st->vref_reg))
> +		regulator_disable(st->vref_reg);
> +	iio_device_unregister(iio_dev);
> +	return 0;
> +}
> +
> +static const struct spi_device_id ad5761_id[] = {
> +	{"ad5721", ID_AD5721},
> +	{"ad5721r", ID_AD5721R},
> +	{"ad5761", ID_AD5761},
> +	{"ad5761r", ID_AD5761R},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(spi, ad5761_id);
> +
> +static struct spi_driver ad5761_driver = {
> +	.driver = {
> +		   .name = "ad5761",
> +		   .owner = THIS_MODULE,
> +		   },
> +	.probe = ad5761_probe,
> +	.remove = ad5761_remove,
> +	.id_table = ad5761_id,
> +};
> +module_spi_driver(ad5761_driver);
> +
> +MODULE_AUTHOR("Ricardo Ribalda <ricardo.ribalda@gmail.com>");
> +MODULE_DESCRIPTION("Analog Devices AD5721, AD5721R, AD5761, AD5761R driver");
> +MODULE_LICENSE("GPL v2");
> 

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)

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

* Re: [PATCH] iio: add ad5761 DAC driver
  2015-12-19 16:31 ` Jonathan Cameron
@ 2015-12-20 11:19   ` Ricardo Ribalda Delgado
  2015-12-22 18:13     ` Jonathan Cameron
  0 siblings, 1 reply; 8+ messages in thread
From: Ricardo Ribalda Delgado @ 2015-12-20 11:19 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Michael Hennerich, Hartmut Knaack,
	Peter Meerwald, linux-iio, LKML

Hello Jonthan

Thanks for your comments, I have fixed all the style problems in v2,
so we can focus in the range parameter.

On Sat, Dec 19, 2015 at 5:31 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> Range isn't actually specified in the ABI docs.
> Documenation\ABI\testing\sysfs-bus-iio*
> The control interface for this is normally scale rather than range
> (we had to pick one of the two and that is the way it fell out)  Usually
> hardware designers care about range, but userspace programs are often
> most directly interested in scale factors that need to be applied.
> (and that was my most rediculously over generalized statement for the day ;)

What about a first version of the driver where the range is set via
pdata and is not userland configurable?
That way the four chips will be supported and we can have more
feedback from other users about the range issue.

>
> I can see this is rather complex here given the random looking collection
> of associated scales and offsets.  You would have to have _available
> attributes to say what offsets are available at a given scale I think.
> Also we'd have to then define a precedence order in the docs for the
> two attributes (worth doing to make it obvious what to do when this
> sort of setup arises).

The problem with that approach is that there will be two operations to
set the range: one  to change the scale, and another for the offset.  The output
voltage will change twice in this process and may have an intermediate value
that can damage a circuit.

I also believe that my approach with a text description is more user friendly
(but problably because I programmed it :P)

In any case, I will implement whatever we agree ;)

Best regards!

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

* Re: [PATCH] iio: add ad5761 DAC driver
  2015-12-19 17:06 ` Peter Meerwald-Stadler
@ 2015-12-20 11:24   ` Ricardo Ribalda Delgado
  0 siblings, 0 replies; 8+ messages in thread
From: Ricardo Ribalda Delgado @ 2015-12-20 11:24 UTC (permalink / raw)
  To: Peter Meerwald-Stadler
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Hartmut Knaack, linux-iio, LKML

Hello Peter:


Thanks for your review! I agree on all your points but one:

On Sat, Dec 19, 2015 at 6:06 PM, Peter Meerwald-Stadler
<pmeerw@pmeerw.net> wrote:
>> +     .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),   \
>> +     .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |  \
>> +             BIT(IIO_CHAN_INFO_OFFSET),              \
>> +     .scan_type = {                                  \
>
> the driver does not support buffered mode, not sure if scan_type should be
> used to store the number of bits; I think it would be cleaner to put this
> in chip_info


Although I understand your point, must of the other dac drivers are
doing the same and in my humble opinion is very elegant.



Best regards!

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

* Re: [PATCH] iio: add ad5761 DAC driver
  2015-12-20 11:19   ` Ricardo Ribalda Delgado
@ 2015-12-22 18:13     ` Jonathan Cameron
  0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2015-12-22 18:13 UTC (permalink / raw)
  To: Ricardo Ribalda Delgado
  Cc: Lars-Peter Clausen, Michael Hennerich, Hartmut Knaack,
	Peter Meerwald, linux-iio, LKML

On 20/12/15 11:19, Ricardo Ribalda Delgado wrote:
> Hello Jonthan
> 
> Thanks for your comments, I have fixed all the style problems in v2,
> so we can focus in the range parameter.
> 
> On Sat, Dec 19, 2015 at 5:31 PM, Jonathan Cameron <jic23@kernel.org> wrote:
>> Range isn't actually specified in the ABI docs.
>> Documenation\ABI\testing\sysfs-bus-iio*
>> The control interface for this is normally scale rather than range
>> (we had to pick one of the two and that is the way it fell out)  Usually
>> hardware designers care about range, but userspace programs are often
>> most directly interested in scale factors that need to be applied.
>> (and that was my most rediculously over generalized statement for the day ;)
> 
> What about a first version of the driver where the range is set via
> pdata and is not userland configurable?
> That way the four chips will be supported and we can have more
> feedback from other users about the range issue.
That would be fine.  Generally I'd imagine a given board will only want to
have one sensible choice anyway! (other than devkits at least)
> 
>>
>> I can see this is rather complex here given the random looking collection
>> of associated scales and offsets.  You would have to have _available
>> attributes to say what offsets are available at a given scale I think.
>> Also we'd have to then define a precedence order in the docs for the
>> two attributes (worth doing to make it obvious what to do when this
>> sort of setup arises).
> 
> The problem with that approach is that there will be two operations to
> set the range: one  to change the scale, and another for the offset.  The output
> voltage will change twice in this process and may have an intermediate value
> that can damage a circuit.
Fair point - I had not thought of that.  Hmm.. Could add a commit type attribute
but that's ugly too.

Even if we do allow changing the range ultimately, I think
it would need some hard restrictions in platform data on what is 'safe' for a
particular board.
> 
> I also believe that my approach with a text description is more user friendly
> (but problably because I programmed it :P)
> 
> In any case, I will implement whatever we agree ;)
For now, pdata sounds like the best plan and revisit this at a later date.

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


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

* [PATCH] iio: add ad5761 DAC driver
@ 2016-01-05 22:41 Ricardo Ribalda Delgado
  2016-01-05 22:42 ` Ricardo Ribalda Delgado
  0 siblings, 1 reply; 8+ messages in thread
From: Ricardo Ribalda Delgado @ 2016-01-05 22:41 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Hartmut Knaack, Peter Meerwald, linux-iio, linux-kernel
  Cc: Ricardo Ribalda Delgado

ad5761 is a 1-channel DAC with configurable output range.
The driver uses the regulator interface for its voltage ref.

It shares its register layout with ad5761r, ad5721 and ad5721r.

Differences:
ad5761* are 16 bit, ad5721* are 12 bits.
ad57*1r have an internal reference.

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
v5: CodeStyle and check val2 value
    Thanks to Peter Meerwald-Stadler <pmeerw@pmeerw.net> and

v4: Use locking mechanism
    Populate errors
    Thanks to Lars-Peter Clausen <lars@metafoo.de>

v3: Move programmable voltage range to pdata
    Exit on error instead of using internal voltage reference
    Thanks to Jonathan Cameron <jic23@kernel.org>

v2: A lot of CodeStyle Fixes
    Thanks to Peter Meerwald-Stadler <pmeerw@pmeerw.net> and
    Jonathan Cameron <jic23@kernel.org>

 CREDITS                        |   1 +
 drivers/iio/dac/Kconfig        |  10 +
 drivers/iio/dac/Makefile       |   1 +
 drivers/iio/dac/ad5761.c       | 432 +++++++++++++++++++++++++++++++++++++++++
 include/linux/iio/dac/ad5761.h |  36 ++++
 5 files changed, 480 insertions(+)
 create mode 100644 drivers/iio/dac/ad5761.c
 create mode 100644 include/linux/iio/dac/ad5761.h

diff --git a/CREDITS b/CREDITS
index 8207cc62ee9d..44ea433d70a1 100644
--- a/CREDITS
+++ b/CREDITS
@@ -3035,6 +3035,7 @@ D: PLX USB338x driver
 D: PCA9634 driver
 D: Option GTM671WFS
 D: Fintek F81216A
+D: AD5761 iio driver
 D: Various kernel hacks
 S: Qtechnology A/S
 S: Valby Langgade 142
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index e701e28fb1cd..4caedd671360 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -111,6 +111,16 @@ config AD5755
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad5755.
 
+config AD5761
+	tristate "Analog Devices AD5761/61R/21/21R DAC driver"
+	depends on SPI_MASTER
+	help
+	  Say yes here to build support for Analog Devices AD5761, AD5761R, AD5721,
+	  AD5721R Digital to Analog Converter.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad5761.
+
 config AD5764
 	tristate "Analog Devices AD5764/64R/44/44R DAC driver"
 	depends on SPI_MASTER
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 63ae05633e0c..cb525b53fc7b 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_AD5504) += ad5504.o
 obj-$(CONFIG_AD5446) += ad5446.o
 obj-$(CONFIG_AD5449) += ad5449.o
 obj-$(CONFIG_AD5755) += ad5755.o
+obj-$(CONFIG_AD5761) += ad5761.o
 obj-$(CONFIG_AD5764) += ad5764.o
 obj-$(CONFIG_AD5791) += ad5791.o
 obj-$(CONFIG_AD5686) += ad5686.o
diff --git a/drivers/iio/dac/ad5761.c b/drivers/iio/dac/ad5761.c
new file mode 100644
index 000000000000..433dd7254c10
--- /dev/null
+++ b/drivers/iio/dac/ad5761.c
@@ -0,0 +1,432 @@
+/*
+ * AD5721, AD5721R, AD5761, AD5761R, Voltage Output Digital to Analog Converter
+ *
+ * Copyright 2016 Qtechnology A/S
+ * 2016 Ricardo Ribalda <ricardo.ribalda@gmail.com>
+ *
+ * Licensed under the GPL-2.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+#include <linux/bitops.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/regulator/consumer.h>
+#include <linux/iio/dac/ad5761.h>
+
+#define AD5761_ADDR(addr)		((addr & 0xf) << 16)
+#define AD5761_ADDR_NOOP		0x0
+#define AD5761_ADDR_DAC_WRITE		0x3
+#define AD5761_ADDR_CTRL_WRITE_REG	0x4
+#define AD5761_ADDR_SW_DATA_RESET	0x7
+#define AD5761_ADDR_DAC_READ		0xb
+#define AD5761_ADDR_CTRL_READ_REG	0xc
+#define AD5761_ADDR_SW_FULL_RESET	0xf
+
+#define AD5761_CTRL_USE_INTVREF		BIT(5)
+#define AD5761_CTRL_ETS			BIT(6)
+
+/**
+ * struct ad5761_chip_info - chip specific information
+ * @int_vref:	Value of the internal reference voltage in mV - 0 if external
+ *		reference voltage is used
+ * @channel:	channel specification
+*/
+
+struct ad5761_chip_info {
+	unsigned long int_vref;
+	const struct iio_chan_spec channel;
+};
+
+struct ad5761_range_params {
+	int m;
+	int c;
+};
+
+enum ad5761_supported_device_ids {
+	ID_AD5721,
+	ID_AD5721R,
+	ID_AD5761,
+	ID_AD5761R,
+};
+
+/**
+ * struct ad5761_state - driver instance specific data
+ * @spi:		spi_device
+ * @vref_reg:		reference voltage regulator
+ * @use_intref:		true when the internal voltage reference is used
+ * @vref:		actual voltage reference in mVolts
+ * @range:		output range mode used
+ * @data:		cache aligned spi buffer
+ */
+struct ad5761_state {
+	struct spi_device		*spi;
+	struct regulator		*vref_reg;
+
+	bool use_intref;
+	int vref;
+	enum ad5761_voltage_range range;
+
+	/*
+	 * 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 struct ad5761_range_params ad5761_range_params[] = {
+	[AD5761_VOLTAGE_RANGE_M10V_10V] = {
+		.m = 80,
+		.c = 40,
+	},
+	[AD5761_VOLTAGE_RANGE_0V_10V] = {
+		.m = 40,
+		.c = 0,
+	},
+	[AD5761_VOLTAGE_RANGE_M5V_5V] = {
+		.m = 40,
+		.c = 20,
+	},
+	[AD5761_VOLTAGE_RANGE_0V_5V] = {
+		.m = 20,
+		.c = 0,
+	},
+	[AD5761_VOLTAGE_RANGE_M2V5_7V5] = {
+		.m = 40,
+		.c = 10,
+	},
+	[AD5761_VOLTAGE_RANGE_M3V_3V] = {
+		.m = 24,
+		.c = 12,
+	},
+	[AD5761_VOLTAGE_RANGE_0V_16V] = {
+		.m = 64,
+		.c = 0,
+	},
+	[AD5761_VOLTAGE_RANGE_0V_20V] = {
+		.m = 80,
+		.c = 0,
+	},
+};
+
+static int _ad5761_spi_write(struct ad5761_state *st, u8 addr, u16 val)
+{
+	st->data[0].d32 = cpu_to_be32(AD5761_ADDR(addr) | val);
+
+	return spi_write(st->spi, &st->data[0].d8[1], 3);
+}
+
+static int ad5761_spi_write(struct iio_dev *indio_dev, u8 addr, u16 val)
+{
+	struct ad5761_state *st = iio_priv(indio_dev);
+	int ret;
+
+	mutex_lock(&indio_dev->mlock);
+	ret = _ad5761_spi_write(st, addr, val);
+	mutex_unlock(&indio_dev->mlock);
+
+	return ret;
+}
+
+static int _ad5761_spi_read(struct ad5761_state *st, u8 addr, u16 *val)
+{
+	int ret;
+	struct spi_transfer xfers[] = {
+		{
+			.tx_buf = &st->data[0].d8[1],
+			.bits_per_word = 8,
+			.len = 3,
+			.cs_change = true,
+		}, {
+			.tx_buf = &st->data[1].d8[1],
+			.rx_buf = &st->data[2].d8[1],
+			.bits_per_word = 8,
+			.len = 3,
+		},
+	};
+
+	st->data[0].d32 = cpu_to_be32(AD5761_ADDR(addr));
+	st->data[1].d32 = cpu_to_be32(AD5761_ADDR(AD5761_ADDR_NOOP));
+
+	ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
+
+	*val = be32_to_cpu(st->data[2].d32);
+
+	return ret;
+}
+
+static int ad5761_spi_read(struct iio_dev *indio_dev, u8 addr, u16 *val)
+{
+	struct ad5761_state *st = iio_priv(indio_dev);
+	int ret;
+
+	mutex_lock(&indio_dev->mlock);
+	ret = _ad5761_spi_read(st, addr, val);
+	mutex_unlock(&indio_dev->mlock);
+
+	return ret;
+}
+
+static int ad5761_spi_set_range(struct ad5761_state *st,
+				enum ad5761_voltage_range range)
+{
+	u16 aux;
+	int ret;
+
+	aux = (range & 0x7) | AD5761_CTRL_ETS;
+
+	if (st->use_intref)
+		aux |= AD5761_CTRL_USE_INTVREF;
+
+	ret = _ad5761_spi_write(st, AD5761_ADDR_SW_FULL_RESET, 0);
+	if (ret)
+		return ret;
+
+	ret = _ad5761_spi_write(st, AD5761_ADDR_CTRL_WRITE_REG, aux);
+	if (ret)
+		return ret;
+
+	st->range = range;
+
+	return 0;
+}
+
+static int ad5761_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val,
+			   int *val2,
+			   long mask)
+{
+	struct ad5761_state *st;
+	int ret;
+	u16 aux;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = ad5761_spi_read(indio_dev, AD5761_ADDR_DAC_READ, &aux);
+		if (ret)
+			return ret;
+		*val = aux >> chan->scan_type.shift;
+		*val2 = 0;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		st = iio_priv(indio_dev);
+		*val = st->vref * ad5761_range_params[st->range].m;
+		*val /= 10;
+		*val2 = chan->scan_type.realbits;
+		return IIO_VAL_FRACTIONAL_LOG2;
+	case IIO_CHAN_INFO_OFFSET:
+		st = iio_priv(indio_dev);
+		*val = -(1 << chan->scan_type.realbits);
+		*val *=	ad5761_range_params[st->range].c;
+		*val /=	ad5761_range_params[st->range].m;
+		*val2 = 0;
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad5761_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val,
+			    int val2,
+			    long mask)
+{
+	u16 aux;
+
+	if (mask != IIO_CHAN_INFO_RAW)
+		return -EINVAL;
+
+	if (val2 || (val << chan->scan_type.shift) > 0xffff || val < 0)
+		return -EINVAL;
+
+	aux = val << chan->scan_type.shift;
+
+	return ad5761_spi_write(indio_dev, AD5761_ADDR_DAC_WRITE, aux);
+}
+
+static const struct iio_info ad5761_info = {
+	.read_raw = &ad5761_read_raw,
+	.write_raw = &ad5761_write_raw,
+	.driver_module = THIS_MODULE,
+};
+
+#define AD5761_CHAN(_bits) {				\
+	.type = IIO_VOLTAGE,				\
+	.output = 1,					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
+		BIT(IIO_CHAN_INFO_OFFSET),		\
+	.scan_type = {					\
+		.sign = 'u',				\
+		.realbits = (_bits),			\
+		.storagebits = 16,			\
+		.shift = 16 - (_bits),			\
+	},						\
+}
+
+static const struct ad5761_chip_info ad5761_chip_infos[] = {
+	[ID_AD5721] = {
+		.int_vref = 0,
+		.channel = AD5761_CHAN(12),
+	},
+	[ID_AD5721R] = {
+		.int_vref = 2500,
+		.channel = AD5761_CHAN(12),
+	},
+	[ID_AD5761] = {
+		.int_vref = 0,
+		.channel = AD5761_CHAN(16),
+	},
+	[ID_AD5761R] = {
+		.int_vref = 2500,
+		.channel = AD5761_CHAN(16),
+	},
+};
+
+static int ad5761_get_vref(struct ad5761_state *st,
+			   const struct ad5761_chip_info *chip_info)
+{
+	int ret;
+
+	st->vref_reg = devm_regulator_get_optional(&st->spi->dev, "vref");
+	if (!st->vref_reg || PTR_ERR(st->vref_reg) == -ENODEV) {
+		/* Use Internal regulator */
+		if (!chip_info->int_vref) {
+			dev_err(&st->spi->dev,
+				"Voltage reference not found\n");
+			return -EIO;
+		}
+
+		st->use_intref = true;
+		st->vref = chip_info->int_vref;
+		return 0;
+	}
+
+	if (IS_ERR(st->vref_reg)) {
+		dev_err(&st->spi->dev,
+			"Error getting voltage reference regulator\n");
+		return PTR_ERR(st->vref_reg);
+	}
+
+	ret = regulator_enable(st->vref_reg);
+	if (ret) {
+		dev_err(&st->spi->dev,
+			 "Failed to enable voltage reference\n");
+		return ret;
+	}
+
+	ret = regulator_get_voltage(st->vref_reg);
+	if (ret < 0) {
+		dev_err(&st->spi->dev,
+			 "Failed to get voltage reference value\n");
+		goto disable_regulator_vref;
+	}
+
+	if (ret < 2000000 || ret > 3000000) {
+		dev_warn(&st->spi->dev,
+			 "Invalid external voltage ref. value %d uV\n", ret);
+		ret = -EIO;
+		goto disable_regulator_vref;
+	}
+
+	st->vref = ret / 1000;
+	st->use_intref = false;
+
+	return 0;
+
+disable_regulator_vref:
+	regulator_disable(st->vref_reg);
+	st->vref_reg = NULL;
+	return ret;
+}
+
+static int ad5761_probe(struct spi_device *spi)
+{
+	struct iio_dev *iio_dev;
+	struct ad5761_state *st;
+	int ret;
+	const struct ad5761_chip_info *chip_info =
+		&ad5761_chip_infos[spi_get_device_id(spi)->driver_data];
+	enum ad5761_voltage_range voltage_range = AD5761_VOLTAGE_RANGE_0V_5V;
+	struct ad5761_platform_data *pdata = dev_get_platdata(&spi->dev);
+
+	iio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!iio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(iio_dev);
+
+	st->spi = spi;
+	spi_set_drvdata(spi, iio_dev);
+
+	ret = ad5761_get_vref(st, chip_info);
+	if (ret)
+		return ret;
+
+	if (pdata)
+		voltage_range = pdata->voltage_range;
+
+	ret = ad5761_spi_set_range(st, voltage_range);
+	if (ret)
+		goto disable_regulator_err;
+
+	iio_dev->dev.parent = &spi->dev;
+	iio_dev->info = &ad5761_info;
+	iio_dev->modes = INDIO_DIRECT_MODE;
+	iio_dev->channels = &chip_info->channel;
+	iio_dev->num_channels = 1;
+	iio_dev->name = spi_get_device_id(st->spi)->name;
+	ret = iio_device_register(iio_dev);
+	if (ret)
+		goto disable_regulator_err;
+
+	return 0;
+
+disable_regulator_err:
+	if (!IS_ERR_OR_NULL(st->vref_reg))
+		regulator_disable(st->vref_reg);
+
+	return ret;
+}
+
+static int ad5761_remove(struct spi_device *spi)
+{
+	struct iio_dev *iio_dev = spi_get_drvdata(spi);
+	struct ad5761_state *st = iio_priv(iio_dev);
+
+	iio_device_unregister(iio_dev);
+
+	if (!IS_ERR_OR_NULL(st->vref_reg))
+		regulator_disable(st->vref_reg);
+
+	return 0;
+}
+
+static const struct spi_device_id ad5761_id[] = {
+	{"ad5721", ID_AD5721},
+	{"ad5721r", ID_AD5721R},
+	{"ad5761", ID_AD5761},
+	{"ad5761r", ID_AD5761R},
+	{}
+};
+MODULE_DEVICE_TABLE(spi, ad5761_id);
+
+static struct spi_driver ad5761_driver = {
+	.driver = {
+		   .name = "ad5761",
+		   },
+	.probe = ad5761_probe,
+	.remove = ad5761_remove,
+	.id_table = ad5761_id,
+};
+module_spi_driver(ad5761_driver);
+
+MODULE_AUTHOR("Ricardo Ribalda <ricardo.ribalda@gmail.com>");
+MODULE_DESCRIPTION("Analog Devices AD5721, AD5721R, AD5761, AD5761R driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/iio/dac/ad5761.h b/include/linux/iio/dac/ad5761.h
new file mode 100644
index 000000000000..e3699aff627b
--- /dev/null
+++ b/include/linux/iio/dac/ad5761.h
@@ -0,0 +1,36 @@
+#ifndef __IIO_DAC_AD5761_H__
+#define __IIO_DAC_AD5761_H__
+
+/**
+ * enum ad5761_voltage_range - Voltage range the AD5761 is configured for.
+ * @AD5761_VOLTAGE_RANGE_M10V_10V:  -10V to  10V
+ * @AD5761_VOLTAGE_RANGE_0V_10V:      0V to  10V
+ * @AD5761_VOLTAGE_RANGE_M5V_5V:     -5V to   5V
+ * @AD5761_VOLTAGE_RANGE_M2V5_7V5: -2.5V to 7.5V
+ * @AD5761_VOLTAGE_RANGE_M3V_3V:     -3V to   3V
+ * @AD5761_VOLTAGE_RANGE_0V_16V:      0V to  16V
+ * @AD5761_VOLTAGE_RANGE_0V_20V:      0V to  20V
+ */
+
+enum ad5761_voltage_range {
+	AD5761_VOLTAGE_RANGE_M10V_10V,
+	AD5761_VOLTAGE_RANGE_0V_10V,
+	AD5761_VOLTAGE_RANGE_M5V_5V,
+	AD5761_VOLTAGE_RANGE_0V_5V,
+	AD5761_VOLTAGE_RANGE_M2V5_7V5,
+	AD5761_VOLTAGE_RANGE_M3V_3V,
+	AD5761_VOLTAGE_RANGE_0V_16V,
+	AD5761_VOLTAGE_RANGE_0V_20V,
+};
+
+/**
+ * struct ad5761_platform_data - AD5761 DAC driver platform data
+ * @voltage_range: Voltage range the AD5761 is configured for
+ */
+
+struct ad5761_platform_data {
+	enum ad5761_voltage_range voltage_range;
+};
+
+
+#endif
-- 
2.6.2


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

* Re: [PATCH] iio: add ad5761 DAC driver
  2016-01-05 22:41 Ricardo Ribalda Delgado
@ 2016-01-05 22:42 ` Ricardo Ribalda Delgado
  0 siblings, 0 replies; 8+ messages in thread
From: Ricardo Ribalda Delgado @ 2016-01-05 22:42 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Hartmut Knaack, Peter Meerwald, linux-iio, LKML
  Cc: Ricardo Ribalda Delgado

This is v5, I forgot to add it to the subject sorry.



On Tue, Jan 5, 2016 at 11:41 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> ad5761 is a 1-channel DAC with configurable output range.
> The driver uses the regulator interface for its voltage ref.
>
> It shares its register layout with ad5761r, ad5721 and ad5721r.
>
> Differences:
> ad5761* are 16 bit, ad5721* are 12 bits.
> ad57*1r have an internal reference.
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
> v5: CodeStyle and check val2 value
>     Thanks to Peter Meerwald-Stadler <pmeerw@pmeerw.net> and
>
> v4: Use locking mechanism
>     Populate errors
>     Thanks to Lars-Peter Clausen <lars@metafoo.de>
>
> v3: Move programmable voltage range to pdata
>     Exit on error instead of using internal voltage reference
>     Thanks to Jonathan Cameron <jic23@kernel.org>
>
> v2: A lot of CodeStyle Fixes
>     Thanks to Peter Meerwald-Stadler <pmeerw@pmeerw.net> and
>     Jonathan Cameron <jic23@kernel.org>
>
>  CREDITS                        |   1 +
>  drivers/iio/dac/Kconfig        |  10 +
>  drivers/iio/dac/Makefile       |   1 +
>  drivers/iio/dac/ad5761.c       | 432 +++++++++++++++++++++++++++++++++++++++++
>  include/linux/iio/dac/ad5761.h |  36 ++++
>  5 files changed, 480 insertions(+)
>  create mode 100644 drivers/iio/dac/ad5761.c
>  create mode 100644 include/linux/iio/dac/ad5761.h
>
> diff --git a/CREDITS b/CREDITS
> index 8207cc62ee9d..44ea433d70a1 100644
> --- a/CREDITS
> +++ b/CREDITS
> @@ -3035,6 +3035,7 @@ D: PLX USB338x driver
>  D: PCA9634 driver
>  D: Option GTM671WFS
>  D: Fintek F81216A
> +D: AD5761 iio driver
>  D: Various kernel hacks
>  S: Qtechnology A/S
>  S: Valby Langgade 142
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index e701e28fb1cd..4caedd671360 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -111,6 +111,16 @@ config AD5755
>           To compile this driver as a module, choose M here: the
>           module will be called ad5755.
>
> +config AD5761
> +       tristate "Analog Devices AD5761/61R/21/21R DAC driver"
> +       depends on SPI_MASTER
> +       help
> +         Say yes here to build support for Analog Devices AD5761, AD5761R, AD5721,
> +         AD5721R Digital to Analog Converter.
> +
> +         To compile this driver as a module, choose M here: the
> +         module will be called ad5761.
> +
>  config AD5764
>         tristate "Analog Devices AD5764/64R/44/44R DAC driver"
>         depends on SPI_MASTER
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 63ae05633e0c..cb525b53fc7b 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_AD5504) += ad5504.o
>  obj-$(CONFIG_AD5446) += ad5446.o
>  obj-$(CONFIG_AD5449) += ad5449.o
>  obj-$(CONFIG_AD5755) += ad5755.o
> +obj-$(CONFIG_AD5761) += ad5761.o
>  obj-$(CONFIG_AD5764) += ad5764.o
>  obj-$(CONFIG_AD5791) += ad5791.o
>  obj-$(CONFIG_AD5686) += ad5686.o
> diff --git a/drivers/iio/dac/ad5761.c b/drivers/iio/dac/ad5761.c
> new file mode 100644
> index 000000000000..433dd7254c10
> --- /dev/null
> +++ b/drivers/iio/dac/ad5761.c
> @@ -0,0 +1,432 @@
> +/*
> + * AD5721, AD5721R, AD5761, AD5761R, Voltage Output Digital to Analog Converter
> + *
> + * Copyright 2016 Qtechnology A/S
> + * 2016 Ricardo Ribalda <ricardo.ribalda@gmail.com>
> + *
> + * Licensed under the GPL-2.
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/bitops.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/iio/dac/ad5761.h>
> +
> +#define AD5761_ADDR(addr)              ((addr & 0xf) << 16)
> +#define AD5761_ADDR_NOOP               0x0
> +#define AD5761_ADDR_DAC_WRITE          0x3
> +#define AD5761_ADDR_CTRL_WRITE_REG     0x4
> +#define AD5761_ADDR_SW_DATA_RESET      0x7
> +#define AD5761_ADDR_DAC_READ           0xb
> +#define AD5761_ADDR_CTRL_READ_REG      0xc
> +#define AD5761_ADDR_SW_FULL_RESET      0xf
> +
> +#define AD5761_CTRL_USE_INTVREF                BIT(5)
> +#define AD5761_CTRL_ETS                        BIT(6)
> +
> +/**
> + * struct ad5761_chip_info - chip specific information
> + * @int_vref:  Value of the internal reference voltage in mV - 0 if external
> + *             reference voltage is used
> + * @channel:   channel specification
> +*/
> +
> +struct ad5761_chip_info {
> +       unsigned long int_vref;
> +       const struct iio_chan_spec channel;
> +};
> +
> +struct ad5761_range_params {
> +       int m;
> +       int c;
> +};
> +
> +enum ad5761_supported_device_ids {
> +       ID_AD5721,
> +       ID_AD5721R,
> +       ID_AD5761,
> +       ID_AD5761R,
> +};
> +
> +/**
> + * struct ad5761_state - driver instance specific data
> + * @spi:               spi_device
> + * @vref_reg:          reference voltage regulator
> + * @use_intref:                true when the internal voltage reference is used
> + * @vref:              actual voltage reference in mVolts
> + * @range:             output range mode used
> + * @data:              cache aligned spi buffer
> + */
> +struct ad5761_state {
> +       struct spi_device               *spi;
> +       struct regulator                *vref_reg;
> +
> +       bool use_intref;
> +       int vref;
> +       enum ad5761_voltage_range range;
> +
> +       /*
> +        * 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 struct ad5761_range_params ad5761_range_params[] = {
> +       [AD5761_VOLTAGE_RANGE_M10V_10V] = {
> +               .m = 80,
> +               .c = 40,
> +       },
> +       [AD5761_VOLTAGE_RANGE_0V_10V] = {
> +               .m = 40,
> +               .c = 0,
> +       },
> +       [AD5761_VOLTAGE_RANGE_M5V_5V] = {
> +               .m = 40,
> +               .c = 20,
> +       },
> +       [AD5761_VOLTAGE_RANGE_0V_5V] = {
> +               .m = 20,
> +               .c = 0,
> +       },
> +       [AD5761_VOLTAGE_RANGE_M2V5_7V5] = {
> +               .m = 40,
> +               .c = 10,
> +       },
> +       [AD5761_VOLTAGE_RANGE_M3V_3V] = {
> +               .m = 24,
> +               .c = 12,
> +       },
> +       [AD5761_VOLTAGE_RANGE_0V_16V] = {
> +               .m = 64,
> +               .c = 0,
> +       },
> +       [AD5761_VOLTAGE_RANGE_0V_20V] = {
> +               .m = 80,
> +               .c = 0,
> +       },
> +};
> +
> +static int _ad5761_spi_write(struct ad5761_state *st, u8 addr, u16 val)
> +{
> +       st->data[0].d32 = cpu_to_be32(AD5761_ADDR(addr) | val);
> +
> +       return spi_write(st->spi, &st->data[0].d8[1], 3);
> +}
> +
> +static int ad5761_spi_write(struct iio_dev *indio_dev, u8 addr, u16 val)
> +{
> +       struct ad5761_state *st = iio_priv(indio_dev);
> +       int ret;
> +
> +       mutex_lock(&indio_dev->mlock);
> +       ret = _ad5761_spi_write(st, addr, val);
> +       mutex_unlock(&indio_dev->mlock);
> +
> +       return ret;
> +}
> +
> +static int _ad5761_spi_read(struct ad5761_state *st, u8 addr, u16 *val)
> +{
> +       int ret;
> +       struct spi_transfer xfers[] = {
> +               {
> +                       .tx_buf = &st->data[0].d8[1],
> +                       .bits_per_word = 8,
> +                       .len = 3,
> +                       .cs_change = true,
> +               }, {
> +                       .tx_buf = &st->data[1].d8[1],
> +                       .rx_buf = &st->data[2].d8[1],
> +                       .bits_per_word = 8,
> +                       .len = 3,
> +               },
> +       };
> +
> +       st->data[0].d32 = cpu_to_be32(AD5761_ADDR(addr));
> +       st->data[1].d32 = cpu_to_be32(AD5761_ADDR(AD5761_ADDR_NOOP));
> +
> +       ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
> +
> +       *val = be32_to_cpu(st->data[2].d32);
> +
> +       return ret;
> +}
> +
> +static int ad5761_spi_read(struct iio_dev *indio_dev, u8 addr, u16 *val)
> +{
> +       struct ad5761_state *st = iio_priv(indio_dev);
> +       int ret;
> +
> +       mutex_lock(&indio_dev->mlock);
> +       ret = _ad5761_spi_read(st, addr, val);
> +       mutex_unlock(&indio_dev->mlock);
> +
> +       return ret;
> +}
> +
> +static int ad5761_spi_set_range(struct ad5761_state *st,
> +                               enum ad5761_voltage_range range)
> +{
> +       u16 aux;
> +       int ret;
> +
> +       aux = (range & 0x7) | AD5761_CTRL_ETS;
> +
> +       if (st->use_intref)
> +               aux |= AD5761_CTRL_USE_INTVREF;
> +
> +       ret = _ad5761_spi_write(st, AD5761_ADDR_SW_FULL_RESET, 0);
> +       if (ret)
> +               return ret;
> +
> +       ret = _ad5761_spi_write(st, AD5761_ADDR_CTRL_WRITE_REG, aux);
> +       if (ret)
> +               return ret;
> +
> +       st->range = range;
> +
> +       return 0;
> +}
> +
> +static int ad5761_read_raw(struct iio_dev *indio_dev,
> +                          struct iio_chan_spec const *chan,
> +                          int *val,
> +                          int *val2,
> +                          long mask)
> +{
> +       struct ad5761_state *st;
> +       int ret;
> +       u16 aux;
> +
> +       switch (mask) {
> +       case IIO_CHAN_INFO_RAW:
> +               ret = ad5761_spi_read(indio_dev, AD5761_ADDR_DAC_READ, &aux);
> +               if (ret)
> +                       return ret;
> +               *val = aux >> chan->scan_type.shift;
> +               *val2 = 0;
> +               return IIO_VAL_INT;
> +       case IIO_CHAN_INFO_SCALE:
> +               st = iio_priv(indio_dev);
> +               *val = st->vref * ad5761_range_params[st->range].m;
> +               *val /= 10;
> +               *val2 = chan->scan_type.realbits;
> +               return IIO_VAL_FRACTIONAL_LOG2;
> +       case IIO_CHAN_INFO_OFFSET:
> +               st = iio_priv(indio_dev);
> +               *val = -(1 << chan->scan_type.realbits);
> +               *val *= ad5761_range_params[st->range].c;
> +               *val /= ad5761_range_params[st->range].m;
> +               *val2 = 0;
> +               return IIO_VAL_INT;
> +       default:
> +               return -EINVAL;
> +       }
> +}
> +
> +static int ad5761_write_raw(struct iio_dev *indio_dev,
> +                           struct iio_chan_spec const *chan,
> +                           int val,
> +                           int val2,
> +                           long mask)
> +{
> +       u16 aux;
> +
> +       if (mask != IIO_CHAN_INFO_RAW)
> +               return -EINVAL;
> +
> +       if (val2 || (val << chan->scan_type.shift) > 0xffff || val < 0)
> +               return -EINVAL;
> +
> +       aux = val << chan->scan_type.shift;
> +
> +       return ad5761_spi_write(indio_dev, AD5761_ADDR_DAC_WRITE, aux);
> +}
> +
> +static const struct iio_info ad5761_info = {
> +       .read_raw = &ad5761_read_raw,
> +       .write_raw = &ad5761_write_raw,
> +       .driver_module = THIS_MODULE,
> +};
> +
> +#define AD5761_CHAN(_bits) {                           \
> +       .type = IIO_VOLTAGE,                            \
> +       .output = 1,                                    \
> +       .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),   \
> +       .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |  \
> +               BIT(IIO_CHAN_INFO_OFFSET),              \
> +       .scan_type = {                                  \
> +               .sign = 'u',                            \
> +               .realbits = (_bits),                    \
> +               .storagebits = 16,                      \
> +               .shift = 16 - (_bits),                  \
> +       },                                              \
> +}
> +
> +static const struct ad5761_chip_info ad5761_chip_infos[] = {
> +       [ID_AD5721] = {
> +               .int_vref = 0,
> +               .channel = AD5761_CHAN(12),
> +       },
> +       [ID_AD5721R] = {
> +               .int_vref = 2500,
> +               .channel = AD5761_CHAN(12),
> +       },
> +       [ID_AD5761] = {
> +               .int_vref = 0,
> +               .channel = AD5761_CHAN(16),
> +       },
> +       [ID_AD5761R] = {
> +               .int_vref = 2500,
> +               .channel = AD5761_CHAN(16),
> +       },
> +};
> +
> +static int ad5761_get_vref(struct ad5761_state *st,
> +                          const struct ad5761_chip_info *chip_info)
> +{
> +       int ret;
> +
> +       st->vref_reg = devm_regulator_get_optional(&st->spi->dev, "vref");
> +       if (!st->vref_reg || PTR_ERR(st->vref_reg) == -ENODEV) {
> +               /* Use Internal regulator */
> +               if (!chip_info->int_vref) {
> +                       dev_err(&st->spi->dev,
> +                               "Voltage reference not found\n");
> +                       return -EIO;
> +               }
> +
> +               st->use_intref = true;
> +               st->vref = chip_info->int_vref;
> +               return 0;
> +       }
> +
> +       if (IS_ERR(st->vref_reg)) {
> +               dev_err(&st->spi->dev,
> +                       "Error getting voltage reference regulator\n");
> +               return PTR_ERR(st->vref_reg);
> +       }
> +
> +       ret = regulator_enable(st->vref_reg);
> +       if (ret) {
> +               dev_err(&st->spi->dev,
> +                        "Failed to enable voltage reference\n");
> +               return ret;
> +       }
> +
> +       ret = regulator_get_voltage(st->vref_reg);
> +       if (ret < 0) {
> +               dev_err(&st->spi->dev,
> +                        "Failed to get voltage reference value\n");
> +               goto disable_regulator_vref;
> +       }
> +
> +       if (ret < 2000000 || ret > 3000000) {
> +               dev_warn(&st->spi->dev,
> +                        "Invalid external voltage ref. value %d uV\n", ret);
> +               ret = -EIO;
> +               goto disable_regulator_vref;
> +       }
> +
> +       st->vref = ret / 1000;
> +       st->use_intref = false;
> +
> +       return 0;
> +
> +disable_regulator_vref:
> +       regulator_disable(st->vref_reg);
> +       st->vref_reg = NULL;
> +       return ret;
> +}
> +
> +static int ad5761_probe(struct spi_device *spi)
> +{
> +       struct iio_dev *iio_dev;
> +       struct ad5761_state *st;
> +       int ret;
> +       const struct ad5761_chip_info *chip_info =
> +               &ad5761_chip_infos[spi_get_device_id(spi)->driver_data];
> +       enum ad5761_voltage_range voltage_range = AD5761_VOLTAGE_RANGE_0V_5V;
> +       struct ad5761_platform_data *pdata = dev_get_platdata(&spi->dev);
> +
> +       iio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +       if (!iio_dev)
> +               return -ENOMEM;
> +
> +       st = iio_priv(iio_dev);
> +
> +       st->spi = spi;
> +       spi_set_drvdata(spi, iio_dev);
> +
> +       ret = ad5761_get_vref(st, chip_info);
> +       if (ret)
> +               return ret;
> +
> +       if (pdata)
> +               voltage_range = pdata->voltage_range;
> +
> +       ret = ad5761_spi_set_range(st, voltage_range);
> +       if (ret)
> +               goto disable_regulator_err;
> +
> +       iio_dev->dev.parent = &spi->dev;
> +       iio_dev->info = &ad5761_info;
> +       iio_dev->modes = INDIO_DIRECT_MODE;
> +       iio_dev->channels = &chip_info->channel;
> +       iio_dev->num_channels = 1;
> +       iio_dev->name = spi_get_device_id(st->spi)->name;
> +       ret = iio_device_register(iio_dev);
> +       if (ret)
> +               goto disable_regulator_err;
> +
> +       return 0;
> +
> +disable_regulator_err:
> +       if (!IS_ERR_OR_NULL(st->vref_reg))
> +               regulator_disable(st->vref_reg);
> +
> +       return ret;
> +}
> +
> +static int ad5761_remove(struct spi_device *spi)
> +{
> +       struct iio_dev *iio_dev = spi_get_drvdata(spi);
> +       struct ad5761_state *st = iio_priv(iio_dev);
> +
> +       iio_device_unregister(iio_dev);
> +
> +       if (!IS_ERR_OR_NULL(st->vref_reg))
> +               regulator_disable(st->vref_reg);
> +
> +       return 0;
> +}
> +
> +static const struct spi_device_id ad5761_id[] = {
> +       {"ad5721", ID_AD5721},
> +       {"ad5721r", ID_AD5721R},
> +       {"ad5761", ID_AD5761},
> +       {"ad5761r", ID_AD5761R},
> +       {}
> +};
> +MODULE_DEVICE_TABLE(spi, ad5761_id);
> +
> +static struct spi_driver ad5761_driver = {
> +       .driver = {
> +                  .name = "ad5761",
> +                  },
> +       .probe = ad5761_probe,
> +       .remove = ad5761_remove,
> +       .id_table = ad5761_id,
> +};
> +module_spi_driver(ad5761_driver);
> +
> +MODULE_AUTHOR("Ricardo Ribalda <ricardo.ribalda@gmail.com>");
> +MODULE_DESCRIPTION("Analog Devices AD5721, AD5721R, AD5761, AD5761R driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/iio/dac/ad5761.h b/include/linux/iio/dac/ad5761.h
> new file mode 100644
> index 000000000000..e3699aff627b
> --- /dev/null
> +++ b/include/linux/iio/dac/ad5761.h
> @@ -0,0 +1,36 @@
> +#ifndef __IIO_DAC_AD5761_H__
> +#define __IIO_DAC_AD5761_H__
> +
> +/**
> + * enum ad5761_voltage_range - Voltage range the AD5761 is configured for.
> + * @AD5761_VOLTAGE_RANGE_M10V_10V:  -10V to  10V
> + * @AD5761_VOLTAGE_RANGE_0V_10V:      0V to  10V
> + * @AD5761_VOLTAGE_RANGE_M5V_5V:     -5V to   5V
> + * @AD5761_VOLTAGE_RANGE_M2V5_7V5: -2.5V to 7.5V
> + * @AD5761_VOLTAGE_RANGE_M3V_3V:     -3V to   3V
> + * @AD5761_VOLTAGE_RANGE_0V_16V:      0V to  16V
> + * @AD5761_VOLTAGE_RANGE_0V_20V:      0V to  20V
> + */
> +
> +enum ad5761_voltage_range {
> +       AD5761_VOLTAGE_RANGE_M10V_10V,
> +       AD5761_VOLTAGE_RANGE_0V_10V,
> +       AD5761_VOLTAGE_RANGE_M5V_5V,
> +       AD5761_VOLTAGE_RANGE_0V_5V,
> +       AD5761_VOLTAGE_RANGE_M2V5_7V5,
> +       AD5761_VOLTAGE_RANGE_M3V_3V,
> +       AD5761_VOLTAGE_RANGE_0V_16V,
> +       AD5761_VOLTAGE_RANGE_0V_20V,
> +};
> +
> +/**
> + * struct ad5761_platform_data - AD5761 DAC driver platform data
> + * @voltage_range: Voltage range the AD5761 is configured for
> + */
> +
> +struct ad5761_platform_data {
> +       enum ad5761_voltage_range voltage_range;
> +};
> +
> +
> +#endif
> --
> 2.6.2
>



-- 
Ricardo Ribalda

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

end of thread, other threads:[~2016-01-05 22:42 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-16 13:27 [PATCH] iio: add ad5761 DAC driver Ricardo Ribalda Delgado
2015-12-19 16:31 ` Jonathan Cameron
2015-12-20 11:19   ` Ricardo Ribalda Delgado
2015-12-22 18:13     ` Jonathan Cameron
2015-12-19 17:06 ` Peter Meerwald-Stadler
2015-12-20 11:24   ` Ricardo Ribalda Delgado
  -- strict thread matches above, loose matches on Subject: below --
2016-01-05 22:41 Ricardo Ribalda Delgado
2016-01-05 22:42 ` Ricardo Ribalda Delgado

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox