Re: [PATCH v5 5/5] iio: adc: versal-sysmon: add oversampling support

[Erim, Salih]

Hi Andy,

On 09/06/2026 18:57, Andy Shevchenko wrote:
> On Mon, Jun 08, 2026 at 07:38:01PM +0100, Salih Erim wrote:
> > Add support for reading and writing the oversampling ratio through
> > the IIO oversampling_ratio attribute. The hardware supports averaging
> > 2, 4, 8, or 16 samples, plus a ratio of 1 (no averaging).
> >
> > Temperature and supply channels share oversampling configuration at
> > the type level (all temperature channels share one ratio, all supply
> > channels share another), exposed through info_mask_shared_by_type.
> >
> > The hardware encoding uses sample_count / 2 in a 4-bit field within
> > the CONFIG register. Per-channel averaging enable registers must also
> > be updated to activate or deactivate averaging.
>
> ...
>
> > +static int sysmon_osr_write(struct sysmon *sysmon, int channel_type, int val)
> > +{
> > +     /*
> > +      * HW register encoding is sample_count / 2:
> > +      * 0=none, 1=2x, 2=4x, 4=8x, 8=16x (not log2-based).
> > +      */
> > +     int hw_val = val >> 1;
> > +     unsigned int readback;
> > +     int ret;
> > +
> > +     switch (channel_type) {
> > +     case IIO_TEMP:
> > +             ret = regmap_update_bits(sysmon->regmap, SYSMON_CONFIG,
> > +                                     SYSMON_CONFIG_TEMP_SAT_OSR,
> > +                                     FIELD_PREP(SYSMON_CONFIG_TEMP_SAT_OSR,
> > +                                                hw_val));
>
> Broken indentation.

Accepted. Sorry for all.
> > +             if (ret)
> > +                     return ret;
> > +
> > +             /*
> > +              * Readback fence: the SysMon CONFIG register resides in the
> > +              * PMC domain behind the NoC. A posted write may not reach the
> > +              * hardware before the next MMIO access. Reading the register
> > +              * back forces the interconnect to complete the write, preventing
> > +              * a bus hang on the subsequent access.
> > +              */
> > +             regmap_read(sysmon->regmap, SYSMON_CONFIG, &readback);
> > +
> > +             return sysmon_set_avg_enable(sysmon, SYSMON_TEMP_EN_AVG_BASE,
> > +                                          SYSMON_TEMP_EN_AVG_COUNT,
> > +                                          hw_val ? ~0U : 0);
> > +     case IIO_VOLTAGE:
> > +             ret = regmap_update_bits(sysmon->regmap, SYSMON_CONFIG,
> > +                                     SYSMON_CONFIG_SUPPLY_OSR,
> > +                                     FIELD_PREP(SYSMON_CONFIG_SUPPLY_OSR,
> > +                                                hw_val));
>
> Ditto.

Accepted.

> > +             if (ret)
> > +                     return ret;
> > +
> > +             /* Readback fence -- see above */
> > +             regmap_read(sysmon->regmap, SYSMON_CONFIG, &readback);
> > +
> > +             return sysmon_set_avg_enable(sysmon, SYSMON_SUPPLY_EN_AVG_BASE,
> > +                                          SYSMON_SUPPLY_EN_AVG_COUNT,
> > +                                          hw_val ? ~0U : 0);
> > +     default:
> > +             return -EINVAL;
> > +     }
> > +}
>
> ...
>
> > +static int sysmon_write_raw(struct iio_dev *indio_dev,
> > +                         struct iio_chan_spec const *chan,
> > +                         int val, int val2, long mask)
> > +{
> > +     struct sysmon *sysmon = iio_priv(indio_dev);
> > +     int i, ret;
>
> Why is 'i' signed?

No reason. Will change to unsigned int.

Thanks,
Salih.

> > +
> > +     if (mask != IIO_CHAN_INFO_OVERSAMPLING_RATIO)
> > +             return -EINVAL;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(sysmon_oversampling_avail); i++) {
> > +             if (val == sysmon_oversampling_avail[i])
> > +                     break;
> > +     }
> > +     if (i == ARRAY_SIZE(sysmon_oversampling_avail))
> > +             return -EINVAL;
> > +
> > +     guard(mutex)(&sysmon->lock);
> > +
> > +     ret = sysmon_osr_write(sysmon, chan->type, val);
> > +     if (ret)
> > +             return ret;
> > +
> > +     if (chan->type == IIO_TEMP)
> > +             sysmon->temp_oversampling = val;
> > +     else
> > +             sysmon->supply_oversampling = val;
> > +
> > +     return 0;
> > +}
>
> --
> With Best Regards,
> Andy Shevchenko