| Message ID | 20231208-ad7380-mainline-v1-2-2b33fe2f44ae@baylibre.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adc: add new ad7380 driver | expand |
On Fri, Dec 8, 2023 at 9:52 AM David Lechner <dlechner@baylibre.com> wrote: > > This adds a new driver for the AD7380 family ADCs. > > The driver currently implements basic support for the AD7380, AD7381, > AD7383, and AD7384 2-channel differential ADCs. Support for additional > single-ended and 4-channel chips that use the same register map as well > as additional features of the chip will be added in future patches. > > Co-developed-by: Stefan Popa <stefan.popa@analog.com> > Signed-off-by: Stefan Popa <stefan.popa@analog.com> > Signed-off-by: David Lechner <dlechner@baylibre.com> > --- ... > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > index bee11d442af4..e046d8004f41 100644 > --- a/drivers/iio/adc/Makefile > +++ b/drivers/iio/adc/Makefile > @@ -16,6 +16,7 @@ obj-$(CONFIG_AD7291) += ad7291.o > obj-$(CONFIG_AD7292) += ad7292.o > obj-$(CONFIG_AD7298) += ad7298.o > obj-$(CONFIG_AD7923) += ad7923.o > +obj-$(CONFIG_AD7476) += ad7380.o copy/paste oops: should be CONFIG_AD7380 > obj-$(CONFIG_AD7476) += ad7476.o > obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o > obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o
On Fri, 8 Dec 2023 09:51:41 -0600 David Lechner <dlechner@baylibre.com> wrote: > This adds a new driver for the AD7380 family ADCs. > > The driver currently implements basic support for the AD7380, AD7381, > AD7383, and AD7384 2-channel differential ADCs. Support for additional > single-ended and 4-channel chips that use the same register map as well > as additional features of the chip will be added in future patches. > > Co-developed-by: Stefan Popa <stefan.popa@analog.com> > Signed-off-by: Stefan Popa <stefan.popa@analog.com> > Signed-off-by: David Lechner <dlechner@baylibre.com> Hi David / Stefan, A few minor things inline. Other than that question following through from the bindings about 1-wire / 2-wire description it's all trivial and some might be considered my odd tastes :) Nice to see such a clean v1 Jonathan > diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c > new file mode 100644 > index 000000000000..6a5ec59bd1fd > --- /dev/null > +++ b/drivers/iio/adc/ad7380.c > +struct ad7380_state { > + const struct ad7380_chip_info *chip_info; > + struct spi_device *spi; > + struct regulator *vref; > + struct regmap *regmap; > + /* > + * DMA (thus cache coherency maintenance) requires the > + * transfer buffers to live in their own cache lines. > + * Make the buffer large enough for 2 16-bit samples and one 64-bit > + * aligned 64 bit timestamp. > + */ > + struct { > + u16 raw[2]; > + s64 ts __aligned(8); > + } scan_data __aligned(IIO_DMA_MINALIGN); > + u16 tx[2]; > + u16 rx[2]; > +}; ... > +static irqreturn_t ad7380_trigger_handler(int irq, void *p) > +{ > + struct iio_poll_func *pf = p; > + struct iio_dev *indio_dev = pf->indio_dev; > + struct ad7380_state *st = iio_priv(indio_dev); > + struct spi_transfer xfer = { > + .bits_per_word = st->chip_info->channels[0].scan_type.realbits, > + .len = 4, > + .rx_buf = &st->scan_data, I'd make it explicit you are reading into st->scan_data->raw rather than using the address of the containing structure. > + }; > + int ret; > + > + ret = spi_sync_transfer(st->spi, &xfer, 1); > + > + if (ret == 0) I'm not keen on this pattern. It saves a bit of text but makes things slightly less obvious when compared to all the other error paths. if (ret) goto error; iio_push_to_buffers... error: iio_trigger_notify_done... Is my preference. > + iio_push_to_buffers_with_timestamp(indio_dev, &st->scan_data, > + pf->timestamp); > + > + iio_trigger_notify_done(indio_dev->trig); > + > + return IRQ_HANDLED; > +} > + > +static int ad7380_read_direct(struct ad7380_state *st, > + struct iio_chan_spec const *chan, int *val) > +{ > + struct spi_transfer xfers[] = { > + /* toggle CS (no data xfer) to trigger a conversion */ > + { > + .speed_hz = AD7380_REG_WR_SPEED_HZ, > + .bits_per_word = chan->scan_type.realbits, > + .delay = { > + .value = 190, /* t[CONVERT] */ > + .unit = SPI_DELAY_UNIT_NSECS, > + }, > + .cs_change = 1, > + .cs_change_delay = { > + .value = 10, /* t[CSH] */ > + .unit = SPI_DELAY_UNIT_NSECS, > + }, > + }, > + /* then read both channels */ > + { > + .speed_hz = AD7380_REG_WR_SPEED_HZ, > + .bits_per_word = chan->scan_type.realbits, > + .rx_buf = &st->rx[0], > + .len = 4, > + }, > + }; > + int ret; > + > + ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers)); > + Trivial, but no blank line here. It's good to keep error check and the call in the same block. > + if (ret < 0) > + return ret; > + > + *val = sign_extend32(st->rx[chan->scan_index], > + chan->scan_type.realbits - 1); > + > + return IIO_VAL_INT; > +} > + ... > + > +static int ad7380_init(struct ad7380_state *st) > +{ > + int ret; > + > + /* perform hard reset */ > + ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG2, > + AD7380_CONFIG2_RESET, > + FIELD_PREP(AD7380_CONFIG2_RESET, > + AD7380_CONFIG2_RESET_HARD)); > + if (ret < 0) > + return ret; > + > + Trivial: Single blank line > + /* select internal or external reference voltage */ > + ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG1, > + AD7380_CONFIG1_REFSEL, > + FIELD_PREP(AD7380_CONFIG1_REFSEL, !!st->vref)); > + if (ret < 0) > + return ret; > + > + /* SPI 1-wire mode */ > + return regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG2, > + AD7380_CONFIG2_SDO, > + FIELD_PREP(AD7380_CONFIG2_SDO, 1)); > +} > + > +static void ad7380_release_regulator(void *p) > +{ > + struct regulator *reg = p; The local variable doesn't really add anything over putting p in the regulator_disable() call. > + > + regulator_disable(reg); > +} > + > +static int ad7380_probe(struct spi_device *spi) > +{ > + struct iio_dev *indio_dev; > + struct ad7380_state *st; > + const char *str_val; > + int ret; > + > + ret = device_property_read_string(&spi->dev, "adi,sdo-mode", &str_val); > + if (ret < 0) > + return dev_err_probe(&spi->dev, ret, > + "Failed to read adi,sdo-mode property\n"); > + > + if (strcmp(str_val, "1-wire")) > + return dev_err_probe(&spi->dev, -EINVAL, > + "Only 1-wire SDO is supported\n"); Discussion on this binding in the dt-binding patch. As mentioned there, it feels like a place where a default would be sensible. > + > + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); > + if (!indio_dev) > + return -ENOMEM; > + > + st = iio_priv(indio_dev); > + st->spi = spi; > + st->chip_info = spi_get_device_match_data(spi); > + > + st->vref = devm_regulator_get_optional(&spi->dev, "refio"); > + if (IS_ERR(st->vref)) { > + /* > + * If there is no REFIO supply, then it means that we are using > + * the internal 2.5V reference. > + */ > + if (PTR_ERR(st->vref) == -ENODEV) > + st->vref = NULL; > + else > + return dev_err_probe(&spi->dev, PTR_ERR(st->vref), > + "Failed to get refio regulator\n"); > + } > + > + if (st->vref) { > + ret = regulator_enable(st->vref); > + if (ret) > + return ret; > + > + ret = devm_add_action_or_reset(&spi->dev, ad7380_release_regulator, Naming wise maybe ad7380_disable_regulator is less misleading that release (which in my mind is the bit the unwind for devm_regulator_get_optional() is doing) > + st->vref); > + if (ret) > + return ret; > + }
Hi David, minor stuff from me... On Fri, 2023-12-08 at 09:51 -0600, David Lechner wrote: > This adds a new driver for the AD7380 family ADCs. > > The driver currently implements basic support for the AD7380, AD7381, > AD7383, and AD7384 2-channel differential ADCs. Support for additional > single-ended and 4-channel chips that use the same register map as well > as additional features of the chip will be added in future patches. > > Co-developed-by: Stefan Popa <stefan.popa@analog.com> > Signed-off-by: Stefan Popa <stefan.popa@analog.com> > Signed-off-by: David Lechner <dlechner@baylibre.com> > --- > MAINTAINERS | 1 + > drivers/iio/adc/Kconfig | 16 ++ > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/ad7380.c | 467 > +++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 485 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index e2a998be5879..5a54620a31b8 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -438,6 +438,7 @@ S: Supported > W: > https://wiki.analog.com/resources/tools-software/linux-drivers/iio-adc/ad738x > W: https://ez.analog.com/linux-software-drivers > F: Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml > +F: drivers/iio/adc/ad7380.c > > AD7877 TOUCHSCREEN DRIVER > M: Michael Hennerich <michael.hennerich@analog.com> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index 35f9867da12c..cbfd626712e3 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -122,6 +122,22 @@ config AD7298 > To compile this driver as a module, choose M here: the > module will be called ad7298. > > +config AD7380 > + tristate "Analog Devices AD7380 ADC driver" > + depends on SPI_MASTER > + select IIO_BUFFER > + select IIO_TRIGGER > + select IIO_TRIGGERED_BUFFER > + help > + AD7380 is a family of simultaneous sampling ADCs that share the > same > + SPI register map and have similar pinouts. > + > + Say yes here to build support for Analog Devices AD7380 ADC and > + similar chips. > + > + To compile this driver as a module, choose M here: the module will > be > + called ad7380. > + > config AD7476 > tristate "Analog Devices AD7476 1-channel ADCs driver and other > similar devices from AD and TI" > depends on SPI > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > index bee11d442af4..e046d8004f41 100644 > --- a/drivers/iio/adc/Makefile > +++ b/drivers/iio/adc/Makefile > @@ -16,6 +16,7 @@ obj-$(CONFIG_AD7291) += ad7291.o > obj-$(CONFIG_AD7292) += ad7292.o > obj-$(CONFIG_AD7298) += ad7298.o > obj-$(CONFIG_AD7923) += ad7923.o > +obj-$(CONFIG_AD7476) += ad7380.o > obj-$(CONFIG_AD7476) += ad7476.o > obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o > obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o > diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c > new file mode 100644 > index 000000000000..6a5ec59bd1fd > --- /dev/null > +++ b/drivers/iio/adc/ad7380.c > @@ -0,0 +1,467 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Analog Devices AD738x Simultaneous Sampling SAR ADCs > + * > + * Copyright 2017 Analog Devices Inc. > + * Copyright 2023 BayLibre, SAS > + */ > + > +#include <linux/bitfield.h> > +#include <linux/bitops.h> > +#include <linux/cleanup.h> > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/regmap.h> > +#include <linux/regulator/consumer.h> > +#include <linux/slab.h> > +#include <linux/spi/spi.h> > +#include <linux/sysfs.h> > + > +#include <linux/iio/buffer.h> > +#include <linux/iio/iio.h> > +#include <linux/iio/sysfs.h> > +#include <linux/iio/trigger_consumer.h> > +#include <linux/iio/triggered_buffer.h> > + > ... > + > +static int ad7380_debugfs_reg_access(struct iio_dev *indio_dev, u32 reg, > + u32 writeval, u32 *readval) > +{ > + struct ad7380_state *st = iio_priv(indio_dev); > + int ret; > + > + ret = iio_device_claim_direct_mode(indio_dev); > + if (ret) > + return ret; > + potential controversial take: do we really need locking in here? regmap already has it's own lock (I think you're not disabling it) and if someone plays with registers it shouldn't while buffering, well... This is a debug interface so I would probably not worry much. One could anyways for write stuff by going directly to regmap :) That said, fine to be as-is from my side (just wanted to take it out of my system :))... > + if (readval) > + ret = regmap_read(st->regmap, reg, readval); > + else > + ret = regmap_write(st->regmap, reg, writeval); > + > + iio_device_release_direct_mode(indio_dev); > + > + return ret; > +} > + > +static irqreturn_t ad7380_trigger_handler(int irq, void *p) > +{ > + struct iio_poll_func *pf = p; > + struct iio_dev *indio_dev = pf->indio_dev; > + struct ad7380_state *st = iio_priv(indio_dev); > + struct spi_transfer xfer = { > + .bits_per_word = st->chip_info- > >channels[0].scan_type.realbits, > + .len = 4, > + .rx_buf = &st->scan_data, > + }; > + int ret; > + > + ret = spi_sync_transfer(st->spi, &xfer, 1); > + > + if (ret == 0) > + iio_push_to_buffers_with_timestamp(indio_dev, &st->scan_data, > + pf->timestamp); > + > + iio_trigger_notify_done(indio_dev->trig); > + > + return IRQ_HANDLED; > +} > + > +static int ad7380_read_direct(struct ad7380_state *st, > + struct iio_chan_spec const *chan, int *val) > +{ > + struct spi_transfer xfers[] = { > + /* toggle CS (no data xfer) to trigger a conversion */ > + { > + .speed_hz = AD7380_REG_WR_SPEED_HZ, > + .bits_per_word = chan->scan_type.realbits, > + .delay = { > + .value = 190, /* t[CONVERT] */ > + .unit = SPI_DELAY_UNIT_NSECS, > + }, > + .cs_change = 1, > + .cs_change_delay = { > + .value = 10, /* t[CSH] */ > + .unit = SPI_DELAY_UNIT_NSECS, > + }, > + }, > + /* then read both channels */ > + { > + .speed_hz = AD7380_REG_WR_SPEED_HZ, > + .bits_per_word = chan->scan_type.realbits, > + .rx_buf = &st->rx[0], > + .len = 4, > + }, > + }; > + int ret; > + > + ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers)); > + > + if (ret < 0) > + return ret; > + > + *val = sign_extend32(st->rx[chan->scan_index], > + chan->scan_type.realbits - 1); > + > + return IIO_VAL_INT; > +} > + > +static int ad7380_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long info) > +{ > + struct ad7380_state *st = iio_priv(indio_dev); > + int ret; > + > + switch (info) { > + case IIO_CHAN_INFO_RAW: > + ret = iio_device_claim_direct_mode(indio_dev); > + if (ret) > + return ret; > + > + ret = ad7380_read_direct(st, chan, val); > + iio_device_release_direct_mode(indio_dev); > + > + return ret; > + case IIO_CHAN_INFO_SCALE: > + if (st->vref) { > + ret = regulator_get_voltage(st->vref); nit: I wonder how likely it is for vref to change at runtime... > + if (ret < 0) > + return ret; > + > + *val = ret / 1000; > + } else { > + *val = AD7380_INTERNAL_REF_MV; > + } > + > + *val2 = chan->scan_type.realbits; > + > + return IIO_VAL_FRACTIONAL_LOG2; > + } > + > + return -EINVAL; > +} > + > +static const struct iio_info ad7380_info = { > + .read_raw = &ad7380_read_raw, > + .debugfs_reg_access = &ad7380_debugfs_reg_access, > +}; > + > +static int ad7380_init(struct ad7380_state *st) > +{ > + int ret; > + > + /* perform hard reset */ > + ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG2, > + AD7380_CONFIG2_RESET, > + FIELD_PREP(AD7380_CONFIG2_RESET, > + AD7380_CONFIG2_RESET_HARD)); > + if (ret < 0) > + return ret; > + > + > + /* select internal or external reference voltage */ > + ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG1, > + AD7380_CONFIG1_REFSEL, > + FIELD_PREP(AD7380_CONFIG1_REFSEL, !!st- > >vref)); > + if (ret < 0) > + return ret; > + > + /* SPI 1-wire mode */ > + return regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG2, > + AD7380_CONFIG2_SDO, > + FIELD_PREP(AD7380_CONFIG2_SDO, 1)); > +} > + > +static void ad7380_release_regulator(void *p) > +{ > + struct regulator *reg = p; > + > + regulator_disable(reg); > +} > + > +static int ad7380_probe(struct spi_device *spi) > +{ > + struct iio_dev *indio_dev; > + struct ad7380_state *st; > + const char *str_val; > + int ret; > + > + ret = device_property_read_string(&spi->dev, "adi,sdo-mode", > &str_val); > + if (ret < 0) > + return dev_err_probe(&spi->dev, ret, > + "Failed to read adi,sdo-mode > property\n"); > + > + if (strcmp(str_val, "1-wire")) > + return dev_err_probe(&spi->dev, -EINVAL, > + "Only 1-wire SDO is supported\n"); > + > + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); > + if (!indio_dev) > + return -ENOMEM; > + > + st = iio_priv(indio_dev); > + st->spi = spi; > + st->chip_info = spi_get_device_match_data(spi); > + if (!st->chip_info) ? > + st->vref = devm_regulator_get_optional(&spi->dev, "refio"); > + if (IS_ERR(st->vref)) { > + /* > + * If there is no REFIO supply, then it means that we are > using > + * the internal 2.5V reference. > + */ > + if (PTR_ERR(st->vref) == -ENODEV) > + st->vref = NULL; > + else > + return dev_err_probe(&spi->dev, PTR_ERR(st->vref), > + "Failed to get refio > regulator\n"); > + } > + > + if (st->vref) { > + ret = regulator_enable(st->vref); > + if (ret) > + return ret; > + > + ret = devm_add_action_or_reset(&spi->dev, > ad7380_release_regulator, > + st->vref); > + if (ret) > + return ret; > + } > + > + st->regmap = devm_regmap_init(&spi->dev, NULL, st, > &ad7380_regmap_config); > + if (IS_ERR(st->regmap)) > + return dev_err_probe(&spi->dev, PTR_ERR(st->regmap), > + "failed to allocate register map\n"); > + Could we instead have a custom regmap_bus instead of bypass regmap with reg_read() and reg_write()? > + > + indio_dev->channels = st->chip_info->channels; > + indio_dev->num_channels = st->chip_info->num_channels; > + indio_dev->dev.parent = &spi->dev; no need to assign parent (the core does it for you)... - Nuno Sá
On Tue, Dec 12, 2023 at 4:16 PM Nuno Sá <noname.nuno@gmail.com> wrote: > > Hi David, > > minor stuff from me... > > On Fri, 2023-12-08 at 09:51 -0600, David Lechner wrote: > > This adds a new driver for the AD7380 family ADCs. > > > > The driver currently implements basic support for the AD7380, AD7381, > > AD7383, and AD7384 2-channel differential ADCs. Support for additional > > single-ended and 4-channel chips that use the same register map as well > > as additional features of the chip will be added in future patches. > > > > Co-developed-by: Stefan Popa <stefan.popa@analog.com> > > Signed-off-by: Stefan Popa <stefan.popa@analog.com> > > Signed-off-by: David Lechner <dlechner@baylibre.com> > > --- > > MAINTAINERS | 1 + > > drivers/iio/adc/Kconfig | 16 ++ > > drivers/iio/adc/Makefile | 1 + > > drivers/iio/adc/ad7380.c | 467 > > +++++++++++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 485 insertions(+) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index e2a998be5879..5a54620a31b8 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -438,6 +438,7 @@ S: Supported > > W: > > https://wiki.analog.com/resources/tools-software/linux-drivers/iio-adc/ad738x > > W: https://ez.analog.com/linux-software-drivers > > F: Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml > > +F: drivers/iio/adc/ad7380.c > > > > AD7877 TOUCHSCREEN DRIVER > > M: Michael Hennerich <michael.hennerich@analog.com> > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > index 35f9867da12c..cbfd626712e3 100644 > > --- a/drivers/iio/adc/Kconfig > > +++ b/drivers/iio/adc/Kconfig > > @@ -122,6 +122,22 @@ config AD7298 > > To compile this driver as a module, choose M here: the > > module will be called ad7298. > > > > +config AD7380 > > + tristate "Analog Devices AD7380 ADC driver" > > + depends on SPI_MASTER > > + select IIO_BUFFER > > + select IIO_TRIGGER > > + select IIO_TRIGGERED_BUFFER > > + help > > + AD7380 is a family of simultaneous sampling ADCs that share the > > same > > + SPI register map and have similar pinouts. > > + > > + Say yes here to build support for Analog Devices AD7380 ADC and > > + similar chips. > > + > > + To compile this driver as a module, choose M here: the module will > > be > > + called ad7380. > > + > > config AD7476 > > tristate "Analog Devices AD7476 1-channel ADCs driver and other > > similar devices from AD and TI" > > depends on SPI > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > > index bee11d442af4..e046d8004f41 100644 > > --- a/drivers/iio/adc/Makefile > > +++ b/drivers/iio/adc/Makefile > > @@ -16,6 +16,7 @@ obj-$(CONFIG_AD7291) += ad7291.o > > obj-$(CONFIG_AD7292) += ad7292.o > > obj-$(CONFIG_AD7298) += ad7298.o > > obj-$(CONFIG_AD7923) += ad7923.o > > +obj-$(CONFIG_AD7476) += ad7380.o > > obj-$(CONFIG_AD7476) += ad7476.o > > obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o > > obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o > > diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c > > new file mode 100644 > > index 000000000000..6a5ec59bd1fd > > --- /dev/null > > +++ b/drivers/iio/adc/ad7380.c > > @@ -0,0 +1,467 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Analog Devices AD738x Simultaneous Sampling SAR ADCs > > + * > > + * Copyright 2017 Analog Devices Inc. > > + * Copyright 2023 BayLibre, SAS > > + */ > > + > > +#include <linux/bitfield.h> > > +#include <linux/bitops.h> > > +#include <linux/cleanup.h> > > +#include <linux/device.h> > > +#include <linux/err.h> > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/regmap.h> > > +#include <linux/regulator/consumer.h> > > +#include <linux/slab.h> > > +#include <linux/spi/spi.h> > > +#include <linux/sysfs.h> > > + > > +#include <linux/iio/buffer.h> > > +#include <linux/iio/iio.h> > > +#include <linux/iio/sysfs.h> > > +#include <linux/iio/trigger_consumer.h> > > +#include <linux/iio/triggered_buffer.h> > > + > > > > ... > > > + > > +static int ad7380_debugfs_reg_access(struct iio_dev *indio_dev, u32 reg, > > + u32 writeval, u32 *readval) > > +{ > > + struct ad7380_state *st = iio_priv(indio_dev); > > + int ret; > > + > > + ret = iio_device_claim_direct_mode(indio_dev); > > + if (ret) > > + return ret; > > + > > potential controversial take: do we really need locking in here? regmap already > has it's own lock (I think you're not disabling it) and if someone plays with > registers it shouldn't while buffering, well... This is a debug interface so I > would probably not worry much. One could anyways for write stuff by going > directly to regmap :) > > That said, fine to be as-is from my side (just wanted to take it out of my > system :))... I put it in because we often run `iio_info` which reads register 0 via debugfs. So even though we "shouldn't" be using debug while buffer is enabled, it is easy to do unintentionally. (This was particularly problem while working on offload support.) > > > + if (readval) > > + ret = regmap_read(st->regmap, reg, readval); > > + else > > + ret = regmap_write(st->regmap, reg, writeval); > > + > > + iio_device_release_direct_mode(indio_dev); > > + > > + return ret; > > +} > > + > > +static irqreturn_t ad7380_trigger_handler(int irq, void *p) > > +{ > > + struct iio_poll_func *pf = p; > > + struct iio_dev *indio_dev = pf->indio_dev; > > + struct ad7380_state *st = iio_priv(indio_dev); > > + struct spi_transfer xfer = { > > + .bits_per_word = st->chip_info- > > >channels[0].scan_type.realbits, > > + .len = 4, > > + .rx_buf = &st->scan_data, > > + }; > > + int ret; > > + > > + ret = spi_sync_transfer(st->spi, &xfer, 1); > > + > > + if (ret == 0) > > + iio_push_to_buffers_with_timestamp(indio_dev, &st->scan_data, > > + pf->timestamp); > > + > > + iio_trigger_notify_done(indio_dev->trig); > > + > > + return IRQ_HANDLED; > > +} > > + > > +static int ad7380_read_direct(struct ad7380_state *st, > > + struct iio_chan_spec const *chan, int *val) > > +{ > > + struct spi_transfer xfers[] = { > > + /* toggle CS (no data xfer) to trigger a conversion */ > > + { > > + .speed_hz = AD7380_REG_WR_SPEED_HZ, > > + .bits_per_word = chan->scan_type.realbits, > > + .delay = { > > + .value = 190, /* t[CONVERT] */ > > + .unit = SPI_DELAY_UNIT_NSECS, > > + }, > > + .cs_change = 1, > > + .cs_change_delay = { > > + .value = 10, /* t[CSH] */ > > + .unit = SPI_DELAY_UNIT_NSECS, > > + }, > > + }, > > + /* then read both channels */ > > + { > > + .speed_hz = AD7380_REG_WR_SPEED_HZ, > > + .bits_per_word = chan->scan_type.realbits, > > + .rx_buf = &st->rx[0], > > + .len = 4, > > + }, > > + }; > > + int ret; > > + > > + ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers)); > > + > > + if (ret < 0) > > + return ret; > > + > > + *val = sign_extend32(st->rx[chan->scan_index], > > + chan->scan_type.realbits - 1); > > + > > + return IIO_VAL_INT; > > +} > > + > > +static int ad7380_read_raw(struct iio_dev *indio_dev, > > + struct iio_chan_spec const *chan, > > + int *val, int *val2, long info) > > +{ > > + struct ad7380_state *st = iio_priv(indio_dev); > > + int ret; > > + > > + switch (info) { > > + case IIO_CHAN_INFO_RAW: > > + ret = iio_device_claim_direct_mode(indio_dev); > > + if (ret) > > + return ret; > > + > > + ret = ad7380_read_direct(st, chan, val); > > + iio_device_release_direct_mode(indio_dev); > > + > > + return ret; > > + case IIO_CHAN_INFO_SCALE: > > + if (st->vref) { > > + ret = regulator_get_voltage(st->vref); > > nit: I wonder how likely it is for vref to change at runtime... > > > + if (ret < 0) > > + return ret; > > + > > + *val = ret / 1000; > > + } else { > > + *val = AD7380_INTERNAL_REF_MV; > > + } > > + > > + *val2 = chan->scan_type.realbits; > > + > > + return IIO_VAL_FRACTIONAL_LOG2; > > + } > > + > > + return -EINVAL; > > +} > > + > > +static const struct iio_info ad7380_info = { > > + .read_raw = &ad7380_read_raw, > > + .debugfs_reg_access = &ad7380_debugfs_reg_access, > > +}; > > + > > +static int ad7380_init(struct ad7380_state *st) > > +{ > > + int ret; > > + > > + /* perform hard reset */ > > + ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG2, > > + AD7380_CONFIG2_RESET, > > + FIELD_PREP(AD7380_CONFIG2_RESET, > > + AD7380_CONFIG2_RESET_HARD)); > > + if (ret < 0) > > + return ret; > > + > > + > > + /* select internal or external reference voltage */ > > + ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG1, > > + AD7380_CONFIG1_REFSEL, > > + FIELD_PREP(AD7380_CONFIG1_REFSEL, !!st- > > >vref)); > > + if (ret < 0) > > + return ret; > > + > > + /* SPI 1-wire mode */ > > + return regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG2, > > + AD7380_CONFIG2_SDO, > > + FIELD_PREP(AD7380_CONFIG2_SDO, 1)); > > +} > > + > > +static void ad7380_release_regulator(void *p) > > +{ > > + struct regulator *reg = p; > > + > > + regulator_disable(reg); > > +} > > + > > +static int ad7380_probe(struct spi_device *spi) > > +{ > > + struct iio_dev *indio_dev; > > + struct ad7380_state *st; > > + const char *str_val; > > + int ret; > > + > > + ret = device_property_read_string(&spi->dev, "adi,sdo-mode", > > &str_val); > > + if (ret < 0) > > + return dev_err_probe(&spi->dev, ret, > > + "Failed to read adi,sdo-mode > > property\n"); > > + > > + if (strcmp(str_val, "1-wire")) > > + return dev_err_probe(&spi->dev, -EINVAL, > > + "Only 1-wire SDO is supported\n"); > > + > > + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); > > + if (!indio_dev) > > + return -ENOMEM; > > + > > + st = iio_priv(indio_dev); > > + st->spi = spi; > > + st->chip_info = spi_get_device_match_data(spi); > > + > > if (!st->chip_info) ? It looks like quite a few drivers don't check this, but I guess safer to check anyway. > > > + st->vref = devm_regulator_get_optional(&spi->dev, "refio"); > > + if (IS_ERR(st->vref)) { > > + /* > > + * If there is no REFIO supply, then it means that we are > > using > > + * the internal 2.5V reference. > > + */ > > + if (PTR_ERR(st->vref) == -ENODEV) > > + st->vref = NULL; > > + else > > + return dev_err_probe(&spi->dev, PTR_ERR(st->vref), > > + "Failed to get refio > > regulator\n"); > > + } > > + > > + if (st->vref) { > > + ret = regulator_enable(st->vref); > > + if (ret) > > + return ret; > > + > > + ret = devm_add_action_or_reset(&spi->dev, > > ad7380_release_regulator, > > + st->vref); > > + if (ret) > > + return ret; > > + } > > + > > + st->regmap = devm_regmap_init(&spi->dev, NULL, st, > > &ad7380_regmap_config); > > + if (IS_ERR(st->regmap)) > > + return dev_err_probe(&spi->dev, PTR_ERR(st->regmap), > > + "failed to allocate register map\n"); > > + > > Could we instead have a custom regmap_bus instead of bypass regmap with > reg_read() and reg_write()? > > + > > + indio_dev->channels = st->chip_info->channels; > > + indio_dev->num_channels = st->chip_info->num_channels; > > + indio_dev->dev.parent = &spi->dev; > > no need to assign parent (the core does it for you)... > > - Nuno Sá >
On Tue, 2023-12-12 at 16:42 +0100, David Lechner wrote: > On Tue, Dec 12, 2023 at 4:16 PM Nuno Sá <noname.nuno@gmail.com> wrote: > > > > Hi David, > > > > minor stuff from me... > > > > On Fri, 2023-12-08 at 09:51 -0600, David Lechner wrote: > > > This adds a new driver for the AD7380 family ADCs. > > > > > > The driver currently implements basic support for the AD7380, AD7381, > > > AD7383, and AD7384 2-channel differential ADCs. Support for additional > > > single-ended and 4-channel chips that use the same register map as well > > > as additional features of the chip will be added in future patches. > > > > > > Co-developed-by: Stefan Popa <stefan.popa@analog.com> > > > Signed-off-by: Stefan Popa <stefan.popa@analog.com> > > > Signed-off-by: David Lechner <dlechner@baylibre.com> > > > --- > > > MAINTAINERS | 1 + > > > drivers/iio/adc/Kconfig | 16 ++ > > > drivers/iio/adc/Makefile | 1 + > > > drivers/iio/adc/ad7380.c | 467 > > > +++++++++++++++++++++++++++++++++++++++++++++++ > > > 4 files changed, 485 insertions(+) > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index e2a998be5879..5a54620a31b8 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -438,6 +438,7 @@ S: Supported > > > W: > > > https://wiki.analog.com/resources/tools-software/linux-drivers/iio-adc/ad738x > > > W: https://ez.analog.com/linux-software-drivers > > > F: Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml > > > +F: drivers/iio/adc/ad7380.c > > > > > > AD7877 TOUCHSCREEN DRIVER > > > M: Michael Hennerich <michael.hennerich@analog.com> > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > > index 35f9867da12c..cbfd626712e3 100644 > > > --- a/drivers/iio/adc/Kconfig > > > +++ b/drivers/iio/adc/Kconfig > > > @@ -122,6 +122,22 @@ config AD7298 > > > To compile this driver as a module, choose M here: the > > > module will be called ad7298. > > > > > > +config AD7380 > > > + tristate "Analog Devices AD7380 ADC driver" > > > + depends on SPI_MASTER > > > + select IIO_BUFFER > > > + select IIO_TRIGGER > > > + select IIO_TRIGGERED_BUFFER > > > + help > > > + AD7380 is a family of simultaneous sampling ADCs that share the > > > same > > > + SPI register map and have similar pinouts. > > > + > > > + Say yes here to build support for Analog Devices AD7380 ADC and > > > + similar chips. > > > + > > > + To compile this driver as a module, choose M here: the module will > > > be > > > + called ad7380. > > > + > > > config AD7476 > > > tristate "Analog Devices AD7476 1-channel ADCs driver and other > > > similar devices from AD and TI" > > > depends on SPI > > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > > > index bee11d442af4..e046d8004f41 100644 > > > --- a/drivers/iio/adc/Makefile > > > +++ b/drivers/iio/adc/Makefile > > > @@ -16,6 +16,7 @@ obj-$(CONFIG_AD7291) += ad7291.o > > > obj-$(CONFIG_AD7292) += ad7292.o > > > obj-$(CONFIG_AD7298) += ad7298.o > > > obj-$(CONFIG_AD7923) += ad7923.o > > > +obj-$(CONFIG_AD7476) += ad7380.o > > > obj-$(CONFIG_AD7476) += ad7476.o > > > obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o > > > obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o > > > diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c > > > new file mode 100644 > > > index 000000000000..6a5ec59bd1fd > > > --- /dev/null > > > +++ b/drivers/iio/adc/ad7380.c > > > @@ -0,0 +1,467 @@ > > > +// SPDX-License-Identifier: GPL-2.0-only > > > +/* > > > + * Analog Devices AD738x Simultaneous Sampling SAR ADCs > > > + * > > > + * Copyright 2017 Analog Devices Inc. > > > + * Copyright 2023 BayLibre, SAS > > > + */ > > > + > > > +#include <linux/bitfield.h> > > > +#include <linux/bitops.h> > > > +#include <linux/cleanup.h> > > > +#include <linux/device.h> > > > +#include <linux/err.h> > > > +#include <linux/kernel.h> > > > +#include <linux/module.h> > > > +#include <linux/regmap.h> > > > +#include <linux/regulator/consumer.h> > > > +#include <linux/slab.h> > > > +#include <linux/spi/spi.h> > > > +#include <linux/sysfs.h> > > > + > > > +#include <linux/iio/buffer.h> > > > +#include <linux/iio/iio.h> > > > +#include <linux/iio/sysfs.h> > > > +#include <linux/iio/trigger_consumer.h> > > > +#include <linux/iio/triggered_buffer.h> > > > + > > > > > > > ... > > > > > + > > > +static int ad7380_debugfs_reg_access(struct iio_dev *indio_dev, u32 reg, > > > + u32 writeval, u32 *readval) > > > +{ > > > + struct ad7380_state *st = iio_priv(indio_dev); > > > + int ret; > > > + > > > + ret = iio_device_claim_direct_mode(indio_dev); > > > + if (ret) > > > + return ret; > > > + > > > > potential controversial take: do we really need locking in here? regmap already > > has it's own lock (I think you're not disabling it) and if someone plays with > > registers it shouldn't while buffering, well... This is a debug interface so I > > would probably not worry much. One could anyways for write stuff by going > > directly to regmap :) > > > > That said, fine to be as-is from my side (just wanted to take it out of my > > system :))... > > I put it in because we often run `iio_info` which reads register 0 via > debugfs. So even though we "shouldn't" be using debug while buffer is > enabled, it is easy to do unintentionally. (This was particularly > problem while working on offload support.) > fair enough... > > > > > + if (readval) > > > + ret = regmap_read(st->regmap, reg, readval); > > > + else > > > + ret = regmap_write(st->regmap, reg, writeval); > > > + > > > + iio_device_release_direct_mode(indio_dev); > > > + > > > + return ret; > > > +} > > > + > > > +static irqreturn_t ad7380_trigger_handler(int irq, void *p) > > > +{ > > > + struct iio_poll_func *pf = p; > > > + struct iio_dev *indio_dev = pf->indio_dev; > > > + struct ad7380_state *st = iio_priv(indio_dev); > > > + struct spi_transfer xfer = { > > > + .bits_per_word = st->chip_info- > > > > channels[0].scan_type.realbits, > > > + .len = 4, > > > + .rx_buf = &st->scan_data, > > > + }; > > > + int ret; > > > + > > > + ret = spi_sync_transfer(st->spi, &xfer, 1); > > > + > > > + if (ret == 0) > > > + iio_push_to_buffers_with_timestamp(indio_dev, &st->scan_data, > > > + pf->timestamp); > > > + > > > + iio_trigger_notify_done(indio_dev->trig); > > > + > > > + return IRQ_HANDLED; > > > +} > > > + > > > +static int ad7380_read_direct(struct ad7380_state *st, > > > + struct iio_chan_spec const *chan, int *val) > > > +{ > > > + struct spi_transfer xfers[] = { > > > + /* toggle CS (no data xfer) to trigger a conversion */ > > > + { > > > + .speed_hz = AD7380_REG_WR_SPEED_HZ, > > > + .bits_per_word = chan->scan_type.realbits, > > > + .delay = { > > > + .value = 190, /* t[CONVERT] */ > > > + .unit = SPI_DELAY_UNIT_NSECS, > > > + }, > > > + .cs_change = 1, > > > + .cs_change_delay = { > > > + .value = 10, /* t[CSH] */ > > > + .unit = SPI_DELAY_UNIT_NSECS, > > > + }, > > > + }, > > > + /* then read both channels */ > > > + { > > > + .speed_hz = AD7380_REG_WR_SPEED_HZ, > > > + .bits_per_word = chan->scan_type.realbits, > > > + .rx_buf = &st->rx[0], > > > + .len = 4, > > > + }, > > > + }; > > > + int ret; > > > + > > > + ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers)); > > > + > > > + if (ret < 0) > > > + return ret; > > > + > > > + *val = sign_extend32(st->rx[chan->scan_index], > > > + chan->scan_type.realbits - 1); > > > + > > > + return IIO_VAL_INT; > > > +} > > > + > > > +static int ad7380_read_raw(struct iio_dev *indio_dev, > > > + struct iio_chan_spec const *chan, > > > + int *val, int *val2, long info) > > > +{ > > > + struct ad7380_state *st = iio_priv(indio_dev); > > > + int ret; > > > + > > > + switch (info) { > > > + case IIO_CHAN_INFO_RAW: > > > + ret = iio_device_claim_direct_mode(indio_dev); > > > + if (ret) > > > + return ret; > > > + > > > + ret = ad7380_read_direct(st, chan, val); > > > + iio_device_release_direct_mode(indio_dev); > > > + > > > + return ret; > > > + case IIO_CHAN_INFO_SCALE: > > > + if (st->vref) { > > > + ret = regulator_get_voltage(st->vref); > > > > nit: I wonder how likely it is for vref to change at runtime... > > > > > + if (ret < 0) > > > + return ret; > > > + > > > + *val = ret / 1000; > > > + } else { > > > + *val = AD7380_INTERNAL_REF_MV; > > > + } > > > + > > > + *val2 = chan->scan_type.realbits; > > > + > > > + return IIO_VAL_FRACTIONAL_LOG2; > > > + } > > > + > > > + return -EINVAL; > > > +} > > > + > > > +static const struct iio_info ad7380_info = { > > > + .read_raw = &ad7380_read_raw, > > > + .debugfs_reg_access = &ad7380_debugfs_reg_access, > > > +}; > > > + > > > +static int ad7380_init(struct ad7380_state *st) > > > +{ > > > + int ret; > > > + > > > + /* perform hard reset */ > > > + ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG2, > > > + AD7380_CONFIG2_RESET, > > > + FIELD_PREP(AD7380_CONFIG2_RESET, > > > + AD7380_CONFIG2_RESET_HARD)); > > > + if (ret < 0) > > > + return ret; > > > + > > > + > > > + /* select internal or external reference voltage */ > > > + ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG1, > > > + AD7380_CONFIG1_REFSEL, > > > + FIELD_PREP(AD7380_CONFIG1_REFSEL, !!st- > > > > vref)); > > > + if (ret < 0) > > > + return ret; > > > + > > > + /* SPI 1-wire mode */ > > > + return regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG2, > > > + AD7380_CONFIG2_SDO, > > > + FIELD_PREP(AD7380_CONFIG2_SDO, 1)); > > > +} > > > + > > > +static void ad7380_release_regulator(void *p) > > > +{ > > > + struct regulator *reg = p; > > > + > > > + regulator_disable(reg); > > > +} > > > + > > > +static int ad7380_probe(struct spi_device *spi) > > > +{ > > > + struct iio_dev *indio_dev; > > > + struct ad7380_state *st; > > > + const char *str_val; > > > + int ret; > > > + > > > + ret = device_property_read_string(&spi->dev, "adi,sdo-mode", > > > &str_val); > > > + if (ret < 0) > > > + return dev_err_probe(&spi->dev, ret, > > > + "Failed to read adi,sdo-mode > > > property\n"); > > > + > > > + if (strcmp(str_val, "1-wire")) > > > + return dev_err_probe(&spi->dev, -EINVAL, > > > + "Only 1-wire SDO is supported\n"); > > > + > > > + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); > > > + if (!indio_dev) > > > + return -ENOMEM; > > > + > > > + st = iio_priv(indio_dev); > > > + st->spi = spi; > > > + st->chip_info = spi_get_device_match_data(spi); > > > + > > > > if (!st->chip_info) ? > > It looks like quite a few drivers don't check this, but I guess safer > to check anyway. > Don't make it right :). I mean, FWIW, I'm not quite sure if you can really get NULL in here but since it's a possibility (from the API point of view), better to do it properly. - Nuno Sá
diff --git a/MAINTAINERS b/MAINTAINERS index e2a998be5879..5a54620a31b8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -438,6 +438,7 @@ S: Supported W: https://wiki.analog.com/resources/tools-software/linux-drivers/iio-adc/ad738x W: https://ez.analog.com/linux-software-drivers F: Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml +F: drivers/iio/adc/ad7380.c AD7877 TOUCHSCREEN DRIVER M: Michael Hennerich <michael.hennerich@analog.com> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index 35f9867da12c..cbfd626712e3 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -122,6 +122,22 @@ config AD7298 To compile this driver as a module, choose M here: the module will be called ad7298. +config AD7380 + tristate "Analog Devices AD7380 ADC driver" + depends on SPI_MASTER + select IIO_BUFFER + select IIO_TRIGGER + select IIO_TRIGGERED_BUFFER + help + AD7380 is a family of simultaneous sampling ADCs that share the same + SPI register map and have similar pinouts. + + Say yes here to build support for Analog Devices AD7380 ADC and + similar chips. + + To compile this driver as a module, choose M here: the module will be + called ad7380. + config AD7476 tristate "Analog Devices AD7476 1-channel ADCs driver and other similar devices from AD and TI" depends on SPI diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile index bee11d442af4..e046d8004f41 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -16,6 +16,7 @@ obj-$(CONFIG_AD7291) += ad7291.o obj-$(CONFIG_AD7292) += ad7292.o obj-$(CONFIG_AD7298) += ad7298.o obj-$(CONFIG_AD7923) += ad7923.o +obj-$(CONFIG_AD7476) += ad7380.o obj-$(CONFIG_AD7476) += ad7476.o obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c new file mode 100644 index 000000000000..6a5ec59bd1fd --- /dev/null +++ b/drivers/iio/adc/ad7380.c @@ -0,0 +1,467 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Analog Devices AD738x Simultaneous Sampling SAR ADCs + * + * Copyright 2017 Analog Devices Inc. + * Copyright 2023 BayLibre, SAS + */ + +#include <linux/bitfield.h> +#include <linux/bitops.h> +#include <linux/cleanup.h> +#include <linux/device.h> +#include <linux/err.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/regmap.h> +#include <linux/regulator/consumer.h> +#include <linux/slab.h> +#include <linux/spi/spi.h> +#include <linux/sysfs.h> + +#include <linux/iio/buffer.h> +#include <linux/iio/iio.h> +#include <linux/iio/sysfs.h> +#include <linux/iio/trigger_consumer.h> +#include <linux/iio/triggered_buffer.h> + +/* 2.5V internal reference voltage */ +#define AD7380_INTERNAL_REF_MV 2500 + +/* reading and writing registers is more reliable at lower than max speed */ +#define AD7380_REG_WR_SPEED_HZ 10000000 + +#define AD7380_REG_WR BIT(15) +#define AD7380_REG_REGADDR GENMASK(14, 12) +#define AD7380_REG_DATA GENMASK(11, 0) + +#define AD7380_REG_ADDR_NOP 0x0 +#define AD7380_REG_ADDR_CONFIG1 0x1 +#define AD7380_REG_ADDR_CONFIG2 0x2 +#define AD7380_REG_ADDR_ALERT 0x3 +#define AD7380_REG_ADDR_ALERT_LOW_TH 0x4 +#define AD7380_REG_ADDR_ALERT_HIGH_TH 0x5 + +#define AD7380_CONFIG1_OS_MODE BIT(9) +#define AD7380_CONFIG1_OSR GENMASK(8, 6) +#define AD7380_CONFIG1_CRC_W BIT(5) +#define AD7380_CONFIG1_CRC_R BIT(4) +#define AD7380_CONFIG1_ALERTEN BIT(3) +#define AD7380_CONFIG1_RES BIT(2) +#define AD7380_CONFIG1_REFSEL BIT(1) +#define AD7380_CONFIG1_PMODE BIT(0) + +#define AD7380_CONFIG2_SDO2 GENMASK(9, 8) +#define AD7380_CONFIG2_SDO BIT(8) +#define AD7380_CONFIG2_RESET GENMASK(7, 0) + +#define AD7380_CONFIG2_RESET_SOFT 0x3C +#define AD7380_CONFIG2_RESET_HARD 0xFF + +#define AD7380_ALERT_LOW_TH GENMASK(11, 0) +#define AD7380_ALERT_HIGH_TH GENMASK(11, 0) + +struct ad7380_chip_info { + const char *name; + const struct iio_chan_spec *channels; + unsigned int num_channels; +}; + +#define AD7380_DIFFERENTIAL_CHANNEL(index, bits) { \ + .type = IIO_VOLTAGE, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ + .indexed = 1, \ + .differential = 1, \ + .channel = 2 * (index), \ + .channel2 = 2 * (index) + 1, \ + .scan_index = (index), \ + .scan_type = { \ + .sign = 's', \ + .realbits = (bits), \ + .storagebits = 16, \ + .endianness = IIO_CPU, \ + }, \ +} + +#define DEFINE_AD7380_DIFFERENTIAL_2_CHANNEL(name, bits) \ +static const struct iio_chan_spec name[] = { \ + AD7380_DIFFERENTIAL_CHANNEL(0, bits), \ + AD7380_DIFFERENTIAL_CHANNEL(1, bits), \ + IIO_CHAN_SOFT_TIMESTAMP(2), \ +} + +/* fully differential */ +DEFINE_AD7380_DIFFERENTIAL_2_CHANNEL(ad7380_channels, 16); +DEFINE_AD7380_DIFFERENTIAL_2_CHANNEL(ad7381_channels, 14); +/* pseudo differential */ +DEFINE_AD7380_DIFFERENTIAL_2_CHANNEL(ad7383_channels, 16); +DEFINE_AD7380_DIFFERENTIAL_2_CHANNEL(ad7384_channels, 14); + +static const struct ad7380_chip_info ad7380_chip_info = { + .name = "ad7380", + .channels = ad7380_channels, + .num_channels = ARRAY_SIZE(ad7380_channels), +}; + +static const struct ad7380_chip_info ad7381_chip_info = { + .name = "ad7381", + .channels = ad7381_channels, + .num_channels = ARRAY_SIZE(ad7381_channels), +}; + +static const struct ad7380_chip_info ad7383_chip_info = { + .name = "ad7383", + .channels = ad7383_channels, + .num_channels = ARRAY_SIZE(ad7383_channels), +}; + +static const struct ad7380_chip_info ad7384_chip_info = { + .name = "ad7384", + .channels = ad7384_channels, + .num_channels = ARRAY_SIZE(ad7384_channels), +}; + +struct ad7380_state { + const struct ad7380_chip_info *chip_info; + struct spi_device *spi; + struct regulator *vref; + struct regmap *regmap; + /* + * DMA (thus cache coherency maintenance) requires the + * transfer buffers to live in their own cache lines. + * Make the buffer large enough for 2 16-bit samples and one 64-bit + * aligned 64 bit timestamp. + */ + struct { + u16 raw[2]; + s64 ts __aligned(8); + } scan_data __aligned(IIO_DMA_MINALIGN); + u16 tx[2]; + u16 rx[2]; +}; + +static int ad7380_regmap_reg_write(void *context, unsigned int reg, + unsigned int val) +{ + struct ad7380_state *st = context; + struct spi_transfer xfer = { + .speed_hz = AD7380_REG_WR_SPEED_HZ, + .bits_per_word = 16, + .len = 2, + .tx_buf = &st->tx[0], + }; + + st->tx[0] = FIELD_PREP(AD7380_REG_WR, 1) | + FIELD_PREP(AD7380_REG_REGADDR, reg) | + FIELD_PREP(AD7380_REG_DATA, val); + + return spi_sync_transfer(st->spi, &xfer, 1); +} + +static int ad7380_regmap_reg_read(void *context, unsigned int reg, + unsigned int *val) +{ + struct ad7380_state *st = context; + struct spi_transfer xfers[] = { + { + .speed_hz = AD7380_REG_WR_SPEED_HZ, + .bits_per_word = 16, + .len = 2, + .tx_buf = &st->tx[0], + .cs_change = 1, + .cs_change_delay = { + .value = 10, /* t[CSH] */ + .unit = SPI_DELAY_UNIT_NSECS, + }, + }, { + .speed_hz = AD7380_REG_WR_SPEED_HZ, + .bits_per_word = 16, + .len = 2, + .rx_buf = &st->rx[0], + }, + }; + int ret; + + st->tx[0] = FIELD_PREP(AD7380_REG_WR, 0) | + FIELD_PREP(AD7380_REG_REGADDR, reg) | + FIELD_PREP(AD7380_REG_DATA, 0); + + ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers)); + if (ret < 0) + return ret; + + *val = FIELD_GET(AD7380_REG_DATA, st->rx[0]); + + return 0; +} + +const struct regmap_config ad7380_regmap_config = { + .reg_bits = 3, + .val_bits = 12, + .reg_read = ad7380_regmap_reg_read, + .reg_write = ad7380_regmap_reg_write, + .max_register = AD7380_REG_ADDR_ALERT_HIGH_TH, + .can_sleep = true, +}; + +static int ad7380_debugfs_reg_access(struct iio_dev *indio_dev, u32 reg, + u32 writeval, u32 *readval) +{ + struct ad7380_state *st = iio_priv(indio_dev); + int ret; + + ret = iio_device_claim_direct_mode(indio_dev); + if (ret) + return ret; + + if (readval) + ret = regmap_read(st->regmap, reg, readval); + else + ret = regmap_write(st->regmap, reg, writeval); + + iio_device_release_direct_mode(indio_dev); + + return ret; +} + +static irqreturn_t ad7380_trigger_handler(int irq, void *p) +{ + struct iio_poll_func *pf = p; + struct iio_dev *indio_dev = pf->indio_dev; + struct ad7380_state *st = iio_priv(indio_dev); + struct spi_transfer xfer = { + .bits_per_word = st->chip_info->channels[0].scan_type.realbits, + .len = 4, + .rx_buf = &st->scan_data, + }; + int ret; + + ret = spi_sync_transfer(st->spi, &xfer, 1); + + if (ret == 0) + iio_push_to_buffers_with_timestamp(indio_dev, &st->scan_data, + pf->timestamp); + + iio_trigger_notify_done(indio_dev->trig); + + return IRQ_HANDLED; +} + +static int ad7380_read_direct(struct ad7380_state *st, + struct iio_chan_spec const *chan, int *val) +{ + struct spi_transfer xfers[] = { + /* toggle CS (no data xfer) to trigger a conversion */ + { + .speed_hz = AD7380_REG_WR_SPEED_HZ, + .bits_per_word = chan->scan_type.realbits, + .delay = { + .value = 190, /* t[CONVERT] */ + .unit = SPI_DELAY_UNIT_NSECS, + }, + .cs_change = 1, + .cs_change_delay = { + .value = 10, /* t[CSH] */ + .unit = SPI_DELAY_UNIT_NSECS, + }, + }, + /* then read both channels */ + { + .speed_hz = AD7380_REG_WR_SPEED_HZ, + .bits_per_word = chan->scan_type.realbits, + .rx_buf = &st->rx[0], + .len = 4, + }, + }; + int ret; + + ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers)); + + if (ret < 0) + return ret; + + *val = sign_extend32(st->rx[chan->scan_index], + chan->scan_type.realbits - 1); + + return IIO_VAL_INT; +} + +static int ad7380_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val, int *val2, long info) +{ + struct ad7380_state *st = iio_priv(indio_dev); + int ret; + + switch (info) { + case IIO_CHAN_INFO_RAW: + ret = iio_device_claim_direct_mode(indio_dev); + if (ret) + return ret; + + ret = ad7380_read_direct(st, chan, val); + iio_device_release_direct_mode(indio_dev); + + return ret; + case IIO_CHAN_INFO_SCALE: + if (st->vref) { + ret = regulator_get_voltage(st->vref); + if (ret < 0) + return ret; + + *val = ret / 1000; + } else { + *val = AD7380_INTERNAL_REF_MV; + } + + *val2 = chan->scan_type.realbits; + + return IIO_VAL_FRACTIONAL_LOG2; + } + + return -EINVAL; +} + +static const struct iio_info ad7380_info = { + .read_raw = &ad7380_read_raw, + .debugfs_reg_access = &ad7380_debugfs_reg_access, +}; + +static int ad7380_init(struct ad7380_state *st) +{ + int ret; + + /* perform hard reset */ + ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG2, + AD7380_CONFIG2_RESET, + FIELD_PREP(AD7380_CONFIG2_RESET, + AD7380_CONFIG2_RESET_HARD)); + if (ret < 0) + return ret; + + + /* select internal or external reference voltage */ + ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG1, + AD7380_CONFIG1_REFSEL, + FIELD_PREP(AD7380_CONFIG1_REFSEL, !!st->vref)); + if (ret < 0) + return ret; + + /* SPI 1-wire mode */ + return regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG2, + AD7380_CONFIG2_SDO, + FIELD_PREP(AD7380_CONFIG2_SDO, 1)); +} + +static void ad7380_release_regulator(void *p) +{ + struct regulator *reg = p; + + regulator_disable(reg); +} + +static int ad7380_probe(struct spi_device *spi) +{ + struct iio_dev *indio_dev; + struct ad7380_state *st; + const char *str_val; + int ret; + + ret = device_property_read_string(&spi->dev, "adi,sdo-mode", &str_val); + if (ret < 0) + return dev_err_probe(&spi->dev, ret, + "Failed to read adi,sdo-mode property\n"); + + if (strcmp(str_val, "1-wire")) + return dev_err_probe(&spi->dev, -EINVAL, + "Only 1-wire SDO is supported\n"); + + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); + if (!indio_dev) + return -ENOMEM; + + st = iio_priv(indio_dev); + st->spi = spi; + st->chip_info = spi_get_device_match_data(spi); + + st->vref = devm_regulator_get_optional(&spi->dev, "refio"); + if (IS_ERR(st->vref)) { + /* + * If there is no REFIO supply, then it means that we are using + * the internal 2.5V reference. + */ + if (PTR_ERR(st->vref) == -ENODEV) + st->vref = NULL; + else + return dev_err_probe(&spi->dev, PTR_ERR(st->vref), + "Failed to get refio regulator\n"); + } + + if (st->vref) { + ret = regulator_enable(st->vref); + if (ret) + return ret; + + ret = devm_add_action_or_reset(&spi->dev, ad7380_release_regulator, + st->vref); + if (ret) + return ret; + } + + st->regmap = devm_regmap_init(&spi->dev, NULL, st, &ad7380_regmap_config); + if (IS_ERR(st->regmap)) + return dev_err_probe(&spi->dev, PTR_ERR(st->regmap), + "failed to allocate register map\n"); + + + indio_dev->channels = st->chip_info->channels; + indio_dev->num_channels = st->chip_info->num_channels; + indio_dev->dev.parent = &spi->dev; + indio_dev->name = st->chip_info->name; + indio_dev->info = &ad7380_info; + indio_dev->modes = INDIO_DIRECT_MODE; + + ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, + iio_pollfunc_store_time, + ad7380_trigger_handler, NULL); + if (ret) + return ret; + + ret = ad7380_init(st); + if (ret) + return ret; + + return devm_iio_device_register(&spi->dev, indio_dev); +} + +static const struct of_device_id ad7380_of_match_table[] = { + { .compatible = "adi,ad7380", .data = &ad7380_chip_info }, + { .compatible = "adi,ad7381", .data = &ad7381_chip_info }, + { .compatible = "adi,ad7383", .data = &ad7383_chip_info }, + { .compatible = "adi,ad7384", .data = &ad7384_chip_info }, + { } +}; + +static const struct spi_device_id ad7380_id_table[] = { + { "ad7380", (kernel_ulong_t)&ad7380_chip_info }, + { "ad7381", (kernel_ulong_t)&ad7381_chip_info }, + { "ad7383", (kernel_ulong_t)&ad7383_chip_info }, + { "ad7384", (kernel_ulong_t)&ad7384_chip_info }, + { } +}; +MODULE_DEVICE_TABLE(spi, ad7380_id_table); + +static struct spi_driver ad7380_driver = { + .driver = { + .name = "ad7380", + .of_match_table = ad7380_of_match_table, + }, + .probe = ad7380_probe, + .id_table = ad7380_id_table, +}; +module_spi_driver(ad7380_driver); + +MODULE_AUTHOR("Stefan Popa <stefan.popa@analog.com>"); +MODULE_DESCRIPTION("Analog Devices AD738x ADC driver"); +MODULE_LICENSE("GPL");