On Tue, 18 Jun 2024 18:08:34 +0200
Olivier Moysan <olivier.moysan@xxxxxxxxxxx> wrote:
> Add scaling support to STM32 DFSDM.
Perhaps a short description here of how this works? Where does the scale come
from, what assumptions are made etc.
>
> Signed-off-by: Olivier Moysan <olivier.moysan@xxxxxxxxxxx>
Some minor stuff.
> diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
> index 69b4764d7cba..93bf6035bd6d 100644
> --- a/drivers/iio/adc/stm32-dfsdm-adc.c
> +++ b/drivers/iio/adc/stm32-dfsdm-adc.c
urn 0;
> }
>
> @@ -1060,7 +1072,7 @@ static int stm32_dfsdm_update_scan_mode(struct iio_dev *indio_dev,
> static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
> {
> struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
> - int ret;
> + int i = 0, ret;
Don't mix assigned and unassigned variable declarations. Just use a separate line
as this can mean subtle assignment or lack of assignment issues sneak in.
>
> /* Reset adc buffer index */
> adc->bufi = 0;
> @@ -1071,6 +1083,15 @@ static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
> return ret;
> }
>
> + if (adc->backend) {
> + while (adc->backend[i]) {
Could do similar to the suggestion below.
Mostly I don't like the index variable manipulation.
> + ret = iio_backend_enable(&indio_dev->dev, adc->backend[i]);
> + if (ret < 0)
> + return ret;
> + i++;
> + }
> + }
> +
> ret = stm32_dfsdm_start_dfsdm(adc->dfsdm);
> if (ret < 0)
> goto err_stop_hwc;
> @@ -1103,6 +1124,7 @@ static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
> static int stm32_dfsdm_predisable(struct iio_dev *indio_dev)
> {
> struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
> + int i = 0;
>
> stm32_dfsdm_stop_conv(indio_dev);
>
> @@ -1110,6 +1132,13 @@ static int stm32_dfsdm_predisable(struct iio_dev *indio_dev)
>
> stm32_dfsdm_stop_dfsdm(adc->dfsdm);
>
> + if (adc->backend) {
> + while (adc->backend[i]) {
> + iio_backend_disable(&indio_dev->dev, adc->backend[i]);
> + i++;
> + }
Something like
struct iio_backend **be = &adc->backend[0];
do {
iio_backend_disable(&indio-dev->dev, be);
} while (be++);
maybe. Up to you.
> + }
> @@ -1320,6 +1360,45 @@ static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
> *val = adc->sample_freq;
>
> return IIO_VAL_INT;
> +
> + case IIO_CHAN_INFO_SCALE:
> + /*
> + * Scale is expressed in mV.
> + * When fast mode is disabled, actual resolution may be lower
> + * than 2^n, where n=realbits-1.
As below, use a few more spaces.
> + * This leads to underestimating input voltage. To
> + * compensate this deviation, the voltage reference can be
> + * corrected with a factor = realbits resolution / actual max
> + */
> + if (adc->backend[idx]) {
> + iio_backend_read_raw(adc->backend[idx], val, val2, mask);
> +
> + *val = div_u64((u64)*val * (u64)BIT(DFSDM_DATA_RES - 1), max);
> + *val2 = chan->scan_type.realbits;
> + if (chan->differential)
> + *val *= 2;
> + }
> + return IIO_VAL_FRACTIONAL_LOG2;
> +
> + case IIO_CHAN_INFO_OFFSET:
> + /*
> + * DFSDM output data are in the range [-2^n,2^n],
Use a few more spaces. [-2^2, 2^n]
> + * with n=realbits-1.
n = realbits - 1
Just to keep it closer to the C coding style.
> + * - Differential modulator:
> + * Offset correspond to SD modulator offset.
> + * - Single ended modulator:
> + * Input is in [0V,Vref] range, where 0V corresponds to -2^n, and Vref to 2^n.
Avoid that long line with a suitable line break.
> + * Add 2^n to offset. (i.e. middle of input range)
> + * offset = offset(sd) * vref / res(sd) * max / vref.
> + */
> + if (adc->backend[idx]) {
> + iio_backend_read_raw(adc->backend[idx], val, val2, mask);
> +
> + *val = div_u64((u64)max * *val, BIT(*val2 - 1));
> + if (!chan->differential)
> + *val += max;
> + }
> + return IIO_VAL_INT;
> }
>
> return -EINVAL;
> @@ -1449,7 +1528,15 @@ static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev, struct iio_c
> * IIO_CHAN_INFO_RAW: used to compute regular conversion
> * IIO_CHAN_INFO_OVERSAMPLING_RATIO: used to set oversampling
> */
> - ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> + if (child) {
> + ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE) |
> + BIT(IIO_CHAN_INFO_OFFSET);
Indent looks a little odd. Maybe one more space neede?
> + } else {
> + /* Legacy. Scaling not supported */
> + ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> + }
> +
> ch->info_mask_shared_by_all = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
> BIT(IIO_CHAN_INFO_SAMP_FREQ);
>
> @@ -1816,3 +1903,4 @@ module_platform_driver(stm32_dfsdm_adc_driver);
> MODULE_DESCRIPTION("STM32 sigma delta ADC");
> MODULE_AUTHOR("Arnaud Pouliquen <arnaud.pouliquen@xxxxxx>");
> MODULE_LICENSE("GPL v2");
> +MODULE_IMPORT_NS(IIO_BACKEND);
