On Mon, 19 Aug 2024 09:47:17 +0300 Alexandru Ardelean <aardelean@xxxxxxxxxxxx> wrote: > The AD7606C-16 and AD7606C-18 are pretty similar with the AD7606B. > The main difference between AD7606C-16 & AD7606C-18 is the precision in > bits (16 vs 18). > Because of that, some scales need to be defined for the 18-bit variants, as > they need to be computed against 2**18 (vs 2**16 for the 16 bit-variants). > > Because the AD7606C-16,18 also supports bipolar & differential channels, > for SW-mode, the default range of 10 V or ±10V should be set at probe. > On reset, the default range (in the registers) is set to value 0x3 which > corresponds to '±10 V single-ended range', regardless of bipolar or > differential configuration. > > Aside from the scale/ranges, the AD7606C-16 is similar to the AD7606B. > > And the AD7606C-18 variant offers 18-bit precision. The unfortunate effect > of this 18-bit sample size, is that there is no simple/neat way to get the > samples into a 32-bit array without having to do a home-brewed bit-buffer. > The ADC must read all samples (from all 8 channels) in order to get the > N-th sample (this could be reworked to do up-to-N-th sample for scan-direct). > There doesn't seem to be any quick-trick to be usable to pad the samples > up to at least 24 bits. > Even the optional status-header is 8-bits, which would mean 26-bits of data > per sample. > That means that when using a simple SPI controller (which can usually read > 8 bit multiples) a simple bit-buffer trick is required. > > Datasheet links: > https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-16.pdf > https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-18.pdf > > Signed-off-by: Alexandru Ardelean <aardelean@xxxxxxxxxxxx> A few minor things. If we can just start with 18 bit word spi controllers only maybe that's worth doing to make things simpler. > +static int ad7606c_sw_mode_setup_channels(struct iio_dev *indio_dev, > + ad7606c_chan_setup_cb_t chan_setup_cb) > +{ > + unsigned int num_channels = indio_dev->num_channels - 1; > + struct ad7606_state *st = iio_priv(indio_dev); > + bool chan_configured[AD760X_MAX_CHANNELS]; = {}; and drop the memset. > + struct device *dev = st->dev; > + int ret; > + u32 ch; > + > + /* We need to hook this first */ > + ret = st->bops->sw_mode_config(indio_dev); > + if (ret) > + return ret; > + > + indio_dev->info = &ad7606c_info_sw_mode; > + > + memset(chan_configured, 0, sizeof(chan_configured)); > + > + device_for_each_child_node_scoped(dev, child) { > + bool bipolar, differential; > + > + ret = fwnode_property_read_u32(child, "reg", &ch); > + if (ret) > + continue; > + > + if (ch >= num_channels) { > + dev_warn(st->dev, > + "Invalid channel number (ignoring): %d\n", ch); > + continue; > + } > + > + bipolar = fwnode_property_present(child, "bipolar"); > + differential = fwnode_property_present(child, "diff-channel"); > + > + chan_setup_cb(st, ch, bipolar, differential); > + chan_configured[ch] = true; > + } > + > + /* Apply default configuration to unconfigured (via DT) channels */ > + for (ch = 0; ch < num_channels; ch++) { > + struct ad7606_chan_scale *cs; > + unsigned int *scale_avail_show; > + int i; > + > + if (!chan_configured[ch]) > + chan_setup_cb(st, ch, false, false); > + > + /* AD7606C supports different scales per channel */ > + cs = &st->chan_scales[ch]; > + > + scale_avail_show = devm_kcalloc(st->dev, cs->num_scales * 2, > + sizeof(*scale_avail_show), > + GFP_KERNEL); Maybe just make it big enough for worst case and stick it in st always? How big can it get? > + if (!scale_avail_show) > + return -ENOMEM; > + > + /* Generate a scale_avail list for showing to userspace */ > + for (i = 0; i < cs->num_scales; i++) { > + scale_avail_show[i * 2] = 0; > + scale_avail_show[i * 2 + 1] = cs->scale_avail[i]; > + } > + > + cs->scale_avail_show = scale_avail_show; > + } > + > + return 0; > +} > > diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c > index dd0075c97c24..73a7b0007bf8 100644 > --- a/drivers/iio/adc/ad7606_spi.c > +++ b/drivers/iio/adc/ad7606_spi.c > @@ -45,6 +45,8 @@ > > +static int ad7606_spi_read_block18to32(struct device *dev, > + int count, void *buf) > +{ > + struct spi_device *spi = to_spi_device(dev); > + u32 i, bit_buffer, buf_size, bit_buf_size; > + u32 *data = buf; > + u8 *bdata = buf; > + int j, ret; > + > + /** Not kernel doc. /* > + * With the 18 bit ADC variants (here) is that we can't assume that all > + * SPI controllers will pad 18-bit sequences into 32-bit arrays, > + * so we need to do a bit of buffer magic here. > + * Alternatively, we can have a variant of this function that works > + * for SPI controllers that can pad 18-bit samples into 32-bit arrays. > + */ > + > + /* Write 'count' bytes to the right, to not overwrite samples */ > + bdata += count; > + > + /* Read 24 bits only, as we'll only get samples of 18 bits each */ > + buf_size = count * 3; > + ret = spi_read(spi, bdata, buf_size); > + if (ret < 0) { > + dev_err(&spi->dev, "SPI read error\n"); > + return ret; > + } > + > + bit_buffer = 0; > + bit_buf_size = 0; > + for (j = 0, i = 0; i < buf_size; i++) { > + u32 sample; > + > + bit_buffer = (bit_buffer << 8) | bdata[i]; > + bit_buf_size += 8; > + > + if (bit_buf_size < 18) > + continue; > + > + bit_buf_size -= 18; > + sample = (bit_buffer >> bit_buf_size) & AD7606C_18_SAMPLE_MASK; > + data[j++] = sign_extend32(sample, 17); > + > + if (j == count) > + break; > + } > + > + return 0; > +}
