Re: [PATCH 3/3] iio: adc: ltc2497: add 2x conversion speed mode

[Andy Shevchenko]

On Wed, Aug 5, 2026 at 12:44 PM Andrei Stancovici
<andrei.stancovici@xxxxxxxxxx> wrote:
>
> The LTC2499 supports a 2x output rate (SPD bit in the second
> configuration byte).  In 2x mode the offset auto-calibration is
> disabled, roughly doubling the conversion rate (~13.6Hz vs ~6.8Hz in
> simultaneous 50/60Hz rejection) while leaving linearity and full-scale
> errors unchanged (datasheet).  During a temperature measurement the part
> always converts at 1x regardless of SPD.
>
> Expose the rate through the standard sampling_frequency /
> sampling_frequency_available ABI on the voltage channels only: SPD is
> ignored for temperature conversions, so the temperature channel
> deliberately carries no SAMP_FREQ attribute.  A new has_speed_mode
> capability flag gates the feature (LTC2499); the two-byte command path
> is now taken for has_temp || has_speed_mode, since both features need the
> second config byte.
>
> The conversion-time wait becomes mode dependent: 150ms at 1x, 76ms at 2x
> (datasheet t_CONV max, simultaneous rejection, rounded up).  The wait is
> keyed on the conversion currently in flight, whose duration is fixed by
> the mode that was active when it started - not by the newly selected
> mode.  This matters on a 1x->2x switch: a 1x conversion may still be
> running when the first 2x read arrives, and reprogramming the device
> before it finishes would be NACKed with -EIO.  Timing is centralized in
> ltc2497core_conv_time_ms() so a future FA/FB rejection-mode selection
> can extend it into a [rejection][speed] lookup without touching callers.
>
> LTC2496/LTC2497 (no speed mode) keep the single-byte path and the
> unchanged 150ms wait.
>
> Validated on a live LTC2499: 20 reads take ~3.1s at 1x and ~1.6s at 2x
> (~0.5x, no -EIO), voltage and temperature readings stay sane in both
> modes, and the temperature/voltage interleave (sticky-PTAT) regression
> still passes at 1x and 2x.

...

>  #include <linux/delay.h>
> +#include <linux/device.h>

Why? On a brief look I haven't noticed the use of this header.

>  #include <linux/iio/iio.h>
>  #include <linux/iio/driver.h>
>  #include <linux/math64.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/slab.h>

Neither is this.

Perhaps you wanted the device/devres.h?

...

> +static int ltc2497core_write_raw(struct iio_dev *indio_dev,
> +                                struct iio_chan_spec const *chan,
> +                                int val, int val2, long mask)
> +{
> +       struct ltc2497core_driverdata *ddata = iio_priv(indio_dev);
> +       bool sped_2x;
> +       int i;

Why signed?

> +       switch (mask) {
> +       case IIO_CHAN_INFO_SAMP_FREQ:
> +               /* Match the (val, val2) pair against the advertised rates. */
> +               for (i = 0; i < ARRAY_SIZE(ltc2497core_samp_freq_avail); i += 2) {
> +                       if (val == ltc2497core_samp_freq_avail[i] &&
> +                           val2 == ltc2497core_samp_freq_avail[i + 1])
> +                               break;
> +               }
> +               if (i >= ARRAY_SIZE(ltc2497core_samp_freq_avail))

'==' should suffice.

> +                       return -EINVAL;
> +
> +               sped_2x = i / 2;
> +
> +               mutex_lock(&ddata->lock);
> +               ddata->sped_2x = sped_2x;
> +               /*
> +                * The new speed only takes effect once the second command byte
> +                * is reprogrammed, so force the next read to reprogram rather
> +                * than reuse the value already latched for this address.
> +                * LTC2497_CONFIG_DEFAULT is not a valid channel/temperature
> +                * address, so it is a safe re-arm sentinel (as used at probe).
> +                *
> +                * A conversion started under the old speed may still be in
> +                * flight; its own duration (conv_time_prev), not the new mode's,
> +                * still gates the next reprogram, so the timing state is left
> +                * untouched here.
> +                */
> +               ddata->addr_prev = LTC2497_CONFIG_DEFAULT;
> +               mutex_unlock(&ddata->lock);
> +
> +               return 0;

...

> +       if (ddata->chip_info->has_speed_mode) {
> +               struct iio_chan_spec *channels;
> +               unsigned int i;
> +
> +               channels = devm_kmemdup(dev, ltc2497core_channel,
> +                                       sizeof(ltc2497core_channel), GFP_KERNEL);
> +               if (!channels)
> +                       return -ENOMEM;

> +               for (i = 0; i < indio_dev->num_channels; i++) {

Iterrator is not used outside, so can be

  for (unsigned int i...)

> +                       if (channels[i].type != IIO_VOLTAGE)
> +                               continue;
> +                       channels[i].info_mask_shared_by_type |=
> +                               BIT(IIO_CHAN_INFO_SAMP_FREQ);
> +                       channels[i].info_mask_shared_by_type_available |=
> +                               BIT(IIO_CHAN_INFO_SAMP_FREQ);
> +               }
> +
> +               indio_dev->channels = channels;
> +       }

...

> +#define LTC2497_CONV_TIME_1X_MS                150ULL  /* t_CONV_1 simult. max 149.9 */
> +#define LTC2499_CONV_TIME_2X_MS                76ULL   /* t_CONV_2 simult. max  75.1 */

I didn't get the rounding rules. Is it just a ceiling for all cases?
Can we use more precise (10x) values instead?

-- 
With Best Regards,
Andy Shevchenko