All of lore.kernel.org
 help / color / mirror / Atom feed
* (no subject)
       [not found] <502E10A4.1080800@metafoo.de>
@ 2012-08-20 19:23 ` Jean-François Dagenais
  2012-08-20 19:23   ` [PATCH] iio:ad5446: Add support for I2C based DACs Jean-François Dagenais
  2012-08-21 14:28 ` [PATCH v3.6-rc2 V2 0/2] iio:ad5446: add I2C DACs and header cleanup Jean-Francois Dagenais
  1 sibling, 1 reply; 10+ messages in thread
From: Jean-François Dagenais @ 2012-08-20 19:23 UTC (permalink / raw)
  To: linux-iio; +Cc: lars, device-drivers-devel

This is a follow-up to a preliminary patch from Lars-Peter Clausen to add
support for I2C based DACs on ad5446.c.

Turns out this patch was 1 line away from a hole-in-one, a simple compile
error and voilà! Drove my ad5622's perfectly.

Great job Lars-Peter!

 drivers/iio/dac/Kconfig  |   9 +++--
 drivers/iio/dac/ad5446.c | 362 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------
 drivers/iio/dac/ad5446.h |   5 ++-
 3 files changed, 256 insertions(+), 120 deletions(-)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] iio:ad5446: Add support for I2C based DACs
  2012-08-20 19:23 ` Jean-François Dagenais
@ 2012-08-20 19:23   ` Jean-François Dagenais
  2012-08-21 13:17     ` Jean-François Dagenais
  0 siblings, 1 reply; 10+ messages in thread
From: Jean-François Dagenais @ 2012-08-20 19:23 UTC (permalink / raw)
  To: linux-iio; +Cc: lars, device-drivers-devel, Jean-François Dagenais

From: Lars-Peter Clausen <lars@metafoo.de>

This patch adds support for I2C based single channel DACs to the ad5446
driver. Specifically AD5602, AD5612 and AD5622.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Jean-François Dagenais <jeff.dagenais@gmail.com>
---
 drivers/iio/dac/Kconfig  |   9 +-
 drivers/iio/dac/ad5446.c | 362 ++++++++++++++++++++++++++++++++---------------
 drivers/iio/dac/ad5446.h |   5 +-
 3 files changed, 256 insertions(+), 120 deletions(-)

diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 1be15fa..293b61d 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -57,11 +57,12 @@ config AD5624R_SPI
 
 config AD5446
 	tristate "Analog Devices AD5446 and similar single channel DACs driver"
-	depends on SPI
+	depends on (SPI_MASTER || I2C)
 	help
-	  Say yes here to build support for Analog Devices AD5444, AD5446, AD5450,
-	  AD5451, AD5452, AD5453, AD5512A, AD5541A, AD5542A, AD5543, AD5553, AD5601,
-	  AD5611, AD5620, AD5621, AD5640, AD5660, AD5662 DACs.
+	  Say yes here to build support for Analog Devices AD5602, AD5612, AD5622,
+	  AD5444, AD5446, AD5450, AD5451, AD5452, AD5453, AD5512A, AD5541A, AD5542A,
+	  AD5543, AD5553, AD5601, AD5611, AD5620, AD5621, AD5640, AD5660, AD5662
+	  DACs.
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad5446.
diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c
index 2ca5059..4df4432 100644
--- a/drivers/iio/dac/ad5446.c
+++ b/drivers/iio/dac/ad5446.c
@@ -14,6 +14,7 @@
 #include <linux/sysfs.h>
 #include <linux/list.h>
 #include <linux/spi/spi.h>
+#include <linux/i2c.h>
 #include <linux/regulator/consumer.h>
 #include <linux/err.h>
 #include <linux/module.h>
@@ -23,23 +24,6 @@
 
 #include "ad5446.h"
 
-static int ad5446_write(struct ad5446_state *st, unsigned val)
-{
-	__be16 data = cpu_to_be16(val);
-	return spi_write(st->spi, &data, sizeof(data));
-}
-
-static int ad5660_write(struct ad5446_state *st, unsigned val)
-{
-	uint8_t data[3];
-
-	data[0] = (val >> 16) & 0xFF;
-	data[1] = (val >> 8) & 0xFF;
-	data[2] = val & 0xFF;
-
-	return spi_write(st->spi, data, sizeof(data));
-}
-
 static const char * const ad5446_powerdown_modes[] = {
 	"1kohm_to_gnd", "100kohm_to_gnd", "three_state"
 };
@@ -110,7 +94,7 @@ static ssize_t ad5446_write_dac_powerdown(struct iio_dev *indio_dev,
 	return ret ? ret : len;
 }
 
-static const struct iio_chan_spec_ext_info ad5064_ext_info_powerdown[] = {
+static const struct iio_chan_spec_ext_info ad5446_ext_info_powerdown[] = {
 	{
 		.name = "powerdown",
 		.read = ad5446_read_dac_powerdown,
@@ -136,84 +120,7 @@ static const struct iio_chan_spec_ext_info ad5064_ext_info_powerdown[] = {
 	_AD5446_CHANNEL(bits, storage, shift, NULL)
 
 #define AD5446_CHANNEL_POWERDOWN(bits, storage, shift) \
-	_AD5446_CHANNEL(bits, storage, shift, ad5064_ext_info_powerdown)
-
-static const struct ad5446_chip_info ad5446_chip_info_tbl[] = {
-	[ID_AD5444] = {
-		.channel = AD5446_CHANNEL(12, 16, 2),
-		.write = ad5446_write,
-	},
-	[ID_AD5446] = {
-		.channel = AD5446_CHANNEL(14, 16, 0),
-		.write = ad5446_write,
-	},
-	[ID_AD5450] = {
-		.channel = AD5446_CHANNEL(8, 16, 6),
-		.write = ad5446_write,
-	},
-	[ID_AD5451] = {
-		.channel = AD5446_CHANNEL(10, 16, 4),
-		.write = ad5446_write,
-	},
-	[ID_AD5541A] = {
-		.channel = AD5446_CHANNEL(16, 16, 0),
-		.write = ad5446_write,
-	},
-	[ID_AD5512A] = {
-		.channel = AD5446_CHANNEL(12, 16, 4),
-		.write = ad5446_write,
-	},
-	[ID_AD5553] = {
-		.channel = AD5446_CHANNEL(14, 16, 0),
-		.write = ad5446_write,
-	},
-	[ID_AD5601] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
-		.write = ad5446_write,
-	},
-	[ID_AD5611] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
-		.write = ad5446_write,
-	},
-	[ID_AD5621] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
-		.write = ad5446_write,
-	},
-	[ID_AD5620_2500] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
-		.int_vref_mv = 2500,
-		.write = ad5446_write,
-	},
-	[ID_AD5620_1250] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
-		.int_vref_mv = 1250,
-		.write = ad5446_write,
-	},
-	[ID_AD5640_2500] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
-		.int_vref_mv = 2500,
-		.write = ad5446_write,
-	},
-	[ID_AD5640_1250] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
-		.int_vref_mv = 1250,
-		.write = ad5446_write,
-	},
-	[ID_AD5660_2500] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
-		.int_vref_mv = 2500,
-		.write = ad5660_write,
-	},
-	[ID_AD5660_1250] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
-		.int_vref_mv = 1250,
-		.write = ad5660_write,
-	},
-	[ID_AD5662] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
-		.write = ad5660_write,
-	},
-};
+	_AD5446_CHANNEL(bits, storage, shift, ad5446_ext_info_powerdown)
 
 static int ad5446_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
@@ -272,14 +179,15 @@ static const struct iio_info ad5446_info = {
 	.driver_module = THIS_MODULE,
 };
 
-static int __devinit ad5446_probe(struct spi_device *spi)
+static int __devinit ad5446_probe(struct device *dev, const char *name,
+	const struct ad5446_chip_info *chip_info)
 {
 	struct ad5446_state *st;
 	struct iio_dev *indio_dev;
 	struct regulator *reg;
 	int ret, voltage_uv = 0;
 
-	reg = regulator_get(&spi->dev, "vcc");
+	reg = regulator_get(dev, "vcc");
 	if (!IS_ERR(reg)) {
 		ret = regulator_enable(reg);
 		if (ret)
@@ -294,16 +202,15 @@ static int __devinit ad5446_probe(struct spi_device *spi)
 		goto error_disable_reg;
 	}
 	st = iio_priv(indio_dev);
-	st->chip_info =
-		&ad5446_chip_info_tbl[spi_get_device_id(spi)->driver_data];
+	st->chip_info = chip_info;
 
-	spi_set_drvdata(spi, indio_dev);
+	dev_set_drvdata(dev, indio_dev);
 	st->reg = reg;
-	st->spi = spi;
+	st->dev = dev;
 
-	/* Establish that the iio_dev is a child of the spi device */
-	indio_dev->dev.parent = &spi->dev;
-	indio_dev->name = spi_get_device_id(spi)->name;
+	/* Establish that the iio_dev is a child of the device */
+	indio_dev->dev.parent = dev;
+	indio_dev->name = name;
 	indio_dev->info = &ad5446_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = &st->chip_info->channel;
@@ -316,7 +223,7 @@ static int __devinit ad5446_probe(struct spi_device *spi)
 	else if (voltage_uv)
 		st->vref_mv = voltage_uv / 1000;
 	else
-		dev_warn(&spi->dev, "reference voltage unspecified\n");
+		dev_warn(dev, "reference voltage unspecified\n");
 
 	ret = iio_device_register(indio_dev);
 	if (ret)
@@ -336,9 +243,9 @@ error_put_reg:
 	return ret;
 }
 
-static int ad5446_remove(struct spi_device *spi)
+static int ad5446_remove(struct device *dev)
 {
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5446_state *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
@@ -351,7 +258,106 @@ static int ad5446_remove(struct spi_device *spi)
 	return 0;
 }
 
-static const struct spi_device_id ad5446_id[] = {
+#if IS_ENABLED(CONFIG_SPI_MASTER)
+
+static int ad5446_write(struct ad5446_state *st, unsigned val)
+{
+	struct spi_device *spi = to_spi_device(st->dev);
+	__be16 data = cpu_to_be16(val);
+
+	return spi_write(spi, &data, sizeof(data));
+}
+
+static int ad5660_write(struct ad5446_state *st, unsigned val)
+{
+	struct spi_device *spi = to_spi_device(st->dev);
+	uint8_t data[3];
+
+	data[0] = (val >> 16) & 0xFF;
+	data[1] = (val >> 8) & 0xFF;
+	data[2] = val & 0xFF;
+
+	return spi_write(spi, data, sizeof(data));
+}
+
+static const struct ad5446_chip_info ad5446_spi_chip_info[] = {
+	[ID_AD5444] = {
+		.channel = AD5446_CHANNEL(12, 16, 2),
+		.write = ad5446_write,
+	},
+	[ID_AD5446] = {
+		.channel = AD5446_CHANNEL(14, 16, 0),
+		.write = ad5446_write,
+	},
+	[ID_AD5450] = {
+		.channel = AD5446_CHANNEL(8, 16, 6),
+		.write = ad5446_write,
+	},
+	[ID_AD5451] = {
+		.channel = AD5446_CHANNEL(10, 16, 4),
+		.write = ad5446_write,
+	},
+	[ID_AD5541A] = {
+		.channel = AD5446_CHANNEL(16, 16, 0),
+		.write = ad5446_write,
+	},
+	[ID_AD5512A] = {
+		.channel = AD5446_CHANNEL(12, 16, 4),
+		.write = ad5446_write,
+	},
+	[ID_AD5553] = {
+		.channel = AD5446_CHANNEL(14, 16, 0),
+		.write = ad5446_write,
+	},
+	[ID_AD5601] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
+		.write = ad5446_write,
+	},
+	[ID_AD5611] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
+		.write = ad5446_write,
+	},
+	[ID_AD5621] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
+		.write = ad5446_write,
+	},
+	[ID_AD5620_2500] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
+		.int_vref_mv = 2500,
+		.write = ad5446_write,
+	},
+	[ID_AD5620_1250] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
+		.int_vref_mv = 1250,
+		.write = ad5446_write,
+	},
+	[ID_AD5640_2500] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
+		.int_vref_mv = 2500,
+		.write = ad5446_write,
+	},
+	[ID_AD5640_1250] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
+		.int_vref_mv = 1250,
+		.write = ad5446_write,
+	},
+	[ID_AD5660_2500] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
+		.int_vref_mv = 2500,
+		.write = ad5660_write,
+	},
+	[ID_AD5660_1250] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
+		.int_vref_mv = 1250,
+		.write = ad5660_write,
+	},
+	[ID_AD5662] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
+		.write = ad5660_write,
+	},
+};
+
+static const struct spi_device_id ad5446_spi_ids[] = {
 	{"ad5444", ID_AD5444},
 	{"ad5446", ID_AD5446},
 	{"ad5450", ID_AD5450},
@@ -375,18 +381,144 @@ static const struct spi_device_id ad5446_id[] = {
 	{"ad5662", ID_AD5662},
 	{}
 };
-MODULE_DEVICE_TABLE(spi, ad5446_id);
+MODULE_DEVICE_TABLE(spi, ad5446_spi_ids);
+
+static int __devinit ad5446_spi_probe(struct spi_device *spi)
+{
+	const struct spi_device_id *id = spi_get_device_id(spi);
 
-static struct spi_driver ad5446_driver = {
+	return ad5446_probe(&spi->dev, id->name,
+		&ad5446_spi_chip_info[id->driver_data]);
+}
+
+static int __devexit ad5446_spi_remove(struct spi_device *spi)
+{
+	return ad5446_remove(&spi->dev);
+}
+
+static struct spi_driver ad5446_spi_driver = {
 	.driver = {
 		.name	= "ad5446",
 		.owner	= THIS_MODULE,
 	},
-	.probe		= ad5446_probe,
-	.remove		= __devexit_p(ad5446_remove),
-	.id_table	= ad5446_id,
+	.probe		= ad5446_spi_probe,
+	.remove		= __devexit_p(ad5446_spi_remove),
+	.id_table	= ad5446_spi_ids,
+};
+
+static int __init ad5446_spi_register_driver(void)
+{
+	return spi_register_driver(&ad5446_spi_driver);
+}
+
+static void ad5446_spi_unregister_driver(void)
+{
+	spi_unregister_driver(&ad5446_spi_driver);
+}
+
+#else
+
+static inline int ad5446_spi_register_driver(void) { return 0; }
+static inline void ad5446_spi_unregister_driver(void) { }
+
+#endif
+
+#if IS_ENABLED(CONFIG_I2C)
+
+static int ad5622_write(struct ad5446_state *st, unsigned val)
+{
+	struct i2c_client *client = to_i2c_client(st->dev);
+	__be16 data = cpu_to_be16(val);
+
+	return i2c_master_send(client, (char *)&data, sizeof(data));
+}
+
+static const struct ad5446_chip_info ad5446_i2c_chip_info[] = {
+	[ID_AD5602] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 4),
+		.write = ad5622_write,
+	},
+	[ID_AD5612] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 2),
+		.write = ad5622_write,
+	},
+	[ID_AD5622] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 0),
+		.write = ad5622_write,
+	},
+};
+
+static int __devinit ad5446_i2c_probe(struct i2c_client *i2c,
+	const struct i2c_device_id *id)
+{
+	return ad5446_probe(&i2c->dev, id->name,
+		&ad5446_i2c_chip_info[id->driver_data]);
+}
+
+static int __devexit ad5446_i2c_remove(struct i2c_client *i2c)
+{
+	return ad5446_remove(&i2c->dev);
+}
+
+static const struct i2c_device_id ad5446_i2c_ids[] = {
+	{"ad5602", ID_AD5602},
+	{"ad5612", ID_AD5612},
+	{"ad5622", ID_AD5622},
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, ad5446_i2c_ids);
+
+static struct i2c_driver ad5446_i2c_driver = {
+	.driver = {
+		   .name = "ad5446",
+		   .owner = THIS_MODULE,
+	},
+	.probe = ad5446_i2c_probe,
+	.remove = __devexit_p(ad5446_i2c_remove),
+	.id_table = ad5446_i2c_ids,
 };
-module_spi_driver(ad5446_driver);
+
+static int __init ad5446_i2c_register_driver(void)
+{
+	return i2c_add_driver(&ad5446_i2c_driver);
+}
+
+static void __exit ad5446_i2c_unregister_driver(void)
+{
+	i2c_del_driver(&ad5446_i2c_driver);
+}
+
+#else
+
+static inline int ad5446_i2c_register_driver(void) { return 0; }
+static inline void ad5446_i2c_unregister_driver(void) { }
+
+#endif
+
+static int __init ad5446_init(void)
+{
+	int ret;
+
+	ret = ad5446_spi_register_driver();
+	if (ret)
+		return ret;
+
+	ret = ad5446_i2c_register_driver();
+	if (ret) {
+		ad5446_spi_unregister_driver();
+		return ret;
+	}
+
+	return 0;
+}
+module_init(ad5446_init);
+
+static void __exit ad5446_exit(void)
+{
+	ad5446_i2c_unregister_driver();
+	ad5446_spi_unregister_driver();
+}
+module_exit(ad5446_exit);
 
 MODULE_AUTHOR("Michael Hennerich <hennerich@blackfin.uclinux.org>");
 MODULE_DESCRIPTION("Analog Devices AD5444/AD5446 DAC");
diff --git a/drivers/iio/dac/ad5446.h b/drivers/iio/dac/ad5446.h
index 2934269..aedd7a5 100644
--- a/drivers/iio/dac/ad5446.h
+++ b/drivers/iio/dac/ad5446.h
@@ -38,7 +38,7 @@
  */
 
 struct ad5446_state {
-	struct spi_device		*spi;
+	struct device		*dev;
 	const struct ad5446_chip_info	*chip_info;
 	struct regulator		*reg;
 	unsigned short			vref_mv;
@@ -86,6 +86,9 @@ enum ad5446_supported_device_ids {
 	ID_AD5660_2500,
 	ID_AD5660_1250,
 	ID_AD5662,
+	ID_AD5602,
+	ID_AD5612,
+	ID_AD5622,
 };
 
 #endif /* IIO_DAC_AD5446_H_ */
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] iio:ad5446: Add support for I2C based DACs
  2012-08-20 19:23   ` [PATCH] iio:ad5446: Add support for I2C based DACs Jean-François Dagenais
@ 2012-08-21 13:17     ` Jean-François Dagenais
  0 siblings, 0 replies; 10+ messages in thread
From: Jean-François Dagenais @ 2012-08-21 13:17 UTC (permalink / raw)
  To: Jean-François Dagenais; +Cc: linux-iio, lars, device-drivers-devel

Hi all,

Since this has not been integrated anywhere yet...

Got my brain back after a good night sleep, and something caught my attention
this morning. See below...

On 2012-08-20, at 3:23 PM, Jean-François Dagenais wrote:

> From: Lars-Peter Clausen <lars@metafoo.de>
> 
> This patch adds support for I2C based single channel DACs to the ad5446
> driver. Specifically AD5602, AD5612 and AD5622.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Jean-François Dagenais <jeff.dagenais@gmail.com>
> ---
> drivers/iio/dac/Kconfig  |   9 +-
> drivers/iio/dac/ad5446.c | 362 ++++++++++++++++++++++++++++++++---------------
> drivers/iio/dac/ad5446.h |   5 +-
> 3 files changed, 256 insertions(+), 120 deletions(-)
...
> @@ -375,18 +381,144 @@ static const struct spi_device_id ad5446_id[] = {
> 	{"ad5662", ID_AD5662},
> 	{}
> };
> -MODULE_DEVICE_TABLE(spi, ad5446_id);
> +MODULE_DEVICE_TABLE(spi, ad5446_spi_ids);
> +
> +static int __devinit ad5446_spi_probe(struct spi_device *spi)
> +{
> +	const struct spi_device_id *id = spi_get_device_id(spi);
> 
> -static struct spi_driver ad5446_driver = {
> +	return ad5446_probe(&spi->dev, id->name,
> +		&ad5446_spi_chip_info[id->driver_data]);
> +}
> +
> +static int __devexit ad5446_spi_remove(struct spi_device *spi)
> +{
> +	return ad5446_remove(&spi->dev);
> +}
> +
> +static struct spi_driver ad5446_spi_driver = {
> 	.driver = {
> 		.name	= "ad5446",
> 		.owner	= THIS_MODULE,
> 	},
> -	.probe		= ad5446_probe,
> -	.remove		= __devexit_p(ad5446_remove),
> -	.id_table	= ad5446_id,
> +	.probe		= ad5446_spi_probe,
> +	.remove		= __devexit_p(ad5446_spi_remove),
> +	.id_table	= ad5446_spi_ids,
> +};
> +
> +static int __init ad5446_spi_register_driver(void)
> +{
> +	return spi_register_driver(&ad5446_spi_driver);
> +}
> +
> +static void ad5446_spi_unregister_driver(void)
> +{
> +	spi_unregister_driver(&ad5446_spi_driver);
> +}
> +
> +#else
> +
> +static inline int ad5446_spi_register_driver(void) { return 0; }
> +static inline void ad5446_spi_unregister_driver(void) { }
> +
> +#endif
> +
> +#if IS_ENABLED(CONFIG_I2C)
> +
> +static int ad5622_write(struct ad5446_state *st, unsigned val)
> +{
> +	struct i2c_client *client = to_i2c_client(st->dev);
> +	__be16 data = cpu_to_be16(val);
> +
> +	return i2c_master_send(client, (char *)&data, sizeof(data));
> +}
> +
> +static const struct ad5446_chip_info ad5446_i2c_chip_info[] = {
> +	[ID_AD5602] = {
> +		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 4),
> +		.write = ad5622_write,
> +	},
> +	[ID_AD5612] = {
> +		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 2),
> +		.write = ad5622_write,
> +	},
> +	[ID_AD5622] = {
> +		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 0),
> +		.write = ad5622_write,
> +	},
> +};

This array is uselessly long, see the ID_AD5xyz enum in the .h

> +
> +static int __devinit ad5446_i2c_probe(struct i2c_client *i2c,
> +	const struct i2c_device_id *id)
> +{
> +	return ad5446_probe(&i2c->dev, id->name,
> +		&ad5446_i2c_chip_info[id->driver_data]);
> +}
...
> diff --git a/drivers/iio/dac/ad5446.h b/drivers/iio/dac/ad5446.h
> index 2934269..aedd7a5 100644
> --- a/drivers/iio/dac/ad5446.h
> +++ b/drivers/iio/dac/ad5446.h
> @@ -38,7 +38,7 @@
>  */
> 
> struct ad5446_state {
> -	struct spi_device		*spi;
> +	struct device		*dev;
> 	const struct ad5446_chip_info	*chip_info;
> 	struct regulator		*reg;
> 	unsigned short			vref_mv;
> @@ -86,6 +86,9 @@ enum ad5446_supported_device_ids {
> 	ID_AD5660_2500,
> 	ID_AD5660_1250,
> 	ID_AD5662,


Here we should split the SPI and I2C enums so I2C chips ID, which are exclusive
to each drivers we register (SPI and I2C), start back at value 0. Or at the very
least, with explicit comments, force the enum back to 0.


> +	ID_AD5602,
> +	ID_AD5612,
> +	ID_AD5622,
> };
> 
> #endif /* IIO_DAC_AD5446_H_ */
> -- 
> 1.7.11.3
> 

I will test this, and if I get no feedback, I will submit a V2 accordingly.

/jfd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH  v3.6-rc2 V2 0/2] iio:ad5446: add I2C DACs and header cleanup
       [not found] <502E10A4.1080800@metafoo.de>
  2012-08-20 19:23 ` Jean-François Dagenais
@ 2012-08-21 14:28 ` Jean-Francois Dagenais
  2012-08-21 14:28   ` [PATCH v3.6-rc2 V2 1/2] iio:ad5446: Add support for I2C based DACs Jean-Francois Dagenais
  2012-08-21 14:28   ` [PATCH v3.6-rc2 V2 2/2] iio:ad5446: get rid of private header file Jean-Francois Dagenais
  1 sibling, 2 replies; 10+ messages in thread
From: Jean-Francois Dagenais @ 2012-08-21 14:28 UTC (permalink / raw)
  To: linux-iio; +Cc: lars, device-drivers-devel


Here's a V2 of Lars' "Add support for I2C based DACs" and a commit that
removes the seamingly useless private header file.

 drivers/iio/dac/Kconfig  |   9 ++--
 drivers/iio/dac/ad5446.c | 429 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------------------------
 drivers/iio/dac/ad5446.h |  91 --------------------------------------
 3 files changed, 322 insertions(+), 207 deletions(-)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3.6-rc2 V2 1/2] iio:ad5446: Add support for I2C based DACs
  2012-08-21 14:28 ` [PATCH v3.6-rc2 V2 0/2] iio:ad5446: add I2C DACs and header cleanup Jean-Francois Dagenais
@ 2012-08-21 14:28   ` Jean-Francois Dagenais
  2012-08-22 18:24     ` Jonathan Cameron
  2012-08-21 14:28   ` [PATCH v3.6-rc2 V2 2/2] iio:ad5446: get rid of private header file Jean-Francois Dagenais
  1 sibling, 1 reply; 10+ messages in thread
From: Jean-Francois Dagenais @ 2012-08-21 14:28 UTC (permalink / raw)
  To: linux-iio; +Cc: lars, device-drivers-devel, Jean-Francois Dagenais

This patch adds support for I2C based single channel DACs to the ad5446
driver. Specifically AD5602, AD5612 and AD5622.

V1: from Lars-Peter Clausen <lars@metafoo.de>
V2: Split the device IDs into two enums and move them to the c file.

Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
---
 drivers/iio/dac/Kconfig  |   9 +-
 drivers/iio/dac/ad5446.c | 402 +++++++++++++++++++++++++++++++++--------------
 drivers/iio/dac/ad5446.h |  29 +---
 3 files changed, 293 insertions(+), 147 deletions(-)

diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 1be15fa..293b61d 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -57,11 +57,12 @@ config AD5624R_SPI
 
 config AD5446
 	tristate "Analog Devices AD5446 and similar single channel DACs driver"
-	depends on SPI
+	depends on (SPI_MASTER || I2C)
 	help
-	  Say yes here to build support for Analog Devices AD5444, AD5446, AD5450,
-	  AD5451, AD5452, AD5453, AD5512A, AD5541A, AD5542A, AD5543, AD5553, AD5601,
-	  AD5611, AD5620, AD5621, AD5640, AD5660, AD5662 DACs.
+	  Say yes here to build support for Analog Devices AD5602, AD5612, AD5622,
+	  AD5444, AD5446, AD5450, AD5451, AD5452, AD5453, AD5512A, AD5541A, AD5542A,
+	  AD5543, AD5553, AD5601, AD5611, AD5620, AD5621, AD5640, AD5660, AD5662
+	  DACs.
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad5446.
diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c
index 2ca5059..241665b 100644
--- a/drivers/iio/dac/ad5446.c
+++ b/drivers/iio/dac/ad5446.c
@@ -14,6 +14,7 @@
 #include <linux/sysfs.h>
 #include <linux/list.h>
 #include <linux/spi/spi.h>
+#include <linux/i2c.h>
 #include <linux/regulator/consumer.h>
 #include <linux/err.h>
 #include <linux/module.h>
@@ -23,23 +24,6 @@
 
 #include "ad5446.h"
 
-static int ad5446_write(struct ad5446_state *st, unsigned val)
-{
-	__be16 data = cpu_to_be16(val);
-	return spi_write(st->spi, &data, sizeof(data));
-}
-
-static int ad5660_write(struct ad5446_state *st, unsigned val)
-{
-	uint8_t data[3];
-
-	data[0] = (val >> 16) & 0xFF;
-	data[1] = (val >> 8) & 0xFF;
-	data[2] = val & 0xFF;
-
-	return spi_write(st->spi, data, sizeof(data));
-}
-
 static const char * const ad5446_powerdown_modes[] = {
 	"1kohm_to_gnd", "100kohm_to_gnd", "three_state"
 };
@@ -110,7 +94,7 @@ static ssize_t ad5446_write_dac_powerdown(struct iio_dev *indio_dev,
 	return ret ? ret : len;
 }
 
-static const struct iio_chan_spec_ext_info ad5064_ext_info_powerdown[] = {
+static const struct iio_chan_spec_ext_info ad5446_ext_info_powerdown[] = {
 	{
 		.name = "powerdown",
 		.read = ad5446_read_dac_powerdown,
@@ -136,84 +120,7 @@ static const struct iio_chan_spec_ext_info ad5064_ext_info_powerdown[] = {
 	_AD5446_CHANNEL(bits, storage, shift, NULL)
 
 #define AD5446_CHANNEL_POWERDOWN(bits, storage, shift) \
-	_AD5446_CHANNEL(bits, storage, shift, ad5064_ext_info_powerdown)
-
-static const struct ad5446_chip_info ad5446_chip_info_tbl[] = {
-	[ID_AD5444] = {
-		.channel = AD5446_CHANNEL(12, 16, 2),
-		.write = ad5446_write,
-	},
-	[ID_AD5446] = {
-		.channel = AD5446_CHANNEL(14, 16, 0),
-		.write = ad5446_write,
-	},
-	[ID_AD5450] = {
-		.channel = AD5446_CHANNEL(8, 16, 6),
-		.write = ad5446_write,
-	},
-	[ID_AD5451] = {
-		.channel = AD5446_CHANNEL(10, 16, 4),
-		.write = ad5446_write,
-	},
-	[ID_AD5541A] = {
-		.channel = AD5446_CHANNEL(16, 16, 0),
-		.write = ad5446_write,
-	},
-	[ID_AD5512A] = {
-		.channel = AD5446_CHANNEL(12, 16, 4),
-		.write = ad5446_write,
-	},
-	[ID_AD5553] = {
-		.channel = AD5446_CHANNEL(14, 16, 0),
-		.write = ad5446_write,
-	},
-	[ID_AD5601] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
-		.write = ad5446_write,
-	},
-	[ID_AD5611] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
-		.write = ad5446_write,
-	},
-	[ID_AD5621] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
-		.write = ad5446_write,
-	},
-	[ID_AD5620_2500] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
-		.int_vref_mv = 2500,
-		.write = ad5446_write,
-	},
-	[ID_AD5620_1250] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
-		.int_vref_mv = 1250,
-		.write = ad5446_write,
-	},
-	[ID_AD5640_2500] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
-		.int_vref_mv = 2500,
-		.write = ad5446_write,
-	},
-	[ID_AD5640_1250] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
-		.int_vref_mv = 1250,
-		.write = ad5446_write,
-	},
-	[ID_AD5660_2500] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
-		.int_vref_mv = 2500,
-		.write = ad5660_write,
-	},
-	[ID_AD5660_1250] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
-		.int_vref_mv = 1250,
-		.write = ad5660_write,
-	},
-	[ID_AD5662] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
-		.write = ad5660_write,
-	},
-};
+	_AD5446_CHANNEL(bits, storage, shift, ad5446_ext_info_powerdown)
 
 static int ad5446_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
@@ -272,14 +179,15 @@ static const struct iio_info ad5446_info = {
 	.driver_module = THIS_MODULE,
 };
 
-static int __devinit ad5446_probe(struct spi_device *spi)
+static int __devinit ad5446_probe(struct device *dev, const char *name,
+	const struct ad5446_chip_info *chip_info)
 {
 	struct ad5446_state *st;
 	struct iio_dev *indio_dev;
 	struct regulator *reg;
 	int ret, voltage_uv = 0;
 
-	reg = regulator_get(&spi->dev, "vcc");
+	reg = regulator_get(dev, "vcc");
 	if (!IS_ERR(reg)) {
 		ret = regulator_enable(reg);
 		if (ret)
@@ -294,16 +202,15 @@ static int __devinit ad5446_probe(struct spi_device *spi)
 		goto error_disable_reg;
 	}
 	st = iio_priv(indio_dev);
-	st->chip_info =
-		&ad5446_chip_info_tbl[spi_get_device_id(spi)->driver_data];
+	st->chip_info = chip_info;
 
-	spi_set_drvdata(spi, indio_dev);
+	dev_set_drvdata(dev, indio_dev);
 	st->reg = reg;
-	st->spi = spi;
+	st->dev = dev;
 
-	/* Establish that the iio_dev is a child of the spi device */
-	indio_dev->dev.parent = &spi->dev;
-	indio_dev->name = spi_get_device_id(spi)->name;
+	/* Establish that the iio_dev is a child of the device */
+	indio_dev->dev.parent = dev;
+	indio_dev->name = name;
 	indio_dev->info = &ad5446_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = &st->chip_info->channel;
@@ -316,7 +223,7 @@ static int __devinit ad5446_probe(struct spi_device *spi)
 	else if (voltage_uv)
 		st->vref_mv = voltage_uv / 1000;
 	else
-		dev_warn(&spi->dev, "reference voltage unspecified\n");
+		dev_warn(dev, "reference voltage unspecified\n");
 
 	ret = iio_device_register(indio_dev);
 	if (ret)
@@ -336,9 +243,9 @@ error_put_reg:
 	return ret;
 }
 
-static int ad5446_remove(struct spi_device *spi)
+static int ad5446_remove(struct device *dev)
 {
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5446_state *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
@@ -351,7 +258,133 @@ static int ad5446_remove(struct spi_device *spi)
 	return 0;
 }
 
-static const struct spi_device_id ad5446_id[] = {
+#if IS_ENABLED(CONFIG_SPI_MASTER)
+
+static int ad5446_write(struct ad5446_state *st, unsigned val)
+{
+	struct spi_device *spi = to_spi_device(st->dev);
+	__be16 data = cpu_to_be16(val);
+
+	return spi_write(spi, &data, sizeof(data));
+}
+
+static int ad5660_write(struct ad5446_state *st, unsigned val)
+{
+	struct spi_device *spi = to_spi_device(st->dev);
+	uint8_t data[3];
+
+	data[0] = (val >> 16) & 0xFF;
+	data[1] = (val >> 8) & 0xFF;
+	data[2] = val & 0xFF;
+
+	return spi_write(spi, data, sizeof(data));
+}
+
+/**
+ * ad5446_supported_spi_device_ids:
+ * The AD5620/40/60 parts are available in different fixed internal reference
+ * voltage options. The actual part numbers may look differently
+ * (and a bit cryptic), however this style is used to make clear which
+ * parts are supported here.
+ */
+enum ad5446_supported_spi_device_ids {
+	ID_AD5444,
+	ID_AD5446,
+	ID_AD5450,
+	ID_AD5451,
+	ID_AD5541A,
+	ID_AD5512A,
+	ID_AD5553,
+	ID_AD5601,
+	ID_AD5611,
+	ID_AD5621,
+	ID_AD5620_2500,
+	ID_AD5620_1250,
+	ID_AD5640_2500,
+	ID_AD5640_1250,
+	ID_AD5660_2500,
+	ID_AD5660_1250,
+	ID_AD5662,
+};
+
+static const struct ad5446_chip_info ad5446_spi_chip_info[] = {
+	[ID_AD5444] = {
+		.channel = AD5446_CHANNEL(12, 16, 2),
+		.write = ad5446_write,
+	},
+	[ID_AD5446] = {
+		.channel = AD5446_CHANNEL(14, 16, 0),
+		.write = ad5446_write,
+	},
+	[ID_AD5450] = {
+		.channel = AD5446_CHANNEL(8, 16, 6),
+		.write = ad5446_write,
+	},
+	[ID_AD5451] = {
+		.channel = AD5446_CHANNEL(10, 16, 4),
+		.write = ad5446_write,
+	},
+	[ID_AD5541A] = {
+		.channel = AD5446_CHANNEL(16, 16, 0),
+		.write = ad5446_write,
+	},
+	[ID_AD5512A] = {
+		.channel = AD5446_CHANNEL(12, 16, 4),
+		.write = ad5446_write,
+	},
+	[ID_AD5553] = {
+		.channel = AD5446_CHANNEL(14, 16, 0),
+		.write = ad5446_write,
+	},
+	[ID_AD5601] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
+		.write = ad5446_write,
+	},
+	[ID_AD5611] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
+		.write = ad5446_write,
+	},
+	[ID_AD5621] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
+		.write = ad5446_write,
+	},
+	[ID_AD5620_2500] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
+		.int_vref_mv = 2500,
+		.write = ad5446_write,
+	},
+	[ID_AD5620_1250] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
+		.int_vref_mv = 1250,
+		.write = ad5446_write,
+	},
+	[ID_AD5640_2500] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
+		.int_vref_mv = 2500,
+		.write = ad5446_write,
+	},
+	[ID_AD5640_1250] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
+		.int_vref_mv = 1250,
+		.write = ad5446_write,
+	},
+	[ID_AD5660_2500] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
+		.int_vref_mv = 2500,
+		.write = ad5660_write,
+	},
+	[ID_AD5660_1250] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
+		.int_vref_mv = 1250,
+		.write = ad5660_write,
+	},
+	[ID_AD5662] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
+		.write = ad5660_write,
+	},
+};
+
+static const struct spi_device_id ad5446_spi_ids[] = {
 	{"ad5444", ID_AD5444},
 	{"ad5446", ID_AD5446},
 	{"ad5450", ID_AD5450},
@@ -375,18 +408,157 @@ static const struct spi_device_id ad5446_id[] = {
 	{"ad5662", ID_AD5662},
 	{}
 };
-MODULE_DEVICE_TABLE(spi, ad5446_id);
+MODULE_DEVICE_TABLE(spi, ad5446_spi_ids);
+
+static int __devinit ad5446_spi_probe(struct spi_device *spi)
+{
+	const struct spi_device_id *id = spi_get_device_id(spi);
+
+	return ad5446_probe(&spi->dev, id->name,
+		&ad5446_spi_chip_info[id->driver_data]);
+}
 
-static struct spi_driver ad5446_driver = {
+static int __devexit ad5446_spi_remove(struct spi_device *spi)
+{
+	return ad5446_remove(&spi->dev);
+}
+
+static struct spi_driver ad5446_spi_driver = {
 	.driver = {
 		.name	= "ad5446",
 		.owner	= THIS_MODULE,
 	},
-	.probe		= ad5446_probe,
-	.remove		= __devexit_p(ad5446_remove),
-	.id_table	= ad5446_id,
+	.probe		= ad5446_spi_probe,
+	.remove		= __devexit_p(ad5446_spi_remove),
+	.id_table	= ad5446_spi_ids,
+};
+
+static int __init ad5446_spi_register_driver(void)
+{
+	return spi_register_driver(&ad5446_spi_driver);
+}
+
+static void ad5446_spi_unregister_driver(void)
+{
+	spi_unregister_driver(&ad5446_spi_driver);
+}
+
+#else
+
+static inline int ad5446_spi_register_driver(void) { return 0; }
+static inline void ad5446_spi_unregister_driver(void) { }
+
+#endif
+
+#if IS_ENABLED(CONFIG_I2C)
+
+static int ad5622_write(struct ad5446_state *st, unsigned val)
+{
+	struct i2c_client *client = to_i2c_client(st->dev);
+	__be16 data = cpu_to_be16(val);
+
+	return i2c_master_send(client, (char *)&data, sizeof(data));
+}
+
+/**
+ * ad5446_supported_i2c_device_ids:
+ * The AD5620/40/60 parts are available in different fixed internal reference
+ * voltage options. The actual part numbers may look differently
+ * (and a bit cryptic), however this style is used to make clear which
+ * parts are supported here.
+ */
+enum ad5446_supported_i2c_device_ids {
+	ID_AD5602,
+	ID_AD5612,
+	ID_AD5622,
+};
+
+static const struct ad5446_chip_info ad5446_i2c_chip_info[] = {
+	[ID_AD5602] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 4),
+		.write = ad5622_write,
+	},
+	[ID_AD5612] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 2),
+		.write = ad5622_write,
+	},
+	[ID_AD5622] = {
+		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 0),
+		.write = ad5622_write,
+	},
 };
-module_spi_driver(ad5446_driver);
+
+static int __devinit ad5446_i2c_probe(struct i2c_client *i2c,
+	const struct i2c_device_id *id)
+{
+	return ad5446_probe(&i2c->dev, id->name,
+		&ad5446_i2c_chip_info[id->driver_data]);
+}
+
+static int __devexit ad5446_i2c_remove(struct i2c_client *i2c)
+{
+	return ad5446_remove(&i2c->dev);
+}
+
+static const struct i2c_device_id ad5446_i2c_ids[] = {
+	{"ad5602", ID_AD5602},
+	{"ad5612", ID_AD5612},
+	{"ad5622", ID_AD5622},
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, ad5446_i2c_ids);
+
+static struct i2c_driver ad5446_i2c_driver = {
+	.driver = {
+		   .name = "ad5446",
+		   .owner = THIS_MODULE,
+	},
+	.probe = ad5446_i2c_probe,
+	.remove = __devexit_p(ad5446_i2c_remove),
+	.id_table = ad5446_i2c_ids,
+};
+
+static int __init ad5446_i2c_register_driver(void)
+{
+	return i2c_add_driver(&ad5446_i2c_driver);
+}
+
+static void __exit ad5446_i2c_unregister_driver(void)
+{
+	i2c_del_driver(&ad5446_i2c_driver);
+}
+
+#else
+
+static inline int ad5446_i2c_register_driver(void) { return 0; }
+static inline void ad5446_i2c_unregister_driver(void) { }
+
+#endif
+
+static int __init ad5446_init(void)
+{
+	int ret;
+
+	ret = ad5446_spi_register_driver();
+	if (ret)
+		return ret;
+
+	ret = ad5446_i2c_register_driver();
+	if (ret) {
+		ad5446_spi_unregister_driver();
+		return ret;
+	}
+
+	return 0;
+}
+module_init(ad5446_init);
+
+static void __exit ad5446_exit(void)
+{
+	ad5446_i2c_unregister_driver();
+	ad5446_spi_unregister_driver();
+}
+module_exit(ad5446_exit);
 
 MODULE_AUTHOR("Michael Hennerich <hennerich@blackfin.uclinux.org>");
 MODULE_DESCRIPTION("Analog Devices AD5444/AD5446 DAC");
diff --git a/drivers/iio/dac/ad5446.h b/drivers/iio/dac/ad5446.h
index 2934269..6b7a176 100644
--- a/drivers/iio/dac/ad5446.h
+++ b/drivers/iio/dac/ad5446.h
@@ -38,7 +38,7 @@
  */
 
 struct ad5446_state {
-	struct spi_device		*spi;
+	struct device		*dev;
 	const struct ad5446_chip_info	*chip_info;
 	struct regulator		*reg;
 	unsigned short			vref_mv;
@@ -60,32 +60,5 @@ struct ad5446_chip_info {
 	int			(*write)(struct ad5446_state *st, unsigned val);
 };
 
-/**
- * ad5446_supported_device_ids:
- * The AD5620/40/60 parts are available in different fixed internal reference
- * voltage options. The actual part numbers may look differently
- * (and a bit cryptic), however this style is used to make clear which
- * parts are supported here.
- */
-
-enum ad5446_supported_device_ids {
-	ID_AD5444,
-	ID_AD5446,
-	ID_AD5450,
-	ID_AD5451,
-	ID_AD5541A,
-	ID_AD5512A,
-	ID_AD5553,
-	ID_AD5601,
-	ID_AD5611,
-	ID_AD5621,
-	ID_AD5620_2500,
-	ID_AD5620_1250,
-	ID_AD5640_2500,
-	ID_AD5640_1250,
-	ID_AD5660_2500,
-	ID_AD5660_1250,
-	ID_AD5662,
-};
 
 #endif /* IIO_DAC_AD5446_H_ */
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3.6-rc2 V2 2/2] iio:ad5446: get rid of private header file
  2012-08-21 14:28 ` [PATCH v3.6-rc2 V2 0/2] iio:ad5446: add I2C DACs and header cleanup Jean-Francois Dagenais
  2012-08-21 14:28   ` [PATCH v3.6-rc2 V2 1/2] iio:ad5446: Add support for I2C based DACs Jean-Francois Dagenais
@ 2012-08-21 14:28   ` Jean-Francois Dagenais
  1 sibling, 0 replies; 10+ messages in thread
From: Jean-Francois Dagenais @ 2012-08-21 14:28 UTC (permalink / raw)
  To: linux-iio; +Cc: lars, device-drivers-devel, Jean-Francois Dagenais

Most of the defines in there were not even used, and the structs left are
private to the .c file. Makes the driver more in line with most of the
kernel drivers.

Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
---
 drivers/iio/dac/ad5446.c | 35 +++++++++++++++++++++++++-
 drivers/iio/dac/ad5446.h | 64 ------------------------------------------------
 2 files changed, 34 insertions(+), 65 deletions(-)
 delete mode 100644 drivers/iio/dac/ad5446.h

diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c
index 241665b..7f11c1c 100644
--- a/drivers/iio/dac/ad5446.c
+++ b/drivers/iio/dac/ad5446.c
@@ -22,7 +22,40 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
-#include "ad5446.h"
+#define MODE_PWRDWN_1k		0x1
+#define MODE_PWRDWN_100k	0x2
+#define MODE_PWRDWN_TRISTATE	0x3
+
+/**
+ * struct ad5446_state - driver instance specific data
+ * @spi:		spi_device
+ * @chip_info:		chip model specific constants, available modes etc
+ * @reg:		supply regulator
+ * @vref_mv:		actual reference voltage used
+ */
+
+struct ad5446_state {
+	struct device		*dev;
+	const struct ad5446_chip_info	*chip_info;
+	struct regulator		*reg;
+	unsigned short			vref_mv;
+	unsigned			cached_val;
+	unsigned			pwr_down_mode;
+	unsigned			pwr_down;
+};
+
+/**
+ * struct ad5446_chip_info - chip specific information
+ * @channel:		channel spec for the DAC
+ * @int_vref_mv:	AD5620/40/60: the internal reference voltage
+ * @write:		chip specific helper function to write to the register
+ */
+
+struct ad5446_chip_info {
+	struct iio_chan_spec	channel;
+	u16			int_vref_mv;
+	int			(*write)(struct ad5446_state *st, unsigned val);
+};
 
 static const char * const ad5446_powerdown_modes[] = {
 	"1kohm_to_gnd", "100kohm_to_gnd", "three_state"
diff --git a/drivers/iio/dac/ad5446.h b/drivers/iio/dac/ad5446.h
deleted file mode 100644
index 6b7a176..0000000
--- a/drivers/iio/dac/ad5446.h
+++ /dev/null
@@ -1,64 +0,0 @@
-/*
- * AD5446 SPI DAC driver
- *
- * Copyright 2010 Analog Devices Inc.
- *
- * Licensed under the GPL-2 or later.
- */
-#ifndef IIO_DAC_AD5446_H_
-#define IIO_DAC_AD5446_H_
-
-/* DAC Control Bits */
-
-#define AD5446_LOAD		(0x0 << 14) /* Load and update */
-#define AD5446_SDO_DIS		(0x1 << 14) /* Disable SDO */
-#define AD5446_NOP		(0x2 << 14) /* No operation */
-#define AD5446_CLK_RISING	(0x3 << 14) /* Clock data on rising edge */
-
-#define AD5620_LOAD		(0x0 << 14) /* Load and update Norm Operation*/
-#define AD5620_PWRDWN_1k	(0x1 << 14) /* Power-down: 1kOhm to GND */
-#define AD5620_PWRDWN_100k	(0x2 << 14) /* Power-down: 100kOhm to GND */
-#define AD5620_PWRDWN_TRISTATE	(0x3 << 14) /* Power-down: Three-state */
-
-#define AD5660_LOAD		(0x0 << 16) /* Load and update Norm Operation*/
-#define AD5660_PWRDWN_1k	(0x1 << 16) /* Power-down: 1kOhm to GND */
-#define AD5660_PWRDWN_100k	(0x2 << 16) /* Power-down: 100kOhm to GND */
-#define AD5660_PWRDWN_TRISTATE	(0x3 << 16) /* Power-down: Three-state */
-
-#define MODE_PWRDWN_1k		0x1
-#define MODE_PWRDWN_100k	0x2
-#define MODE_PWRDWN_TRISTATE	0x3
-
-/**
- * struct ad5446_state - driver instance specific data
- * @spi:		spi_device
- * @chip_info:		chip model specific constants, available modes etc
- * @reg:		supply regulator
- * @vref_mv:		actual reference voltage used
- */
-
-struct ad5446_state {
-	struct device		*dev;
-	const struct ad5446_chip_info	*chip_info;
-	struct regulator		*reg;
-	unsigned short			vref_mv;
-	unsigned			cached_val;
-	unsigned			pwr_down_mode;
-	unsigned			pwr_down;
-};
-
-/**
- * struct ad5446_chip_info - chip specific information
- * @channel:		channel spec for the DAC
- * @int_vref_mv:	AD5620/40/60: the internal reference voltage
- * @write:		chip specific helper function to write to the register
- */
-
-struct ad5446_chip_info {
-	struct iio_chan_spec	channel;
-	u16			int_vref_mv;
-	int			(*write)(struct ad5446_state *st, unsigned val);
-};
-
-
-#endif /* IIO_DAC_AD5446_H_ */
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3.6-rc2 V2 1/2] iio:ad5446: Add support for I2C based DACs
  2012-08-21 14:28   ` [PATCH v3.6-rc2 V2 1/2] iio:ad5446: Add support for I2C based DACs Jean-Francois Dagenais
@ 2012-08-22 18:24     ` Jonathan Cameron
  2012-08-24 11:38       ` Lars-Peter Clausen
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2012-08-22 18:24 UTC (permalink / raw)
  To: Jean-Francois Dagenais; +Cc: linux-iio, lars, device-drivers-devel

On 08/21/2012 03:28 PM, Jean-Francois Dagenais wrote:
> This patch adds support for I2C based single channel DACs to the ad5446
> driver. Specifically AD5602, AD5612 and AD5622.
> 
> V1: from Lars-Peter Clausen <lars@metafoo.de>
> V2: Split the device IDs into two enums and move them to the c file.
Sensible change.  Patch is fine but given we are still well away from
the merge window I'll wait for Lars-Peter's response before merging this.

> 
> Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
> ---
>  drivers/iio/dac/Kconfig  |   9 +-
>  drivers/iio/dac/ad5446.c | 402 +++++++++++++++++++++++++++++++++--------------
>  drivers/iio/dac/ad5446.h |  29 +---
>  3 files changed, 293 insertions(+), 147 deletions(-)
> 
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 1be15fa..293b61d 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -57,11 +57,12 @@ config AD5624R_SPI
>  
>  config AD5446
>  	tristate "Analog Devices AD5446 and similar single channel DACs driver"
> -	depends on SPI
> +	depends on (SPI_MASTER || I2C)
>  	help
> -	  Say yes here to build support for Analog Devices AD5444, AD5446, AD5450,
> -	  AD5451, AD5452, AD5453, AD5512A, AD5541A, AD5542A, AD5543, AD5553, AD5601,
> -	  AD5611, AD5620, AD5621, AD5640, AD5660, AD5662 DACs.
> +	  Say yes here to build support for Analog Devices AD5602, AD5612, AD5622,
> +	  AD5444, AD5446, AD5450, AD5451, AD5452, AD5453, AD5512A, AD5541A, AD5542A,
> +	  AD5543, AD5553, AD5601, AD5611, AD5620, AD5621, AD5640, AD5660, AD5662
> +	  DACs.
>  
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ad5446.
> diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c
> index 2ca5059..241665b 100644
> --- a/drivers/iio/dac/ad5446.c
> +++ b/drivers/iio/dac/ad5446.c
> @@ -14,6 +14,7 @@
>  #include <linux/sysfs.h>
>  #include <linux/list.h>
>  #include <linux/spi/spi.h>
> +#include <linux/i2c.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/err.h>
>  #include <linux/module.h>
> @@ -23,23 +24,6 @@
>  
>  #include "ad5446.h"
>  
> -static int ad5446_write(struct ad5446_state *st, unsigned val)
> -{
> -	__be16 data = cpu_to_be16(val);
> -	return spi_write(st->spi, &data, sizeof(data));
> -}
> -
> -static int ad5660_write(struct ad5446_state *st, unsigned val)
> -{
> -	uint8_t data[3];
> -
> -	data[0] = (val >> 16) & 0xFF;
> -	data[1] = (val >> 8) & 0xFF;
> -	data[2] = val & 0xFF;
> -
> -	return spi_write(st->spi, data, sizeof(data));
> -}
> -
>  static const char * const ad5446_powerdown_modes[] = {
>  	"1kohm_to_gnd", "100kohm_to_gnd", "three_state"
>  };
> @@ -110,7 +94,7 @@ static ssize_t ad5446_write_dac_powerdown(struct iio_dev *indio_dev,
>  	return ret ? ret : len;
>  }
>  
> -static const struct iio_chan_spec_ext_info ad5064_ext_info_powerdown[] = {
> +static const struct iio_chan_spec_ext_info ad5446_ext_info_powerdown[] = {
>  	{
>  		.name = "powerdown",
>  		.read = ad5446_read_dac_powerdown,
> @@ -136,84 +120,7 @@ static const struct iio_chan_spec_ext_info ad5064_ext_info_powerdown[] = {
>  	_AD5446_CHANNEL(bits, storage, shift, NULL)
>  
>  #define AD5446_CHANNEL_POWERDOWN(bits, storage, shift) \
> -	_AD5446_CHANNEL(bits, storage, shift, ad5064_ext_info_powerdown)
> -
> -static const struct ad5446_chip_info ad5446_chip_info_tbl[] = {
> -	[ID_AD5444] = {
> -		.channel = AD5446_CHANNEL(12, 16, 2),
> -		.write = ad5446_write,
> -	},
> -	[ID_AD5446] = {
> -		.channel = AD5446_CHANNEL(14, 16, 0),
> -		.write = ad5446_write,
> -	},
> -	[ID_AD5450] = {
> -		.channel = AD5446_CHANNEL(8, 16, 6),
> -		.write = ad5446_write,
> -	},
> -	[ID_AD5451] = {
> -		.channel = AD5446_CHANNEL(10, 16, 4),
> -		.write = ad5446_write,
> -	},
> -	[ID_AD5541A] = {
> -		.channel = AD5446_CHANNEL(16, 16, 0),
> -		.write = ad5446_write,
> -	},
> -	[ID_AD5512A] = {
> -		.channel = AD5446_CHANNEL(12, 16, 4),
> -		.write = ad5446_write,
> -	},
> -	[ID_AD5553] = {
> -		.channel = AD5446_CHANNEL(14, 16, 0),
> -		.write = ad5446_write,
> -	},
> -	[ID_AD5601] = {
> -		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
> -		.write = ad5446_write,
> -	},
> -	[ID_AD5611] = {
> -		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
> -		.write = ad5446_write,
> -	},
> -	[ID_AD5621] = {
> -		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
> -		.write = ad5446_write,
> -	},
> -	[ID_AD5620_2500] = {
> -		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
> -		.int_vref_mv = 2500,
> -		.write = ad5446_write,
> -	},
> -	[ID_AD5620_1250] = {
> -		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
> -		.int_vref_mv = 1250,
> -		.write = ad5446_write,
> -	},
> -	[ID_AD5640_2500] = {
> -		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
> -		.int_vref_mv = 2500,
> -		.write = ad5446_write,
> -	},
> -	[ID_AD5640_1250] = {
> -		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
> -		.int_vref_mv = 1250,
> -		.write = ad5446_write,
> -	},
> -	[ID_AD5660_2500] = {
> -		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
> -		.int_vref_mv = 2500,
> -		.write = ad5660_write,
> -	},
> -	[ID_AD5660_1250] = {
> -		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
> -		.int_vref_mv = 1250,
> -		.write = ad5660_write,
> -	},
> -	[ID_AD5662] = {
> -		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
> -		.write = ad5660_write,
> -	},
> -};
> +	_AD5446_CHANNEL(bits, storage, shift, ad5446_ext_info_powerdown)
>  
>  static int ad5446_read_raw(struct iio_dev *indio_dev,
>  			   struct iio_chan_spec const *chan,
> @@ -272,14 +179,15 @@ static const struct iio_info ad5446_info = {
>  	.driver_module = THIS_MODULE,
>  };
>  
> -static int __devinit ad5446_probe(struct spi_device *spi)
> +static int __devinit ad5446_probe(struct device *dev, const char *name,
> +	const struct ad5446_chip_info *chip_info)
>  {
>  	struct ad5446_state *st;
>  	struct iio_dev *indio_dev;
>  	struct regulator *reg;
>  	int ret, voltage_uv = 0;
>  
> -	reg = regulator_get(&spi->dev, "vcc");
> +	reg = regulator_get(dev, "vcc");
>  	if (!IS_ERR(reg)) {
>  		ret = regulator_enable(reg);
>  		if (ret)
> @@ -294,16 +202,15 @@ static int __devinit ad5446_probe(struct spi_device *spi)
>  		goto error_disable_reg;
>  	}
>  	st = iio_priv(indio_dev);
> -	st->chip_info =
> -		&ad5446_chip_info_tbl[spi_get_device_id(spi)->driver_data];
> +	st->chip_info = chip_info;
>  
> -	spi_set_drvdata(spi, indio_dev);
> +	dev_set_drvdata(dev, indio_dev);
>  	st->reg = reg;
> -	st->spi = spi;
> +	st->dev = dev;
>  
> -	/* Establish that the iio_dev is a child of the spi device */
> -	indio_dev->dev.parent = &spi->dev;
> -	indio_dev->name = spi_get_device_id(spi)->name;
> +	/* Establish that the iio_dev is a child of the device */
> +	indio_dev->dev.parent = dev;
> +	indio_dev->name = name;
>  	indio_dev->info = &ad5446_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	indio_dev->channels = &st->chip_info->channel;
> @@ -316,7 +223,7 @@ static int __devinit ad5446_probe(struct spi_device *spi)
>  	else if (voltage_uv)
>  		st->vref_mv = voltage_uv / 1000;
>  	else
> -		dev_warn(&spi->dev, "reference voltage unspecified\n");
> +		dev_warn(dev, "reference voltage unspecified\n");
>  
>  	ret = iio_device_register(indio_dev);
>  	if (ret)
> @@ -336,9 +243,9 @@ error_put_reg:
>  	return ret;
>  }
>  
> -static int ad5446_remove(struct spi_device *spi)
> +static int ad5446_remove(struct device *dev)
>  {
> -	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5446_state *st = iio_priv(indio_dev);
>  
>  	iio_device_unregister(indio_dev);
> @@ -351,7 +258,133 @@ static int ad5446_remove(struct spi_device *spi)
>  	return 0;
>  }
>  
> -static const struct spi_device_id ad5446_id[] = {
> +#if IS_ENABLED(CONFIG_SPI_MASTER)
> +
> +static int ad5446_write(struct ad5446_state *st, unsigned val)
> +{
> +	struct spi_device *spi = to_spi_device(st->dev);
> +	__be16 data = cpu_to_be16(val);
> +
> +	return spi_write(spi, &data, sizeof(data));
> +}
> +
> +static int ad5660_write(struct ad5446_state *st, unsigned val)
> +{
> +	struct spi_device *spi = to_spi_device(st->dev);
> +	uint8_t data[3];
> +
> +	data[0] = (val >> 16) & 0xFF;
> +	data[1] = (val >> 8) & 0xFF;
> +	data[2] = val & 0xFF;
> +
> +	return spi_write(spi, data, sizeof(data));
> +}
> +
> +/**
> + * ad5446_supported_spi_device_ids:
> + * The AD5620/40/60 parts are available in different fixed internal reference
> + * voltage options. The actual part numbers may look differently
> + * (and a bit cryptic), however this style is used to make clear which
> + * parts are supported here.
> + */
> +enum ad5446_supported_spi_device_ids {
> +	ID_AD5444,
> +	ID_AD5446,
> +	ID_AD5450,
> +	ID_AD5451,
> +	ID_AD5541A,
> +	ID_AD5512A,
> +	ID_AD5553,
> +	ID_AD5601,
> +	ID_AD5611,
> +	ID_AD5621,
> +	ID_AD5620_2500,
> +	ID_AD5620_1250,
> +	ID_AD5640_2500,
> +	ID_AD5640_1250,
> +	ID_AD5660_2500,
> +	ID_AD5660_1250,
> +	ID_AD5662,
> +};
> +
> +static const struct ad5446_chip_info ad5446_spi_chip_info[] = {
> +	[ID_AD5444] = {
> +		.channel = AD5446_CHANNEL(12, 16, 2),
> +		.write = ad5446_write,
> +	},
> +	[ID_AD5446] = {
> +		.channel = AD5446_CHANNEL(14, 16, 0),
> +		.write = ad5446_write,
> +	},
> +	[ID_AD5450] = {
> +		.channel = AD5446_CHANNEL(8, 16, 6),
> +		.write = ad5446_write,
> +	},
> +	[ID_AD5451] = {
> +		.channel = AD5446_CHANNEL(10, 16, 4),
> +		.write = ad5446_write,
> +	},
> +	[ID_AD5541A] = {
> +		.channel = AD5446_CHANNEL(16, 16, 0),
> +		.write = ad5446_write,
> +	},
> +	[ID_AD5512A] = {
> +		.channel = AD5446_CHANNEL(12, 16, 4),
> +		.write = ad5446_write,
> +	},
> +	[ID_AD5553] = {
> +		.channel = AD5446_CHANNEL(14, 16, 0),
> +		.write = ad5446_write,
> +	},
> +	[ID_AD5601] = {
> +		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
> +		.write = ad5446_write,
> +	},
> +	[ID_AD5611] = {
> +		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
> +		.write = ad5446_write,
> +	},
> +	[ID_AD5621] = {
> +		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
> +		.write = ad5446_write,
> +	},
> +	[ID_AD5620_2500] = {
> +		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
> +		.int_vref_mv = 2500,
> +		.write = ad5446_write,
> +	},
> +	[ID_AD5620_1250] = {
> +		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
> +		.int_vref_mv = 1250,
> +		.write = ad5446_write,
> +	},
> +	[ID_AD5640_2500] = {
> +		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
> +		.int_vref_mv = 2500,
> +		.write = ad5446_write,
> +	},
> +	[ID_AD5640_1250] = {
> +		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
> +		.int_vref_mv = 1250,
> +		.write = ad5446_write,
> +	},
> +	[ID_AD5660_2500] = {
> +		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
> +		.int_vref_mv = 2500,
> +		.write = ad5660_write,
> +	},
> +	[ID_AD5660_1250] = {
> +		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
> +		.int_vref_mv = 1250,
> +		.write = ad5660_write,
> +	},
> +	[ID_AD5662] = {
> +		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
> +		.write = ad5660_write,
> +	},
> +};
> +
> +static const struct spi_device_id ad5446_spi_ids[] = {
>  	{"ad5444", ID_AD5444},
>  	{"ad5446", ID_AD5446},
>  	{"ad5450", ID_AD5450},
> @@ -375,18 +408,157 @@ static const struct spi_device_id ad5446_id[] = {
>  	{"ad5662", ID_AD5662},
>  	{}
>  };
> -MODULE_DEVICE_TABLE(spi, ad5446_id);
> +MODULE_DEVICE_TABLE(spi, ad5446_spi_ids);
> +
> +static int __devinit ad5446_spi_probe(struct spi_device *spi)
> +{
> +	const struct spi_device_id *id = spi_get_device_id(spi);
> +
> +	return ad5446_probe(&spi->dev, id->name,
> +		&ad5446_spi_chip_info[id->driver_data]);
> +}
>  
> -static struct spi_driver ad5446_driver = {
> +static int __devexit ad5446_spi_remove(struct spi_device *spi)
> +{
> +	return ad5446_remove(&spi->dev);
> +}
> +
> +static struct spi_driver ad5446_spi_driver = {
>  	.driver = {
>  		.name	= "ad5446",
>  		.owner	= THIS_MODULE,
>  	},
> -	.probe		= ad5446_probe,
> -	.remove		= __devexit_p(ad5446_remove),
> -	.id_table	= ad5446_id,
> +	.probe		= ad5446_spi_probe,
> +	.remove		= __devexit_p(ad5446_spi_remove),
> +	.id_table	= ad5446_spi_ids,
> +};
> +
> +static int __init ad5446_spi_register_driver(void)
> +{
> +	return spi_register_driver(&ad5446_spi_driver);
> +}
> +
> +static void ad5446_spi_unregister_driver(void)
> +{
> +	spi_unregister_driver(&ad5446_spi_driver);
> +}
> +
> +#else
> +
> +static inline int ad5446_spi_register_driver(void) { return 0; }
> +static inline void ad5446_spi_unregister_driver(void) { }
> +
> +#endif
> +
> +#if IS_ENABLED(CONFIG_I2C)
> +
> +static int ad5622_write(struct ad5446_state *st, unsigned val)
> +{
> +	struct i2c_client *client = to_i2c_client(st->dev);
> +	__be16 data = cpu_to_be16(val);
> +
> +	return i2c_master_send(client, (char *)&data, sizeof(data));
> +}
> +
> +/**
> + * ad5446_supported_i2c_device_ids:
> + * The AD5620/40/60 parts are available in different fixed internal reference
> + * voltage options. The actual part numbers may look differently
> + * (and a bit cryptic), however this style is used to make clear which
> + * parts are supported here.
> + */
> +enum ad5446_supported_i2c_device_ids {
> +	ID_AD5602,
> +	ID_AD5612,
> +	ID_AD5622,
> +};
> +
> +static const struct ad5446_chip_info ad5446_i2c_chip_info[] = {
> +	[ID_AD5602] = {
> +		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 4),
> +		.write = ad5622_write,
> +	},
> +	[ID_AD5612] = {
> +		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 2),
> +		.write = ad5622_write,
> +	},
> +	[ID_AD5622] = {
> +		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 0),
> +		.write = ad5622_write,
> +	},
>  };
> -module_spi_driver(ad5446_driver);
> +
> +static int __devinit ad5446_i2c_probe(struct i2c_client *i2c,
> +	const struct i2c_device_id *id)
> +{
> +	return ad5446_probe(&i2c->dev, id->name,
> +		&ad5446_i2c_chip_info[id->driver_data]);
> +}
> +
> +static int __devexit ad5446_i2c_remove(struct i2c_client *i2c)
> +{
> +	return ad5446_remove(&i2c->dev);
> +}
> +
> +static const struct i2c_device_id ad5446_i2c_ids[] = {
> +	{"ad5602", ID_AD5602},
> +	{"ad5612", ID_AD5612},
> +	{"ad5622", ID_AD5622},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, ad5446_i2c_ids);
> +
> +static struct i2c_driver ad5446_i2c_driver = {
> +	.driver = {
> +		   .name = "ad5446",
> +		   .owner = THIS_MODULE,
> +	},
> +	.probe = ad5446_i2c_probe,
> +	.remove = __devexit_p(ad5446_i2c_remove),
> +	.id_table = ad5446_i2c_ids,
> +};
> +
> +static int __init ad5446_i2c_register_driver(void)
> +{
> +	return i2c_add_driver(&ad5446_i2c_driver);
> +}
> +
> +static void __exit ad5446_i2c_unregister_driver(void)
> +{
> +	i2c_del_driver(&ad5446_i2c_driver);
> +}
> +
> +#else
> +
> +static inline int ad5446_i2c_register_driver(void) { return 0; }
> +static inline void ad5446_i2c_unregister_driver(void) { }
> +
> +#endif
> +
> +static int __init ad5446_init(void)
> +{
> +	int ret;
> +
> +	ret = ad5446_spi_register_driver();
> +	if (ret)
> +		return ret;
> +
> +	ret = ad5446_i2c_register_driver();
> +	if (ret) {
> +		ad5446_spi_unregister_driver();
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +module_init(ad5446_init);
> +
> +static void __exit ad5446_exit(void)
> +{
> +	ad5446_i2c_unregister_driver();
> +	ad5446_spi_unregister_driver();
> +}
> +module_exit(ad5446_exit);
>  
>  MODULE_AUTHOR("Michael Hennerich <hennerich@blackfin.uclinux.org>");
>  MODULE_DESCRIPTION("Analog Devices AD5444/AD5446 DAC");
> diff --git a/drivers/iio/dac/ad5446.h b/drivers/iio/dac/ad5446.h
> index 2934269..6b7a176 100644
> --- a/drivers/iio/dac/ad5446.h
> +++ b/drivers/iio/dac/ad5446.h
> @@ -38,7 +38,7 @@
>   */
>  
>  struct ad5446_state {
> -	struct spi_device		*spi;
> +	struct device		*dev;
>  	const struct ad5446_chip_info	*chip_info;
>  	struct regulator		*reg;
>  	unsigned short			vref_mv;
> @@ -60,32 +60,5 @@ struct ad5446_chip_info {
>  	int			(*write)(struct ad5446_state *st, unsigned val);
>  };
>  
> -/**
> - * ad5446_supported_device_ids:
> - * The AD5620/40/60 parts are available in different fixed internal reference
> - * voltage options. The actual part numbers may look differently
> - * (and a bit cryptic), however this style is used to make clear which
> - * parts are supported here.
> - */
> -
> -enum ad5446_supported_device_ids {
> -	ID_AD5444,
> -	ID_AD5446,
> -	ID_AD5450,
> -	ID_AD5451,
> -	ID_AD5541A,
> -	ID_AD5512A,
> -	ID_AD5553,
> -	ID_AD5601,
> -	ID_AD5611,
> -	ID_AD5621,
> -	ID_AD5620_2500,
> -	ID_AD5620_1250,
> -	ID_AD5640_2500,
> -	ID_AD5640_1250,
> -	ID_AD5660_2500,
> -	ID_AD5660_1250,
> -	ID_AD5662,
> -};
>  
>  #endif /* IIO_DAC_AD5446_H_ */
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3.6-rc2 V2 1/2] iio:ad5446: Add support for I2C based DACs
  2012-08-22 18:24     ` Jonathan Cameron
@ 2012-08-24 11:38       ` Lars-Peter Clausen
  2012-09-03  8:01         ` Lars-Peter Clausen
  0 siblings, 1 reply; 10+ messages in thread
From: Lars-Peter Clausen @ 2012-08-24 11:38 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Jean-Francois Dagenais, linux-iio, device-drivers-devel

On 08/22/2012 08:24 PM, Jonathan Cameron wrote:
> On 08/21/2012 03:28 PM, Jean-Francois Dagenais wrote:
>> This patch adds support for I2C based single channel DACs to the ad5446
>> driver. Specifically AD5602, AD5612 and AD5622.
>>
>> V1: from Lars-Peter Clausen <lars@metafoo.de>
>> V2: Split the device IDs into two enums and move them to the c file.
> Sensible change.  Patch is fine but given we are still well away from
> the merge window I'll wait for Lars-Peter's response before merging this.
> 

Looks good to me on first sight. I'd have done the two patches in reverse
order, but it doesn't really matter.

Will take a closer look when I'm back from vacation, but I'm already quite
confident that it can go in as is.

>>
>> Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
>> ---
>>  drivers/iio/dac/Kconfig  |   9 +-
>>  drivers/iio/dac/ad5446.c | 402 +++++++++++++++++++++++++++++++++--------------
>>  drivers/iio/dac/ad5446.h |  29 +---
>>  3 files changed, 293 insertions(+), 147 deletions(-)
>>
>> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
>> index 1be15fa..293b61d 100644
>> --- a/drivers/iio/dac/Kconfig
>> +++ b/drivers/iio/dac/Kconfig
>> @@ -57,11 +57,12 @@ config AD5624R_SPI
>>  
>>  config AD5446
>>  	tristate "Analog Devices AD5446 and similar single channel DACs driver"
>> -	depends on SPI
>> +	depends on (SPI_MASTER || I2C)
>>  	help
>> -	  Say yes here to build support for Analog Devices AD5444, AD5446, AD5450,
>> -	  AD5451, AD5452, AD5453, AD5512A, AD5541A, AD5542A, AD5543, AD5553, AD5601,
>> -	  AD5611, AD5620, AD5621, AD5640, AD5660, AD5662 DACs.
>> +	  Say yes here to build support for Analog Devices AD5602, AD5612, AD5622,
>> +	  AD5444, AD5446, AD5450, AD5451, AD5452, AD5453, AD5512A, AD5541A, AD5542A,
>> +	  AD5543, AD5553, AD5601, AD5611, AD5620, AD5621, AD5640, AD5660, AD5662
>> +	  DACs.
>>  
>>  	  To compile this driver as a module, choose M here: the
>>  	  module will be called ad5446.
>> diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c
>> index 2ca5059..241665b 100644
>> --- a/drivers/iio/dac/ad5446.c
>> +++ b/drivers/iio/dac/ad5446.c
>> @@ -14,6 +14,7 @@
>>  #include <linux/sysfs.h>
>>  #include <linux/list.h>
>>  #include <linux/spi/spi.h>
>> +#include <linux/i2c.h>
>>  #include <linux/regulator/consumer.h>
>>  #include <linux/err.h>
>>  #include <linux/module.h>
>> @@ -23,23 +24,6 @@
>>  
>>  #include "ad5446.h"
>>  
>> -static int ad5446_write(struct ad5446_state *st, unsigned val)
>> -{
>> -	__be16 data = cpu_to_be16(val);
>> -	return spi_write(st->spi, &data, sizeof(data));
>> -}
>> -
>> -static int ad5660_write(struct ad5446_state *st, unsigned val)
>> -{
>> -	uint8_t data[3];
>> -
>> -	data[0] = (val >> 16) & 0xFF;
>> -	data[1] = (val >> 8) & 0xFF;
>> -	data[2] = val & 0xFF;
>> -
>> -	return spi_write(st->spi, data, sizeof(data));
>> -}
>> -
>>  static const char * const ad5446_powerdown_modes[] = {
>>  	"1kohm_to_gnd", "100kohm_to_gnd", "three_state"
>>  };
>> @@ -110,7 +94,7 @@ static ssize_t ad5446_write_dac_powerdown(struct iio_dev *indio_dev,
>>  	return ret ? ret : len;
>>  }
>>  
>> -static const struct iio_chan_spec_ext_info ad5064_ext_info_powerdown[] = {
>> +static const struct iio_chan_spec_ext_info ad5446_ext_info_powerdown[] = {
>>  	{
>>  		.name = "powerdown",
>>  		.read = ad5446_read_dac_powerdown,
>> @@ -136,84 +120,7 @@ static const struct iio_chan_spec_ext_info ad5064_ext_info_powerdown[] = {
>>  	_AD5446_CHANNEL(bits, storage, shift, NULL)
>>  
>>  #define AD5446_CHANNEL_POWERDOWN(bits, storage, shift) \
>> -	_AD5446_CHANNEL(bits, storage, shift, ad5064_ext_info_powerdown)
>> -
>> -static const struct ad5446_chip_info ad5446_chip_info_tbl[] = {
>> -	[ID_AD5444] = {
>> -		.channel = AD5446_CHANNEL(12, 16, 2),
>> -		.write = ad5446_write,
>> -	},
>> -	[ID_AD5446] = {
>> -		.channel = AD5446_CHANNEL(14, 16, 0),
>> -		.write = ad5446_write,
>> -	},
>> -	[ID_AD5450] = {
>> -		.channel = AD5446_CHANNEL(8, 16, 6),
>> -		.write = ad5446_write,
>> -	},
>> -	[ID_AD5451] = {
>> -		.channel = AD5446_CHANNEL(10, 16, 4),
>> -		.write = ad5446_write,
>> -	},
>> -	[ID_AD5541A] = {
>> -		.channel = AD5446_CHANNEL(16, 16, 0),
>> -		.write = ad5446_write,
>> -	},
>> -	[ID_AD5512A] = {
>> -		.channel = AD5446_CHANNEL(12, 16, 4),
>> -		.write = ad5446_write,
>> -	},
>> -	[ID_AD5553] = {
>> -		.channel = AD5446_CHANNEL(14, 16, 0),
>> -		.write = ad5446_write,
>> -	},
>> -	[ID_AD5601] = {
>> -		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
>> -		.write = ad5446_write,
>> -	},
>> -	[ID_AD5611] = {
>> -		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
>> -		.write = ad5446_write,
>> -	},
>> -	[ID_AD5621] = {
>> -		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>> -		.write = ad5446_write,
>> -	},
>> -	[ID_AD5620_2500] = {
>> -		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>> -		.int_vref_mv = 2500,
>> -		.write = ad5446_write,
>> -	},
>> -	[ID_AD5620_1250] = {
>> -		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>> -		.int_vref_mv = 1250,
>> -		.write = ad5446_write,
>> -	},
>> -	[ID_AD5640_2500] = {
>> -		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
>> -		.int_vref_mv = 2500,
>> -		.write = ad5446_write,
>> -	},
>> -	[ID_AD5640_1250] = {
>> -		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
>> -		.int_vref_mv = 1250,
>> -		.write = ad5446_write,
>> -	},
>> -	[ID_AD5660_2500] = {
>> -		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
>> -		.int_vref_mv = 2500,
>> -		.write = ad5660_write,
>> -	},
>> -	[ID_AD5660_1250] = {
>> -		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
>> -		.int_vref_mv = 1250,
>> -		.write = ad5660_write,
>> -	},
>> -	[ID_AD5662] = {
>> -		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
>> -		.write = ad5660_write,
>> -	},
>> -};
>> +	_AD5446_CHANNEL(bits, storage, shift, ad5446_ext_info_powerdown)
>>  
>>  static int ad5446_read_raw(struct iio_dev *indio_dev,
>>  			   struct iio_chan_spec const *chan,
>> @@ -272,14 +179,15 @@ static const struct iio_info ad5446_info = {
>>  	.driver_module = THIS_MODULE,
>>  };
>>  
>> -static int __devinit ad5446_probe(struct spi_device *spi)
>> +static int __devinit ad5446_probe(struct device *dev, const char *name,
>> +	const struct ad5446_chip_info *chip_info)
>>  {
>>  	struct ad5446_state *st;
>>  	struct iio_dev *indio_dev;
>>  	struct regulator *reg;
>>  	int ret, voltage_uv = 0;
>>  
>> -	reg = regulator_get(&spi->dev, "vcc");
>> +	reg = regulator_get(dev, "vcc");
>>  	if (!IS_ERR(reg)) {
>>  		ret = regulator_enable(reg);
>>  		if (ret)
>> @@ -294,16 +202,15 @@ static int __devinit ad5446_probe(struct spi_device *spi)
>>  		goto error_disable_reg;
>>  	}
>>  	st = iio_priv(indio_dev);
>> -	st->chip_info =
>> -		&ad5446_chip_info_tbl[spi_get_device_id(spi)->driver_data];
>> +	st->chip_info = chip_info;
>>  
>> -	spi_set_drvdata(spi, indio_dev);
>> +	dev_set_drvdata(dev, indio_dev);
>>  	st->reg = reg;
>> -	st->spi = spi;
>> +	st->dev = dev;
>>  
>> -	/* Establish that the iio_dev is a child of the spi device */
>> -	indio_dev->dev.parent = &spi->dev;
>> -	indio_dev->name = spi_get_device_id(spi)->name;
>> +	/* Establish that the iio_dev is a child of the device */
>> +	indio_dev->dev.parent = dev;
>> +	indio_dev->name = name;
>>  	indio_dev->info = &ad5446_info;
>>  	indio_dev->modes = INDIO_DIRECT_MODE;
>>  	indio_dev->channels = &st->chip_info->channel;
>> @@ -316,7 +223,7 @@ static int __devinit ad5446_probe(struct spi_device *spi)
>>  	else if (voltage_uv)
>>  		st->vref_mv = voltage_uv / 1000;
>>  	else
>> -		dev_warn(&spi->dev, "reference voltage unspecified\n");
>> +		dev_warn(dev, "reference voltage unspecified\n");
>>  
>>  	ret = iio_device_register(indio_dev);
>>  	if (ret)
>> @@ -336,9 +243,9 @@ error_put_reg:
>>  	return ret;
>>  }
>>  
>> -static int ad5446_remove(struct spi_device *spi)
>> +static int ad5446_remove(struct device *dev)
>>  {
>> -	struct iio_dev *indio_dev = spi_get_drvdata(spi);
>> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>>  	struct ad5446_state *st = iio_priv(indio_dev);
>>  
>>  	iio_device_unregister(indio_dev);
>> @@ -351,7 +258,133 @@ static int ad5446_remove(struct spi_device *spi)
>>  	return 0;
>>  }
>>  
>> -static const struct spi_device_id ad5446_id[] = {
>> +#if IS_ENABLED(CONFIG_SPI_MASTER)
>> +
>> +static int ad5446_write(struct ad5446_state *st, unsigned val)
>> +{
>> +	struct spi_device *spi = to_spi_device(st->dev);
>> +	__be16 data = cpu_to_be16(val);
>> +
>> +	return spi_write(spi, &data, sizeof(data));
>> +}
>> +
>> +static int ad5660_write(struct ad5446_state *st, unsigned val)
>> +{
>> +	struct spi_device *spi = to_spi_device(st->dev);
>> +	uint8_t data[3];
>> +
>> +	data[0] = (val >> 16) & 0xFF;
>> +	data[1] = (val >> 8) & 0xFF;
>> +	data[2] = val & 0xFF;
>> +
>> +	return spi_write(spi, data, sizeof(data));
>> +}
>> +
>> +/**
>> + * ad5446_supported_spi_device_ids:
>> + * The AD5620/40/60 parts are available in different fixed internal reference
>> + * voltage options. The actual part numbers may look differently
>> + * (and a bit cryptic), however this style is used to make clear which
>> + * parts are supported here.
>> + */
>> +enum ad5446_supported_spi_device_ids {
>> +	ID_AD5444,
>> +	ID_AD5446,
>> +	ID_AD5450,
>> +	ID_AD5451,
>> +	ID_AD5541A,
>> +	ID_AD5512A,
>> +	ID_AD5553,
>> +	ID_AD5601,
>> +	ID_AD5611,
>> +	ID_AD5621,
>> +	ID_AD5620_2500,
>> +	ID_AD5620_1250,
>> +	ID_AD5640_2500,
>> +	ID_AD5640_1250,
>> +	ID_AD5660_2500,
>> +	ID_AD5660_1250,
>> +	ID_AD5662,
>> +};
>> +
>> +static const struct ad5446_chip_info ad5446_spi_chip_info[] = {
>> +	[ID_AD5444] = {
>> +		.channel = AD5446_CHANNEL(12, 16, 2),
>> +		.write = ad5446_write,
>> +	},
>> +	[ID_AD5446] = {
>> +		.channel = AD5446_CHANNEL(14, 16, 0),
>> +		.write = ad5446_write,
>> +	},
>> +	[ID_AD5450] = {
>> +		.channel = AD5446_CHANNEL(8, 16, 6),
>> +		.write = ad5446_write,
>> +	},
>> +	[ID_AD5451] = {
>> +		.channel = AD5446_CHANNEL(10, 16, 4),
>> +		.write = ad5446_write,
>> +	},
>> +	[ID_AD5541A] = {
>> +		.channel = AD5446_CHANNEL(16, 16, 0),
>> +		.write = ad5446_write,
>> +	},
>> +	[ID_AD5512A] = {
>> +		.channel = AD5446_CHANNEL(12, 16, 4),
>> +		.write = ad5446_write,
>> +	},
>> +	[ID_AD5553] = {
>> +		.channel = AD5446_CHANNEL(14, 16, 0),
>> +		.write = ad5446_write,
>> +	},
>> +	[ID_AD5601] = {
>> +		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
>> +		.write = ad5446_write,
>> +	},
>> +	[ID_AD5611] = {
>> +		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
>> +		.write = ad5446_write,
>> +	},
>> +	[ID_AD5621] = {
>> +		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>> +		.write = ad5446_write,
>> +	},
>> +	[ID_AD5620_2500] = {
>> +		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>> +		.int_vref_mv = 2500,
>> +		.write = ad5446_write,
>> +	},
>> +	[ID_AD5620_1250] = {
>> +		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>> +		.int_vref_mv = 1250,
>> +		.write = ad5446_write,
>> +	},
>> +	[ID_AD5640_2500] = {
>> +		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
>> +		.int_vref_mv = 2500,
>> +		.write = ad5446_write,
>> +	},
>> +	[ID_AD5640_1250] = {
>> +		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
>> +		.int_vref_mv = 1250,
>> +		.write = ad5446_write,
>> +	},
>> +	[ID_AD5660_2500] = {
>> +		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
>> +		.int_vref_mv = 2500,
>> +		.write = ad5660_write,
>> +	},
>> +	[ID_AD5660_1250] = {
>> +		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
>> +		.int_vref_mv = 1250,
>> +		.write = ad5660_write,
>> +	},
>> +	[ID_AD5662] = {
>> +		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
>> +		.write = ad5660_write,
>> +	},
>> +};
>> +
>> +static const struct spi_device_id ad5446_spi_ids[] = {
>>  	{"ad5444", ID_AD5444},
>>  	{"ad5446", ID_AD5446},
>>  	{"ad5450", ID_AD5450},
>> @@ -375,18 +408,157 @@ static const struct spi_device_id ad5446_id[] = {
>>  	{"ad5662", ID_AD5662},
>>  	{}
>>  };
>> -MODULE_DEVICE_TABLE(spi, ad5446_id);
>> +MODULE_DEVICE_TABLE(spi, ad5446_spi_ids);
>> +
>> +static int __devinit ad5446_spi_probe(struct spi_device *spi)
>> +{
>> +	const struct spi_device_id *id = spi_get_device_id(spi);
>> +
>> +	return ad5446_probe(&spi->dev, id->name,
>> +		&ad5446_spi_chip_info[id->driver_data]);
>> +}
>>  
>> -static struct spi_driver ad5446_driver = {
>> +static int __devexit ad5446_spi_remove(struct spi_device *spi)
>> +{
>> +	return ad5446_remove(&spi->dev);
>> +}
>> +
>> +static struct spi_driver ad5446_spi_driver = {
>>  	.driver = {
>>  		.name	= "ad5446",
>>  		.owner	= THIS_MODULE,
>>  	},
>> -	.probe		= ad5446_probe,
>> -	.remove		= __devexit_p(ad5446_remove),
>> -	.id_table	= ad5446_id,
>> +	.probe		= ad5446_spi_probe,
>> +	.remove		= __devexit_p(ad5446_spi_remove),
>> +	.id_table	= ad5446_spi_ids,
>> +};
>> +
>> +static int __init ad5446_spi_register_driver(void)
>> +{
>> +	return spi_register_driver(&ad5446_spi_driver);
>> +}
>> +
>> +static void ad5446_spi_unregister_driver(void)
>> +{
>> +	spi_unregister_driver(&ad5446_spi_driver);
>> +}
>> +
>> +#else
>> +
>> +static inline int ad5446_spi_register_driver(void) { return 0; }
>> +static inline void ad5446_spi_unregister_driver(void) { }
>> +
>> +#endif
>> +
>> +#if IS_ENABLED(CONFIG_I2C)
>> +
>> +static int ad5622_write(struct ad5446_state *st, unsigned val)
>> +{
>> +	struct i2c_client *client = to_i2c_client(st->dev);
>> +	__be16 data = cpu_to_be16(val);
>> +
>> +	return i2c_master_send(client, (char *)&data, sizeof(data));
>> +}
>> +
>> +/**
>> + * ad5446_supported_i2c_device_ids:
>> + * The AD5620/40/60 parts are available in different fixed internal reference
>> + * voltage options. The actual part numbers may look differently
>> + * (and a bit cryptic), however this style is used to make clear which
>> + * parts are supported here.
>> + */
>> +enum ad5446_supported_i2c_device_ids {
>> +	ID_AD5602,
>> +	ID_AD5612,
>> +	ID_AD5622,
>> +};
>> +
>> +static const struct ad5446_chip_info ad5446_i2c_chip_info[] = {
>> +	[ID_AD5602] = {
>> +		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 4),
>> +		.write = ad5622_write,
>> +	},
>> +	[ID_AD5612] = {
>> +		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 2),
>> +		.write = ad5622_write,
>> +	},
>> +	[ID_AD5622] = {
>> +		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 0),
>> +		.write = ad5622_write,
>> +	},
>>  };
>> -module_spi_driver(ad5446_driver);
>> +
>> +static int __devinit ad5446_i2c_probe(struct i2c_client *i2c,
>> +	const struct i2c_device_id *id)
>> +{
>> +	return ad5446_probe(&i2c->dev, id->name,
>> +		&ad5446_i2c_chip_info[id->driver_data]);
>> +}
>> +
>> +static int __devexit ad5446_i2c_remove(struct i2c_client *i2c)
>> +{
>> +	return ad5446_remove(&i2c->dev);
>> +}
>> +
>> +static const struct i2c_device_id ad5446_i2c_ids[] = {
>> +	{"ad5602", ID_AD5602},
>> +	{"ad5612", ID_AD5612},
>> +	{"ad5622", ID_AD5622},
>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(i2c, ad5446_i2c_ids);
>> +
>> +static struct i2c_driver ad5446_i2c_driver = {
>> +	.driver = {
>> +		   .name = "ad5446",
>> +		   .owner = THIS_MODULE,
>> +	},
>> +	.probe = ad5446_i2c_probe,
>> +	.remove = __devexit_p(ad5446_i2c_remove),
>> +	.id_table = ad5446_i2c_ids,
>> +};
>> +
>> +static int __init ad5446_i2c_register_driver(void)
>> +{
>> +	return i2c_add_driver(&ad5446_i2c_driver);
>> +}
>> +
>> +static void __exit ad5446_i2c_unregister_driver(void)
>> +{
>> +	i2c_del_driver(&ad5446_i2c_driver);
>> +}
>> +
>> +#else
>> +
>> +static inline int ad5446_i2c_register_driver(void) { return 0; }
>> +static inline void ad5446_i2c_unregister_driver(void) { }
>> +
>> +#endif
>> +
>> +static int __init ad5446_init(void)
>> +{
>> +	int ret;
>> +
>> +	ret = ad5446_spi_register_driver();
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = ad5446_i2c_register_driver();
>> +	if (ret) {
>> +		ad5446_spi_unregister_driver();
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +module_init(ad5446_init);
>> +
>> +static void __exit ad5446_exit(void)
>> +{
>> +	ad5446_i2c_unregister_driver();
>> +	ad5446_spi_unregister_driver();
>> +}
>> +module_exit(ad5446_exit);
>>  
>>  MODULE_AUTHOR("Michael Hennerich <hennerich@blackfin.uclinux.org>");
>>  MODULE_DESCRIPTION("Analog Devices AD5444/AD5446 DAC");
>> diff --git a/drivers/iio/dac/ad5446.h b/drivers/iio/dac/ad5446.h
>> index 2934269..6b7a176 100644
>> --- a/drivers/iio/dac/ad5446.h
>> +++ b/drivers/iio/dac/ad5446.h
>> @@ -38,7 +38,7 @@
>>   */
>>  
>>  struct ad5446_state {
>> -	struct spi_device		*spi;
>> +	struct device		*dev;
>>  	const struct ad5446_chip_info	*chip_info;
>>  	struct regulator		*reg;
>>  	unsigned short			vref_mv;
>> @@ -60,32 +60,5 @@ struct ad5446_chip_info {
>>  	int			(*write)(struct ad5446_state *st, unsigned val);
>>  };
>>  
>> -/**
>> - * ad5446_supported_device_ids:
>> - * The AD5620/40/60 parts are available in different fixed internal reference
>> - * voltage options. The actual part numbers may look differently
>> - * (and a bit cryptic), however this style is used to make clear which
>> - * parts are supported here.
>> - */
>> -
>> -enum ad5446_supported_device_ids {
>> -	ID_AD5444,
>> -	ID_AD5446,
>> -	ID_AD5450,
>> -	ID_AD5451,
>> -	ID_AD5541A,
>> -	ID_AD5512A,
>> -	ID_AD5553,
>> -	ID_AD5601,
>> -	ID_AD5611,
>> -	ID_AD5621,
>> -	ID_AD5620_2500,
>> -	ID_AD5620_1250,
>> -	ID_AD5640_2500,
>> -	ID_AD5640_1250,
>> -	ID_AD5660_2500,
>> -	ID_AD5660_1250,
>> -	ID_AD5662,
>> -};
>>  
>>  #endif /* IIO_DAC_AD5446_H_ */
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3.6-rc2 V2 1/2] iio:ad5446: Add support for I2C based DACs
  2012-08-24 11:38       ` Lars-Peter Clausen
@ 2012-09-03  8:01         ` Lars-Peter Clausen
  2012-09-03 20:11           ` Jonathan Cameron
  0 siblings, 1 reply; 10+ messages in thread
From: Lars-Peter Clausen @ 2012-09-03  8:01 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Jean-Francois Dagenais, linux-iio, device-drivers-devel

On 08/24/2012 01:38 PM, Lars-Peter Clausen wrote:
> On 08/22/2012 08:24 PM, Jonathan Cameron wrote:
>> On 08/21/2012 03:28 PM, Jean-Francois Dagenais wrote:
>>> This patch adds support for I2C based single channel DACs to the ad5446
>>> driver. Specifically AD5602, AD5612 and AD5622.
>>>
>>> V1: from Lars-Peter Clausen <lars@metafoo.de>
>>> V2: Split the device IDs into two enums and move them to the c file.
>> Sensible change.  Patch is fine but given we are still well away from
>> the merge window I'll wait for Lars-Peter's response before merging this.
>>
> 
> Looks good to me on first sight. I'd have done the two patches in reverse
> order, but it doesn't really matter.
> 
> Will take a closer look when I'm back from vacation, but I'm already quite
> confident that it can go in as is.
> 


Both patches:
Acked-by: Lars-Peter Clausen <lars@metafoo.de>

Thanks.

>>>
>>> Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
>>> ---
>>>  drivers/iio/dac/Kconfig  |   9 +-
>>>  drivers/iio/dac/ad5446.c | 402 +++++++++++++++++++++++++++++++++--------------
>>>  drivers/iio/dac/ad5446.h |  29 +---
>>>  3 files changed, 293 insertions(+), 147 deletions(-)
>>>
>>> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
>>> index 1be15fa..293b61d 100644
>>> --- a/drivers/iio/dac/Kconfig
>>> +++ b/drivers/iio/dac/Kconfig
>>> @@ -57,11 +57,12 @@ config AD5624R_SPI
>>>  
>>>  config AD5446
>>>  	tristate "Analog Devices AD5446 and similar single channel DACs driver"
>>> -	depends on SPI
>>> +	depends on (SPI_MASTER || I2C)
>>>  	help
>>> -	  Say yes here to build support for Analog Devices AD5444, AD5446, AD5450,
>>> -	  AD5451, AD5452, AD5453, AD5512A, AD5541A, AD5542A, AD5543, AD5553, AD5601,
>>> -	  AD5611, AD5620, AD5621, AD5640, AD5660, AD5662 DACs.
>>> +	  Say yes here to build support for Analog Devices AD5602, AD5612, AD5622,
>>> +	  AD5444, AD5446, AD5450, AD5451, AD5452, AD5453, AD5512A, AD5541A, AD5542A,
>>> +	  AD5543, AD5553, AD5601, AD5611, AD5620, AD5621, AD5640, AD5660, AD5662
>>> +	  DACs.
>>>  
>>>  	  To compile this driver as a module, choose M here: the
>>>  	  module will be called ad5446.
>>> diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c
>>> index 2ca5059..241665b 100644
>>> --- a/drivers/iio/dac/ad5446.c
>>> +++ b/drivers/iio/dac/ad5446.c
>>> @@ -14,6 +14,7 @@
>>>  #include <linux/sysfs.h>
>>>  #include <linux/list.h>
>>>  #include <linux/spi/spi.h>
>>> +#include <linux/i2c.h>
>>>  #include <linux/regulator/consumer.h>
>>>  #include <linux/err.h>
>>>  #include <linux/module.h>
>>> @@ -23,23 +24,6 @@
>>>  
>>>  #include "ad5446.h"
>>>  
>>> -static int ad5446_write(struct ad5446_state *st, unsigned val)
>>> -{
>>> -	__be16 data = cpu_to_be16(val);
>>> -	return spi_write(st->spi, &data, sizeof(data));
>>> -}
>>> -
>>> -static int ad5660_write(struct ad5446_state *st, unsigned val)
>>> -{
>>> -	uint8_t data[3];
>>> -
>>> -	data[0] = (val >> 16) & 0xFF;
>>> -	data[1] = (val >> 8) & 0xFF;
>>> -	data[2] = val & 0xFF;
>>> -
>>> -	return spi_write(st->spi, data, sizeof(data));
>>> -}
>>> -
>>>  static const char * const ad5446_powerdown_modes[] = {
>>>  	"1kohm_to_gnd", "100kohm_to_gnd", "three_state"
>>>  };
>>> @@ -110,7 +94,7 @@ static ssize_t ad5446_write_dac_powerdown(struct iio_dev *indio_dev,
>>>  	return ret ? ret : len;
>>>  }
>>>  
>>> -static const struct iio_chan_spec_ext_info ad5064_ext_info_powerdown[] = {
>>> +static const struct iio_chan_spec_ext_info ad5446_ext_info_powerdown[] = {
>>>  	{
>>>  		.name = "powerdown",
>>>  		.read = ad5446_read_dac_powerdown,
>>> @@ -136,84 +120,7 @@ static const struct iio_chan_spec_ext_info ad5064_ext_info_powerdown[] = {
>>>  	_AD5446_CHANNEL(bits, storage, shift, NULL)
>>>  
>>>  #define AD5446_CHANNEL_POWERDOWN(bits, storage, shift) \
>>> -	_AD5446_CHANNEL(bits, storage, shift, ad5064_ext_info_powerdown)
>>> -
>>> -static const struct ad5446_chip_info ad5446_chip_info_tbl[] = {
>>> -	[ID_AD5444] = {
>>> -		.channel = AD5446_CHANNEL(12, 16, 2),
>>> -		.write = ad5446_write,
>>> -	},
>>> -	[ID_AD5446] = {
>>> -		.channel = AD5446_CHANNEL(14, 16, 0),
>>> -		.write = ad5446_write,
>>> -	},
>>> -	[ID_AD5450] = {
>>> -		.channel = AD5446_CHANNEL(8, 16, 6),
>>> -		.write = ad5446_write,
>>> -	},
>>> -	[ID_AD5451] = {
>>> -		.channel = AD5446_CHANNEL(10, 16, 4),
>>> -		.write = ad5446_write,
>>> -	},
>>> -	[ID_AD5541A] = {
>>> -		.channel = AD5446_CHANNEL(16, 16, 0),
>>> -		.write = ad5446_write,
>>> -	},
>>> -	[ID_AD5512A] = {
>>> -		.channel = AD5446_CHANNEL(12, 16, 4),
>>> -		.write = ad5446_write,
>>> -	},
>>> -	[ID_AD5553] = {
>>> -		.channel = AD5446_CHANNEL(14, 16, 0),
>>> -		.write = ad5446_write,
>>> -	},
>>> -	[ID_AD5601] = {
>>> -		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
>>> -		.write = ad5446_write,
>>> -	},
>>> -	[ID_AD5611] = {
>>> -		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
>>> -		.write = ad5446_write,
>>> -	},
>>> -	[ID_AD5621] = {
>>> -		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>>> -		.write = ad5446_write,
>>> -	},
>>> -	[ID_AD5620_2500] = {
>>> -		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>>> -		.int_vref_mv = 2500,
>>> -		.write = ad5446_write,
>>> -	},
>>> -	[ID_AD5620_1250] = {
>>> -		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>>> -		.int_vref_mv = 1250,
>>> -		.write = ad5446_write,
>>> -	},
>>> -	[ID_AD5640_2500] = {
>>> -		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
>>> -		.int_vref_mv = 2500,
>>> -		.write = ad5446_write,
>>> -	},
>>> -	[ID_AD5640_1250] = {
>>> -		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
>>> -		.int_vref_mv = 1250,
>>> -		.write = ad5446_write,
>>> -	},
>>> -	[ID_AD5660_2500] = {
>>> -		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
>>> -		.int_vref_mv = 2500,
>>> -		.write = ad5660_write,
>>> -	},
>>> -	[ID_AD5660_1250] = {
>>> -		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
>>> -		.int_vref_mv = 1250,
>>> -		.write = ad5660_write,
>>> -	},
>>> -	[ID_AD5662] = {
>>> -		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
>>> -		.write = ad5660_write,
>>> -	},
>>> -};
>>> +	_AD5446_CHANNEL(bits, storage, shift, ad5446_ext_info_powerdown)
>>>  
>>>  static int ad5446_read_raw(struct iio_dev *indio_dev,
>>>  			   struct iio_chan_spec const *chan,
>>> @@ -272,14 +179,15 @@ static const struct iio_info ad5446_info = {
>>>  	.driver_module = THIS_MODULE,
>>>  };
>>>  
>>> -static int __devinit ad5446_probe(struct spi_device *spi)
>>> +static int __devinit ad5446_probe(struct device *dev, const char *name,
>>> +	const struct ad5446_chip_info *chip_info)
>>>  {
>>>  	struct ad5446_state *st;
>>>  	struct iio_dev *indio_dev;
>>>  	struct regulator *reg;
>>>  	int ret, voltage_uv = 0;
>>>  
>>> -	reg = regulator_get(&spi->dev, "vcc");
>>> +	reg = regulator_get(dev, "vcc");
>>>  	if (!IS_ERR(reg)) {
>>>  		ret = regulator_enable(reg);
>>>  		if (ret)
>>> @@ -294,16 +202,15 @@ static int __devinit ad5446_probe(struct spi_device *spi)
>>>  		goto error_disable_reg;
>>>  	}
>>>  	st = iio_priv(indio_dev);
>>> -	st->chip_info =
>>> -		&ad5446_chip_info_tbl[spi_get_device_id(spi)->driver_data];
>>> +	st->chip_info = chip_info;
>>>  
>>> -	spi_set_drvdata(spi, indio_dev);
>>> +	dev_set_drvdata(dev, indio_dev);
>>>  	st->reg = reg;
>>> -	st->spi = spi;
>>> +	st->dev = dev;
>>>  
>>> -	/* Establish that the iio_dev is a child of the spi device */
>>> -	indio_dev->dev.parent = &spi->dev;
>>> -	indio_dev->name = spi_get_device_id(spi)->name;
>>> +	/* Establish that the iio_dev is a child of the device */
>>> +	indio_dev->dev.parent = dev;
>>> +	indio_dev->name = name;
>>>  	indio_dev->info = &ad5446_info;
>>>  	indio_dev->modes = INDIO_DIRECT_MODE;
>>>  	indio_dev->channels = &st->chip_info->channel;
>>> @@ -316,7 +223,7 @@ static int __devinit ad5446_probe(struct spi_device *spi)
>>>  	else if (voltage_uv)
>>>  		st->vref_mv = voltage_uv / 1000;
>>>  	else
>>> -		dev_warn(&spi->dev, "reference voltage unspecified\n");
>>> +		dev_warn(dev, "reference voltage unspecified\n");
>>>  
>>>  	ret = iio_device_register(indio_dev);
>>>  	if (ret)
>>> @@ -336,9 +243,9 @@ error_put_reg:
>>>  	return ret;
>>>  }
>>>  
>>> -static int ad5446_remove(struct spi_device *spi)
>>> +static int ad5446_remove(struct device *dev)
>>>  {
>>> -	struct iio_dev *indio_dev = spi_get_drvdata(spi);
>>> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>>>  	struct ad5446_state *st = iio_priv(indio_dev);
>>>  
>>>  	iio_device_unregister(indio_dev);
>>> @@ -351,7 +258,133 @@ static int ad5446_remove(struct spi_device *spi)
>>>  	return 0;
>>>  }
>>>  
>>> -static const struct spi_device_id ad5446_id[] = {
>>> +#if IS_ENABLED(CONFIG_SPI_MASTER)
>>> +
>>> +static int ad5446_write(struct ad5446_state *st, unsigned val)
>>> +{
>>> +	struct spi_device *spi = to_spi_device(st->dev);
>>> +	__be16 data = cpu_to_be16(val);
>>> +
>>> +	return spi_write(spi, &data, sizeof(data));
>>> +}
>>> +
>>> +static int ad5660_write(struct ad5446_state *st, unsigned val)
>>> +{
>>> +	struct spi_device *spi = to_spi_device(st->dev);
>>> +	uint8_t data[3];
>>> +
>>> +	data[0] = (val >> 16) & 0xFF;
>>> +	data[1] = (val >> 8) & 0xFF;
>>> +	data[2] = val & 0xFF;
>>> +
>>> +	return spi_write(spi, data, sizeof(data));
>>> +}
>>> +
>>> +/**
>>> + * ad5446_supported_spi_device_ids:
>>> + * The AD5620/40/60 parts are available in different fixed internal reference
>>> + * voltage options. The actual part numbers may look differently
>>> + * (and a bit cryptic), however this style is used to make clear which
>>> + * parts are supported here.
>>> + */
>>> +enum ad5446_supported_spi_device_ids {
>>> +	ID_AD5444,
>>> +	ID_AD5446,
>>> +	ID_AD5450,
>>> +	ID_AD5451,
>>> +	ID_AD5541A,
>>> +	ID_AD5512A,
>>> +	ID_AD5553,
>>> +	ID_AD5601,
>>> +	ID_AD5611,
>>> +	ID_AD5621,
>>> +	ID_AD5620_2500,
>>> +	ID_AD5620_1250,
>>> +	ID_AD5640_2500,
>>> +	ID_AD5640_1250,
>>> +	ID_AD5660_2500,
>>> +	ID_AD5660_1250,
>>> +	ID_AD5662,
>>> +};
>>> +
>>> +static const struct ad5446_chip_info ad5446_spi_chip_info[] = {
>>> +	[ID_AD5444] = {
>>> +		.channel = AD5446_CHANNEL(12, 16, 2),
>>> +		.write = ad5446_write,
>>> +	},
>>> +	[ID_AD5446] = {
>>> +		.channel = AD5446_CHANNEL(14, 16, 0),
>>> +		.write = ad5446_write,
>>> +	},
>>> +	[ID_AD5450] = {
>>> +		.channel = AD5446_CHANNEL(8, 16, 6),
>>> +		.write = ad5446_write,
>>> +	},
>>> +	[ID_AD5451] = {
>>> +		.channel = AD5446_CHANNEL(10, 16, 4),
>>> +		.write = ad5446_write,
>>> +	},
>>> +	[ID_AD5541A] = {
>>> +		.channel = AD5446_CHANNEL(16, 16, 0),
>>> +		.write = ad5446_write,
>>> +	},
>>> +	[ID_AD5512A] = {
>>> +		.channel = AD5446_CHANNEL(12, 16, 4),
>>> +		.write = ad5446_write,
>>> +	},
>>> +	[ID_AD5553] = {
>>> +		.channel = AD5446_CHANNEL(14, 16, 0),
>>> +		.write = ad5446_write,
>>> +	},
>>> +	[ID_AD5601] = {
>>> +		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
>>> +		.write = ad5446_write,
>>> +	},
>>> +	[ID_AD5611] = {
>>> +		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
>>> +		.write = ad5446_write,
>>> +	},
>>> +	[ID_AD5621] = {
>>> +		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>>> +		.write = ad5446_write,
>>> +	},
>>> +	[ID_AD5620_2500] = {
>>> +		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>>> +		.int_vref_mv = 2500,
>>> +		.write = ad5446_write,
>>> +	},
>>> +	[ID_AD5620_1250] = {
>>> +		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>>> +		.int_vref_mv = 1250,
>>> +		.write = ad5446_write,
>>> +	},
>>> +	[ID_AD5640_2500] = {
>>> +		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
>>> +		.int_vref_mv = 2500,
>>> +		.write = ad5446_write,
>>> +	},
>>> +	[ID_AD5640_1250] = {
>>> +		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
>>> +		.int_vref_mv = 1250,
>>> +		.write = ad5446_write,
>>> +	},
>>> +	[ID_AD5660_2500] = {
>>> +		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
>>> +		.int_vref_mv = 2500,
>>> +		.write = ad5660_write,
>>> +	},
>>> +	[ID_AD5660_1250] = {
>>> +		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
>>> +		.int_vref_mv = 1250,
>>> +		.write = ad5660_write,
>>> +	},
>>> +	[ID_AD5662] = {
>>> +		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
>>> +		.write = ad5660_write,
>>> +	},
>>> +};
>>> +
>>> +static const struct spi_device_id ad5446_spi_ids[] = {
>>>  	{"ad5444", ID_AD5444},
>>>  	{"ad5446", ID_AD5446},
>>>  	{"ad5450", ID_AD5450},
>>> @@ -375,18 +408,157 @@ static const struct spi_device_id ad5446_id[] = {
>>>  	{"ad5662", ID_AD5662},
>>>  	{}
>>>  };
>>> -MODULE_DEVICE_TABLE(spi, ad5446_id);
>>> +MODULE_DEVICE_TABLE(spi, ad5446_spi_ids);
>>> +
>>> +static int __devinit ad5446_spi_probe(struct spi_device *spi)
>>> +{
>>> +	const struct spi_device_id *id = spi_get_device_id(spi);
>>> +
>>> +	return ad5446_probe(&spi->dev, id->name,
>>> +		&ad5446_spi_chip_info[id->driver_data]);
>>> +}
>>>  
>>> -static struct spi_driver ad5446_driver = {
>>> +static int __devexit ad5446_spi_remove(struct spi_device *spi)
>>> +{
>>> +	return ad5446_remove(&spi->dev);
>>> +}
>>> +
>>> +static struct spi_driver ad5446_spi_driver = {
>>>  	.driver = {
>>>  		.name	= "ad5446",
>>>  		.owner	= THIS_MODULE,
>>>  	},
>>> -	.probe		= ad5446_probe,
>>> -	.remove		= __devexit_p(ad5446_remove),
>>> -	.id_table	= ad5446_id,
>>> +	.probe		= ad5446_spi_probe,
>>> +	.remove		= __devexit_p(ad5446_spi_remove),
>>> +	.id_table	= ad5446_spi_ids,
>>> +};
>>> +
>>> +static int __init ad5446_spi_register_driver(void)
>>> +{
>>> +	return spi_register_driver(&ad5446_spi_driver);
>>> +}
>>> +
>>> +static void ad5446_spi_unregister_driver(void)
>>> +{
>>> +	spi_unregister_driver(&ad5446_spi_driver);
>>> +}
>>> +
>>> +#else
>>> +
>>> +static inline int ad5446_spi_register_driver(void) { return 0; }
>>> +static inline void ad5446_spi_unregister_driver(void) { }
>>> +
>>> +#endif
>>> +
>>> +#if IS_ENABLED(CONFIG_I2C)
>>> +
>>> +static int ad5622_write(struct ad5446_state *st, unsigned val)
>>> +{
>>> +	struct i2c_client *client = to_i2c_client(st->dev);
>>> +	__be16 data = cpu_to_be16(val);
>>> +
>>> +	return i2c_master_send(client, (char *)&data, sizeof(data));
>>> +}
>>> +
>>> +/**
>>> + * ad5446_supported_i2c_device_ids:
>>> + * The AD5620/40/60 parts are available in different fixed internal reference
>>> + * voltage options. The actual part numbers may look differently
>>> + * (and a bit cryptic), however this style is used to make clear which
>>> + * parts are supported here.
>>> + */
>>> +enum ad5446_supported_i2c_device_ids {
>>> +	ID_AD5602,
>>> +	ID_AD5612,
>>> +	ID_AD5622,
>>> +};
>>> +
>>> +static const struct ad5446_chip_info ad5446_i2c_chip_info[] = {
>>> +	[ID_AD5602] = {
>>> +		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 4),
>>> +		.write = ad5622_write,
>>> +	},
>>> +	[ID_AD5612] = {
>>> +		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 2),
>>> +		.write = ad5622_write,
>>> +	},
>>> +	[ID_AD5622] = {
>>> +		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 0),
>>> +		.write = ad5622_write,
>>> +	},
>>>  };
>>> -module_spi_driver(ad5446_driver);
>>> +
>>> +static int __devinit ad5446_i2c_probe(struct i2c_client *i2c,
>>> +	const struct i2c_device_id *id)
>>> +{
>>> +	return ad5446_probe(&i2c->dev, id->name,
>>> +		&ad5446_i2c_chip_info[id->driver_data]);
>>> +}
>>> +
>>> +static int __devexit ad5446_i2c_remove(struct i2c_client *i2c)
>>> +{
>>> +	return ad5446_remove(&i2c->dev);
>>> +}
>>> +
>>> +static const struct i2c_device_id ad5446_i2c_ids[] = {
>>> +	{"ad5602", ID_AD5602},
>>> +	{"ad5612", ID_AD5612},
>>> +	{"ad5622", ID_AD5622},
>>> +	{}
>>> +};
>>> +MODULE_DEVICE_TABLE(i2c, ad5446_i2c_ids);
>>> +
>>> +static struct i2c_driver ad5446_i2c_driver = {
>>> +	.driver = {
>>> +		   .name = "ad5446",
>>> +		   .owner = THIS_MODULE,
>>> +	},
>>> +	.probe = ad5446_i2c_probe,
>>> +	.remove = __devexit_p(ad5446_i2c_remove),
>>> +	.id_table = ad5446_i2c_ids,
>>> +};
>>> +
>>> +static int __init ad5446_i2c_register_driver(void)
>>> +{
>>> +	return i2c_add_driver(&ad5446_i2c_driver);
>>> +}
>>> +
>>> +static void __exit ad5446_i2c_unregister_driver(void)
>>> +{
>>> +	i2c_del_driver(&ad5446_i2c_driver);
>>> +}
>>> +
>>> +#else
>>> +
>>> +static inline int ad5446_i2c_register_driver(void) { return 0; }
>>> +static inline void ad5446_i2c_unregister_driver(void) { }
>>> +
>>> +#endif
>>> +
>>> +static int __init ad5446_init(void)
>>> +{
>>> +	int ret;
>>> +
>>> +	ret = ad5446_spi_register_driver();
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	ret = ad5446_i2c_register_driver();
>>> +	if (ret) {
>>> +		ad5446_spi_unregister_driver();
>>> +		return ret;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +module_init(ad5446_init);
>>> +
>>> +static void __exit ad5446_exit(void)
>>> +{
>>> +	ad5446_i2c_unregister_driver();
>>> +	ad5446_spi_unregister_driver();
>>> +}
>>> +module_exit(ad5446_exit);
>>>  
>>>  MODULE_AUTHOR("Michael Hennerich <hennerich@blackfin.uclinux.org>");
>>>  MODULE_DESCRIPTION("Analog Devices AD5444/AD5446 DAC");
>>> diff --git a/drivers/iio/dac/ad5446.h b/drivers/iio/dac/ad5446.h
>>> index 2934269..6b7a176 100644
>>> --- a/drivers/iio/dac/ad5446.h
>>> +++ b/drivers/iio/dac/ad5446.h
>>> @@ -38,7 +38,7 @@
>>>   */
>>>  
>>>  struct ad5446_state {
>>> -	struct spi_device		*spi;
>>> +	struct device		*dev;
>>>  	const struct ad5446_chip_info	*chip_info;
>>>  	struct regulator		*reg;
>>>  	unsigned short			vref_mv;
>>> @@ -60,32 +60,5 @@ struct ad5446_chip_info {
>>>  	int			(*write)(struct ad5446_state *st, unsigned val);
>>>  };
>>>  
>>> -/**
>>> - * ad5446_supported_device_ids:
>>> - * The AD5620/40/60 parts are available in different fixed internal reference
>>> - * voltage options. The actual part numbers may look differently
>>> - * (and a bit cryptic), however this style is used to make clear which
>>> - * parts are supported here.
>>> - */
>>> -
>>> -enum ad5446_supported_device_ids {
>>> -	ID_AD5444,
>>> -	ID_AD5446,
>>> -	ID_AD5450,
>>> -	ID_AD5451,
>>> -	ID_AD5541A,
>>> -	ID_AD5512A,
>>> -	ID_AD5553,
>>> -	ID_AD5601,
>>> -	ID_AD5611,
>>> -	ID_AD5621,
>>> -	ID_AD5620_2500,
>>> -	ID_AD5620_1250,
>>> -	ID_AD5640_2500,
>>> -	ID_AD5640_1250,
>>> -	ID_AD5660_2500,
>>> -	ID_AD5660_1250,
>>> -	ID_AD5662,
>>> -};
>>>  
>>>  #endif /* IIO_DAC_AD5446_H_ */
>>>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3.6-rc2 V2 1/2] iio:ad5446: Add support for I2C based DACs
  2012-09-03  8:01         ` Lars-Peter Clausen
@ 2012-09-03 20:11           ` Jonathan Cameron
  0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2012-09-03 20:11 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Jean-Francois Dagenais, linux-iio, device-drivers-devel

On 09/03/2012 09:01 AM, Lars-Peter Clausen wrote:
> On 08/24/2012 01:38 PM, Lars-Peter Clausen wrote:
>> On 08/22/2012 08:24 PM, Jonathan Cameron wrote:
>>> On 08/21/2012 03:28 PM, Jean-Francois Dagenais wrote:
>>>> This patch adds support for I2C based single channel DACs to the ad5446
>>>> driver. Specifically AD5602, AD5612 and AD5622.
>>>>
>>>> V1: from Lars-Peter Clausen <lars@metafoo.de>
>>>> V2: Split the device IDs into two enums and move them to the c file.
>>> Sensible change.  Patch is fine but given we are still well away from
>>> the merge window I'll wait for Lars-Peter's response before merging this.
>>>
>>
>> Looks good to me on first sight. I'd have done the two patches in reverse
>> order, but it doesn't really matter.
>>
>> Will take a closer look when I'm back from vacation, but I'm already quite
>> confident that it can go in as is.
>>
> 
> 
> Both patches:
> Acked-by: Lars-Peter Clausen <lars@metafoo.de>

Merge and pull request sent to Greg.

Thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-09-03 20:11 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <502E10A4.1080800@metafoo.de>
2012-08-20 19:23 ` Jean-François Dagenais
2012-08-20 19:23   ` [PATCH] iio:ad5446: Add support for I2C based DACs Jean-François Dagenais
2012-08-21 13:17     ` Jean-François Dagenais
2012-08-21 14:28 ` [PATCH v3.6-rc2 V2 0/2] iio:ad5446: add I2C DACs and header cleanup Jean-Francois Dagenais
2012-08-21 14:28   ` [PATCH v3.6-rc2 V2 1/2] iio:ad5446: Add support for I2C based DACs Jean-Francois Dagenais
2012-08-22 18:24     ` Jonathan Cameron
2012-08-24 11:38       ` Lars-Peter Clausen
2012-09-03  8:01         ` Lars-Peter Clausen
2012-09-03 20:11           ` Jonathan Cameron
2012-08-21 14:28   ` [PATCH v3.6-rc2 V2 2/2] iio:ad5446: get rid of private header file Jean-Francois Dagenais

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.