[PATCH 1/2] iio: add Kconfig option and Makefile entry for mcp4725 I2C DAC driver

[PATCH 1/2] iio: add Kconfig option and Makefile entry for mcp4725 I2C DAC driver

[Peter Meerwald]

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>

---
 drivers/staging/iio/dac/Kconfig  |   11 +++++++++++
 drivers/staging/iio/dac/Makefile |    1 +
 2 files changed, 12 insertions(+)

diff --git a/drivers/staging/iio/dac/Kconfig b/drivers/staging/iio/dac/Kconfig
index a57803a..9308118 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
-- 
1.7.9.5

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

[Peter Meerwald]

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>

---
 drivers/staging/iio/dac/mcp4725.c |  210 +++++++++++++++++++++++++++++++++++++
 drivers/staging/iio/dac/mcp4725.h |   16 +++
 2 files changed, 226 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/mcp4725.c b/drivers/staging/iio/dac/mcp4725.c
new file mode 100644
index 0000000..8603c66
--- /dev/null
+++ b/drivers/staging/iio/dac/mcp4725.c
@@ -0,0 +1,210 @@
+/*
+ * 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 "../iio.h"
+#include "../sysfs.h"
+#include "dac.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);
+	struct i2c_client *client = data->client;
+	u8 outbuf[2];
+	int res;
+	long val;
+
+	res = strict_strtol(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(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_allocate_device(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_free_device(indio_dev);
+exit:
+	return err;
+}
+
+static int mcp4725_remove(struct i2c_client *client)
+{
+	iio_free_device(i2c_get_clientdata(client));
+
+	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 1/2] iio: add Kconfig option and Makefile entry for mcp4725 I2C DAC driver

[Jonathan Cameron]

Hi Peter,

Welcome to IIO!
This is fine, but needs to be part of the same patch as the actual code.
Convention is to introduce build options in the same patch as what
they cover. Also if like here you introduce it first the kernel becomes
non bisectable inbetween the two patches which won't go down
well if anyone hits it!
> Signed-off-by: Peter Meerwald<pmeerw@pmeerw.net>
>
> ---
>   drivers/staging/iio/dac/Kconfig  |   11 +++++++++++
>   drivers/staging/iio/dac/Makefile |    1 +
>   2 files changed, 12 insertions(+)
>
> diff --git a/drivers/staging/iio/dac/Kconfig b/drivers/staging/iio/dac/Kconfig
> index a57803a..9308118 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

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

[Jonathan Cameron]

On 4/29/2012 6:13 PM, Peter Meerwald wrote:
> Signed-off-by: Peter Meerwald<pmeerw@pmeerw.net>
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.

Sometime soon we'll bring it up to the latest and greatest.

Sorry about that!

Anyhow, I've made some suggestions inline.  What you do with them is 
dependent on
how much time you want to spend on this. I'm happy to see it go into 
staging/iio
as is, but would want to move to the chan_spec stuff to take it directly 
into drivers/iio/
without bounchign through staging.

The only vital change is updating your header locations as some of them 
have moved.

At somepoint I suspect someone will add regulator support for that 
reference voltage.
Obviously if it is no use to you there is no reason for you to add it!

Jonathan
> ---
>   drivers/staging/iio/dac/mcp4725.c |  210 +++++++++++++++++++++++++++++++++++++
>   drivers/staging/iio/dac/mcp4725.h |   16 +++
>   2 files changed, 226 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/mcp4725.c b/drivers/staging/iio/dac/mcp4725.c
> new file mode 100644
> index 0000000..8603c66
> --- /dev/null
> +++ b/drivers/staging/iio/dac/mcp4725.c
> @@ -0,0 +1,210 @@
> +/*
> + * mcp4725.c - Support for Microchip MCP4725
> + *
> + * Copyright (C) 2012 Peter Meerwald<pmeerw@pmeerw.net>
> + *
> + * Based on max517 by Roland Stigge<stigge@antcom.de>
That's a little unfortuante as Roland's driver is prior to the change
to doing pretty much everything through iio_chan_spec descriptions
of the channels and read_raw and write_raw callbacks.
The reasoning behind this is that it makes it possible for other drivers
within the kernel to make use of the facilities the device provides.
Still in a nice simple driver like this it won't take too long.

Without that change I'm not happy with this going into drivers/iio/dac
but it could go straight into drivers/staging/iio/dac
> + *
> + * 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 "../iio.h"
> +#include "../sysfs.h"
You are lagging a bit.  Please test against the latest staging-next 
branch of:

git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git

Note that the core of IIO is out of staging in that tree so we are happy to
take drivers directly into the non staging side if they have nothing messy
or controversial.


> +#include "dac.h"
Hohum.   That's a header we need to kill off with moving all the remaining
dac drivers over the chan_spec stuff.
> +
> +#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);
> +	struct i2c_client *client = data->client;
General point: As you only use client once in the function, I'd just put
data->client into that call and drop this line.
> +	u8 outbuf[2];
> +	int res;
> +	long val;
> +
> +	res = strict_strtol(buf, 10,&val);
Deprecated function.  Please use kstrtol or similar. Otherwise someone
will just post a patch switching it over 2 days after this hits the tree!
> +	if (res)
> +		return res;
> +
> +	if (val<  0 || val>  0xfff)
> +		return -EINVAL;
> +
> +	outbuf[0] = (val>>  8)&  0xf;
> +	outbuf[1] = val&  0xff;
Might be marginally clearer with outbuf as a u16 then mask and
use cpu_to_le16 ( I think that's the right one - I've not had enough
coffee yet to be sure ;)
> +
> +	res = i2c_master_send(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_allocate_device(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_free_device(indio_dev);
> +exit:
> +	return err;
> +}
> +
> +static int mcp4725_remove(struct i2c_client *client)
> +{
> +	iio_free_device(i2c_get_clientdata(client));
> +
> +	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 2/2] iio: add mcp4725 I2C DAC driver

[Peter Meerwald]

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?

regards, p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)

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

[Lars-Peter Clausen]

On 04/30/2012 11:43 AM, Jonathan Cameron wrote:
> On 4/29/2012 6:13 PM, Peter Meerwald wrote:
>> Signed-off-by: Peter Meerwald<pmeerw@pmeerw.net>
> 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.
> 
> Sometime soon we'll bring it up to the latest and greatest.
> 
> Sorry about that!
> 
> Anyhow, I've made some suggestions inline.  What you do with them is
> dependent on
> how much time you want to spend on this. I'm happy to see it go into
> staging/iio
> as is, but would want to move to the chan_spec stuff to take it directly
> into drivers/iio/
> without bounchign through staging.
> 
> The only vital change is updating your header locations as some of them
> have moved.
> 
> At somepoint I suspect someone will add regulator support for that
> reference voltage.
> Obviously if it is no use to you there is no reason for you to add it!
> 
> Jonathan
>> ---
>>   drivers/staging/iio/dac/mcp4725.c |  210
>> +++++++++++++++++++++++++++++++++++++
>>   drivers/staging/iio/dac/mcp4725.h |   16 +++
>>   2 files changed, 226 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/mcp4725.c
>> b/drivers/staging/iio/dac/mcp4725.c
>> new file mode 100644
>> index 0000000..8603c66
>> --- /dev/null
>> +++ b/drivers/staging/iio/dac/mcp4725.c
>> @@ -0,0 +1,210 @@
>> +/*
>> + * mcp4725.c - Support for Microchip MCP4725
>> + *
>> + * Copyright (C) 2012 Peter Meerwald<pmeerw@pmeerw.net>
>> + *
>> + * Based on max517 by Roland Stigge<stigge@antcom.de>
> That's a little unfortuante as Roland's driver is prior to the change
> to doing pretty much everything through iio_chan_spec descriptions
> of the channels and read_raw and write_raw callbacks.
> The reasoning behind this is that it makes it possible for other drivers
> within the kernel to make use of the facilities the device provides.
> Still in a nice simple driver like this it won't take too long.
> 
> Without that change I'm not happy with this going into drivers/iio/dac
> but it could go straight into drivers/staging/iio/dac
>> + *
>> + * 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 "../iio.h"
>> +#include "../sysfs.h"
> You are lagging a bit.  Please test against the latest staging-next
> branch of:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
> 
> Note that the core of IIO is out of staging in that tree so we are happy to
> take drivers directly into the non staging side if they have nothing messy
> or controversial.
> 

Also note that the "Streamline API function naming" patch has been merged now.
So iio_allocate_device is now iio_device_alloc, etc.

>> +
>> +static int mcp4725_remove(struct i2c_client *client)
>> +{

There is iio_device_unregister missing here. Bonus points for also fixing it in
the max517 driver.

>> +    iio_free_device(i2c_get_clientdata(client));
>> +
>> +    return 0;
>> +}

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

[PATCH 1/2] iio: replace strict_strtol() with kstrtol() in max517 driver

[Peter Meerwald]

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/staging/iio/dac/max517.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/dac/max517.c b/drivers/staging/iio/dac/max517.c
index a3f588df..60aef18 100644
--- a/drivers/staging/iio/dac/max517.c
+++ b/drivers/staging/iio/dac/max517.c
@@ -67,7 +67,7 @@ static ssize_t max517_set_value(struct device *dev,
 	int res;
 	long val;
 
-	res = strict_strtol(buf, 10, &val);
+	res = kstrtol(buf, 10, &val);
 
 	if (res)
 		return res;
-- 
1.7.9.5

[PATCH 2/2] iio: call iio_device_unregister() in max517_remove()

[Peter Meerwald]

Reported-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/staging/iio/dac/max517.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/iio/dac/max517.c b/drivers/staging/iio/dac/max517.c
index 60aef18..672fa51 100644
--- a/drivers/staging/iio/dac/max517.c
+++ b/drivers/staging/iio/dac/max517.c
@@ -264,6 +264,7 @@ exit:
 
 static int max517_remove(struct i2c_client *client)
 {
+	iio_device_unregister(i2c_get_clientdata(client));
 	iio_device_free(i2c_get_clientdata(client));
 
 	return 0;
-- 
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 1/2] iio: replace strict_strtol() with kstrtol() in max517 driver

[Jonathan Cameron]

This one doesn't really matter as it'll get eaten up in a conversion to 
chan_spec shortly
anyway.  But it is an improvement even if a temporary one so what the 
heck...

On 4/30/2012 3:12 PM, Peter Meerwald wrote:
> Signed-off-by: Peter Meerwald<pmeerw@pmeerw.net>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>   drivers/staging/iio/dac/max517.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/iio/dac/max517.c b/drivers/staging/iio/dac/max517.c
> index a3f588df..60aef18 100644
> --- a/drivers/staging/iio/dac/max517.c
> +++ b/drivers/staging/iio/dac/max517.c
> @@ -67,7 +67,7 @@ static ssize_t max517_set_value(struct device *dev,
>   	int res;
>   	long val;
>
> -	res = strict_strtol(buf, 10,&val);
> +	res = kstrtol(buf, 10,&val);
>
>   	if (res)
>   		return res;

Re: [PATCH 2/2] iio: call iio_device_unregister() in max517_remove()

[Jonathan Cameron]

On 4/30/2012 3:12 PM, Peter Meerwald wrote:
> Reported-by: Lars-Peter Clausen<lars@metafoo.de>
> Signed-off-by: Peter Meerwald<pmeerw@pmeerw.net>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>   drivers/staging/iio/dac/max517.c |    1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/iio/dac/max517.c b/drivers/staging/iio/dac/max517.c
> index 60aef18..672fa51 100644
> --- a/drivers/staging/iio/dac/max517.c
> +++ b/drivers/staging/iio/dac/max517.c
> @@ -264,6 +264,7 @@ exit:
>
>   static int max517_remove(struct i2c_client *client)
>   {
> +	iio_device_unregister(i2c_get_clientdata(client));
>   	iio_device_free(i2c_get_clientdata(client));
>
>   	return 0;

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>

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>
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

[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_ */

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.
>

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)

[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 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_ */

[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 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_ */

[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