On Thu, 9 Jun 2022 11:32:03 +0300
Claudiu Beznea <claudiu.beznea@xxxxxxxxxxxxx> wrote:
> Add 64 and 256 oversampling ratio support. It is necessary for temperature
> sensor.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea@xxxxxxxxxxxxx>
> ---
> drivers/iio/adc/at91-sama5d2_adc.c | 31 +++++++++++++++++++++++++++---
> 1 file changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
> index 7321a4b519af..b52f1020feaf 100644
> --- a/drivers/iio/adc/at91-sama5d2_adc.c
> +++ b/drivers/iio/adc/at91-sama5d2_adc.c
> @@ -142,6 +142,8 @@ struct at91_adc_reg_layout {
> #define AT91_SAMA5D2_EMR_OSR_1SAMPLES 0
> #define AT91_SAMA5D2_EMR_OSR_4SAMPLES 1
> #define AT91_SAMA5D2_EMR_OSR_16SAMPLES 2
> +#define AT91_SAMA5D2_EMR_OSR_64SAMPLES 3
> +#define AT91_SAMA5D2_EMR_OSR_256SAMPLES 4
>
> /* Extended Mode Register - Averaging on single trigger event */
> #define AT91_SAMA5D2_EMR_ASTE(V) ((V) << 20)
> @@ -308,6 +310,8 @@ static const struct at91_adc_reg_layout sama7g5_layout = {
> #define AT91_OSR_1SAMPLES 1
> #define AT91_OSR_4SAMPLES 4
> #define AT91_OSR_16SAMPLES 16
> +#define AT91_OSR_64SAMPLES 64
> +#define AT91_OSR_256SAMPLES 256
These defines seems a bit silly. Better to use the values inline than
to have these.
>
> #define AT91_SAMA5D2_CHAN_SINGLE(index, num, addr) \
> { \
> @@ -640,7 +644,9 @@ static const struct at91_adc_platform sama7g5_platform = {
> .osr_mask = GENMASK(18, 16),
> .osr_vals = BIT(AT91_SAMA5D2_EMR_OSR_1SAMPLES) |
> BIT(AT91_SAMA5D2_EMR_OSR_4SAMPLES) |
> - BIT(AT91_SAMA5D2_EMR_OSR_16SAMPLES),
> + BIT(AT91_SAMA5D2_EMR_OSR_16SAMPLES) |
> + BIT(AT91_SAMA5D2_EMR_OSR_64SAMPLES) |
> + BIT(AT91_SAMA5D2_EMR_OSR_256SAMPLES),
> .chan_realbits = 16,
> };
>
> @@ -774,6 +780,18 @@ static int at91_adc_config_emr(struct at91_adc_state *st,
> emr |= AT91_SAMA5D2_EMR_OSR(AT91_SAMA5D2_EMR_OSR_16SAMPLES,
> osr_mask);
> break;
> + case AT91_OSR_64SAMPLES:
> + if (!(osr_vals & BIT(AT91_SAMA5D2_EMR_OSR_64SAMPLES)))
> + return -EINVAL;
> + emr |= AT91_SAMA5D2_EMR_OSR(AT91_SAMA5D2_EMR_OSR_64SAMPLES,
> + osr_mask);
> + break;
> + case AT91_OSR_256SAMPLES:
> + if (!(osr_vals & BIT(AT91_SAMA5D2_EMR_OSR_256SAMPLES)))
> + return -EINVAL;
> + emr |= AT91_SAMA5D2_EMR_OSR(AT91_SAMA5D2_EMR_OSR_256SAMPLES,
> + osr_mask);
> + break;
> }
>
> at91_adc_writel(st, EMR, emr);
> @@ -791,6 +809,10 @@ static int at91_adc_adjust_val_osr(struct at91_adc_state *st, int *val)
> nbits = 13;
> else if (st->oversampling_ratio == AT91_OSR_16SAMPLES)
> nbits = 14;
> + else if (st->oversampling_ratio == AT91_OSR_64SAMPLES)
> + nbits = 15;
> + else if (st->oversampling_ratio == AT91_OSR_256SAMPLES)
> + nbits = 16;
>
> /*
> * We have nbits of real data and channel is registered as
> @@ -1679,7 +1701,8 @@ static int at91_adc_write_raw(struct iio_dev *indio_dev,
> switch (mask) {
> case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> if ((val != AT91_OSR_1SAMPLES) && (val != AT91_OSR_4SAMPLES) &&
> - (val != AT91_OSR_16SAMPLES))
> + (val != AT91_OSR_16SAMPLES) && (val != AT91_OSR_64SAMPLES) &&
> + (val != AT91_OSR_256SAMPLES))
Dropping this partial validity check and moving into a default in the switch statement
in config_emr() would be nice cleanup (I also replied to earlier patch based on what
is visible here).
> return -EINVAL;
> /* if no change, optimize out */
> mutex_lock(&st->lock);
> @@ -1897,7 +1920,9 @@ static IIO_CONST_ATTR(hwfifo_watermark_max, AT91_HWFIFO_MAX_SIZE_STR);
> static IIO_CONST_ATTR(oversampling_ratio_available,
> __stringify(AT91_OSR_1SAMPLES) " "
> __stringify(AT91_OSR_4SAMPLES) " "
> - __stringify(AT91_OSR_16SAMPLES));
> + __stringify(AT91_OSR_16SAMPLES) " "
> + __stringify(AT91_OSR_64SAMPLES) " "
> + __stringify(AT91_OSR_256SAMPLES));
At somepoint it would be good to move this over to the read_avail() callback rather than
hand rolling it. We are slowly working through doing this for all the IIO drivers
but it will take a long time yet!
>
> static struct attribute *at91_adc_attributes[] = {
> &iio_const_attr_oversampling_ratio_available.dev_attr.attr,
