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

[Jonathan Cameron]

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

[PATCH] iio: add mcp4725 I2C DAC driver

[Peter Meerwald]

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

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

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

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

[Jonathan Cameron]

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

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

[Lars-Peter Clausen]

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

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

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

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;
>> +}

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