Re: [PATCH v1 13/15] iio: adc: ad7768-1: add multiple scan types to support 16-bits mode
From: David Lechner <dlechner@baylibre.com>
Date: 2025-01-07 23:49:07
Also in:
linux-iio, lkml
On 1/7/25 9:26 AM, Jonathan Santos wrote:
When the device is configured to Sinc5 filter and decimation x8, output data is reduced to 16-bits in order to support 1 MHz of sampling frequency due to clock limitation.
We aren't going to get a 1 MHz sample rate without SPI offload support so maybe we should save this patch until then? In this patch, we are still reading 24-bits per sample, so we aren't really getting any benefit. It is probably fine for now to leave it as 24-bit even if the last 8 bits are all 0 or just noise. Also, the datasheet says: this path allows viewing of wider bandwidth; however, it is quantization noise limited so that output data is reduced to 16 bits So this doesn't actually seem related to higher sample rates. There is a CONVLEN bit in the INTERFACE_FORMAT register that globally reduces the output size to 16-bit, which I suspect would be what we will need for achieving the highest sample rate when we add SPI offload support.
quoted hunk ↗ jump to hunk
Use multiple scan types feature to enable the driver to switch scan type in runtime, making possible to support both 24-bit and 16-bit resolution. Signed-off-by: Jonathan Santos <redacted> --- drivers/iio/adc/ad7768-1.c | 65 ++++++++++++++++++++++++++++++++------ 1 file changed, 56 insertions(+), 9 deletions(-)diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c index 9741a6d47942..5e4e7d387f9a 100644 --- a/drivers/iio/adc/ad7768-1.c +++ b/drivers/iio/adc/ad7768-1.c@@ -134,6 +134,11 @@ struct ad7768_clk_configuration { enum ad7768_pwrmode pwrmode; }; +enum ad7768_scan_type { + AD7768_SCAN_TYPE_NORMAL, + AD7768_SCAN_TYPE_HIGH_SPEED, +}; + static const char * const ad7768_vcm_modes[] = { "(AVDD1-AVSS)/2", "2V5",@@ -145,6 +150,10 @@ static const char * const ad7768_vcm_modes[] = { "OFF", }; +static const int ad7768_mclk_div_rates[4] = { + 16, 8, 4, 2, +}; + static const struct ad7768_clk_configuration ad7768_clk_config[] = { { AD7768_MCLK_DIV_2, AD7768_DEC_RATE_8, 16, AD7768_FAST_MODE }, { AD7768_MCLK_DIV_2, AD7768_DEC_RATE_16, 32, AD7768_FAST_MODE },@@ -159,6 +168,21 @@ static const struct ad7768_clk_configuration ad7768_clk_config[] = { { AD7768_MCLK_DIV_16, AD7768_DEC_RATE_1024, 16384, AD7768_ECO_MODE }, }; +static const struct iio_scan_type ad7768_scan_type[] = { + [AD7768_SCAN_TYPE_NORMAL] = { + .sign = 's', + .realbits = 24, + .storagebits = 32,
What happened to .shift = 8, ? If there is a reason for removing it, please add that to the commit description.
+ .endianness = IIO_BE,
+ },
+ [AD7768_SCAN_TYPE_HIGH_SPEED] = {
+ .sign = 's',
+ .realbits = 16,
+ .storagebits = 32,I guess it doesn't matter much since we are reading one sample at a time, but I would expect storagebits to be 16 instead of 32. Or if it really needs to be 32, does it need shift = 16?
+ .endianness = IIO_BE, + }, +}; + static int ad7768_get_vcm(struct iio_dev *dev, const struct iio_chan_spec *chan); static int ad7768_set_vcm(struct iio_dev *dev, const struct iio_chan_spec *chan, unsigned int mode);
...
quoted hunk ↗ jump to hunk
@@ -308,6 +329,15 @@ static int ad7768_scan_direct(struct iio_dev *indio_dev) ret = ad7768_spi_reg_read(st, AD7768_REG_ADC_DATA, &readval, 3); if (ret < 0) return ret; + + /* + * When the decimation rate is set to x8, the ADC data precision is reduced + * from 24 bits to 16 bits. Since the AD7768_REG_ADC_DATA register provides + * 24-bit data, the precision is reduced by right-shifting the read value + * by 8 bits. + */ + if (st->dec_rate == 8) + readval = readval >> 8;
Why not change size of ad7768_spi_reg_read() instead of reading 3 bytes and throwing one away?
/* * Any SPI configuration of the AD7768-1 can only be * performed in continuous conversion mode.