Linux driver for MAX517/518/519

Quoted 5 lines

I've written a driver for the Maxim MAX517/MAX518/MAX519 1/2-channel DAC. See also the branch "max51x" of repository linux-2.6 at git.antcom.de (alternatively the patch at
http://www.rolandstigge.de/dp/download/patches/max51x.patch).

Quoted 7 lines

It's currently located in drivers/misc/max51x.c, implemented similarly to the other drivers like e.g. ti_dac7512.c. The hwmon maintainer pointed me to drivers/staging/iio/.

Is it advisible now to port the driver to drivers/staging/iio or keep it in drivers/misc? Where should I forward the code/patch for Linux integration?

It's best to convert to the iio subsystem, as that is the way to go in the future.

As for where to send patches for it, send them here, and cc: the iio developers and me and we will be glad to comment on them and apply them to the kernel tree for submission to Linus's tree.

Hi,

Quoted 13 lines · Greg KH

Quoted 5 lines

I've written a driver for the Maxim MAX517/MAX518/MAX519 1/2-channel DAC. See also the branch "max51x" of repository linux-2.6 at git.antcom.de (alternatively the patch at
http://www.rolandstigge.de/dp/download/patches/max51x.patch).

Quoted 7 lines

It's currently located in drivers/misc/max51x.c, implemented similarly to the other drivers like e.g. ti_dac7512.c. The hwmon maintainer pointed me to drivers/staging/iio/.

Is it advisible now to port the driver to drivers/staging/iio or keep it in drivers/misc? Where should I forward the code/patch for Linux integration?

Quoted 6 lines

It's best to convert to the iio subsystem, as that is the way to go in the future.

As for where to send patches for it, send them here, and cc: the iio developers and me and we will be glad to comment on them and apply them to the kernel tree for submission to Linus's tree.

OK, done. Attaching the patch. (Also available at the above mentioned git repository for merging.)

Thanks in advance,

Roland

BTW: Maybe we should also call it "Analog to digital converters" instead of "Analog to digital convertors", see drivers/staging/iio/adc/Kconfig

Added max51x iio driver

Signed-off-by Roland Stigge <[email protected]>

drivers/staging/iio/Documentation/dac/max51x new file +38 -0

new file mode 100644
index 0000000..cf61a25
@@ -0,0 +1,38 @@
+Kernel driver max51x
+====================
+
+Supported chips:
+  * Maxim MAX517, MAX518, MAX519
+    Prefix: 'max51x'
+    Datasheet: Publicly available at the Maxim website
+               http://www.maxim-ic.com/
+
+Author:
+        Roland Stigge <[email protected]>
+
+Description
+-----------
+
+The Maxim MAX51x is an 8-bit DAC on the I2C bus. The following table shows the
+different feature sets of the variants MAX517, MAX518 and MAX519:
+
+Feature                              MAX517 MAX518 MAX519
+--------------------------------------------------------------------------
+One output channel                   X
+Two output channels                         X      X
+Simultaneous output updates                 X      X
+Supply voltage as reference                 X
+Separate reference input             X
+Reference input for each DAC                       X
+
+Via the iio sysfs interface, there are three attributes available: value_1,
+value_2 and value_both. With value_1 and value_2, the current output values of
+the DACs can be read back from the driver and written to the device. value_both
+can be used to set both output channel values simultaneously.
+
+With MAX517, only value_1 is available.
+
+When the operating system goes to a power down state, the Power Down function
+of the chip is activated, reducing the supply current to 4uA.
+
+On power-up, the device is in 0V-output state.

drivers/staging/iio/Kconfig +1 -0

index ed48815..6fc35ad 100644
@@ -42,6 +42,7 @@ config IIO_TRIGGER
 
 source "drivers/staging/iio/accel/Kconfig"
 source "drivers/staging/iio/adc/Kconfig"
+source "drivers/staging/iio/dac/Kconfig"
 source "drivers/staging/iio/gyro/Kconfig"
 source "drivers/staging/iio/imu/Kconfig"
 source "drivers/staging/iio/light/Kconfig"

drivers/staging/iio/Makefile +0 -0

index e909674..8766e07 100644

drivers/staging/iio/Makefile +0 -0

 obj-y += gyro/
 obj-y += imu/
 obj-y += light/

drivers/staging/iio/dac/Kconfig new file +0 -0

new file mode 100644
index 0000000..f2c9c84

drivers/staging/iio/dac/Kconfig new file +8 -0

+	tristate "Maxim MAX51X DAC driver"
+	depends on I2C && EXPERIMENTAL
+	help
+	  If you say yes here you get support for the Maxim chips MAX517,
+	  MAX518 and MAX519 (I2C 8-Bit DACs with rail-to-rail outputs).
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called max51x.

drivers/staging/iio/dac/Makefile new file +5 -0

new file mode 100644
index 0000000..3caa871
@@ -0,0 +1,5 @@
+#
+# Makefile for industrial I/O DAC drivers
+#
+
+obj-$(CONFIG_MAX51X) += max51x.o

drivers/staging/iio/dac/max51x.c new file +69 -0

new file mode 100644
index 0000000..6d05428
@@ -0,0 +1,258 @@
+/*
+ *  max51x.c - Support for Maxim MAX517, MAX518 and MAX519
+ *
+ *  Copyright (C) 2010 Roland Stigge <[email protected]>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/jiffies.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+
+#include "../iio.h"
+
+#define MAX51X_DRV_NAME	"max51x"
+
+/* Commands */
+#define COMMAND_CHANNEL0	0x00
+#define COMMAND_CHANNEL1	0x01 /* for MAX518 and MAX519 */
+#define COMMAND_PD		0x08 /* Power Down */
+
+struct max51x_data {
+	u8			values[2];
+	struct iio_dev		*indio_dev;
+};
+
+/*
+ * channel: bit 0: channel 1
+ *          bit 1: channel 2
+ * (this way, it's possible to set both channels at once)
+ */
+static ssize_t max51x_set_value(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count, int channel)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct max51x_data *data = i2c_get_clientdata(client);
+	u8 outbuf[4]; /* 1x or 2x command + value */
+	int outbuf_size = 0;
+	int res;
+	long val;
+
+	res = strict_strtol(buf, 10, &val);
+
+	if (res)
+		return res;
+
+	if (val < 0 || val > 255)
+		return -EINVAL;
+
+	if (channel & 1) {
+		data->values[0] = val;
+		outbuf[outbuf_size++] = COMMAND_CHANNEL0;
+		outbuf[outbuf_size++] = val;

drivers/staging/iio/dac/max51x.c new file +28 -0

+	return count;
+}
+
+static ssize_t max51x_set_value_1(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	return max51x_set_value(dev, attr, buf, count, 1);
+}
+
+static ssize_t max51x_set_value_2(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	return max51x_set_value(dev, attr, buf, count, 2);
+}
+
+static ssize_t max51x_set_value_both(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	return max51x_set_value(dev, attr, buf, count, 3);
+}
+
+static ssize_t max51x_get_value(struct device *dev, struct device_attribute *da,
+				char *buf, int channel)
+{
+	struct i2c_client *client = to_i2c_client(dev);

drivers/staging/iio/dac/max51x.c new file +6 -0

+}
+
+static ssize_t max51x_get_value_1(struct device *dev,
+				  struct device_attribute *da, char *buf)
+{
+	return max51x_get_value(dev, da, buf, 1);

drivers/staging/iio/dac/max51x.c new file +36 -0

+static IIO_DEVICE_ATTR(value_2, S_IWUSR | S_IRUGO,
+		max51x_get_value_2, max51x_set_value_2, 1);
+static IIO_DEVICE_ATTR(value_both, S_IWUSR, NULL, max51x_set_value_both, -1);
+
+static struct attribute *max51x_attributes[] = {
+	&iio_dev_attr_value_1.dev_attr.attr,
+	&iio_dev_attr_value_2.dev_attr.attr,
+	&iio_dev_attr_value_both.dev_attr.attr,
+	NULL
+};
+
+static struct attribute *max517_attributes[] = {
+	&iio_dev_attr_value_1.dev_attr.attr,
+	NULL
+};
+
+static struct attribute_group max51x_attribute_group = {
+	.attrs = max51x_attributes,
+};
+
+static int max51x_suspend(struct i2c_client *client, pm_message_t mesg)
+{
+	return i2c_smbus_write_byte(client, COMMAND_PD);
+}
+
+static int max51x_resume(struct i2c_client *client)
+{
+	return i2c_smbus_write_byte(client, 0);
+}
+
+static int max51x_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct max51x_data *data;
+	int err;
+

drivers/staging/iio/dac/max51x.c new file +24 -0

+
+	data->indio_dev = iio_allocate_device();
+	if (data->indio_dev == NULL) {
+		err = -ENOMEM;
+		goto exit_free_data;
+	}
+
+	/* establish that the iio_dev is a child of the i2c device */
+	data->indio_dev->dev.parent = &client->dev;
+	data->indio_dev->attrs = &max51x_attribute_group;
+	data->indio_dev->dev_data = (void *)(data);
+	data->indio_dev->driver_module = THIS_MODULE;
+	data->indio_dev->modes = INDIO_DIRECT_MODE;
+
+	err = iio_device_register(data->indio_dev);
+	if (err)
+		goto exit_free_device;
+
+	dev_info(&client->dev, "DAC registered\n");
+
+	return 0;
+
+exit_free_device:
+	iio_free_device(data->indio_dev);

drivers/staging/iio/dac/max51x.c new file +8 -0

+	kfree(data);
+
+	return 0;
+}
+
+static const struct i2c_device_id max51x_id[] = {
+	{ "max517", 0 },
+	{ "max518", 0 },

drivers/staging/iio/dac/max51x.c new file +22 -0

+	.remove		= max51x_remove,
+	.suspend	= max51x_suspend,
+	.resume		= max51x_resume,
+	.id_table	= max51x_id,
+};
+
+static int __init max51x_init(void)
+{
+	return i2c_add_driver(&max51x_driver);
+}
+
+static void __exit max51x_exit(void)
+{
+	i2c_del_driver(&max51x_driver);
+}
+
+MODULE_AUTHOR("Roland Stigge <[email protected]>");
+MODULE_DESCRIPTION("MAX51X 8-bit DAC");
+MODULE_LICENSE("GPL");
+
+module_init(max51x_init);
+module_exit(max51x_exit);

Pretty clean and nice driver so it was an easy review and should be trivial to fix up for a merge.

I don't think we have previously had a device that allows setting multiple inputs together. Two options come to mind that will generalize more than your _both.
=20

output1&2_raw
=20
output_raw (suppress the index hence indicating that it sets both). =20
What do you think is the clearest approach?
Which ever we pick it will also need proper documentation. Whilst we are here, please can you explain your use case? From a datasheet read I think the first channel is latched after the value byte is passe=

then the second only after it's value has been passed over?

Hi,
=20

Quoted 2 lines · Jonathan Cameron

Pretty clean and nice driver so it was an easy review and should be trivial to fix up for a merge.

=20
Thank you for your review - I will include the suggested changes in t=

next update.
=20

Quoted 11 lines

I don't think we have previously had a device that allows setting multiple inputs together. Two options come to mind that will generalize more than your _both.

output1&2_raw

output_raw (suppress the index hence indicating that it sets both).

What do you think is the clearest approach?
Which ever we pick it will also need proper documentation. Whilst w=

Quoted 2 lines

are here, please can you explain your use case? From a datasheet read I think the first channel is latched after the value byte is pa=

Quoted 1 line

then the second only after it's value has been passed over?

=20
It's actually latched after the _complete_ transmission. See datashee=

=20
"The data is transferred to the DAC=E2=80=99s output latch during the STOP condition following the transmis- sion. This allows both DACs of the MAX518/MAX519 to be updated simultaneously."

=20
I will also document this in the driver to make it clearer.

=20
That's also why I'm constructing the I2C transfer in this nonstandard way. Datasheet doesn't claim to support smbus, so I will change to th=

_ic2_ (non _smbus_) interface.

=20
bye,
Roland
=20

Pretty clean and nice driver so it was an easy review and should be trivial to fix up for a merge.

Hi,

Quoted 2 lines · Jonathan Cameron

Pretty clean and nice driver so it was an easy review and should be trivial to fix up for a merge.

Thanks for your review!

I'm attaching my update. Some notes:

* I used the attributes out1_* how I found it in the other DAC driver already available

* suspend doesn't need the protective #ifdefs
* As for other drivers, drivers/staging/iio/dac/max517.h needs to go to include/linux/iio as soon as available

I think I addressed everything from your notes. Let me know if there is still some blocker.

Thanks in advance!

Roland

IIO Driver for Maxim MAX517, MAX518 and MAX519 DAC

Signed-off-by Roland Stigge <[email protected]>

drivers/staging/iio/Documentation/dac/max517 new file +41 -0

new file mode 100644
index 0000000..e60ec2f
@@ -0,0 +1,41 @@
+Kernel driver max517
+====================
+
+Supported chips:
+  * Maxim MAX517, MAX518, MAX519
+    Prefix: 'max517'
+    Datasheet: Publicly available at the Maxim website
+               http://www.maxim-ic.com/
+
+Author:
+        Roland Stigge <[email protected]>
+
+Description
+-----------
+
+The Maxim MAX517/518/519 is an 8-bit DAC on the I2C bus. The following table
+shows the different feature sets of the variants MAX517, MAX518 and MAX519:
+
+Feature                              MAX517 MAX518 MAX519
+--------------------------------------------------------------------------
+One output channel                   X
+Two output channels                         X      X
+Simultaneous output updates                 X      X
+Supply voltage as reference                 X
+Separate reference input             X
+Reference input for each DAC                       X
+
+Via the iio sysfs interface, there are three attributes available: out1_raw,
+out2_raw and out12_raw. With out1_raw and out2_raw, the current output values
+(0..255) of the DACs can be written to the device. out12_raw can be used to set
+both output channel values simultaneously.
+
+With MAX517, only out1_raw is available.
+
+Via out1_scale (and where appropriate, out2_scale), the current scaling factor
+in mV can be read.
+
+When the operating system goes to a power down state, the Power Down function
+of the chip is activated, reducing the supply current to 4uA.
+
+On power-up, the device is in 0V-output state.

drivers/staging/iio/dac/Kconfig +10 -0

index 9191bd2..2120904 100644
@@ -19,3 +19,13 @@ config AD5446
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad5446.
+
+config MAX517
+	tristate "Maxim MAX517/518/519 DAC driver"
+	depends on I2C && EXPERIMENTAL
+	help
+	  If you say yes here you get support for the Maxim chips MAX517,
+	  MAX518 and MAX519 (I2C 8-Bit DACs with rail-to-rail outputs).
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called max517.

drivers/staging/iio/dac/Makefile +1 -0

index 7cf331b..1197aef 100644
@@ -4,3 +4,4 @@
 
 obj-$(CONFIG_AD5624R_SPI) += ad5624r_spi.o
 obj-$(CONFIG_AD5446) += ad5446.o
+obj-$(CONFIG_MAX517) += max517.o

drivers/staging/iio/dac/max517.c new file +214 -0

new file mode 100644
index 0000000..5c57523
@@ -0,0 +1,294 @@
+/*
+ *  max517.c - Support for Maxim MAX517, MAX518 and MAX519
+ *
+ *  Copyright (C) 2010, 2011 Roland Stigge <[email protected]>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/jiffies.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+
+#include "../iio.h"
+#include "dac.h"
+
+#include "max517.h"
+
+#define MAX517_DRV_NAME	"max517"
+
+/* Commands */
+#define COMMAND_CHANNEL0	0x00
+#define COMMAND_CHANNEL1	0x01 /* for MAX518 and MAX519 */
+#define COMMAND_PD		0x08 /* Power Down */
+
+enum max517_device_ids {
+	ID_MAX517,
+	ID_MAX518,
+	ID_MAX519,
+};
+
+struct max517_data {
+	struct iio_dev		*indio_dev;
+	unsigned short		vref_mv[2];
+};
+
+/*
+ * channel: bit 0: channel 1
+ *          bit 1: channel 2
+ * (this way, it's possible to set both channels at once)
+ */
+static ssize_t max517_set_value(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count, int channel)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	u8 outbuf[4]; /* 1x or 2x command + value */
+	int outbuf_size = 0;
+	int res;
+	long val;
+
+	res = strict_strtol(buf, 10, &val);
+
+	if (res)
+		return res;
+
+	if (val < 0 || val > 255)
+		return -EINVAL;
+
+	if (channel & 1) {
+		outbuf[outbuf_size++] = COMMAND_CHANNEL0;
+		outbuf[outbuf_size++] = val;
+	}
+	if (channel & 2) {
+		outbuf[outbuf_size++] = COMMAND_CHANNEL1;
+		outbuf[outbuf_size++] = val;
+	}
+
+	/*
+	 * At this point, there are always 1 or 2 two-byte commands in
+	 * outbuf. With 2 commands, the device can set two outputs
+	 * simultaneously, latching the values upon the end of the I2C
+	 * transfer.
+	 */
+
+	res = i2c_master_send(client, outbuf, outbuf_size);
+	if (res < 0)
+		return res;
+
+	return count;
+}
+
+static ssize_t max517_set_value_1(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	return max517_set_value(dev, attr, buf, count, 1);
+}
+static IIO_DEV_ATTR_OUT_RAW(1, max517_set_value_1, 0);
+
+static ssize_t max517_set_value_2(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	return max517_set_value(dev, attr, buf, count, 2);
+}
+static IIO_DEV_ATTR_OUT_RAW(2, max517_set_value_2, 1);
+
+static ssize_t max517_set_value_both(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	return max517_set_value(dev, attr, buf, count, 3);
+}
+static IIO_DEV_ATTR_OUT_RAW(12, max517_set_value_both, -1);
+
+static ssize_t max517_show_scale(struct device *dev,
+				struct device_attribute *attr,
+				char *buf, int channel)
+{
+	struct iio_dev *dev_info = dev_get_drvdata(dev);
+	struct max517_data *data = iio_dev_get_devdata(dev_info);
+	/* Corresponds to Vref / 2^(bits) */
+	unsigned int scale_uv = (data->vref_mv[channel - 1] * 1000) >> 8;
+
+	return sprintf(buf, "%d.%03d\n", scale_uv / 1000, scale_uv % 1000);
+}
+
+static ssize_t max517_show_scale1(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	return max517_show_scale(dev, attr, buf, 1);
+}
+static IIO_DEVICE_ATTR(out1_scale, S_IRUGO, max517_show_scale1, NULL, 0);
+
+static ssize_t max517_show_scale2(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	return max517_show_scale(dev, attr, buf, 2);
+}
+static IIO_DEVICE_ATTR(out2_scale, S_IRUGO, max517_show_scale2, NULL, 0);
+
+/* On MAX517 variant, we have two outputs */
+static struct attribute *max517_attributes[] = {
+	&iio_dev_attr_out1_raw.dev_attr.attr,
+	&iio_dev_attr_out1_scale.dev_attr.attr,
+	NULL
+};
+
+static struct attribute_group max517_attribute_group = {
+	.attrs = max517_attributes,
+};
+
+/* On MAX518 and MAX518 variant, we have two outputs */
+static struct attribute *max518_attributes[] = {
+	&iio_dev_attr_out1_raw.dev_attr.attr,
+	&iio_dev_attr_out1_scale.dev_attr.attr,
+	&iio_dev_attr_out2_raw.dev_attr.attr,
+	&iio_dev_attr_out2_scale.dev_attr.attr,
+	&iio_dev_attr_out12_raw.dev_attr.attr,
+	NULL
+};
+
+static struct attribute_group max518_attribute_group = {
+	.attrs = max518_attributes,
+};
+
+static int max517_suspend(struct i2c_client *client, pm_message_t mesg)
+{
+	u8 outbuf = COMMAND_PD;
+
+	return i2c_master_send(client, &outbuf, 1);
+}
+
+static int max517_resume(struct i2c_client *client)
+{
+	u8 outbuf = 0;
+
+	return i2c_master_send(client, &outbuf, 1);
+}
+
+static int max517_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct max517_data *data;
+	struct max517_platform_data *platform_data = client->dev.platform_data;
+	int err;
+
+	data = kzalloc(sizeof(struct max517_data), GFP_KERNEL);
+	if (!data) {
+		err = -ENOMEM;
+		goto exit;
+	}
+
+	i2c_set_clientdata(client, data);
+
+	data->indio_dev = iio_allocate_device();
+	if (data->indio_dev == NULL) {
+		err = -ENOMEM;
+		goto exit_free_data;
+	}
+
+	/* establish that the iio_dev is a child of the i2c device */
+	data->indio_dev->dev.parent = &client->dev;
+
+	/* reduced attribute set for MAX517 */
+	if (id->driver_data == ID_MAX517)
+		data->indio_dev->attrs = &max517_attribute_group;

drivers/staging/iio/dac/max517.c new file +70 -0

+	if (id->driver_data == ID_MAX518)
+		data->vref_mv[0] = data->vref_mv[1] = 5000; /* mV */
+	else
+		if (platform_data) {
+			data->vref_mv[0] = platform_data->vref_mv[0];
+			data->vref_mv[1] = platform_data->vref_mv[1];
+		} else /* 5V by default */
+			data->vref_mv[0] = data->vref_mv[1] = 5000;
+
+	err = iio_device_register(data->indio_dev);
+	if (err)
+		goto exit_free_device;
+
+	dev_info(&client->dev, "DAC registered\n");
+
+	return 0;
+
+exit_free_device:
+	iio_free_device(data->indio_dev);
+exit_free_data:
+	kfree(data);
+exit:
+	return err;
+}
+
+static int max517_remove(struct i2c_client *client)
+{
+	struct max517_data *data = i2c_get_clientdata(client);
+
+	iio_free_device(data->indio_dev);
+	kfree(data);
+
+	return 0;
+}
+
+static const struct i2c_device_id max517_id[] = {
+	{ "max517", ID_MAX517 },
+	{ "max518", ID_MAX518 },
+	{ "max519", ID_MAX519 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, max517_id);
+
+static struct i2c_driver max517_driver = {
+	.driver = {
+		.name	= MAX517_DRV_NAME,
+	},
+	.probe		= max517_probe,
+	.remove		= max517_remove,
+	.suspend	= max517_suspend,
+	.resume		= max517_resume,
+	.id_table	= max517_id,
+};
+
+static int __init max517_init(void)
+{
+	return i2c_add_driver(&max517_driver);
+}
+
+static void __exit max517_exit(void)
+{
+	i2c_del_driver(&max517_driver);
+}
+
+MODULE_AUTHOR("Roland Stigge <[email protected]>");
+MODULE_DESCRIPTION("MAX517/MAX518/MAX519 8-bit DAC");
+MODULE_LICENSE("GPL");
+
+module_init(max517_init);
+module_exit(max517_exit);

drivers/staging/iio/dac/max517.h new file +19 -0

new file mode 100644
index 0000000..8106cf2
@@ -0,0 +1,19 @@
+/*
+ * MAX517 DAC driver
+ *
+ * Copyright 2011 Roland Stigge <[email protected]>
+ *
+ * Licensed under the GPL-2 or later.
+ */
+#ifndef IIO_DAC_MAX517_H_
+#define IIO_DAC_MAX517_H_
+
+/*
+ * TODO: struct max517_platform_data needs to go into include/linux/iio
+ */
+
+struct max517_platform_data {
+	u16				vref_mv[2];
+};
+
+#endif /* IIO_DAC_MAX517_H_ */

Quoted 8 lines · Jonathan Cameron

Quoted 2 lines

Pretty clean and nice driver so it was an easy review and should be trivial to fix up for a merge.

Thanks for your review!

I'm attaching my update. Some notes:

Quoted 2 lines

* I used the attributes out1_* how I found it in the other DAC driver already available

Cool, except for the out12_raw which could easily be the 12th channel. This has to be one of the two options suggested the other day. May require some macro magic. IIRC the relevant macro would be.

IIO_DEVICE_ATTR_NAMED(out1and2_raw, out1&2_raw, S_IWUSR, ...

Hi,

Quoted 14 lines · Jonathan Cameron

Quoted 10 lines · Jonathan Cameron

Quoted 2 lines

Pretty clean and nice driver so it was an easy review and should be trivial to fix up for a merge.

Thanks for your review!

I'm attaching my update. Some notes:

* I used the attributes out1_* how I found it in the other DAC driver already available

Cool, except for the out12_raw which could easily be the 12th channel. This has to be one of the two options suggested the other day. May require some macro magic. IIRC the relevant macro would be.

Quoted 1 line

IIO_DEVICE_ATTR_NAMED(out1and2_raw, out1&2_raw, S_IWUSR, ...

Update attached as suggested. :-)

Thanks!

Roland

Quoted 21 lines · Roland Stigge

Hi,

Quoted 8 lines · Jonathan Cameron

Quoted 8 lines · Jonathan Cameron

Quoted 2 lines

Pretty clean and nice driver so it was an easy review and should be trivial to fix up for a merge.

Thanks for your review!

I'm attaching my update. Some notes:

Quoted 7 lines

Quoted 2 lines

* I used the attributes out1_* how I found it in the other DAC driver already available

Cool, except for the out12_raw which could easily be the 12th channel. This has to be one of the two options suggested the other day. May require some macro magic. IIRC the relevant macro would be.

IIO_DEVICE_ATTR_NAMED(out1and2_raw, out1&2_raw, S_IWUSR, ...

Update attached as suggested. :-)

Quoted 3 lines

Thanks!

Roland

Probably worth sending it to Greg KH in a new thread so that he picks up on the fact that it is ready to merge. Given the volume of email he probably gets I doubt he is still reading this thread! No need to cc that to the list given we have this email.

Quoted 22 lines · Jonathan Cameron

Quoted 21 lines · Roland Stigge

Hi,

Quoted 8 lines · Jonathan Cameron

Quoted 8 lines · Jonathan Cameron

Quoted 2 lines

Pretty clean and nice driver so it was an easy review and should be trivial to fix up for a merge.

Thanks for your review!

I'm attaching my update. Some notes:

Quoted 7 lines

Quoted 2 lines

* I used the attributes out1_* how I found it in the other DAC driver already available

Cool, except for the out12_raw which could easily be the 12th channel. This has to be one of the two options suggested the other day. May require some macro magic. IIRC the relevant macro would be.

IIO_DEVICE_ATTR_NAMED(out1and2_raw, out1&2_raw, S_IWUSR, ...

Update attached as suggested. :-)

Quoted 8 lines

Quoted 3 lines

Thanks!

Roland

Probably worth sending it to Greg KH in a new thread so that he picks up on the fact that it is ready to merge. Given the volume of email he probably gets I doubt he is still reading this thread! No need to cc that to the list given we have this email.

Heh, I'm still here reading this :)

Roland sent me an updated version, I'll queue that up after .38-rc1 is out.

thanks,

greg k-h

Quoted 8 lines

Heh, I'm still here reading this :)

Roland sent me an updated version, I'll queue that up after .38-rc1 is out.

thanks,

greg k-h

Thanks. I'm seriously impressed at the number of mails you must read every day :)

It's best to convert to the iio subsystem, as that is the way to go in the future.

Hi,

Quoted 2 lines · Greg KH

It's best to convert to the iio subsystem, as that is the way to go in the future.

I'm also preparing a plain PWM driver for the LPC3250 SoC.

Would IIO be an appropriate place?

Thanks in advance,

Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 9 lines · Roland Stigge

Hi,

Quoted 2 lines · Greg KH

It's best to convert to the iio subsystem, as that is the way to go in the future.

I'm also preparing a plain PWM driver for the LPC3250 SoC.

Would IIO be an appropriate place?

No. There is a pwm subsystem. I think thee actual drivers tend to go in the relevant arch directory. So far I think all of the are all arch specific. We do have some recently added support for direct digital synthesis (from Analog devices). There's obviously some common functionality, though the DDS devices tend to be for modulation purposes. Obviously pwm's have loads of uses, but tend to only need a very simple interface. I have wondered if we ought to support this as part of the dds interface, but so far I don't think we have a device anyone would actually use as simply a pwm.

Anyhow, see include/linux/pwm.h for pwm interface. Examples of implementations in arch/arm/plat-pxa and a few others in arch arm. Not that many users yet based on a quick grep.

Anyhow, the code in place looks to have come from Russell King (arm maintainer). I know Bill Gatliff proposed something similar... http://lists.openwall.net/linux-kernel/2010/02/02/353

Hope that helps. Will take a look at ADC driver sometime tomorrow with a bit of luck.

Thanks,

Jonathan

Quoted 9 lines

Thanks in advance,

Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 2 lines

Hope that helps. Will take a look at ADC driver sometime tomorrow with a bit of luck.

Doh! DAC...