[PATCH] iio: dac: mcp4728: Add support for MCP4728

[PATCH] iio: dac: mcp4728: Add support for MCP4728

[Evgeny Boger]

Microchip MCP4728 is a 12-bit quad channel
digital-to-analog converter (DAC) with I2C interface.

This patch adds support for most of the features of MCP4728,
including per-channel gain, power state and power down mode control.
Driver also supports saving the current state to on-chip EEPROM.

At present, only internal voltage reference is supported.

Signed-off-by: Evgeny Boger <boger@contactless.ru>
---
 drivers/iio/dac/Kconfig         |  11 +
 drivers/iio/dac/Makefile        |   1 +
 drivers/iio/dac/mcp4728.c       | 479 ++++++++++++++++++++++++++++++++++++++++
 include/linux/iio/dac/mcp4728.h |  16 ++
 4 files changed, 507 insertions(+)
 create mode 100644 drivers/iio/dac/mcp4728.c
 create mode 100644 include/linux/iio/dac/mcp4728.h

diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index e701e28..8da6039 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -186,6 +186,17 @@ config MCP4725
 	  To compile this driver as a module, choose M here: the module
 	  will be called mcp4725.
 
+config MCP4728
+	tristate "MCP4728 DAC driver"
+	depends on I2C
+	---help---
+	  Say Y here if you want to build a driver for the Microchip
+	  MCP 4728 12-bit quad digital-to-analog converter (DAC) with I2C
+	  interface.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called mcp4728.
+
 config MCP4922
 	tristate "MCP4902, MCP4912, MCP4922 DAC driver"
 	depends on SPI
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 63ae056..2c9decc 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -20,4 +20,5 @@ obj-$(CONFIG_M62332) += m62332.o
 obj-$(CONFIG_MAX517) += max517.o
 obj-$(CONFIG_MAX5821) += max5821.o
 obj-$(CONFIG_MCP4725) += mcp4725.o
+obj-$(CONFIG_MCP4728) += mcp4728.o
 obj-$(CONFIG_MCP4922) += mcp4922.o
diff --git a/drivers/iio/dac/mcp4728.c b/drivers/iio/dac/mcp4728.c
new file mode 100644
index 0000000..6dbbfac
--- /dev/null
+++ b/drivers/iio/dac/mcp4728.c
@@ -0,0 +1,479 @@
+/*
+ * mcp4728.c - Support for Microchip MCP4728
+ *
+ * Copyright (C) 2016 Evgeny Boger <boger@contactless.ru>
+ *
+ * Based on mcp4728 by Peter Meerwald <pmeerw@pmeerw.net>
+ *
+ * This file is subject to the terms and conditions of version 2 of
+ * the GNU General Public License.  See the file COPYING in the main
+ * directory of this archive for more details.
+ *
+ * driver for the Microchip MCP4728 I2C 12-bit quad digital-to-analog
+ * converter (DAC)
+ * (7-bit I2C slave address 0x60, the three LSBs can be configured in
+ * hardware)
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+#include <linux/delay.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#include <linux/iio/dac/mcp4728.h>
+
+#define MCP4728_DRV_NAME "mcp4728"
+#define MCP4728_NUM_CHANNELS 4
+#define MCP4728_INT_VREF_MV 2048
+#define MCP4728_RESOLUTION 12
+
+struct mcp4728_scale {
+	unsigned int		integer;
+	unsigned int		micro;
+};
+
+struct mcp4728_channel_state {
+	u16 dac_value;
+	bool powerdown;
+	u8 gain;
+	u8 vref;
+	unsigned powerdown_mode;
+};
+
+struct mcp4728_data {
+	struct i2c_client *client;
+	struct mcp4728_channel_state channel_state[MCP4728_NUM_CHANNELS];
+	struct mcp4728_scale	scale_avail[MCP4728_NUM_CHANNELS][2];
+
+};
+
+static int mcp4728_write_reg(struct mcp4728_data *data, int ch, bool eeprom)
+{
+	struct mcp4728_channel_state *state = &data->channel_state[ch];
+	u8 outbuf[3];
+	u8 pd;
+	int ret;
+
+	if (state->dac_value >= (1 << MCP4728_RESOLUTION) ||
+				state->dac_value < 0)
+		return -EINVAL;
+/*
+  Write input register. EEPROM is not updated:
+  c2=0, c1=1, c0=0, w1=0, w0=0 : 0x40
+
+  Write input and EEPROM registers:
+  c2=0, c1=1, c0=0, w1=1, w0=1 : 0x58
+
+  b[0]: c2 | c1 | c0 | w1 | w0 | dac1 | dac0 | /udac
+  b[1]: vref | pd1 | pd0 | gain | d11 | d10 | d9 | d8
+  b[2]: d7-d0
+*/
+	pd = state->powerdown ? (state->powerdown_mode + 1) : 0;
+
+	outbuf[0] = (eeprom ? 0x58 : 0x40) |
+				((ch & 0x03) << 1);
+	outbuf[1] = ((state->vref & 0x01) << 7) |
+				((pd & 0x3) << 5)    |
+				((state->gain & 0x01) << 4) |
+				((state->dac_value >> 8) & 0x0f);
+	outbuf[2] = state->dac_value & 0xff;
+
+	ret = i2c_master_send(data->client, outbuf, 3);
+	if (ret < 0)
+		return ret;
+	else if (ret != 3)
+		return -EIO;
+	else
+		return 0;
+}
+
+static int mcp4728_write_input_reg(struct mcp4728_data *data, int ch)
+{
+	return mcp4728_write_reg(data, ch, false);
+}
+
+static int mcp4728_suspend(struct device *dev)
+{
+	struct mcp4728_data *data = iio_priv(i2c_get_clientdata(
+		to_i2c_client(dev)));
+	int ch, ret;
+
+	for (ch = 0; ch < MCP4728_NUM_CHANNELS; ++ch) {
+		data->channel_state[ch].powerdown = true;
+		ret = mcp4728_write_input_reg(data, ch);
+		if (ret < 0)
+			return ret;
+	}
+
+	return ret;
+}
+
+static int mcp4728_resume(struct device *dev)
+{
+	struct mcp4728_data *data = iio_priv(i2c_get_clientdata(
+		to_i2c_client(dev)));
+	int ch, ret;
+
+	for (ch = 0; ch < MCP4728_NUM_CHANNELS; ++ch) {
+		data->channel_state[ch].powerdown = false;
+		ret = mcp4728_write_input_reg(data, ch);
+		if (ret < 0)
+			return ret;
+	}
+
+	return ret;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static SIMPLE_DEV_PM_OPS(mcp4728_pm_ops, mcp4728_suspend, mcp4728_resume);
+#define MCP4728_PM_OPS (&mcp4728_pm_ops)
+#else
+#define MCP4728_PM_OPS NULL
+#endif
+
+static ssize_t mcp4728_store_eeprom(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct mcp4728_data *data = iio_priv(indio_dev);
+	int tries;
+	u8 inbuf[3];
+	bool state;
+	int ret, ch;
+
+	ret = strtobool(buf, &state);
+	if (ret < 0)
+		return ret;
+
+	if (!state)
+		return 0;
+
+	for (ch = 0; ch < MCP4728_NUM_CHANNELS; ++ch) {
+		tries = 20;
+		mcp4728_write_reg(data, ch, true);
+
+
+		/* wait for write complete, takes up to 50ms */
+		while (tries--) {
+			msleep(20);
+			ret = i2c_master_recv(data->client, inbuf, 1);
+			if (ret < 0)
+				return ret;
+			else if (ret != 1)
+				return -EIO;
+
+			if (inbuf[0] & 0x80)
+				break;
+		}
+
+		if (tries < 0) {
+			dev_err(&data->client->dev,
+				"mcp4728_store_eeprom() failed, incomplete\n");
+			return -EIO;
+		}
+	}
+
+	return len;
+}
+
+static IIO_DEVICE_ATTR(store_eeprom, S_IWUSR, NULL, mcp4728_store_eeprom, 0);
+
+
+static ssize_t mcp4728_show_scale_available(struct device *dev,
+		struct device_attribute *attr,
+		char *buf)
+{
+	struct iio_dev_attr *iio_attr = to_iio_dev_attr(attr);
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct mcp4728_data *data = iio_priv(indio_dev);
+	u8 ch = iio_attr->address;
+	int i, len = 0;
+
+	for (i = 0; i < ARRAY_SIZE(data->scale_avail[ch]); i++)
+		len += sprintf(buf + len, "%u.%06u ",
+			       data->scale_avail[ch][i].integer,
+			       data->scale_avail[ch][i].micro);
+
+	len += sprintf(buf + len, "\n");
+
+	return len;
+}
+
+#define SHOW_SCALE_AVAILABLE_ATTR(ch)					\
+static IIO_DEVICE_ATTR(out_voltage##ch##_scale_available, S_IRUGO,	\
+		       mcp4728_show_scale_available, NULL, ch)
+
+SHOW_SCALE_AVAILABLE_ATTR(0);
+SHOW_SCALE_AVAILABLE_ATTR(1);
+SHOW_SCALE_AVAILABLE_ATTR(2);
+SHOW_SCALE_AVAILABLE_ATTR(3);
+
+static struct attribute *mcp4728_attributes[] = {
+	&iio_dev_attr_store_eeprom.dev_attr.attr,
+	&iio_dev_attr_out_voltage0_scale_available.dev_attr.attr,
+	&iio_dev_attr_out_voltage1_scale_available.dev_attr.attr,
+	&iio_dev_attr_out_voltage2_scale_available.dev_attr.attr,
+	&iio_dev_attr_out_voltage3_scale_available.dev_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group mcp4728_attribute_group = {
+	.attrs = mcp4728_attributes,
+};
+
+static const char * const mcp4728_powerdown_modes[] = {
+	"1kohm_to_gnd",
+	"100kohm_to_gnd",
+	"500kohm_to_gnd"
+};
+
+static int mcp4728_get_powerdown_mode(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan)
+{
+	struct mcp4728_data *data = iio_priv(indio_dev);
+
+	return data->channel_state[chan->channel].powerdown_mode;
+}
+
+static int mcp4728_set_powerdown_mode(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, unsigned mode)
+{
+	struct mcp4728_data *data = iio_priv(indio_dev);
+
+	data->channel_state[chan->channel].powerdown_mode = mode;
+
+	return 0;
+}
+
+static ssize_t mcp4728_read_powerdown(struct iio_dev *indio_dev,
+	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
+{
+	struct mcp4728_data *data = iio_priv(indio_dev);
+
+	return sprintf(buf, "%d\n",
+		data->channel_state[chan->channel].powerdown);
+}
+
+static ssize_t mcp4728_write_powerdown(struct iio_dev *indio_dev,
+	 uintptr_t private, const struct iio_chan_spec *chan,
+	 const char *buf, size_t len)
+{
+	struct mcp4728_data *data = iio_priv(indio_dev);
+	struct mcp4728_channel_state *state =
+			&data->channel_state[chan->channel];
+	bool powerdown;
+	int ret;
+
+	ret = strtobool(buf, &powerdown);
+	if (ret)
+		return ret;
+
+	state->powerdown = powerdown;
+	ret = mcp4728_write_input_reg(data, chan->channel);
+
+	if (ret < 0)
+		return ret;
+
+	return len;
+}
+
+static const struct iio_enum mcp4728_powerdown_mode_enum = {
+	.items = mcp4728_powerdown_modes,
+	.num_items = ARRAY_SIZE(mcp4728_powerdown_modes),
+	.get = mcp4728_get_powerdown_mode,
+	.set = mcp4728_set_powerdown_mode,
+};
+
+static const struct iio_chan_spec_ext_info mcp4728_ext_info[] = {
+	{
+		.name = "powerdown",
+		.read = mcp4728_read_powerdown,
+		.write = mcp4728_write_powerdown,
+		.shared = IIO_SEPARATE,
+	},
+	IIO_ENUM("powerdown_mode", IIO_SEPARATE, &mcp4728_powerdown_mode_enum),
+	IIO_ENUM_AVAILABLE("powerdown_mode", &mcp4728_powerdown_mode_enum),
+	{ },
+};
+
+#define MCP4728_CHAN(chan) {			\
+	.type		= IIO_VOLTAGE, \
+	.indexed	= 1, \
+	.output		= 1, \
+	.channel	= chan, \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+			      BIT(IIO_CHAN_INFO_SCALE), \
+	.ext_info	= mcp4728_ext_info, \
+}
+
+
+static const struct iio_chan_spec mcp4728_channels[MCP4728_NUM_CHANNELS] = {
+	MCP4728_CHAN(0),
+	MCP4728_CHAN(1),
+	MCP4728_CHAN(2),
+	MCP4728_CHAN(3),
+};
+
+static int mcp4728_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct mcp4728_data *data = iio_priv(indio_dev);
+	struct mcp4728_channel_state *state =
+			&data->channel_state[chan->channel];
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		*val = data->channel_state[chan->channel].dac_value;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = (state->gain + 1) * MCP4728_INT_VREF_MV;
+		*val2 = MCP4728_RESOLUTION;
+		return IIO_VAL_FRACTIONAL_LOG2;
+	}
+	return -EINVAL;
+}
+
+static int mcp4728_write_raw(struct iio_dev *indio_dev,
+			       struct iio_chan_spec const *chan,
+			       int val, int val2, long mask)
+{
+	struct mcp4728_data *data = iio_priv(indio_dev);
+	struct mcp4728_channel_state *state =
+			&data->channel_state[chan->channel];
+	struct mcp4728_scale *scale_avail =
+		data->scale_avail[chan->channel];
+
+	int ret, gain;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		state->dac_value = val;
+		ret = mcp4728_write_input_reg(data, chan->channel);
+		break;
+	case IIO_CHAN_INFO_SCALE:
+		ret = -EINVAL;
+
+		for (gain = 0; gain < 2; ++gain) {
+			if (val == scale_avail[gain].integer &&
+			val2 == scale_avail[gain].micro) {
+
+				state->gain = gain;
+				ret = mcp4728_write_input_reg(data,
+					chan->channel);
+			}
+		}
+
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+static const struct iio_info mcp4728_info = {
+	.read_raw = mcp4728_read_raw,
+	.write_raw = mcp4728_write_raw,
+	.attrs = &mcp4728_attribute_group,
+	.driver_module = THIS_MODULE,
+};
+
+static int mcp4728_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct mcp4728_data *data;
+	struct iio_dev *indio_dev;
+	u8 inbuf[24];
+	u8 *reg;
+	u8 pd;
+	int err;
+	int i, gain;
+	u64 scale_uv;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (indio_dev == NULL)
+		return -ENOMEM;
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &mcp4728_info;
+	indio_dev->channels = mcp4728_channels;
+	indio_dev->num_channels = MCP4728_NUM_CHANNELS;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	/* read current DAC value */
+	err = i2c_master_recv(client, inbuf, 24);
+	if (err < 0) {
+		dev_err(&client->dev, "failed to read DAC value");
+		return err;
+	}
+
+	for (i = 0; i < MCP4728_NUM_CHANNELS; ++i) {
+		reg = &inbuf[6*i];
+		pd = (reg[1] >> 5) & 0x3;
+		data->channel_state[i].powerdown = pd > 0 ? true : false;
+
+		/* set to 500kohm_to_gnd if channel is now powered down */
+		data->channel_state[i].powerdown_mode = pd ? pd-1 : 2;
+		data->channel_state[i].dac_value = ((reg[1] & 0x0f) << 8) |
+						   (reg[2]);
+		data->channel_state[i].gain = (reg[1] >> 4) & 0x01;
+
+		/* Use internal reference */
+		data->channel_state[i].vref = 1;
+
+		/* Populate available ADC input ranges */
+		for (gain = 0; gain < 2; gain++) {
+			/*
+			  [gain=0] = Gain Setting 1, vref x 1
+			  [gain=1] = Gain Setting 2, vref x 2
+
+			  The scale is calculated by doing:
+			    Vref >> (realbits - gain)
+			  which multiplies by two on the second component
+			  of the array.
+			 */
+			scale_uv = ((u64) MCP4728_INT_VREF_MV * 100000) >>
+				   (MCP4728_RESOLUTION - gain);
+			data->scale_avail[i][gain].micro =
+					do_div(scale_uv, 100000) * 10;
+			data->scale_avail[i][gain].integer = scale_uv;
+		}
+	}
+
+	return iio_device_register(indio_dev);
+}
+
+static int mcp4728_remove(struct i2c_client *client)
+{
+	iio_device_unregister(i2c_get_clientdata(client));
+	return 0;
+}
+
+static const struct i2c_device_id mcp4728_id[] = {
+	{ "mcp4728", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, mcp4728_id);
+
+static struct i2c_driver mcp4728_driver = {
+	.driver = {
+		.name	= MCP4728_DRV_NAME,
+		.pm	= MCP4728_PM_OPS,
+	},
+	.probe		= mcp4728_probe,
+	.remove		= mcp4728_remove,
+	.id_table	= mcp4728_id,
+};
+module_i2c_driver(mcp4728_driver);
+
+MODULE_AUTHOR("Evgeny Boger <boger@contactless.ru>");
+MODULE_DESCRIPTION("MCP4728 quad 12-bit DAC");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/iio/dac/mcp4728.h b/include/linux/iio/dac/mcp4728.h
new file mode 100644
index 0000000..14adb0c
--- /dev/null
+++ b/include/linux/iio/dac/mcp4728.h
@@ -0,0 +1,16 @@
+/*
+ * MCP4728 DAC driver
+ *
+ * Copyright (C) 2016 Evgeny Boger <boger@contactless.ru>
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#ifndef IIO_DAC_MCP4728_H_
+#define IIO_DAC_MCP4728_H_
+
+struct mcp4728_platform_data {
+	u16 vref_mv;
+};
+
+#endif /* IIO_DAC_MCP4728_H_ */
-- 
2.5.0

Re: [PATCH] iio: dac: mcp4728: Add support for MCP4728

[Peter Meerwald-Stadler]

> Microchip MCP4728 is a 12-bit quad channel
> digital-to-analog converter (DAC) with I2C interface.
> 
> This patch adds support for most of the features of MCP4728,
> including per-channel gain, power state and power down mode control.
> Driver also supports saving the current state to on-chip EEPROM.

the mcp4728 seems to be quite similar to the mcp4725 for which a driver 
already exists; the preferred way to add support for mcp4728 is by 
extending the mcp4725 driver

> At present, only internal voltage reference is supported.
> 
> Signed-off-by: Evgeny Boger <boger@contactless.ru>
> ---
>  drivers/iio/dac/Kconfig         |  11 +
>  drivers/iio/dac/Makefile        |   1 +
>  drivers/iio/dac/mcp4728.c       | 479 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/iio/dac/mcp4728.h |  16 ++
>  4 files changed, 507 insertions(+)
>  create mode 100644 drivers/iio/dac/mcp4728.c
>  create mode 100644 include/linux/iio/dac/mcp4728.h
> 
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index e701e28..8da6039 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -186,6 +186,17 @@ config MCP4725
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called mcp4725.
>  
> +config MCP4728
> +	tristate "MCP4728 DAC driver"
> +	depends on I2C
> +	---help---
> +	  Say Y here if you want to build a driver for the Microchip
> +	  MCP 4728 12-bit quad digital-to-analog converter (DAC) with I2C
> +	  interface.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called mcp4728.
> +
>  config MCP4922
>  	tristate "MCP4902, MCP4912, MCP4922 DAC driver"
>  	depends on SPI
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 63ae056..2c9decc 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -20,4 +20,5 @@ obj-$(CONFIG_M62332) += m62332.o
>  obj-$(CONFIG_MAX517) += max517.o
>  obj-$(CONFIG_MAX5821) += max5821.o
>  obj-$(CONFIG_MCP4725) += mcp4725.o
> +obj-$(CONFIG_MCP4728) += mcp4728.o
>  obj-$(CONFIG_MCP4922) += mcp4922.o
> diff --git a/drivers/iio/dac/mcp4728.c b/drivers/iio/dac/mcp4728.c
> new file mode 100644
> index 0000000..6dbbfac
> --- /dev/null
> +++ b/drivers/iio/dac/mcp4728.c
> @@ -0,0 +1,479 @@
> +/*
> + * mcp4728.c - Support for Microchip MCP4728
> + *
> + * Copyright (C) 2016 Evgeny Boger <boger@contactless.ru>
> + *
> + * Based on mcp4728 by Peter Meerwald <pmeerw@pmeerw.net>
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License.  See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * driver for the Microchip MCP4728 I2C 12-bit quad digital-to-analog
> + * converter (DAC)
> + * (7-bit I2C slave address 0x60, the three LSBs can be configured in
> + * hardware)
> + */
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/err.h>
> +#include <linux/delay.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#include <linux/iio/dac/mcp4728.h>
> +
> +#define MCP4728_DRV_NAME "mcp4728"
> +#define MCP4728_NUM_CHANNELS 4
> +#define MCP4728_INT_VREF_MV 2048
> +#define MCP4728_RESOLUTION 12
> +
> +struct mcp4728_scale {
> +	unsigned int		integer;
> +	unsigned int		micro;
> +};
> +
> +struct mcp4728_channel_state {
> +	u16 dac_value;
> +	bool powerdown;
> +	u8 gain;
> +	u8 vref;
> +	unsigned powerdown_mode;
> +};
> +
> +struct mcp4728_data {
> +	struct i2c_client *client;
> +	struct mcp4728_channel_state channel_state[MCP4728_NUM_CHANNELS];
> +	struct mcp4728_scale	scale_avail[MCP4728_NUM_CHANNELS][2];
> +
> +};
> +
> +static int mcp4728_write_reg(struct mcp4728_data *data, int ch, bool eeprom)
> +{
> +	struct mcp4728_channel_state *state = &data->channel_state[ch];
> +	u8 outbuf[3];
> +	u8 pd;
> +	int ret;
> +
> +	if (state->dac_value >= (1 << MCP4728_RESOLUTION) ||
> +				state->dac_value < 0)
> +		return -EINVAL;
> +/*
> +  Write input register. EEPROM is not updated:
> +  c2=0, c1=1, c0=0, w1=0, w0=0 : 0x40
> +
> +  Write input and EEPROM registers:
> +  c2=0, c1=1, c0=0, w1=1, w0=1 : 0x58
> +
> +  b[0]: c2 | c1 | c0 | w1 | w0 | dac1 | dac0 | /udac
> +  b[1]: vref | pd1 | pd0 | gain | d11 | d10 | d9 | d8
> +  b[2]: d7-d0
> +*/
> +	pd = state->powerdown ? (state->powerdown_mode + 1) : 0;
> +
> +	outbuf[0] = (eeprom ? 0x58 : 0x40) |
> +				((ch & 0x03) << 1);
> +	outbuf[1] = ((state->vref & 0x01) << 7) |
> +				((pd & 0x3) << 5)    |
> +				((state->gain & 0x01) << 4) |
> +				((state->dac_value >> 8) & 0x0f);
> +	outbuf[2] = state->dac_value & 0xff;
> +
> +	ret = i2c_master_send(data->client, outbuf, 3);
> +	if (ret < 0)
> +		return ret;
> +	else if (ret != 3)
> +		return -EIO;
> +	else
> +		return 0;
> +}
> +
> +static int mcp4728_write_input_reg(struct mcp4728_data *data, int ch)
> +{
> +	return mcp4728_write_reg(data, ch, false);
> +}
> +
> +static int mcp4728_suspend(struct device *dev)
> +{
> +	struct mcp4728_data *data = iio_priv(i2c_get_clientdata(
> +		to_i2c_client(dev)));
> +	int ch, ret;
> +
> +	for (ch = 0; ch < MCP4728_NUM_CHANNELS; ++ch) {
> +		data->channel_state[ch].powerdown = true;
> +		ret = mcp4728_write_input_reg(data, ch);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +static int mcp4728_resume(struct device *dev)
> +{
> +	struct mcp4728_data *data = iio_priv(i2c_get_clientdata(
> +		to_i2c_client(dev)));
> +	int ch, ret;
> +
> +	for (ch = 0; ch < MCP4728_NUM_CHANNELS; ++ch) {
> +		data->channel_state[ch].powerdown = false;
> +		ret = mcp4728_write_input_reg(data, ch);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static SIMPLE_DEV_PM_OPS(mcp4728_pm_ops, mcp4728_suspend, mcp4728_resume);
> +#define MCP4728_PM_OPS (&mcp4728_pm_ops)
> +#else
> +#define MCP4728_PM_OPS NULL
> +#endif
> +
> +static ssize_t mcp4728_store_eeprom(struct device *dev,
> +	struct device_attribute *attr, const char *buf, size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +	int tries;
> +	u8 inbuf[3];
> +	bool state;
> +	int ret, ch;
> +
> +	ret = strtobool(buf, &state);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (!state)
> +		return 0;
> +
> +	for (ch = 0; ch < MCP4728_NUM_CHANNELS; ++ch) {
> +		tries = 20;
> +		mcp4728_write_reg(data, ch, true);
> +
> +
> +		/* wait for write complete, takes up to 50ms */
> +		while (tries--) {
> +			msleep(20);
> +			ret = i2c_master_recv(data->client, inbuf, 1);
> +			if (ret < 0)
> +				return ret;
> +			else if (ret != 1)
> +				return -EIO;
> +
> +			if (inbuf[0] & 0x80)
> +				break;
> +		}
> +
> +		if (tries < 0) {
> +			dev_err(&data->client->dev,
> +				"mcp4728_store_eeprom() failed, incomplete\n");
> +			return -EIO;
> +		}
> +	}
> +
> +	return len;
> +}
> +
> +static IIO_DEVICE_ATTR(store_eeprom, S_IWUSR, NULL, mcp4728_store_eeprom, 0);
> +
> +
> +static ssize_t mcp4728_show_scale_available(struct device *dev,
> +		struct device_attribute *attr,
> +		char *buf)
> +{
> +	struct iio_dev_attr *iio_attr = to_iio_dev_attr(attr);
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +	u8 ch = iio_attr->address;
> +	int i, len = 0;
> +
> +	for (i = 0; i < ARRAY_SIZE(data->scale_avail[ch]); i++)
> +		len += sprintf(buf + len, "%u.%06u ",
> +			       data->scale_avail[ch][i].integer,
> +			       data->scale_avail[ch][i].micro);
> +
> +	len += sprintf(buf + len, "\n");
> +
> +	return len;
> +}
> +
> +#define SHOW_SCALE_AVAILABLE_ATTR(ch)					\
> +static IIO_DEVICE_ATTR(out_voltage##ch##_scale_available, S_IRUGO,	\
> +		       mcp4728_show_scale_available, NULL, ch)
> +
> +SHOW_SCALE_AVAILABLE_ATTR(0);
> +SHOW_SCALE_AVAILABLE_ATTR(1);
> +SHOW_SCALE_AVAILABLE_ATTR(2);
> +SHOW_SCALE_AVAILABLE_ATTR(3);
> +
> +static struct attribute *mcp4728_attributes[] = {
> +	&iio_dev_attr_store_eeprom.dev_attr.attr,
> +	&iio_dev_attr_out_voltage0_scale_available.dev_attr.attr,
> +	&iio_dev_attr_out_voltage1_scale_available.dev_attr.attr,
> +	&iio_dev_attr_out_voltage2_scale_available.dev_attr.attr,
> +	&iio_dev_attr_out_voltage3_scale_available.dev_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group mcp4728_attribute_group = {
> +	.attrs = mcp4728_attributes,
> +};
> +
> +static const char * const mcp4728_powerdown_modes[] = {
> +	"1kohm_to_gnd",
> +	"100kohm_to_gnd",
> +	"500kohm_to_gnd"
> +};
> +
> +static int mcp4728_get_powerdown_mode(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan)
> +{
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +
> +	return data->channel_state[chan->channel].powerdown_mode;
> +}
> +
> +static int mcp4728_set_powerdown_mode(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan, unsigned mode)
> +{
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +
> +	data->channel_state[chan->channel].powerdown_mode = mode;
> +
> +	return 0;
> +}
> +
> +static ssize_t mcp4728_read_powerdown(struct iio_dev *indio_dev,
> +	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
> +{
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +
> +	return sprintf(buf, "%d\n",
> +		data->channel_state[chan->channel].powerdown);
> +}
> +
> +static ssize_t mcp4728_write_powerdown(struct iio_dev *indio_dev,
> +	 uintptr_t private, const struct iio_chan_spec *chan,
> +	 const char *buf, size_t len)
> +{
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +	struct mcp4728_channel_state *state =
> +			&data->channel_state[chan->channel];
> +	bool powerdown;
> +	int ret;
> +
> +	ret = strtobool(buf, &powerdown);
> +	if (ret)
> +		return ret;
> +
> +	state->powerdown = powerdown;
> +	ret = mcp4728_write_input_reg(data, chan->channel);
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	return len;
> +}
> +
> +static const struct iio_enum mcp4728_powerdown_mode_enum = {
> +	.items = mcp4728_powerdown_modes,
> +	.num_items = ARRAY_SIZE(mcp4728_powerdown_modes),
> +	.get = mcp4728_get_powerdown_mode,
> +	.set = mcp4728_set_powerdown_mode,
> +};
> +
> +static const struct iio_chan_spec_ext_info mcp4728_ext_info[] = {
> +	{
> +		.name = "powerdown",
> +		.read = mcp4728_read_powerdown,
> +		.write = mcp4728_write_powerdown,
> +		.shared = IIO_SEPARATE,
> +	},
> +	IIO_ENUM("powerdown_mode", IIO_SEPARATE, &mcp4728_powerdown_mode_enum),
> +	IIO_ENUM_AVAILABLE("powerdown_mode", &mcp4728_powerdown_mode_enum),
> +	{ },
> +};
> +
> +#define MCP4728_CHAN(chan) {			\
> +	.type		= IIO_VOLTAGE, \
> +	.indexed	= 1, \
> +	.output		= 1, \
> +	.channel	= chan, \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> +			      BIT(IIO_CHAN_INFO_SCALE), \
> +	.ext_info	= mcp4728_ext_info, \
> +}
> +
> +
> +static const struct iio_chan_spec mcp4728_channels[MCP4728_NUM_CHANNELS] = {
> +	MCP4728_CHAN(0),
> +	MCP4728_CHAN(1),
> +	MCP4728_CHAN(2),
> +	MCP4728_CHAN(3),
> +};
> +
> +static int mcp4728_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long mask)
> +{
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +	struct mcp4728_channel_state *state =
> +			&data->channel_state[chan->channel];
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		*val = data->channel_state[chan->channel].dac_value;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = (state->gain + 1) * MCP4728_INT_VREF_MV;
> +		*val2 = MCP4728_RESOLUTION;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	}
> +	return -EINVAL;
> +}
> +
> +static int mcp4728_write_raw(struct iio_dev *indio_dev,
> +			       struct iio_chan_spec const *chan,
> +			       int val, int val2, long mask)
> +{
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +	struct mcp4728_channel_state *state =
> +			&data->channel_state[chan->channel];
> +	struct mcp4728_scale *scale_avail =
> +		data->scale_avail[chan->channel];
> +
> +	int ret, gain;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		state->dac_value = val;
> +		ret = mcp4728_write_input_reg(data, chan->channel);
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		ret = -EINVAL;
> +
> +		for (gain = 0; gain < 2; ++gain) {
> +			if (val == scale_avail[gain].integer &&
> +			val2 == scale_avail[gain].micro) {
> +
> +				state->gain = gain;
> +				ret = mcp4728_write_input_reg(data,
> +					chan->channel);
> +			}
> +		}
> +
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct iio_info mcp4728_info = {
> +	.read_raw = mcp4728_read_raw,
> +	.write_raw = mcp4728_write_raw,
> +	.attrs = &mcp4728_attribute_group,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static int mcp4728_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct mcp4728_data *data;
> +	struct iio_dev *indio_dev;
> +	u8 inbuf[24];
> +	u8 *reg;
> +	u8 pd;
> +	int err;
> +	int i, gain;
> +	u64 scale_uv;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (indio_dev == NULL)
> +		return -ENOMEM;
> +	data = iio_priv(indio_dev);
> +	i2c_set_clientdata(client, indio_dev);
> +	data->client = client;
> +
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->info = &mcp4728_info;
> +	indio_dev->channels = mcp4728_channels;
> +	indio_dev->num_channels = MCP4728_NUM_CHANNELS;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	/* read current DAC value */
> +	err = i2c_master_recv(client, inbuf, 24);
> +	if (err < 0) {
> +		dev_err(&client->dev, "failed to read DAC value");
> +		return err;
> +	}
> +
> +	for (i = 0; i < MCP4728_NUM_CHANNELS; ++i) {
> +		reg = &inbuf[6*i];
> +		pd = (reg[1] >> 5) & 0x3;
> +		data->channel_state[i].powerdown = pd > 0 ? true : false;
> +
> +		/* set to 500kohm_to_gnd if channel is now powered down */
> +		data->channel_state[i].powerdown_mode = pd ? pd-1 : 2;
> +		data->channel_state[i].dac_value = ((reg[1] & 0x0f) << 8) |
> +						   (reg[2]);
> +		data->channel_state[i].gain = (reg[1] >> 4) & 0x01;
> +
> +		/* Use internal reference */
> +		data->channel_state[i].vref = 1;
> +
> +		/* Populate available ADC input ranges */
> +		for (gain = 0; gain < 2; gain++) {
> +			/*
> +			  [gain=0] = Gain Setting 1, vref x 1
> +			  [gain=1] = Gain Setting 2, vref x 2
> +
> +			  The scale is calculated by doing:
> +			    Vref >> (realbits - gain)
> +			  which multiplies by two on the second component
> +			  of the array.
> +			 */
> +			scale_uv = ((u64) MCP4728_INT_VREF_MV * 100000) >>
> +				   (MCP4728_RESOLUTION - gain);
> +			data->scale_avail[i][gain].micro =
> +					do_div(scale_uv, 100000) * 10;
> +			data->scale_avail[i][gain].integer = scale_uv;
> +		}
> +	}
> +
> +	return iio_device_register(indio_dev);
> +}
> +
> +static int mcp4728_remove(struct i2c_client *client)
> +{
> +	iio_device_unregister(i2c_get_clientdata(client));
> +	return 0;
> +}
> +
> +static const struct i2c_device_id mcp4728_id[] = {
> +	{ "mcp4728", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, mcp4728_id);
> +
> +static struct i2c_driver mcp4728_driver = {
> +	.driver = {
> +		.name	= MCP4728_DRV_NAME,
> +		.pm	= MCP4728_PM_OPS,
> +	},
> +	.probe		= mcp4728_probe,
> +	.remove		= mcp4728_remove,
> +	.id_table	= mcp4728_id,
> +};
> +module_i2c_driver(mcp4728_driver);
> +
> +MODULE_AUTHOR("Evgeny Boger <boger@contactless.ru>");
> +MODULE_DESCRIPTION("MCP4728 quad 12-bit DAC");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/iio/dac/mcp4728.h b/include/linux/iio/dac/mcp4728.h
> new file mode 100644
> index 0000000..14adb0c
> --- /dev/null
> +++ b/include/linux/iio/dac/mcp4728.h
> @@ -0,0 +1,16 @@
> +/*
> + * MCP4728 DAC driver
> + *
> + * Copyright (C) 2016 Evgeny Boger <boger@contactless.ru>
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#ifndef IIO_DAC_MCP4728_H_
> +#define IIO_DAC_MCP4728_H_
> +
> +struct mcp4728_platform_data {
> +	u16 vref_mv;
> +};
> +
> +#endif /* IIO_DAC_MCP4728_H_ */
> 

-- 

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

Re: [PATCH] iio: dac: mcp4728: Add support for MCP4728

[Evgeny Boger]

On 03/14/2016 12:04 AM, Peter Meerwald-Stadler wrote:
>> Microchip MCP4728 is a 12-bit quad channel
>> digital-to-analog converter (DAC) with I2C interface.
>>
>> This patch adds support for most of the features of MCP4728,
>> including per-channel gain, power state and power down mode control.
>> Driver also supports saving the current state to on-chip EEPROM.
> the mcp4728 seems to be quite similar to the mcp4725 for which a driver
> already exists; the preferred way to add support for mcp4728 is by
> extending the mcp4725 driver

while it's indeed quite similar to mcp4725, there are significant 
differences:
1) The command formats are quite different. They share some common 
properties,
but each time we're going to communicate with the chip we need to make 
two if/else branches for mcp4725 and mcp4728.
2) MCP4725 lacks programmable gain support. The whole associated bunch 
of code won't be used.
3) MCP4728 has latched output feature which is not implemented in driver 
yet (frankly, I'm not quite sure it's possible in IIO at all).
4) As far as I understand, the scale property is declared as shared in 
mcp4725, i.e. the sysfs entry is "out_voltage_scale".
Since mcp4728 has per-channel scales, the unified driver will either 
break backwards compatibility or will have to implement ugly ifdefs to 
preserve current behaviour of MCP4725.

What do these two chips have in common:
1) The ability to store initial values in on-chip EEPROM (the message 
formats are different though).
2) The same set of power down modes

To summarize, in principle, one can think of MCP4725 as a heavily 
stripped-down 1-channel version of MCP4728.
It's possible to implement support for mcp4725 in mcp4728 driver, but 
I'm still not quite sure it's a good idea.
I would expect the combined driver source code to be obscure while 
re-using only small amount of code.




>
>> At present, only internal voltage reference is supported.
>>
>> Signed-off-by: Evgeny Boger <boger@contactless.ru>
>> ---
>>   drivers/iio/dac/Kconfig         |  11 +
>>   drivers/iio/dac/Makefile        |   1 +
>>   drivers/iio/dac/mcp4728.c       | 479 ++++++++++++++++++++++++++++++++++++++++
>>   include/linux/iio/dac/mcp4728.h |  16 ++
>>   4 files changed, 507 insertions(+)
>>   create mode 100644 drivers/iio/dac/mcp4728.c
>>   create mode 100644 include/linux/iio/dac/mcp4728.h
>>
>> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
>> index e701e28..8da6039 100644
>> --- a/drivers/iio/dac/Kconfig
>> +++ b/drivers/iio/dac/Kconfig
>> @@ -186,6 +186,17 @@ config MCP4725
>>   	  To compile this driver as a module, choose M here: the module
>>   	  will be called mcp4725.
>>   
>> +config MCP4728
>> +	tristate "MCP4728 DAC driver"
>> +	depends on I2C
>> +	---help---
>> +	  Say Y here if you want to build a driver for the Microchip
>> +	  MCP 4728 12-bit quad digital-to-analog converter (DAC) with I2C
>> +	  interface.
>> +
>> +	  To compile this driver as a module, choose M here: the module
>> +	  will be called mcp4728.
>> +
>>   config MCP4922
>>   	tristate "MCP4902, MCP4912, MCP4922 DAC driver"
>>   	depends on SPI
>> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
>> index 63ae056..2c9decc 100644
>> --- a/drivers/iio/dac/Makefile
>> +++ b/drivers/iio/dac/Makefile
>> @@ -20,4 +20,5 @@ obj-$(CONFIG_M62332) += m62332.o
>>   obj-$(CONFIG_MAX517) += max517.o
>>   obj-$(CONFIG_MAX5821) += max5821.o
>>   obj-$(CONFIG_MCP4725) += mcp4725.o
>> +obj-$(CONFIG_MCP4728) += mcp4728.o
>>   obj-$(CONFIG_MCP4922) += mcp4922.o
>> diff --git a/drivers/iio/dac/mcp4728.c b/drivers/iio/dac/mcp4728.c
>> new file mode 100644
>> index 0000000..6dbbfac
>> --- /dev/null
>> +++ b/drivers/iio/dac/mcp4728.c
>> @@ -0,0 +1,479 @@
>> +/*
>> + * mcp4728.c - Support for Microchip MCP4728
>> + *
>> + * Copyright (C) 2016 Evgeny Boger <boger@contactless.ru>
>> + *
>> + * Based on mcp4728 by Peter Meerwald <pmeerw@pmeerw.net>
>> + *
>> + * This file is subject to the terms and conditions of version 2 of
>> + * the GNU General Public License.  See the file COPYING in the main
>> + * directory of this archive for more details.
>> + *
>> + * driver for the Microchip MCP4728 I2C 12-bit quad digital-to-analog
>> + * converter (DAC)
>> + * (7-bit I2C slave address 0x60, the three LSBs can be configured in
>> + * hardware)
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/i2c.h>
>> +#include <linux/err.h>
>> +#include <linux/delay.h>
>> +
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/sysfs.h>
>> +
>> +#include <linux/iio/dac/mcp4728.h>
>> +
>> +#define MCP4728_DRV_NAME "mcp4728"
>> +#define MCP4728_NUM_CHANNELS 4
>> +#define MCP4728_INT_VREF_MV 2048
>> +#define MCP4728_RESOLUTION 12
>> +
>> +struct mcp4728_scale {
>> +	unsigned int		integer;
>> +	unsigned int		micro;
>> +};
>> +
>> +struct mcp4728_channel_state {
>> +	u16 dac_value;
>> +	bool powerdown;
>> +	u8 gain;
>> +	u8 vref;
>> +	unsigned powerdown_mode;
>> +};
>> +
>> +struct mcp4728_data {
>> +	struct i2c_client *client;
>> +	struct mcp4728_channel_state channel_state[MCP4728_NUM_CHANNELS];
>> +	struct mcp4728_scale	scale_avail[MCP4728_NUM_CHANNELS][2];
>> +
>> +};
>> +
>> +static int mcp4728_write_reg(struct mcp4728_data *data, int ch, bool eeprom)
>> +{
>> +	struct mcp4728_channel_state *state = &data->channel_state[ch];
>> +	u8 outbuf[3];
>> +	u8 pd;
>> +	int ret;
>> +
>> +	if (state->dac_value >= (1 << MCP4728_RESOLUTION) ||
>> +				state->dac_value < 0)
>> +		return -EINVAL;
>> +/*
>> +  Write input register. EEPROM is not updated:
>> +  c2=0, c1=1, c0=0, w1=0, w0=0 : 0x40
>> +
>> +  Write input and EEPROM registers:
>> +  c2=0, c1=1, c0=0, w1=1, w0=1 : 0x58
>> +
>> +  b[0]: c2 | c1 | c0 | w1 | w0 | dac1 | dac0 | /udac
>> +  b[1]: vref | pd1 | pd0 | gain | d11 | d10 | d9 | d8
>> +  b[2]: d7-d0
>> +*/
>> +	pd = state->powerdown ? (state->powerdown_mode + 1) : 0;
>> +
>> +	outbuf[0] = (eeprom ? 0x58 : 0x40) |
>> +				((ch & 0x03) << 1);
>> +	outbuf[1] = ((state->vref & 0x01) << 7) |
>> +				((pd & 0x3) << 5)    |
>> +				((state->gain & 0x01) << 4) |
>> +				((state->dac_value >> 8) & 0x0f);
>> +	outbuf[2] = state->dac_value & 0xff;
>> +
>> +	ret = i2c_master_send(data->client, outbuf, 3);
>> +	if (ret < 0)
>> +		return ret;
>> +	else if (ret != 3)
>> +		return -EIO;
>> +	else
>> +		return 0;
>> +}
>> +
>> +static int mcp4728_write_input_reg(struct mcp4728_data *data, int ch)
>> +{
>> +	return mcp4728_write_reg(data, ch, false);
>> +}
>> +
>> +static int mcp4728_suspend(struct device *dev)
>> +{
>> +	struct mcp4728_data *data = iio_priv(i2c_get_clientdata(
>> +		to_i2c_client(dev)));
>> +	int ch, ret;
>> +
>> +	for (ch = 0; ch < MCP4728_NUM_CHANNELS; ++ch) {
>> +		data->channel_state[ch].powerdown = true;
>> +		ret = mcp4728_write_input_reg(data, ch);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int mcp4728_resume(struct device *dev)
>> +{
>> +	struct mcp4728_data *data = iio_priv(i2c_get_clientdata(
>> +		to_i2c_client(dev)));
>> +	int ch, ret;
>> +
>> +	for (ch = 0; ch < MCP4728_NUM_CHANNELS; ++ch) {
>> +		data->channel_state[ch].powerdown = false;
>> +		ret = mcp4728_write_input_reg(data, ch);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static SIMPLE_DEV_PM_OPS(mcp4728_pm_ops, mcp4728_suspend, mcp4728_resume);
>> +#define MCP4728_PM_OPS (&mcp4728_pm_ops)
>> +#else
>> +#define MCP4728_PM_OPS NULL
>> +#endif
>> +
>> +static ssize_t mcp4728_store_eeprom(struct device *dev,
>> +	struct device_attribute *attr, const char *buf, size_t len)
>> +{
>> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +	int tries;
>> +	u8 inbuf[3];
>> +	bool state;
>> +	int ret, ch;
>> +
>> +	ret = strtobool(buf, &state);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	if (!state)
>> +		return 0;
>> +
>> +	for (ch = 0; ch < MCP4728_NUM_CHANNELS; ++ch) {
>> +		tries = 20;
>> +		mcp4728_write_reg(data, ch, true);
>> +
>> +
>> +		/* wait for write complete, takes up to 50ms */
>> +		while (tries--) {
>> +			msleep(20);
>> +			ret = i2c_master_recv(data->client, inbuf, 1);
>> +			if (ret < 0)
>> +				return ret;
>> +			else if (ret != 1)
>> +				return -EIO;
>> +
>> +			if (inbuf[0] & 0x80)
>> +				break;
>> +		}
>> +
>> +		if (tries < 0) {
>> +			dev_err(&data->client->dev,
>> +				"mcp4728_store_eeprom() failed, incomplete\n");
>> +			return -EIO;
>> +		}
>> +	}
>> +
>> +	return len;
>> +}
>> +
>> +static IIO_DEVICE_ATTR(store_eeprom, S_IWUSR, NULL, mcp4728_store_eeprom, 0);
>> +
>> +
>> +static ssize_t mcp4728_show_scale_available(struct device *dev,
>> +		struct device_attribute *attr,
>> +		char *buf)
>> +{
>> +	struct iio_dev_attr *iio_attr = to_iio_dev_attr(attr);
>> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +	u8 ch = iio_attr->address;
>> +	int i, len = 0;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(data->scale_avail[ch]); i++)
>> +		len += sprintf(buf + len, "%u.%06u ",
>> +			       data->scale_avail[ch][i].integer,
>> +			       data->scale_avail[ch][i].micro);
>> +
>> +	len += sprintf(buf + len, "\n");
>> +
>> +	return len;
>> +}
>> +
>> +#define SHOW_SCALE_AVAILABLE_ATTR(ch)					\
>> +static IIO_DEVICE_ATTR(out_voltage##ch##_scale_available, S_IRUGO,	\
>> +		       mcp4728_show_scale_available, NULL, ch)
>> +
>> +SHOW_SCALE_AVAILABLE_ATTR(0);
>> +SHOW_SCALE_AVAILABLE_ATTR(1);
>> +SHOW_SCALE_AVAILABLE_ATTR(2);
>> +SHOW_SCALE_AVAILABLE_ATTR(3);
>> +
>> +static struct attribute *mcp4728_attributes[] = {
>> +	&iio_dev_attr_store_eeprom.dev_attr.attr,
>> +	&iio_dev_attr_out_voltage0_scale_available.dev_attr.attr,
>> +	&iio_dev_attr_out_voltage1_scale_available.dev_attr.attr,
>> +	&iio_dev_attr_out_voltage2_scale_available.dev_attr.attr,
>> +	&iio_dev_attr_out_voltage3_scale_available.dev_attr.attr,
>> +	NULL,
>> +};
>> +
>> +static const struct attribute_group mcp4728_attribute_group = {
>> +	.attrs = mcp4728_attributes,
>> +};
>> +
>> +static const char * const mcp4728_powerdown_modes[] = {
>> +	"1kohm_to_gnd",
>> +	"100kohm_to_gnd",
>> +	"500kohm_to_gnd"
>> +};
>> +
>> +static int mcp4728_get_powerdown_mode(struct iio_dev *indio_dev,
>> +	const struct iio_chan_spec *chan)
>> +{
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +
>> +	return data->channel_state[chan->channel].powerdown_mode;
>> +}
>> +
>> +static int mcp4728_set_powerdown_mode(struct iio_dev *indio_dev,
>> +	const struct iio_chan_spec *chan, unsigned mode)
>> +{
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +
>> +	data->channel_state[chan->channel].powerdown_mode = mode;
>> +
>> +	return 0;
>> +}
>> +
>> +static ssize_t mcp4728_read_powerdown(struct iio_dev *indio_dev,
>> +	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
>> +{
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +
>> +	return sprintf(buf, "%d\n",
>> +		data->channel_state[chan->channel].powerdown);
>> +}
>> +
>> +static ssize_t mcp4728_write_powerdown(struct iio_dev *indio_dev,
>> +	 uintptr_t private, const struct iio_chan_spec *chan,
>> +	 const char *buf, size_t len)
>> +{
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +	struct mcp4728_channel_state *state =
>> +			&data->channel_state[chan->channel];
>> +	bool powerdown;
>> +	int ret;
>> +
>> +	ret = strtobool(buf, &powerdown);
>> +	if (ret)
>> +		return ret;
>> +
>> +	state->powerdown = powerdown;
>> +	ret = mcp4728_write_input_reg(data, chan->channel);
>> +
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return len;
>> +}
>> +
>> +static const struct iio_enum mcp4728_powerdown_mode_enum = {
>> +	.items = mcp4728_powerdown_modes,
>> +	.num_items = ARRAY_SIZE(mcp4728_powerdown_modes),
>> +	.get = mcp4728_get_powerdown_mode,
>> +	.set = mcp4728_set_powerdown_mode,
>> +};
>> +
>> +static const struct iio_chan_spec_ext_info mcp4728_ext_info[] = {
>> +	{
>> +		.name = "powerdown",
>> +		.read = mcp4728_read_powerdown,
>> +		.write = mcp4728_write_powerdown,
>> +		.shared = IIO_SEPARATE,
>> +	},
>> +	IIO_ENUM("powerdown_mode", IIO_SEPARATE, &mcp4728_powerdown_mode_enum),
>> +	IIO_ENUM_AVAILABLE("powerdown_mode", &mcp4728_powerdown_mode_enum),
>> +	{ },
>> +};
>> +
>> +#define MCP4728_CHAN(chan) {			\
>> +	.type		= IIO_VOLTAGE, \
>> +	.indexed	= 1, \
>> +	.output		= 1, \
>> +	.channel	= chan, \
>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
>> +			      BIT(IIO_CHAN_INFO_SCALE), \
>> +	.ext_info	= mcp4728_ext_info, \
>> +}
>> +
>> +
>> +static const struct iio_chan_spec mcp4728_channels[MCP4728_NUM_CHANNELS] = {
>> +	MCP4728_CHAN(0),
>> +	MCP4728_CHAN(1),
>> +	MCP4728_CHAN(2),
>> +	MCP4728_CHAN(3),
>> +};
>> +
>> +static int mcp4728_read_raw(struct iio_dev *indio_dev,
>> +			   struct iio_chan_spec const *chan,
>> +			   int *val, int *val2, long mask)
>> +{
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +	struct mcp4728_channel_state *state =
>> +			&data->channel_state[chan->channel];
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_RAW:
>> +		*val = data->channel_state[chan->channel].dac_value;
>> +		return IIO_VAL_INT;
>> +	case IIO_CHAN_INFO_SCALE:
>> +		*val = (state->gain + 1) * MCP4728_INT_VREF_MV;
>> +		*val2 = MCP4728_RESOLUTION;
>> +		return IIO_VAL_FRACTIONAL_LOG2;
>> +	}
>> +	return -EINVAL;
>> +}
>> +
>> +static int mcp4728_write_raw(struct iio_dev *indio_dev,
>> +			       struct iio_chan_spec const *chan,
>> +			       int val, int val2, long mask)
>> +{
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +	struct mcp4728_channel_state *state =
>> +			&data->channel_state[chan->channel];
>> +	struct mcp4728_scale *scale_avail =
>> +		data->scale_avail[chan->channel];
>> +
>> +	int ret, gain;
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_RAW:
>> +		state->dac_value = val;
>> +		ret = mcp4728_write_input_reg(data, chan->channel);
>> +		break;
>> +	case IIO_CHAN_INFO_SCALE:
>> +		ret = -EINVAL;
>> +
>> +		for (gain = 0; gain < 2; ++gain) {
>> +			if (val == scale_avail[gain].integer &&
>> +			val2 == scale_avail[gain].micro) {
>> +
>> +				state->gain = gain;
>> +				ret = mcp4728_write_input_reg(data,
>> +					chan->channel);
>> +			}
>> +		}
>> +
>> +		break;
>> +	default:
>> +		ret = -EINVAL;
>> +		break;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct iio_info mcp4728_info = {
>> +	.read_raw = mcp4728_read_raw,
>> +	.write_raw = mcp4728_write_raw,
>> +	.attrs = &mcp4728_attribute_group,
>> +	.driver_module = THIS_MODULE,
>> +};
>> +
>> +static int mcp4728_probe(struct i2c_client *client,
>> +			 const struct i2c_device_id *id)
>> +{
>> +	struct mcp4728_data *data;
>> +	struct iio_dev *indio_dev;
>> +	u8 inbuf[24];
>> +	u8 *reg;
>> +	u8 pd;
>> +	int err;
>> +	int i, gain;
>> +	u64 scale_uv;
>> +
>> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>> +	if (indio_dev == NULL)
>> +		return -ENOMEM;
>> +	data = iio_priv(indio_dev);
>> +	i2c_set_clientdata(client, indio_dev);
>> +	data->client = client;
>> +
>> +	indio_dev->dev.parent = &client->dev;
>> +	indio_dev->info = &mcp4728_info;
>> +	indio_dev->channels = mcp4728_channels;
>> +	indio_dev->num_channels = MCP4728_NUM_CHANNELS;
>> +	indio_dev->modes = INDIO_DIRECT_MODE;
>> +
>> +	/* read current DAC value */
>> +	err = i2c_master_recv(client, inbuf, 24);
>> +	if (err < 0) {
>> +		dev_err(&client->dev, "failed to read DAC value");
>> +		return err;
>> +	}
>> +
>> +	for (i = 0; i < MCP4728_NUM_CHANNELS; ++i) {
>> +		reg = &inbuf[6*i];
>> +		pd = (reg[1] >> 5) & 0x3;
>> +		data->channel_state[i].powerdown = pd > 0 ? true : false;
>> +
>> +		/* set to 500kohm_to_gnd if channel is now powered down */
>> +		data->channel_state[i].powerdown_mode = pd ? pd-1 : 2;
>> +		data->channel_state[i].dac_value = ((reg[1] & 0x0f) << 8) |
>> +						   (reg[2]);
>> +		data->channel_state[i].gain = (reg[1] >> 4) & 0x01;
>> +
>> +		/* Use internal reference */
>> +		data->channel_state[i].vref = 1;
>> +
>> +		/* Populate available ADC input ranges */
>> +		for (gain = 0; gain < 2; gain++) {
>> +			/*
>> +			  [gain=0] = Gain Setting 1, vref x 1
>> +			  [gain=1] = Gain Setting 2, vref x 2
>> +
>> +			  The scale is calculated by doing:
>> +			    Vref >> (realbits - gain)
>> +			  which multiplies by two on the second component
>> +			  of the array.
>> +			 */
>> +			scale_uv = ((u64) MCP4728_INT_VREF_MV * 100000) >>
>> +				   (MCP4728_RESOLUTION - gain);
>> +			data->scale_avail[i][gain].micro =
>> +					do_div(scale_uv, 100000) * 10;
>> +			data->scale_avail[i][gain].integer = scale_uv;
>> +		}
>> +	}
>> +
>> +	return iio_device_register(indio_dev);
>> +}
>> +
>> +static int mcp4728_remove(struct i2c_client *client)
>> +{
>> +	iio_device_unregister(i2c_get_clientdata(client));
>> +	return 0;
>> +}
>> +
>> +static const struct i2c_device_id mcp4728_id[] = {
>> +	{ "mcp4728", 0 },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(i2c, mcp4728_id);
>> +
>> +static struct i2c_driver mcp4728_driver = {
>> +	.driver = {
>> +		.name	= MCP4728_DRV_NAME,
>> +		.pm	= MCP4728_PM_OPS,
>> +	},
>> +	.probe		= mcp4728_probe,
>> +	.remove		= mcp4728_remove,
>> +	.id_table	= mcp4728_id,
>> +};
>> +module_i2c_driver(mcp4728_driver);
>> +
>> +MODULE_AUTHOR("Evgeny Boger <boger@contactless.ru>");
>> +MODULE_DESCRIPTION("MCP4728 quad 12-bit DAC");
>> +MODULE_LICENSE("GPL");
>> diff --git a/include/linux/iio/dac/mcp4728.h b/include/linux/iio/dac/mcp4728.h
>> new file mode 100644
>> index 0000000..14adb0c
>> --- /dev/null
>> +++ b/include/linux/iio/dac/mcp4728.h
>> @@ -0,0 +1,16 @@
>> +/*
>> + * MCP4728 DAC driver
>> + *
>> + * Copyright (C) 2016 Evgeny Boger <boger@contactless.ru>
>> + *
>> + * Licensed under the GPL-2 or later.
>> + */
>> +
>> +#ifndef IIO_DAC_MCP4728_H_
>> +#define IIO_DAC_MCP4728_H_
>> +
>> +struct mcp4728_platform_data {
>> +	u16 vref_mv;
>> +};
>> +
>> +#endif /* IIO_DAC_MCP4728_H_ */
>>


-- 
С уважением,
     Евгений Богер / Evgeny Boger
     Wiren Board Team
     ООО Бесконтактные устройства
     http://contactless.ru
     +7 919 965 88 36

Re: [PATCH] iio: dac: mcp4728: Add support for MCP4728

[Jonathan Cameron]

On 13/03/16 19:11, Evgeny Boger wrote:
> Microchip MCP4728 is a 12-bit quad channel
> digital-to-analog converter (DAC) with I2C interface.
> 
> This patch adds support for most of the features of MCP4728,
> including per-channel gain, power state and power down mode control.
> Driver also supports saving the current state to on-chip EEPROM.
> 
> At present, only internal voltage reference is supported.
> 
> Signed-off-by: Evgeny Boger <boger@contactless.ru>
A couple of tiny points inline.  Nothing signficant.

Jonathan
> ---
>  drivers/iio/dac/Kconfig         |  11 +
>  drivers/iio/dac/Makefile        |   1 +
>  drivers/iio/dac/mcp4728.c       | 479 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/iio/dac/mcp4728.h |  16 ++
>  4 files changed, 507 insertions(+)
>  create mode 100644 drivers/iio/dac/mcp4728.c
>  create mode 100644 include/linux/iio/dac/mcp4728.h
> 
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index e701e28..8da6039 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -186,6 +186,17 @@ config MCP4725
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called mcp4725.
>  
> +config MCP4728
> +	tristate "MCP4728 DAC driver"
> +	depends on I2C
> +	---help---
> +	  Say Y here if you want to build a driver for the Microchip
> +	  MCP 4728 12-bit quad digital-to-analog converter (DAC) with I2C
> +	  interface.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called mcp4728.
> +
>  config MCP4922
>  	tristate "MCP4902, MCP4912, MCP4922 DAC driver"
>  	depends on SPI
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 63ae056..2c9decc 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -20,4 +20,5 @@ obj-$(CONFIG_M62332) += m62332.o
>  obj-$(CONFIG_MAX517) += max517.o
>  obj-$(CONFIG_MAX5821) += max5821.o
>  obj-$(CONFIG_MCP4725) += mcp4725.o
> +obj-$(CONFIG_MCP4728) += mcp4728.o
>  obj-$(CONFIG_MCP4922) += mcp4922.o
> diff --git a/drivers/iio/dac/mcp4728.c b/drivers/iio/dac/mcp4728.c
> new file mode 100644
> index 0000000..6dbbfac
> --- /dev/null
> +++ b/drivers/iio/dac/mcp4728.c
> @@ -0,0 +1,479 @@
> +/*
> + * mcp4728.c - Support for Microchip MCP4728
> + *
> + * Copyright (C) 2016 Evgeny Boger <boger@contactless.ru>
> + *
> + * Based on mcp4728 by Peter Meerwald <pmeerw@pmeerw.net>
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License.  See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * driver for the Microchip MCP4728 I2C 12-bit quad digital-to-analog
> + * converter (DAC)
> + * (7-bit I2C slave address 0x60, the three LSBs can be configured in
> + * hardware)
> + */
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/err.h>
> +#include <linux/delay.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#include <linux/iio/dac/mcp4728.h>
> +
> +#define MCP4728_DRV_NAME "mcp4728"
> +#define MCP4728_NUM_CHANNELS 4
> +#define MCP4728_INT_VREF_MV 2048
> +#define MCP4728_RESOLUTION 12
> +
> +struct mcp4728_scale {
> +	unsigned int		integer;
> +	unsigned int		micro;
> +};
> +
> +struct mcp4728_channel_state {
> +	u16 dac_value;
> +	bool powerdown;
> +	u8 gain;
> +	u8 vref;
> +	unsigned powerdown_mode;
> +};
> +
> +struct mcp4728_data {
> +	struct i2c_client *client;
> +	struct mcp4728_channel_state channel_state[MCP4728_NUM_CHANNELS];
> +	struct mcp4728_scale	scale_avail[MCP4728_NUM_CHANNELS][2];
Spacing a bit random looking here.  Just go with a single space
every time as it's a lot easier to maintain without noise if additional
elements are added to this structure.
> +
> +};
> +
> +static int mcp4728_write_reg(struct mcp4728_data *data, int ch, bool eeprom)
> +{
> +	struct mcp4728_channel_state *state = &data->channel_state[ch];
> +	u8 outbuf[3];
> +	u8 pd;
> +	int ret;
> +
> +	if (state->dac_value >= (1 << MCP4728_RESOLUTION) ||
> +				state->dac_value < 0)
> +		return -EINVAL;
> +/*
> +  Write input register. EEPROM is not updated:
> +  c2=0, c1=1, c0=0, w1=0, w0=0 : 0x40
> +
> +  Write input and EEPROM registers:
> +  c2=0, c1=1, c0=0, w1=1, w0=1 : 0x58
> +
> +  b[0]: c2 | c1 | c0 | w1 | w0 | dac1 | dac0 | /udac
> +  b[1]: vref | pd1 | pd0 | gain | d11 | d10 | d9 | d8
> +  b[2]: d7-d0
> +*/
> +	pd = state->powerdown ? (state->powerdown_mode + 1) : 0;
> +
> +	outbuf[0] = (eeprom ? 0x58 : 0x40) |
> +				((ch & 0x03) << 1);
> +	outbuf[1] = ((state->vref & 0x01) << 7) |
> +				((pd & 0x3) << 5)    |
> +				((state->gain & 0x01) << 4) |
> +				((state->dac_value >> 8) & 0x0f);
> +	outbuf[2] = state->dac_value & 0xff;
> +
> +	ret = i2c_master_send(data->client, outbuf, 3);
> +	if (ret < 0)
> +		return ret;
> +	else if (ret != 3)
> +		return -EIO;
> +	else
> +		return 0;
> +}
> +
> +static int mcp4728_write_input_reg(struct mcp4728_data *data, int ch)
> +{
> +	return mcp4728_write_reg(data, ch, false);
> +}
> +
> +static int mcp4728_suspend(struct device *dev)
> +{
> +	struct mcp4728_data *data = iio_priv(i2c_get_clientdata(
> +		to_i2c_client(dev)));
> +	int ch, ret;
> +
> +	for (ch = 0; ch < MCP4728_NUM_CHANNELS; ++ch) {
> +		data->channel_state[ch].powerdown = true;
> +		ret = mcp4728_write_input_reg(data, ch);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +static int mcp4728_resume(struct device *dev)
> +{
> +	struct mcp4728_data *data = iio_priv(i2c_get_clientdata(
> +		to_i2c_client(dev)));
> +	int ch, ret;
> +
> +	for (ch = 0; ch < MCP4728_NUM_CHANNELS; ++ch) {
> +		data->channel_state[ch].powerdown = false;
> +		ret = mcp4728_write_input_reg(data, ch);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static SIMPLE_DEV_PM_OPS(mcp4728_pm_ops, mcp4728_suspend, mcp4728_resume);
> +#define MCP4728_PM_OPS (&mcp4728_pm_ops)
> +#else
> +#define MCP4728_PM_OPS NULL
> +#endif
> +
> +static ssize_t mcp4728_store_eeprom(struct device *dev,
> +	struct device_attribute *attr, const char *buf, size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +	int tries;
> +	u8 inbuf[3];
> +	bool state;
> +	int ret, ch;
> +
> +	ret = strtobool(buf, &state);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (!state)
> +		return 0;
> +
> +	for (ch = 0; ch < MCP4728_NUM_CHANNELS; ++ch) {
> +		tries = 20;
> +		mcp4728_write_reg(data, ch, true);
> +
> +
> +		/* wait for write complete, takes up to 50ms */
> +		while (tries--) {
> +			msleep(20);
> +			ret = i2c_master_recv(data->client, inbuf, 1);
> +			if (ret < 0)
> +				return ret;
> +			else if (ret != 1)
> +				return -EIO;
> +
> +			if (inbuf[0] & 0x80)
> +				break;
> +		}
> +
> +		if (tries < 0) {
> +			dev_err(&data->client->dev,
> +				"mcp4728_store_eeprom() failed, incomplete\n");
> +			return -EIO;
> +		}
> +	}
> +
> +	return len;
> +}
> +
> +static IIO_DEVICE_ATTR(store_eeprom, S_IWUSR, NULL, mcp4728_store_eeprom, 0);
> +
One blank line is pretty much always enough.
> +
> +static ssize_t mcp4728_show_scale_available(struct device *dev,
> +		struct device_attribute *attr,
> +		char *buf)
> +{
> +	struct iio_dev_attr *iio_attr = to_iio_dev_attr(attr);
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +	u8 ch = iio_attr->address;
> +	int i, len = 0;
> +
> +	for (i = 0; i < ARRAY_SIZE(data->scale_avail[ch]); i++)
> +		len += sprintf(buf + len, "%u.%06u ",
> +			       data->scale_avail[ch][i].integer,
> +			       data->scale_avail[ch][i].micro);
> +
> +	len += sprintf(buf + len, "\n");
> +
> +	return len;
> +}
> +
> +#define SHOW_SCALE_AVAILABLE_ATTR(ch)					\
> +static IIO_DEVICE_ATTR(out_voltage##ch##_scale_available, S_IRUGO,	\
> +		       mcp4728_show_scale_available, NULL, ch)
> +
> +SHOW_SCALE_AVAILABLE_ATTR(0);
> +SHOW_SCALE_AVAILABLE_ATTR(1);
> +SHOW_SCALE_AVAILABLE_ATTR(2);
> +SHOW_SCALE_AVAILABLE_ATTR(3);
> +
> +static struct attribute *mcp4728_attributes[] = {
> +	&iio_dev_attr_store_eeprom.dev_attr.attr,
> +	&iio_dev_attr_out_voltage0_scale_available.dev_attr.attr,
> +	&iio_dev_attr_out_voltage1_scale_available.dev_attr.attr,
> +	&iio_dev_attr_out_voltage2_scale_available.dev_attr.attr,
> +	&iio_dev_attr_out_voltage3_scale_available.dev_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group mcp4728_attribute_group = {
> +	.attrs = mcp4728_attributes,
> +};
> +
> +static const char * const mcp4728_powerdown_modes[] = {
> +	"1kohm_to_gnd",
> +	"100kohm_to_gnd",
> +	"500kohm_to_gnd"
> +};
> +
> +static int mcp4728_get_powerdown_mode(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan)
> +{
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +
> +	return data->channel_state[chan->channel].powerdown_mode;
> +}
> +
> +static int mcp4728_set_powerdown_mode(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan, unsigned mode)
> +{
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +
> +	data->channel_state[chan->channel].powerdown_mode = mode;
> +
> +	return 0;
> +}
> +
> +static ssize_t mcp4728_read_powerdown(struct iio_dev *indio_dev,
> +	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
> +{
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +
> +	return sprintf(buf, "%d\n",
> +		data->channel_state[chan->channel].powerdown);
> +}
> +
> +static ssize_t mcp4728_write_powerdown(struct iio_dev *indio_dev,
> +	 uintptr_t private, const struct iio_chan_spec *chan,
> +	 const char *buf, size_t len)
> +{
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +	struct mcp4728_channel_state *state =
> +			&data->channel_state[chan->channel];
> +	bool powerdown;
> +	int ret;
> +
> +	ret = strtobool(buf, &powerdown);
> +	if (ret)
> +		return ret;
> +
> +	state->powerdown = powerdown;
> +	ret = mcp4728_write_input_reg(data, chan->channel);
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	return len;
> +}
> +
> +static const struct iio_enum mcp4728_powerdown_mode_enum = {
> +	.items = mcp4728_powerdown_modes,
> +	.num_items = ARRAY_SIZE(mcp4728_powerdown_modes),
> +	.get = mcp4728_get_powerdown_mode,
> +	.set = mcp4728_set_powerdown_mode,
> +};
> +
> +static const struct iio_chan_spec_ext_info mcp4728_ext_info[] = {
> +	{
> +		.name = "powerdown",
> +		.read = mcp4728_read_powerdown,
> +		.write = mcp4728_write_powerdown,
> +		.shared = IIO_SEPARATE,
> +	},
> +	IIO_ENUM("powerdown_mode", IIO_SEPARATE, &mcp4728_powerdown_mode_enum),
> +	IIO_ENUM_AVAILABLE("powerdown_mode", &mcp4728_powerdown_mode_enum),
> +	{ },
> +};
> +
> +#define MCP4728_CHAN(chan) {			\
> +	.type		= IIO_VOLTAGE, \
> +	.indexed	= 1, \
> +	.output		= 1, \
> +	.channel	= chan, \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> +			      BIT(IIO_CHAN_INFO_SCALE), \
> +	.ext_info	= mcp4728_ext_info, \
> +}
> +
> +
> +static const struct iio_chan_spec mcp4728_channels[MCP4728_NUM_CHANNELS] = {
> +	MCP4728_CHAN(0),
> +	MCP4728_CHAN(1),
> +	MCP4728_CHAN(2),
> +	MCP4728_CHAN(3),
> +};
> +
> +static int mcp4728_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long mask)
> +{
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +	struct mcp4728_channel_state *state =
> +			&data->channel_state[chan->channel];
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		*val = data->channel_state[chan->channel].dac_value;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = (state->gain + 1) * MCP4728_INT_VREF_MV;
> +		*val2 = MCP4728_RESOLUTION;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	}
> +	return -EINVAL;
> +}
> +
> +static int mcp4728_write_raw(struct iio_dev *indio_dev,
> +			       struct iio_chan_spec const *chan,
> +			       int val, int val2, long mask)
> +{
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +	struct mcp4728_channel_state *state =
> +			&data->channel_state[chan->channel];
> +	struct mcp4728_scale *scale_avail =
> +		data->scale_avail[chan->channel];
> +
> +	int ret, gain;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		state->dac_value = val;
> +		ret = mcp4728_write_input_reg(data, chan->channel);
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		ret = -EINVAL;
> +
> +		for (gain = 0; gain < 2; ++gain) {
> +			if (val == scale_avail[gain].integer &&
> +			val2 == scale_avail[gain].micro) {
> +
The blank line here doesn't add much value in readability so drop it.
> +				state->gain = gain;
> +				ret = mcp4728_write_input_reg(data,
> +					chan->channel);
> +			}
> +		}
> +
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct iio_info mcp4728_info = {
> +	.read_raw = mcp4728_read_raw,
> +	.write_raw = mcp4728_write_raw,
> +	.attrs = &mcp4728_attribute_group,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static int mcp4728_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct mcp4728_data *data;
> +	struct iio_dev *indio_dev;
> +	u8 inbuf[24];
> +	u8 *reg;
> +	u8 pd;
> +	int err;
> +	int i, gain;
> +	u64 scale_uv;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (indio_dev == NULL)
> +		return -ENOMEM;
> +	data = iio_priv(indio_dev);
> +	i2c_set_clientdata(client, indio_dev);
> +	data->client = client;
> +
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->info = &mcp4728_info;
> +	indio_dev->channels = mcp4728_channels;
> +	indio_dev->num_channels = MCP4728_NUM_CHANNELS;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	/* read current DAC value */
> +	err = i2c_master_recv(client, inbuf, 24);
> +	if (err < 0) {
> +		dev_err(&client->dev, "failed to read DAC value");
> +		return err;
> +	}
> +
> +	for (i = 0; i < MCP4728_NUM_CHANNELS; ++i) {
> +		reg = &inbuf[6*i];
> +		pd = (reg[1] >> 5) & 0x3;
> +		data->channel_state[i].powerdown = pd > 0 ? true : false;
> +
> +		/* set to 500kohm_to_gnd if channel is now powered down */
> +		data->channel_state[i].powerdown_mode = pd ? pd-1 : 2;
> +		data->channel_state[i].dac_value = ((reg[1] & 0x0f) << 8) |
> +						   (reg[2]);
> +		data->channel_state[i].gain = (reg[1] >> 4) & 0x01;
> +
> +		/* Use internal reference */
> +		data->channel_state[i].vref = 1;
> +
> +		/* Populate available ADC input ranges */
ADC?
> +		for (gain = 0; gain < 2; gain++) {
> +			/*
> +			  [gain=0] = Gain Setting 1, vref x 1
> +			  [gain=1] = Gain Setting 2, vref x 2
> +
> +			  The scale is calculated by doing:
> +			    Vref >> (realbits - gain)
> +			  which multiplies by two on the second component
> +			  of the array.
> +			 */
> +			scale_uv = ((u64) MCP4728_INT_VREF_MV * 100000) >>
> +				   (MCP4728_RESOLUTION - gain);
> +			data->scale_avail[i][gain].micro =
> +					do_div(scale_uv, 100000) * 10;
> +			data->scale_avail[i][gain].integer = scale_uv;
> +		}
> +	}
> +
> +	return iio_device_register(indio_dev);
> +}
> +
> +static int mcp4728_remove(struct i2c_client *client)
> +{
> +	iio_device_unregister(i2c_get_clientdata(client));
If there really isn't anything else to do in the remove, use
devm_iio_device_register and you can drop the remove function entirely.

> +	return 0;
> +}
> +
> +static const struct i2c_device_id mcp4728_id[] = {
> +	{ "mcp4728", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, mcp4728_id);
> +
> +static struct i2c_driver mcp4728_driver = {
> +	.driver = {
> +		.name	= MCP4728_DRV_NAME,
> +		.pm	= MCP4728_PM_OPS,
> +	},
> +	.probe		= mcp4728_probe,
> +	.remove		= mcp4728_remove,
> +	.id_table	= mcp4728_id,
> +};
> +module_i2c_driver(mcp4728_driver);
> +
> +MODULE_AUTHOR("Evgeny Boger <boger@contactless.ru>");
> +MODULE_DESCRIPTION("MCP4728 quad 12-bit DAC");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/iio/dac/mcp4728.h b/include/linux/iio/dac/mcp4728.h
> new file mode 100644
> index 0000000..14adb0c
> --- /dev/null
> +++ b/include/linux/iio/dac/mcp4728.h
> @@ -0,0 +1,16 @@
> +/*
> + * MCP4728 DAC driver
> + *
> + * Copyright (C) 2016 Evgeny Boger <boger@contactless.ru>
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#ifndef IIO_DAC_MCP4728_H_
> +#define IIO_DAC_MCP4728_H_
> +
> +struct mcp4728_platform_data {
> +	u16 vref_mv;
> +};
> +
> +#endif /* IIO_DAC_MCP4728_H_ */
> 

Re: [PATCH] iio: dac: mcp4728: Add support for MCP4728

[Peter Meerwald-Stadler]

> Microchip MCP4728 is a 12-bit quad channel
> digital-to-analog converter (DAC) with I2C interface.
> 
> This patch adds support for most of the features of MCP4728,
> including per-channel gain, power state and power down mode control.
> Driver also supports saving the current state to on-chip EEPROM.

since Jonathan seems to be OK with a separate driver, here are my review 
comments...
 
> Signed-off-by: Evgeny Boger <boger@contactless.ru>
> ---
>  drivers/iio/dac/Kconfig         |  11 +
>  drivers/iio/dac/Makefile        |   1 +
>  drivers/iio/dac/mcp4728.c       | 479 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/iio/dac/mcp4728.h |  16 ++
>  4 files changed, 507 insertions(+)
>  create mode 100644 drivers/iio/dac/mcp4728.c
>  create mode 100644 include/linux/iio/dac/mcp4728.h
> 
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index e701e28..8da6039 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -186,6 +186,17 @@ config MCP4725
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called mcp4725.
>  
> +config MCP4728
> +	tristate "MCP4728 DAC driver"
> +	depends on I2C
> +	---help---
> +	  Say Y here if you want to build a driver for the Microchip
> +	  MCP 4728 12-bit quad digital-to-analog converter (DAC) with I2C
> +	  interface.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called mcp4728.
> +
>  config MCP4922
>  	tristate "MCP4902, MCP4912, MCP4922 DAC driver"
>  	depends on SPI
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 63ae056..2c9decc 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -20,4 +20,5 @@ obj-$(CONFIG_M62332) += m62332.o
>  obj-$(CONFIG_MAX517) += max517.o
>  obj-$(CONFIG_MAX5821) += max5821.o
>  obj-$(CONFIG_MCP4725) += mcp4725.o
> +obj-$(CONFIG_MCP4728) += mcp4728.o
>  obj-$(CONFIG_MCP4922) += mcp4922.o
> diff --git a/drivers/iio/dac/mcp4728.c b/drivers/iio/dac/mcp4728.c
> new file mode 100644
> index 0000000..6dbbfac
> --- /dev/null
> +++ b/drivers/iio/dac/mcp4728.c
> @@ -0,0 +1,479 @@
> +/*
> + * mcp4728.c - Support for Microchip MCP4728
> + *
> + * Copyright (C) 2016 Evgeny Boger <boger@contactless.ru>
> + *
> + * Based on mcp4728 by Peter Meerwald <pmeerw@pmeerw.net>
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License.  See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * driver for the Microchip MCP4728 I2C 12-bit quad digital-to-analog
> + * converter (DAC)
> + * (7-bit I2C slave address 0x60, the three LSBs can be configured in
> + * hardware)
> + */
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/err.h>
> +#include <linux/delay.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#include <linux/iio/dac/mcp4728.h>
> +
> +#define MCP4728_DRV_NAME "mcp4728"
> +#define MCP4728_NUM_CHANNELS 4
> +#define MCP4728_INT_VREF_MV 2048
> +#define MCP4728_RESOLUTION 12
> +
> +struct mcp4728_scale {
> +	unsigned int		integer;
> +	unsigned int		micro;

use just a single space

> +};
> +
> +struct mcp4728_channel_state {
> +	u16 dac_value;
> +	bool powerdown;
> +	u8 gain;
> +	u8 vref;

could this be bool?

> +	unsigned powerdown_mode;
> +};
> +
> +struct mcp4728_data {
> +	struct i2c_client *client;
> +	struct mcp4728_channel_state channel_state[MCP4728_NUM_CHANNELS];
> +	struct mcp4728_scale	scale_avail[MCP4728_NUM_CHANNELS][2];
> +
> +};
> +
> +static int mcp4728_write_reg(struct mcp4728_data *data, int ch, bool eeprom)
> +{
> +	struct mcp4728_channel_state *state = &data->channel_state[ch];
> +	u8 outbuf[3];
> +	u8 pd;
> +	int ret;
> +
> +	if (state->dac_value >= (1 << MCP4728_RESOLUTION) ||
> +				state->dac_value < 0)
> +		return -EINVAL;
> +/*
> +  Write input register. EEPROM is not updated:
> +  c2=0, c1=1, c0=0, w1=0, w0=0 : 0x40
> +
> +  Write input and EEPROM registers:
> +  c2=0, c1=1, c0=0, w1=1, w0=1 : 0x58
> +
> +  b[0]: c2 | c1 | c0 | w1 | w0 | dac1 | dac0 | /udac
> +  b[1]: vref | pd1 | pd0 | gain | d11 | d10 | d9 | d8
> +  b[2]: d7-d0
> +*/

maybe indent the block above

> +	pd = state->powerdown ? (state->powerdown_mode + 1) : 0;
> +
> +	outbuf[0] = (eeprom ? 0x58 : 0x40) |
> +				((ch & 0x03) << 1);
> +	outbuf[1] = ((state->vref & 0x01) << 7) |
> +				((pd & 0x3) << 5)    |
> +				((state->gain & 0x01) << 4) |
> +				((state->dac_value >> 8) & 0x0f);
> +	outbuf[2] = state->dac_value & 0xff;
> +
> +	ret = i2c_master_send(data->client, outbuf, 3);

3 could be replaced by sizeof(outbuf), here and below

> +	if (ret < 0)
> +		return ret;
> +	else if (ret != 3)
> +		return -EIO;
> +	else
> +		return 0;
> +}
> +
> +static int mcp4728_write_input_reg(struct mcp4728_data *data, int ch)
> +{
> +	return mcp4728_write_reg(data, ch, false);
> +}
> +
> +static int mcp4728_suspend(struct device *dev)
> +{
> +	struct mcp4728_data *data = iio_priv(i2c_get_clientdata(
> +		to_i2c_client(dev)));
> +	int ch, ret;
> +
> +	for (ch = 0; ch < MCP4728_NUM_CHANNELS; ++ch) {
> +		data->channel_state[ch].powerdown = true;
> +		ret = mcp4728_write_input_reg(data, ch);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return ret;
> +}
> +

_suspend()/_resume() code is almost identical, split out common code?

> +static int mcp4728_resume(struct device *dev)
> +{
> +	struct mcp4728_data *data = iio_priv(i2c_get_clientdata(
> +		to_i2c_client(dev)));
> +	int ch, ret;
> +
> +	for (ch = 0; ch < MCP4728_NUM_CHANNELS; ++ch) {
> +		data->channel_state[ch].powerdown = false;
> +		ret = mcp4728_write_input_reg(data, ch);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static SIMPLE_DEV_PM_OPS(mcp4728_pm_ops, mcp4728_suspend, mcp4728_resume);
> +#define MCP4728_PM_OPS (&mcp4728_pm_ops)
> +#else
> +#define MCP4728_PM_OPS NULL
> +#endif
> +
> +static ssize_t mcp4728_store_eeprom(struct device *dev,
> +	struct device_attribute *attr, const char *buf, size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +	int tries;
> +	u8 inbuf[3];

actually just one byte is used?

> +	bool state;
> +	int ret, ch;
> +
> +	ret = strtobool(buf, &state);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (!state)
> +		return 0;
> +
> +	for (ch = 0; ch < MCP4728_NUM_CHANNELS; ++ch) {
> +		tries = 20;

make the variable 'tries' local to the loop?
maybe 'inbuf' as well

> +		mcp4728_write_reg(data, ch, true);
> +
> +
> +		/* wait for write complete, takes up to 50ms */
> +		while (tries--) {
> +			msleep(20);
> +			ret = i2c_master_recv(data->client, inbuf, 1);
> +			if (ret < 0)
> +				return ret;
> +			else if (ret != 1)
> +				return -EIO;
> +
> +			if (inbuf[0] & 0x80)
> +				break;
> +		}
> +
> +		if (tries < 0) {
> +			dev_err(&data->client->dev,
> +				"mcp4728_store_eeprom() failed, incomplete\n");
> +			return -EIO;
> +		}
> +	}
> +
> +	return len;
> +}
> +
> +static IIO_DEVICE_ATTR(store_eeprom, S_IWUSR, NULL, mcp4728_store_eeprom, 0);
> +
> +
> +static ssize_t mcp4728_show_scale_available(struct device *dev,
> +		struct device_attribute *attr,
> +		char *buf)
> +{
> +	struct iio_dev_attr *iio_attr = to_iio_dev_attr(attr);
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +	u8 ch = iio_attr->address;
> +	int i, len = 0;
> +
> +	for (i = 0; i < ARRAY_SIZE(data->scale_avail[ch]); i++)
> +		len += sprintf(buf + len, "%u.%06u ",
> +			       data->scale_avail[ch][i].integer,
> +			       data->scale_avail[ch][i].micro);
> +
> +	len += sprintf(buf + len, "\n");
> +
> +	return len;
> +}
> +
> +#define SHOW_SCALE_AVAILABLE_ATTR(ch)					\
> +static IIO_DEVICE_ATTR(out_voltage##ch##_scale_available, S_IRUGO,	\
> +		       mcp4728_show_scale_available, NULL, ch)
> +
> +SHOW_SCALE_AVAILABLE_ATTR(0);
> +SHOW_SCALE_AVAILABLE_ATTR(1);
> +SHOW_SCALE_AVAILABLE_ATTR(2);
> +SHOW_SCALE_AVAILABLE_ATTR(3);
> +
> +static struct attribute *mcp4728_attributes[] = {
> +	&iio_dev_attr_store_eeprom.dev_attr.attr,
> +	&iio_dev_attr_out_voltage0_scale_available.dev_attr.attr,
> +	&iio_dev_attr_out_voltage1_scale_available.dev_attr.attr,
> +	&iio_dev_attr_out_voltage2_scale_available.dev_attr.attr,
> +	&iio_dev_attr_out_voltage3_scale_available.dev_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group mcp4728_attribute_group = {
> +	.attrs = mcp4728_attributes,
> +};
> +
> +static const char * const mcp4728_powerdown_modes[] = {
> +	"1kohm_to_gnd",
> +	"100kohm_to_gnd",
> +	"500kohm_to_gnd"
> +};
> +
> +static int mcp4728_get_powerdown_mode(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan)
> +{
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +
> +	return data->channel_state[chan->channel].powerdown_mode;
> +}
> +
> +static int mcp4728_set_powerdown_mode(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan, unsigned mode)
> +{
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +
> +	data->channel_state[chan->channel].powerdown_mode = mode;
> +
> +	return 0;
> +}
> +
> +static ssize_t mcp4728_read_powerdown(struct iio_dev *indio_dev,
> +	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
> +{
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +
> +	return sprintf(buf, "%d\n",
> +		data->channel_state[chan->channel].powerdown);
> +}
> +
> +static ssize_t mcp4728_write_powerdown(struct iio_dev *indio_dev,
> +	 uintptr_t private, const struct iio_chan_spec *chan,
> +	 const char *buf, size_t len)
> +{
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +	struct mcp4728_channel_state *state =
> +			&data->channel_state[chan->channel];
> +	bool powerdown;
> +	int ret;
> +
> +	ret = strtobool(buf, &powerdown);
> +	if (ret)
> +		return ret;
> +
> +	state->powerdown = powerdown;
> +	ret = mcp4728_write_input_reg(data, chan->channel);
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	return len;
> +}
> +
> +static const struct iio_enum mcp4728_powerdown_mode_enum = {
> +	.items = mcp4728_powerdown_modes,
> +	.num_items = ARRAY_SIZE(mcp4728_powerdown_modes),
> +	.get = mcp4728_get_powerdown_mode,
> +	.set = mcp4728_set_powerdown_mode,
> +};
> +
> +static const struct iio_chan_spec_ext_info mcp4728_ext_info[] = {
> +	{
> +		.name = "powerdown",
> +		.read = mcp4728_read_powerdown,
> +		.write = mcp4728_write_powerdown,
> +		.shared = IIO_SEPARATE,
> +	},
> +	IIO_ENUM("powerdown_mode", IIO_SEPARATE, &mcp4728_powerdown_mode_enum),
> +	IIO_ENUM_AVAILABLE("powerdown_mode", &mcp4728_powerdown_mode_enum),
> +	{ },
> +};
> +
> +#define MCP4728_CHAN(chan) {			\
> +	.type		= IIO_VOLTAGE, \
> +	.indexed	= 1, \
> +	.output		= 1, \
> +	.channel	= chan, \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> +			      BIT(IIO_CHAN_INFO_SCALE), \
> +	.ext_info	= mcp4728_ext_info, \
> +}
> +
> +
> +static const struct iio_chan_spec mcp4728_channels[MCP4728_NUM_CHANNELS] = {
> +	MCP4728_CHAN(0),
> +	MCP4728_CHAN(1),
> +	MCP4728_CHAN(2),
> +	MCP4728_CHAN(3),
> +};
> +
> +static int mcp4728_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long mask)
> +{
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +	struct mcp4728_channel_state *state =
> +			&data->channel_state[chan->channel];
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		*val = data->channel_state[chan->channel].dac_value;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = (state->gain + 1) * MCP4728_INT_VREF_MV;
> +		*val2 = MCP4728_RESOLUTION;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	}
> +	return -EINVAL;
> +}
> +
> +static int mcp4728_write_raw(struct iio_dev *indio_dev,
> +			       struct iio_chan_spec const *chan,
> +			       int val, int val2, long mask)
> +{
> +	struct mcp4728_data *data = iio_priv(indio_dev);
> +	struct mcp4728_channel_state *state =
> +			&data->channel_state[chan->channel];
> +	struct mcp4728_scale *scale_avail =
> +		data->scale_avail[chan->channel];
> +
> +	int ret, gain;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		state->dac_value = val;
> +		ret = mcp4728_write_input_reg(data, chan->channel);
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		ret = -EINVAL;
> +
> +		for (gain = 0; gain < 2; ++gain) {
> +			if (val == scale_avail[gain].integer &&
> +			val2 == scale_avail[gain].micro) {
> +
> +				state->gain = gain;
> +				ret = mcp4728_write_input_reg(data,
> +					chan->channel);
> +			}
> +		}
> +
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct iio_info mcp4728_info = {
> +	.read_raw = mcp4728_read_raw,
> +	.write_raw = mcp4728_write_raw,
> +	.attrs = &mcp4728_attribute_group,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static int mcp4728_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct mcp4728_data *data;
> +	struct iio_dev *indio_dev;
> +	u8 inbuf[24];
> +	u8 *reg;
> +	u8 pd;
> +	int err;
> +	int i, gain;
> +	u64 scale_uv;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (indio_dev == NULL)
> +		return -ENOMEM;
> +	data = iio_priv(indio_dev);
> +	i2c_set_clientdata(client, indio_dev);
> +	data->client = client;
> +
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->info = &mcp4728_info;

indio_dev->name ?

> +	indio_dev->channels = mcp4728_channels;
> +	indio_dev->num_channels = MCP4728_NUM_CHANNELS;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	/* read current DAC value */
> +	err = i2c_master_recv(client, inbuf, 24);
> +	if (err < 0) {
> +		dev_err(&client->dev, "failed to read DAC value");

missing \n

> +		return err;
> +	}
> +
> +	for (i = 0; i < MCP4728_NUM_CHANNELS; ++i) {
> +		reg = &inbuf[6*i];
> +		pd = (reg[1] >> 5) & 0x3;
> +		data->channel_state[i].powerdown = pd > 0 ? true : false;
> +
> +		/* set to 500kohm_to_gnd if channel is now powered down */
> +		data->channel_state[i].powerdown_mode = pd ? pd-1 : 2;
> +		data->channel_state[i].dac_value = ((reg[1] & 0x0f) << 8) |
> +						   (reg[2]);
> +		data->channel_state[i].gain = (reg[1] >> 4) & 0x01;
> +
> +		/* Use internal reference */
> +		data->channel_state[i].vref = 1;
> +
> +		/* Populate available ADC input ranges */
> +		for (gain = 0; gain < 2; gain++) {
> +			/*
> +			  [gain=0] = Gain Setting 1, vref x 1
> +			  [gain=1] = Gain Setting 2, vref x 2
> +
> +			  The scale is calculated by doing:
> +			    Vref >> (realbits - gain)
> +			  which multiplies by two on the second component
> +			  of the array.
> +			 */
> +			scale_uv = ((u64) MCP4728_INT_VREF_MV * 100000) >>
> +				   (MCP4728_RESOLUTION - gain);
> +			data->scale_avail[i][gain].micro =
> +					do_div(scale_uv, 100000) * 10;
> +			data->scale_avail[i][gain].integer = scale_uv;
> +		}
> +	}
> +
> +	return iio_device_register(indio_dev);
> +}
> +
> +static int mcp4728_remove(struct i2c_client *client)
> +{
> +	iio_device_unregister(i2c_get_clientdata(client));
> +	return 0;
> +}
> +
> +static const struct i2c_device_id mcp4728_id[] = {
> +	{ "mcp4728", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, mcp4728_id);
> +
> +static struct i2c_driver mcp4728_driver = {
> +	.driver = {
> +		.name	= MCP4728_DRV_NAME,
> +		.pm	= MCP4728_PM_OPS,
> +	},
> +	.probe		= mcp4728_probe,
> +	.remove		= mcp4728_remove,
> +	.id_table	= mcp4728_id,
> +};
> +module_i2c_driver(mcp4728_driver);
> +
> +MODULE_AUTHOR("Evgeny Boger <boger@contactless.ru>");
> +MODULE_DESCRIPTION("MCP4728 quad 12-bit DAC");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/iio/dac/mcp4728.h b/include/linux/iio/dac/mcp4728.h
> new file mode 100644
> index 0000000..14adb0c
> --- /dev/null
> +++ b/include/linux/iio/dac/mcp4728.h
> @@ -0,0 +1,16 @@
> +/*
> + * MCP4728 DAC driver
> + *
> + * Copyright (C) 2016 Evgeny Boger <boger@contactless.ru>
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#ifndef IIO_DAC_MCP4728_H_
> +#define IIO_DAC_MCP4728_H_
> +
> +struct mcp4728_platform_data {
> +	u16 vref_mv;
> +};
> +
> +#endif /* IIO_DAC_MCP4728_H_ */
> 

-- 

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

Re: [PATCH] iio: dac: mcp4728: Add support for MCP4728

[Evgeny Boger]

Thank you for your comments! See below

On 03/16/2016 02:02 AM, Peter Meerwald-Stadler wrote:
>> Microchip MCP4728 is a 12-bit quad channel
>> digital-to-analog converter (DAC) with I2C interface.
>>
>> This patch adds support for most of the features of MCP4728,
>> including per-channel gain, power state and power down mode control.
>> Driver also supports saving the current state to on-chip EEPROM.
> since Jonathan seems to be OK with a separate driver, here are my review
> comments...
>   
>> Signed-off-by: Evgeny Boger <boger@contactless.ru>
>> ---
>>   drivers/iio/dac/Kconfig         |  11 +
>>   drivers/iio/dac/Makefile        |   1 +
>>   drivers/iio/dac/mcp4728.c       | 479 ++++++++++++++++++++++++++++++++++++++++
>>   include/linux/iio/dac/mcp4728.h |  16 ++
>>   4 files changed, 507 insertions(+)
>>   create mode 100644 drivers/iio/dac/mcp4728.c
>>   create mode 100644 include/linux/iio/dac/mcp4728.h
>>
>> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
>> index e701e28..8da6039 100644
>> --- a/drivers/iio/dac/Kconfig
>> +++ b/drivers/iio/dac/Kconfig
>> @@ -186,6 +186,17 @@ config MCP4725
>>   	  To compile this driver as a module, choose M here: the module
>>   	  will be called mcp4725.
>>   
>> +config MCP4728
>> +	tristate "MCP4728 DAC driver"
>> +	depends on I2C
>> +	---help---
>> +	  Say Y here if you want to build a driver for the Microchip
>> +	  MCP 4728 12-bit quad digital-to-analog converter (DAC) with I2C
>> +	  interface.
>> +
>> +	  To compile this driver as a module, choose M here: the module
>> +	  will be called mcp4728.
>> +
>>   config MCP4922
>>   	tristate "MCP4902, MCP4912, MCP4922 DAC driver"
>>   	depends on SPI
>> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
>> index 63ae056..2c9decc 100644
>> --- a/drivers/iio/dac/Makefile
>> +++ b/drivers/iio/dac/Makefile
>> @@ -20,4 +20,5 @@ obj-$(CONFIG_M62332) += m62332.o
>>   obj-$(CONFIG_MAX517) += max517.o
>>   obj-$(CONFIG_MAX5821) += max5821.o
>>   obj-$(CONFIG_MCP4725) += mcp4725.o
>> +obj-$(CONFIG_MCP4728) += mcp4728.o
>>   obj-$(CONFIG_MCP4922) += mcp4922.o
>> diff --git a/drivers/iio/dac/mcp4728.c b/drivers/iio/dac/mcp4728.c
>> new file mode 100644
>> index 0000000..6dbbfac
>> --- /dev/null
>> +++ b/drivers/iio/dac/mcp4728.c
>> @@ -0,0 +1,479 @@
>> +/*
>> + * mcp4728.c - Support for Microchip MCP4728
>> + *
>> + * Copyright (C) 2016 Evgeny Boger <boger@contactless.ru>
>> + *
>> + * Based on mcp4728 by Peter Meerwald <pmeerw@pmeerw.net>
>> + *
>> + * This file is subject to the terms and conditions of version 2 of
>> + * the GNU General Public License.  See the file COPYING in the main
>> + * directory of this archive for more details.
>> + *
>> + * driver for the Microchip MCP4728 I2C 12-bit quad digital-to-analog
>> + * converter (DAC)
>> + * (7-bit I2C slave address 0x60, the three LSBs can be configured in
>> + * hardware)
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/i2c.h>
>> +#include <linux/err.h>
>> +#include <linux/delay.h>
>> +
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/sysfs.h>
>> +
>> +#include <linux/iio/dac/mcp4728.h>
>> +
>> +#define MCP4728_DRV_NAME "mcp4728"
>> +#define MCP4728_NUM_CHANNELS 4
>> +#define MCP4728_INT_VREF_MV 2048
>> +#define MCP4728_RESOLUTION 12
>> +
>> +struct mcp4728_scale {
>> +	unsigned int		integer;
>> +	unsigned int		micro;
> use just a single space
>
>> +};
>> +
>> +struct mcp4728_channel_state {
>> +	u16 dac_value;
>> +	bool powerdown;
>> +	u8 gain;
>> +	u8 vref;
> could this be bool?
u8 vref => bool int_vref
to clarify it doesn't store the value of reference voltage
>
>> +	unsigned powerdown_mode;
>> +};
>> +
>> +struct mcp4728_data {
>> +	struct i2c_client *client;
>> +	struct mcp4728_channel_state channel_state[MCP4728_NUM_CHANNELS];
>> +	struct mcp4728_scale	scale_avail[MCP4728_NUM_CHANNELS][2];
>> +
>> +};
>> +
>> +static int mcp4728_write_reg(struct mcp4728_data *data, int ch, bool eeprom)
>> +{
>> +	struct mcp4728_channel_state *state = &data->channel_state[ch];
>> +	u8 outbuf[3];
>> +	u8 pd;
>> +	int ret;
>> +
>> +	if (state->dac_value >= (1 << MCP4728_RESOLUTION) ||
>> +				state->dac_value < 0)
>> +		return -EINVAL;
>> +/*
>> +  Write input register. EEPROM is not updated:
>> +  c2=0, c1=1, c0=0, w1=0, w0=0 : 0x40
>> +
>> +  Write input and EEPROM registers:
>> +  c2=0, c1=1, c0=0, w1=1, w0=1 : 0x58
>> +
>> +  b[0]: c2 | c1 | c0 | w1 | w0 | dac1 | dac0 | /udac
>> +  b[1]: vref | pd1 | pd0 | gain | d11 | d10 | d9 | d8
>> +  b[2]: d7-d0
>> +*/
> maybe indent the block above
>
>> +	pd = state->powerdown ? (state->powerdown_mode + 1) : 0;
>> +
>> +	outbuf[0] = (eeprom ? 0x58 : 0x40) |
>> +				((ch & 0x03) << 1);
>> +	outbuf[1] = ((state->vref & 0x01) << 7) |
>> +				((pd & 0x3) << 5)    |
>> +				((state->gain & 0x01) << 4) |
>> +				((state->dac_value >> 8) & 0x0f);
>> +	outbuf[2] = state->dac_value & 0xff;
>> +
>> +	ret = i2c_master_send(data->client, outbuf, 3);
> 3 could be replaced by sizeof(outbuf), here and below
>
>> +	if (ret < 0)
>> +		return ret;
>> +	else if (ret != 3)
>> +		return -EIO;
>> +	else
>> +		return 0;
>> +}
>> +
>> +static int mcp4728_write_input_reg(struct mcp4728_data *data, int ch)
>> +{
>> +	return mcp4728_write_reg(data, ch, false);
>> +}
>> +
>> +static int mcp4728_suspend(struct device *dev)
>> +{
>> +	struct mcp4728_data *data = iio_priv(i2c_get_clientdata(
>> +		to_i2c_client(dev)));
>> +	int ch, ret;
>> +
>> +	for (ch = 0; ch < MCP4728_NUM_CHANNELS; ++ch) {
>> +		data->channel_state[ch].powerdown = true;
>> +		ret = mcp4728_write_input_reg(data, ch);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
> _suspend()/_resume() code is almost identical, split out common code?

On a second thought: it seems like the present  _suspend/_resume logic 
is somewhat flawed.
Suppose user has manually switched off some of the channels via sysfs. 
In this case, one would expect the driver to restore the configured 
state after suspend/resume cycle, i.e. keep some of the channels in 
powerdown state.
So if there are no objections, I'm going to rewrite this part to store 
the current power state for each channel on suspend and restore it on 
resume.


>
>> +static int mcp4728_resume(struct device *dev)
>> +{
>> +	struct mcp4728_data *data = iio_priv(i2c_get_clientdata(
>> +		to_i2c_client(dev)));
>> +	int ch, ret;
>> +
>> +	for (ch = 0; ch < MCP4728_NUM_CHANNELS; ++ch) {
>> +		data->channel_state[ch].powerdown = false;
>> +		ret = mcp4728_write_input_reg(data, ch);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static SIMPLE_DEV_PM_OPS(mcp4728_pm_ops, mcp4728_suspend, mcp4728_resume);
>> +#define MCP4728_PM_OPS (&mcp4728_pm_ops)
>> +#else
>> +#define MCP4728_PM_OPS NULL
>> +#endif
>> +
>> +static ssize_t mcp4728_store_eeprom(struct device *dev,
>> +	struct device_attribute *attr, const char *buf, size_t len)
>> +{
>> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +	int tries;
>> +	u8 inbuf[3];
> actually just one byte is used?
>
>> +	bool state;
>> +	int ret, ch;
>> +
>> +	ret = strtobool(buf, &state);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	if (!state)
>> +		return 0;
>> +
>> +	for (ch = 0; ch < MCP4728_NUM_CHANNELS; ++ch) {
>> +		tries = 20;
> make the variable 'tries' local to the loop?
> maybe 'inbuf' as well
>
>> +		mcp4728_write_reg(data, ch, true);
>> +
>> +
>> +		/* wait for write complete, takes up to 50ms */
>> +		while (tries--) {
>> +			msleep(20);
>> +			ret = i2c_master_recv(data->client, inbuf, 1);
>> +			if (ret < 0)
>> +				return ret;
>> +			else if (ret != 1)
>> +				return -EIO;
>> +
>> +			if (inbuf[0] & 0x80)
>> +				break;
>> +		}
>> +
>> +		if (tries < 0) {
>> +			dev_err(&data->client->dev,
>> +				"mcp4728_store_eeprom() failed, incomplete\n");
>> +			return -EIO;
>> +		}
>> +	}
>> +
>> +	return len;
>> +}
>> +
>> +static IIO_DEVICE_ATTR(store_eeprom, S_IWUSR, NULL, mcp4728_store_eeprom, 0);
>> +
>> +
>> +static ssize_t mcp4728_show_scale_available(struct device *dev,
>> +		struct device_attribute *attr,
>> +		char *buf)
>> +{
>> +	struct iio_dev_attr *iio_attr = to_iio_dev_attr(attr);
>> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +	u8 ch = iio_attr->address;
>> +	int i, len = 0;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(data->scale_avail[ch]); i++)
>> +		len += sprintf(buf + len, "%u.%06u ",
>> +			       data->scale_avail[ch][i].integer,
>> +			       data->scale_avail[ch][i].micro);
>> +
>> +	len += sprintf(buf + len, "\n");
>> +
>> +	return len;
>> +}
>> +
>> +#define SHOW_SCALE_AVAILABLE_ATTR(ch)					\
>> +static IIO_DEVICE_ATTR(out_voltage##ch##_scale_available, S_IRUGO,	\
>> +		       mcp4728_show_scale_available, NULL, ch)
>> +
>> +SHOW_SCALE_AVAILABLE_ATTR(0);
>> +SHOW_SCALE_AVAILABLE_ATTR(1);
>> +SHOW_SCALE_AVAILABLE_ATTR(2);
>> +SHOW_SCALE_AVAILABLE_ATTR(3);
>> +
>> +static struct attribute *mcp4728_attributes[] = {
>> +	&iio_dev_attr_store_eeprom.dev_attr.attr,
>> +	&iio_dev_attr_out_voltage0_scale_available.dev_attr.attr,
>> +	&iio_dev_attr_out_voltage1_scale_available.dev_attr.attr,
>> +	&iio_dev_attr_out_voltage2_scale_available.dev_attr.attr,
>> +	&iio_dev_attr_out_voltage3_scale_available.dev_attr.attr,
>> +	NULL,
>> +};
>> +
>> +static const struct attribute_group mcp4728_attribute_group = {
>> +	.attrs = mcp4728_attributes,
>> +};
>> +
>> +static const char * const mcp4728_powerdown_modes[] = {
>> +	"1kohm_to_gnd",
>> +	"100kohm_to_gnd",
>> +	"500kohm_to_gnd"
>> +};
>> +
>> +static int mcp4728_get_powerdown_mode(struct iio_dev *indio_dev,
>> +	const struct iio_chan_spec *chan)
>> +{
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +
>> +	return data->channel_state[chan->channel].powerdown_mode;
>> +}
>> +
>> +static int mcp4728_set_powerdown_mode(struct iio_dev *indio_dev,
>> +	const struct iio_chan_spec *chan, unsigned mode)
>> +{
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +
>> +	data->channel_state[chan->channel].powerdown_mode = mode;
>> +
>> +	return 0;
>> +}
>> +
>> +static ssize_t mcp4728_read_powerdown(struct iio_dev *indio_dev,
>> +	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
>> +{
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +
>> +	return sprintf(buf, "%d\n",
>> +		data->channel_state[chan->channel].powerdown);
>> +}
>> +
>> +static ssize_t mcp4728_write_powerdown(struct iio_dev *indio_dev,
>> +	 uintptr_t private, const struct iio_chan_spec *chan,
>> +	 const char *buf, size_t len)
>> +{
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +	struct mcp4728_channel_state *state =
>> +			&data->channel_state[chan->channel];
>> +	bool powerdown;
>> +	int ret;
>> +
>> +	ret = strtobool(buf, &powerdown);
>> +	if (ret)
>> +		return ret;
>> +
>> +	state->powerdown = powerdown;
>> +	ret = mcp4728_write_input_reg(data, chan->channel);
>> +
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return len;
>> +}
>> +
>> +static const struct iio_enum mcp4728_powerdown_mode_enum = {
>> +	.items = mcp4728_powerdown_modes,
>> +	.num_items = ARRAY_SIZE(mcp4728_powerdown_modes),
>> +	.get = mcp4728_get_powerdown_mode,
>> +	.set = mcp4728_set_powerdown_mode,
>> +};
>> +
>> +static const struct iio_chan_spec_ext_info mcp4728_ext_info[] = {
>> +	{
>> +		.name = "powerdown",
>> +		.read = mcp4728_read_powerdown,
>> +		.write = mcp4728_write_powerdown,
>> +		.shared = IIO_SEPARATE,
>> +	},
>> +	IIO_ENUM("powerdown_mode", IIO_SEPARATE, &mcp4728_powerdown_mode_enum),
>> +	IIO_ENUM_AVAILABLE("powerdown_mode", &mcp4728_powerdown_mode_enum),
>> +	{ },
>> +};
>> +
>> +#define MCP4728_CHAN(chan) {			\
>> +	.type		= IIO_VOLTAGE, \
>> +	.indexed	= 1, \
>> +	.output		= 1, \
>> +	.channel	= chan, \
>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
>> +			      BIT(IIO_CHAN_INFO_SCALE), \
>> +	.ext_info	= mcp4728_ext_info, \
>> +}
>> +
>> +
>> +static const struct iio_chan_spec mcp4728_channels[MCP4728_NUM_CHANNELS] = {
>> +	MCP4728_CHAN(0),
>> +	MCP4728_CHAN(1),
>> +	MCP4728_CHAN(2),
>> +	MCP4728_CHAN(3),
>> +};
>> +
>> +static int mcp4728_read_raw(struct iio_dev *indio_dev,
>> +			   struct iio_chan_spec const *chan,
>> +			   int *val, int *val2, long mask)
>> +{
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +	struct mcp4728_channel_state *state =
>> +			&data->channel_state[chan->channel];
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_RAW:
>> +		*val = data->channel_state[chan->channel].dac_value;
>> +		return IIO_VAL_INT;
>> +	case IIO_CHAN_INFO_SCALE:
>> +		*val = (state->gain + 1) * MCP4728_INT_VREF_MV;
>> +		*val2 = MCP4728_RESOLUTION;
>> +		return IIO_VAL_FRACTIONAL_LOG2;
>> +	}
>> +	return -EINVAL;
>> +}
>> +
>> +static int mcp4728_write_raw(struct iio_dev *indio_dev,
>> +			       struct iio_chan_spec const *chan,
>> +			       int val, int val2, long mask)
>> +{
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +	struct mcp4728_channel_state *state =
>> +			&data->channel_state[chan->channel];
>> +	struct mcp4728_scale *scale_avail =
>> +		data->scale_avail[chan->channel];
>> +
>> +	int ret, gain;
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_RAW:
>> +		state->dac_value = val;
>> +		ret = mcp4728_write_input_reg(data, chan->channel);
>> +		break;
>> +	case IIO_CHAN_INFO_SCALE:
>> +		ret = -EINVAL;
>> +
>> +		for (gain = 0; gain < 2; ++gain) {
>> +			if (val == scale_avail[gain].integer &&
>> +			val2 == scale_avail[gain].micro) {
>> +
>> +				state->gain = gain;
>> +				ret = mcp4728_write_input_reg(data,
>> +					chan->channel);
>> +			}
>> +		}
>> +
>> +		break;
>> +	default:
>> +		ret = -EINVAL;
>> +		break;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct iio_info mcp4728_info = {
>> +	.read_raw = mcp4728_read_raw,
>> +	.write_raw = mcp4728_write_raw,
>> +	.attrs = &mcp4728_attribute_group,
>> +	.driver_module = THIS_MODULE,
>> +};
>> +
>> +static int mcp4728_probe(struct i2c_client *client,
>> +			 const struct i2c_device_id *id)
>> +{
>> +	struct mcp4728_data *data;
>> +	struct iio_dev *indio_dev;
>> +	u8 inbuf[24];
>> +	u8 *reg;
>> +	u8 pd;
>> +	int err;
>> +	int i, gain;
>> +	u64 scale_uv;
>> +
>> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>> +	if (indio_dev == NULL)
>> +		return -ENOMEM;
>> +	data = iio_priv(indio_dev);
>> +	i2c_set_clientdata(client, indio_dev);
>> +	data->client = client;
>> +
>> +	indio_dev->dev.parent = &client->dev;
>> +	indio_dev->info = &mcp4728_info;
> indio_dev->name ?
>
>> +	indio_dev->channels = mcp4728_channels;
>> +	indio_dev->num_channels = MCP4728_NUM_CHANNELS;
>> +	indio_dev->modes = INDIO_DIRECT_MODE;
>> +
>> +	/* read current DAC value */
>> +	err = i2c_master_recv(client, inbuf, 24);
>> +	if (err < 0) {
>> +		dev_err(&client->dev, "failed to read DAC value");
> missing \n
>
>> +		return err;
>> +	}
>> +
>> +	for (i = 0; i < MCP4728_NUM_CHANNELS; ++i) {
>> +		reg = &inbuf[6*i];
>> +		pd = (reg[1] >> 5) & 0x3;
>> +		data->channel_state[i].powerdown = pd > 0 ? true : false;
>> +
>> +		/* set to 500kohm_to_gnd if channel is now powered down */
>> +		data->channel_state[i].powerdown_mode = pd ? pd-1 : 2;
>> +		data->channel_state[i].dac_value = ((reg[1] & 0x0f) << 8) |
>> +						   (reg[2]);
>> +		data->channel_state[i].gain = (reg[1] >> 4) & 0x01;
>> +
>> +		/* Use internal reference */
>> +		data->channel_state[i].vref = 1;
>> +
>> +		/* Populate available ADC input ranges */
>> +		for (gain = 0; gain < 2; gain++) {
>> +			/*
>> +			  [gain=0] = Gain Setting 1, vref x 1
>> +			  [gain=1] = Gain Setting 2, vref x 2
>> +
>> +			  The scale is calculated by doing:
>> +			    Vref >> (realbits - gain)
>> +			  which multiplies by two on the second component
>> +			  of the array.
>> +			 */
>> +			scale_uv = ((u64) MCP4728_INT_VREF_MV * 100000) >>
>> +				   (MCP4728_RESOLUTION - gain);
>> +			data->scale_avail[i][gain].micro =
>> +					do_div(scale_uv, 100000) * 10;
>> +			data->scale_avail[i][gain].integer = scale_uv;
>> +		}
>> +	}
>> +
>> +	return iio_device_register(indio_dev);
>> +}
>> +
>> +static int mcp4728_remove(struct i2c_client *client)
>> +{
>> +	iio_device_unregister(i2c_get_clientdata(client));
>> +	return 0;
>> +}
>> +
>> +static const struct i2c_device_id mcp4728_id[] = {
>> +	{ "mcp4728", 0 },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(i2c, mcp4728_id);
>> +
>> +static struct i2c_driver mcp4728_driver = {
>> +	.driver = {
>> +		.name	= MCP4728_DRV_NAME,
>> +		.pm	= MCP4728_PM_OPS,
>> +	},
>> +	.probe		= mcp4728_probe,
>> +	.remove		= mcp4728_remove,
>> +	.id_table	= mcp4728_id,
>> +};
>> +module_i2c_driver(mcp4728_driver);
>> +
>> +MODULE_AUTHOR("Evgeny Boger <boger@contactless.ru>");
>> +MODULE_DESCRIPTION("MCP4728 quad 12-bit DAC");
>> +MODULE_LICENSE("GPL");
>> diff --git a/include/linux/iio/dac/mcp4728.h b/include/linux/iio/dac/mcp4728.h
>> new file mode 100644
>> index 0000000..14adb0c
>> --- /dev/null
>> +++ b/include/linux/iio/dac/mcp4728.h
>> @@ -0,0 +1,16 @@
>> +/*
>> + * MCP4728 DAC driver
>> + *
>> + * Copyright (C) 2016 Evgeny Boger <boger@contactless.ru>
>> + *
>> + * Licensed under the GPL-2 or later.
>> + */
>> +
>> +#ifndef IIO_DAC_MCP4728_H_
>> +#define IIO_DAC_MCP4728_H_
>> +
>> +struct mcp4728_platform_data {
>> +	u16 vref_mv;
>> +};
>> +
>> +#endif /* IIO_DAC_MCP4728_H_ */
>>


-- 
С уважением,
     Евгений Богер / Evgeny Boger
     Wiren Board Team
     ООО Бесконтактные устройства
     http://contactless.ru
     +7 919 965 88 36

Re: [PATCH] iio: dac: mcp4728: Add support for MCP4728

[Jonathan Cameron]

On 15 March 2016 23:02:34 GMT+00:00, Peter Meerwald-Stadler <pmeerw@pmeerw.net> wrote:
>
>> Microchip MCP4728 is a 12-bit quad channel
>> digital-to-analog converter (DAC) with I2C interface.
>> 
>> This patch adds support for most of the features of MCP4728,
>> including per-channel gain, power state and power down mode control.
>> Driver also supports saving the current state to on-chip EEPROM.
>
>since Jonathan seems to be OK with a separate driver, here are my
>review 
>comments...
Was marginal whether a combined driver made sense (lots of minor differences).

Irritating that the hardware wasn't more consistent!
> 
>> Signed-off-by: Evgeny Boger <boger@contactless.ru>
>> ---
>>  drivers/iio/dac/Kconfig         |  11 +
>>  drivers/iio/dac/Makefile        |   1 +
>>  drivers/iio/dac/mcp4728.c       | 479
>++++++++++++++++++++++++++++++++++++++++
>>  include/linux/iio/dac/mcp4728.h |  16 ++
>>  4 files changed, 507 insertions(+)
>>  create mode 100644 drivers/iio/dac/mcp4728.c
>>  create mode 100644 include/linux/iio/dac/mcp4728.h
>> 
>> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
>> index e701e28..8da6039 100644
>> --- a/drivers/iio/dac/Kconfig
>> +++ b/drivers/iio/dac/Kconfig
>> @@ -186,6 +186,17 @@ config MCP4725
>>  	  To compile this driver as a module, choose M here: the module
>>  	  will be called mcp4725.
>>  
>> +config MCP4728
>> +	tristate "MCP4728 DAC driver"
>> +	depends on I2C
>> +	---help---
>> +	  Say Y here if you want to build a driver for the Microchip
>> +	  MCP 4728 12-bit quad digital-to-analog converter (DAC) with I2C
>> +	  interface.
>> +
>> +	  To compile this driver as a module, choose M here: the module
>> +	  will be called mcp4728.
>> +
>>  config MCP4922
>>  	tristate "MCP4902, MCP4912, MCP4922 DAC driver"
>>  	depends on SPI
>> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
>> index 63ae056..2c9decc 100644
>> --- a/drivers/iio/dac/Makefile
>> +++ b/drivers/iio/dac/Makefile
>> @@ -20,4 +20,5 @@ obj-$(CONFIG_M62332) += m62332.o
>>  obj-$(CONFIG_MAX517) += max517.o
>>  obj-$(CONFIG_MAX5821) += max5821.o
>>  obj-$(CONFIG_MCP4725) += mcp4725.o
>> +obj-$(CONFIG_MCP4728) += mcp4728.o
>>  obj-$(CONFIG_MCP4922) += mcp4922.o
>> diff --git a/drivers/iio/dac/mcp4728.c b/drivers/iio/dac/mcp4728.c
>> new file mode 100644
>> index 0000000..6dbbfac
>> --- /dev/null
>> +++ b/drivers/iio/dac/mcp4728.c
>> @@ -0,0 +1,479 @@
>> +/*
>> + * mcp4728.c - Support for Microchip MCP4728
>> + *
>> + * Copyright (C) 2016 Evgeny Boger <boger@contactless.ru>
>> + *
>> + * Based on mcp4728 by Peter Meerwald <pmeerw@pmeerw.net>
>> + *
>> + * This file is subject to the terms and conditions of version 2 of
>> + * the GNU General Public License.  See the file COPYING in the main
>> + * directory of this archive for more details.
>> + *
>> + * driver for the Microchip MCP4728 I2C 12-bit quad
>digital-to-analog
>> + * converter (DAC)
>> + * (7-bit I2C slave address 0x60, the three LSBs can be configured
>in
>> + * hardware)
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/i2c.h>
>> +#include <linux/err.h>
>> +#include <linux/delay.h>
>> +
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/sysfs.h>
>> +
>> +#include <linux/iio/dac/mcp4728.h>
>> +
>> +#define MCP4728_DRV_NAME "mcp4728"
>> +#define MCP4728_NUM_CHANNELS 4
>> +#define MCP4728_INT_VREF_MV 2048
>> +#define MCP4728_RESOLUTION 12
>> +
>> +struct mcp4728_scale {
>> +	unsigned int		integer;
>> +	unsigned int		micro;
>
>use just a single space
>
>> +};
>> +
>> +struct mcp4728_channel_state {
>> +	u16 dac_value;
>> +	bool powerdown;
>> +	u8 gain;
>> +	u8 vref;
>
>could this be bool?
>
>> +	unsigned powerdown_mode;
>> +};
>> +
>> +struct mcp4728_data {
>> +	struct i2c_client *client;
>> +	struct mcp4728_channel_state channel_state[MCP4728_NUM_CHANNELS];
>> +	struct mcp4728_scale	scale_avail[MCP4728_NUM_CHANNELS][2];
>> +
>> +};
>> +
>> +static int mcp4728_write_reg(struct mcp4728_data *data, int ch, bool
>eeprom)
>> +{
>> +	struct mcp4728_channel_state *state = &data->channel_state[ch];
>> +	u8 outbuf[3];
>> +	u8 pd;
>> +	int ret;
>> +
>> +	if (state->dac_value >= (1 << MCP4728_RESOLUTION) ||
>> +				state->dac_value < 0)
>> +		return -EINVAL;
>> +/*
>> +  Write input register. EEPROM is not updated:
>> +  c2=0, c1=1, c0=0, w1=0, w0=0 : 0x40
>> +
>> +  Write input and EEPROM registers:
>> +  c2=0, c1=1, c0=0, w1=1, w0=1 : 0x58
>> +
>> +  b[0]: c2 | c1 | c0 | w1 | w0 | dac1 | dac0 | /udac
>> +  b[1]: vref | pd1 | pd0 | gain | d11 | d10 | d9 | d8
>> +  b[2]: d7-d0
>> +*/
>
>maybe indent the block above
>
>> +	pd = state->powerdown ? (state->powerdown_mode + 1) : 0;
>> +
>> +	outbuf[0] = (eeprom ? 0x58 : 0x40) |
>> +				((ch & 0x03) << 1);
>> +	outbuf[1] = ((state->vref & 0x01) << 7) |
>> +				((pd & 0x3) << 5)    |
>> +				((state->gain & 0x01) << 4) |
>> +				((state->dac_value >> 8) & 0x0f);
>> +	outbuf[2] = state->dac_value & 0xff;
>> +
>> +	ret = i2c_master_send(data->client, outbuf, 3);
>
>3 could be replaced by sizeof(outbuf), here and below
>
>> +	if (ret < 0)
>> +		return ret;
>> +	else if (ret != 3)
>> +		return -EIO;
>> +	else
>> +		return 0;
>> +}
>> +
>> +static int mcp4728_write_input_reg(struct mcp4728_data *data, int
>ch)
>> +{
>> +	return mcp4728_write_reg(data, ch, false);
>> +}
>> +
>> +static int mcp4728_suspend(struct device *dev)
>> +{
>> +	struct mcp4728_data *data = iio_priv(i2c_get_clientdata(
>> +		to_i2c_client(dev)));
>> +	int ch, ret;
>> +
>> +	for (ch = 0; ch < MCP4728_NUM_CHANNELS; ++ch) {
>> +		data->channel_state[ch].powerdown = true;
>> +		ret = mcp4728_write_input_reg(data, ch);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>
>_suspend()/_resume() code is almost identical, split out common code?
>
>> +static int mcp4728_resume(struct device *dev)
>> +{
>> +	struct mcp4728_data *data = iio_priv(i2c_get_clientdata(
>> +		to_i2c_client(dev)));
>> +	int ch, ret;
>> +
>> +	for (ch = 0; ch < MCP4728_NUM_CHANNELS; ++ch) {
>> +		data->channel_state[ch].powerdown = false;
>> +		ret = mcp4728_write_input_reg(data, ch);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static SIMPLE_DEV_PM_OPS(mcp4728_pm_ops, mcp4728_suspend,
>mcp4728_resume);
>> +#define MCP4728_PM_OPS (&mcp4728_pm_ops)
>> +#else
>> +#define MCP4728_PM_OPS NULL
>> +#endif
>> +
>> +static ssize_t mcp4728_store_eeprom(struct device *dev,
>> +	struct device_attribute *attr, const char *buf, size_t len)
>> +{
>> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +	int tries;
>> +	u8 inbuf[3];
>
>actually just one byte is used?
>
>> +	bool state;
>> +	int ret, ch;
>> +
>> +	ret = strtobool(buf, &state);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	if (!state)
>> +		return 0;
>> +
>> +	for (ch = 0; ch < MCP4728_NUM_CHANNELS; ++ch) {
>> +		tries = 20;
>
>make the variable 'tries' local to the loop?
>maybe 'inbuf' as well
>
>> +		mcp4728_write_reg(data, ch, true);
>> +
>> +
>> +		/* wait for write complete, takes up to 50ms */
>> +		while (tries--) {
>> +			msleep(20);
>> +			ret = i2c_master_recv(data->client, inbuf, 1);
>> +			if (ret < 0)
>> +				return ret;
>> +			else if (ret != 1)
>> +				return -EIO;
>> +
>> +			if (inbuf[0] & 0x80)
>> +				break;
>> +		}
>> +
>> +		if (tries < 0) {
>> +			dev_err(&data->client->dev,
>> +				"mcp4728_store_eeprom() failed, incomplete\n");
>> +			return -EIO;
>> +		}
>> +	}
>> +
>> +	return len;
>> +}
>> +
>> +static IIO_DEVICE_ATTR(store_eeprom, S_IWUSR, NULL,
>mcp4728_store_eeprom, 0);
>> +
>> +
>> +static ssize_t mcp4728_show_scale_available(struct device *dev,
>> +		struct device_attribute *attr,
>> +		char *buf)
>> +{
>> +	struct iio_dev_attr *iio_attr = to_iio_dev_attr(attr);
>> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +	u8 ch = iio_attr->address;
>> +	int i, len = 0;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(data->scale_avail[ch]); i++)
>> +		len += sprintf(buf + len, "%u.%06u ",
>> +			       data->scale_avail[ch][i].integer,
>> +			       data->scale_avail[ch][i].micro);
>> +
>> +	len += sprintf(buf + len, "\n");
>> +
>> +	return len;
>> +}
>> +
>> +#define SHOW_SCALE_AVAILABLE_ATTR(ch)					\
>> +static IIO_DEVICE_ATTR(out_voltage##ch##_scale_available, S_IRUGO,	\
>> +		       mcp4728_show_scale_available, NULL, ch)
>> +
>> +SHOW_SCALE_AVAILABLE_ATTR(0);
>> +SHOW_SCALE_AVAILABLE_ATTR(1);
>> +SHOW_SCALE_AVAILABLE_ATTR(2);
>> +SHOW_SCALE_AVAILABLE_ATTR(3);
>> +
>> +static struct attribute *mcp4728_attributes[] = {
>> +	&iio_dev_attr_store_eeprom.dev_attr.attr,
>> +	&iio_dev_attr_out_voltage0_scale_available.dev_attr.attr,
>> +	&iio_dev_attr_out_voltage1_scale_available.dev_attr.attr,
>> +	&iio_dev_attr_out_voltage2_scale_available.dev_attr.attr,
>> +	&iio_dev_attr_out_voltage3_scale_available.dev_attr.attr,
>> +	NULL,
>> +};
>> +
>> +static const struct attribute_group mcp4728_attribute_group = {
>> +	.attrs = mcp4728_attributes,
>> +};
>> +
>> +static const char * const mcp4728_powerdown_modes[] = {
>> +	"1kohm_to_gnd",
>> +	"100kohm_to_gnd",
>> +	"500kohm_to_gnd"
>> +};
>> +
>> +static int mcp4728_get_powerdown_mode(struct iio_dev *indio_dev,
>> +	const struct iio_chan_spec *chan)
>> +{
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +
>> +	return data->channel_state[chan->channel].powerdown_mode;
>> +}
>> +
>> +static int mcp4728_set_powerdown_mode(struct iio_dev *indio_dev,
>> +	const struct iio_chan_spec *chan, unsigned mode)
>> +{
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +
>> +	data->channel_state[chan->channel].powerdown_mode = mode;
>> +
>> +	return 0;
>> +}
>> +
>> +static ssize_t mcp4728_read_powerdown(struct iio_dev *indio_dev,
>> +	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
>> +{
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +
>> +	return sprintf(buf, "%d\n",
>> +		data->channel_state[chan->channel].powerdown);
>> +}
>> +
>> +static ssize_t mcp4728_write_powerdown(struct iio_dev *indio_dev,
>> +	 uintptr_t private, const struct iio_chan_spec *chan,
>> +	 const char *buf, size_t len)
>> +{
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +	struct mcp4728_channel_state *state =
>> +			&data->channel_state[chan->channel];
>> +	bool powerdown;
>> +	int ret;
>> +
>> +	ret = strtobool(buf, &powerdown);
>> +	if (ret)
>> +		return ret;
>> +
>> +	state->powerdown = powerdown;
>> +	ret = mcp4728_write_input_reg(data, chan->channel);
>> +
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return len;
>> +}
>> +
>> +static const struct iio_enum mcp4728_powerdown_mode_enum = {
>> +	.items = mcp4728_powerdown_modes,
>> +	.num_items = ARRAY_SIZE(mcp4728_powerdown_modes),
>> +	.get = mcp4728_get_powerdown_mode,
>> +	.set = mcp4728_set_powerdown_mode,
>> +};
>> +
>> +static const struct iio_chan_spec_ext_info mcp4728_ext_info[] = {
>> +	{
>> +		.name = "powerdown",
>> +		.read = mcp4728_read_powerdown,
>> +		.write = mcp4728_write_powerdown,
>> +		.shared = IIO_SEPARATE,
>> +	},
>> +	IIO_ENUM("powerdown_mode", IIO_SEPARATE,
>&mcp4728_powerdown_mode_enum),
>> +	IIO_ENUM_AVAILABLE("powerdown_mode", &mcp4728_powerdown_mode_enum),
>> +	{ },
>> +};
>> +
>> +#define MCP4728_CHAN(chan) {			\
>> +	.type		= IIO_VOLTAGE, \
>> +	.indexed	= 1, \
>> +	.output		= 1, \
>> +	.channel	= chan, \
>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
>> +			      BIT(IIO_CHAN_INFO_SCALE), \
>> +	.ext_info	= mcp4728_ext_info, \
>> +}
>> +
>> +
>> +static const struct iio_chan_spec
>mcp4728_channels[MCP4728_NUM_CHANNELS] = {
>> +	MCP4728_CHAN(0),
>> +	MCP4728_CHAN(1),
>> +	MCP4728_CHAN(2),
>> +	MCP4728_CHAN(3),
>> +};
>> +
>> +static int mcp4728_read_raw(struct iio_dev *indio_dev,
>> +			   struct iio_chan_spec const *chan,
>> +			   int *val, int *val2, long mask)
>> +{
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +	struct mcp4728_channel_state *state =
>> +			&data->channel_state[chan->channel];
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_RAW:
>> +		*val = data->channel_state[chan->channel].dac_value;
>> +		return IIO_VAL_INT;
>> +	case IIO_CHAN_INFO_SCALE:
>> +		*val = (state->gain + 1) * MCP4728_INT_VREF_MV;
>> +		*val2 = MCP4728_RESOLUTION;
>> +		return IIO_VAL_FRACTIONAL_LOG2;
>> +	}
>> +	return -EINVAL;
>> +}
>> +
>> +static int mcp4728_write_raw(struct iio_dev *indio_dev,
>> +			       struct iio_chan_spec const *chan,
>> +			       int val, int val2, long mask)
>> +{
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +	struct mcp4728_channel_state *state =
>> +			&data->channel_state[chan->channel];
>> +	struct mcp4728_scale *scale_avail =
>> +		data->scale_avail[chan->channel];
>> +
>> +	int ret, gain;
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_RAW:
>> +		state->dac_value = val;
>> +		ret = mcp4728_write_input_reg(data, chan->channel);
>> +		break;
>> +	case IIO_CHAN_INFO_SCALE:
>> +		ret = -EINVAL;
>> +
>> +		for (gain = 0; gain < 2; ++gain) {
>> +			if (val == scale_avail[gain].integer &&
>> +			val2 == scale_avail[gain].micro) {
>> +
>> +				state->gain = gain;
>> +				ret = mcp4728_write_input_reg(data,
>> +					chan->channel);
>> +			}
>> +		}
>> +
>> +		break;
>> +	default:
>> +		ret = -EINVAL;
>> +		break;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct iio_info mcp4728_info = {
>> +	.read_raw = mcp4728_read_raw,
>> +	.write_raw = mcp4728_write_raw,
>> +	.attrs = &mcp4728_attribute_group,
>> +	.driver_module = THIS_MODULE,
>> +};
>> +
>> +static int mcp4728_probe(struct i2c_client *client,
>> +			 const struct i2c_device_id *id)
>> +{
>> +	struct mcp4728_data *data;
>> +	struct iio_dev *indio_dev;
>> +	u8 inbuf[24];
>> +	u8 *reg;
>> +	u8 pd;
>> +	int err;
>> +	int i, gain;
>> +	u64 scale_uv;
>> +
>> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>> +	if (indio_dev == NULL)
>> +		return -ENOMEM;
>> +	data = iio_priv(indio_dev);
>> +	i2c_set_clientdata(client, indio_dev);
>> +	data->client = client;
>> +
>> +	indio_dev->dev.parent = &client->dev;
>> +	indio_dev->info = &mcp4728_info;
>
>indio_dev->name ?
>
>> +	indio_dev->channels = mcp4728_channels;
>> +	indio_dev->num_channels = MCP4728_NUM_CHANNELS;
>> +	indio_dev->modes = INDIO_DIRECT_MODE;
>> +
>> +	/* read current DAC value */
>> +	err = i2c_master_recv(client, inbuf, 24);
>> +	if (err < 0) {
>> +		dev_err(&client->dev, "failed to read DAC value");
>
>missing \n
>
>> +		return err;
>> +	}
>> +
>> +	for (i = 0; i < MCP4728_NUM_CHANNELS; ++i) {
>> +		reg = &inbuf[6*i];
>> +		pd = (reg[1] >> 5) & 0x3;
>> +		data->channel_state[i].powerdown = pd > 0 ? true : false;
>> +
>> +		/* set to 500kohm_to_gnd if channel is now powered down */
>> +		data->channel_state[i].powerdown_mode = pd ? pd-1 : 2;
>> +		data->channel_state[i].dac_value = ((reg[1] & 0x0f) << 8) |
>> +						   (reg[2]);
>> +		data->channel_state[i].gain = (reg[1] >> 4) & 0x01;
>> +
>> +		/* Use internal reference */
>> +		data->channel_state[i].vref = 1;
>> +
>> +		/* Populate available ADC input ranges */
>> +		for (gain = 0; gain < 2; gain++) {
>> +			/*
>> +			  [gain=0] = Gain Setting 1, vref x 1
>> +			  [gain=1] = Gain Setting 2, vref x 2
>> +
>> +			  The scale is calculated by doing:
>> +			    Vref >> (realbits - gain)
>> +			  which multiplies by two on the second component
>> +			  of the array.
>> +			 */
>> +			scale_uv = ((u64) MCP4728_INT_VREF_MV * 100000) >>
>> +				   (MCP4728_RESOLUTION - gain);
>> +			data->scale_avail[i][gain].micro =
>> +					do_div(scale_uv, 100000) * 10;
>> +			data->scale_avail[i][gain].integer = scale_uv;
>> +		}
>> +	}
>> +
>> +	return iio_device_register(indio_dev);
>> +}
>> +
>> +static int mcp4728_remove(struct i2c_client *client)
>> +{
>> +	iio_device_unregister(i2c_get_clientdata(client));
>> +	return 0;
>> +}
>> +
>> +static const struct i2c_device_id mcp4728_id[] = {
>> +	{ "mcp4728", 0 },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(i2c, mcp4728_id);
>> +
>> +static struct i2c_driver mcp4728_driver = {
>> +	.driver = {
>> +		.name	= MCP4728_DRV_NAME,
>> +		.pm	= MCP4728_PM_OPS,
>> +	},
>> +	.probe		= mcp4728_probe,
>> +	.remove		= mcp4728_remove,
>> +	.id_table	= mcp4728_id,
>> +};
>> +module_i2c_driver(mcp4728_driver);
>> +
>> +MODULE_AUTHOR("Evgeny Boger <boger@contactless.ru>");
>> +MODULE_DESCRIPTION("MCP4728 quad 12-bit DAC");
>> +MODULE_LICENSE("GPL");
>> diff --git a/include/linux/iio/dac/mcp4728.h
>b/include/linux/iio/dac/mcp4728.h
>> new file mode 100644
>> index 0000000..14adb0c
>> --- /dev/null
>> +++ b/include/linux/iio/dac/mcp4728.h
>> @@ -0,0 +1,16 @@
>> +/*
>> + * MCP4728 DAC driver
>> + *
>> + * Copyright (C) 2016 Evgeny Boger <boger@contactless.ru>
>> + *
>> + * Licensed under the GPL-2 or later.
>> + */
>> +
>> +#ifndef IIO_DAC_MCP4728_H_
>> +#define IIO_DAC_MCP4728_H_
>> +
>> +struct mcp4728_platform_data {
>> +	u16 vref_mv;
>> +};
>> +
>> +#endif /* IIO_DAC_MCP4728_H_ */
>> 

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.