From: Jonathan Cameron <jic23@kernel.org>
To: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
Cc: linux-iio@vger.kernel.org, lars@metafoo.de,
device-drivers-devel@blackfin.uclinux.org
Subject: Re: [PATCH v3.6-rc2 V2 1/2] iio:ad5446: Add support for I2C based DACs
Date: Wed, 22 Aug 2012 19:24:29 +0100 [thread overview]
Message-ID: <503523DD.2030203@kernel.org> (raw)
In-Reply-To: <1345559338-19509-2-git-send-email-jeff.dagenais@gmail.com>
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_ */
>
next prev parent reply other threads:[~2012-08-22 18:24 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
[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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=503523DD.2030203@kernel.org \
--to=jic23@kernel.org \
--cc=device-drivers-devel@blackfin.uclinux.org \
--cc=jeff.dagenais@gmail.com \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox