Re: [PATCH 3/3] iio: adc: ltc2497: add 2x conversion speed mode
Andy Shevchenko <[email protected]>
| Newsgroups | org.kernel.vger.linux-iio,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAHp75VeNFTPwujkLWG-NbSdbJU-r-xThroOiOJrhz_R8+vMikA@mail.gmail.com> |
On Wed, Aug 5, 2026 at 12:44 PM Andrei Stancovici <[email protected]> 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