[PATCH] iio: add mcp4725 I2C DAC driver

[PATCH] iio: add mcp4725 I2C DAC driver

[Peter Meerwald]

v4:
* remove unused indio_dev pointer in mcp4725_data (Jonathan Cameron)
* use u16 instead of unsigned short in mcp4725_data (Jonathan Cameron)
* #include mcp4725.h from linux/iio/dac/

v3:
* move from staging to drivers/iio
* switch to chan_spec
* dev_get_drvdata() -> dev_to_iio_dev()
* annotate probe() and remove() with __devinit and __devexit

v2 (based on comments from Jonathan Cameron and Lars-Peter Clausen):
* did NOT switch to chan_spec yet
* rebase to staging-next tree, update iio header locations
* dropped dac.h #include, not needed
* strict_strtol() -> kstrtol()
* call iio_device_unregister() in remove()
* everything in one patch

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/iio/dac/Kconfig         |   11 ++
 drivers/iio/dac/Makefile        |    1 +
 drivers/iio/dac/mcp4725.c       |  227 +++++++++++++++++++++++++++++++++++++++
 include/linux/iio/dac/mcp4725.h |   16 +++
 4 files changed, 255 insertions(+)
 create mode 100644 drivers/iio/dac/mcp4725.c
 create mode 100644 include/linux/iio/dac/mcp4725.h

diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index a626f03..92fb3a0 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -118,4 +118,15 @@ config MAX517
 	  This driver can also be built as a module.  If so, the module
 	  will be called max517.
 
+config MCP4725
+	tristate "MCP4725 DAC driver"
+	depends on I2C
+	---help---
+	  Say Y here if you want to build a driver for the Microchip
+	  MCP 4725 12-bit digital-to-analog converter (DAC) with I2C
+	  interface.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called mcp4725.
+
 endmenu
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 8ab1d26..9ea3cee 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_AD5764) += ad5764.o
 obj-$(CONFIG_AD5791) += ad5791.o
 obj-$(CONFIG_AD5686) += ad5686.o
 obj-$(CONFIG_MAX517) += max517.o
+obj-$(CONFIG_MCP4725) += mcp4725.o
diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
new file mode 100644
index 0000000..d500693
--- /dev/null
+++ b/drivers/iio/dac/mcp4725.c
@@ -0,0 +1,227 @@
+/*
+ * mcp4725.c - Support for Microchip MCP4725
+ *
+ * Copyright (C) 2012 Peter Meerwald <pmeerw@pmeerw.net>
+ *
+ * Based on max517 by Roland Stigge <stigge@antcom.de>
+ *
+ * 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 I2C 12-bit digital-to-analog converter (DAC)
+ * (7-bit I2C slave address 0x60, the three LSBs can be configured in
+ * hardware)
+ *
+ * writing the DAC value to EEPROM is not supported
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#include <linux/iio/dac/mcp4725.h>
+
+#define MCP4725_DRV_NAME "mcp4725"
+
+struct mcp4725_data {
+	struct i2c_client *client;
+	u16 vref_mv;
+	u16 dac_value;
+};
+
+#ifdef CONFIG_PM_SLEEP
+static int mcp4725_suspend(struct device *dev)
+{
+	u8 outbuf[2];
+
+	outbuf[0] = 0x3 << 4; /* power-down bits, 500 kOhm resistor */
+	outbuf[1] = 0;
+
+	return i2c_master_send(to_i2c_client(dev), &outbuf, 2);
+}
+
+static int mcp4725_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct mcp4725_data *data = iio_priv(indio_dev);
+	u8 outbuf[2];
+
+	/* restore previous DAC value */
+	outbuf[0] = (data->dac_value >> 8) & 0xf;
+	outbuf[1] = data->dac_value & 0xff;
+
+	return i2c_master_send(to_i2c_client(dev), &outbuf, 2);
+}
+
+static SIMPLE_DEV_PM_OPS(mcp4725_pm_ops, mcp4725_suspend, mcp4725_resume);
+#define MCP4725_PM_OPS (&mcp4725_pm_ops)
+#else
+#define MCP4725_PM_OPS NULL
+#endif
+
+static const struct iio_chan_spec mcp4725_channel = {
+	.type		= IIO_VOLTAGE,
+	.indexed	= 1,
+	.output		= 1,
+	.channel	= 0,
+	.info_mask	= IIO_CHAN_INFO_RAW_SEPARATE_BIT |
+			  IIO_CHAN_INFO_SCALE_SHARED_BIT,
+	.scan_type	= IIO_ST('u', 12, 16, 0),
+};
+
+static int mcp4725_set_value(struct iio_dev *indio_dev, int val)
+{
+	struct mcp4725_data *data = iio_priv(indio_dev);
+	u8 outbuf[2];
+	int ret;
+
+	if (val >= (1 << 12) || val < 0)
+		return -EINVAL;
+
+	outbuf[0] = (val >> 8) & 0xf;
+	outbuf[1] = val & 0xff;
+
+	ret = i2c_master_send(data->client, outbuf, 2);
+	if (ret < 0)
+		return ret;
+	else if (ret != 2)
+		return -EIO;
+	else
+		return 0;
+}
+
+static int mcp4725_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct mcp4725_data *data = iio_priv(indio_dev);
+	unsigned long scale_uv;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		*val = data->dac_value;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		scale_uv = (data->vref_mv * 1000) >> 12;
+		*val =  scale_uv / 1000000;
+		*val2 = scale_uv % 1000000;
+		return IIO_VAL_INT_PLUS_MICRO;
+	}
+	return -EINVAL;
+}
+
+static int mcp4725_write_raw(struct iio_dev *indio_dev,
+			       struct iio_chan_spec const *chan,
+			       int val, int val2, long mask)
+{
+	struct mcp4725_data *data = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = mcp4725_set_value(indio_dev, val);
+		data->dac_value = val;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+static const struct iio_info mcp4725_info = {
+	.read_raw = mcp4725_read_raw,
+	.write_raw = mcp4725_write_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static int __devinit mcp4725_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct mcp4725_data *data;
+	struct iio_dev *indio_dev;
+	struct mcp4725_platform_data *platform_data = client->dev.platform_data;
+	u8 inbuf[3];
+	int err;
+
+	if (!platform_data || !platform_data->vref_mv) {
+		dev_err(&client->dev, "invalid platform data");
+		err = -EINVAL;
+		goto exit;
+	}
+
+	indio_dev = iio_device_alloc(sizeof(*data));
+	if (indio_dev == NULL) {
+		err = -ENOMEM;
+		goto exit;
+	}
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &mcp4725_info;
+	indio_dev->channels = &mcp4725_channel;
+	indio_dev->num_channels = 1;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	data->vref_mv = platform_data->vref_mv;
+
+	/* read current DAC value */
+	err = i2c_master_recv(client, inbuf, 3);
+	if (err < 0) {
+		dev_err(&client->dev, "failed to read DAC value");
+		goto exit_free_device;
+	}
+	data->dac_value = (inbuf[1] << 4) | (inbuf[2] >> 4);
+
+	err = iio_device_register(indio_dev);
+	if (err)
+		goto exit_free_device;
+
+	dev_info(&client->dev, "MCP4725 DAC registered\n");
+
+	return 0;
+
+exit_free_device:
+	iio_device_free(indio_dev);
+exit:
+	return err;
+}
+
+static int __devexit mcp4725_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+
+	iio_device_unregister(indio_dev);
+	iio_device_free(indio_dev);
+
+	return 0;
+}
+
+static const struct i2c_device_id mcp4725_id[] = {
+	{ "mcp4725", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, mcp4725_id);
+
+static struct i2c_driver mcp4725_driver = {
+	.driver = {
+		.name	= MCP4725_DRV_NAME,
+		.pm	= MCP4725_PM_OPS,
+	},
+	.probe		= mcp4725_probe,
+	.remove		= __devexit_p(mcp4725_remove),
+	.id_table	= mcp4725_id,
+};
+module_i2c_driver(mcp4725_driver);
+
+MODULE_AUTHOR("Peter Meerwald <pmeerw@pmeerw.net>");
+MODULE_DESCRIPTION("MCP4725 12-bit DAC");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/iio/dac/mcp4725.h b/include/linux/iio/dac/mcp4725.h
new file mode 100644
index 0000000..91530e6
--- /dev/null
+++ b/include/linux/iio/dac/mcp4725.h
@@ -0,0 +1,16 @@
+/*
+ * MCP4725 DAC driver
+ *
+ * Copyright (C) 2012 Peter Meerwald <pmeerw@pmeerw.net>
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#ifndef IIO_DAC_MCP4725_H_
+#define IIO_DAC_MCP4725_H_
+
+struct mcp4725_platform_data {
+	u16 vref_mv;
+};
+
+#endif /* IIO_DAC_MCP4725_H_ */
-- 
1.7.9.5

Re: [PATCH] iio: add mcp4725 I2C DAC driver

[Jonathan Cameron]

On 06/07/2012 09:30 AM, Peter Meerwald wrote:
> v4:
> * remove unused indio_dev pointer in mcp4725_data (Jonathan Cameron)
> * use u16 instead of unsigned short in mcp4725_data (Jonathan Cameron)
> * #include mcp4725.h from linux/iio/dac/
> 
> v3:
> * move from staging to drivers/iio
> * switch to chan_spec
> * dev_get_drvdata() -> dev_to_iio_dev()
> * annotate probe() and remove() with __devinit and __devexit
> 
> v2 (based on comments from Jonathan Cameron and Lars-Peter Clausen):
> * did NOT switch to chan_spec yet
> * rebase to staging-next tree, update iio header locations
> * dropped dac.h #include, not needed
> * strict_strtol() -> kstrtol()
> * call iio_device_unregister() in remove()
> * everything in one patch
> 
Sorry, didn't actually compile this whilst there were outstanding issues
from reading the code. Anyhow few things came up via sparse and and
my arm toolchain... (not sure which version off the top of my head)..

  CHECK   drivers/iio/dac/mcp4725.c
drivers/iio/dac/mcp4725.c:45:53: warning: incorrect type in argument 2
(different base types)
drivers/iio/dac/mcp4725.c:45:53:    expected char const *buf
drivers/iio/dac/mcp4725.c:45:53:    got unsigned char [usertype] (
*<noident> )[2]
drivers/iio/dac/mcp4725.c:58:53: warning: incorrect type in argument 2
(different base types)
drivers/iio/dac/mcp4725.c:58:53:    expected char const *buf
drivers/iio/dac/mcp4725.c:58:53:    got unsigned char [usertype] (
*<noident> )[2]
  CC [M]  drivers/iio/dac/mcp4725.o
drivers/iio/dac/mcp4725.c: In function 'mcp4725_suspend':
drivers/iio/dac/mcp4725.c:45:25: warning: passing argument 2 of
'i2c_master_send' from incompatible pointer type
include/linux/i2c.h:62:12: note: expected 'const char *' but argument is
of type 'u8 (*)[2]'
drivers/iio/dac/mcp4725.c: In function 'mcp4725_resume':
drivers/iio/dac/mcp4725.c:58:25: warning: passing argument 2 of
'i2c_master_send' from incompatible pointer type
include/linux/i2c.h:62:12: note: expected 'const char *' but argument is
of type 'u8 (*)[2]'

ouch.  Fix is pretty obvious.

Please make sure you are building all your code in future. sparse is
simple and useful. If you are feeling thorough, coccicheck and smatch
can catch more issues still.



> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
> ---
>  drivers/iio/dac/Kconfig         |   11 ++
>  drivers/iio/dac/Makefile        |    1 +
>  drivers/iio/dac/mcp4725.c       |  227 +++++++++++++++++++++++++++++++++++++++
>  include/linux/iio/dac/mcp4725.h |   16 +++
>  4 files changed, 255 insertions(+)
>  create mode 100644 drivers/iio/dac/mcp4725.c
>  create mode 100644 include/linux/iio/dac/mcp4725.h
> 
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index a626f03..92fb3a0 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -118,4 +118,15 @@ config MAX517
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called max517.
>  
> +config MCP4725
> +	tristate "MCP4725 DAC driver"
> +	depends on I2C
> +	---help---
> +	  Say Y here if you want to build a driver for the Microchip
> +	  MCP 4725 12-bit digital-to-analog converter (DAC) with I2C
> +	  interface.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called mcp4725.
> +
>  endmenu
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 8ab1d26..9ea3cee 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -13,3 +13,4 @@ obj-$(CONFIG_AD5764) += ad5764.o
>  obj-$(CONFIG_AD5791) += ad5791.o
>  obj-$(CONFIG_AD5686) += ad5686.o
>  obj-$(CONFIG_MAX517) += max517.o
> +obj-$(CONFIG_MCP4725) += mcp4725.o
> diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
> new file mode 100644
> index 0000000..d500693
> --- /dev/null
> +++ b/drivers/iio/dac/mcp4725.c
> @@ -0,0 +1,227 @@
> +/*
> + * mcp4725.c - Support for Microchip MCP4725
> + *
> + * Copyright (C) 2012 Peter Meerwald <pmeerw@pmeerw.net>
> + *
> + * Based on max517 by Roland Stigge <stigge@antcom.de>
> + *
> + * 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 I2C 12-bit digital-to-analog converter (DAC)
> + * (7-bit I2C slave address 0x60, the three LSBs can be configured in
> + * hardware)
> + *
> + * writing the DAC value to EEPROM is not supported
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/i2c.h>
> +#include <linux/err.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#include <linux/iio/dac/mcp4725.h>
> +
> +#define MCP4725_DRV_NAME "mcp4725"
> +
> +struct mcp4725_data {
> +	struct i2c_client *client;
> +	u16 vref_mv;
> +	u16 dac_value;
> +};
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int mcp4725_suspend(struct device *dev)
> +{
> +	u8 outbuf[2];
> +
> +	outbuf[0] = 0x3 << 4; /* power-down bits, 500 kOhm resistor */
> +	outbuf[1] = 0;
> +
> +	return i2c_master_send(to_i2c_client(dev), &outbuf, 2);
> +}
> +
> +static int mcp4725_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct mcp4725_data *data = iio_priv(indio_dev);
> +	u8 outbuf[2];
> +
> +	/* restore previous DAC value */
> +	outbuf[0] = (data->dac_value >> 8) & 0xf;
> +	outbuf[1] = data->dac_value & 0xff;
> +
> +	return i2c_master_send(to_i2c_client(dev), &outbuf, 2);
> +}
> +
> +static SIMPLE_DEV_PM_OPS(mcp4725_pm_ops, mcp4725_suspend, mcp4725_resume);
> +#define MCP4725_PM_OPS (&mcp4725_pm_ops)
> +#else
> +#define MCP4725_PM_OPS NULL
> +#endif
> +
> +static const struct iio_chan_spec mcp4725_channel = {
> +	.type		= IIO_VOLTAGE,
> +	.indexed	= 1,
> +	.output		= 1,
> +	.channel	= 0,
> +	.info_mask	= IIO_CHAN_INFO_RAW_SEPARATE_BIT |
> +			  IIO_CHAN_INFO_SCALE_SHARED_BIT,
> +	.scan_type	= IIO_ST('u', 12, 16, 0),
> +};
> +
> +static int mcp4725_set_value(struct iio_dev *indio_dev, int val)
> +{
> +	struct mcp4725_data *data = iio_priv(indio_dev);
> +	u8 outbuf[2];
> +	int ret;
> +
> +	if (val >= (1 << 12) || val < 0)
> +		return -EINVAL;
> +
> +	outbuf[0] = (val >> 8) & 0xf;
> +	outbuf[1] = val & 0xff;
> +
> +	ret = i2c_master_send(data->client, outbuf, 2);
> +	if (ret < 0)
> +		return ret;
> +	else if (ret != 2)
> +		return -EIO;
> +	else
> +		return 0;
> +}
> +
> +static int mcp4725_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long mask)
> +{
> +	struct mcp4725_data *data = iio_priv(indio_dev);
> +	unsigned long scale_uv;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		*val = data->dac_value;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		scale_uv = (data->vref_mv * 1000) >> 12;
> +		*val =  scale_uv / 1000000;
> +		*val2 = scale_uv % 1000000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	}
> +	return -EINVAL;
> +}
> +
> +static int mcp4725_write_raw(struct iio_dev *indio_dev,
> +			       struct iio_chan_spec const *chan,
> +			       int val, int val2, long mask)
> +{
> +	struct mcp4725_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = mcp4725_set_value(indio_dev, val);
> +		data->dac_value = val;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct iio_info mcp4725_info = {
> +	.read_raw = mcp4725_read_raw,
> +	.write_raw = mcp4725_write_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static int __devinit mcp4725_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	struct mcp4725_data *data;
> +	struct iio_dev *indio_dev;
> +	struct mcp4725_platform_data *platform_data = client->dev.platform_data;
> +	u8 inbuf[3];
> +	int err;
> +
> +	if (!platform_data || !platform_data->vref_mv) {
> +		dev_err(&client->dev, "invalid platform data");
> +		err = -EINVAL;
> +		goto exit;
> +	}
> +
> +	indio_dev = iio_device_alloc(sizeof(*data));
> +	if (indio_dev == NULL) {
> +		err = -ENOMEM;
> +		goto exit;
> +	}
> +	data = iio_priv(indio_dev);
> +	i2c_set_clientdata(client, indio_dev);
> +	data->client = client;
> +
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->info = &mcp4725_info;
> +	indio_dev->channels = &mcp4725_channel;
> +	indio_dev->num_channels = 1;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	data->vref_mv = platform_data->vref_mv;
> +
> +	/* read current DAC value */
> +	err = i2c_master_recv(client, inbuf, 3);
> +	if (err < 0) {
> +		dev_err(&client->dev, "failed to read DAC value");
> +		goto exit_free_device;
> +	}
> +	data->dac_value = (inbuf[1] << 4) | (inbuf[2] >> 4);
> +
> +	err = iio_device_register(indio_dev);
> +	if (err)
> +		goto exit_free_device;
> +
> +	dev_info(&client->dev, "MCP4725 DAC registered\n");
> +
> +	return 0;
> +
> +exit_free_device:
> +	iio_device_free(indio_dev);
> +exit:
> +	return err;
> +}
> +
> +static int __devexit mcp4725_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +
> +	iio_device_unregister(indio_dev);
> +	iio_device_free(indio_dev);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id mcp4725_id[] = {
> +	{ "mcp4725", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, mcp4725_id);
> +
> +static struct i2c_driver mcp4725_driver = {
> +	.driver = {
> +		.name	= MCP4725_DRV_NAME,
> +		.pm	= MCP4725_PM_OPS,
> +	},
> +	.probe		= mcp4725_probe,
> +	.remove		= __devexit_p(mcp4725_remove),
> +	.id_table	= mcp4725_id,
> +};
> +module_i2c_driver(mcp4725_driver);
> +
> +MODULE_AUTHOR("Peter Meerwald <pmeerw@pmeerw.net>");
> +MODULE_DESCRIPTION("MCP4725 12-bit DAC");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/iio/dac/mcp4725.h b/include/linux/iio/dac/mcp4725.h
> new file mode 100644
> index 0000000..91530e6
> --- /dev/null
> +++ b/include/linux/iio/dac/mcp4725.h
> @@ -0,0 +1,16 @@
> +/*
> + * MCP4725 DAC driver
> + *
> + * Copyright (C) 2012 Peter Meerwald <pmeerw@pmeerw.net>
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#ifndef IIO_DAC_MCP4725_H_
> +#define IIO_DAC_MCP4725_H_
> +
> +struct mcp4725_platform_data {
> +	u16 vref_mv;
> +};
> +
> +#endif /* IIO_DAC_MCP4725_H_ */

Re: [PATCH] iio: add mcp4725 I2C DAC driver

[Peter Meerwald]

Hello Jonathan,

> from reading the code. Anyhow few things came up via sparse and and
> my arm toolchain... (not sure which version off the top of my head)..

>   CHECK   drivers/iio/dac/mcp4725.c
> drivers/iio/dac/mcp4725.c:45:53: warning: incorrect type in argument 2
> (different base types)
> drivers/iio/dac/mcp4725.c:45:53:    expected char const *buf
> drivers/iio/dac/mcp4725.c:45:53:    got unsigned char [usertype] (
> *<noident> )[2]
> drivers/iio/dac/mcp4725.c:58:53: warning: incorrect type in argument 2
> (different base types)
> drivers/iio/dac/mcp4725.c:58:53:    expected char const *buf
> drivers/iio/dac/mcp4725.c:58:53:    got unsigned char [usertype] (
> *<noident> )[2]
>   CC [M]  drivers/iio/dac/mcp4725.o

I did not know about sparse

compiling out-of-tree with my toolchain did not yield above warnings, 
sorry for the extra review cycle

regards, p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [PATCH] iio: add mcp4725 I2C DAC driver

[Jonathan Cameron]

On 6/8/2012 8:12 AM, Peter Meerwald wrote:
> Hello Jonathan,
>
>> from reading the code. Anyhow few things came up via sparse and and
>> my arm toolchain... (not sure which version off the top of my head)..
>
>>    CHECK   drivers/iio/dac/mcp4725.c
>> drivers/iio/dac/mcp4725.c:45:53: warning: incorrect type in argument 2
>> (different base types)
>> drivers/iio/dac/mcp4725.c:45:53:    expected char const *buf
>> drivers/iio/dac/mcp4725.c:45:53:    got unsigned char [usertype] (
>> *<noident>  )[2]
>> drivers/iio/dac/mcp4725.c:58:53: warning: incorrect type in argument 2
>> (different base types)
>> drivers/iio/dac/mcp4725.c:58:53:    expected char const *buf
>> drivers/iio/dac/mcp4725.c:58:53:    got unsigned char [usertype] (
>> *<noident>  )[2]
>>    CC [M]  drivers/iio/dac/mcp4725.o
>
> I did not know about sparse
>
> compiling out-of-tree with my toolchain did not yield above warnings,
> sorry for the extra review cycle
That's quite all right!  I've done plenty of similar things myself
and had others poitn them out.
>
> regards, p.
>

[PATCH] iio: add mcp4725 I2C DAC driver

[Peter Meerwald]

v5:
* fix warnings (Jonathan Cameron)

v4:
* remove unused indio_dev pointer in mcp4725_data (Jonathan Cameron)
* use u16 instead of unsigned short in mcp4725_data (Jonathan Cameron)
* #include mcp4725.h from linux/iio/dac/

v3:
* move from staging to drivers/iio
* switch to chan_spec
* dev_get_drvdata() -> dev_to_iio_dev()
* annotate probe() and remove() with __devinit and __devexit

v2 (based on comments from Jonathan Cameron and Lars-Peter Clausen):
* did NOT switch to chan_spec yet
* rebase to staging-next tree, update iio header locations
* dropped dac.h #include, not needed
* strict_strtol() -> kstrtol()
* call iio_device_unregister() in remove()
* everything in one patch

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
Acked-by: Jonathan Cameron <jic23@kernel.org>

---
 drivers/iio/dac/Kconfig         |   11 ++
 drivers/iio/dac/Makefile        |    1 +
 drivers/iio/dac/mcp4725.c       |  227 +++++++++++++++++++++++++++++++++++++++
 include/linux/iio/dac/mcp4725.h |   16 +++
 4 files changed, 255 insertions(+)
 create mode 100644 drivers/iio/dac/mcp4725.c
 create mode 100644 include/linux/iio/dac/mcp4725.h

diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index a626f03..92fb3a0 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -118,4 +118,15 @@ config MAX517
 	  This driver can also be built as a module.  If so, the module
 	  will be called max517.
 
+config MCP4725
+	tristate "MCP4725 DAC driver"
+	depends on I2C
+	---help---
+	  Say Y here if you want to build a driver for the Microchip
+	  MCP 4725 12-bit digital-to-analog converter (DAC) with I2C
+	  interface.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called mcp4725.
+
 endmenu
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 8ab1d26..9ea3cee 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_AD5764) += ad5764.o
 obj-$(CONFIG_AD5791) += ad5791.o
 obj-$(CONFIG_AD5686) += ad5686.o
 obj-$(CONFIG_MAX517) += max517.o
+obj-$(CONFIG_MCP4725) += mcp4725.o
diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
new file mode 100644
index 0000000..e0e168b
--- /dev/null
+++ b/drivers/iio/dac/mcp4725.c
@@ -0,0 +1,227 @@
+/*
+ * mcp4725.c - Support for Microchip MCP4725
+ *
+ * Copyright (C) 2012 Peter Meerwald <pmeerw@pmeerw.net>
+ *
+ * Based on max517 by Roland Stigge <stigge@antcom.de>
+ *
+ * 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 I2C 12-bit digital-to-analog converter (DAC)
+ * (7-bit I2C slave address 0x60, the three LSBs can be configured in
+ * hardware)
+ *
+ * writing the DAC value to EEPROM is not supported
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#include <linux/iio/dac/mcp4725.h>
+
+#define MCP4725_DRV_NAME "mcp4725"
+
+struct mcp4725_data {
+	struct i2c_client *client;
+	u16 vref_mv;
+	u16 dac_value;
+};
+
+#ifdef CONFIG_PM_SLEEP
+static int mcp4725_suspend(struct device *dev)
+{
+	u8 outbuf[2];
+
+	outbuf[0] = 0x3 << 4; /* power-down bits, 500 kOhm resistor */
+	outbuf[1] = 0;
+
+	return i2c_master_send(to_i2c_client(dev), outbuf, 2);
+}
+
+static int mcp4725_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct mcp4725_data *data = iio_priv(indio_dev);
+	u8 outbuf[2];
+
+	/* restore previous DAC value */
+	outbuf[0] = (data->dac_value >> 8) & 0xf;
+	outbuf[1] = data->dac_value & 0xff;
+
+	return i2c_master_send(to_i2c_client(dev), outbuf, 2);
+}
+
+static SIMPLE_DEV_PM_OPS(mcp4725_pm_ops, mcp4725_suspend, mcp4725_resume);
+#define MCP4725_PM_OPS (&mcp4725_pm_ops)
+#else
+#define MCP4725_PM_OPS NULL
+#endif
+
+static const struct iio_chan_spec mcp4725_channel = {
+	.type		= IIO_VOLTAGE,
+	.indexed	= 1,
+	.output		= 1,
+	.channel	= 0,
+	.info_mask	= IIO_CHAN_INFO_RAW_SEPARATE_BIT |
+			  IIO_CHAN_INFO_SCALE_SHARED_BIT,
+	.scan_type	= IIO_ST('u', 12, 16, 0),
+};
+
+static int mcp4725_set_value(struct iio_dev *indio_dev, int val)
+{
+	struct mcp4725_data *data = iio_priv(indio_dev);
+	u8 outbuf[2];
+	int ret;
+
+	if (val >= (1 << 12) || val < 0)
+		return -EINVAL;
+
+	outbuf[0] = (val >> 8) & 0xf;
+	outbuf[1] = val & 0xff;
+
+	ret = i2c_master_send(data->client, outbuf, 2);
+	if (ret < 0)
+		return ret;
+	else if (ret != 2)
+		return -EIO;
+	else
+		return 0;
+}
+
+static int mcp4725_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct mcp4725_data *data = iio_priv(indio_dev);
+	unsigned long scale_uv;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		*val = data->dac_value;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		scale_uv = (data->vref_mv * 1000) >> 12;
+		*val =  scale_uv / 1000000;
+		*val2 = scale_uv % 1000000;
+		return IIO_VAL_INT_PLUS_MICRO;
+	}
+	return -EINVAL;
+}
+
+static int mcp4725_write_raw(struct iio_dev *indio_dev,
+			       struct iio_chan_spec const *chan,
+			       int val, int val2, long mask)
+{
+	struct mcp4725_data *data = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = mcp4725_set_value(indio_dev, val);
+		data->dac_value = val;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+static const struct iio_info mcp4725_info = {
+	.read_raw = mcp4725_read_raw,
+	.write_raw = mcp4725_write_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static int __devinit mcp4725_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct mcp4725_data *data;
+	struct iio_dev *indio_dev;
+	struct mcp4725_platform_data *platform_data = client->dev.platform_data;
+	u8 inbuf[3];
+	int err;
+
+	if (!platform_data || !platform_data->vref_mv) {
+		dev_err(&client->dev, "invalid platform data");
+		err = -EINVAL;
+		goto exit;
+	}
+
+	indio_dev = iio_device_alloc(sizeof(*data));
+	if (indio_dev == NULL) {
+		err = -ENOMEM;
+		goto exit;
+	}
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &mcp4725_info;
+	indio_dev->channels = &mcp4725_channel;
+	indio_dev->num_channels = 1;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	data->vref_mv = platform_data->vref_mv;
+
+	/* read current DAC value */
+	err = i2c_master_recv(client, inbuf, 3);
+	if (err < 0) {
+		dev_err(&client->dev, "failed to read DAC value");
+		goto exit_free_device;
+	}
+	data->dac_value = (inbuf[1] << 4) | (inbuf[2] >> 4);
+
+	err = iio_device_register(indio_dev);
+	if (err)
+		goto exit_free_device;
+
+	dev_info(&client->dev, "MCP4725 DAC registered\n");
+
+	return 0;
+
+exit_free_device:
+	iio_device_free(indio_dev);
+exit:
+	return err;
+}
+
+static int __devexit mcp4725_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+
+	iio_device_unregister(indio_dev);
+	iio_device_free(indio_dev);
+
+	return 0;
+}
+
+static const struct i2c_device_id mcp4725_id[] = {
+	{ "mcp4725", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, mcp4725_id);
+
+static struct i2c_driver mcp4725_driver = {
+	.driver = {
+		.name	= MCP4725_DRV_NAME,
+		.pm	= MCP4725_PM_OPS,
+	},
+	.probe		= mcp4725_probe,
+	.remove		= __devexit_p(mcp4725_remove),
+	.id_table	= mcp4725_id,
+};
+module_i2c_driver(mcp4725_driver);
+
+MODULE_AUTHOR("Peter Meerwald <pmeerw@pmeerw.net>");
+MODULE_DESCRIPTION("MCP4725 12-bit DAC");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/iio/dac/mcp4725.h b/include/linux/iio/dac/mcp4725.h
new file mode 100644
index 0000000..91530e6
--- /dev/null
+++ b/include/linux/iio/dac/mcp4725.h
@@ -0,0 +1,16 @@
+/*
+ * MCP4725 DAC driver
+ *
+ * Copyright (C) 2012 Peter Meerwald <pmeerw@pmeerw.net>
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#ifndef IIO_DAC_MCP4725_H_
+#define IIO_DAC_MCP4725_H_
+
+struct mcp4725_platform_data {
+	u16 vref_mv;
+};
+
+#endif /* IIO_DAC_MCP4725_H_ */
-- 
1.7.9.5

Re: [PATCH] iio: add mcp4725 I2C DAC driver

[Greg KH]

On Fri, Jun 08, 2012 at 06:06:45PM +0200, Peter Meerwald wrote:
> v5:
> * fix warnings (Jonathan Cameron)
> 
> v4:
> * remove unused indio_dev pointer in mcp4725_data (Jonathan Cameron)
> * use u16 instead of unsigned short in mcp4725_data (Jonathan Cameron)
> * #include mcp4725.h from linux/iio/dac/

Shouldn't that file be in include/linux/platform_data/ instead?  It
really doesn't have anything to do with iio specifically.

Oh, nevermind, all the other drivers put their platform data in that
directory already, so I guess it's ok.  But it might be nice to move all
of them out of there soon...

thanks,

greg k-h

[PATCH] iio: add mcp4725 I2C DAC driver

[Peter Meerwald]

v5:
* fix warnings (Jonathan Cameron)

v4:
* remove unused indio_dev pointer in mcp4725_data (Jonathan Cameron)
* use u16 instead of unsigned short in mcp4725_data (Jonathan Cameron)
* #include mcp4725.h from linux/iio/dac/

v3:
* move from staging to drivers/iio
* switch to chan_spec
* dev_get_drvdata() -> dev_to_iio_dev()
* annotate probe() and remove() with __devinit and __devexit

v2 (based on comments from Jonathan Cameron and Lars-Peter Clausen):
* did NOT switch to chan_spec yet
* rebase to staging-next tree, update iio header locations
* dropped dac.h #include, not needed
* strict_strtol() -> kstrtol()
* call iio_device_unregister() in remove()
* everything in one patch

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/iio/dac/Kconfig         |   11 ++
 drivers/iio/dac/Makefile        |    1 +
 drivers/iio/dac/mcp4725.c       |  227 +++++++++++++++++++++++++++++++++++++++
 include/linux/iio/dac/mcp4725.h |   16 +++
 4 files changed, 255 insertions(+)
 create mode 100644 drivers/iio/dac/mcp4725.c
 create mode 100644 include/linux/iio/dac/mcp4725.h

diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index a626f03..92fb3a0 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -118,4 +118,15 @@ config MAX517
 	  This driver can also be built as a module.  If so, the module
 	  will be called max517.
 
+config MCP4725
+	tristate "MCP4725 DAC driver"
+	depends on I2C
+	---help---
+	  Say Y here if you want to build a driver for the Microchip
+	  MCP 4725 12-bit digital-to-analog converter (DAC) with I2C
+	  interface.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called mcp4725.
+
 endmenu
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 8ab1d26..9ea3cee 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_AD5764) += ad5764.o
 obj-$(CONFIG_AD5791) += ad5791.o
 obj-$(CONFIG_AD5686) += ad5686.o
 obj-$(CONFIG_MAX517) += max517.o
+obj-$(CONFIG_MCP4725) += mcp4725.o
diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
new file mode 100644
index 0000000..e0e168b
--- /dev/null
+++ b/drivers/iio/dac/mcp4725.c
@@ -0,0 +1,227 @@
+/*
+ * mcp4725.c - Support for Microchip MCP4725
+ *
+ * Copyright (C) 2012 Peter Meerwald <pmeerw@pmeerw.net>
+ *
+ * Based on max517 by Roland Stigge <stigge@antcom.de>
+ *
+ * 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 I2C 12-bit digital-to-analog converter (DAC)
+ * (7-bit I2C slave address 0x60, the three LSBs can be configured in
+ * hardware)
+ *
+ * writing the DAC value to EEPROM is not supported
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#include <linux/iio/dac/mcp4725.h>
+
+#define MCP4725_DRV_NAME "mcp4725"
+
+struct mcp4725_data {
+	struct i2c_client *client;
+	u16 vref_mv;
+	u16 dac_value;
+};
+
+#ifdef CONFIG_PM_SLEEP
+static int mcp4725_suspend(struct device *dev)
+{
+	u8 outbuf[2];
+
+	outbuf[0] = 0x3 << 4; /* power-down bits, 500 kOhm resistor */
+	outbuf[1] = 0;
+
+	return i2c_master_send(to_i2c_client(dev), outbuf, 2);
+}
+
+static int mcp4725_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct mcp4725_data *data = iio_priv(indio_dev);
+	u8 outbuf[2];
+
+	/* restore previous DAC value */
+	outbuf[0] = (data->dac_value >> 8) & 0xf;
+	outbuf[1] = data->dac_value & 0xff;
+
+	return i2c_master_send(to_i2c_client(dev), outbuf, 2);
+}
+
+static SIMPLE_DEV_PM_OPS(mcp4725_pm_ops, mcp4725_suspend, mcp4725_resume);
+#define MCP4725_PM_OPS (&mcp4725_pm_ops)
+#else
+#define MCP4725_PM_OPS NULL
+#endif
+
+static const struct iio_chan_spec mcp4725_channel = {
+	.type		= IIO_VOLTAGE,
+	.indexed	= 1,
+	.output		= 1,
+	.channel	= 0,
+	.info_mask	= IIO_CHAN_INFO_RAW_SEPARATE_BIT |
+			  IIO_CHAN_INFO_SCALE_SHARED_BIT,
+	.scan_type	= IIO_ST('u', 12, 16, 0),
+};
+
+static int mcp4725_set_value(struct iio_dev *indio_dev, int val)
+{
+	struct mcp4725_data *data = iio_priv(indio_dev);
+	u8 outbuf[2];
+	int ret;
+
+	if (val >= (1 << 12) || val < 0)
+		return -EINVAL;
+
+	outbuf[0] = (val >> 8) & 0xf;
+	outbuf[1] = val & 0xff;
+
+	ret = i2c_master_send(data->client, outbuf, 2);
+	if (ret < 0)
+		return ret;
+	else if (ret != 2)
+		return -EIO;
+	else
+		return 0;
+}
+
+static int mcp4725_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct mcp4725_data *data = iio_priv(indio_dev);
+	unsigned long scale_uv;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		*val = data->dac_value;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		scale_uv = (data->vref_mv * 1000) >> 12;
+		*val =  scale_uv / 1000000;
+		*val2 = scale_uv % 1000000;
+		return IIO_VAL_INT_PLUS_MICRO;
+	}
+	return -EINVAL;
+}
+
+static int mcp4725_write_raw(struct iio_dev *indio_dev,
+			       struct iio_chan_spec const *chan,
+			       int val, int val2, long mask)
+{
+	struct mcp4725_data *data = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = mcp4725_set_value(indio_dev, val);
+		data->dac_value = val;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+static const struct iio_info mcp4725_info = {
+	.read_raw = mcp4725_read_raw,
+	.write_raw = mcp4725_write_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static int __devinit mcp4725_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct mcp4725_data *data;
+	struct iio_dev *indio_dev;
+	struct mcp4725_platform_data *platform_data = client->dev.platform_data;
+	u8 inbuf[3];
+	int err;
+
+	if (!platform_data || !platform_data->vref_mv) {
+		dev_err(&client->dev, "invalid platform data");
+		err = -EINVAL;
+		goto exit;
+	}
+
+	indio_dev = iio_device_alloc(sizeof(*data));
+	if (indio_dev == NULL) {
+		err = -ENOMEM;
+		goto exit;
+	}
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &mcp4725_info;
+	indio_dev->channels = &mcp4725_channel;
+	indio_dev->num_channels = 1;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	data->vref_mv = platform_data->vref_mv;
+
+	/* read current DAC value */
+	err = i2c_master_recv(client, inbuf, 3);
+	if (err < 0) {
+		dev_err(&client->dev, "failed to read DAC value");
+		goto exit_free_device;
+	}
+	data->dac_value = (inbuf[1] << 4) | (inbuf[2] >> 4);
+
+	err = iio_device_register(indio_dev);
+	if (err)
+		goto exit_free_device;
+
+	dev_info(&client->dev, "MCP4725 DAC registered\n");
+
+	return 0;
+
+exit_free_device:
+	iio_device_free(indio_dev);
+exit:
+	return err;
+}
+
+static int __devexit mcp4725_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+
+	iio_device_unregister(indio_dev);
+	iio_device_free(indio_dev);
+
+	return 0;
+}
+
+static const struct i2c_device_id mcp4725_id[] = {
+	{ "mcp4725", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, mcp4725_id);
+
+static struct i2c_driver mcp4725_driver = {
+	.driver = {
+		.name	= MCP4725_DRV_NAME,
+		.pm	= MCP4725_PM_OPS,
+	},
+	.probe		= mcp4725_probe,
+	.remove		= __devexit_p(mcp4725_remove),
+	.id_table	= mcp4725_id,
+};
+module_i2c_driver(mcp4725_driver);
+
+MODULE_AUTHOR("Peter Meerwald <pmeerw@pmeerw.net>");
+MODULE_DESCRIPTION("MCP4725 12-bit DAC");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/iio/dac/mcp4725.h b/include/linux/iio/dac/mcp4725.h
new file mode 100644
index 0000000..91530e6
--- /dev/null
+++ b/include/linux/iio/dac/mcp4725.h
@@ -0,0 +1,16 @@
+/*
+ * MCP4725 DAC driver
+ *
+ * Copyright (C) 2012 Peter Meerwald <pmeerw@pmeerw.net>
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#ifndef IIO_DAC_MCP4725_H_
+#define IIO_DAC_MCP4725_H_
+
+struct mcp4725_platform_data {
+	u16 vref_mv;
+};
+
+#endif /* IIO_DAC_MCP4725_H_ */
-- 
1.7.9.5

Re: [PATCH] iio: add mcp4725 I2C DAC driver

[Jonathan Cameron]

On 6/8/2012 8:07 AM, Peter Meerwald wrote:
> v5:
> * fix warnings (Jonathan Cameron)
>
> v4:
> * remove unused indio_dev pointer in mcp4725_data (Jonathan Cameron)
> * use u16 instead of unsigned short in mcp4725_data (Jonathan Cameron)
> * #include mcp4725.h from linux/iio/dac/
>
> v3:
> * move from staging to drivers/iio
> * switch to chan_spec
> * dev_get_drvdata() ->  dev_to_iio_dev()
> * annotate probe() and remove() with __devinit and __devexit
>
> v2 (based on comments from Jonathan Cameron and Lars-Peter Clausen):
> * did NOT switch to chan_spec yet
> * rebase to staging-next tree, update iio header locations
> * dropped dac.h #include, not needed
> * strict_strtol() ->  kstrtol()
> * call iio_device_unregister() in remove()
> * everything in one patch
>
> Signed-off-by: Peter Meerwald<pmeerw@pmeerw.net>
Acked-by: Jonathan Cameron <jic23@kernel.org>

Please send on to Greg KH GregKH@Linuxfoundation.org for merging
as he's kindly continuing to handle that side of IIO at the moment.

Just as a small aside for future patches. It's really helpful to edit
the patch title to be
[PATCH V5] iio:...
That way it's easy to be sure one has the latest version when 
reviewing/merging.

Jonathan
> ---
>   drivers/iio/dac/Kconfig         |   11 ++
>   drivers/iio/dac/Makefile        |    1 +
>   drivers/iio/dac/mcp4725.c       |  227 +++++++++++++++++++++++++++++++++++++++
>   include/linux/iio/dac/mcp4725.h |   16 +++
>   4 files changed, 255 insertions(+)
>   create mode 100644 drivers/iio/dac/mcp4725.c
>   create mode 100644 include/linux/iio/dac/mcp4725.h
>
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index a626f03..92fb3a0 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -118,4 +118,15 @@ config MAX517
>   	  This driver can also be built as a module.  If so, the module
>   	  will be called max517.
>
> +config MCP4725
> +	tristate "MCP4725 DAC driver"
> +	depends on I2C
> +	---help---
> +	  Say Y here if you want to build a driver for the Microchip
> +	  MCP 4725 12-bit digital-to-analog converter (DAC) with I2C
> +	  interface.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called mcp4725.
> +
>   endmenu
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 8ab1d26..9ea3cee 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -13,3 +13,4 @@ obj-$(CONFIG_AD5764) += ad5764.o
>   obj-$(CONFIG_AD5791) += ad5791.o
>   obj-$(CONFIG_AD5686) += ad5686.o
>   obj-$(CONFIG_MAX517) += max517.o
> +obj-$(CONFIG_MCP4725) += mcp4725.o
> diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
> new file mode 100644
> index 0000000..e0e168b
> --- /dev/null
> +++ b/drivers/iio/dac/mcp4725.c
> @@ -0,0 +1,227 @@
> +/*
> + * mcp4725.c - Support for Microchip MCP4725
> + *
> + * Copyright (C) 2012 Peter Meerwald<pmeerw@pmeerw.net>
> + *
> + * Based on max517 by Roland Stigge<stigge@antcom.de>
> + *
> + * 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 I2C 12-bit digital-to-analog converter (DAC)
> + * (7-bit I2C slave address 0x60, the three LSBs can be configured in
> + * hardware)
> + *
> + * writing the DAC value to EEPROM is not supported
> + */
> +
> +#include<linux/module.h>
> +#include<linux/init.h>
> +#include<linux/i2c.h>
> +#include<linux/err.h>
> +
> +#include<linux/iio/iio.h>
> +#include<linux/iio/sysfs.h>
> +
> +#include<linux/iio/dac/mcp4725.h>
> +
> +#define MCP4725_DRV_NAME "mcp4725"
> +
> +struct mcp4725_data {
> +	struct i2c_client *client;
> +	u16 vref_mv;
> +	u16 dac_value;
> +};
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int mcp4725_suspend(struct device *dev)
> +{
> +	u8 outbuf[2];
> +
> +	outbuf[0] = 0x3<<  4; /* power-down bits, 500 kOhm resistor */
> +	outbuf[1] = 0;
> +
> +	return i2c_master_send(to_i2c_client(dev), outbuf, 2);
> +}
> +
> +static int mcp4725_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct mcp4725_data *data = iio_priv(indio_dev);
> +	u8 outbuf[2];
> +
> +	/* restore previous DAC value */
> +	outbuf[0] = (data->dac_value>>  8)&  0xf;
> +	outbuf[1] = data->dac_value&  0xff;
> +
> +	return i2c_master_send(to_i2c_client(dev), outbuf, 2);
> +}
> +
> +static SIMPLE_DEV_PM_OPS(mcp4725_pm_ops, mcp4725_suspend, mcp4725_resume);
> +#define MCP4725_PM_OPS (&mcp4725_pm_ops)
> +#else
> +#define MCP4725_PM_OPS NULL
> +#endif
> +
> +static const struct iio_chan_spec mcp4725_channel = {
> +	.type		= IIO_VOLTAGE,
> +	.indexed	= 1,
> +	.output		= 1,
> +	.channel	= 0,
> +	.info_mask	= IIO_CHAN_INFO_RAW_SEPARATE_BIT |
> +			  IIO_CHAN_INFO_SCALE_SHARED_BIT,
> +	.scan_type	= IIO_ST('u', 12, 16, 0),
> +};
> +
> +static int mcp4725_set_value(struct iio_dev *indio_dev, int val)
> +{
> +	struct mcp4725_data *data = iio_priv(indio_dev);
> +	u8 outbuf[2];
> +	int ret;
> +
> +	if (val>= (1<<  12) || val<  0)
> +		return -EINVAL;
> +
> +	outbuf[0] = (val>>  8)&  0xf;
> +	outbuf[1] = val&  0xff;
> +
> +	ret = i2c_master_send(data->client, outbuf, 2);
> +	if (ret<  0)
> +		return ret;
> +	else if (ret != 2)
> +		return -EIO;
> +	else
> +		return 0;
> +}
> +
> +static int mcp4725_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long mask)
> +{
> +	struct mcp4725_data *data = iio_priv(indio_dev);
> +	unsigned long scale_uv;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		*val = data->dac_value;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		scale_uv = (data->vref_mv * 1000)>>  12;
> +		*val =  scale_uv / 1000000;
> +		*val2 = scale_uv % 1000000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	}
> +	return -EINVAL;
> +}
> +
> +static int mcp4725_write_raw(struct iio_dev *indio_dev,
> +			       struct iio_chan_spec const *chan,
> +			       int val, int val2, long mask)
> +{
> +	struct mcp4725_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = mcp4725_set_value(indio_dev, val);
> +		data->dac_value = val;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct iio_info mcp4725_info = {
> +	.read_raw = mcp4725_read_raw,
> +	.write_raw = mcp4725_write_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static int __devinit mcp4725_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	struct mcp4725_data *data;
> +	struct iio_dev *indio_dev;
> +	struct mcp4725_platform_data *platform_data = client->dev.platform_data;
> +	u8 inbuf[3];
> +	int err;
> +
> +	if (!platform_data || !platform_data->vref_mv) {
> +		dev_err(&client->dev, "invalid platform data");
> +		err = -EINVAL;
> +		goto exit;
> +	}
> +
> +	indio_dev = iio_device_alloc(sizeof(*data));
> +	if (indio_dev == NULL) {
> +		err = -ENOMEM;
> +		goto exit;
> +	}
> +	data = iio_priv(indio_dev);
> +	i2c_set_clientdata(client, indio_dev);
> +	data->client = client;
> +
> +	indio_dev->dev.parent =&client->dev;
> +	indio_dev->info =&mcp4725_info;
> +	indio_dev->channels =&mcp4725_channel;
> +	indio_dev->num_channels = 1;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	data->vref_mv = platform_data->vref_mv;
> +
> +	/* read current DAC value */
> +	err = i2c_master_recv(client, inbuf, 3);
> +	if (err<  0) {
> +		dev_err(&client->dev, "failed to read DAC value");
> +		goto exit_free_device;
> +	}
> +	data->dac_value = (inbuf[1]<<  4) | (inbuf[2]>>  4);
> +
> +	err = iio_device_register(indio_dev);
> +	if (err)
> +		goto exit_free_device;
> +
> +	dev_info(&client->dev, "MCP4725 DAC registered\n");
> +
> +	return 0;
> +
> +exit_free_device:
> +	iio_device_free(indio_dev);
> +exit:
> +	return err;
> +}
> +
> +static int __devexit mcp4725_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +
> +	iio_device_unregister(indio_dev);
> +	iio_device_free(indio_dev);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id mcp4725_id[] = {
> +	{ "mcp4725", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, mcp4725_id);
> +
> +static struct i2c_driver mcp4725_driver = {
> +	.driver = {
> +		.name	= MCP4725_DRV_NAME,
> +		.pm	= MCP4725_PM_OPS,
> +	},
> +	.probe		= mcp4725_probe,
> +	.remove		= __devexit_p(mcp4725_remove),
> +	.id_table	= mcp4725_id,
> +};
> +module_i2c_driver(mcp4725_driver);
> +
> +MODULE_AUTHOR("Peter Meerwald<pmeerw@pmeerw.net>");
> +MODULE_DESCRIPTION("MCP4725 12-bit DAC");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/iio/dac/mcp4725.h b/include/linux/iio/dac/mcp4725.h
> new file mode 100644
> index 0000000..91530e6
> --- /dev/null
> +++ b/include/linux/iio/dac/mcp4725.h
> @@ -0,0 +1,16 @@
> +/*
> + * MCP4725 DAC driver
> + *
> + * Copyright (C) 2012 Peter Meerwald<pmeerw@pmeerw.net>
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#ifndef IIO_DAC_MCP4725_H_
> +#define IIO_DAC_MCP4725_H_
> +
> +struct mcp4725_platform_data {
> +	u16 vref_mv;
> +};
> +
> +#endif /* IIO_DAC_MCP4725_H_ */

[PATCH] iio: add mcp4725 I2C DAC driver

[Peter Meerwald]

v3:
* move from staging to drivers/iio
* switch to chan_spec
* dev_get_drvdata() -> dev_to_iio_dev()
* annotate probe() and remove() with __devinit and __devexit

v2 (based on comments from Jonathan Cameron and Lars-Peter Clausen):
* did NOT switch to chan_spec yet
* rebase to staging-next tree, update iio header locations
* dropped dac.h #include, not needed
* strict_strtol() -> kstrtol()
* call iio_device_unregister() in remove()
* everything in one patch

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/iio/dac/Kconfig         |   11 ++
 drivers/iio/dac/Makefile        |    1 +
 drivers/iio/dac/mcp4725.c       |  228 +++++++++++++++++++++++++++++++++++++++
 include/linux/iio/dac/mcp4725.h |   16 +++
 4 files changed, 256 insertions(+)
 create mode 100644 drivers/iio/dac/mcp4725.c
 create mode 100644 include/linux/iio/dac/mcp4725.h

diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index a626f03..92fb3a0 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -118,4 +118,15 @@ config MAX517
 	  This driver can also be built as a module.  If so, the module
 	  will be called max517.
 
+config MCP4725
+	tristate "MCP4725 DAC driver"
+	depends on I2C
+	---help---
+	  Say Y here if you want to build a driver for the Microchip
+	  MCP 4725 12-bit digital-to-analog converter (DAC) with I2C
+	  interface.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called mcp4725.
+
 endmenu
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 8ab1d26..9ea3cee 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_AD5764) += ad5764.o
 obj-$(CONFIG_AD5791) += ad5791.o
 obj-$(CONFIG_AD5686) += ad5686.o
 obj-$(CONFIG_MAX517) += max517.o
+obj-$(CONFIG_MCP4725) += mcp4725.o
diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
new file mode 100644
index 0000000..244ccf6
--- /dev/null
+++ b/drivers/iio/dac/mcp4725.c
@@ -0,0 +1,228 @@
+/*
+ * mcp4725.c - Support for Microchip MCP4725
+ *
+ * Copyright (C) 2012 Peter Meerwald <pmeerw@pmeerw.net>
+ *
+ * Based on max517 by Roland Stigge <stigge@antcom.de>
+ *
+ * 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 I2C 12-bit digital-to-analog converter (DAC)
+ * (7-bit I2C slave address 0x60, the three LSBs can be configured in
+ * hardware)
+ *
+ * writing the DAC value to EEPROM is not supported
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#include "mcp4725.h"
+
+#define MCP4725_DRV_NAME "mcp4725"
+
+struct mcp4725_data {
+	struct iio_dev *indio_dev;
+	struct i2c_client *client;
+	unsigned short vref_mv;
+	unsigned short dac_value;
+};
+
+#ifdef CONFIG_PM_SLEEP
+static int mcp4725_suspend(struct device *dev)
+{
+	u8 outbuf[2];
+
+	outbuf[0] = 0x3 << 4; /* power-down bits, 500 kOhm resistor */
+	outbuf[1] = 0;
+
+	return i2c_master_send(to_i2c_client(dev), &outbuf, 2);
+}
+
+static int mcp4725_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct mcp4725_data *data = iio_priv(indio_dev);
+	u8 outbuf[2];
+
+	/* restore previous DAC value */
+	outbuf[0] = (data->dac_value >> 8) & 0xf;
+	outbuf[1] = data->dac_value & 0xff;
+
+	return i2c_master_send(to_i2c_client(dev), &outbuf, 2);
+}
+
+static SIMPLE_DEV_PM_OPS(mcp4725_pm_ops, mcp4725_suspend, mcp4725_resume);
+#define MCP4725_PM_OPS (&mcp4725_pm_ops)
+#else
+#define MCP4725_PM_OPS NULL
+#endif
+
+static const struct iio_chan_spec mcp4725_channel = {
+	.type		= IIO_VOLTAGE,
+	.indexed	= 1,
+	.output		= 1,
+	.channel	= 0,
+	.info_mask	= IIO_CHAN_INFO_RAW_SEPARATE_BIT |
+			  IIO_CHAN_INFO_SCALE_SHARED_BIT,
+	.scan_type	= IIO_ST('u', 12, 16, 0),
+};
+
+static int mcp4725_set_value(struct iio_dev *indio_dev, int val)
+{
+	struct mcp4725_data *data = iio_priv(indio_dev);
+	u8 outbuf[2];
+	int ret;
+
+	if (val >= (1 << 12) || val < 0)
+		return -EINVAL;
+
+	outbuf[0] = (val >> 8) & 0xf;
+	outbuf[1] = val & 0xff;
+
+	ret = i2c_master_send(data->client, outbuf, 2);
+	if (ret < 0)
+		return ret;
+	else if (ret != 2)
+		return -EIO;
+	else
+		return 0;
+}
+
+static int mcp4725_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct mcp4725_data *data = iio_priv(indio_dev);
+	unsigned long scale_uv;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		*val = data->dac_value;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		scale_uv = (data->vref_mv * 1000) >> 12;
+		*val =  scale_uv / 1000000;
+		*val2 = scale_uv % 1000000;
+		return IIO_VAL_INT_PLUS_MICRO;
+	}
+	return -EINVAL;
+}
+
+static int mcp4725_write_raw(struct iio_dev *indio_dev,
+			       struct iio_chan_spec const *chan,
+			       int val, int val2, long mask)
+{
+	struct mcp4725_data *data = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = mcp4725_set_value(indio_dev, val);
+		data->dac_value = val;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+static const struct iio_info mcp4725_info = {
+	.read_raw = mcp4725_read_raw,
+	.write_raw = mcp4725_write_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static int __devinit mcp4725_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct mcp4725_data *data;
+	struct iio_dev *indio_dev;
+	struct mcp4725_platform_data *platform_data = client->dev.platform_data;
+	u8 inbuf[3];
+	int err;
+
+	if (!platform_data || !platform_data->vref_mv) {
+		dev_err(&client->dev, "invalid platform data");
+		err = -EINVAL;
+		goto exit;
+	}
+
+	indio_dev = iio_device_alloc(sizeof(*data));
+	if (indio_dev == NULL) {
+		err = -ENOMEM;
+		goto exit;
+	}
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &mcp4725_info;
+	indio_dev->channels = &mcp4725_channel;
+	indio_dev->num_channels = 1;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	data->vref_mv = platform_data->vref_mv;
+
+	/* read current DAC value */
+	err = i2c_master_recv(client, inbuf, 3);
+	if (err < 0) {
+		dev_err(&client->dev, "failed to read DAC value");
+		goto exit_free_device;
+	}
+	data->dac_value = (inbuf[1] << 4) | (inbuf[2] >> 4);
+
+	err = iio_device_register(indio_dev);
+	if (err)
+		goto exit_free_device;
+
+	dev_info(&client->dev, "MCP4725 DAC registered\n");
+
+	return 0;
+
+exit_free_device:
+	iio_device_free(indio_dev);
+exit:
+	return err;
+}
+
+static int __devexit mcp4725_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+
+	iio_device_unregister(indio_dev);
+	iio_device_free(indio_dev);
+
+	return 0;
+}
+
+static const struct i2c_device_id mcp4725_id[] = {
+	{ "mcp4725", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, mcp4725_id);
+
+static struct i2c_driver mcp4725_driver = {
+	.driver = {
+		.name	= MCP4725_DRV_NAME,
+		.pm	= MCP4725_PM_OPS,
+	},
+	.probe		= mcp4725_probe,
+	.remove		= __devexit_p(mcp4725_remove),
+	.id_table	= mcp4725_id,
+};
+module_i2c_driver(mcp4725_driver);
+
+MODULE_AUTHOR("Peter Meerwald <pmeerw@pmeerw.net>");
+MODULE_DESCRIPTION("MCP4725 12-bit DAC");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/iio/dac/mcp4725.h b/include/linux/iio/dac/mcp4725.h
new file mode 100644
index 0000000..91530e6
--- /dev/null
+++ b/include/linux/iio/dac/mcp4725.h
@@ -0,0 +1,16 @@
+/*
+ * MCP4725 DAC driver
+ *
+ * Copyright (C) 2012 Peter Meerwald <pmeerw@pmeerw.net>
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#ifndef IIO_DAC_MCP4725_H_
+#define IIO_DAC_MCP4725_H_
+
+struct mcp4725_platform_data {
+	u16 vref_mv;
+};
+
+#endif /* IIO_DAC_MCP4725_H_ */
-- 
1.7.9.5

Re: [PATCH] iio: add mcp4725 I2C DAC driver

[Jonathan Cameron]

On 6/6/2012 10:40 PM, Peter Meerwald wrote:
> v3:
> * move from staging to drivers/iio
> * switch to chan_spec
> * dev_get_drvdata() ->  dev_to_iio_dev()
> * annotate probe() and remove() with __devinit and __devexit
>
> v2 (based on comments from Jonathan Cameron and Lars-Peter Clausen):
> * did NOT switch to chan_spec yet
> * rebase to staging-next tree, update iio header locations
> * dropped dac.h #include, not needed
> * strict_strtol() ->  kstrtol()
> * call iio_device_unregister() in remove()
> * everything in one patch

That's what I like.  A driver short enough to review whilst having a 
coffee before starting work at the day job ;)

Couple of comments. Only important one is I don't think that
the iio_dev pointer in your *_data structure is used anywhere...

Jonathan
>
> Signed-off-by: Peter Meerwald<pmeerw@pmeerw.net>
> ---
>   drivers/iio/dac/Kconfig         |   11 ++
>   drivers/iio/dac/Makefile        |    1 +
>   drivers/iio/dac/mcp4725.c       |  228 +++++++++++++++++++++++++++++++++++++++
>   include/linux/iio/dac/mcp4725.h |   16 +++
>   4 files changed, 256 insertions(+)
>   create mode 100644 drivers/iio/dac/mcp4725.c
>   create mode 100644 include/linux/iio/dac/mcp4725.h
>
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index a626f03..92fb3a0 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -118,4 +118,15 @@ config MAX517
>   	  This driver can also be built as a module.  If so, the module
>   	  will be called max517.
>
> +config MCP4725
> +	tristate "MCP4725 DAC driver"
> +	depends on I2C
> +	---help---
> +	  Say Y here if you want to build a driver for the Microchip
> +	  MCP 4725 12-bit digital-to-analog converter (DAC) with I2C
> +	  interface.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called mcp4725.
> +
>   endmenu
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 8ab1d26..9ea3cee 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -13,3 +13,4 @@ obj-$(CONFIG_AD5764) += ad5764.o
>   obj-$(CONFIG_AD5791) += ad5791.o
>   obj-$(CONFIG_AD5686) += ad5686.o
>   obj-$(CONFIG_MAX517) += max517.o
> +obj-$(CONFIG_MCP4725) += mcp4725.o
> diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
> new file mode 100644
> index 0000000..244ccf6
> --- /dev/null
> +++ b/drivers/iio/dac/mcp4725.c
> @@ -0,0 +1,228 @@
> +/*
> + * mcp4725.c - Support for Microchip MCP4725
> + *
> + * Copyright (C) 2012 Peter Meerwald<pmeerw@pmeerw.net>
> + *
> + * Based on max517 by Roland Stigge<stigge@antcom.de>
> + *
> + * 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 I2C 12-bit digital-to-analog converter (DAC)
> + * (7-bit I2C slave address 0x60, the three LSBs can be configured in
> + * hardware)
> + *
> + * writing the DAC value to EEPROM is not supported
> + */
> +
> +#include<linux/module.h>
> +#include<linux/init.h>
> +#include<linux/i2c.h>
> +#include<linux/err.h>
> +
> +#include<linux/iio/iio.h>
> +#include<linux/iio/sysfs.h>
> +
> +#include "mcp4725.h"
> +
> +#define MCP4725_DRV_NAME "mcp4725"
> +
> +struct mcp4725_data {
> +	struct iio_dev *indio_dev;
I can't immediately see whta this iio_dev pointer is for?
Don't think it is even set anywhere.
> +	struct i2c_client *client;
> +	unsigned short vref_mv;
> +	unsigned short dac_value;
Rather than a short, I'd use a defined length type
u16 probably. Doesn't matter much though.
> +};
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int mcp4725_suspend(struct device *dev)
> +{
> +	u8 outbuf[2];
> +
> +	outbuf[0] = 0x3<<  4; /* power-down bits, 500 kOhm resistor */
> +	outbuf[1] = 0;
> +
> +	return i2c_master_send(to_i2c_client(dev),&outbuf, 2);
> +}
> +
> +static int mcp4725_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct mcp4725_data *data = iio_priv(indio_dev);
> +	u8 outbuf[2];
> +
> +	/* restore previous DAC value */
> +	outbuf[0] = (data->dac_value>>  8)&  0xf;
> +	outbuf[1] = data->dac_value&  0xff;
> +
> +	return i2c_master_send(to_i2c_client(dev),&outbuf, 2);
> +}
> +
> +static SIMPLE_DEV_PM_OPS(mcp4725_pm_ops, mcp4725_suspend, mcp4725_resume);
> +#define MCP4725_PM_OPS (&mcp4725_pm_ops)
> +#else
> +#define MCP4725_PM_OPS NULL
> +#endif
> +
> +static const struct iio_chan_spec mcp4725_channel = {
> +	.type		= IIO_VOLTAGE,
> +	.indexed	= 1,
> +	.output		= 1,
> +	.channel	= 0,
> +	.info_mask	= IIO_CHAN_INFO_RAW_SEPARATE_BIT |
> +			  IIO_CHAN_INFO_SCALE_SHARED_BIT,
> +	.scan_type	= IIO_ST('u', 12, 16, 0),
Arguably this acts as documentation, but personally I'd not bother
specifying scan_type as output drivers don't currently use them.
> +};
> +
> +static int mcp4725_set_value(struct iio_dev *indio_dev, int val)
> +{
> +	struct mcp4725_data *data = iio_priv(indio_dev);
> +	u8 outbuf[2];
> +	int ret;
> +
> +	if (val>= (1<<  12) || val<  0)
> +		return -EINVAL;
> +
> +	outbuf[0] = (val>>  8)&  0xf;
> +	outbuf[1] = val&  0xff;
> +
> +	ret = i2c_master_send(data->client, outbuf, 2);
> +	if (ret<  0)
> +		return ret;
> +	else if (ret != 2)
> +		return -EIO;
> +	else
> +		return 0;
> +}
> +
> +static int mcp4725_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long mask)
> +{
> +	struct mcp4725_data *data = iio_priv(indio_dev);
> +	unsigned long scale_uv;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		*val = data->dac_value;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		scale_uv = (data->vref_mv * 1000)>>  12;
> +		*val =  scale_uv / 1000000;
> +		*val2 = scale_uv % 1000000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	}
> +	return -EINVAL;
> +}
> +
> +static int mcp4725_write_raw(struct iio_dev *indio_dev,
> +			       struct iio_chan_spec const *chan,
> +			       int val, int val2, long mask)
> +{
> +	struct mcp4725_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = mcp4725_set_value(indio_dev, val);
Convention would be to probably not update the cache on a
failure. But then if we get a failiure the state is unknown
anyway so what the heck...
> +		data->dac_value = val;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct iio_info mcp4725_info = {
> +	.read_raw = mcp4725_read_raw,
> +	.write_raw = mcp4725_write_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static int __devinit mcp4725_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	struct mcp4725_data *data;
> +	struct iio_dev *indio_dev;
> +	struct mcp4725_platform_data *platform_data = client->dev.platform_data;
> +	u8 inbuf[3];
> +	int err;
> +
> +	if (!platform_data || !platform_data->vref_mv) {
> +		dev_err(&client->dev, "invalid platform data");
> +		err = -EINVAL;
> +		goto exit;
> +	}
> +
> +	indio_dev = iio_device_alloc(sizeof(*data));
> +	if (indio_dev == NULL) {
> +		err = -ENOMEM;
> +		goto exit;
> +	}
> +	data = iio_priv(indio_dev);
> +	i2c_set_clientdata(client, indio_dev);
> +	data->client = client;
> +
> +	indio_dev->dev.parent =&client->dev;
> +	indio_dev->info =&mcp4725_info;
> +	indio_dev->channels =&mcp4725_channel;
> +	indio_dev->num_channels = 1;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	data->vref_mv = platform_data->vref_mv;
> +
> +	/* read current DAC value */
> +	err = i2c_master_recv(client, inbuf, 3);
> +	if (err<  0) {
> +		dev_err(&client->dev, "failed to read DAC value");
> +		goto exit_free_device;
> +	}
> +	data->dac_value = (inbuf[1]<<  4) | (inbuf[2]>>  4);
> +
> +	err = iio_device_register(indio_dev);
> +	if (err)
> +		goto exit_free_device;
> +
> +	dev_info(&client->dev, "MCP4725 DAC registered\n");
> +
> +	return 0;
> +
> +exit_free_device:
> +	iio_device_free(indio_dev);
> +exit:
> +	return err;
> +}
> +
> +static int __devexit mcp4725_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +
> +	iio_device_unregister(indio_dev);
> +	iio_device_free(indio_dev);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id mcp4725_id[] = {
> +	{ "mcp4725", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, mcp4725_id);
> +
> +static struct i2c_driver mcp4725_driver = {
> +	.driver = {
> +		.name	= MCP4725_DRV_NAME,
> +		.pm	= MCP4725_PM_OPS,
> +	},
> +	.probe		= mcp4725_probe,
> +	.remove		= __devexit_p(mcp4725_remove),
> +	.id_table	= mcp4725_id,
> +};
> +module_i2c_driver(mcp4725_driver);
> +
> +MODULE_AUTHOR("Peter Meerwald<pmeerw@pmeerw.net>");
> +MODULE_DESCRIPTION("MCP4725 12-bit DAC");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/iio/dac/mcp4725.h b/include/linux/iio/dac/mcp4725.h
> new file mode 100644
> index 0000000..91530e6
> --- /dev/null
> +++ b/include/linux/iio/dac/mcp4725.h
> @@ -0,0 +1,16 @@
> +/*
> + * MCP4725 DAC driver
> + *
> + * Copyright (C) 2012 Peter Meerwald<pmeerw@pmeerw.net>
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#ifndef IIO_DAC_MCP4725_H_
> +#define IIO_DAC_MCP4725_H_
> +
> +struct mcp4725_platform_data {
> +	u16 vref_mv;
> +};
> +
> +#endif /* IIO_DAC_MCP4725_H_ */

Re: [PATCH 2/2] iio: add mcp4725 I2C DAC driver

[Jonathan Cameron]

On 4/30/2012 11:18 AM, Peter Meerwald wrote:
> Hello Jonathan,
>
>> Mostly fine.   As I explain below you were a little unlucky in which driver to
>> base on.  Roland's driver is lovely and clean, but is lagging a bit in interface
>> terms.
> thank you for your comments!
> I'll see if I understand the chan_spec interface enough
>
> so the plan is to have part of iio in staging and the other part (core)
> in 3.5? and I can submit a driver to go in one or the other?
Yes. So far I've concentrated on moving the core infrastructure out of 
staging.
This was not really because anyone was happy with it all, but rather that
some chunks of code were getting stalled because the companies in question
weren't willing to have drivers for them in staging and others were 
insisting that
the driver be in IIO. The userspace abi's are stable (kernel ones no 
where near!)
so there was nothing blocking the move.
> regards, p.
>

[PATCH] iio: add mcp4725 I2C DAC driver

[Peter Meerwald]

v2 (based on comments from Jonathan Cameron and Lars-Peter Clausen):
* did NOT switch to chan_spec yet
* rebase to staging-next tree, update iio header locations
* dropped dac.h #include, not needed
* strict_strtol() -> kstrtol()
* call iio_device_unregister() in remove()
* everything in one patch

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/staging/iio/dac/Kconfig   |   11 ++
 drivers/staging/iio/dac/Makefile  |    1 +
 drivers/staging/iio/dac/mcp4725.c |  211 +++++++++++++++++++++++++++++++++++++
 drivers/staging/iio/dac/mcp4725.h |   16 +++
 4 files changed, 239 insertions(+)
 create mode 100644 drivers/staging/iio/dac/mcp4725.c
 create mode 100644 drivers/staging/iio/dac/mcp4725.h

diff --git a/drivers/staging/iio/dac/Kconfig b/drivers/staging/iio/dac/Kconfig
index a626f03..960ae06 100644
--- a/drivers/staging/iio/dac/Kconfig
+++ b/drivers/staging/iio/dac/Kconfig
@@ -118,4 +118,15 @@ config MAX517
 	  This driver can also be built as a module.  If so, the module
 	  will be called max517.
 
+config MCP4725
+	tristate "MCP4725 DAC driver"
+	depends on I2C
+	---help---
+	  Say Y here if you want to build a driver for the Microchip
+	  MCP 4725 12-bit digital-to-analog convertor (DAC) with I2C
+	  interface.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called mcp4725.
+
 endmenu
diff --git a/drivers/staging/iio/dac/Makefile b/drivers/staging/iio/dac/Makefile
index 8ab1d26..9ea3cee 100644
--- a/drivers/staging/iio/dac/Makefile
+++ b/drivers/staging/iio/dac/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_AD5764) += ad5764.o
 obj-$(CONFIG_AD5791) += ad5791.o
 obj-$(CONFIG_AD5686) += ad5686.o
 obj-$(CONFIG_MAX517) += max517.o
+obj-$(CONFIG_MCP4725) += mcp4725.o
diff --git a/drivers/staging/iio/dac/mcp4725.c b/drivers/staging/iio/dac/mcp4725.c
new file mode 100644
index 0000000..de2fc93
--- /dev/null
+++ b/drivers/staging/iio/dac/mcp4725.c
@@ -0,0 +1,211 @@
+/*
+ * mcp4725.c - Support for Microchip MCP4725
+ *
+ * Copyright (C) 2012 Peter Meerwald <pmeerw@pmeerw.net>
+ *
+ * Based on max517 by Roland Stigge <stigge@antcom.de>
+ *
+ * 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 I2C 12-bit digital-to-analog converter (DAC)
+ * (7-bit I2C slave address 0x60, the three LSBs can be configured in
+ * hardware)
+ *
+ * writing the DAC value to EEPROM is not supported
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#include "mcp4725.h"
+
+#define MCP4725_DRV_NAME	"mcp4725"
+
+struct mcp4725_data {
+	struct iio_dev		*indio_dev;
+	struct i2c_client	*client;
+	unsigned short		vref_mv;
+	unsigned short		dac_value;
+};
+
+static ssize_t mcp4725_set_value(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct mcp4725_data *data = iio_priv(indio_dev);
+	u8 outbuf[2];
+	int res;
+	long val;
+
+	res = kstrtol(buf, 10, &val);
+	if (res)
+		return res;
+
+	if (val < 0 || val > 0xfff)
+		return -EINVAL;
+
+	outbuf[0] = (val >> 8) & 0xf;
+	outbuf[1] = val & 0xff;
+
+	res = i2c_master_send(data->client, outbuf, 2);
+	if (res < 0)
+		return res;
+
+	data->dac_value = val;
+
+	return count;
+}
+
+static IIO_DEVICE_ATTR(out_voltage_raw, S_IWUSR,
+		       NULL, mcp4725_set_value, 0);
+
+static ssize_t mcp4725_show_scale(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct mcp4725_data *data = iio_priv(indio_dev);
+
+	unsigned int scale_uv = (data->vref_mv * 1000) >> 12;
+
+	return sprintf(buf, "%d.%03d\n", scale_uv / 1000, scale_uv % 1000);
+}
+
+static IIO_DEVICE_ATTR(out_voltage_scale, S_IRUGO,
+		       mcp4725_show_scale, NULL, 0);
+
+static struct attribute *mcp4725_attributes[] = {
+	&iio_dev_attr_out_voltage_raw.dev_attr.attr,
+	&iio_dev_attr_out_voltage_scale.dev_attr.attr,
+	NULL
+};
+
+static struct attribute_group mcp4725_attribute_group = {
+	.attrs = mcp4725_attributes,
+};
+
+#ifdef CONFIG_PM_SLEEP
+static int mcp4725_suspend(struct device *dev)
+{
+	u8 outbuf[2];
+
+	outbuf[0] = 0x3 << 4; /* power-down bits, 500 kOhm resistor */
+	outbuf[1] = 0;
+
+	return i2c_master_send(to_i2c_client(dev), &outbuf, 2);
+}
+
+static int mcp4725_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct mcp4725_data *data = iio_priv(indio_dev);
+	u8 outbuf[2];
+
+	/* restore previous DAC value */
+	outbuf[0] = (data->dac_value >> 8) & 0xf;
+	outbuf[1] = data->dac_value & 0xff;
+
+	return i2c_master_send(to_i2c_client(dev), &outbuf, 2);
+}
+
+static SIMPLE_DEV_PM_OPS(mcp4725_pm_ops, mcp4725_suspend, mcp4725_resume);
+#define MCP4725_PM_OPS (&mcp4725_pm_ops)
+#else
+#define MCP4725_PM_OPS NULL
+#endif
+
+static const struct iio_info mcp4725_info = {
+	.attrs = &mcp4725_attribute_group,
+	.driver_module = THIS_MODULE,
+};
+
+static int mcp4725_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct mcp4725_data *data;
+	struct iio_dev *indio_dev;
+	struct mcp4725_platform_data *platform_data = client->dev.platform_data;
+	u8 inbuf[3];
+	int err;
+
+	if (!platform_data || !platform_data->vref_mv) {
+		dev_err(&client->dev, "invalid platform data");
+		err = -EINVAL;
+		goto exit;
+	}
+
+	indio_dev = iio_device_alloc(sizeof(*data));
+	if (indio_dev == NULL) {
+		err = -ENOMEM;
+		goto exit;
+	}
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &mcp4725_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	data->vref_mv = platform_data->vref_mv;
+
+	/* read current DAC value */
+	err = i2c_master_recv(client, inbuf, 3);
+	if (err < 0) {
+		dev_err(&client->dev, "failed to read DAC value");
+		goto exit_free_device;
+	}
+	data->dac_value = (inbuf[1] << 4) | (inbuf[2] >> 4);
+
+	err = iio_device_register(indio_dev);
+	if (err)
+		goto exit_free_device;
+
+	dev_info(&client->dev, "MCP4725 DAC registered\n");
+
+	return 0;
+
+exit_free_device:
+	iio_device_free(indio_dev);
+exit:
+	return err;
+}
+
+static int mcp4725_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+
+	iio_device_unregister(indio_dev);
+	iio_device_free(indio_dev);
+
+	return 0;
+}
+
+static const struct i2c_device_id mcp4725_id[] = {
+	{ "mcp4725", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, mcp4725_id);
+
+static struct i2c_driver mcp4725_driver = {
+	.driver = {
+		.name	= MCP4725_DRV_NAME,
+		.pm	= MCP4725_PM_OPS,
+	},
+	.probe		= mcp4725_probe,
+	.remove		= mcp4725_remove,
+	.id_table	= mcp4725_id,
+};
+module_i2c_driver(mcp4725_driver);
+
+MODULE_AUTHOR("Peter Meerwald <pmeerw@pmeerw.net>");
+MODULE_DESCRIPTION("MCP4725 12-bit DAC");
+MODULE_LICENSE("GPL");
diff --git a/drivers/staging/iio/dac/mcp4725.h b/drivers/staging/iio/dac/mcp4725.h
new file mode 100644
index 0000000..91530e6
--- /dev/null
+++ b/drivers/staging/iio/dac/mcp4725.h
@@ -0,0 +1,16 @@
+/*
+ * MCP4725 DAC driver
+ *
+ * Copyright (C) 2012 Peter Meerwald <pmeerw@pmeerw.net>
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#ifndef IIO_DAC_MCP4725_H_
+#define IIO_DAC_MCP4725_H_
+
+struct mcp4725_platform_data {
+	u16 vref_mv;
+};
+
+#endif /* IIO_DAC_MCP4725_H_ */
-- 
1.7.9.5

Re: [PATCH] iio: add mcp4725 I2C DAC driver

[Jonathan Cameron]

On 4/30/2012 2:54 PM, Peter Meerwald wrote:
> v2 (based on comments from Jonathan Cameron and Lars-Peter Clausen):
> * did NOT switch to chan_spec yet
I guess from this you are intending to? I guess whether we merge this as 
is depends
on what timescale you are thinking of to make that change?
> * rebase to staging-next tree, update iio header locations
> * dropped dac.h #include, not needed
> * strict_strtol() ->  kstrtol()
> * call iio_device_unregister() in remove()
> * everything in one patch
If you want to send it on to Greg KH now and Lars-Peter is happy, then 
that's fine by me.
> Signed-off-by: Peter Meerwald<pmeerw@pmeerw.net>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>   drivers/staging/iio/dac/Kconfig   |   11 ++
>   drivers/staging/iio/dac/Makefile  |    1 +
>   drivers/staging/iio/dac/mcp4725.c |  211 +++++++++++++++++++++++++++++++++++++
>   drivers/staging/iio/dac/mcp4725.h |   16 +++
>   4 files changed, 239 insertions(+)
>   create mode 100644 drivers/staging/iio/dac/mcp4725.c
>   create mode 100644 drivers/staging/iio/dac/mcp4725.h
>
> diff --git a/drivers/staging/iio/dac/Kconfig b/drivers/staging/iio/dac/Kconfig
> index a626f03..960ae06 100644
> --- a/drivers/staging/iio/dac/Kconfig
> +++ b/drivers/staging/iio/dac/Kconfig
> @@ -118,4 +118,15 @@ config MAX517
>   	  This driver can also be built as a module.  If so, the module
>   	  will be called max517.
>
> +config MCP4725
> +	tristate "MCP4725 DAC driver"
> +	depends on I2C
> +	---help---
> +	  Say Y here if you want to build a driver for the Microchip
> +	  MCP 4725 12-bit digital-to-analog convertor (DAC) with I2C
> +	  interface.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called mcp4725.
> +
>   endmenu
> diff --git a/drivers/staging/iio/dac/Makefile b/drivers/staging/iio/dac/Makefile
> index 8ab1d26..9ea3cee 100644
> --- a/drivers/staging/iio/dac/Makefile
> +++ b/drivers/staging/iio/dac/Makefile
> @@ -13,3 +13,4 @@ obj-$(CONFIG_AD5764) += ad5764.o
>   obj-$(CONFIG_AD5791) += ad5791.o
>   obj-$(CONFIG_AD5686) += ad5686.o
>   obj-$(CONFIG_MAX517) += max517.o
> +obj-$(CONFIG_MCP4725) += mcp4725.o
> diff --git a/drivers/staging/iio/dac/mcp4725.c b/drivers/staging/iio/dac/mcp4725.c
> new file mode 100644
> index 0000000..de2fc93
> --- /dev/null
> +++ b/drivers/staging/iio/dac/mcp4725.c
> @@ -0,0 +1,211 @@
> +/*
> + * mcp4725.c - Support for Microchip MCP4725
> + *
> + * Copyright (C) 2012 Peter Meerwald<pmeerw@pmeerw.net>
> + *
> + * Based on max517 by Roland Stigge<stigge@antcom.de>
> + *
> + * 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 I2C 12-bit digital-to-analog converter (DAC)
> + * (7-bit I2C slave address 0x60, the three LSBs can be configured in
> + * hardware)
> + *
> + * writing the DAC value to EEPROM is not supported
> + */
> +
> +#include<linux/module.h>
> +#include<linux/init.h>
> +#include<linux/i2c.h>
> +#include<linux/err.h>
> +
> +#include<linux/iio/iio.h>
> +#include<linux/iio/sysfs.h>
> +
> +#include "mcp4725.h"
> +
> +#define MCP4725_DRV_NAME	"mcp4725"
> +
> +struct mcp4725_data {
> +	struct iio_dev		*indio_dev;
> +	struct i2c_client	*client;
> +	unsigned short		vref_mv;
> +	unsigned short		dac_value;
> +};
> +
> +static ssize_t mcp4725_set_value(struct device *dev,
> +				 struct device_attribute *attr,
> +				 const char *buf, size_t count)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct mcp4725_data *data = iio_priv(indio_dev);
> +	u8 outbuf[2];
> +	int res;
> +	long val;
> +
> +	res = kstrtol(buf, 10,&val);
> +	if (res)
> +		return res;
> +
> +	if (val<  0 || val>  0xfff)
> +		return -EINVAL;
> +
> +	outbuf[0] = (val>>  8)&  0xf;
> +	outbuf[1] = val&  0xff;
> +
> +	res = i2c_master_send(data->client, outbuf, 2);
> +	if (res<  0)
> +		return res;
> +
> +	data->dac_value = val;
> +
> +	return count;
> +}
> +
> +static IIO_DEVICE_ATTR(out_voltage_raw, S_IWUSR,
> +		       NULL, mcp4725_set_value, 0);
> +
> +static ssize_t mcp4725_show_scale(struct device *dev,
> +				struct device_attribute *attr,
> +				char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct mcp4725_data *data = iio_priv(indio_dev);
> +
> +	unsigned int scale_uv = (data->vref_mv * 1000)>>  12;
> +
> +	return sprintf(buf, "%d.%03d\n", scale_uv / 1000, scale_uv % 1000);
> +}
> +
> +static IIO_DEVICE_ATTR(out_voltage_scale, S_IRUGO,
> +		       mcp4725_show_scale, NULL, 0);
> +
> +static struct attribute *mcp4725_attributes[] = {
> +	&iio_dev_attr_out_voltage_raw.dev_attr.attr,
> +	&iio_dev_attr_out_voltage_scale.dev_attr.attr,
> +	NULL
> +};
> +
> +static struct attribute_group mcp4725_attribute_group = {
> +	.attrs = mcp4725_attributes,
> +};
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int mcp4725_suspend(struct device *dev)
> +{
> +	u8 outbuf[2];
> +
> +	outbuf[0] = 0x3<<  4; /* power-down bits, 500 kOhm resistor */
> +	outbuf[1] = 0;
> +
> +	return i2c_master_send(to_i2c_client(dev),&outbuf, 2);
> +}
> +
> +static int mcp4725_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct mcp4725_data *data = iio_priv(indio_dev);
> +	u8 outbuf[2];
> +
> +	/* restore previous DAC value */
> +	outbuf[0] = (data->dac_value>>  8)&  0xf;
> +	outbuf[1] = data->dac_value&  0xff;
> +
> +	return i2c_master_send(to_i2c_client(dev),&outbuf, 2);
> +}
> +
> +static SIMPLE_DEV_PM_OPS(mcp4725_pm_ops, mcp4725_suspend, mcp4725_resume);
> +#define MCP4725_PM_OPS (&mcp4725_pm_ops)
> +#else
> +#define MCP4725_PM_OPS NULL
> +#endif
> +
> +static const struct iio_info mcp4725_info = {
> +	.attrs =&mcp4725_attribute_group,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static int mcp4725_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	struct mcp4725_data *data;
> +	struct iio_dev *indio_dev;
> +	struct mcp4725_platform_data *platform_data = client->dev.platform_data;
> +	u8 inbuf[3];
> +	int err;
> +
> +	if (!platform_data || !platform_data->vref_mv) {
> +		dev_err(&client->dev, "invalid platform data");
> +		err = -EINVAL;
> +		goto exit;
> +	}
> +
> +	indio_dev = iio_device_alloc(sizeof(*data));
> +	if (indio_dev == NULL) {
> +		err = -ENOMEM;
> +		goto exit;
> +	}
> +	data = iio_priv(indio_dev);
> +	i2c_set_clientdata(client, indio_dev);
> +	data->client = client;
> +
> +	indio_dev->dev.parent =&client->dev;
> +	indio_dev->info =&mcp4725_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	data->vref_mv = platform_data->vref_mv;
> +
> +	/* read current DAC value */
> +	err = i2c_master_recv(client, inbuf, 3);
> +	if (err<  0) {
> +		dev_err(&client->dev, "failed to read DAC value");
> +		goto exit_free_device;
> +	}
> +	data->dac_value = (inbuf[1]<<  4) | (inbuf[2]>>  4);
> +
> +	err = iio_device_register(indio_dev);
> +	if (err)
> +		goto exit_free_device;
> +
> +	dev_info(&client->dev, "MCP4725 DAC registered\n");
> +
> +	return 0;
> +
> +exit_free_device:
> +	iio_device_free(indio_dev);
> +exit:
> +	return err;
> +}
> +
> +static int mcp4725_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +
> +	iio_device_unregister(indio_dev);
> +	iio_device_free(indio_dev);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id mcp4725_id[] = {
> +	{ "mcp4725", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, mcp4725_id);
> +
> +static struct i2c_driver mcp4725_driver = {
> +	.driver = {
> +		.name	= MCP4725_DRV_NAME,
> +		.pm	= MCP4725_PM_OPS,
> +	},
> +	.probe		= mcp4725_probe,
> +	.remove		= mcp4725_remove,
> +	.id_table	= mcp4725_id,
> +};
> +module_i2c_driver(mcp4725_driver);
> +
> +MODULE_AUTHOR("Peter Meerwald<pmeerw@pmeerw.net>");
> +MODULE_DESCRIPTION("MCP4725 12-bit DAC");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/staging/iio/dac/mcp4725.h b/drivers/staging/iio/dac/mcp4725.h
> new file mode 100644
> index 0000000..91530e6
> --- /dev/null
> +++ b/drivers/staging/iio/dac/mcp4725.h
> @@ -0,0 +1,16 @@
> +/*
> + * MCP4725 DAC driver
> + *
> + * Copyright (C) 2012 Peter Meerwald<pmeerw@pmeerw.net>
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#ifndef IIO_DAC_MCP4725_H_
> +#define IIO_DAC_MCP4725_H_
> +
> +struct mcp4725_platform_data {
> +	u16 vref_mv;
> +};
> +
> +#endif /* IIO_DAC_MCP4725_H_ */

Re: [PATCH] iio: add mcp4725 I2C DAC driver

[Lars-Peter Clausen]

On 04/30/2012 04:39 PM, Jonathan Cameron wrote:
> On 4/30/2012 2:54 PM, Peter Meerwald wrote:
>> v2 (based on comments from Jonathan Cameron and Lars-Peter Clausen):
>> * did NOT switch to chan_spec yet
> I guess from this you are intending to? I guess whether we merge this as
> is depends
> on what timescale you are thinking of to make that change?
>> * rebase to staging-next tree, update iio header locations
>> * dropped dac.h #include, not needed
>> * strict_strtol() ->  kstrtol()
>> * call iio_device_unregister() in remove()
>> * everything in one patch
> If you want to send it on to Greg KH now and Lars-Peter is happy, then
> that's fine by me.

I don't think it's a good idea to introduce new non chan_spec drivers.
Especially for simple drivers like this one where it is relatively easy to add
chan_spec support. So no ack from my side, but that's not necessarily a blocker
for the patch.

>> Signed-off-by: Peter Meerwald<pmeerw@pmeerw.net>
> Acked-by: Jonathan Cameron <jic23@kernel.org>