On Fri, 2025-01-17 at 15:07 +0200, Antoniu Miclaus wrote:
> Add support for the AD485X a fully buffered, 8-channel simultaneous
> sampling, 16/20-bit, 1 MSPS data acquisition system (DAS) with
> differential, wide common-mode range inputs.
>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx>
> ---
> changes in v10:
> - use u64 for scale_tbl cast.
> - use IIO_VAL_FRACTIONAL_LOG2
> - add comment for not using bulk_read
> - use _u and _b suffix for scan_type structures.
> - add comment for not setting scan_type in macro definitions.
> - fix some wrapping.
> - rework vrefbuf_en and vrefio_en handling.
> - add depends on PWM in Kconfig
> - add unipolar/bipolar suffix where requested.
> - rename REFSEL REFBUF macros.
> - use indio_dev->channels[i].channel for iio_backend_chan_enable
> - drop initialization for 'buf'
> - use st->bipolar_ch[chan->channel], not chan->differential
> - use u32 for softspan_val
> - move info_mask_separate above as suggested.
> - handle sign in parse channel function and use `u` as default in macro.
> - check `reg` range.
> - use fwnode_property_read_bool
> - drop redundant `reg` parsing
> - rework channel2 statement in channel macro and parse_channels.
> - set ad4851_channels->has_ext_scan_type = 1
> drivers/iio/adc/Kconfig | 14 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ad4851.c | 1293 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 1308 insertions(+)
> create mode 100644 drivers/iio/adc/ad4851.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 849c90203071..afd83fddda76 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -61,6 +61,20 @@ config AD4695
> To compile this driver as a module, choose M here: the module will be
> called ad4695.
>
> +config AD4851
> + tristate "Analog Device AD4851 DAS Driver"
> + depends on SPI
> + depends on PWM
> + select REGMAP_SPI
> + select IIO_BACKEND
> + help
> + Say yes here to build support for Analog Devices AD4851, AD4852,
> + AD4853, AD4854, AD4855, AD4856, AD4857, AD4858, AD4858I high speed
> + data acquisition system (DAS).
> +
> + To compile this driver as a module, choose M here: the module will be
> + called ad4851.
> +
> config AD7091R
> tristate
>
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index ee19afba62b7..e4d8ba12f841 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
> obj-$(CONFIG_AD4000) += ad4000.o
> obj-$(CONFIG_AD4130) += ad4130.o
> obj-$(CONFIG_AD4695) += ad4695.o
> +obj-$(CONFIG_AD4851) += ad4851.o
> obj-$(CONFIG_AD7091R) += ad7091r-base.o
> obj-$(CONFIG_AD7091R5) += ad7091r5.o
> obj-$(CONFIG_AD7091R8) += ad7091r8.o
> diff --git a/drivers/iio/adc/ad4851.c b/drivers/iio/adc/ad4851.c
> new file mode 100644
> index 000000000000..a39d863bf62d
> --- /dev/null
> +++ b/drivers/iio/adc/ad4851.c
> @@ -0,0 +1,1293 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Analog Devices AD4851 DAS driver
> + *
> + * Copyright 2024 Analog Devices Inc.
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/minmax.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/pwm.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
> +#include <linux/units.h>
> +
...
> +#include <linux/iio/backend.h>
> +#include <linux/iio/iio.h>
> +
> +#define AD4851_REG_INTERFACE_CONFIG_A 0x00
> +#define AD4851_REG_INTERFACE_CONFIG_B 0x01
> +#define AD4851_REG_PRODUCT_ID_L 0x04
> +#define AD4851_REG_PRODUCT_ID_H 0x05
> +#define AD4851_REG_DEVICE_CTRL 0x25
> +#define AD4851_REG_PACKET 0x26
> +#define AD4851_REG_OVERSAMPLE 0x27
> +
> +#define AD4851_REG_CH_CONFIG_BASE 0x2A
> +#define AD4851_REG_CHX_SOFTSPAN(ch) ((0x12 * (ch)) +
> AD4851_REG_CH_CONFIG_BASE)
> +#define AD4851_REG_CHX_OFFSET(ch) (AD4851_REG_CHX_SOFTSPAN(ch) + 0x01)
> +#define AD4851_REG_CHX_OFFSET_LSB(ch) AD4851_REG_CHX_OFFSET(ch)
> +#define AD4851_REG_CHX_OFFSET_MID(ch) (AD4851_REG_CHX_OFFSET_LSB(ch) + 0x01)
> +#define AD4851_REG_CHX_OFFSET_MSB(ch) (AD4851_REG_CHX_OFFSET_MID(ch) + 0x01)
> +#define AD4851_REG_CHX_GAIN(ch) (AD4851_REG_CHX_OFFSET(ch) + 0x03)
> +#define AD4851_REG_CHX_GAIN_LSB(ch) AD4851_REG_CHX_GAIN(ch)
> +#define AD4851_REG_CHX_GAIN_MSB(ch) (AD4851_REG_CHX_GAIN(ch) + 0x01)
> +#define AD4851_REG_CHX_PHASE(ch) (AD4851_REG_CHX_GAIN(ch) + 0x02)
> +#define AD4851_REG_CHX_PHASE_LSB(ch) AD4851_REG_CHX_PHASE(ch)
> +#define AD4851_REG_CHX_PHASE_MSB(ch) (AD4851_REG_CHX_PHASE_LSB(ch) + 0x01)
> +
> +#define AD4851_REG_TESTPAT_0(c) (0x38 + (c) * 0x12)
> +#define AD4851_REG_TESTPAT_1(c) (0x39 + (c) * 0x12)
> +#define AD4851_REG_TESTPAT_2(c) (0x3A + (c) * 0x12)
> +#define AD4851_REG_TESTPAT_3(c) (0x3B + (c) * 0x12)
> +
> +#define AD4851_SW_RESET (BIT(7) | BIT(0))
> +#define AD4851_SDO_ENABLE BIT(4)
> +#define AD4851_SINGLE_INSTRUCTION BIT(7)
> +#define AD4851_REFBUF BIT(2)
> +#define AD4851_REFSEL BIT(1)
> +#define AD4851_ECHO_CLOCK_MODE BIT(0)
> +
> +#define AD4851_PACKET_FORMAT_0 0
> +#define AD4851_PACKET_FORMAT_1 1
> +#define AD4851_PACKET_FORMAT_MASK GENMASK(1, 0)
> +
> +#define AD4851_OS_EN_MSK BIT(7)
> +#define AD4851_OS_RATIO_MSK GENMASK(3, 0)
> +
> +#define AD4851_TEST_PAT BIT(2)
> +
> +#define AD4858_PACKET_SIZE_20 0
> +#define AD4858_PACKET_SIZE_24 1
> +#define AD4858_PACKET_SIZE_32 2
> +
> +#define AD4857_PACKET_SIZE_16 0
> +#define AD4857_PACKET_SIZE_24 1
> +
> +#define AD4851_TESTPAT_0_DEFAULT 0x2A
> +#define AD4851_TESTPAT_1_DEFAULT 0x3C
> +#define AD4851_TESTPAT_2_DEFAULT 0xCE
> +#define AD4851_TESTPAT_3_DEFAULT(c) (0x0A + (0x10 * (c)))
> +
> +#define AD4851_SOFTSPAN_0V_2V5 0
> +#define AD4851_SOFTSPAN_N2V5_2V5 1
> +#define AD4851_SOFTSPAN_0V_5V 2
> +#define AD4851_SOFTSPAN_N5V_5V 3
> +#define AD4851_SOFTSPAN_0V_6V25 4
> +#define AD4851_SOFTSPAN_N6V25_6V25 5
> +#define AD4851_SOFTSPAN_0V_10V 6
> +#define AD4851_SOFTSPAN_N10V_10V 7
> +#define AD4851_SOFTSPAN_0V_12V5 8
> +#define AD4851_SOFTSPAN_N12V5_12V5 9
> +#define AD4851_SOFTSPAN_0V_20V 10
> +#define AD4851_SOFTSPAN_N20V_20V 11
> +#define AD4851_SOFTSPAN_0V_25V 12
> +#define AD4851_SOFTSPAN_N25V_25V 13
> +#define AD4851_SOFTSPAN_0V_40V 14
> +#define AD4851_SOFTSPAN_N40V_40V 15
> +
> +#define AD4851_MAX_LANES 8
> +#define AD4851_MAX_IODELAY 32
> +
> +#define AD4851_T_CNVH_NS 40
> +#define AD4851_T_CNVH_NS_MARGIN 10
> +
> +#define AD4841_MAX_SCALE_AVAIL 8
> +
> +#define AD4851_MAX_CH_NR 8
> +#define AD4851_CH_START 0
> +
> +struct ad4851_scale {
> + unsigned int scale_val;
> + u8 reg_val;
> +};
> +
> +static const struct ad4851_scale ad4851_scale_table_unipolar[] = {
> + { 2500, 0x0 },
> + { 5000, 0x2 },
> + { 6250, 0x4 },
> + { 10000, 0x6 },
> + { 12500, 0x8 },
> + { 20000, 0xA },
> + { 25000, 0xC },
> + { 40000, 0xE },
> +};
> +
> +static const struct ad4851_scale ad4851_scale_table_bipolar[] = {
> + { 5000, 0x1 },
> + { 10000, 0x3 },
> + { 12500, 0x5 },
> + { 20000, 0x7 },
> + { 25000, 0x9 },
> + { 40000, 0xB },
> + { 50000, 0xD },
> + { 80000, 0xF },
> +};
> +
> +static const unsigned int ad4851_scale_avail_unipolar[] = {
> + 2500,
> + 5000,
> + 6250,
> + 10000,
> + 12500,
> + 20000,
> + 25000,
> + 40000,
> +};
> +
> +static const unsigned int ad4851_scale_avail_bipolar[] = {
> + 5000,
> + 10000,
> + 12500,
> + 20000,
> + 25000,
> + 40000,
> + 50000,
> + 80000,
> +};
> +
> +struct ad4851_chip_info {
> + const char *name;
> + unsigned int product_id;
> + int num_scales;
> + unsigned long max_sample_rate_hz;
> + unsigned int resolution;
> + int (*parse_channels)(struct iio_dev *indio_dev);
> +};
> +
> +enum {
> + AD4851_SCAN_TYPE_NORMAL,
> + AD4851_SCAN_TYPE_RESOLUTION_BOOST,
> +};
> +
> +struct ad4851_state {
> + struct spi_device *spi;
> + struct pwm_device *cnv;
> + struct iio_backend *back;
> + /*
> + * Synchronize access to members the of driver state, and ensure
> + * atomicity of consecutive regmap operations.
> + */
> + struct mutex lock;
> + struct regmap *regmap;
> + const struct ad4851_chip_info *info;
> + struct gpio_desc *pd_gpio;
> + bool resolution_boost_enabled;
> + unsigned long cnv_trigger_rate_hz;
> + unsigned int osr;
> + bool vrefbuf_en;
> + bool vrefio_en;
> + bool bipolar_ch[AD4851_MAX_CH_NR];
> + unsigned int scales_unipolar[AD4841_MAX_SCALE_AVAIL][2];
> + unsigned int scales_bipolar[AD4841_MAX_SCALE_AVAIL][2];
> +};
> +
> +static int ad4851_reg_access(struct iio_dev *indio_dev,
> + unsigned int reg,
> + unsigned int writeval,
> + unsigned int *readval)
> +{
> + struct ad4851_state *st = iio_priv(indio_dev);
> +
> + guard(mutex)(&st->lock);
> +
no need for the above lock. I guess you're not disabling regmap internal lock...
> + if (readval)
> + return regmap_read(st->regmap, reg, readval);
> +
> + return regmap_write(st->regmap, reg, writeval);
> +}
> +
...
>
> +static int ad4851_set_oversampling_ratio(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + unsigned int osr)
> +{
> + struct ad4851_state *st = iio_priv(indio_dev);
> + int val, ret;
> +
> + guard(mutex)(&st->lock);
> +
> + if (osr == 1) {
> + ret = regmap_clear_bits(st->regmap, AD4851_REG_OVERSAMPLE,
> + AD4851_OS_EN_MSK);
> + if (ret)
> + return ret;
> + } else {
> + val = ad4851_osr_to_regval(osr);
> + if (val < 0)
> + return -EINVAL;
> +
> + ret = regmap_update_bits(st->regmap, AD4851_REG_OVERSAMPLE,
> + AD4851_OS_EN_MSK |
> + AD4851_OS_RATIO_MSK,
> + FIELD_PREP(AD4851_OS_EN_MSK, 1) |
> + FIELD_PREP(AD4851_OS_RATIO_MSK, val));
> + if (ret)
> + return ret;
> + }
> +
> + ret = iio_backend_oversampling_ratio_set(st->back, osr);
> + if (ret)
> + return ret;
> +
> + switch (st->info->resolution) {
> + case 20:
> + switch (osr) {
> + case 0:
> + return -EINVAL;
> + case 1:
> + val = 20;
> + break;
> + default:
> + val = 24;
> + break;
> + }
> + break;
> + case 16:
> + val = 16;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + ret = iio_backend_data_size_set(st->back, val);
> + if (ret)
> + return ret;
> +
> + if (osr == 1 || st->info->resolution == 16) {
> + ret = regmap_clear_bits(st->regmap, AD4851_REG_PACKET,
> + AD4851_PACKET_FORMAT_MASK);
> + if (ret)
> + return ret;
> +
> + st->resolution_boost_enabled = false;
> + } else {
> + ret = regmap_update_bits(st->regmap, AD4851_REG_PACKET,
> + AD4851_PACKET_FORMAT_MASK,
> + FIELD_PREP(AD4851_PACKET_FORMAT_MASK,
> 1));
> + if (ret)
> + return ret;
> +
> + st->resolution_boost_enabled = true;
> + }
> +
> + if (st->osr != osr) {
> + ret = ad4851_scale_fill(indio_dev);
> + if (ret)
> + return ret;
> +
> + st->osr = osr;
> + }
nit: I would instead do
if (st->osr == osr)
return 0;
...
> +
> + return 0;
> +}
> +
> +static int ad4851_get_oversampling_ratio(struct ad4851_state *st, unsigned int
> *val)
> +{
> + unsigned int osr;
> + int ret;
> +
> + guard(mutex)(&st->lock);
> +
> + ret = regmap_read(st->regmap, AD4851_REG_OVERSAMPLE, &osr);
> + if (ret)
> + return ret;
> +
> + if (!FIELD_GET(AD4851_OS_EN_MSK, osr))
> + *val = 1;
> + else
> + *val = ad4851_oversampling_ratios[FIELD_GET(AD4851_OS_RATIO_MSK,
> osr) + 1];
> +
> + st->osr = *val;
> +
> + return IIO_VAL_INT;
> +}
> +
> +static void ad4851_pwm_disable(void *data)
> +{
> + pwm_disable(data);
> +}
> +
> +static int ad4851_setup(struct ad4851_state *st)
> +{
> + unsigned int product_id;
> + int ret;
> +
> + if (st->pd_gpio) {
> + /* To initiate a global reset, bring the PD pin high twice */
> + gpiod_set_value(st->pd_gpio, 1);
> + fsleep(1);
> + gpiod_set_value(st->pd_gpio, 0);
> + fsleep(1);
> + gpiod_set_value(st->pd_gpio, 1);
> + fsleep(1);
> + gpiod_set_value(st->pd_gpio, 0);
> + fsleep(1000);
> + } else {
> + ret = regmap_set_bits(st->regmap, AD4851_REG_INTERFACE_CONFIG_A,
> + AD4851_SW_RESET);
> + if (ret)
> + return ret;
> + }
> +
> + if (st->vrefbuf_en) {
> + ret = regmap_set_bits(st->regmap, AD4851_REG_DEVICE_CTRL,
> + AD4851_REFBUF);
> + if (ret)
> + return ret;
> + }
> +
> + if (st->vrefio_en) {
> + ret = regmap_set_bits(st->regmap, AD4851_REG_DEVICE_CTRL,
> + AD4851_REFSEL);
> + if (ret)
> + return ret;
> + }
> +
> + ret = regmap_write(st->regmap, AD4851_REG_INTERFACE_CONFIG_B,
> + AD4851_SINGLE_INSTRUCTION);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(st->regmap, AD4851_REG_INTERFACE_CONFIG_A,
> + AD4851_SDO_ENABLE);
> + if (ret)
> + return ret;
> +
> + ret = regmap_read(st->regmap, AD4851_REG_PRODUCT_ID_L, &product_id);
> + if (ret)
> + return ret;
> +
> + if (product_id != st->info->product_id)
> + dev_info(&st->spi->dev, "Unknown product ID: 0x%02X\n",
> + product_id);
> +
> + ret = regmap_set_bits(st->regmap, AD4851_REG_DEVICE_CTRL,
> + AD4851_ECHO_CLOCK_MODE);
> + if (ret)
> + return ret;
> +
> + return regmap_write(st->regmap, AD4851_REG_PACKET, 0);
> +}
> +
> +static int ad4851_find_opt(bool *field, u32 size, u32 *ret_start)
> +{
> + unsigned int i, cnt = 0, max_cnt = 0, max_start = 0;
> + int start;
> +
> + for (i = 0, start = -1; i < size; i++) {
> + if (field[i] == 0) {
> + if (start == -1)
> + start = i;
> + cnt++;
> + } else {
> + if (cnt > max_cnt) {
> + max_cnt = cnt;
> + max_start = start;
> + }
> + start = -1;
> + cnt = 0;
> + }
> + }
> + /*
> + * Find the longest consecutive sequence of false values from field
> + * and return starting index.
> + */
> + if (cnt > max_cnt) {
> + max_cnt = cnt;
> + max_start = start;
> + }
> +
> + if (!max_cnt)
> + return -ENOENT;
> +
> + *ret_start = max_start;
> +
> + return max_cnt;
> +}
>
Your status seems to be a bitmap now, can you take a look in here and see if you can
do something similar?
https://elixir.bootlin.com/linux/v6.12.6/source/drivers/iio/adc/ad9467.c#L633
Anyways, look at the bitmap API. It might have helpers to make the above function
simpler.
> +
> +static int ad4851_calibrate(struct iio_dev *indio_dev)
> +{
> + struct ad4851_state *st = iio_priv(indio_dev);
> + unsigned int opt_delay, num_lanes, delay, i, s, c;
> + enum iio_backend_interface_type interface_type;
> + DECLARE_BITMAP(pn_status, AD4851_MAX_LANES * AD4851_MAX_IODELAY);
> + bool status;
> + int ret;
> +
> + ret = iio_backend_interface_type_get(st->back, &interface_type);
> + if (ret)
> + return ret;
> +
> + switch (interface_type) {
> + case IIO_BACKEND_INTERFACE_SERIAL_CMOS:
> + num_lanes = indio_dev->num_channels;
> + break;
> + case IIO_BACKEND_INTERFACE_SERIAL_LVDS:
> + num_lanes = 1;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + if (st->info->resolution == 16) {
> + ret = iio_backend_data_size_set(st->back, 24);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(st->regmap, AD4851_REG_PACKET,
> + AD4851_TEST_PAT | AD4857_PACKET_SIZE_24);
> + if (ret)
> + return ret;
> + } else {
> + ret = iio_backend_data_size_set(st->back, 32);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(st->regmap, AD4851_REG_PACKET,
> + AD4851_TEST_PAT | AD4858_PACKET_SIZE_32);
> + if (ret)
> + return ret;
> + }
> +
> + for (i = 0; i < indio_dev->num_channels; i++) {
> + ret = regmap_write(st->regmap, AD4851_REG_TESTPAT_0(i),
> + AD4851_TESTPAT_0_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(st->regmap, AD4851_REG_TESTPAT_1(i),
> + AD4851_TESTPAT_1_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(st->regmap, AD4851_REG_TESTPAT_2(i),
> + AD4851_TESTPAT_2_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(st->regmap, AD4851_REG_TESTPAT_3(i),
> + AD4851_TESTPAT_3_DEFAULT(i));
> + if (ret)
> + return ret;
> +
> + ret = iio_backend_chan_enable(st->back,
> + indio_dev->channels[i].channel);
> + if (ret)
> + return ret;
> + }
> +
> + for (i = 0; i < num_lanes; i++) {
> + for (delay = 0; delay < AD4851_MAX_IODELAY; delay++) {
> + ret = iio_backend_iodelay_set(st->back, i, delay);
> + if (ret)
> + return ret;
> +
> + ret = iio_backend_chan_status(st->back, i, &status);
> + if (ret)
> + return ret;
> +
> + if (status)
> + set_bit(i * AD4851_MAX_IODELAY + delay,
> pn_status);
> + else
> + clear_bit(i * AD4851_MAX_IODELAY + delay,
> pn_status);
> + }
you can use the lighter forms __set_bit()/__clear_bit(). But going one step further,
maybe __assign_bit()?
> + }
> +
> + for (i = 0; i < num_lanes; i++) {
> + status = test_bit(i * AD4851_MAX_IODELAY, pn_status);
> + c = ad4851_find_opt(&status, AD4851_MAX_IODELAY, &s);
status is just a bool and it seems your iterating on it? Am I missing something?
> + if (c < 0)
> + return c;
> +
> + opt_delay = s + c / 2;
> + ret = iio_backend_iodelay_set(st->back, i, opt_delay);
> + if (ret)
> + return ret;
> + }
> +
> + for (i = 0; i < indio_dev->num_channels; i++) {
> + ret = iio_backend_chan_disable(st->back, i);
> + if (ret)
> + return ret;
> + }
> +
> + ret = iio_backend_data_size_set(st->back, 20);
> + if (ret)
> + return ret;
> +
> + return regmap_write(st->regmap, AD4851_REG_PACKET, 0);
> +}
> +
>
...
> +static int ad4851_read_raw(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + int *val, int *val2, long info)
> +{
> + struct ad4851_state *st = iio_priv(indio_dev);
> +
> + switch (info) {
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + *val = st->cnv_trigger_rate_hz / st->osr;
> + return IIO_VAL_FRACTIONAL;
> + case IIO_CHAN_INFO_CALIBSCALE:
> + return ad4851_get_calibscale(st, chan->channel, val, val2);
> + case IIO_CHAN_INFO_SCALE:
> + return ad4851_get_scale(indio_dev, chan, val, val2);
Maybe this was discussed already and I missed it but I'm a bit puzzled. Don't we
still need OFFSET for differential channels? How do you express negative voltages?
- Nuno Sá
