| Message ID | 20240923101206.3753-7-antoniu.miclaus@analog.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | *** Add support for AD485x DAS Family *** | expand |
On Mon, Sep 23, 2024 at 01:10:23PM +0300, Antoniu Miclaus wrote: > Add support for the AD485X DAS familiy. ... > +#include <asm/unaligned.h> It's better to move this after linux/*.h > +#include <linux/delay.h> > +#include <linux/iio/backend.h> > +#include <linux/iio/iio.h> This can be split to a group after the other linux/*.h > +#include <linux/pwm.h> > +#include <linux/regmap.h> > +#include <linux/regulator/consumer.h> > +#include <linux/spi/spi.h> > +#include <linux/units.h> Seems missing headers: + array_size.h + bits.h + device.h + err.h + mod_devicetable.h + module.h + mutex.h + types.h ... > +static int ad485x_set_sampling_freq(struct ad485x_state *st, unsigned int freq) > +{ > + struct pwm_state cnv_state = { > + .duty_cycle = AD485X_T_CNVH_NS, > + .enabled = true, > + }; > + int ret; > + > + if (freq > st->info->throughput) > + freq = st->info->throughput; clamp() ? (will need minmax.h) > + cnv_state.period = DIV_ROUND_CLOSEST_ULL(1000000000, freq); We have time / frequency constants, like HZ_PER_..., NSEC_PER_.... > + ret = pwm_apply_might_sleep(st->cnv, &cnv_state); > + if (ret) > + return ret; > + > + st->sampling_freq = freq; > + > + return 0; > +} > + > +static int ad485x_setup(struct ad485x_state *st) > +{ > + unsigned int product_id; > + int ret; > + > + ret = ad485x_set_sampling_freq(st, 1000000); Ditto. > + if (ret) > + return ret; > + > + ret = regmap_write(st->regmap, AD485X_REG_INTERFACE_CONFIG_A, > + AD485X_SW_RESET); > + if (ret) > + return ret; > + > + ret = regmap_write(st->regmap, AD485X_REG_INTERFACE_CONFIG_B, > + AD485X_SINGLE_INSTRUCTION); > + if (ret) > + return ret; > + > + ret = regmap_write(st->regmap, AD485X_REG_INTERFACE_CONFIG_A, > + AD485X_SDO_ENABLE); > + if (ret) > + return ret; > + > + ret = regmap_read(st->regmap, AD485X_REG_PRODUCT_ID_L, &product_id); > + if (ret) > + return ret; > + > + if (product_id != st->info->product_id) > + dev_warn(&st->spi->dev, "Unknown product ID: 0x%02X\n", > + product_id); > + > + ret = regmap_write(st->regmap, AD485X_REG_DEVICE_CTRL, > + AD485X_ECHO_CLOCK_MODE); > + if (ret) > + return ret; > + > + return regmap_write(st->regmap, AD485X_REG_PACKET, 0); > +} ... > +static int ad485x_find_opt(bool *field, u32 size, u32 *ret_start) > +{ > + int i, cnt = 0, max_cnt = 0, start, max_start = 0; Why are they signed? > + 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; > + } > + } > + > + if (cnt > max_cnt) { > + max_cnt = cnt; > + max_start = start; > + } > + > + if (!max_cnt) > + return -EIO; > + > + *ret_start = max_start; > + > + return max_cnt; > +} ... > +static int ad485x_calibrate(struct ad485x_state *st) > +{ > + int opt_delay, lane_num, delay, i, s, c; Why e.g. i is signed? > + enum iio_backend_interface_type interface_type; > + bool pn_status[AD485X_MAX_LANES][AD485X_MAX_IODELAY]; Why not a bitmap? > + int ret; > +} ... > +static const char *const ad4858_packet_fmts[] = { > + "20-bit", "24-bit", "32-bit" Leave trailing comma, it may help extending the list in case it becomes a multi-line. > +}; > + > +static const char *const ad4857_packet_fmts[] = { > + "16-bit", "24-bit" Ditto. > +}; ... > +static int ad485x_get_packet_format(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan) > +{ > + struct ad485x_state *st = iio_priv(indio_dev); > + unsigned int format; > + int ret; > + > + ret = regmap_read(st->regmap, AD485X_REG_PACKET, &format); > + if (ret) > + return ret; > + format = FIELD_GET(AD485X_PACKET_FORMAT_MASK, format); > + > + return format; return FIELD_GET(...); > +} > +static int ad485x_get_calibscale(struct ad485x_state *st, int ch, int *val, > + int *val2) One line? ... > + ret = regmap_read(st->regmap, AD485X_REG_CHX_GAIN_MSB(ch), > + ®_val); > + if (ret) > + return ret; > + > + gain = (reg_val & 0xFF) << 8; > + > + ret = regmap_read(st->regmap, AD485X_REG_CHX_GAIN_LSB(ch), > + ®_val); > + if (ret) > + return ret; > + > + gain |= reg_val & 0xFF; Why not bulk read and proper __le16/__be16 type? ... > +static int ad485x_set_calibscale(struct ad485x_state *st, int ch, int val, > + int val2) > +{ > + unsigned long long gain; > + unsigned int reg_val; > + int ret; > + > + gain = val * 1000000 + val2; MICRO? > + gain = gain * 32768; > + gain = DIV_U64_ROUND_CLOSEST(gain, 1000000); Can be combined into one lien. > + reg_val = gain; ... > + ret = regmap_write(st->regmap, AD485X_REG_CHX_GAIN_MSB(ch), > + reg_val >> 8); > + if (ret) > + return ret; > + > + return regmap_write(st->regmap, AD485X_REG_CHX_GAIN_LSB(ch), > + reg_val & 0xFF); Bulk write? > +} ... > + ret = regmap_read(st->regmap, AD485X_REG_CHX_OFFSET_MSB(ch), > + &msb); > + if (ret) > + return ret; > + > + ret = regmap_read(st->regmap, AD485X_REG_CHX_OFFSET_MID(ch), > + &mid); > + if (ret) > + return ret; > + > + ret = regmap_read(st->regmap, AD485X_REG_CHX_OFFSET_LSB(ch), > + &lsb); > + if (ret) > + return ret; Bulk read? > + if (st->info->resolution == 16) { > + *val = msb << 8; > + *val |= mid; > + *val = sign_extend32(*val, 15); So, please use respective byteorder conversions... > + } else { > + *val = msb << 12; > + *val |= mid << 4; > + *val |= lsb >> 4; > + *val = sign_extend32(*val, 19); ...here as well (we have _be14()/_le24 ones). > + } ... > +static int ad485x_set_calibbias(struct ad485x_state *st, int ch, int val, > + int val2) > +{ > + unsigned int lsb, mid, msb; > + int ret; > + > + if (st->info->resolution == 16) { > + lsb = 0; > + mid = val & 0xFF; > + msb = (val >> 8) & 0xFF; > + } else { > + lsb = (val << 4) & 0xFF; > + mid = (val >> 4) & 0xFF; > + msb = (val >> 12) & 0xFF; > + } Ditto. > + guard(mutex)(&st->lock); > + > + ret = regmap_write(st->regmap, AD485X_REG_CHX_OFFSET_LSB(ch), lsb); > + if (ret) > + return ret; > + > + ret = regmap_write(st->regmap, AD485X_REG_CHX_OFFSET_MID(ch), mid); > + if (ret) > + return ret; > + > + return regmap_write(st->regmap, AD485X_REG_CHX_OFFSET_MSB(ch), msb); Bulk write? > +} ... > +static const unsigned int ad485x_scale_table[][2] = { > + {2500, 0x0}, {5000, 0x1}, {5000, 0x2}, {10000, 0x3}, {6250, 0x04}, > + {12500, 0x5}, {10000, 0x6}, {20000, 0x7}, {12500, 0x8}, {25000, 0x9}, > + {20000, 0xA}, {40000, 0xB}, {25000, 0xC}, {50000, 0xD}, {40000, 0xE}, > + {80000, 0xF} It's more natural to list them in pow-of-two per line manner. Moreover the last item is redundant. It's equal to the index. Why do you need it? > +}; ... > +static const int ad4857_offset_table[] = { > + 0, -32768 Here, and above and everywhere else in such cases leave a trailing comma. > +}; ... > + for (i = 0; i < info->num_scales; i++) { > + __ad485x_get_scale(st, info->scale_table[i][0], > + &scale_val[0], &scale_val[1]); > + if (scale_val[0] != val || scale_val[1] != val2) > + continue; > + > + /* > + * Adjust the softspan value (differential or single ended) > + * based on the scale value selected and current offset of > + * the channel. > + * > + * If the offset is 0 then continue iterations until finding > + * the next matching scale value which always corresponds to > + * the single ended mode. > + */ > + if (!st->offsets[chan->channel] && !j) { > + j++; So, j may be named properly and be boolean for the sake of the semantic usage. > + continue; > + } > + > + return regmap_write(st->regmap, > + AD485X_REG_CHX_SOFTSPAN(chan->channel), > + info->scale_table[i][1]); > + } ... > +{ > + guard(mutex)(&st->lock); > + > + if (st->offsets[chan->channel] != val) { Invert and drop indentation for the next lines. > + st->offsets[chan->channel] = val; > + /* Restore to the default range if offset changes */ > + if (st->offsets[chan->channel]) > + return regmap_write(st->regmap, > + AD485X_REG_CHX_SOFTSPAN(chan->channel), > + AD485X_SOFTSPAN_N40V_40V); > + return regmap_write(st->regmap, > + AD485X_REG_CHX_SOFTSPAN(chan->channel), > + AD485X_SOFTSPAN_0V_40V); > + } > + > + return 0; > +} ... > +static int ad485x_read_raw(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, > + int *val, int *val2, long info) > +{ > + struct ad485x_state *st = iio_priv(indio_dev); > + > + switch (info) { > + case IIO_CHAN_INFO_SAMP_FREQ: > + *val = st->sampling_freq; > + return IIO_VAL_INT; > + case IIO_CHAN_INFO_CALIBSCALE: > + return ad485x_get_calibscale(st, chan->channel, val, val2); > + case IIO_CHAN_INFO_SCALE: > + return ad485x_get_scale(st, chan, val, val2); > + case IIO_CHAN_INFO_CALIBBIAS: > + return ad485x_get_calibbias(st, chan->channel, val, val2); > + case IIO_CHAN_INFO_OFFSET: > + scoped_guard(mutex, &st->lock) > + * val = st->offsets[chan->channel]; This is interesting white space location... > + return IIO_VAL_INT; > + default: > + return -EINVAL; > + } > +} ... > + struct ad485x_state *st = iio_priv(indio_dev); > + unsigned int c; > + int ret; > + > + for (c = 0; c < st->info->num_channels; c++) { > + if (test_bit(c, scan_mask)) Do we have a helper now for this iterator? > + ret = iio_backend_chan_enable(st->back, c); > + else > + ret = iio_backend_chan_disable(st->back, c); > + if (ret) > + return ret; > + } ... > +static struct iio_chan_spec_ext_info ad4858_ext_info[] = { > + IIO_ENUM("packet_format", IIO_SHARED_BY_ALL, &ad4858_packet_fmt), > + IIO_ENUM_AVAILABLE("packet_format", > + IIO_SHARED_BY_ALL, &ad4858_packet_fmt), > + {}, Please, drop comma in the terminator lines. > +}; ... > +static const struct ad485x_chip_info ad4858i_info = { > + .name = "ad4858i", > + .product_id = AD4858I_PROD_ID, > + .scale_table = ad485x_scale_table, > + .num_scales = ARRAY_SIZE(ad485x_scale_table), > + .offset_table = ad4858_offset_table, > + .num_offset = ARRAY_SIZE(ad4858_offset_table), > + .channels = ad4858_channels, > + .num_channels = ARRAY_SIZE(ad4858_channels), > + .throughput = 1000000, How many people wrote this patch? It has inconsistency at least in multipliers like this all over the code. Who knows what's else also being inconsistent... > + .resolution = 20, > +}; ... > +static const struct regmap_config regmap_config = { > + .reg_bits = 16, > + .val_bits = 8, > + .read_flag_mask = BIT(7), > +}; Why do you need regmap lock? ... > +static const char * const ad485x_power_supplies[] = { > + "vcc", "vdd", "vddh", "vio" Leave trailing comma. > +}; ... > +static int ad485x_probe(struct spi_device *spi) > +{ > + struct iio_dev *indio_dev; > + struct ad485x_state *st; > + int ret; > + > + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); > + if (!indio_dev) > + return -ENOMEM; > + > + st = iio_priv(indio_dev); > + st->spi = spi; > + mutex_init(&st->lock); Why not devm? > + ret = devm_regulator_bulk_get_enable(&spi->dev, > + ARRAY_SIZE(ad485x_power_supplies), > + ad485x_power_supplies); > + if (ret) > + return dev_err_probe(&spi->dev, ret, > + "failed to get and enable supplies\n"); > + > + st->cnv = devm_pwm_get(&spi->dev, NULL); > + if (IS_ERR(st->cnv)) > + return PTR_ERR(st->cnv); > + > + st->info = spi_get_device_match_data(spi); > + if (!st->info) > + return -ENODEV; > + > + st->regmap = devm_regmap_init_spi(spi, ®map_config); > + if (IS_ERR(st->regmap)) > + return PTR_ERR(st->regmap); > + > + ret = ad485x_scale_offset_fill(st); > + if (ret) > + return ret; > + > + ret = ad485x_setup(st); > + if (ret) > + return ret; > + > + indio_dev->name = st->info->name; > + indio_dev->channels = st->info->channels; > + indio_dev->num_channels = st->info->num_channels; > + indio_dev->info = &ad485x_info; > + > + st->back = devm_iio_backend_get(&spi->dev, NULL); > + if (IS_ERR(st->back)) > + return PTR_ERR(st->back); > + > + ret = devm_iio_backend_request_buffer(&spi->dev, st->back, indio_dev); > + if (ret) > + return ret; > + > + ret = devm_iio_backend_enable(&spi->dev, st->back); > + if (ret) > + return ret; > + > + ret = ad485x_calibrate(st); > + if (ret) > + return ret; > + > + return devm_iio_device_register(&spi->dev, indio_dev); > +}
Hi Antoniu, kernel test robot noticed the following build errors: [auto build test ERROR on robh/for-next] [also build test ERROR on linus/master v6.11] [cannot apply to jic23-iio/togreg next-20240924] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Antoniu-Miclaus/iio-backend-add-API-for-interface-get/20240923-182050 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next patch link: https://lore.kernel.org/r/20240923101206.3753-7-antoniu.miclaus%40analog.com patch subject: [PATCH 6/7] iio: adc: ad485x: add ad485x driver config: openrisc-allyesconfig (https://download.01.org/0day-ci/archive/20240924/202409242353.rDAcuGYR-lkp@intel.com/config) compiler: or1k-linux-gcc (GCC) 14.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240924/202409242353.rDAcuGYR-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202409242353.rDAcuGYR-lkp@intel.com/ All errors (new ones prefixed by >>): drivers/iio/adc/ad485x.c: In function 'ad485x_get_packet_format': >> drivers/iio/adc/ad485x.c:396:18: error: implicit declaration of function 'FIELD_GET' [-Wimplicit-function-declaration] 396 | format = FIELD_GET(AD485X_PACKET_FORMAT_MASK, format); | ^~~~~~~~~ drivers/iio/adc/ad485x.c: At top level: drivers/iio/adc/ad485x.c:854:23: warning: initialized field overwritten [-Woverride-init] 854 | .resolution = 16, | ^~ drivers/iio/adc/ad485x.c:854:23: note: (near initialization for 'ad4856_info.resolution') vim +/FIELD_GET +396 drivers/iio/adc/ad485x.c 384 385 static int ad485x_get_packet_format(struct iio_dev *indio_dev, 386 const struct iio_chan_spec *chan) 387 { 388 struct ad485x_state *st = iio_priv(indio_dev); 389 unsigned int format; 390 int ret; 391 392 ret = regmap_read(st->regmap, AD485X_REG_PACKET, &format); 393 if (ret) 394 return ret; 395 > 396 format = FIELD_GET(AD485X_PACKET_FORMAT_MASK, format); 397 398 return format; 399 } 400
On Mon, Sep 23, 2024 at 12:17 PM Antoniu Miclaus <antoniu.miclaus@analog.com> wrote: > > Add support for the AD485X DAS familiy. > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> > --- ... > +static int ad485x_find_opt(bool *field, u32 size, u32 *ret_start) > +{ > + int i, cnt = 0, max_cnt = 0, start, max_start = 0; > + > + 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; > + } > + } > + > + if (cnt > max_cnt) { > + max_cnt = cnt; > + max_start = start; > + } > + > + if (!max_cnt) > + return -EIO; EIO seems an odd choice since this function doesn't actually do any I/O. Maybe EINVAL would be better? > + > + *ret_start = max_start; > + > + return max_cnt; > +} > + ... > +static int ad485x_set_calibscale(struct ad485x_state *st, int ch, int val, > + int val2) > +{ > + unsigned long long gain; > + unsigned int reg_val; > + int ret; > + Need to check both val and val2 for negative here before converting to unsigned. if (val < 0 || val2 < 0) return -EINVAL; > + gain = val * 1000000 + val2; > + gain = gain * 32768; > + gain = DIV_U64_ROUND_CLOSEST(gain, 1000000); > + > + reg_val = gain; > + > + guard(mutex)(&st->lock); > + > + ret = regmap_write(st->regmap, AD485X_REG_CHX_GAIN_MSB(ch), > + reg_val >> 8); > + if (ret) > + return ret; > + > + return regmap_write(st->regmap, AD485X_REG_CHX_GAIN_LSB(ch), > + reg_val & 0xFF); > +} > + > +static int ad485x_get_calibbias(struct ad485x_state *st, int ch, int *val, > + int *val2) > +{ val2 is unused and can be removed > + unsigned int lsb, mid, msb; > + int ret; > + > + guard(mutex)(&st->lock); > + > + ret = regmap_read(st->regmap, AD485X_REG_CHX_OFFSET_MSB(ch), > + &msb); > + if (ret) > + return ret; > + > + ret = regmap_read(st->regmap, AD485X_REG_CHX_OFFSET_MID(ch), > + &mid); > + if (ret) > + return ret; > + > + ret = regmap_read(st->regmap, AD485X_REG_CHX_OFFSET_LSB(ch), > + &lsb); > + if (ret) > + return ret; > + > + if (st->info->resolution == 16) { > + *val = msb << 8; > + *val |= mid; > + *val = sign_extend32(*val, 15); > + } else { > + *val = msb << 12; > + *val |= mid << 4; > + *val |= lsb >> 4; > + *val = sign_extend32(*val, 19); > + } > + > + return IIO_VAL_INT; > +} > + > +static int ad485x_set_calibbias(struct ad485x_state *st, int ch, int val, > + int val2) > +{ val2 is unused here. It would also be a good idea to implement the write_raw_get_fmt callback to select IIO_VAL_INT for this attribute to avoid having to deal with negative val2. > + unsigned int lsb, mid, msb; > + int ret; Should check for negative val here before converting to unsigned. > + > + if (st->info->resolution == 16) { > + lsb = 0; > + mid = val & 0xFF; > + msb = (val >> 8) & 0xFF; > + } else { > + lsb = (val << 4) & 0xFF; > + mid = (val >> 4) & 0xFF; > + msb = (val >> 12) & 0xFF; > + } > + > + guard(mutex)(&st->lock); > + > + ret = regmap_write(st->regmap, AD485X_REG_CHX_OFFSET_LSB(ch), lsb); > + if (ret) > + return ret; > + > + ret = regmap_write(st->regmap, AD485X_REG_CHX_OFFSET_MID(ch), mid); > + if (ret) > + return ret; > + > + return regmap_write(st->regmap, AD485X_REG_CHX_OFFSET_MSB(ch), msb); > +} > + ... > +static int ad485x_set_offset(struct ad485x_state *st, > + const struct iio_chan_spec *chan, int val) > +{ > + guard(mutex)(&st->lock); > + > + if (st->offsets[chan->channel] != val) { > + st->offsets[chan->channel] = val; > + /* Restore to the default range if offset changes */ > + if (st->offsets[chan->channel]) > + return regmap_write(st->regmap, > + AD485X_REG_CHX_SOFTSPAN(chan->channel), > + AD485X_SOFTSPAN_N40V_40V); > + return regmap_write(st->regmap, > + AD485X_REG_CHX_SOFTSPAN(chan->channel), > + AD485X_SOFTSPAN_0V_40V); > + } > + > + return 0; > +} I'm not sure I understand the relationship between softspan and the offset. A raw value of 0 always means we measured 0V no matter what the softspan setting is. So it seems like the offset should always be 0. I'm guessing the intent was to handle bipolar vs. unipolar softspans, but this doesn't actually work mathematically. So far, I've only seen inputs that can be bipolar or unipolar specified in the devicetree. I'm not aware of a way to select this at runtime. > +static struct iio_chan_spec_ext_info ad4858_ext_info[] = { > + IIO_ENUM("packet_format", IIO_SHARED_BY_ALL, &ad4858_packet_fmt), > + IIO_ENUM_AVAILABLE("packet_format", > + IIO_SHARED_BY_ALL, &ad4858_packet_fmt), > + {}, > +}; > + > +static struct iio_chan_spec_ext_info ad4857_ext_info[] = { > + IIO_ENUM("packet_format", IIO_SHARED_BY_ALL, &ad4857_packet_fmt), > + IIO_ENUM_AVAILABLE("packet_format", > + IIO_SHARED_BY_ALL, &ad4857_packet_fmt), > + {}, > +}; Usually, something like this packet format would be automatically selected when buffered reads are enabled based on what other features it provides are needed, i.e only enable the status bits when events are enabled. (For those that didn't read the datasheet, the different packet formats basically enable extra status bits per sample. And in the case of oversampling, one of the formats also selects a reduced number of sample bits.) We have quite a few parts in the pipline right like this one that have per-sample status bits. In the past, these were generally handled with IIO events, but this doesn't really work for these high-speed backends since the data is being piped directly to DMA and we don't look at each sample in the ADC driver. So it would be worthwhile to try to find some general solution here for handling this sort of thing. > + > +static const struct iio_chan_spec ad4858_channels[] = { > + AD485X_IIO_CHANNEL(0, 20, 32, ad4858_ext_info), > + AD485X_IIO_CHANNEL(1, 20, 32, ad4858_ext_info), > + AD485X_IIO_CHANNEL(2, 20, 32, ad4858_ext_info), > + AD485X_IIO_CHANNEL(3, 20, 32, ad4858_ext_info), > + AD485X_IIO_CHANNEL(4, 20, 32, ad4858_ext_info), > + AD485X_IIO_CHANNEL(5, 20, 32, ad4858_ext_info), > + AD485X_IIO_CHANNEL(6, 20, 32, ad4858_ext_info), > + AD485X_IIO_CHANNEL(7, 20, 32, ad4858_ext_info), > +}; > + > +static const struct iio_chan_spec ad4857_channels[] = { > + AD485X_IIO_CHANNEL(0, 16, 16, ad4857_ext_info), > + AD485X_IIO_CHANNEL(1, 16, 16, ad4857_ext_info), > + AD485X_IIO_CHANNEL(2, 16, 16, ad4857_ext_info), > + AD485X_IIO_CHANNEL(3, 16, 16, ad4857_ext_info), > + AD485X_IIO_CHANNEL(4, 16, 16, ad4857_ext_info), > + AD485X_IIO_CHANNEL(5, 16, 16, ad4857_ext_info), > + AD485X_IIO_CHANNEL(6, 16, 16, ad4857_ext_info), > + AD485X_IIO_CHANNEL(7, 16, 16, ad4857_ext_info), > +}; How does 16-bit storage size work when packet format is 24-bit?
On Thu, Sep 26, 2024 at 04:39:18PM +0200, David Lechner wrote: > On Mon, Sep 23, 2024 at 12:17 PM Antoniu Miclaus > <antoniu.miclaus@analog.com> wrote: ... > > +static int ad485x_find_opt(bool *field, u32 size, u32 *ret_start) > > +{ > > + int i, cnt = 0, max_cnt = 0, start, max_start = 0; > > + > > + 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; > > + } > > + } > > + > > + if (cnt > max_cnt) { > > + max_cnt = cnt; > > + max_start = start; > > + } > > + > > + if (!max_cnt) > > + return -EIO; > > EIO seems an odd choice since this function doesn't actually do any > I/O. Maybe EINVAL would be better? I would even go with -ENOENT as function called 'find'. > > + *ret_start = max_start; > > + > > + return max_cnt; > > +}
> > > +static struct iio_chan_spec_ext_info ad4858_ext_info[] = { > > + IIO_ENUM("packet_format", IIO_SHARED_BY_ALL, &ad4858_packet_fmt), > > + IIO_ENUM_AVAILABLE("packet_format", > > + IIO_SHARED_BY_ALL, &ad4858_packet_fmt), > > + {}, > > +}; > > + > > +static struct iio_chan_spec_ext_info ad4857_ext_info[] = { > > + IIO_ENUM("packet_format", IIO_SHARED_BY_ALL, &ad4857_packet_fmt), > > + IIO_ENUM_AVAILABLE("packet_format", > > + IIO_SHARED_BY_ALL, &ad4857_packet_fmt), > > + {}, > > +}; > > Usually, something like this packet format would be automatically > selected when buffered reads are enabled based on what other features > it provides are needed, i.e only enable the status bits when events > are enabled. > > (For those that didn't read the datasheet, the different packet > formats basically enable extra status bits per sample. And in the case > of oversampling, one of the formats also selects a reduced number of > sample bits.) > > We have quite a few parts in the pipline right like this one that have > per-sample status bits. In the past, these were generally handled with > IIO events, but this doesn't really work for these high-speed backends > since the data is being piped directly to DMA and we don't look at > each sample in the ADC driver. So it would be worthwhile to try to > find some general solution here for handling this sort of thing. We have previously talked about schemes to describe metadata alongside channels. I guess maybe it's time to actually look at how that works. I'm not sure dynamic control of that metadata is going to be easy to do though or if we even want to (as opposed to always on or off for a particular device).
On Mon, 23 Sep 2024 13:10:23 +0300 Antoniu Miclaus <antoniu.miclaus@analog.com> wrote: > Add support for the AD485X DAS familiy. > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> A few additional comments from me. Thanks, Jonathan > diff --git a/drivers/iio/adc/ad485x.c b/drivers/iio/adc/ad485x.c > new file mode 100644 > index 000000000000..3333cad9ed8f > --- /dev/null > +++ b/drivers/iio/adc/ad485x.c > @@ -0,0 +1,1061 @@ > +static int ad485x_setup(struct ad485x_state *st) > +{ > + unsigned int product_id; > + int ret; > + > + ret = ad485x_set_sampling_freq(st, 1000000); > + if (ret) > + return ret; > + > + ret = regmap_write(st->regmap, AD485X_REG_INTERFACE_CONFIG_A, > + AD485X_SW_RESET); > + if (ret) > + return ret; > + > + ret = regmap_write(st->regmap, AD485X_REG_INTERFACE_CONFIG_B, > + AD485X_SINGLE_INSTRUCTION); > + if (ret) > + return ret; > + > + ret = regmap_write(st->regmap, AD485X_REG_INTERFACE_CONFIG_A, > + AD485X_SDO_ENABLE); > + if (ret) > + return ret; > + > + ret = regmap_read(st->regmap, AD485X_REG_PRODUCT_ID_L, &product_id); > + if (ret) > + return ret; > + > + if (product_id != st->info->product_id) > + dev_warn(&st->spi->dev, "Unknown product ID: 0x%02X\n", > + product_id); dev_info() for this is probably better. Might be fine afterall if the new part is fully backwards compatible with this one. > + > + ret = regmap_write(st->regmap, AD485X_REG_DEVICE_CTRL, > + AD485X_ECHO_CLOCK_MODE); > + if (ret) > + return ret; > + > + return regmap_write(st->regmap, AD485X_REG_PACKET, 0); > +} > + > +static int ad485x_get_packet_format(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan) > +{ > + struct ad485x_state *st = iio_priv(indio_dev); > + unsigned int format; > + int ret; > + > + ret = regmap_read(st->regmap, AD485X_REG_PACKET, &format); > + if (ret) > + return ret; > + > + format = FIELD_GET(AD485X_PACKET_FORMAT_MASK, format); > + > + return format; return FIELD_GET() > +} > + > +static int ad485x_get_calibscale(struct ad485x_state *st, int ch, int *val, > + int *val2) > +{ > + unsigned int reg_val; > + int gain; > + int ret; > + > + guard(mutex)(&st->lock); > + > + ret = regmap_read(st->regmap, AD485X_REG_CHX_GAIN_MSB(ch), > + ®_val); > + if (ret) > + return ret; > + > + gain = (reg_val & 0xFF) << 8; Use a byte array into which you write the regval then a get_unalignedb_be16 or similar to get the value. > + > + ret = regmap_read(st->regmap, AD485X_REG_CHX_GAIN_LSB(ch), > + ®_val); > + if (ret) > + return ret; > + > + gain |= reg_val & 0xFF; > + > + *val = gain; > + *val2 = 32768; > + > + return IIO_VAL_FRACTIONAL; > +} > +static int ad485x_get_calibbias(struct ad485x_state *st, int ch, int *val, > + int *val2) > +{ > + unsigned int lsb, mid, msb; > + int ret; > + > + guard(mutex)(&st->lock); > + > + ret = regmap_read(st->regmap, AD485X_REG_CHX_OFFSET_MSB(ch), > + &msb); > + if (ret) > + return ret; > + > + ret = regmap_read(st->regmap, AD485X_REG_CHX_OFFSET_MID(ch), > + &mid); > + if (ret) > + return ret; > + > + ret = regmap_read(st->regmap, AD485X_REG_CHX_OFFSET_LSB(ch), > + &lsb); > + if (ret) > + return ret; > + > + if (st->info->resolution == 16) { > + *val = msb << 8; > + *val |= mid; > + *val = sign_extend32(*val, 15); > + } else { > + *val = msb << 12; > + *val |= mid << 4; > + *val |= lsb >> 4; As below I'd use a byte array then you can do get_unaligned_be24 to + a right shift by 4 of the whole thing. > + *val = sign_extend32(*val, 19); > + } > + > + return IIO_VAL_INT; > +} > + > +static int ad485x_set_calibbias(struct ad485x_state *st, int ch, int val, > + int val2) > +{ > + unsigned int lsb, mid, msb; > + int ret; > + > + if (st->info->resolution == 16) { > + lsb = 0; > + mid = val & 0xFF; > + msb = (val >> 8) & 0xFF; > + } else { > + lsb = (val << 4) & 0xFF; Might as well mask by 0xF0 to make it really clear nothing in bottom few bits. > + mid = (val >> 4) & 0xFF; > + msb = (val >> 12) & 0xFF; I'd be tempted to shift the whole thing left 4 bits and then use a put_unaligned_be24 on it into a byte array then pick out the bytes from that array. Will be easier to read. Above 16 bit case would be a put_unaligned_be16 > + } > + > + guard(mutex)(&st->lock); > + > + ret = regmap_write(st->regmap, AD485X_REG_CHX_OFFSET_LSB(ch), lsb); > + if (ret) > + return ret; > + > + ret = regmap_write(st->regmap, AD485X_REG_CHX_OFFSET_MID(ch), mid); > + if (ret) > + return ret; > + > + return regmap_write(st->regmap, AD485X_REG_CHX_OFFSET_MSB(ch), msb); I think you already got this question, but consider bulk writes. > +} > + > +static const unsigned int ad485x_scale_table[][2] = { > + {2500, 0x0}, {5000, 0x1}, {5000, 0x2}, {10000, 0x3}, {6250, 0x04}, > + {12500, 0x5}, {10000, 0x6}, {20000, 0x7}, {12500, 0x8}, {25000, 0x9}, > + {20000, 0xA}, {40000, 0xB}, {25000, 0xC}, {50000, 0xD}, {40000, 0xE}, > + {80000, 0xF} Spaces after { and before } preferred. Maybe just have this as one item per line. I think that is more readable. > +}; > + > +static int ad485x_read_raw(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, > + int *val, int *val2, long info) > +{ > + struct ad485x_state *st = iio_priv(indio_dev); > + > + switch (info) { > + case IIO_CHAN_INFO_SAMP_FREQ: > + *val = st->sampling_freq; > + return IIO_VAL_INT; > + case IIO_CHAN_INFO_CALIBSCALE: > + return ad485x_get_calibscale(st, chan->channel, val, val2); > + case IIO_CHAN_INFO_SCALE: > + return ad485x_get_scale(st, chan, val, val2); > + case IIO_CHAN_INFO_CALIBBIAS: > + return ad485x_get_calibbias(st, chan->channel, val, val2); > + case IIO_CHAN_INFO_OFFSET: > + scoped_guard(mutex, &st->lock) > + * val = st->offsets[chan->channel]; *val = ... > + return IIO_VAL_INT; > + default: > + return -EINVAL; > + } > +} > + > +#define AD485X_IIO_CHANNEL(index, real, storage, info) \ > +{ \ > + .type = IIO_VOLTAGE, \ > + .ext_info = info, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_CALIBSCALE) | \ > + BIT(IIO_CHAN_INFO_CALIBBIAS) | \ > + BIT(IIO_CHAN_INFO_SCALE) | \ > + BIT(IIO_CHAN_INFO_OFFSET), \ > + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \ > + .info_mask_shared_by_type_available = \ > + BIT(IIO_CHAN_INFO_SCALE) | \ > + BIT(IIO_CHAN_INFO_OFFSET), \ Maybe add avail for calibscale and calibbias as well. > + > +static struct iio_chan_spec_ext_info ad4858_ext_info[] = { > + IIO_ENUM("packet_format", IIO_SHARED_BY_ALL, &ad4858_packet_fmt), > + IIO_ENUM_AVAILABLE("packet_format", > + IIO_SHARED_BY_ALL, &ad4858_packet_fmt), > + {}, > +}; > + > +static struct iio_chan_spec_ext_info ad4857_ext_info[] = { > + IIO_ENUM("packet_format", IIO_SHARED_BY_ALL, &ad4857_packet_fmt), > + IIO_ENUM_AVAILABLE("packet_format", > + IIO_SHARED_BY_ALL, &ad4857_packet_fmt), > + {}, I think Andy pointed these out. No trailing comma on terminators. However, I'm not sure this ABI is practical currently. Basically I have no idea what it is or how to use it! > +}; > + > +static const struct ad485x_chip_info ad4858i_info = { > + .name = "ad4858i", > + .product_id = AD4858I_PROD_ID, > + .scale_table = ad485x_scale_table, > + .num_scales = ARRAY_SIZE(ad485x_scale_table), > + .offset_table = ad4858_offset_table, > + .num_offset = ARRAY_SIZE(ad4858_offset_table), > + .channels = ad4858_channels, > + .num_channels = ARRAY_SIZE(ad4858_channels), > + .throughput = 1000000, Odd you use MEGA above, but not here. > + .resolution = 20, > +}; > +static int ad485x_probe(struct spi_device *spi) > +{ > + struct iio_dev *indio_dev; > + struct ad485x_state *st; > + int ret; > + > + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); > + if (!indio_dev) > + return -ENOMEM; > + > + st = iio_priv(indio_dev); > + st->spi = spi; > + > + mutex_init(&st->lock); Ever so slight preference for the new option of. ret = devm_mutex_init(dev, &st->lock); if (ret) return ret; ... > +} > + > +static const struct of_device_id ad485x_of_match[] = { > + { .compatible = "adi,ad4858", .data = &ad4858_info, }, > + { .compatible = "adi,ad4857", .data = &ad4857_info, }, > + { .compatible = "adi,ad4856", .data = &ad4856_info, }, > + { .compatible = "adi,ad4855", .data = &ad4855_info, }, > + { .compatible = "adi,ad4854", .data = &ad4854_info, }, > + { .compatible = "adi,ad4853", .data = &ad4853_info, }, > + { .compatible = "adi,ad4852", .data = &ad4852_info, }, > + { .compatible = "adi,ad4851", .data = &ad4851_info, }, > + { .compatible = "adi,ad4858i", .data = &ad4858i_info, }, > + {} { } > +}; > +
On Sat, Sep 28, 2024 at 8:30 PM Jonathan Cameron <jic23@kernel.org> wrote: ... > > We have quite a few parts in the pipline right like this one that have > > per-sample status bits. In the past, these were generally handled with > > IIO events, but this doesn't really work for these high-speed backends > > since the data is being piped directly to DMA and we don't look at > > each sample in the ADC driver. So it would be worthwhile to try to > > find some general solution here for handling this sort of thing. > > We have previously talked about schemes to describe metadata > alongside channels. I guess maybe it's time to actually look at how > that works. I'm not sure dynamic control of that metadata > is going to be easy to do though or if we even want to > (as opposed to always on or off for a particular device). Time for the kernel to return JSON on a per channel basis? Just saying :-)
On Sat, 2024-09-28 at 18:30 +0100, Jonathan Cameron wrote: > > > > > > +static struct iio_chan_spec_ext_info ad4858_ext_info[] = { > > > + IIO_ENUM("packet_format", IIO_SHARED_BY_ALL, &ad4858_packet_fmt), > > > + IIO_ENUM_AVAILABLE("packet_format", > > > + IIO_SHARED_BY_ALL, &ad4858_packet_fmt), > > > + {}, > > > +}; > > > + > > > +static struct iio_chan_spec_ext_info ad4857_ext_info[] = { > > > + IIO_ENUM("packet_format", IIO_SHARED_BY_ALL, &ad4857_packet_fmt), > > > + IIO_ENUM_AVAILABLE("packet_format", > > > + IIO_SHARED_BY_ALL, &ad4857_packet_fmt), > > > + {}, > > > +}; > > > > Usually, something like this packet format would be automatically > > selected when buffered reads are enabled based on what other features > > it provides are needed, i.e only enable the status bits when events > > are enabled. > > > > (For those that didn't read the datasheet, the different packet > > formats basically enable extra status bits per sample. And in the case > > of oversampling, one of the formats also selects a reduced number of > > sample bits.) > > > > We have quite a few parts in the pipline right like this one that have > > per-sample status bits. In the past, these were generally handled with > > IIO events, but this doesn't really work for these high-speed backends > > since the data is being piped directly to DMA and we don't look at > > each sample in the ADC driver. So it would be worthwhile to try to > > find some general solution here for handling this sort of thing. I did not read the datasheet that extensively but here it goes my 2 cents (basically my internal feedback on this one): So this packet format thingy may be a very "funny" discussion if we really need to support it. I'm not sure how useful it is the 32 bits format rather than being used in test pattern. I'm not seeing too much benefit on the channel id or span id information (we can already get that info with other attributes). The OR/UR is the one that could be more useful but is there someone using it? Do we really need to have it close to the sample? If not, there's the status register and... Also, I think this can be implemented using IIO events (likely what we will be asked). So what comes to mind could be: For test_pattern (could be implemented as ext_info or an additional channel I think - not for now I guess) we can easily look at our word side and dynamically set the proper packet size. So, to me, this is effectively the only place where 32bits would make sense (assuming we don't implement OR/UR for now). For oversampling we can have both 20/24 bit averaged data. But from the datasheet: "Oversampling is useful in applications requiring lower noise and higher dynamic range per output data-word, which the AD4858 supports with 24-bit output resolution and reduced average output data rates" So from the above it looks like it could make sense to default to 24bit packets if oversampling is enabled. Now the question is OR/UR. If that is something we can support with events, we could see when one of OR/UR is being requested to either enable 24 packets (no oversampling) or 32 bit packets (oversampling on). > > We have previously talked about schemes to describe metadata > alongside channels. I guess maybe it's time to actually look at how > that works. I'm not sure dynamic control of that metadata > is going to be easy to do though or if we even want to > (as opposed to always on or off for a particular device). > Indeed this is something we have been discussing and the ability to have status alongside a buffered samples is starting to be requested more and more. Some parts do have the status bit alongside the sample (meaning in the same register read) which means it basically goes with the sample as part of it's storage_bits. While not ideal, an application caring about those bits still has access to the complete raw sample and can access them. It gets more complicated where the status (sometimes a per device status register) is located in another register. I guess we can have two case: 1) A device status which applies for all channels being sampled; 2) A per channel status (where the .metada approach could make sense). But I'm not sure how we could define something like this other than assuming that raw status data is being sent to userspace (given that every device has it's own custom status bits and quirks). - Nuno Sá
> > +static int ad485x_get_calibbias(struct ad485x_state *st, int ch, int *val, > > + int *val2) > > +{ > > + unsigned int lsb, mid, msb; > > + int ret; > > + > > + guard(mutex)(&st->lock); > > + > > + ret = regmap_read(st->regmap, AD485X_REG_CHX_OFFSET_MSB(ch), > > + &msb); > > + if (ret) > > + return ret; > > + > > + ret = regmap_read(st->regmap, AD485X_REG_CHX_OFFSET_MID(ch), > > + &mid); > > + if (ret) > > + return ret; > > + > > + ret = regmap_read(st->regmap, AD485X_REG_CHX_OFFSET_LSB(ch), > > + &lsb); > > + if (ret) > > + return ret; > > + > > + if (st->info->resolution == 16) { > > + *val = msb << 8; > > + *val |= mid; > > + *val = sign_extend32(*val, 15); > > + } else { > > + *val = msb << 12; > > + *val |= mid << 4; > > + *val |= lsb >> 4; > As below I'd use a byte array then you can do get_unaligned_be24 to > + a right shift by 4 of the whole thing. Regarding the bulk writes/reads, the msb/mid/lsb registers need to be read/write in a specific order and the addresses are not incremental, so I am not sure how the bulk functions fit. On this matter, we will need unsigned int (not u8) to store the values read via regmap_read, and in this case we will need extra casts and assignments to use get_unaligned. > > > + *val = sign_extend32(*val, 19); > > + } > > + > > + return IIO_VAL_INT; > > +} ...
On Tue, Oct 01, 2024 at 11:53:18AM +0000, Miclaus, Antoniu wrote: > Regarding the bulk writes/reads, the msb/mid/lsb registers need to be > read/write in a specific order and the addresses are not incremental, We have _noinc() variants of regmap accessors. > so I am not sure how the bulk functions fit. On this matter, we will need > unsigned int (not u8) to store the values read via regmap_read, and in this > case we will need extra casts and assignments to use get_unaligned.
> > Regarding the bulk writes/reads, the msb/mid/lsb registers need to be > > read/write in a specific order and the addresses are not incremental, > > We have _noinc() variants of regmap accessors. [Miclaus, Antoniu] I think _noinc() functions read from the same register address so it doesn't apply. I am reading values from multiple register addresses that are not reg_addr, reg_addr+1, reg_addr+2. > > so I am not sure how the bulk functions fit. On this matter, we will need > > unsigned int (not u8) to store the values read via regmap_read, and in this > > case we will need extra casts and assignments to use get_unaligned. > > -- > With Best Regards, > Andy Shevchenko >
On 10/1/24 8:51 AM, Miclaus, Antoniu wrote: >>> Regarding the bulk writes/reads, the msb/mid/lsb registers need to be >>> read/write in a specific order and the addresses are not incremental, >> >> We have _noinc() variants of regmap accessors. > [Miclaus, Antoniu] > I think _noinc() functions read from the same register address so it doesn't > apply. > I am reading values from multiple register addresses that are not reg_addr, > reg_addr+1, reg_addr+2. I'm confused by the statement that the registers are not incremental. For example, this patch defines... +#define AD485X_REG_CHX_OFFSET_LSB(ch) AD485X_REG_CHX_OFFSET(ch) +#define AD485X_REG_CHX_OFFSET_MID(ch) (AD485X_REG_CHX_OFFSET_LSB(ch) + 0x01) +#define AD485X_REG_CHX_OFFSET_MSB(ch) (AD485X_REG_CHX_OFFSET_MID(ch) + 0x01) This looks exactly like reg_addr, reg_addr+1, reg_addr+2 to me. > >>> so I am not sure how the bulk functions fit. On this matter, we will need >>> unsigned int (not u8) to store the values read via regmap_read, and in this >>> case we will need extra casts and assignments to use get_unaligned. >> >> -- >> With Best Regards, >> Andy Shevchenko >> >
> On 10/1/24 8:51 AM, Miclaus, Antoniu wrote: > >>> Regarding the bulk writes/reads, the msb/mid/lsb registers need to be > >>> read/write in a specific order and the addresses are not incremental, > >> > >> We have _noinc() variants of regmap accessors. > > [Miclaus, Antoniu] > > I think _noinc() functions read from the same register address so it doesn't > > apply. > > I am reading values from multiple register addresses that are not reg_addr, > > reg_addr+1, reg_addr+2. > > I'm confused by the statement that the registers are not incremental. > > For example, this patch defines... > > +#define AD485X_REG_CHX_OFFSET_LSB(ch) > AD485X_REG_CHX_OFFSET(ch) > +#define AD485X_REG_CHX_OFFSET_MID(ch) > (AD485X_REG_CHX_OFFSET_LSB(ch) + 0x01) > +#define AD485X_REG_CHX_OFFSET_MSB(ch) > (AD485X_REG_CHX_OFFSET_MID(ch) + 0x01) > > This looks exactly like reg_addr, reg_addr+1, reg_addr+2 to me. Yes you are right. Although I tested with hardware and it seems that the registers are not properly written when using bulk operations. My guess is that holding CS low during the entire transaction might be a possible issue. Any suggestions are appreciated. > > > >>> so I am not sure how the bulk functions fit. On this matter, we will need > >>> unsigned int (not u8) to store the values read via regmap_read, and in this > >>> case we will need extra casts and assignments to use get_unaligned. > >> > >> -- > >> With Best Regards, > >> Andy Shevchenko > >> > >
On Thu, Oct 03, 2024 at 10:14:57AM +0000, Miclaus, Antoniu wrote: > > On 10/1/24 8:51 AM, Miclaus, Antoniu wrote: > > >>> Regarding the bulk writes/reads, the msb/mid/lsb registers need to be > > >>> read/write in a specific order and the addresses are not incremental, > > >> > > >> We have _noinc() variants of regmap accessors. > > > [Miclaus, Antoniu] > > > I think _noinc() functions read from the same register address so it doesn't > > > apply. > > > I am reading values from multiple register addresses that are not reg_addr, > > > reg_addr+1, reg_addr+2. > > > > I'm confused by the statement that the registers are not incremental. > > > > For example, this patch defines... > > > > +#define AD485X_REG_CHX_OFFSET_LSB(ch) > > AD485X_REG_CHX_OFFSET(ch) > > +#define AD485X_REG_CHX_OFFSET_MID(ch) > > (AD485X_REG_CHX_OFFSET_LSB(ch) + 0x01) > > +#define AD485X_REG_CHX_OFFSET_MSB(ch) > > (AD485X_REG_CHX_OFFSET_MID(ch) + 0x01) > > > > This looks exactly like reg_addr, reg_addr+1, reg_addr+2 to me. > Yes you are right. Although I tested with hardware and it seems that the registers > are not properly written when using bulk operations. My guess is that holding CS low during > the entire transaction might be a possible issue. Any suggestions are appreciated. Okay, so each byte has to be written as a separate SPI transfer? I believe we have already examples of the drivers for such a hardware in the Linux kernel, but I can't throw any example form top of my head. > > >>> so I am not sure how the bulk functions fit. On this matter, we will need > > >>> unsigned int (not u8) to store the values read via regmap_read, and in this > > >>> case we will need extra casts and assignments to use get_unaligned.
On 10/3/24 5:14 AM, Miclaus, Antoniu wrote: >> On 10/1/24 8:51 AM, Miclaus, Antoniu wrote: >>>>> Regarding the bulk writes/reads, the msb/mid/lsb registers need to be >>>>> read/write in a specific order and the addresses are not incremental, >>>> >>>> We have _noinc() variants of regmap accessors. >>> [Miclaus, Antoniu] >>> I think _noinc() functions read from the same register address so it doesn't >>> apply. >>> I am reading values from multiple register addresses that are not reg_addr, >>> reg_addr+1, reg_addr+2. >> >> I'm confused by the statement that the registers are not incremental. >> >> For example, this patch defines... >> >> +#define AD485X_REG_CHX_OFFSET_LSB(ch) >> AD485X_REG_CHX_OFFSET(ch) >> +#define AD485X_REG_CHX_OFFSET_MID(ch) >> (AD485X_REG_CHX_OFFSET_LSB(ch) + 0x01) >> +#define AD485X_REG_CHX_OFFSET_MSB(ch) >> (AD485X_REG_CHX_OFFSET_MID(ch) + 0x01) >> >> This looks exactly like reg_addr, reg_addr+1, reg_addr+2 to me. > Yes you are right. Although I tested with hardware and it seems that the registers > are not properly written when using bulk operations. My guess is that holding CS low during > the entire transaction might be a possible issue. Any suggestions are appreciated. Is ADDR_DIR in SPI_CONFIG_A set to the correct value to match how the regmap is configured for bulk writes? I had to set this bit for AD4695 which has a similar register map (although on that one I used two regmaps, an 8-bit one and a 16-bit one, instead of doing bulk operations on the 8-bit one). > >>> >>>>> so I am not sure how the bulk functions fit. On this matter, we will need >>>>> unsigned int (not u8) to store the values read via regmap_read, and in this >>>>> case we will need extra casts and assignments to use get_unaligned. >>>> >>>> -- >>>> With Best Regards, >>>> Andy Shevchenko >>>> >>> >
> On 10/3/24 5:14 AM, Miclaus, Antoniu wrote: > >> On 10/1/24 8:51 AM, Miclaus, Antoniu wrote: > >>>>> Regarding the bulk writes/reads, the msb/mid/lsb registers need to be > >>>>> read/write in a specific order and the addresses are not incremental, > >>>> > >>>> We have _noinc() variants of regmap accessors. > >>> [Miclaus, Antoniu] > >>> I think _noinc() functions read from the same register address so it doesn't > >>> apply. > >>> I am reading values from multiple register addresses that are not reg_addr, > >>> reg_addr+1, reg_addr+2. > >> > >> I'm confused by the statement that the registers are not incremental. > >> > >> For example, this patch defines... > >> > >> +#define AD485X_REG_CHX_OFFSET_LSB(ch) > >> AD485X_REG_CHX_OFFSET(ch) > >> +#define AD485X_REG_CHX_OFFSET_MID(ch) > >> (AD485X_REG_CHX_OFFSET_LSB(ch) + 0x01) > >> +#define AD485X_REG_CHX_OFFSET_MSB(ch) > >> (AD485X_REG_CHX_OFFSET_MID(ch) + 0x01) > >> > >> This looks exactly like reg_addr, reg_addr+1, reg_addr+2 to me. > > Yes you are right. Although I tested with hardware and it seems that the > registers > > are not properly written when using bulk operations. My guess is that > holding CS low during > > the entire transaction might be a possible issue. Any suggestions are > appreciated. > > Is ADDR_DIR in SPI_CONFIG_A set to the correct value to match how > the regmap is configured for bulk writes? > > I had to set this bit for AD4695 which has a similar register map > (although on that one I used two regmaps, an 8-bit one and a 16-bit > one, instead of doing bulk operations on the 8-bit one). > Thanks for the input! I tried your suggested approach: set the ADDR_DIR to 1 during probe. Unfortunately, this did not fix the issue. I am still not able to perform bulk writes properly to the device. For now I will keep the only working version in v2, since there will be most probably other iterations of the this patch series
On Mon, 30 Sep 2024 09:05:04 +0200 Nuno Sá <noname.nuno@gmail.com> wrote: > On Sat, 2024-09-28 at 18:30 +0100, Jonathan Cameron wrote: > > > > > > > > > +static struct iio_chan_spec_ext_info ad4858_ext_info[] = { > > > > + IIO_ENUM("packet_format", IIO_SHARED_BY_ALL, &ad4858_packet_fmt), > > > > + IIO_ENUM_AVAILABLE("packet_format", > > > > + IIO_SHARED_BY_ALL, &ad4858_packet_fmt), > > > > + {}, > > > > +}; > > > > + > > > > +static struct iio_chan_spec_ext_info ad4857_ext_info[] = { > > > > + IIO_ENUM("packet_format", IIO_SHARED_BY_ALL, &ad4857_packet_fmt), > > > > + IIO_ENUM_AVAILABLE("packet_format", > > > > + IIO_SHARED_BY_ALL, &ad4857_packet_fmt), > > > > + {}, > > > > +}; > > > > > > Usually, something like this packet format would be automatically > > > selected when buffered reads are enabled based on what other features > > > it provides are needed, i.e only enable the status bits when events > > > are enabled. > > > > > > (For those that didn't read the datasheet, the different packet > > > formats basically enable extra status bits per sample. And in the case > > > of oversampling, one of the formats also selects a reduced number of > > > sample bits.) > > > > > > We have quite a few parts in the pipline right like this one that have > > > per-sample status bits. In the past, these were generally handled with > > > IIO events, but this doesn't really work for these high-speed backends > > > since the data is being piped directly to DMA and we don't look at > > > each sample in the ADC driver. So it would be worthwhile to try to > > > find some general solution here for handling this sort of thing. > > I did not read the datasheet that extensively but here it goes my 2 cents > (basically my internal feedback on this one): > > So this packet format thingy may be a very "funny" discussion if we really need > to support it. I'm not sure how useful it is the 32 bits format rather than > being used in test pattern. I'm not seeing too much benefit on the channel id or > span id information (we can already get that info with other attributes). The > OR/UR is the one that could be more useful but is there someone using it? Do we > really need to have it close to the sample? If not, there's the status register > and... Also, I think this can be implemented using IIO events (likely what we > will be asked). So what comes to mind could be: Definite preference for using events, but for a device doing DMA I'm not sure how we can do that without requiring parsing all the data. So we would need some metadata description to know it is there. > > For test_pattern (could be implemented as ext_info or an additional channel I > think - not for now I guess) we can easily look at our word side and dynamically > set the proper packet size. So, to me, this is effectively the only place where > 32bits would make sense (assuming we don't implement OR/UR for now). > For oversampling we can have both 20/24 bit averaged data. But from the > datasheet: > > "Oversampling is useful in applications requiring lower noise and higher dynamic > range per output data-word, which the AD4858 supports with 24-bit output > resolution and reduced average output data rates" > > So from the above it looks like it could make sense to default to 24bit packets > if oversampling is enabled. That sounds like what we do for the DMA oversampling cases that change the resolutions. > > Now the question is OR/UR. If that is something we can support with events, we > could see when one of OR/UR is being requested to either enable 24 packets (no > oversampling) or 32 bit packets (oversampling on). > > > > > > > We have previously talked about schemes to describe metadata > > alongside channels. I guess maybe it's time to actually look at how > > that works. I'm not sure dynamic control of that metadata > > is going to be easy to do though or if we even want to > > (as opposed to always on or off for a particular device). > > > > Indeed this is something we have been discussing and the ability to have status > alongside a buffered samples is starting to be requested more and more. Some > parts do have the status bit alongside the sample (meaning in the same register > read) which means it basically goes with the sample as part of it's > storage_bits. While not ideal, an application caring about those bits still has > access to the complete raw sample and can access them. This has the advantage that if we come along later and define a metadata in storage bits description it is backwards compatible. We've been doing this for years with some devices. > It gets more complicated > where the status (sometimes a per device status register) is located in another > register. I guess we can have two case: > > 1) A device status which applies for all channels being sampled; > 2) A per channel status (where the .metada approach could make sense). If it's a separate register per channel and optional, we'd have to treat it as a metadata channel as no guarantee it would be packed next to the main channel. If we have a description of metadata additions in main channel storage, I'm not against having a IIO_METADATA channel type. If it's a single channel I'm not sure how we'd make as channel description general enough easily as we end up with every field possibly needed an association with a different channel. > > But I'm not sure how we could define something like this other than assuming > that raw status data is being sent to userspace (given that every device has > it's own custom status bits and quirks). That is always fine. Jonathan > > - Nuno Sá
On Fri, 4 Oct 2024 14:07:37 +0000 "Miclaus, Antoniu" <Antoniu.Miclaus@analog.com> wrote: > > On 10/3/24 5:14 AM, Miclaus, Antoniu wrote: > > >> On 10/1/24 8:51 AM, Miclaus, Antoniu wrote: > > >>>>> Regarding the bulk writes/reads, the msb/mid/lsb registers need to be > > >>>>> read/write in a specific order and the addresses are not incremental, > > >>>> > > >>>> We have _noinc() variants of regmap accessors. > > >>> [Miclaus, Antoniu] > > >>> I think _noinc() functions read from the same register address so it doesn't > > >>> apply. > > >>> I am reading values from multiple register addresses that are not reg_addr, > > >>> reg_addr+1, reg_addr+2. > > >> > > >> I'm confused by the statement that the registers are not incremental. > > >> > > >> For example, this patch defines... > > >> > > >> +#define AD485X_REG_CHX_OFFSET_LSB(ch) > > >> AD485X_REG_CHX_OFFSET(ch) > > >> +#define AD485X_REG_CHX_OFFSET_MID(ch) > > >> (AD485X_REG_CHX_OFFSET_LSB(ch) + 0x01) > > >> +#define AD485X_REG_CHX_OFFSET_MSB(ch) > > >> (AD485X_REG_CHX_OFFSET_MID(ch) + 0x01) > > >> > > >> This looks exactly like reg_addr, reg_addr+1, reg_addr+2 to me. > > > Yes you are right. Although I tested with hardware and it seems that the > > registers > > > are not properly written when using bulk operations. My guess is that > > holding CS low during > > > the entire transaction might be a possible issue. Any suggestions are > > appreciated. > > > > Is ADDR_DIR in SPI_CONFIG_A set to the correct value to match how > > the regmap is configured for bulk writes? > > > > I had to set this bit for AD4695 which has a similar register map > > (although on that one I used two regmaps, an 8-bit one and a 16-bit > > one, instead of doing bulk operations on the 8-bit one). > > > Thanks for the input! I tried your suggested approach: set the ADDR_DIR > to 1 during probe. Unfortunately, this did not fix the issue. I am still not able > to perform bulk writes properly to the device. > > For now I will keep the only working version in v2, since there will be > most probably other iterations of the this patch series
On Sat, 2024-10-05 at 18:26 +0100, Jonathan Cameron wrote: > On Mon, 30 Sep 2024 09:05:04 +0200 > Nuno Sá <noname.nuno@gmail.com> wrote: > > > On Sat, 2024-09-28 at 18:30 +0100, Jonathan Cameron wrote: > > > > > > > > > > > > +static struct iio_chan_spec_ext_info ad4858_ext_info[] = { > > > > > + IIO_ENUM("packet_format", IIO_SHARED_BY_ALL, > > > > > &ad4858_packet_fmt), > > > > > + IIO_ENUM_AVAILABLE("packet_format", > > > > > + IIO_SHARED_BY_ALL, &ad4858_packet_fmt), > > > > > + {}, > > > > > +}; > > > > > + > > > > > +static struct iio_chan_spec_ext_info ad4857_ext_info[] = { > > > > > + IIO_ENUM("packet_format", IIO_SHARED_BY_ALL, > > > > > &ad4857_packet_fmt), > > > > > + IIO_ENUM_AVAILABLE("packet_format", > > > > > + IIO_SHARED_BY_ALL, &ad4857_packet_fmt), > > > > > + {}, > > > > > +}; > > > > > > > > Usually, something like this packet format would be automatically > > > > selected when buffered reads are enabled based on what other features > > > > it provides are needed, i.e only enable the status bits when events > > > > are enabled. > > > > > > > > (For those that didn't read the datasheet, the different packet > > > > formats basically enable extra status bits per sample. And in the case > > > > of oversampling, one of the formats also selects a reduced number of > > > > sample bits.) > > > > > > > > We have quite a few parts in the pipline right like this one that have > > > > per-sample status bits. In the past, these were generally handled with > > > > IIO events, but this doesn't really work for these high-speed backends > > > > since the data is being piped directly to DMA and we don't look at > > > > each sample in the ADC driver. So it would be worthwhile to try to > > > > find some general solution here for handling this sort of thing. > > > > I did not read the datasheet that extensively but here it goes my 2 cents > > (basically my internal feedback on this one): > > > > So this packet format thingy may be a very "funny" discussion if we really > > need > > to support it. I'm not sure how useful it is the 32 bits format rather than > > being used in test pattern. I'm not seeing too much benefit on the channel > > id or > > span id information (we can already get that info with other attributes). > > The > > OR/UR is the one that could be more useful but is there someone using it? Do > > we > > really need to have it close to the sample? If not, there's the status > > register > > and... Also, I think this can be implemented using IIO events (likely what > > we > > will be asked). So what comes to mind could be: > > Definite preference for using events, but for a device doing DMA I'm not sure > how we can do that without requiring parsing all the data. > > So we would need some metadata description to know it is there. > > > > > For test_pattern (could be implemented as ext_info or an additional channel > > I > > think - not for now I guess) we can easily look at our word side and > > dynamically > > set the proper packet size. So, to me, this is effectively the only place > > where > > 32bits would make sense (assuming we don't implement OR/UR for now). > > For oversampling we can have both 20/24 bit averaged data. But from the > > datasheet: > > > > "Oversampling is useful in applications requiring lower noise and higher > > dynamic > > range per output data-word, which the AD4858 supports with 24-bit output > > resolution and reduced average output data rates" > > > > So from the above it looks like it could make sense to default to 24bit > > packets > > if oversampling is enabled. > > That sounds like what we do for the DMA oversampling cases that change > the resolutions. > > > > > Now the question is OR/UR. If that is something we can support with events, > > we > > could see when one of OR/UR is being requested to either enable 24 packets > > (no > > oversampling) or 32 bit packets (oversampling on). > > > > > > > > > > > > We have previously talked about schemes to describe metadata > > > alongside channels. I guess maybe it's time to actually look at how > > > that works. I'm not sure dynamic control of that metadata > > > is going to be easy to do though or if we even want to > > > (as opposed to always on or off for a particular device). > > > > > > > Indeed this is something we have been discussing and the ability to have > > status > > alongside a buffered samples is starting to be requested more and more. Some > > parts do have the status bit alongside the sample (meaning in the same > > register > > read) which means it basically goes with the sample as part of it's > > storage_bits. While not ideal, an application caring about those bits still > > has > > access to the complete raw sample and can access them. > > This has the advantage that if we come along later and define a metadata > in storage bits description it is backwards compatible. We've been doing > this for years with some devices. > > > It gets more complicated > > where the status (sometimes a per device status register) is located in > > another > > register. I guess we can have two case: > > > > 1) A device status which applies for all channels being sampled; > > 2) A per channel status (where the .metada approach could make sense). > > If it's a separate register per channel and optional, we'd have to treat it as > a metadata > channel as no guarantee it would be packed next to the main channel. > > If we have a description of metadata additions in main channel storage, I'm > not > against having a IIO_METADATA channel type. > I guess you mean that a complete solution would never only be a IIO_METADATA but also extending 'struct iio_scan_type'? > If it's a single channel I'm not sure how we'd make as channel description > general enough easily as we end up with every field possibly needed an > association > with a different channel. Not sure I followed the above... You mean having a single channel (like one register) pointing to different channels? What I mean with 1) is for example what happens with IMUs (eg: adis16475). They have a DIAG_STAT register (which is pretty much device wide status/error information) that's also part of burst transfers (where we get our samples) and while some bits might not be meaningful for the sampling "session", others might make sense to reason about data integrity or correctness. For the above case, I think the IIO_METADATA channel type would fit. - Nuno Sá
On Fri, 11 Oct 2024 12:23:49 +0200 Nuno Sá <noname.nuno@gmail.com> wrote: > On Sat, 2024-10-05 at 18:26 +0100, Jonathan Cameron wrote: > > On Mon, 30 Sep 2024 09:05:04 +0200 > > Nuno Sá <noname.nuno@gmail.com> wrote: > > > > > On Sat, 2024-09-28 at 18:30 +0100, Jonathan Cameron wrote: > > > > > > > > > > > > > > > +static struct iio_chan_spec_ext_info ad4858_ext_info[] = { > > > > > > + IIO_ENUM("packet_format", IIO_SHARED_BY_ALL, > > > > > > &ad4858_packet_fmt), > > > > > > + IIO_ENUM_AVAILABLE("packet_format", > > > > > > + IIO_SHARED_BY_ALL, &ad4858_packet_fmt), > > > > > > + {}, > > > > > > +}; > > > > > > + > > > > > > +static struct iio_chan_spec_ext_info ad4857_ext_info[] = { > > > > > > + IIO_ENUM("packet_format", IIO_SHARED_BY_ALL, > > > > > > &ad4857_packet_fmt), > > > > > > + IIO_ENUM_AVAILABLE("packet_format", > > > > > > + IIO_SHARED_BY_ALL, &ad4857_packet_fmt), > > > > > > + {}, > > > > > > +}; > > > > > > > > > > Usually, something like this packet format would be automatically > > > > > selected when buffered reads are enabled based on what other features > > > > > it provides are needed, i.e only enable the status bits when events > > > > > are enabled. > > > > > > > > > > (For those that didn't read the datasheet, the different packet > > > > > formats basically enable extra status bits per sample. And in the case > > > > > of oversampling, one of the formats also selects a reduced number of > > > > > sample bits.) > > > > > > > > > > We have quite a few parts in the pipline right like this one that have > > > > > per-sample status bits. In the past, these were generally handled with > > > > > IIO events, but this doesn't really work for these high-speed backends > > > > > since the data is being piped directly to DMA and we don't look at > > > > > each sample in the ADC driver. So it would be worthwhile to try to > > > > > find some general solution here for handling this sort of thing. > > > > > > I did not read the datasheet that extensively but here it goes my 2 cents > > > (basically my internal feedback on this one): > > > > > > So this packet format thingy may be a very "funny" discussion if we really > > > need > > > to support it. I'm not sure how useful it is the 32 bits format rather than > > > being used in test pattern. I'm not seeing too much benefit on the channel > > > id or > > > span id information (we can already get that info with other attributes). > > > The > > > OR/UR is the one that could be more useful but is there someone using it? Do > > > we > > > really need to have it close to the sample? If not, there's the status > > > register > > > and... Also, I think this can be implemented using IIO events (likely what > > > we > > > will be asked). So what comes to mind could be: > > > > Definite preference for using events, but for a device doing DMA I'm not sure > > how we can do that without requiring parsing all the data. > > > > So we would need some metadata description to know it is there. > > > > > > > > For test_pattern (could be implemented as ext_info or an additional channel > > > I > > > think - not for now I guess) we can easily look at our word side and > > > dynamically > > > set the proper packet size. So, to me, this is effectively the only place > > > where > > > 32bits would make sense (assuming we don't implement OR/UR for now). > > > For oversampling we can have both 20/24 bit averaged data. But from the > > > datasheet: > > > > > > "Oversampling is useful in applications requiring lower noise and higher > > > dynamic > > > range per output data-word, which the AD4858 supports with 24-bit output > > > resolution and reduced average output data rates" > > > > > > So from the above it looks like it could make sense to default to 24bit > > > packets > > > if oversampling is enabled. > > > > That sounds like what we do for the DMA oversampling cases that change > > the resolutions. > > > > > > > > Now the question is OR/UR. If that is something we can support with events, > > > we > > > could see when one of OR/UR is being requested to either enable 24 packets > > > (no > > > oversampling) or 32 bit packets (oversampling on). > > > > > > > > > > > > > > > > > We have previously talked about schemes to describe metadata > > > > alongside channels. I guess maybe it's time to actually look at how > > > > that works. I'm not sure dynamic control of that metadata > > > > is going to be easy to do though or if we even want to > > > > (as opposed to always on or off for a particular device). > > > > > > > > > > Indeed this is something we have been discussing and the ability to have > > > status > > > alongside a buffered samples is starting to be requested more and more. Some > > > parts do have the status bit alongside the sample (meaning in the same > > > register > > > read) which means it basically goes with the sample as part of it's > > > storage_bits. While not ideal, an application caring about those bits still > > > has > > > access to the complete raw sample and can access them. > > > > This has the advantage that if we come along later and define a metadata > > in storage bits description it is backwards compatible. We've been doing > > this for years with some devices. > > > > > It gets more complicated > > > where the status (sometimes a per device status register) is located in > > > another > > > register. I guess we can have two case: > > > > > > 1) A device status which applies for all channels being sampled; > > > 2) A per channel status (where the .metada approach could make sense). > > > > If it's a separate register per channel and optional, we'd have to treat it as > > a metadata > > channel as no guarantee it would be packed next to the main channel. > > > > If we have a description of metadata additions in main channel storage, I'm > > not > > against having a IIO_METADATA channel type. > > > > I guess you mean that a complete solution would never only be a IIO_METADATA but > also extending 'struct iio_scan_type'? Yes we needs a way to refer to an existing scan element then we could add additional channels and refer into them as needed. > > > If it's a single channel I'm not sure how we'd make as channel description > > general enough easily as we end up with every field possibly needed an > > association > > with a different channel. > > Not sure I followed the above... You mean having a single channel (like one > register) pointing to different channels? Both way's around occur. Multiple channels, some normal, some metadata some separate pointing to a single storage location and per channel meta data for different 'signals' shoved in one register. > > What I mean with 1) is for example what happens with IMUs (eg: adis16475). They > have a DIAG_STAT register (which is pretty much device wide status/error > information) that's also part of burst transfers (where we get our samples) and > while some bits might not be meaningful for the sampling "session", others might > make sense to reason about data integrity or correctness. > > For the above case, I think the IIO_METADATA channel type would fit. That one is easy. Fiddly case is where we have say overflow bits for multiple signals (i.e. pins) in a single dmabuffer element. To make that work cleanly we need a way to not only describe the contents but cross reference it to the related data. We've discussed ways to group actual channel (i.e. current and power on same pin) in the past but doing this for metadata that is packed in multiple channels is going to be a real pain. Basically it all needs to be very flexible and any attempt to support a subset is likely to wall us into an inflexible ABI. Jonathan > > - Nuno Sá
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index f60fe85a30d5..83f55229d731 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -36,6 +36,18 @@ config AD4130 To compile this driver as a module, choose M here: the module will be called ad4130. +config AD485X + tristate "Analog Device AD485x DAS Driver" + depends on SPI + select REGMAP_SPI + select IIO_BACKEND + help + Say yes here to build support for Analog Devices AD485x high speed + data acquisition system (DAS). + + To compile this driver as a module, choose M here: the module will be + called ad485x. + config AD7091R tristate diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile index d370e066544e..26c1670c8913 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -7,6 +7,7 @@ obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o obj-$(CONFIG_AD4130) += ad4130.o +obj-$(CONFIG_AD485X) += ad485x.o obj-$(CONFIG_AD7091R) += ad7091r-base.o obj-$(CONFIG_AD7091R5) += ad7091r5.o obj-$(CONFIG_AD7091R8) += ad7091r8.o diff --git a/drivers/iio/adc/ad485x.c b/drivers/iio/adc/ad485x.c new file mode 100644 index 000000000000..3333cad9ed8f --- /dev/null +++ b/drivers/iio/adc/ad485x.c @@ -0,0 +1,1061 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Analog Devices AD485x DAS driver + * + * Copyright 2024 Analog Devices Inc. + */ + +#include <asm/unaligned.h> +#include <linux/delay.h> +#include <linux/iio/backend.h> +#include <linux/iio/iio.h> +#include <linux/pwm.h> +#include <linux/regmap.h> +#include <linux/regulator/consumer.h> +#include <linux/spi/spi.h> +#include <linux/units.h> + +#define AD485X_REG_INTERFACE_CONFIG_A 0x00 +#define AD485X_REG_INTERFACE_CONFIG_B 0x01 +#define AD485X_REG_PRODUCT_ID_L 0x04 +#define AD485X_REG_PRODUCT_ID_H 0x05 +#define AD485X_REG_DEVICE_CTRL 0x25 +#define AD485X_REG_PACKET 0x26 + +#define AD485X_REG_CH_CONFIG_BASE 0x2A +#define AD485X_REG_CHX_SOFTSPAN(ch) ((0x12 * (ch)) + AD485X_REG_CH_CONFIG_BASE) +#define AD485X_REG_CHX_OFFSET(ch) (AD485X_REG_CHX_SOFTSPAN(ch) + 0x01) +#define AD485X_REG_CHX_OFFSET_LSB(ch) AD485X_REG_CHX_OFFSET(ch) +#define AD485X_REG_CHX_OFFSET_MID(ch) (AD485X_REG_CHX_OFFSET_LSB(ch) + 0x01) +#define AD485X_REG_CHX_OFFSET_MSB(ch) (AD485X_REG_CHX_OFFSET_MID(ch) + 0x01) +#define AD485X_REG_CHX_GAIN(ch) (AD485X_REG_CHX_OFFSET(ch) + 0x03) +#define AD485X_REG_CHX_GAIN_LSB(ch) AD485X_REG_CHX_GAIN(ch) +#define AD485X_REG_CHX_GAIN_MSB(ch) (AD485X_REG_CHX_GAIN(ch) + 0x01) +#define AD485X_REG_CHX_PHASE(ch) (AD485X_REG_CHX_GAIN(ch) + 0x02) +#define AD485X_REG_CHX_PHASE_LSB(ch) AD485X_REG_CHX_PHASE(ch) +#define AD485X_REG_CHX_PHASE_MSB(ch) (AD485X_REG_CHX_PHASE_LSB(ch) + 0x01) + +#define AD485X_REG_TESTPAT_0(c) (0x38 + (c) * 0x12) +#define AD485X_REG_TESTPAT_1(c) (0x39 + (c) * 0x12) +#define AD485X_REG_TESTPAT_2(c) (0x3A + (c) * 0x12) +#define AD485X_REG_TESTPAT_3(c) (0x3B + (c) * 0x12) + +#define AD485X_SW_RESET (BIT(7) | BIT(0)) +#define AD485X_SDO_ENABLE BIT(4) +#define AD485X_SINGLE_INSTRUCTION BIT(7) +#define AD485X_ECHO_CLOCK_MODE BIT(0) + +#define AD485X_PACKET_FORMAT_0 0 +#define AD485X_PACKET_FORMAT_1 1 +#define AD485X_PACKET_FORMAT_MASK GENMASK(1, 0) +#define AD485X_OS_EN BIT(7) + +#define AD485X_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 AD485X_TESTPAT_0_DEFAULT 0x2A +#define AD485X_TESTPAT_1_DEFAULT 0x3C +#define AD485X_TESTPAT_2_DEFAULT 0xCE +#define AD485X_TESTPAT_3_DEFAULT(c) (0x0A + (0x10 * (c))) + +#define AD485X_SOFTSPAN_0V_2V5 0 +#define AD485X_SOFTSPAN_N2V5_2V5 1 +#define AD485X_SOFTSPAN_0V_5V 2 +#define AD485X_SOFTSPAN_N5V_5V 3 +#define AD485X_SOFTSPAN_0V_6V25 4 +#define AD485X_SOFTSPAN_N6V25_6V25 5 +#define AD485X_SOFTSPAN_0V_10V 6 +#define AD485X_SOFTSPAN_N10V_10V 7 +#define AD485X_SOFTSPAN_0V_12V5 8 +#define AD485X_SOFTSPAN_N12V5_12V5 9 +#define AD485X_SOFTSPAN_0V_20V 10 +#define AD485X_SOFTSPAN_N20V_20V 11 +#define AD485X_SOFTSPAN_0V_25V 12 +#define AD485X_SOFTSPAN_N25V_25V 13 +#define AD485X_SOFTSPAN_0V_40V 14 +#define AD485X_SOFTSPAN_N40V_40V 15 + +#define AD485X_MAX_LANES 8 +#define AD485X_MAX_IODELAY 32 + +#define AD485X_T_CNVH_NS 40 + +#define AD4858_PROD_ID 0x60 +#define AD4857_PROD_ID 0x61 +#define AD4856_PROD_ID 0x62 +#define AD4855_PROD_ID 0x63 +#define AD4854_PROD_ID 0x64 +#define AD4853_PROD_ID 0x65 +#define AD4852_PROD_ID 0x66 +#define AD4851_PROD_ID 0x67 +#define AD4858I_PROD_ID 0x6F + +struct ad485x_chip_info { + const char *name; + unsigned int product_id; + const unsigned int (*scale_table)[2]; + int num_scales; + const int *offset_table; + int num_offset; + const struct iio_chan_spec *channels; + unsigned int num_channels; + unsigned long throughput; + unsigned int resolution; +}; + +struct ad485x_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 ad485x_chip_info *info; + unsigned long sampling_freq; + unsigned int (*scales)[2]; + int *offsets; +}; + +static int ad485x_reg_access(struct iio_dev *indio_dev, + unsigned int reg, + unsigned int writeval, + unsigned int *readval) +{ + struct ad485x_state *st = iio_priv(indio_dev); + + if (readval) + return regmap_read(st->regmap, reg, readval); + + return regmap_write(st->regmap, reg, writeval); +} + +static int ad485x_set_sampling_freq(struct ad485x_state *st, unsigned int freq) +{ + struct pwm_state cnv_state = { + .duty_cycle = AD485X_T_CNVH_NS, + .enabled = true, + }; + int ret; + + if (freq > st->info->throughput) + freq = st->info->throughput; + + cnv_state.period = DIV_ROUND_CLOSEST_ULL(1000000000, freq); + + ret = pwm_apply_might_sleep(st->cnv, &cnv_state); + if (ret) + return ret; + + st->sampling_freq = freq; + + return 0; +} + +static int ad485x_setup(struct ad485x_state *st) +{ + unsigned int product_id; + int ret; + + ret = ad485x_set_sampling_freq(st, 1000000); + if (ret) + return ret; + + ret = regmap_write(st->regmap, AD485X_REG_INTERFACE_CONFIG_A, + AD485X_SW_RESET); + if (ret) + return ret; + + ret = regmap_write(st->regmap, AD485X_REG_INTERFACE_CONFIG_B, + AD485X_SINGLE_INSTRUCTION); + if (ret) + return ret; + + ret = regmap_write(st->regmap, AD485X_REG_INTERFACE_CONFIG_A, + AD485X_SDO_ENABLE); + if (ret) + return ret; + + ret = regmap_read(st->regmap, AD485X_REG_PRODUCT_ID_L, &product_id); + if (ret) + return ret; + + if (product_id != st->info->product_id) + dev_warn(&st->spi->dev, "Unknown product ID: 0x%02X\n", + product_id); + + ret = regmap_write(st->regmap, AD485X_REG_DEVICE_CTRL, + AD485X_ECHO_CLOCK_MODE); + if (ret) + return ret; + + return regmap_write(st->regmap, AD485X_REG_PACKET, 0); +} + +static int ad485x_find_opt(bool *field, u32 size, u32 *ret_start) +{ + int i, cnt = 0, max_cnt = 0, start, max_start = 0; + + 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; + } + } + + if (cnt > max_cnt) { + max_cnt = cnt; + max_start = start; + } + + if (!max_cnt) + return -EIO; + + *ret_start = max_start; + + return max_cnt; +} + +static int ad485x_calibrate(struct ad485x_state *st) +{ + int opt_delay, lane_num, delay, i, s, c; + enum iio_backend_interface_type interface_type; + bool pn_status[AD485X_MAX_LANES][AD485X_MAX_IODELAY]; + int ret; + + ret = iio_backend_interface_type_get(st->back, &interface_type); + if (ret) + return ret; + + if (interface_type == IIO_BACKEND_INTERFACE_CMOS) + lane_num = st->info->num_channels; + else + lane_num = 1; + + if (st->info->resolution == 16) { + ret = iio_backend_data_size_set(st->back, 24); + if (ret) + return ret; + + ret = regmap_write(st->regmap, AD485X_REG_PACKET, + AD485X_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, AD485X_REG_PACKET, + AD485X_TEST_PAT | AD4858_PACKET_SIZE_32); + if (ret) + return ret; + } + + for (i = 0; i < st->info->num_channels; i++) { + ret = regmap_write(st->regmap, AD485X_REG_TESTPAT_0(i), + AD485X_TESTPAT_0_DEFAULT); + if (ret) + return ret; + + ret = regmap_write(st->regmap, AD485X_REG_TESTPAT_1(i), + AD485X_TESTPAT_1_DEFAULT); + if (ret) + return ret; + + ret = regmap_write(st->regmap, AD485X_REG_TESTPAT_2(i), + AD485X_TESTPAT_2_DEFAULT); + if (ret) + return ret; + + ret = regmap_write(st->regmap, AD485X_REG_TESTPAT_3(i), + AD485X_TESTPAT_3_DEFAULT(i)); + if (ret) + return ret; + + ret = iio_backend_chan_enable(st->back, i); + if (ret) + return ret; + } + + for (i = 0; i < lane_num; i++) { + for (delay = 0; delay < AD485X_MAX_IODELAY; delay++) { + ret = iio_backend_iodelay_set(st->back, i, delay); + if (ret) + return ret; + ret = iio_backend_chan_status(st->back, i, + &pn_status[i][delay]); + if (ret) + return ret; + } + } + + for (i = 0; i < lane_num; i++) { + c = ad485x_find_opt(&pn_status[i][0], AD485X_MAX_IODELAY, &s); + 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 < st->info->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, AD485X_REG_PACKET, 0); +} + +static const char *const ad4858_packet_fmts[] = { + "20-bit", "24-bit", "32-bit" +}; + +static const char *const ad4857_packet_fmts[] = { + "16-bit", "24-bit" +}; + +static int ad485x_set_packet_format(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, + unsigned int format) +{ + struct ad485x_state *st = iio_priv(indio_dev); + unsigned int val; + int ret; + + if (chan->scan_type.realbits == 20) + switch (format) { + case 0: + val = 20; + break; + case 1: + val = 24; + break; + case 2: + val = 32; + break; + default: + return -EINVAL; + } + else if (chan->scan_type.realbits == 16) + switch (format) { + case 0: + val = 16; + break; + case 1: + val = 24; + break; + default: + return -EINVAL; + } + else + return -EINVAL; + + ret = iio_backend_data_size_set(st->back, val); + if (ret) + return ret; + + return regmap_update_bits(st->regmap, AD485X_REG_PACKET, + AD485X_PACKET_FORMAT_MASK, format); +} + +static int ad485x_get_packet_format(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan) +{ + struct ad485x_state *st = iio_priv(indio_dev); + unsigned int format; + int ret; + + ret = regmap_read(st->regmap, AD485X_REG_PACKET, &format); + if (ret) + return ret; + + format = FIELD_GET(AD485X_PACKET_FORMAT_MASK, format); + + return format; +} + +static const struct iio_enum ad4858_packet_fmt = { + .items = ad4858_packet_fmts, + .num_items = ARRAY_SIZE(ad4858_packet_fmts), + .set = ad485x_set_packet_format, + .get = ad485x_get_packet_format, +}; + +static const struct iio_enum ad4857_packet_fmt = { + .items = ad4857_packet_fmts, + .num_items = ARRAY_SIZE(ad4857_packet_fmts), + .set = ad485x_set_packet_format, + .get = ad485x_get_packet_format, +}; + +static int ad485x_get_calibscale(struct ad485x_state *st, int ch, int *val, + int *val2) +{ + unsigned int reg_val; + int gain; + int ret; + + guard(mutex)(&st->lock); + + ret = regmap_read(st->regmap, AD485X_REG_CHX_GAIN_MSB(ch), + ®_val); + if (ret) + return ret; + + gain = (reg_val & 0xFF) << 8; + + ret = regmap_read(st->regmap, AD485X_REG_CHX_GAIN_LSB(ch), + ®_val); + if (ret) + return ret; + + gain |= reg_val & 0xFF; + + *val = gain; + *val2 = 32768; + + return IIO_VAL_FRACTIONAL; +} + +static int ad485x_set_calibscale(struct ad485x_state *st, int ch, int val, + int val2) +{ + unsigned long long gain; + unsigned int reg_val; + int ret; + + gain = val * 1000000 + val2; + gain = gain * 32768; + gain = DIV_U64_ROUND_CLOSEST(gain, 1000000); + + reg_val = gain; + + guard(mutex)(&st->lock); + + ret = regmap_write(st->regmap, AD485X_REG_CHX_GAIN_MSB(ch), + reg_val >> 8); + if (ret) + return ret; + + return regmap_write(st->regmap, AD485X_REG_CHX_GAIN_LSB(ch), + reg_val & 0xFF); +} + +static int ad485x_get_calibbias(struct ad485x_state *st, int ch, int *val, + int *val2) +{ + unsigned int lsb, mid, msb; + int ret; + + guard(mutex)(&st->lock); + + ret = regmap_read(st->regmap, AD485X_REG_CHX_OFFSET_MSB(ch), + &msb); + if (ret) + return ret; + + ret = regmap_read(st->regmap, AD485X_REG_CHX_OFFSET_MID(ch), + &mid); + if (ret) + return ret; + + ret = regmap_read(st->regmap, AD485X_REG_CHX_OFFSET_LSB(ch), + &lsb); + if (ret) + return ret; + + if (st->info->resolution == 16) { + *val = msb << 8; + *val |= mid; + *val = sign_extend32(*val, 15); + } else { + *val = msb << 12; + *val |= mid << 4; + *val |= lsb >> 4; + *val = sign_extend32(*val, 19); + } + + return IIO_VAL_INT; +} + +static int ad485x_set_calibbias(struct ad485x_state *st, int ch, int val, + int val2) +{ + unsigned int lsb, mid, msb; + int ret; + + if (st->info->resolution == 16) { + lsb = 0; + mid = val & 0xFF; + msb = (val >> 8) & 0xFF; + } else { + lsb = (val << 4) & 0xFF; + mid = (val >> 4) & 0xFF; + msb = (val >> 12) & 0xFF; + } + + guard(mutex)(&st->lock); + + ret = regmap_write(st->regmap, AD485X_REG_CHX_OFFSET_LSB(ch), lsb); + if (ret) + return ret; + + ret = regmap_write(st->regmap, AD485X_REG_CHX_OFFSET_MID(ch), mid); + if (ret) + return ret; + + return regmap_write(st->regmap, AD485X_REG_CHX_OFFSET_MSB(ch), msb); +} + +static const unsigned int ad485x_scale_table[][2] = { + {2500, 0x0}, {5000, 0x1}, {5000, 0x2}, {10000, 0x3}, {6250, 0x04}, + {12500, 0x5}, {10000, 0x6}, {20000, 0x7}, {12500, 0x8}, {25000, 0x9}, + {20000, 0xA}, {40000, 0xB}, {25000, 0xC}, {50000, 0xD}, {40000, 0xE}, + {80000, 0xF} +}; + +static const int ad4857_offset_table[] = { + 0, -32768 +}; + +static const int ad4858_offset_table[] = { + 0, -524288 +}; + +static const unsigned int ad485x_scale_avail[] = { + 2500, 5000, 10000, 6250, 12500, 20000, 25000, 40000, 50000, 80000 +}; + +static void __ad485x_get_scale(struct ad485x_state *st, int scale_tbl, + unsigned int *val, unsigned int *val2) +{ + const struct ad485x_chip_info *info = st->info; + const struct iio_chan_spec *chan = &info->channels[0]; + unsigned int tmp; + + tmp = (scale_tbl * 1000000ULL) >> chan->scan_type.realbits; + *val = tmp / 1000000; + *val2 = tmp % 1000000; +} + +static int ad485x_set_scale(struct ad485x_state *st, + const struct iio_chan_spec *chan, int val, int val2) +{ + const struct ad485x_chip_info *info = st->info; + unsigned int scale_val[2]; + unsigned int i, j = 0; + + for (i = 0; i < info->num_scales; i++) { + __ad485x_get_scale(st, info->scale_table[i][0], + &scale_val[0], &scale_val[1]); + if (scale_val[0] != val || scale_val[1] != val2) + continue; + + /* + * Adjust the softspan value (differential or single ended) + * based on the scale value selected and current offset of + * the channel. + * + * If the offset is 0 then continue iterations until finding + * the next matching scale value which always corresponds to + * the single ended mode. + */ + if (!st->offsets[chan->channel] && !j) { + j++; + continue; + } + + return regmap_write(st->regmap, + AD485X_REG_CHX_SOFTSPAN(chan->channel), + info->scale_table[i][1]); + } + + return -EINVAL; +} + +static int ad485x_get_scale(struct ad485x_state *st, + const struct iio_chan_spec *chan, int *val, + int *val2) +{ + const struct ad485x_chip_info *info = st->info; + unsigned int i, softspan_val; + int ret; + + ret = regmap_read(st->regmap, AD485X_REG_CHX_SOFTSPAN(chan->channel), + &softspan_val); + if (ret) + return ret; + + for (i = 0; i < info->num_scales; i++) { + if (softspan_val == info->scale_table[i][1]) + break; + } + + if (i == info->num_scales) + return -EIO; + + __ad485x_get_scale(st, info->scale_table[i][0], val, val2); + + return IIO_VAL_INT_PLUS_MICRO; +} + +static int ad485x_set_offset(struct ad485x_state *st, + const struct iio_chan_spec *chan, int val) +{ + guard(mutex)(&st->lock); + + if (st->offsets[chan->channel] != val) { + st->offsets[chan->channel] = val; + /* Restore to the default range if offset changes */ + if (st->offsets[chan->channel]) + return regmap_write(st->regmap, + AD485X_REG_CHX_SOFTSPAN(chan->channel), + AD485X_SOFTSPAN_N40V_40V); + return regmap_write(st->regmap, + AD485X_REG_CHX_SOFTSPAN(chan->channel), + AD485X_SOFTSPAN_0V_40V); + } + + return 0; +} + +static int ad485x_scale_offset_fill(struct ad485x_state *st) +{ + unsigned int i, val1, val2; + + st->offsets = devm_kcalloc(&st->spi->dev, st->info->num_channels, + sizeof(*st->offsets), GFP_KERNEL); + if (!st->offsets) + return -ENOMEM; + + st->scales = devm_kmalloc_array(&st->spi->dev, ARRAY_SIZE(ad485x_scale_avail), + sizeof(*st->scales), GFP_KERNEL); + if (!st->scales) + return -ENOMEM; + + for (i = 0; i < ARRAY_SIZE(ad485x_scale_avail); i++) { + __ad485x_get_scale(st, ad485x_scale_avail[i], &val1, &val2); + st->scales[i][0] = val1; + st->scales[i][1] = val2; + } + + return 0; +} + +static int ad485x_read_raw(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, + int *val, int *val2, long info) +{ + struct ad485x_state *st = iio_priv(indio_dev); + + switch (info) { + case IIO_CHAN_INFO_SAMP_FREQ: + *val = st->sampling_freq; + return IIO_VAL_INT; + case IIO_CHAN_INFO_CALIBSCALE: + return ad485x_get_calibscale(st, chan->channel, val, val2); + case IIO_CHAN_INFO_SCALE: + return ad485x_get_scale(st, chan, val, val2); + case IIO_CHAN_INFO_CALIBBIAS: + return ad485x_get_calibbias(st, chan->channel, val, val2); + case IIO_CHAN_INFO_OFFSET: + scoped_guard(mutex, &st->lock) + * val = st->offsets[chan->channel]; + return IIO_VAL_INT; + default: + return -EINVAL; + } +} + +static int ad485x_write_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int val, int val2, long info) +{ + struct ad485x_state *st = iio_priv(indio_dev); + + switch (info) { + case IIO_CHAN_INFO_SAMP_FREQ: + return ad485x_set_sampling_freq(st, val); + case IIO_CHAN_INFO_SCALE: + return ad485x_set_scale(st, chan, val, val2); + case IIO_CHAN_INFO_CALIBSCALE: + return ad485x_set_calibscale(st, chan->channel, val, val2); + case IIO_CHAN_INFO_CALIBBIAS: + return ad485x_set_calibbias(st, chan->channel, val, val2); + case IIO_CHAN_INFO_OFFSET: + return ad485x_set_offset(st, chan, val); + default: + return -EINVAL; + } +} + +static int ad485x_update_scan_mode(struct iio_dev *indio_dev, + const unsigned long *scan_mask) +{ + struct ad485x_state *st = iio_priv(indio_dev); + unsigned int c; + int ret; + + for (c = 0; c < st->info->num_channels; c++) { + if (test_bit(c, scan_mask)) + ret = iio_backend_chan_enable(st->back, c); + else + ret = iio_backend_chan_disable(st->back, c); + if (ret) + return ret; + } + + return 0; +} + +static int ad485x_read_avail(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + const int **vals, int *type, int *length, + long mask) +{ + struct ad485x_state *st = iio_priv(indio_dev); + + switch (mask) { + case IIO_CHAN_INFO_SCALE: + *vals = (const int *)st->scales; + *type = IIO_VAL_INT_PLUS_MICRO; + /* Values are stored in a 2D matrix */ + *length = ARRAY_SIZE(ad485x_scale_avail) * 2; + return IIO_AVAIL_LIST; + case IIO_CHAN_INFO_OFFSET: + *vals = st->info->offset_table; + *type = IIO_VAL_INT; + *length = st->info->num_offset; + return IIO_AVAIL_LIST; + default: + return -EINVAL; + } +} + +#define AD485X_IIO_CHANNEL(index, real, storage, info) \ +{ \ + .type = IIO_VOLTAGE, \ + .ext_info = info, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_CALIBSCALE) | \ + BIT(IIO_CHAN_INFO_CALIBBIAS) | \ + BIT(IIO_CHAN_INFO_SCALE) | \ + BIT(IIO_CHAN_INFO_OFFSET), \ + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \ + .info_mask_shared_by_type_available = \ + BIT(IIO_CHAN_INFO_SCALE) | \ + BIT(IIO_CHAN_INFO_OFFSET), \ + .indexed = 1, \ + .channel = index, \ + .scan_index = index, \ + .scan_type = { \ + .sign = 's', \ + .realbits = real, \ + .storagebits = storage, \ + }, \ +} + +static struct iio_chan_spec_ext_info ad4858_ext_info[] = { + IIO_ENUM("packet_format", IIO_SHARED_BY_ALL, &ad4858_packet_fmt), + IIO_ENUM_AVAILABLE("packet_format", + IIO_SHARED_BY_ALL, &ad4858_packet_fmt), + {}, +}; + +static struct iio_chan_spec_ext_info ad4857_ext_info[] = { + IIO_ENUM("packet_format", IIO_SHARED_BY_ALL, &ad4857_packet_fmt), + IIO_ENUM_AVAILABLE("packet_format", + IIO_SHARED_BY_ALL, &ad4857_packet_fmt), + {}, +}; + +static const struct iio_chan_spec ad4858_channels[] = { + AD485X_IIO_CHANNEL(0, 20, 32, ad4858_ext_info), + AD485X_IIO_CHANNEL(1, 20, 32, ad4858_ext_info), + AD485X_IIO_CHANNEL(2, 20, 32, ad4858_ext_info), + AD485X_IIO_CHANNEL(3, 20, 32, ad4858_ext_info), + AD485X_IIO_CHANNEL(4, 20, 32, ad4858_ext_info), + AD485X_IIO_CHANNEL(5, 20, 32, ad4858_ext_info), + AD485X_IIO_CHANNEL(6, 20, 32, ad4858_ext_info), + AD485X_IIO_CHANNEL(7, 20, 32, ad4858_ext_info), +}; + +static const struct iio_chan_spec ad4857_channels[] = { + AD485X_IIO_CHANNEL(0, 16, 16, ad4857_ext_info), + AD485X_IIO_CHANNEL(1, 16, 16, ad4857_ext_info), + AD485X_IIO_CHANNEL(2, 16, 16, ad4857_ext_info), + AD485X_IIO_CHANNEL(3, 16, 16, ad4857_ext_info), + AD485X_IIO_CHANNEL(4, 16, 16, ad4857_ext_info), + AD485X_IIO_CHANNEL(5, 16, 16, ad4857_ext_info), + AD485X_IIO_CHANNEL(6, 16, 16, ad4857_ext_info), + AD485X_IIO_CHANNEL(7, 16, 16, ad4857_ext_info), +}; + +static const struct ad485x_chip_info ad4858_info = { + .name = "ad4858", + .product_id = AD4858_PROD_ID, + .scale_table = ad485x_scale_table, + .num_scales = ARRAY_SIZE(ad485x_scale_table), + .offset_table = ad4858_offset_table, + .num_offset = ARRAY_SIZE(ad4858_offset_table), + .channels = ad4858_channels, + .num_channels = ARRAY_SIZE(ad4858_channels), + .throughput = 1 * MEGA, + .resolution = 20, +}; + +static const struct ad485x_chip_info ad4857_info = { + .name = "ad4857", + .product_id = AD4857_PROD_ID, + .scale_table = ad485x_scale_table, + .num_scales = ARRAY_SIZE(ad485x_scale_table), + .offset_table = ad4857_offset_table, + .num_offset = ARRAY_SIZE(ad4857_offset_table), + .channels = ad4857_channels, + .num_channels = ARRAY_SIZE(ad4857_channels), + .throughput = 1 * MEGA, + .resolution = 16, +}; + +static const struct ad485x_chip_info ad4856_info = { + .name = "ad4856", + .product_id = AD4856_PROD_ID, + .scale_table = ad485x_scale_table, + .num_scales = ARRAY_SIZE(ad485x_scale_table), + .offset_table = ad4858_offset_table, + .num_offset = ARRAY_SIZE(ad4858_offset_table), + .channels = ad4858_channels, + .num_channels = ARRAY_SIZE(ad4858_channels), + .throughput = 250 * KILO, + .resolution = 20, + .resolution = 16, +}; + +static const struct ad485x_chip_info ad4855_info = { + .name = "ad4855", + .product_id = AD4855_PROD_ID, + .scale_table = ad485x_scale_table, + .num_scales = ARRAY_SIZE(ad485x_scale_table), + .offset_table = ad4857_offset_table, + .num_offset = ARRAY_SIZE(ad4857_offset_table), + .channels = ad4857_channels, + .num_channels = ARRAY_SIZE(ad4857_channels), + .throughput = 250 * KILO, + .resolution = 16, +}; + +static const struct ad485x_chip_info ad4854_info = { + .name = "ad4854", + .product_id = AD4854_PROD_ID, + .scale_table = ad485x_scale_table, + .num_scales = ARRAY_SIZE(ad485x_scale_table), + .offset_table = ad4858_offset_table, + .num_offset = ARRAY_SIZE(ad4858_offset_table), + .channels = ad4858_channels, + .num_channels = ARRAY_SIZE(ad4858_channels), + .throughput = 1 * MEGA, + .resolution = 20, +}; + +static const struct ad485x_chip_info ad4853_info = { + .name = "ad4853", + .product_id = AD4853_PROD_ID, + .scale_table = ad485x_scale_table, + .num_scales = ARRAY_SIZE(ad485x_scale_table), + .offset_table = ad4857_offset_table, + .num_offset = ARRAY_SIZE(ad4857_offset_table), + .channels = ad4857_channels, + .num_channels = ARRAY_SIZE(ad4857_channels), + .throughput = 1 * MEGA, + .resolution = 16, +}; + +static const struct ad485x_chip_info ad4852_info = { + .name = "ad4852", + .product_id = AD4852_PROD_ID, + .scale_table = ad485x_scale_table, + .num_scales = ARRAY_SIZE(ad485x_scale_table), + .offset_table = ad4858_offset_table, + .num_offset = ARRAY_SIZE(ad4858_offset_table), + .channels = ad4858_channels, + .num_channels = ARRAY_SIZE(ad4858_channels), + .throughput = 250 * KILO, + .resolution = 20, +}; + +static const struct ad485x_chip_info ad4851_info = { + .name = "ad4851", + .product_id = AD4851_PROD_ID, + .scale_table = ad485x_scale_table, + .num_scales = ARRAY_SIZE(ad485x_scale_table), + .offset_table = ad4857_offset_table, + .num_offset = ARRAY_SIZE(ad4857_offset_table), + .channels = ad4857_channels, + .num_channels = ARRAY_SIZE(ad4857_channels), + .throughput = 250 * KILO, + .resolution = 16, +}; + +static const struct ad485x_chip_info ad4858i_info = { + .name = "ad4858i", + .product_id = AD4858I_PROD_ID, + .scale_table = ad485x_scale_table, + .num_scales = ARRAY_SIZE(ad485x_scale_table), + .offset_table = ad4858_offset_table, + .num_offset = ARRAY_SIZE(ad4858_offset_table), + .channels = ad4858_channels, + .num_channels = ARRAY_SIZE(ad4858_channels), + .throughput = 1000000, + .resolution = 20, +}; + +static const struct iio_info ad485x_info = { + .debugfs_reg_access = ad485x_reg_access, + .read_raw = ad485x_read_raw, + .write_raw = ad485x_write_raw, + .update_scan_mode = ad485x_update_scan_mode, + .read_avail = ad485x_read_avail, +}; + +static const struct regmap_config regmap_config = { + .reg_bits = 16, + .val_bits = 8, + .read_flag_mask = BIT(7), +}; + +static const char * const ad485x_power_supplies[] = { + "vcc", "vdd", "vddh", "vio" +}; + +static int ad485x_probe(struct spi_device *spi) +{ + struct iio_dev *indio_dev; + struct ad485x_state *st; + int ret; + + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); + if (!indio_dev) + return -ENOMEM; + + st = iio_priv(indio_dev); + st->spi = spi; + + mutex_init(&st->lock); + + ret = devm_regulator_bulk_get_enable(&spi->dev, + ARRAY_SIZE(ad485x_power_supplies), + ad485x_power_supplies); + if (ret) + return dev_err_probe(&spi->dev, ret, + "failed to get and enable supplies\n"); + + st->cnv = devm_pwm_get(&spi->dev, NULL); + if (IS_ERR(st->cnv)) + return PTR_ERR(st->cnv); + + st->info = spi_get_device_match_data(spi); + if (!st->info) + return -ENODEV; + + st->regmap = devm_regmap_init_spi(spi, ®map_config); + if (IS_ERR(st->regmap)) + return PTR_ERR(st->regmap); + + ret = ad485x_scale_offset_fill(st); + if (ret) + return ret; + + ret = ad485x_setup(st); + if (ret) + return ret; + + indio_dev->name = st->info->name; + indio_dev->channels = st->info->channels; + indio_dev->num_channels = st->info->num_channels; + indio_dev->info = &ad485x_info; + + st->back = devm_iio_backend_get(&spi->dev, NULL); + if (IS_ERR(st->back)) + return PTR_ERR(st->back); + + ret = devm_iio_backend_request_buffer(&spi->dev, st->back, indio_dev); + if (ret) + return ret; + + ret = devm_iio_backend_enable(&spi->dev, st->back); + if (ret) + return ret; + + ret = ad485x_calibrate(st); + if (ret) + return ret; + + return devm_iio_device_register(&spi->dev, indio_dev); +} + +static const struct of_device_id ad485x_of_match[] = { + { .compatible = "adi,ad4858", .data = &ad4858_info, }, + { .compatible = "adi,ad4857", .data = &ad4857_info, }, + { .compatible = "adi,ad4856", .data = &ad4856_info, }, + { .compatible = "adi,ad4855", .data = &ad4855_info, }, + { .compatible = "adi,ad4854", .data = &ad4854_info, }, + { .compatible = "adi,ad4853", .data = &ad4853_info, }, + { .compatible = "adi,ad4852", .data = &ad4852_info, }, + { .compatible = "adi,ad4851", .data = &ad4851_info, }, + { .compatible = "adi,ad4858i", .data = &ad4858i_info, }, + {} +}; + +static const struct spi_device_id ad485x_spi_id[] = { + { "ad4858", (kernel_ulong_t)&ad4858_info }, + { "ad4857", (kernel_ulong_t)&ad4857_info }, + { "ad4856", (kernel_ulong_t)&ad4856_info }, + { "ad4855", (kernel_ulong_t)&ad4855_info }, + { "ad4854", (kernel_ulong_t)&ad4854_info }, + { "ad4853", (kernel_ulong_t)&ad4853_info }, + { "ad4852", (kernel_ulong_t)&ad4852_info }, + { "ad4851", (kernel_ulong_t)&ad4851_info }, + { "ad4858i", (kernel_ulong_t)&ad4858i_info }, + { } +}; +MODULE_DEVICE_TABLE(spi, ad485x_spi_id); + +static struct spi_driver ad485x_driver = { + .probe = ad485x_probe, + .driver = { + .name = "ad485x", + .of_match_table = ad485x_of_match, + }, + .id_table = ad485x_spi_id, +}; +module_spi_driver(ad485x_driver); + +MODULE_AUTHOR("Sergiu Cuciurean <sergiu.cuciurean@analog.com>"); +MODULE_AUTHOR("Dragos Bogdan <dragos.bogdan@analog.com>"); +MODULE_AUTHOR("Antoniu Miclaus <antoniu.miclaus@analog.com>"); +MODULE_DESCRIPTION("Analog Devices AD485x DAS driver"); +MODULE_LICENSE("GPL"); +MODULE_IMPORT_NS(IIO_BACKEND);
Add support for the AD485X DAS familiy. Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> --- drivers/iio/adc/Kconfig | 12 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/ad485x.c | 1061 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 1074 insertions(+) create mode 100644 drivers/iio/adc/ad485x.c