[PATCH] iio: dac: Add support for ltc2632 DACs

[PATCH] iio: dac: Add support for ltc2632 DACs

[Maxime Roussin-Belanger]

Signed-off-by: Maxime Roussin-Belanger <maxime.roussinbelanger@gmail.com>
---
 .../devicetree/bindings/iio/dac/ltc2632.txt        |  23 ++
 drivers/iio/dac/Kconfig                            |   7 +
 drivers/iio/dac/Makefile                           |   1 +
 drivers/iio/dac/ltc2632.c                          | 325 +++++++++++++++++++++
 4 files changed, 356 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/dac/ltc2632.txt
 create mode 100644 drivers/iio/dac/ltc2632.c

diff --git a/Documentation/devicetree/bindings/iio/dac/ltc2632.txt b/Documentation/devicetree/bindings/iio/dac/ltc2632.txt
new file mode 100644
index 0000000..fd9aa0b
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/ltc2632.txt
@@ -0,0 +1,23 @@
+Linear Technology LTC2632 DAC device driver
+
+Required properties:
+ - compatible: Has to contain one of the following
+	lltc,ltc2632-l12
+	lltc,ltc2632-l10
+	lltc,ltc2632-l8
+	lltc,ltc2632-h12
+	lltc,ltc2632-h10
+	lltc,ltc2632-h8
+
+Property rules described in Documentation/devicetree/bindings/spi/spi-bus.txt
+apply. In particular, "reg" and "spi-max-frequency" properties must be given.
+
+Example:
+
+	spi_master {
+		dac: ltc2632@0 {
+			compatible = "lltc,ltc2632";
+			reg = <0>; /* CS0 */
+			spi-max-frequency = <1000000>;
+		};
+	};
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index d3084028..f32ca8a 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -118,6 +118,13 @@ config AD5624R_SPI
 	  Say yes here to build support for Analog Devices AD5624R, AD5644R and
 	  AD5664R converters (DAC). This driver uses the common SPI interface.
 
+config LTC2632
+	tristate "Linear Technology LTC2632-12/10/8 DAC spi driver"
+	depends on SPI
+	help
+	  Say yes here to build support for Linear Technology LTC2632-12/10/8 converters (DAC).
+	  This driver uses the common SPI interface.
+
 config AD5686
 	tristate "Analog Devices AD5686R/AD5685R/AD5684R DAC SPI driver"
 	depends on SPI
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index f01bf4a..6e4d995 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_AD8801) += ad8801.o
 obj-$(CONFIG_CIO_DAC) += cio-dac.o
 obj-$(CONFIG_DPOT_DAC) += dpot-dac.o
 obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o
+obj-$(CONFIG_LTC2632) += ltc2632.o
 obj-$(CONFIG_M62332) += m62332.o
 obj-$(CONFIG_MAX517) += max517.o
 obj-$(CONFIG_MAX5821) += max5821.o
diff --git a/drivers/iio/dac/ltc2632.c b/drivers/iio/dac/ltc2632.c
new file mode 100644
index 0000000..d6160fb
--- /dev/null
+++ b/drivers/iio/dac/ltc2632.c
@@ -0,0 +1,325 @@
+/*
+ * LTC2632 Digital to analog convertors spi driver
+ */
+
+#include <linux/device.h>
+#include <linux/spi/spi.h>
+#include <linux/module.h>
+
+#include <linux/iio/iio.h>
+
+#define LTC2632_DAC_CHANNELS                    2
+
+#define LTC2632_ADDR_DAC0                       0x0
+#define LTC2632_ADDR_DAC1                       0x1
+
+#define LTC2632_CMD_WRITE_INPUT_N               0x0
+#define LTC2632_CMD_UPDATE_DAC_N                0x1
+#define LTC2632_CMD_WRITE_INPUT_N_UPDATE_ALL    0x2
+#define LTC2632_CMD_WRITE_INPUT_N_UPDATE_N      0x3
+#define LTC2632_CMD_POWERDOWN_DAC_N             0x4
+#define LTC2632_CMD_POWERDOWN_CHIP              0x5
+#define LTC2632_CMD_INTERNAL_REFER              0x6
+#define LTC2632_CMD_EXTERNAL_REFER              0x7
+
+
+static ssize_t ltc2632_read_dac_powerdown(struct iio_dev *indio_dev,
+					  uintptr_t private,
+					  const struct iio_chan_spec *chan,
+					  char *buf);
+
+static ssize_t ltc2632_write_dac_powerdown(struct iio_dev *indio_dev,
+					   uintptr_t private,
+					   const struct iio_chan_spec *chan,
+					   const char *buf,
+					   size_t len);
+
+/**
+ * struct ltc2632_chip_info - chip specific information
+ * @channels:		channel spec for the DAC
+ */
+struct ltc2632_chip_info {
+	const struct iio_chan_spec *channels;
+	const int vref_mv;
+};
+
+/**
+ * struct ltc2632_state - driver instance specific data
+ * @spi_dev:		spi_device
+ * @pwr_down_cached	power down mask
+ */
+struct ltc2632_state {
+	struct spi_device *spi_dev;
+	unsigned int pwr_down_cached;
+};
+
+/**
+ * ltc2632_supported_device_ids
+ */
+enum ltc2632_supported_device_ids {
+	ID_LTC2632L12,
+	ID_LTC2632L10,
+	ID_LTC2632L8,
+	ID_LTC2632H12,
+	ID_LTC2632H10,
+	ID_LTC2632H8,
+};
+
+static const struct iio_chan_spec_ext_info ltc2632_ext_info[] = {
+	{
+		.name = "powerdown",
+		.read = ltc2632_read_dac_powerdown,
+		.write = ltc2632_write_dac_powerdown,
+		.shared = IIO_SEPARATE,
+	},
+	{ },
+};
+
+#define LTC2632_CHANNEL(_chan, _bits) { \
+		.type = IIO_VOLTAGE, \
+		.indexed = 1, \
+		.output = 1, \
+		.channel = (_chan), \
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+		.address = (_chan), \
+		.scan_type = { \
+			.sign		= 'u', \
+			.realbits	= (_bits), \
+			.storagebits	= 16, \
+			.shift		= 16 - (_bits), \
+		}, \
+		.ext_info = ltc2632_ext_info, \
+}
+
+#define DECLARE_LTC2632_CHANNELS(_name, _bits) \
+	const struct iio_chan_spec _name ## _channels[] = { \
+		LTC2632_CHANNEL(0, _bits), \
+		LTC2632_CHANNEL(1, _bits), \
+	}
+
+static DECLARE_LTC2632_CHANNELS(ltc2632l12, 12);
+static DECLARE_LTC2632_CHANNELS(ltc2632l10, 10);
+static DECLARE_LTC2632_CHANNELS(ltc2632l8, 8);
+
+static DECLARE_LTC2632_CHANNELS(ltc2632h12, 12);
+static DECLARE_LTC2632_CHANNELS(ltc2632h10, 10);
+static DECLARE_LTC2632_CHANNELS(ltc2632h8, 8);
+
+static const struct ltc2632_chip_info ltc2632_chip_info_tbl[] = {
+	[ID_LTC2632L12] = {
+		.channels	= ltc2632l12_channels,
+		.vref_mv	= 2500,
+	},
+	[ID_LTC2632L10] = {
+		.channels	= ltc2632l10_channels,
+		.vref_mv	= 2500,
+	},
+	[ID_LTC2632L8] =  {
+		.channels	= ltc2632l8_channels,
+		.vref_mv	= 2500,
+	},
+	[ID_LTC2632H12] = {
+		.channels	= ltc2632h12_channels,
+		.vref_mv	= 4096,
+	},
+	[ID_LTC2632H10] = {
+		.channels	= ltc2632h10_channels,
+		.vref_mv	= 4096,
+	},
+	[ID_LTC2632H8] =  {
+		.channels	= ltc2632h8_channels,
+		.vref_mv	= 4096,
+	},
+};
+
+static int ltc2632_spi_write(struct spi_device *spi,
+			     u8 cmd, u8 addr, u16 val, u8 shift)
+{
+	u32 data;
+	u8 msg[3];
+
+	/*
+	 * The input shift register is 24 bits wide.
+	 * The next four are the command bits, C3 to C0,
+	 * followed by the 4-bit DAC address, A3 to A0, and then the
+	 * 12-, 10-, 8-bit data-word. The data-word comprises the 12-,
+	 * 10-, 8-bit input code followed by 4, 6, or 8 don't care bits.
+	 */
+	data = (cmd << 20) | (addr << 16) | (val << shift);
+	msg[0] = data >> 16;
+	msg[1] = data >> 8;
+	msg[2] = data;
+
+	return spi_write(spi, msg, 3);
+}
+
+static int ltc2632_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val,
+			    int *val2,
+			    long m)
+{
+	struct ltc2632_chip_info *chip_info;
+
+	const struct ltc2632_state *st = iio_priv(indio_dev);
+	const struct spi_device_id* spi_dev_id = spi_get_device_id(st->spi_dev);
+
+	chip_info = (struct ltc2632_chip_info*) spi_dev_id->driver_data;
+
+	switch (m) {
+	case IIO_CHAN_INFO_SCALE:
+		*val = chip_info->vref_mv;
+		*val2 = chan->scan_type.realbits;
+		return IIO_VAL_FRACTIONAL_LOG2;
+	}
+	return -EINVAL;
+}
+
+static int ltc2632_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val,
+			     int val2,
+			     long mask)
+{
+	struct ltc2632_state *st = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (val >= (1 << chan->scan_type.realbits) || val < 0)
+			return -EINVAL;
+
+		return ltc2632_spi_write(st->spi_dev,
+					 LTC2632_CMD_WRITE_INPUT_N_UPDATE_N,
+					 chan->address, val,
+					 chan->scan_type.shift);
+	default:
+		return -EINVAL;
+	}
+
+}
+
+static ssize_t ltc2632_read_dac_powerdown(struct iio_dev *indio_dev,
+					  uintptr_t private,
+					  const struct iio_chan_spec *chan,
+					  char *buf)
+{
+	struct ltc2632_state *st = iio_priv(indio_dev);
+
+	return sprintf(buf, "%d\n",
+		       !!(st->pwr_down_cached & (1 << chan->channel)));
+}
+
+static ssize_t ltc2632_write_dac_powerdown(struct iio_dev *indio_dev,
+					   uintptr_t private,
+					   const struct iio_chan_spec *chan,
+					   const char *buf,
+					   size_t len)
+{
+	bool pwr_down;
+	int ret;
+	struct ltc2632_state *st = iio_priv(indio_dev);
+
+	ret = strtobool(buf, &pwr_down);
+	if (ret)
+		return ret;
+
+	if (pwr_down)
+		st->pwr_down_cached |= (1 << chan->channel);
+	else
+		st->pwr_down_cached &= ~(1 << chan->channel);
+
+	ret = ltc2632_spi_write(st->spi_dev,
+				LTC2632_CMD_POWERDOWN_DAC_N,
+				chan->channel, 0, 0);
+
+	return ret ? ret : len;
+}
+
+static const struct iio_info ltc2632_info = {
+	.write_raw	= ltc2632_write_raw,
+	.read_raw	= ltc2632_read_raw,
+	.driver_module	= THIS_MODULE,
+};
+
+static int ltc2632_probe(struct spi_device *spi)
+{
+	struct ltc2632_state *st;
+	struct iio_dev *indio_dev;
+	struct ltc2632_chip_info* chip_info;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+	st = iio_priv(indio_dev);
+
+	spi_set_drvdata(spi, indio_dev);
+	st->spi_dev = spi;
+
+	chip_info = (struct ltc2632_chip_info*)
+			spi_get_device_id(spi)->driver_data;
+
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->info = &ltc2632_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = chip_info->channels;
+	indio_dev->num_channels = LTC2632_DAC_CHANNELS;
+
+	ret = ltc2632_spi_write(spi, LTC2632_CMD_INTERNAL_REFER, 0, 0, 0);
+	if (ret)
+		dev_err(&spi->dev,
+			"Set internal reference command failed, %d\n", ret);
+
+	ret = iio_device_register(indio_dev);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ltc2632_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+
+	iio_device_unregister(indio_dev);
+
+	return 0;
+}
+
+static const struct spi_device_id ltc2632_id[] = {
+	{ "ltc2632-l12", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632L12] },
+	{ "ltc2632-l10", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632L10] },
+	{ "ltc2632-l8", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632L8] },
+	{ "ltc2632-h12", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632H12] },
+	{ "ltc2632-h10", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632H10] },
+	{ "ltc2632-h8", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632H8] },
+	{}
+};
+MODULE_DEVICE_TABLE(spi, ltc2632_id);
+
+static struct spi_driver ltc2632_driver = {
+	.driver		= {
+		.name	= "ltc2632",
+	},
+	.probe		= ltc2632_probe,
+	.remove		= ltc2632_remove,
+	.id_table	= ltc2632_id,
+};
+module_spi_driver(ltc2632_driver);
+
+static const struct of_device_id ltc2632_of_match[] = {
+	{ .compatible = "lltc,ltc2632-l12", .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632L12] },
+	{ .compatible = "lltc,ltc2632-l10", .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632L10] },
+	{ .compatible = "lltc,ltc2632-l8",  .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632L8] },
+	{ .compatible = "lltc,ltc2632-h12", .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632H12] },
+	{ .compatible = "lltc,ltc2632-h10", .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632H10] },
+	{ .compatible = "lltc,ltc2632-h8",  .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632H8] },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ltc2632_of_match);
+
+MODULE_AUTHOR("Maxime Roussin-Belanger <maxime.roussinbelanger@gmail.com>");
+MODULE_DESCRIPTION("LTC2632 DAC SPI driver");
+MODULE_LICENSE("GPL v2");
-- 
2.10.0

Re: [PATCH] iio: dac: Add support for ltc2632 DACs

[Jonathan Cameron]

On 02/03/17 19:01, Maxime Roussin-Belanger wrote:
> Signed-off-by: Maxime Roussin-Belanger <maxime.roussinbelanger@gmail.com>
A brief description of the functionality of the part and the driver as a
patch description would be good. Comes in handy when someone is looking
at the logs to see what is new.

A few minor comments inline, but basically a good little driver.

Looking forward to a V2.

thanks,

Jonathan
> ---
>  .../devicetree/bindings/iio/dac/ltc2632.txt        |  23 ++
>  drivers/iio/dac/Kconfig                            |   7 +
>  drivers/iio/dac/Makefile                           |   1 +
>  drivers/iio/dac/ltc2632.c                          | 325 +++++++++++++++++++++
>  4 files changed, 356 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/dac/ltc2632.txt
>  create mode 100644 drivers/iio/dac/ltc2632.c
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/ltc2632.txt b/Documentation/devicetree/bindings/iio/dac/ltc2632.txt
> new file mode 100644
> index 0000000..fd9aa0b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/ltc2632.txt
> @@ -0,0 +1,23 @@
> +Linear Technology LTC2632 DAC device driver
> +
> +Required properties:
> + - compatible: Has to contain one of the following
> +	lltc,ltc2632-l12
> +	lltc,ltc2632-l10
> +	lltc,ltc2632-l8
> +	lltc,ltc2632-h12
> +	lltc,ltc2632-h10
> +	lltc,ltc2632-h8
> +
> +Property rules described in Documentation/devicetree/bindings/spi/spi-bus.txt
> +apply. In particular, "reg" and "spi-max-frequency" properties must be given.
> +
> +Example:
> +
> +	spi_master {
> +		dac: ltc2632@0 {
> +			compatible = "lltc,ltc2632";
> +			reg = <0>; /* CS0 */
> +			spi-max-frequency = <1000000>;
> +		};
> +	};
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index d3084028..f32ca8a 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -118,6 +118,13 @@ config AD5624R_SPI
>  	  Say yes here to build support for Analog Devices AD5624R, AD5644R and
>  	  AD5664R converters (DAC). This driver uses the common SPI interface.
>  
> +config LTC2632
> +	tristate "Linear Technology LTC2632-12/10/8 DAC spi driver"
> +	depends on SPI
> +	help
> +	  Say yes here to build support for Linear Technology LTC2632-12/10/8 converters (DAC).
> +	  This driver uses the common SPI interface.
Whilst it is common :) that probably doesn't need to be stated here.
A little more detail about the driver would be good.  Also please list all the
part names long hand in the help text.  People tend to grep this.
> +
>  config AD5686
>  	tristate "Analog Devices AD5686R/AD5685R/AD5684R DAC SPI driver"
>  	depends on SPI
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index f01bf4a..6e4d995 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_AD8801) += ad8801.o
>  obj-$(CONFIG_CIO_DAC) += cio-dac.o
>  obj-$(CONFIG_DPOT_DAC) += dpot-dac.o
>  obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o
> +obj-$(CONFIG_LTC2632) += ltc2632.o
>  obj-$(CONFIG_M62332) += m62332.o
>  obj-$(CONFIG_MAX517) += max517.o
>  obj-$(CONFIG_MAX5821) += max5821.o
> diff --git a/drivers/iio/dac/ltc2632.c b/drivers/iio/dac/ltc2632.c
> new file mode 100644
> index 0000000..d6160fb
> --- /dev/null
> +++ b/drivers/iio/dac/ltc2632.c
> @@ -0,0 +1,325 @@
> +/*
> + * LTC2632 Digital to analog convertors spi driver
Perhaps standard copyright statement here would be a good addition?
+ minimal license.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/spi/spi.h>
> +#include <linux/module.h>
> +
> +#include <linux/iio/iio.h>
> +
> +#define LTC2632_DAC_CHANNELS                    2
> +
> +#define LTC2632_ADDR_DAC0                       0x0
> +#define LTC2632_ADDR_DAC1                       0x1
> +
> +#define LTC2632_CMD_WRITE_INPUT_N               0x0
> +#define LTC2632_CMD_UPDATE_DAC_N                0x1
> +#define LTC2632_CMD_WRITE_INPUT_N_UPDATE_ALL    0x2
> +#define LTC2632_CMD_WRITE_INPUT_N_UPDATE_N      0x3
> +#define LTC2632_CMD_POWERDOWN_DAC_N             0x4
> +#define LTC2632_CMD_POWERDOWN_CHIP              0x5
> +#define LTC2632_CMD_INTERNAL_REFER              0x6
> +#define LTC2632_CMD_EXTERNAL_REFER              0x7
> +
> +
Reorder the code so these aren't necessary.
> +static ssize_t ltc2632_read_dac_powerdown(struct iio_dev *indio_dev,
> +					  uintptr_t private,
> +					  const struct iio_chan_spec *chan,
> +					  char *buf);
> +
> +static ssize_t ltc2632_write_dac_powerdown(struct iio_dev *indio_dev,
> +					   uintptr_t private,
> +					   const struct iio_chan_spec *chan,
> +					   const char *buf,
> +					   size_t len);
> +
> +/**
> + * struct ltc2632_chip_info - chip specific information
> + * @channels:		channel spec for the DAC
> + */
> +struct ltc2632_chip_info {
> +	const struct iio_chan_spec *channels;
> +	const int vref_mv;
> +};
> +
> +/**
> + * struct ltc2632_state - driver instance specific data
> + * @spi_dev:		spi_device
> + * @pwr_down_cached	power down mask
> + */
> +struct ltc2632_state {
> +	struct spi_device *spi_dev;
> +	unsigned int pwr_down_cached;
> +};
> +
> +/**
> + * ltc2632_supported_device_ids
> + */
Given the clear enum naming, I'm not certain the comment adds anything.
So I'd drop it.
> +enum ltc2632_supported_device_ids {
> +	ID_LTC2632L12,
> +	ID_LTC2632L10,
> +	ID_LTC2632L8,
> +	ID_LTC2632H12,
> +	ID_LTC2632H10,
> +	ID_LTC2632H8,
> +};
> +
> +static const struct iio_chan_spec_ext_info ltc2632_ext_info[] = {
> +	{
> +		.name = "powerdown",
Why have this as a sysfs attribute? It's new ABI so should be documented
and justified.

Normally having explicit power up and powerdown of devices is somewhat
frowned upon as it can often be implicitly done depending on the state
of the machine etc (runtime pm and similar). If that's not the case here
please provide some explanatory comments.
> +		.read = ltc2632_read_dac_powerdown,
> +		.write = ltc2632_write_dac_powerdown,
> +		.shared = IIO_SEPARATE,
> +	},
> +	{ },
> +};
> +
> +#define LTC2632_CHANNEL(_chan, _bits) { \
> +		.type = IIO_VOLTAGE, \
> +		.indexed = 1, \
> +		.output = 1, \
> +		.channel = (_chan), \
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> +		.address = (_chan), \
> +		.scan_type = { \
> +			.sign		= 'u', \
> +			.realbits	= (_bits), \
> +			.storagebits	= 16, \
> +			.shift		= 16 - (_bits), \
Hmm. Without buffers for output devices some of this is never used, but
it does provide 'documentation' of a type so fine to leave it here.
> +		}, \
> +		.ext_info = ltc2632_ext_info, \
> +}
> +
> +#define DECLARE_LTC2632_CHANNELS(_name, _bits) \
> +	const struct iio_chan_spec _name ## _channels[] = { \
> +		LTC2632_CHANNEL(0, _bits), \
> +		LTC2632_CHANNEL(1, _bits), \
> +	}
> +
> +static DECLARE_LTC2632_CHANNELS(ltc2632l12, 12);
> +static DECLARE_LTC2632_CHANNELS(ltc2632l10, 10);
> +static DECLARE_LTC2632_CHANNELS(ltc2632l8, 8);
> +
> +static DECLARE_LTC2632_CHANNELS(ltc2632h12, 12);
> +static DECLARE_LTC2632_CHANNELS(ltc2632h10, 10);
> +static DECLARE_LTC2632_CHANNELS(ltc2632h8, 8);
> +
> +static const struct ltc2632_chip_info ltc2632_chip_info_tbl[] = {
> +	[ID_LTC2632L12] = {
> +		.channels	= ltc2632l12_channels,
> +		.vref_mv	= 2500,
> +	},
> +	[ID_LTC2632L10] = {
> +		.channels	= ltc2632l10_channels,
> +		.vref_mv	= 2500,
> +	},
> +	[ID_LTC2632L8] =  {
> +		.channels	= ltc2632l8_channels,
> +		.vref_mv	= 2500,
> +	},
> +	[ID_LTC2632H12] = {
> +		.channels	= ltc2632h12_channels,
> +		.vref_mv	= 4096,
> +	},
> +	[ID_LTC2632H10] = {
> +		.channels	= ltc2632h10_channels,
> +		.vref_mv	= 4096,
> +	},
> +	[ID_LTC2632H8] =  {
> +		.channels	= ltc2632h8_channels,
> +		.vref_mv	= 4096,
> +	},
> +};
> +
> +static int ltc2632_spi_write(struct spi_device *spi,
> +			     u8 cmd, u8 addr, u16 val, u8 shift)
> +{
> +	u32 data;
> +	u8 msg[3];
> +
> +	/*
> +	 * The input shift register is 24 bits wide.
> +	 * The next four are the command bits, C3 to C0,
> +	 * followed by the 4-bit DAC address, A3 to A0, and then the
> +	 * 12-, 10-, 8-bit data-word. The data-word comprises the 12-,
> +	 * 10-, 8-bit input code followed by 4, 6, or 8 don't care bits.
> +	 */
> +	data = (cmd << 20) | (addr << 16) | (val << shift);
> +	msg[0] = data >> 16;
> +	msg[1] = data >> 8;
> +	msg[2] = data;
> +
> +	return spi_write(spi, msg, 3);
> +}
> +
> +static int ltc2632_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val,
> +			    int *val2,
> +			    long m)
> +{
> +	struct ltc2632_chip_info *chip_info;
> +
> +	const struct ltc2632_state *st = iio_priv(indio_dev);
> +	const struct spi_device_id* spi_dev_id = spi_get_device_id(st->spi_dev);
> +
> +	chip_info = (struct ltc2632_chip_info*) spi_dev_id->driver_data;
> +
> +	switch (m) {
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = chip_info->vref_mv;
> +		*val2 = chan->scan_type.realbits;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	}
> +	return -EINVAL;
> +}
> +
> +static int ltc2632_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int val,
> +			     int val2,
> +			     long mask)
> +{
> +	struct ltc2632_state *st = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (val >= (1 << chan->scan_type.realbits) || val < 0)
> +			return -EINVAL;
> +
> +		return ltc2632_spi_write(st->spi_dev,
> +					 LTC2632_CMD_WRITE_INPUT_N_UPDATE_N,
> +					 chan->address, val,
> +					 chan->scan_type.shift);
> +	default:
> +		return -EINVAL;
> +	}
> +
> +}
> +
> +static ssize_t ltc2632_read_dac_powerdown(struct iio_dev *indio_dev,
> +					  uintptr_t private,
> +					  const struct iio_chan_spec *chan,
> +					  char *buf)
> +{
> +	struct ltc2632_state *st = iio_priv(indio_dev);
> +
> +	return sprintf(buf, "%d\n",
> +		       !!(st->pwr_down_cached & (1 << chan->channel)));
> +}
> +
> +static ssize_t ltc2632_write_dac_powerdown(struct iio_dev *indio_dev,
> +					   uintptr_t private,
> +					   const struct iio_chan_spec *chan,
> +					   const char *buf,
> +					   size_t len)
> +{
> +	bool pwr_down;
> +	int ret;
> +	struct ltc2632_state *st = iio_priv(indio_dev);
> +
> +	ret = strtobool(buf, &pwr_down);
> +	if (ret)
> +		return ret;
> +
> +	if (pwr_down)
> +		st->pwr_down_cached |= (1 << chan->channel);
> +	else
> +		st->pwr_down_cached &= ~(1 << chan->channel);
> +
> +	ret = ltc2632_spi_write(st->spi_dev,
> +				LTC2632_CMD_POWERDOWN_DAC_N,
> +				chan->channel, 0, 0);
> +
> +	return ret ? ret : len;
> +}
> +
> +static const struct iio_info ltc2632_info = {
> +	.write_raw	= ltc2632_write_raw,
> +	.read_raw	= ltc2632_read_raw,
> +	.driver_module	= THIS_MODULE,
> +};
> +
> +static int ltc2632_probe(struct spi_device *spi)
> +{
> +	struct ltc2632_state *st;
> +	struct iio_dev *indio_dev;
> +	struct ltc2632_chip_info* chip_info;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
Blank line here would make it slightly easer to read.
> +	st = iio_priv(indio_dev);
> +
> +	spi_set_drvdata(spi, indio_dev);
> +	st->spi_dev = spi;
> +
> +	chip_info = (struct ltc2632_chip_info*)
> +			spi_get_device_id(spi)->driver_data;
> +
> +	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->info = &ltc2632_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = chip_info->channels;
> +	indio_dev->num_channels = LTC2632_DAC_CHANNELS;
> +
> +	ret = ltc2632_spi_write(spi, LTC2632_CMD_INTERNAL_REFER, 0, 0, 0);
> +	if (ret)
> +		dev_err(&spi->dev,
> +			"Set internal reference command failed, %d\n", ret);
If it's an error than drop out rather than carrying on regardless.
(i.e. return ret)
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret)
> +		return ret;
return devm_iio_device_register(indio_dev);

The static checkers would have thrown a wobbly about this anyway so best to
fix it now.
> +
> +	return 0;
> +}
> +
> +static int ltc2632_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +
> +	iio_device_unregister(indio_dev);
If all you have in an remove is a single call to iio_device_unregister its
a good sign that you could have used devm_ calls for everything including
devm_iio_device_register and dropped the remove entirely.

We can reintroduce it later if changes to the driver require it.
> +
> +	return 0;
> +}
> +
> +static const struct spi_device_id ltc2632_id[] = {
> +	{ "ltc2632-l12", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632L12] },
> +	{ "ltc2632-l10", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632L10] },
> +	{ "ltc2632-l8", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632L8] },
> +	{ "ltc2632-h12", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632H12] },
> +	{ "ltc2632-h10", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632H10] },
> +	{ "ltc2632-h8", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632H8] },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(spi, ltc2632_id);
> +
> +static struct spi_driver ltc2632_driver = {
> +	.driver		= {
> +		.name	= "ltc2632",
> +	},
> +	.probe		= ltc2632_probe,
> +	.remove		= ltc2632_remove,
> +	.id_table	= ltc2632_id,
> +};
> +module_spi_driver(ltc2632_driver);
> +
> +static const struct of_device_id ltc2632_of_match[] = {
> +	{ .compatible = "lltc,ltc2632-l12", .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632L12] },
> +	{ .compatible = "lltc,ltc2632-l10", .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632L10] },
> +	{ .compatible = "lltc,ltc2632-l8",  .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632L8] },
> +	{ .compatible = "lltc,ltc2632-h12", .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632H12] },
> +	{ .compatible = "lltc,ltc2632-h10", .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632H10] },
> +	{ .compatible = "lltc,ltc2632-h8",  .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632H8] },
> +	{ }
No need for the void * casts.. Any pointer can be assigned to a void * without
cast.
> +};
> +MODULE_DEVICE_TABLE(of, ltc2632_of_match);
> +
> +MODULE_AUTHOR("Maxime Roussin-Belanger <maxime.roussinbelanger@gmail.com>");
> +MODULE_DESCRIPTION("LTC2632 DAC SPI driver");
> +MODULE_LICENSE("GPL v2");
> 

Re: [PATCH] iio: dac: Add support for ltc2632 DACs

[Peter Meerwald-Stadler]

> Signed-off-by: Maxime Roussin-Belanger <maxime.roussinbelanger@gmail.com>

comments below

> ---
>  .../devicetree/bindings/iio/dac/ltc2632.txt        |  23 ++
>  drivers/iio/dac/Kconfig                            |   7 +
>  drivers/iio/dac/Makefile                           |   1 +
>  drivers/iio/dac/ltc2632.c                          | 325 +++++++++++++++++++++
>  4 files changed, 356 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/dac/ltc2632.txt
>  create mode 100644 drivers/iio/dac/ltc2632.c
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/ltc2632.txt b/Documentation/devicetree/bindings/iio/dac/ltc2632.txt
> new file mode 100644
> index 0000000..fd9aa0b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/ltc2632.txt
> @@ -0,0 +1,23 @@
> +Linear Technology LTC2632 DAC device driver
> +
> +Required properties:
> + - compatible: Has to contain one of the following
> +	lltc,ltc2632-l12
> +	lltc,ltc2632-l10
> +	lltc,ltc2632-l8
> +	lltc,ltc2632-h12
> +	lltc,ltc2632-h10
> +	lltc,ltc2632-h8
> +
> +Property rules described in Documentation/devicetree/bindings/spi/spi-bus.txt
> +apply. In particular, "reg" and "spi-max-frequency" properties must be given.
> +
> +Example:
> +
> +	spi_master {
> +		dac: ltc2632@0 {
> +			compatible = "lltc,ltc2632";

ltc2632 should have a suffix as per the list above?

> +			reg = <0>; /* CS0 */
> +			spi-max-frequency = <1000000>;
> +		};
> +	};
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index d3084028..f32ca8a 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -118,6 +118,13 @@ config AD5624R_SPI
>  	  Say yes here to build support for Analog Devices AD5624R, AD5644R and
>  	  AD5664R converters (DAC). This driver uses the common SPI interface.
>  
> +config LTC2632
> +	tristate "Linear Technology LTC2632-12/10/8 DAC spi driver"
> +	depends on SPI
> +	help
> +	  Say yes here to build support for Linear Technology LTC2632-12/10/8 converters (DAC).
> +	  This driver uses the common SPI interface.
> +
>  config AD5686
>  	tristate "Analog Devices AD5686R/AD5685R/AD5684R DAC SPI driver"
>  	depends on SPI
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index f01bf4a..6e4d995 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_AD8801) += ad8801.o
>  obj-$(CONFIG_CIO_DAC) += cio-dac.o
>  obj-$(CONFIG_DPOT_DAC) += dpot-dac.o
>  obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o
> +obj-$(CONFIG_LTC2632) += ltc2632.o
>  obj-$(CONFIG_M62332) += m62332.o
>  obj-$(CONFIG_MAX517) += max517.o
>  obj-$(CONFIG_MAX5821) += max5821.o
> diff --git a/drivers/iio/dac/ltc2632.c b/drivers/iio/dac/ltc2632.c
> new file mode 100644
> index 0000000..d6160fb
> --- /dev/null
> +++ b/drivers/iio/dac/ltc2632.c
> @@ -0,0 +1,325 @@
> +/*
> + * LTC2632 Digital to analog convertors spi driver
> + */
> +
> +#include <linux/device.h>
> +#include <linux/spi/spi.h>
> +#include <linux/module.h>
> +
> +#include <linux/iio/iio.h>
> +
> +#define LTC2632_DAC_CHANNELS                    2
> +
> +#define LTC2632_ADDR_DAC0                       0x0
> +#define LTC2632_ADDR_DAC1                       0x1
> +
> +#define LTC2632_CMD_WRITE_INPUT_N               0x0
> +#define LTC2632_CMD_UPDATE_DAC_N                0x1
> +#define LTC2632_CMD_WRITE_INPUT_N_UPDATE_ALL    0x2
> +#define LTC2632_CMD_WRITE_INPUT_N_UPDATE_N      0x3
> +#define LTC2632_CMD_POWERDOWN_DAC_N             0x4
> +#define LTC2632_CMD_POWERDOWN_CHIP              0x5
> +#define LTC2632_CMD_INTERNAL_REFER              0x6
> +#define LTC2632_CMD_EXTERNAL_REFER              0x7
> +

drop newline

> +
> +static ssize_t ltc2632_read_dac_powerdown(struct iio_dev *indio_dev,
> +					  uintptr_t private,
> +					  const struct iio_chan_spec *chan,
> +					  char *buf);
> +
> +static ssize_t ltc2632_write_dac_powerdown(struct iio_dev *indio_dev,
> +					   uintptr_t private,
> +					   const struct iio_chan_spec *chan,
> +					   const char *buf,
> +					   size_t len);
> +
> +/**
> + * struct ltc2632_chip_info - chip specific information
> + * @channels:		channel spec for the DAC

vref_mv missing

> + */
> +struct ltc2632_chip_info {
> +	const struct iio_chan_spec *channels;
> +	const int vref_mv;
> +};
> +
> +/**
> + * struct ltc2632_state - driver instance specific data
> + * @spi_dev:		spi_device
> + * @pwr_down_cached	power down mask
> + */
> +struct ltc2632_state {
> +	struct spi_device *spi_dev;
> +	unsigned int pwr_down_cached;
> +};
> +
> +/**
> + * ltc2632_supported_device_ids
> + */
> +enum ltc2632_supported_device_ids {
> +	ID_LTC2632L12,
> +	ID_LTC2632L10,
> +	ID_LTC2632L8,
> +	ID_LTC2632H12,
> +	ID_LTC2632H10,
> +	ID_LTC2632H8,
> +};
> +
> +static const struct iio_chan_spec_ext_info ltc2632_ext_info[] = {
> +	{
> +		.name = "powerdown",
> +		.read = ltc2632_read_dac_powerdown,
> +		.write = ltc2632_write_dac_powerdown,
> +		.shared = IIO_SEPARATE,
> +	},
> +	{ },
> +};
> +
> +#define LTC2632_CHANNEL(_chan, _bits) { \
> +		.type = IIO_VOLTAGE, \
> +		.indexed = 1, \
> +		.output = 1, \
> +		.channel = (_chan), \
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> +		.address = (_chan), \
> +		.scan_type = { \
> +			.sign		= 'u', \
> +			.realbits	= (_bits), \
> +			.storagebits	= 16, \
> +			.shift		= 16 - (_bits), \
> +		}, \
> +		.ext_info = ltc2632_ext_info, \
> +}
> +
> +#define DECLARE_LTC2632_CHANNELS(_name, _bits) \
> +	const struct iio_chan_spec _name ## _channels[] = { \
> +		LTC2632_CHANNEL(0, _bits), \
> +		LTC2632_CHANNEL(1, _bits), \
> +	}
> +
> +static DECLARE_LTC2632_CHANNELS(ltc2632l12, 12);
> +static DECLARE_LTC2632_CHANNELS(ltc2632l10, 10);
> +static DECLARE_LTC2632_CHANNELS(ltc2632l8, 8);
> +
> +static DECLARE_LTC2632_CHANNELS(ltc2632h12, 12);
> +static DECLARE_LTC2632_CHANNELS(ltc2632h10, 10);
> +static DECLARE_LTC2632_CHANNELS(ltc2632h8, 8);
> +
> +static const struct ltc2632_chip_info ltc2632_chip_info_tbl[] = {
> +	[ID_LTC2632L12] = {
> +		.channels	= ltc2632l12_channels,
> +		.vref_mv	= 2500,
> +	},
> +	[ID_LTC2632L10] = {
> +		.channels	= ltc2632l10_channels,
> +		.vref_mv	= 2500,
> +	},
> +	[ID_LTC2632L8] =  {
> +		.channels	= ltc2632l8_channels,
> +		.vref_mv	= 2500,
> +	},
> +	[ID_LTC2632H12] = {
> +		.channels	= ltc2632h12_channels,
> +		.vref_mv	= 4096,
> +	},
> +	[ID_LTC2632H10] = {
> +		.channels	= ltc2632h10_channels,
> +		.vref_mv	= 4096,
> +	},
> +	[ID_LTC2632H8] =  {
> +		.channels	= ltc2632h8_channels,
> +		.vref_mv	= 4096,
> +	},
> +};
> +
> +static int ltc2632_spi_write(struct spi_device *spi,
> +			     u8 cmd, u8 addr, u16 val, u8 shift)
> +{
> +	u32 data;
> +	u8 msg[3];
> +
> +	/*
> +	 * The input shift register is 24 bits wide.
> +	 * The next four are the command bits, C3 to C0,
> +	 * followed by the 4-bit DAC address, A3 to A0, and then the
> +	 * 12-, 10-, 8-bit data-word. The data-word comprises the 12-,
> +	 * 10-, 8-bit input code followed by 4, 6, or 8 don't care bits.
> +	 */
> +	data = (cmd << 20) | (addr << 16) | (val << shift);
> +	msg[0] = data >> 16;
> +	msg[1] = data >> 8;
> +	msg[2] = data;
> +
> +	return spi_write(spi, msg, 3);

sizeof(msg)

> +}
> +
> +static int ltc2632_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val,
> +			    int *val2,
> +			    long m)
> +{
> +	struct ltc2632_chip_info *chip_info;
> +
> +	const struct ltc2632_state *st = iio_priv(indio_dev);
> +	const struct spi_device_id* spi_dev_id = spi_get_device_id(st->spi_dev);
> +
> +	chip_info = (struct ltc2632_chip_info*) spi_dev_id->driver_data;
> +
> +	switch (m) {
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = chip_info->vref_mv;
> +		*val2 = chan->scan_type.realbits;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	}
> +	return -EINVAL;
> +}
> +
> +static int ltc2632_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int val,
> +			     int val2,
> +			     long mask)
> +{
> +	struct ltc2632_state *st = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (val >= (1 << chan->scan_type.realbits) || val < 0)
> +			return -EINVAL;
> +
> +		return ltc2632_spi_write(st->spi_dev,
> +					 LTC2632_CMD_WRITE_INPUT_N_UPDATE_N,
> +					 chan->address, val,
> +					 chan->scan_type.shift);
> +	default:
> +		return -EINVAL;
> +	}
> +
> +}
> +
> +static ssize_t ltc2632_read_dac_powerdown(struct iio_dev *indio_dev,
> +					  uintptr_t private,
> +					  const struct iio_chan_spec *chan,
> +					  char *buf)
> +{
> +	struct ltc2632_state *st = iio_priv(indio_dev);
> +
> +	return sprintf(buf, "%d\n",
> +		       !!(st->pwr_down_cached & (1 << chan->channel)));
> +}
> +
> +static ssize_t ltc2632_write_dac_powerdown(struct iio_dev *indio_dev,
> +					   uintptr_t private,
> +					   const struct iio_chan_spec *chan,
> +					   const char *buf,
> +					   size_t len)
> +{
> +	bool pwr_down;
> +	int ret;
> +	struct ltc2632_state *st = iio_priv(indio_dev);
> +
> +	ret = strtobool(buf, &pwr_down);
> +	if (ret)
> +		return ret;
> +
> +	if (pwr_down)
> +		st->pwr_down_cached |= (1 << chan->channel);
> +	else
> +		st->pwr_down_cached &= ~(1 << chan->channel);
> +
> +	ret = ltc2632_spi_write(st->spi_dev,
> +				LTC2632_CMD_POWERDOWN_DAC_N,
> +				chan->channel, 0, 0);
> +
> +	return ret ? ret : len;
> +}
> +
> +static const struct iio_info ltc2632_info = {
> +	.write_raw	= ltc2632_write_raw,
> +	.read_raw	= ltc2632_read_raw,
> +	.driver_module	= THIS_MODULE,
> +};
> +
> +static int ltc2632_probe(struct spi_device *spi)
> +{
> +	struct ltc2632_state *st;
> +	struct iio_dev *indio_dev;
> +	struct ltc2632_chip_info* chip_info;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +	st = iio_priv(indio_dev);
> +
> +	spi_set_drvdata(spi, indio_dev);
> +	st->spi_dev = spi;
> +
> +	chip_info = (struct ltc2632_chip_info*)
> +			spi_get_device_id(spi)->driver_data;
> +
> +	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->info = &ltc2632_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = chip_info->channels;
> +	indio_dev->num_channels = LTC2632_DAC_CHANNELS;
> +
> +	ret = ltc2632_spi_write(spi, LTC2632_CMD_INTERNAL_REFER, 0, 0, 0);
> +	if (ret)
> +		dev_err(&spi->dev,
> +			"Set internal reference command failed, %d\n", ret);
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int ltc2632_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +
> +	iio_device_unregister(indio_dev);
> +
> +	return 0;
> +}
> +
> +static const struct spi_device_id ltc2632_id[] = {
> +	{ "ltc2632-l12", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632L12] },
> +	{ "ltc2632-l10", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632L10] },
> +	{ "ltc2632-l8", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632L8] },
> +	{ "ltc2632-h12", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632H12] },
> +	{ "ltc2632-h10", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632H10] },
> +	{ "ltc2632-h8", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632H8] },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(spi, ltc2632_id);
> +
> +static struct spi_driver ltc2632_driver = {
> +	.driver		= {
> +		.name	= "ltc2632",
> +	},
> +	.probe		= ltc2632_probe,
> +	.remove		= ltc2632_remove,
> +	.id_table	= ltc2632_id,
> +};
> +module_spi_driver(ltc2632_driver);
> +
> +static const struct of_device_id ltc2632_of_match[] = {
> +	{ .compatible = "lltc,ltc2632-l12", .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632L12] },
> +	{ .compatible = "lltc,ltc2632-l10", .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632L10] },
> +	{ .compatible = "lltc,ltc2632-l8",  .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632L8] },
> +	{ .compatible = "lltc,ltc2632-h12", .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632H12] },
> +	{ .compatible = "lltc,ltc2632-h10", .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632H10] },
> +	{ .compatible = "lltc,ltc2632-h8",  .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632H8] },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ltc2632_of_match);
> +
> +MODULE_AUTHOR("Maxime Roussin-Belanger <maxime.roussinbelanger@gmail.com>");
> +MODULE_DESCRIPTION("LTC2632 DAC SPI driver");
> +MODULE_LICENSE("GPL v2");
> 

-- 

Peter Meerwald-Stadler
Mobile: +43 664 24 44 418

Re: [PATCH] iio: dac: Add support for ltc2632 DACs

[Lars-Peter Clausen]

On 03/02/2017 08:01 PM, Maxime Roussin-Belanger wrote:
> Signed-off-by: Maxime Roussin-Belanger <maxime.roussinbelanger@gmail.com>


> ---
>  .../devicetree/bindings/iio/dac/ltc2632.txt        |  23 ++
>  drivers/iio/dac/Kconfig                            |   7 +
>  drivers/iio/dac/Makefile                           |   1 +
>  drivers/iio/dac/ltc2632.c                          | 325 +++++++++++++++++++++
>  4 files changed, 356 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/dac/ltc2632.txt
>  create mode 100644 drivers/iio/dac/ltc2632.c
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/ltc2632.txt b/Documentation/devicetree/bindings/iio/dac/ltc2632.txt
> new file mode 100644
> index 0000000..fd9aa0b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/ltc2632.txt
> @@ -0,0 +1,23 @@
> +Linear Technology LTC2632 DAC device driver
> +
> +Required properties:
> + - compatible: Has to contain one of the following
> +	lltc,ltc2632-l12
> +	lltc,ltc2632-l10
> +	lltc,ltc2632-l8
> +	lltc,ltc2632-h12
> +	lltc,ltc2632-h10
> +	lltc,ltc2632-h8
> +
> +Property rules described in Documentation/devicetree/bindings/spi/spi-bus.txt
> +apply. In particular, "reg" and "spi-max-frequency" properties must be given.
> +
> +Example:
> +
> +	spi_master {
> +		dac: ltc2632@0 {
> +			compatible = "lltc,ltc2632";
> +			reg = <0>; /* CS0 */
> +			spi-max-frequency = <1000000>;
> +		};
> +	};
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index d3084028..f32ca8a 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -118,6 +118,13 @@ config AD5624R_SPI
>  	  Say yes here to build support for Analog Devices AD5624R, AD5644R and
>  	  AD5664R converters (DAC). This driver uses the common SPI interface.
>  
> +config LTC2632
> +	tristate "Linear Technology LTC2632-12/10/8 DAC spi driver"
> +	depends on SPI
> +	help
> +	  Say yes here to build support for Linear Technology LTC2632-12/10/8 converters (DAC).
> +	  This driver uses the common SPI interface.
> +
>  config AD5686
>  	tristate "Analog Devices AD5686R/AD5685R/AD5684R DAC SPI driver"
>  	depends on SPI
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index f01bf4a..6e4d995 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_AD8801) += ad8801.o
>  obj-$(CONFIG_CIO_DAC) += cio-dac.o
>  obj-$(CONFIG_DPOT_DAC) += dpot-dac.o
>  obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o
> +obj-$(CONFIG_LTC2632) += ltc2632.o
>  obj-$(CONFIG_M62332) += m62332.o
>  obj-$(CONFIG_MAX517) += max517.o
>  obj-$(CONFIG_MAX5821) += max5821.o
> diff --git a/drivers/iio/dac/ltc2632.c b/drivers/iio/dac/ltc2632.c
> new file mode 100644
> index 0000000..d6160fb
> --- /dev/null
> +++ b/drivers/iio/dac/ltc2632.c
> @@ -0,0 +1,325 @@
> +/*
> + * LTC2632 Digital to analog convertors spi driver
> + */
> +
> +#include <linux/device.h>
> +#include <linux/spi/spi.h>
> +#include <linux/module.h>
> +
> +#include <linux/iio/iio.h>
> +
> +#define LTC2632_DAC_CHANNELS                    2
> +
> +#define LTC2632_ADDR_DAC0                       0x0
> +#define LTC2632_ADDR_DAC1                       0x1
> +
> +#define LTC2632_CMD_WRITE_INPUT_N               0x0
> +#define LTC2632_CMD_UPDATE_DAC_N                0x1
> +#define LTC2632_CMD_WRITE_INPUT_N_UPDATE_ALL    0x2
> +#define LTC2632_CMD_WRITE_INPUT_N_UPDATE_N      0x3
> +#define LTC2632_CMD_POWERDOWN_DAC_N             0x4
> +#define LTC2632_CMD_POWERDOWN_CHIP              0x5
> +#define LTC2632_CMD_INTERNAL_REFER              0x6
> +#define LTC2632_CMD_EXTERNAL_REFER              0x7

Looks like the SPI versions of the  LTC I2C DACs already supported in
ad5064.c. Since that driver has SPI support, we could support them there
instead of a separate driver. Should be a lot less code.


> +
> +
> +static ssize_t ltc2632_read_dac_powerdown(struct iio_dev *indio_dev,
> +					  uintptr_t private,
> +					  const struct iio_chan_spec *chan,
> +					  char *buf);
> +
> +static ssize_t ltc2632_write_dac_powerdown(struct iio_dev *indio_dev,
> +					   uintptr_t private,
> +					   const struct iio_chan_spec *chan,
> +					   const char *buf,
> +					   size_t len);
> +
> +/**
> + * struct ltc2632_chip_info - chip specific information
> + * @channels:		channel spec for the DAC
> + */
> +struct ltc2632_chip_info {
> +	const struct iio_chan_spec *channels;
> +	const int vref_mv;
> +};
> +
> +/**
> + * struct ltc2632_state - driver instance specific data
> + * @spi_dev:		spi_device
> + * @pwr_down_cached	power down mask
> + */
> +struct ltc2632_state {
> +	struct spi_device *spi_dev;
> +	unsigned int pwr_down_cached;
> +};
> +
> +/**
> + * ltc2632_supported_device_ids
> + */
> +enum ltc2632_supported_device_ids {
> +	ID_LTC2632L12,
> +	ID_LTC2632L10,
> +	ID_LTC2632L8,
> +	ID_LTC2632H12,
> +	ID_LTC2632H10,
> +	ID_LTC2632H8,
> +};
> +
> +static const struct iio_chan_spec_ext_info ltc2632_ext_info[] = {
> +	{
> +		.name = "powerdown",
> +		.read = ltc2632_read_dac_powerdown,
> +		.write = ltc2632_write_dac_powerdown,
> +		.shared = IIO_SEPARATE,
> +	},
> +	{ },
> +};
> +
> +#define LTC2632_CHANNEL(_chan, _bits) { \
> +		.type = IIO_VOLTAGE, \
> +		.indexed = 1, \
> +		.output = 1, \
> +		.channel = (_chan), \
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> +		.address = (_chan), \
> +		.scan_type = { \
> +			.sign		= 'u', \
> +			.realbits	= (_bits), \
> +			.storagebits	= 16, \
> +			.shift		= 16 - (_bits), \
> +		}, \
> +		.ext_info = ltc2632_ext_info, \
> +}
> +
> +#define DECLARE_LTC2632_CHANNELS(_name, _bits) \
> +	const struct iio_chan_spec _name ## _channels[] = { \
> +		LTC2632_CHANNEL(0, _bits), \
> +		LTC2632_CHANNEL(1, _bits), \
> +	}
> +
> +static DECLARE_LTC2632_CHANNELS(ltc2632l12, 12);
> +static DECLARE_LTC2632_CHANNELS(ltc2632l10, 10);
> +static DECLARE_LTC2632_CHANNELS(ltc2632l8, 8);
> +
> +static DECLARE_LTC2632_CHANNELS(ltc2632h12, 12);
> +static DECLARE_LTC2632_CHANNELS(ltc2632h10, 10);
> +static DECLARE_LTC2632_CHANNELS(ltc2632h8, 8);
> +
> +static const struct ltc2632_chip_info ltc2632_chip_info_tbl[] = {
> +	[ID_LTC2632L12] = {
> +		.channels	= ltc2632l12_channels,
> +		.vref_mv	= 2500,
> +	},
> +	[ID_LTC2632L10] = {
> +		.channels	= ltc2632l10_channels,
> +		.vref_mv	= 2500,
> +	},
> +	[ID_LTC2632L8] =  {
> +		.channels	= ltc2632l8_channels,
> +		.vref_mv	= 2500,
> +	},
> +	[ID_LTC2632H12] = {
> +		.channels	= ltc2632h12_channels,
> +		.vref_mv	= 4096,
> +	},
> +	[ID_LTC2632H10] = {
> +		.channels	= ltc2632h10_channels,
> +		.vref_mv	= 4096,
> +	},
> +	[ID_LTC2632H8] =  {
> +		.channels	= ltc2632h8_channels,
> +		.vref_mv	= 4096,
> +	},
> +};
> +
> +static int ltc2632_spi_write(struct spi_device *spi,
> +			     u8 cmd, u8 addr, u16 val, u8 shift)
> +{
> +	u32 data;
> +	u8 msg[3];
> +
> +	/*
> +	 * The input shift register is 24 bits wide.
> +	 * The next four are the command bits, C3 to C0,
> +	 * followed by the 4-bit DAC address, A3 to A0, and then the
> +	 * 12-, 10-, 8-bit data-word. The data-word comprises the 12-,
> +	 * 10-, 8-bit input code followed by 4, 6, or 8 don't care bits.
> +	 */
> +	data = (cmd << 20) | (addr << 16) | (val << shift);
> +	msg[0] = data >> 16;
> +	msg[1] = data >> 8;
> +	msg[2] = data;
> +
> +	return spi_write(spi, msg, 3);
> +}
> +
> +static int ltc2632_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val,
> +			    int *val2,
> +			    long m)
> +{
> +	struct ltc2632_chip_info *chip_info;
> +
> +	const struct ltc2632_state *st = iio_priv(indio_dev);
> +	const struct spi_device_id* spi_dev_id = spi_get_device_id(st->spi_dev);
> +
> +	chip_info = (struct ltc2632_chip_info*) spi_dev_id->driver_data;
> +
> +	switch (m) {
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = chip_info->vref_mv;
> +		*val2 = chan->scan_type.realbits;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	}
> +	return -EINVAL;
> +}
> +
> +static int ltc2632_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int val,
> +			     int val2,
> +			     long mask)
> +{
> +	struct ltc2632_state *st = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (val >= (1 << chan->scan_type.realbits) || val < 0)
> +			return -EINVAL;
> +
> +		return ltc2632_spi_write(st->spi_dev,
> +					 LTC2632_CMD_WRITE_INPUT_N_UPDATE_N,
> +					 chan->address, val,
> +					 chan->scan_type.shift);
> +	default:
> +		return -EINVAL;
> +	}
> +
> +}
> +
> +static ssize_t ltc2632_read_dac_powerdown(struct iio_dev *indio_dev,
> +					  uintptr_t private,
> +					  const struct iio_chan_spec *chan,
> +					  char *buf)
> +{
> +	struct ltc2632_state *st = iio_priv(indio_dev);
> +
> +	return sprintf(buf, "%d\n",
> +		       !!(st->pwr_down_cached & (1 << chan->channel)));
> +}
> +
> +static ssize_t ltc2632_write_dac_powerdown(struct iio_dev *indio_dev,
> +					   uintptr_t private,
> +					   const struct iio_chan_spec *chan,
> +					   const char *buf,
> +					   size_t len)
> +{
> +	bool pwr_down;
> +	int ret;
> +	struct ltc2632_state *st = iio_priv(indio_dev);
> +
> +	ret = strtobool(buf, &pwr_down);
> +	if (ret)
> +		return ret;
> +
> +	if (pwr_down)
> +		st->pwr_down_cached |= (1 << chan->channel);
> +	else
> +		st->pwr_down_cached &= ~(1 << chan->channel);
> +
> +	ret = ltc2632_spi_write(st->spi_dev,
> +				LTC2632_CMD_POWERDOWN_DAC_N,
> +				chan->channel, 0, 0);
> +
> +	return ret ? ret : len;
> +}
> +
> +static const struct iio_info ltc2632_info = {
> +	.write_raw	= ltc2632_write_raw,
> +	.read_raw	= ltc2632_read_raw,
> +	.driver_module	= THIS_MODULE,
> +};
> +
> +static int ltc2632_probe(struct spi_device *spi)
> +{
> +	struct ltc2632_state *st;
> +	struct iio_dev *indio_dev;
> +	struct ltc2632_chip_info* chip_info;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +	st = iio_priv(indio_dev);
> +
> +	spi_set_drvdata(spi, indio_dev);
> +	st->spi_dev = spi;
> +
> +	chip_info = (struct ltc2632_chip_info*)
> +			spi_get_device_id(spi)->driver_data;
> +
> +	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->info = &ltc2632_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = chip_info->channels;
> +	indio_dev->num_channels = LTC2632_DAC_CHANNELS;
> +
> +	ret = ltc2632_spi_write(spi, LTC2632_CMD_INTERNAL_REFER, 0, 0, 0);
> +	if (ret)
> +		dev_err(&spi->dev,
> +			"Set internal reference command failed, %d\n", ret);
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int ltc2632_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +
> +	iio_device_unregister(indio_dev);
> +
> +	return 0;
> +}
> +
> +static const struct spi_device_id ltc2632_id[] = {
> +	{ "ltc2632-l12", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632L12] },
> +	{ "ltc2632-l10", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632L10] },
> +	{ "ltc2632-l8", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632L8] },
> +	{ "ltc2632-h12", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632H12] },
> +	{ "ltc2632-h10", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632H10] },
> +	{ "ltc2632-h8", (kernel_ulong_t)&ltc2632_chip_info_tbl[ID_LTC2632H8] },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(spi, ltc2632_id);
> +
> +static struct spi_driver ltc2632_driver = {
> +	.driver		= {
> +		.name	= "ltc2632",
> +	},
> +	.probe		= ltc2632_probe,
> +	.remove		= ltc2632_remove,
> +	.id_table	= ltc2632_id,
> +};
> +module_spi_driver(ltc2632_driver);
> +
> +static const struct of_device_id ltc2632_of_match[] = {
> +	{ .compatible = "lltc,ltc2632-l12", .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632L12] },
> +	{ .compatible = "lltc,ltc2632-l10", .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632L10] },
> +	{ .compatible = "lltc,ltc2632-l8",  .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632L8] },
> +	{ .compatible = "lltc,ltc2632-h12", .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632H12] },
> +	{ .compatible = "lltc,ltc2632-h10", .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632H10] },
> +	{ .compatible = "lltc,ltc2632-h8",  .data = (void*)&ltc2632_chip_info_tbl[ID_LTC2632H8] },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ltc2632_of_match);
> +
> +MODULE_AUTHOR("Maxime Roussin-Belanger <maxime.roussinbelanger@gmail.com>");
> +MODULE_DESCRIPTION("LTC2632 DAC SPI driver");
> +MODULE_LICENSE("GPL v2");
> 

Re: [PATCH] iio: dac: Add support for ltc2632 DACs

[Maxime Roussin-Belanger]

On Mon, Mar 06, 2017 at 11:24:58AM +0100, Lars-Peter Clausen wrote:
> On 03/02/2017 08:01 PM, Maxime Roussin-Belanger wrote:
> > +
> > +#define LTC2632_CMD_WRITE_INPUT_N               0x0
> > +#define LTC2632_CMD_UPDATE_DAC_N                0x1
> > +#define LTC2632_CMD_WRITE_INPUT_N_UPDATE_ALL    0x2
> > +#define LTC2632_CMD_WRITE_INPUT_N_UPDATE_N      0x3
> > +#define LTC2632_CMD_POWERDOWN_DAC_N             0x4
> > +#define LTC2632_CMD_POWERDOWN_CHIP              0x5
> > +#define LTC2632_CMD_INTERNAL_REFER              0x6
> > +#define LTC2632_CMD_EXTERNAL_REFER              0x7
> 
> Looks like the SPI versions of the  LTC I2C DACs already supported in
> ad5064.c. Since that driver has SPI support, we could support them there
> instead of a separate driver. Should be a lot less code.
> 

We considered it, but the ad5064 is complicated for the simple ltc2632. It
would complicated ad5064, I don't think it is worth it.

For example, we would need another spi_write, because the CMD shift and the
ADDR shift are not the same. The current support for LTC2632 doesn't support
midscale. It only has 2 channels.

We think this new ltc2632 driver could become the base for the rest of the
ltc2632 series as well as other similarly simple DACs.

Re: [PATCH] iio: dac: Add support for ltc2632 DACs

[Jean-Francois Dagenais]

Hi Jonathan,

Thanks for the great feedback. I am working with Maxime on this driver.

> On Mar 4, 2017, at 14:03, Jonathan Cameron <jic23@kernel.org> wrote:
>=20
>> +static const struct iio_chan_spec_ext_info ltc2632_ext_info[] =3D {
>> +	{
>> +		.name =3D "powerdown",
> Why have this as a sysfs attribute? It's new ABI so should be =
documented
> and justified.
>=20
> Normally having explicit power up and powerdown of devices is somewhat
> frowned upon as it can often be implicitly done depending on the state
> of the machine etc (runtime pm and similar). If that's not the case =
here
> please provide some explanatory comments.

I understand what you mean. We just copy/pasted from a neighbouring =
driver but
plan on using the feature. I will give you our use case as an example. =
We use
this DAC to drive amplifiers in an acquisition subsystem. This subsystem =
may be
offline for long durations (when no acquisition is taking place). The =
system
still needs to be fully awake as the user is simply doing something else =
with
the unit at that moment.

Trying to bear with you I would ask: are there ways to define different =
alert
states with more granularity than "sleeping" and "awake", which would =
allow
us to tie the powerdown state of the chips? If so, quick pointers would =
be
appreciated.

>>=20
>> +#define LTC2632_CHANNEL(_chan, _bits) { \
>> +		.type =3D IIO_VOLTAGE, \
>> +		.indexed =3D 1, \
>> +		.output =3D 1, \
>> +		.channel =3D (_chan), \
>> +		.info_mask_separate =3D BIT(IIO_CHAN_INFO_RAW), \
>> +		.info_mask_shared_by_type =3D BIT(IIO_CHAN_INFO_SCALE), =
\
>> +		.address =3D (_chan), \
>> +		.scan_type =3D { \
>> +			.sign		=3D 'u', \
>> +			.realbits	=3D (_bits), \
>> +			.storagebits	=3D 16, \
>> +			.shift		=3D 16 - (_bits), \
> Hmm. Without buffers for output devices some of this is never used, =
but
> it does provide 'documentation' of a type so fine to leave it here.

Guilty of plagiarism, we do not master the iio subsystem yet. We copied
5624r_spi.c and tweaked it for this chip. We did not intend to document =
by doing
this. Can you suggest a different version of this struct?

Thanks again!=

Re: [PATCH] iio: dac: Add support for ltc2632 DACs

[Jonathan Cameron]

On 07/03/17 15:59, Jean-Francois Dagenais wrote:
> Hi Jonathan,
> 
> Thanks for the great feedback. I am working with Maxime on this driver.
> 
>> On Mar 4, 2017, at 14:03, Jonathan Cameron <jic23@kernel.org> wrote:
>>
>>> +static const struct iio_chan_spec_ext_info ltc2632_ext_info[] = {
>>> +	{
>>> +		.name = "powerdown",
>> Why have this as a sysfs attribute? It's new ABI so should be documented
>> and justified.
>>
>> Normally having explicit power up and powerdown of devices is somewhat
>> frowned upon as it can often be implicitly done depending on the state
>> of the machine etc (runtime pm and similar). If that's not the case here
>> please provide some explanatory comments.
> 
> I understand what you mean. We just copy/pasted from a neighbouring driver but
> plan on using the feature. I will give you our use case as an example. We use
> this DAC to drive amplifiers in an acquisition subsystem. This subsystem may be
> offline for long durations (when no acquisition is taking place). The system
> still needs to be fully awake as the user is simply doing something else with
> the unit at that moment.
An interesting use case.  If we were to do this cleanly, the DAC would be explicitly
linked to a driver for the amplifiers (or the wider acquisition system).
Then we could look at doing runtime PM on the wider system and having that feed
back to the DAC driver.  Right now we don't have any callbacks for this, but it
doesn't seem like it would be that difficult to add.

This would require you to have an explicit representation of the whole of the
acquisition system and its dependencies however, which might be non trivial
to put together.
> 
> Trying to bear with you I would ask: are there ways to define different alert
> states with more granularity than "sleeping" and "awake", which would allow
> us to tie the powerdown state of the chips? If so, quick pointers would be
> appreciated.
Not really.

For the DAC case it's rather different to the more usual runtime pm type usage
in input devices.  There we know that if we don't 'look' then we don't know
what the state is (and hence can be powered down).  With DACs it is only whatever
they are wired too that can know whether they need to be powered up.  Afterall
they'll stay in whatever state we put them in indefinitely.

So I am coming around to explicit powerdown controls making sense for a DAC.
> 
>>>
>>> +#define LTC2632_CHANNEL(_chan, _bits) { \
>>> +		.type = IIO_VOLTAGE, \
>>> +		.indexed = 1, \
>>> +		.output = 1, \
>>> +		.channel = (_chan), \
>>> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
>>> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
>>> +		.address = (_chan), \
>>> +		.scan_type = { \
>>> +			.sign		= 'u', \
>>> +			.realbits	= (_bits), \
>>> +			.storagebits	= 16, \
>>> +			.shift		= 16 - (_bits), \
>> Hmm. Without buffers for output devices some of this is never used, but
>> it does provide 'documentation' of a type so fine to leave it here.
> 
> Guilty of plagiarism, we do not master the iio subsystem yet. We copied
> 5624r_spi.c and tweaked it for this chip. We did not intend to document by doing
> this. Can you suggest a different version of this struct?
The core isn't going to use it at all in a DAC currently.
So just don't bother filling in the parts you aren't using within the driver.
> 
> Thanks again!--
> 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
>