On Mon, 22 Oct 2018 23:02:42 +0200 Charles-Antoine Couret <charles-antoine.couret@xxxxxxxxxxxxx> wrote: > Compatible with AD7682 and AD7689 chips. > It is a Analog Devices ADC driver 14/16 bits 4/8 channels > with SPI protocol > > Datasheet of the device: > http://www.analog.com/media/en/technical-documentation/data-sheets/AD7949.pdf > > Signed-off-by: Charles-Antoine Couret <charles-antoine.couret@xxxxxxxxxxxxx> Hi Charles-Antoine There are a couple of really small things in here. Given most of them weren't in my previous list and to save us both time I'll fix them up whilst applying. Please do check I didn't mess it up however! Jonathan > --- > V1 to V2: > * Use GENMASK > * Little refactoring to define SPI message parameters > * Add ____cacheline_aligned for SPI buffer > * Comment required delays > * Clean switch statements > * Init SPI before iio > > V2 to V3: > * Merge message init functions into spi_message_init_with_transfers call > * Add vendor prefix for device tree > * Remove SPI settings overrinding, use info from device tree only > > drivers/iio/adc/Kconfig | 10 ++ > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/ad7949.c | 347 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 358 insertions(+) > create mode 100644 drivers/iio/adc/ad7949.c > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index 4a754921fb6f..42e66efff6c0 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -116,6 +116,16 @@ config AD7923 > To compile this driver as a module, choose M here: the > module will be called ad7923. > > +config AD7949 > + tristate "Analog Devices AD7949 and similar ADCs driver" > + depends on SPI > + help > + Say yes here to build support for Analog Devices > + AD7949, AD7682, AD7689 8 Channel ADCs. > + > + To compile this driver as a module, choose M here: the > + module will be called ad7949. > + > config AD799X > tristate "Analog Devices AD799x ADC driver" > depends on I2C > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > index 03db7b578f9c..88804c867aa9 100644 > --- a/drivers/iio/adc/Makefile > +++ b/drivers/iio/adc/Makefile > @@ -14,6 +14,7 @@ obj-$(CONFIG_AD7766) += ad7766.o > obj-$(CONFIG_AD7791) += ad7791.o > obj-$(CONFIG_AD7793) += ad7793.o > obj-$(CONFIG_AD7887) += ad7887.o > +obj-$(CONFIG_AD7949) += ad7949.o > obj-$(CONFIG_AD799X) += ad799x.o > obj-$(CONFIG_ASPEED_ADC) += aspeed_adc.o > obj-$(CONFIG_AT91_ADC) += at91_adc.o > diff --git a/drivers/iio/adc/ad7949.c b/drivers/iio/adc/ad7949.c > new file mode 100644 > index 000000000000..5f93ab0ebac2 > --- /dev/null > +++ b/drivers/iio/adc/ad7949.c > @@ -0,0 +1,348 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* ad7949.c - Analog Devices ADC driver 14/16 bits 4/8 channels > + * > + * Copyright (C) 2018 CMC NV > + * > + * http://www.analog.com/media/en/technical-documentation/data-sheets/AD7949.pdf > + */ > + > +#include <linux/delay.h> > +#include <linux/iio/iio.h> > +#include <linux/module.h> > +#include <linux/regulator/consumer.h> > +#include <linux/spi/spi.h> > + > +#define AD7949_MASK_CHANNEL_SEL GENMASK(9, 7) > +#define AD7949_MASK_TOTAL GENMASK(13, 0) > +#define AD7949_OFFSET_CHANNEL_SEL 7 > +#define AD7949_CFG_READ_BACK 0x1 > +#define AD7949_CFG_REG_SIZE_BITS 14 > + > +enum { > + ID_AD7949 = 0, > + ID_AD7682, > + ID_AD7689, > +}; > + > +struct ad7949_adc_spec { > + u8 num_channels; > + u8 resolution; > +}; > + > +static const struct ad7949_adc_spec ad7949_adc_spec[] = { > + [ID_AD7949] = { .num_channels = 8, .resolution = 14 }, > + [ID_AD7682] = { .num_channels = 4, .resolution = 16 }, > + [ID_AD7689] = { .num_channels = 8, .resolution = 16 }, > +}; > + > +/** > + * struct ad7949_adc_chip - AD ADC chip > + * @lock: protects write sequences > + * @vref: regulator generating Vref > + * @iio_dev: reference to iio structure > + * @spi: reference to spi structure > + * @resolution: resolution of the chip > + * @cfg: copy of the configuration register > + * @current_channel: current channel in use > + * @buffer: buffer to send / receive data to / from device > + */ > +struct ad7949_adc_chip { > + struct mutex lock; > + struct regulator *vref; > + struct iio_dev *indio_dev; > + struct spi_device *spi; > + u8 resolution; > + u16 cfg; > + unsigned int current_channel; > + u32 buffer ____cacheline_aligned; > +}; > + > +static bool ad7949_spi_cfg_is_read_back(struct ad7949_adc_chip *ad7949_adc) > +{ > + if (!(ad7949_adc->cfg & AD7949_CFG_READ_BACK)) > + return true; > + > + return false; > +} > + > +static int ad7949_spi_bits_per_word(struct ad7949_adc_chip *ad7949_adc) > +{ > + int ret = ad7949_adc->resolution; > + > + if (ad7949_spi_cfg_is_read_back(ad7949_adc)) > + ret += AD7949_CFG_REG_SIZE_BITS; > + > + return ret; > +} > + > +static int ad7949_spi_write_cfg(struct ad7949_adc_chip *ad7949_adc, u16 val, > + u16 mask) > +{ > + int ret; > + int bits_per_word = ad7949_spi_bits_per_word(ad7949_adc); > + int shift = bits_per_word - AD7949_CFG_REG_SIZE_BITS; > + struct spi_message msg; > + struct spi_transfer tx[] = { > + { > + .tx_buf = &ad7949_adc->buffer, > + .len = 4, > + .bits_per_word = bits_per_word, > + }, > + }; > + > + ad7949_adc->cfg = (val & mask) | (ad7949_adc->cfg & ~mask); > + ad7949_adc->buffer = ad7949_adc->cfg << shift; > + spi_message_init_with_transfers(&msg, tx, 1); > + ret = spi_sync(ad7949_adc->spi, &msg); > + > + /* > + * This delay is to avoid a new request before the required time to > + * send a new command to the device > + */ > + udelay(2); > + return ret; > +} > + > +static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val, > + unsigned int channel) > +{ > + int ret; > + int bits_per_word = ad7949_spi_bits_per_word(ad7949_adc); > + int mask = GENMASK(ad7949_adc->resolution, 0); > + struct spi_message msg; > + struct spi_transfer tx[] = { > + { > + .rx_buf = &ad7949_adc->buffer, > + .len = 4, > + .bits_per_word = bits_per_word, > + }, > + }; > + > + ret = ad7949_spi_write_cfg(ad7949_adc, > + channel << AD7949_OFFSET_CHANNEL_SEL, > + AD7949_MASK_CHANNEL_SEL); > + if (ret) > + return ret; > + > + ad7949_adc->buffer = 0; > + spi_message_init_with_transfers(&msg, tx, 1); > + ret = spi_sync(ad7949_adc->spi, &msg); > + if (ret) > + return ret; > + > + /* > + * This delay is to avoid a new request before the required time to > + * send a new command to the device > + */ > + udelay(2); > + > + ad7949_adc->current_channel = channel; > + > + if (ad7949_spi_cfg_is_read_back(ad7949_adc)) > + *val = (ad7949_adc->buffer >> AD7949_CFG_REG_SIZE_BITS) & mask; > + else > + *val = ad7949_adc->buffer & mask; > + > + return 0; > +} > + > +#define AD7949_ADC_CHANNEL(chan) { \ > + .type = IIO_VOLTAGE, \ > + .indexed = 1, \ > + .channel = (chan), \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ > +} > + > +static const struct iio_chan_spec ad7949_adc_channels[] = { > + AD7949_ADC_CHANNEL(0), > + AD7949_ADC_CHANNEL(1), > + AD7949_ADC_CHANNEL(2), > + AD7949_ADC_CHANNEL(3), > + AD7949_ADC_CHANNEL(4), > + AD7949_ADC_CHANNEL(5), > + AD7949_ADC_CHANNEL(6), > + AD7949_ADC_CHANNEL(7), > +}; > + > +static int ad7949_spi_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long mask) > +{ > + struct ad7949_adc_chip *ad7949_adc = iio_priv(indio_dev); > + int ret; > + > + if (!val) > + return -EINVAL; > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + mutex_lock(&ad7949_adc->lock); > + ret = ad7949_spi_read_channel(ad7949_adc, val, chan->channel); > + mutex_unlock(&ad7949_adc->lock); > + > + if (ret < 0) > + return ret; > + > + return IIO_VAL_INT; > + > + case IIO_CHAN_INFO_SCALE: > + ret = regulator_get_voltage(ad7949_adc->vref); > + if (ret < 0) > + return ret; > + > + *val = ret / 5000; > + return IIO_VAL_INT; > + } > + > + return -EINVAL; > +} > + > +static int ad7949_spi_reg_access(struct iio_dev *indio_dev, > + unsigned int reg, unsigned int writeval, > + unsigned int *readval) > +{ > + struct ad7949_adc_chip *ad7949_adc = iio_priv(indio_dev); > + int ret = 0; > + > + if (readval) > + *readval = ad7949_adc->cfg; > + else > + ret = ad7949_spi_write_cfg(ad7949_adc, > + writeval & AD7949_MASK_TOTAL, AD7949_MASK_TOTAL); > + > + return ret; > +} > + > +static const struct iio_info ad7949_spi_info = { > + .read_raw = ad7949_spi_read_raw, > + .debugfs_reg_access = ad7949_spi_reg_access, > +}; > + > +static int ad7949_spi_init(struct ad7949_adc_chip *ad7949_adc) > +{ > + int ret; > + int val; > + > + /* Sequencer disabled, CFG readback disabled, IN0 as default channel */ > + ad7949_adc->current_channel = 0; > + ret = ad7949_spi_write_cfg(ad7949_adc, 0x3C79, AD7949_MASK_TOTAL); > + > + /* Do two dummy conversions to apply the first configuration setting. /* * Do two... > + * Required only after the start up of the device. > + */ > + ad7949_spi_read_channel(ad7949_adc, &val, ad7949_adc->current_channel); I'm not that keen on the absence of error checking on these. Given they aren't real I'll let it go though. > + ad7949_spi_read_channel(ad7949_adc, &val, ad7949_adc->current_channel); > + > + return ret; > +} > + > +static int ad7949_spi_probe(struct spi_device *spi) > +{ > + struct device *dev = &spi->dev; > + const struct ad7949_adc_spec *spec; > + struct ad7949_adc_chip *ad7949_adc; > + struct iio_dev *indio_dev; > + int ret; > + > + indio_dev = devm_iio_device_alloc(dev, sizeof(*ad7949_adc)); > + if (!indio_dev) { > + dev_err(dev, "can not allocate iio device\n"); > + return -ENOMEM; > + } > + > + indio_dev->dev.parent = dev; > + indio_dev->dev.of_node = dev->of_node; > + indio_dev->info = &ad7949_spi_info; > + indio_dev->name = spi_get_device_id(spi)->name; > + indio_dev->modes = INDIO_DIRECT_MODE; > + indio_dev->channels = ad7949_adc_channels; > + spi_set_drvdata(spi, indio_dev); > + > + ad7949_adc = iio_priv(indio_dev); > + ad7949_adc->indio_dev = indio_dev; > + ad7949_adc->spi = spi; > + > + spec = &ad7949_adc_spec[spi_get_device_id(spi)->driver_data]; > + indio_dev->num_channels = spec->num_channels; > + ad7949_adc->resolution = spec->resolution; > + > + ad7949_adc->vref = devm_regulator_get(dev, "vref"); > + if (IS_ERR(ad7949_adc->vref)) { > + dev_err(dev, "fail to request regulator\n"); > + return PTR_ERR(ad7949_adc->vref); > + } > + > + ret = regulator_enable(ad7949_adc->vref); > + if (ret < 0) { > + dev_err(dev, "fail to enable regulator\n"); > + goto err_regulator; > + } > + > + mutex_init(&ad7949_adc->lock); > + > + ret = ad7949_spi_init(ad7949_adc); > + if (ret) { > + dev_err(dev, "enable to init this device: %d\n", ret); > + goto err; > + } > + > + ret = iio_device_register(indio_dev); > + if (ret) { > + dev_err(dev, "fail to register iio device: %d\n", ret); > + goto err; > + } > + > + return 0; > + > +err: > + mutex_destroy(&ad7949_adc->lock); > + regulator_disable(ad7949_adc->vref); > +err_regulator: It's preferred to return directly if there is no cleanup to do. Makes the code more readable (as you don't have to check the cleanup paths on a direct return - other than verifying there shouldn't be any of them). > + return ret; > +} > + > +static int ad7949_spi_remove(struct spi_device *spi) > +{ > + struct iio_dev *indio_dev = spi_get_drvdata(spi); > + struct ad7949_adc_chip *ad7949_adc = iio_priv(indio_dev); > + > + iio_device_unregister(indio_dev); > + mutex_destroy(&ad7949_adc->lock); > + regulator_disable(ad7949_adc->vref); > + > + return 0; > +} > + > +#ifdef CONFIG_OF > +static const struct of_device_id ad7949_spi_of_id[] = { > + { .compatible = "adi,ad7949" }, > + { .compatible = "adi,ad7682" }, > + { .compatible = "adi,ad7689" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, ad7949_spi_of_id); > +#endif > + > +static const struct spi_device_id ad7949_spi_id[] = { > + { "ad7949", ID_AD7949 }, > + { "ad7682", ID_AD7682 }, > + { "ad7689", ID_AD7689 }, > + { } > +}; > +MODULE_DEVICE_TABLE(spi, ad7949_spi_id); > + > +static struct spi_driver ad7949_spi_driver = { > + .driver = { > + .name = "ad7949", > + .of_match_table = of_match_ptr(ad7949_spi_of_id), Now there is an interesting story behind of_match_ptr. We used to only be interested in devicetree id tables if we were running devicetree, then someone had a clever idea of providing a handy way (for testing) of using ACPI (with a magic ACPI id) to instantiate device drivers based on their devicetree pointers. That only works if we don't define the of_match_table to NULL. Upshot is that general view is now don't use of_match_ptr certainly for devices that are likely to be randomly tested on boards (like this might be). The same is true of not defining out the table above. > + }, > + .probe = ad7949_spi_probe, > + .remove = ad7949_spi_remove, > + .id_table = ad7949_spi_id, > +}; > +module_spi_driver(ad7949_spi_driver); > + > +MODULE_AUTHOR("Charles-Antoine Couret <charles-antoine.couret@xxxxxxxxxxxxx>"); > +MODULE_DESCRIPTION("Analog Devices 14/16-bit 8-channel ADC driver"); > +MODULE_LICENSE("GPL v2");
