On 11.06.2022 20:47, Jonathan Cameron wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> 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).
Sure, I'll check it.
>
>> 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!
I'll check this, too.
>
>>
>> static struct attribute *at91_adc_attributes[] = {
>> &iio_const_attr_oversampling_ratio_available.dev_attr.attr,
>
