Skip to contentSkip to conversation

iio: add m62332 DAC driver

PATCH Superseded Dmitry Eremin-Solenikov 3 participants 6 messages Apr 27, 2015 – May 15, 2015 IIO

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH v3 1/1 iio: add m62332 DAC driver

m62332 is a simple 2-channel DAC used on several Sharp Zaurus boards to control LCD voltage, backlight and sound. The driver use regulators to control the reference voltage and enabling/disabling the DAC.

---
Changes since v2:
* Added M62332_CHANNELS to be used instead of magic value 2
* Changed the probe funciton handles error cases

 drivers/iio/dac/Kconfig  |  10 ++
 drivers/iio/dac/Makefile |   1 +
 drivers/iio/dac/m62332.c | 261 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 272 insertions(+)
 create mode 100644 drivers/iio/dac/m62332.c
drivers/iio/dac/Kconfig +10 -0
index 13471a7..e701e28 100644
@@ -142,6 +142,16 @@ config AD7303
 	  To compile this driver as module choose M here: the module will be called
 	  ad7303.
 
+config M62332
+	tristate "Mitsubishi M62332 DAC driver"
+	depends on I2C
+	help
+	  If you say yes here you get support for the Mitsubishi M62332
+	  (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 m62332.
+
 config MAX517
 	tristate "Maxim MAX517/518/519/520/521 DAC driver"
 	depends on I2C
drivers/iio/dac/Makefile +1 -0
index 52be7e1..63ae056 100644
@@ -16,6 +16,7 @@ obj-$(CONFIG_AD5764) += ad5764.o
 obj-$(CONFIG_AD5791) += ad5791.o
 obj-$(CONFIG_AD5686) += ad5686.o
 obj-$(CONFIG_AD7303) += ad7303.o
+obj-$(CONFIG_M62332) += m62332.o
 obj-$(CONFIG_MAX517) += max517.o
 obj-$(CONFIG_MAX5821) += max5821.o
 obj-$(CONFIG_MCP4725) += mcp4725.o
drivers/iio/dac/m62332.c new file +261 -0

261 lines not on this page · raw message

Signature · 1 line
2.1.4
#1 lore raw
Quoted 4 lines

m62332 is a simple 2-channel DAC used on several Sharp Zaurus boards to control LCD voltage, backlight and sound. The driver use regulators to control the reference voltage and enabling/disabling the DAC.

some minor comments below

Quoted 103 lines

---
Changes since v2:
* Added M62332_CHANNELS to be used instead of magic value 2
* Changed the probe funciton handles error cases

 drivers/iio/dac/Kconfig  |  10 ++
 drivers/iio/dac/Makefile |   1 +
 drivers/iio/dac/m62332.c | 261 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 272 insertions(+)
 create mode 100644 drivers/iio/dac/m62332.c
drivers/iio/dac/Kconfig +10 -0
index 13471a7..e701e28 100644
@@ -142,6 +142,16 @@ config AD7303
 	  To compile this driver as module choose M here: the module will be called
 	  ad7303.
 
+config M62332
+	tristate "Mitsubishi M62332 DAC driver"
+	depends on I2C
+	help
+	  If you say yes here you get support for the Mitsubishi M62332
+	  (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 m62332.
+
 config MAX517
 	tristate "Maxim MAX517/518/519/520/521 DAC driver"
 	depends on I2C
drivers/iio/dac/Makefile +1 -0
index 52be7e1..63ae056 100644
@@ -16,6 +16,7 @@ obj-$(CONFIG_AD5764) += ad5764.o
 obj-$(CONFIG_AD5791) += ad5791.o
 obj-$(CONFIG_AD5686) += ad5686.o
 obj-$(CONFIG_AD7303) += ad7303.o
+obj-$(CONFIG_M62332) += m62332.o
 obj-$(CONFIG_MAX517) += max517.o
 obj-$(CONFIG_MAX5821) += max5821.o
 obj-$(CONFIG_MCP4725) += mcp4725.o
drivers/iio/dac/m62332.c new file +52 -0
new file mode 100644
index 0000000..2dc6712
@@ -0,0 +1,261 @@
+/*
+ *  m62332.c - Support for Mitsubishi m62332 DAC
+ *
+ *  Copyright (c) 2014 Dmitry Eremin-Solenikov
+ *
+ *  Based on max517 driver:
+ *  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.
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/driver.h>
+
+#include <linux/regulator/consumer.h>
+
+#define M62332_CHANNELS 2
+
+struct m62332_data {
+	struct i2c_client	*client;
+	u16			vref_mv;
+	struct regulator	*vcc;
+	struct mutex		mutex;
+	u8			raw[M62332_CHANNELS];
+#ifdef CONFIG_PM_SLEEP
+	u8			save[M62332_CHANNELS];
+#endif
+};
+
+static int m62332_set_value(struct iio_dev *indio_dev,
+	u8 val, int channel)
+{
+	struct m62332_data *data = iio_priv(indio_dev);
+	struct i2c_client *client = data->client;
+	u8 outbuf[2];
+	int res = 0;
+
+	if (val == data->raw[channel])
+		return 0;

drivers/iio/dac/m62332.c line 62

Quoted 10 lines
drivers/iio/dac/m62332.c new file +10 -0
+
+	outbuf[0] = channel;
+	outbuf[1] = val;
+
+	mutex_lock(&data->mutex);
+
+	if (val)
+		res = regulator_enable(data->vcc);
+	if (res)
+		goto out;

I would find it marginally clearer to do
if (val) {
res = regulator_enable(..);
if (res)

    goto out;
}

and leave res uninitialized (feel free to ignore)

Quoted 42 lines
drivers/iio/dac/m62332.c new file +42 -0
+
+	res = i2c_master_send(client, outbuf, 2);
+	if (res >= 0 && res != 2)
+		res = -EIO;
+	if (res < 0)
+		goto out;
+
+	data->raw[channel] = val;
+
+	if (!val)
+		regulator_disable(data->vcc);
+
+	mutex_unlock(&data->mutex);
+
+	return 0;
+
+out:
+	mutex_unlock(&data->mutex);
+
+	return res;
+}
+
+static int m62332_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val,
+			   int *val2,
+			   long m)
+{
+	struct m62332_data *data = iio_priv(indio_dev);
+
+	switch (m) {
+	case IIO_CHAN_INFO_SCALE:
+		/* Corresponds to Vref / 2^(bits) */
+		*val = data->vref_mv;
+		*val2 = 8;
+		return IIO_VAL_FRACTIONAL_LOG2;
+	case IIO_CHAN_INFO_RAW:
+		*val = data->raw[chan->channel];
+		return IIO_VAL_INT;
+	default:
+		break;
+	}

drivers/iio/dac/m62332.c line 114

Quoted 10 lines
drivers/iio/dac/m62332.c new file +10 -0
+	return -EINVAL;
+}
+
+static int m62332_write_raw(struct iio_dev *indio_dev,
+	struct iio_chan_spec const *chan, int val, int val2, long mask)
+{
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:

see write_raw_get_fmt, the default is IIO_VAL_INT_PLUS_MICRO; probably you want to overwrite that function to return IIO_VAL_INT

or at least check val2 == 0

Quoted 91 lines
drivers/iio/dac/m62332.c new file +91 -0
+		if (val < 0 || val > 255)
+			return -EINVAL;
+
+		ret = m62332_set_value(indio_dev, val, chan->channel);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int m62332_suspend(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	struct m62332_data *data = iio_priv(indio_dev);
+	int ret;
+
+	data->save[0] = data->raw[0];
+	data->save[1] = data->raw[1];
+
+	ret = m62332_set_value(indio_dev, 0, 0);
+	if (ret < 0)
+		return ret;
+
+	return m62332_set_value(indio_dev, 0, 1);
+}
+
+static int m62332_resume(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	struct m62332_data *data = iio_priv(indio_dev);
+	int ret;
+
+	ret = m62332_set_value(indio_dev, data->save[0], 0);
+	if (ret < 0)
+		return ret;
+
+	return m62332_set_value(indio_dev, data->save[1], 1);
+}
+
+static SIMPLE_DEV_PM_OPS(m62332_pm_ops, m62332_suspend, m62332_resume);
+#define M62332_PM_OPS (&m62332_pm_ops)
+#else
+#define M62332_PM_OPS NULL
+#endif
+
+static const struct iio_info m62332_info = {
+	.read_raw = m62332_read_raw,
+	.write_raw = m62332_write_raw,
+	.driver_module = THIS_MODULE,
+};
+
+#define M62332_CHANNEL(chan) {				\
+	.type = IIO_VOLTAGE,				\
+	.indexed = 1,					\
+	.output = 1,					\
+	.channel = (chan),				\
+	.datasheet_name = "CH" #chan,			\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
+		BIT(IIO_CHAN_INFO_SCALE),		\
+}
+
+static const struct iio_chan_spec m62332_channels[M62332_CHANNELS] = {
+	M62332_CHANNEL(0),
+	M62332_CHANNEL(1)
+};
+
+static int m62332_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct m62332_data *data;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+
+	mutex_init(&data->mutex);
+
+	data->vcc = devm_regulator_get(&client->dev, "VCC");
+	if (IS_ERR(data->vcc))
+		return PTR_ERR(data->vcc);

drivers/iio/dac/m62332.c line 215

Quoted 10 lines
drivers/iio/dac/m62332.c new file +10 -0
+
+	/* establish that the iio_dev is a child of the i2c device */
+	indio_dev->dev.parent = &client->dev;
+
+	indio_dev->num_channels = M62332_CHANNELS;
+	indio_dev->channels = m62332_channels;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &m62332_info;
+
+	data->vref_mv = regulator_get_voltage(data->vcc) / 1000; /* mV */

regulator_get_voltage() may return an error value

Quoted 47 lines
drivers/iio/dac/m62332.c new file +46 -0
+
+	ret = iio_map_array_register(indio_dev, client->dev.platform_data);
+	if (ret < 0)
+		return ret;
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0)
+		goto err;
+
+	return 0;
+
+err:
+	iio_map_array_unregister(indio_dev);
+	return ret;
+}
+
+static int m62332_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+
+	iio_device_unregister(indio_dev);
+	iio_map_array_unregister(indio_dev);
+
+	return 0;
+}
+
+static const struct i2c_device_id m62332_id[] = {
+	{ "m62332", },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, m62332_id);
+
+static struct i2c_driver m62332_driver = {
+	.driver = {
+		.name	= "m62332",
+		.pm	= M62332_PM_OPS,
+	},
+	.probe		= m62332_probe,
+	.remove		= m62332_remove,
+	.id_table	= m62332_id,
+};
+module_i2c_driver(m62332_driver);
+
+MODULE_AUTHOR("Dmitry Eremin-Solenikov");
+MODULE_DESCRIPTION("M62332 8-bit DAC");
+MODULE_LICENSE("GPL v2");
Signature · 2 lines
Peter Meerwald
+43-664-2444418 (mobile)
#2 lore raw

Hello,

2015-04-27 21:55 GMT+03:00 Peter Meerwald <[email protected]>:

Quoted 6 lines
Quoted 4 lines

m62332 is a simple 2-channel DAC used on several Sharp Zaurus boards to control LCD voltage, backlight and sound. The driver use regulators to control the reference voltage and enabling/disabling the DAC.

some minor comments below

Thank you for the review. I had a followup question, see below.

I'll have to submit a V4 anyway -- adding IIO_CHAN_INFO_OFFSET to reflect correct values being generated by DAC.

Quoted 7 lines
Quoted 7 lines

---
Changes since v2:
* Added M62332_CHANNELS to be used instead of magic value 2
* Changed the probe funciton handles error cases

[skipped]

Quoted 6 lines

drivers/iio/dac/m62332.c line 62

Quoted 5 lines
drivers/iio/dac/m62332.c new file +5 -0
+
+     if (val)
+             res = regulator_enable(data->vcc);
+     if (res)
+             goto out;
Quoted 7 lines

I would find it marginally clearer to do
if (val) {
res = regulator_enable(..);
if (res)

    goto out;
}

and leave res uninitialized (feel free to ignore)

Ack

Quoted 4 lines
Quoted 4 lines
drivers/iio/dac/m62332.c new file +3 -0
+
+     res = i2c_master_send(client, outbuf, 2);
+     if (res >= 0 && res != 2)

[skipped]

Quoted 12 lines

drivers/iio/dac/m62332.c line 114

Quoted 8 lines
drivers/iio/dac/m62332.c new file +8 -0
+
+static int m62332_write_raw(struct iio_dev *indio_dev,
+     struct iio_chan_spec const *chan, int val, int val2, long mask)
+{
+     int ret;
+
+     switch (mask) {
+     case IIO_CHAN_INFO_RAW:

see write_raw_get_fmt, the default is IIO_VAL_INT_PLUS_MICRO; probably you want to overwrite that function to return IIO_VAL_INT

Quoted 1 line

or at least check val2 == 0

This is strange. First, according to iio_write_channel_info(), write_raw_get_fmt()
should not return IIO_VAL_INT. Also none of the DAC write_raw functions that I have checked actually make use of val2 or check that it is 0.

[skipped]

Signature · 2 lines
With best wishes
Dmitry
maintainer
#3 lore raw
Quoted 60 lines · Dmitry Eremin-Solenikov

Hello,

2015-04-27 21:55 GMT+03:00 Peter Meerwald <[email protected]>:

Quoted 6 lines
Quoted 4 lines

m62332 is a simple 2-channel DAC used on several Sharp Zaurus boards to control LCD voltage, backlight and sound. The driver use regulators to control the reference voltage and enabling/disabling the DAC.

some minor comments below

Thank you for the review. I had a followup question, see below.

I'll have to submit a V4 anyway -- adding IIO_CHAN_INFO_OFFSET to reflect correct values being generated by DAC.

Quoted 7 lines
Quoted 7 lines

---
Changes since v2:
* Added M62332_CHANNELS to be used instead of magic value 2
* Changed the probe funciton handles error cases

[skipped]

Quoted 6 lines

drivers/iio/dac/m62332.c line 62

Quoted 5 lines
drivers/iio/dac/m62332.c new file +5 -0
+
+     if (val)
+             res = regulator_enable(data->vcc);
+     if (res)
+             goto out;
Quoted 7 lines

I would find it marginally clearer to do
if (val) {
res = regulator_enable(..);
if (res)

    goto out;
}

and leave res uninitialized (feel free to ignore)

Ack

Quoted 4 lines
Quoted 4 lines
drivers/iio/dac/m62332.c new file +3 -0
+
+     res = i2c_master_send(client, outbuf, 2);
+     if (res >= 0 && res != 2)

[skipped]

Quoted 12 lines

drivers/iio/dac/m62332.c line 114

Quoted 8 lines
drivers/iio/dac/m62332.c new file +8 -0
+
+static int m62332_write_raw(struct iio_dev *indio_dev,
+     struct iio_chan_spec const *chan, int val, int val2, long mask)
+{
+     int ret;
+
+     switch (mask) {
+     case IIO_CHAN_INFO_RAW:

see write_raw_get_fmt, the default is IIO_VAL_INT_PLUS_MICRO; probably you want to overwrite that function to return IIO_VAL_INT

Quoted 5 lines
Quoted 1 line

or at least check val2 == 0

This is strange. First, according to iio_write_channel_info(), write_raw_get_fmt()
should not return IIO_VAL_INT.

Based on the lack of specific handling? (e.g. an entry in the case statement int that function).

It will convert the same as IIO_VAL_INT_PLUS_MICRO but the micro bit will be zero.

Or am I missing an indication that it doesn't support IIO_VAL_INT somewhere?

Quoted 2 lines

Also none of the DAC write_raw functions that
I have checked actually make use of val2 or check that it is 0.

They should check it strictly speaking - feel free to post a series fixing them ;)

Note it is just as valid to leave the type as IIO_VAL_INT_PLUS_MICRO and just check that val2==0 if you prefer.

Quoted 4 lines

[skipped]

#4 lore raw

Hello,

2015-05-12 22:17 GMT+03:00 Jonathan Cameron <[email protected]>:

Quoted 34 lines · Dmitry Eremin-Solenikov
Quoted 33 lines

2015-04-27 21:55 GMT+03:00 Peter Meerwald <[email protected]>:

Quoted 6 lines
Quoted 4 lines

m62332 is a simple 2-channel DAC used on several Sharp Zaurus boards to control LCD voltage, backlight and sound. The driver use regulators to control the reference voltage and enabling/disabling the DAC.

some minor comments below

Thank you for the review. I had a followup question, see below.

I'll have to submit a V4 anyway -- adding IIO_CHAN_INFO_OFFSET to reflect correct values being generated by DAC.

Quoted 20 lines
Quoted 8 lines

---
Changes since v2:
* Added M62332_CHANNELS to be used instead of magic value 2
* Changed the probe funciton handles error cases

drivers/iio/dac/m62332.c line 114

Quoted 8 lines
drivers/iio/dac/m62332.c new file +8 -0
+
+static int m62332_write_raw(struct iio_dev *indio_dev,
+     struct iio_chan_spec const *chan, int val, int val2, long mask)
+{
+     int ret;
+
+     switch (mask) {
+     case IIO_CHAN_INFO_RAW:

see write_raw_get_fmt, the default is IIO_VAL_INT_PLUS_MICRO; probably you want to overwrite that function to return IIO_VAL_INT

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

or at least check val2 == 0

This is strange. First, according to iio_write_channel_info(), write_raw_get_fmt()
should not return IIO_VAL_INT.

Based on the lack of specific handling? (e.g. an entry in the case statement int that function).

Because of the function explicitly returning -EINVAL if write_raw_get_fmt() callback
returns anything except IIO_VAL_INT_PLUS_MICRO or IIO_VAL_INT_PLUS_NANO.

Quoted 5 lines

It will convert the same as IIO_VAL_INT_PLUS_MICRO but the micro bit will be zero.

Or am I missing an indication that it doesn't support IIO_VAL_INT somewhere?

Quoted 7 lines
Quoted 2 lines

Also none of the DAC write_raw functions that
I have checked actually make use of val2 or check that it is 0.

They should check it strictly speaking - feel free to post a series fixing them ;)

Note it is just as valid to leave the type as IIO_VAL_INT_PLUS_MICRO and just check that val2==0 if you prefer.

I'm not so sure that it is a good idea. It means that m62332 will have a behaviour that
differs from behaviour of (some of) the rest of IIO devices.

Signature · 2 lines
With best wishes
Dmitry
maintainer
#5 lore raw
Quoted 47 lines · Dmitry Eremin-Solenikov

Hello,

2015-05-12 22:17 GMT+03:00 Jonathan Cameron <[email protected]>:

Quoted 4 lines · Dmitry Eremin-Solenikov
Quoted 3 lines

2015-04-27 21:55 GMT+03:00 Peter Meerwald <[email protected]>:

Quoted 2 lines
Quoted 2 lines

m62332 is a simple 2-channel DAC used on several Sharp Zaurus

boards to

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

control LCD voltage, backlight and sound. The driver use

regulators to

Quoted 17 lines
Quoted 17 lines
Quoted 3 lines
Quoted 1 line

control the reference voltage and enabling/disabling the DAC.

some minor comments below

Thank you for the review. I had a followup question, see below.

I'll have to submit a V4 anyway -- adding IIO_CHAN_INFO_OFFSET to reflect correct values being generated by DAC.

Quoted 7 lines
Quoted 7 lines

---
Changes since v2:
* Added M62332_CHANNELS to be used instead of magic value 2
* Changed the probe funciton handles error cases

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

drivers/iio/dac/m62332.c

Quoted 3 lines
drivers/iio/dac/m62332.c new file +3 -0
+
+static int m62332_write_raw(struct iio_dev *indio_dev,
+     struct iio_chan_spec const *chan, int val, int val2, long

mask)

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

drivers/iio/dac/m62332.c line 114

Quoted 5 lines
drivers/iio/dac/m62332.c new file +5 -0
+{
+     int ret;
+
+     switch (mask) {
+     case IIO_CHAN_INFO_RAW:

see write_raw_get_fmt, the default is IIO_VAL_INT_PLUS_MICRO;

probably you

Quoted 8 lines
Quoted 7 lines
Quoted 3 lines

want to overwrite that function to return IIO_VAL_INT

or at least check val2 == 0

This is strange. First, according to iio_write_channel_info(), write_raw_get_fmt()
should not return IIO_VAL_INT.

Quoted 8 lines
Quoted 1 line

Based on the lack of specific handling? (e.g. an entry in the case

statement

Quoted 1 line

int that function).

Because of the function explicitly returning -EINVAL if write_raw_get_fmt() callback
returns anything except IIO_VAL_INT_PLUS_MICRO or IIO_VAL_INT_PLUS_NANO.

Gah! Sorry, selective eye sight on my part. We clearly want to add at least the into version to that switch statement.

Quoted 20 lines
Quoted 2 lines

It will convert the same as IIO_VAL_INT_PLUS_MICRO but the micro bit

will be zero.

Quoted 2 lines

Or am I missing an indication that it doesn't support IIO_VAL_INT

somewhere?

Quoted 8 lines
Quoted 4 lines

Also none of the DAC write_raw functions that
I have checked actually make use of val2 or check that it is 0.

They should check it strictly speaking - feel free to post a series fixing them ;)

Note it is just as valid to leave the type as IIO_VAL_INT_PLUS_MICRO

and just check

Quoted 1 line

that val2==0 if you prefer.

I'm not so sure that it is a good idea. It means that m62332 will have a behaviour that
differs from behaviour of (some of) the rest of IIO devices.

Signature · 1 line
Sent from my Android device with K-9 Mail. Please excuse my brevity.