Re: [PATCH v2 3/5] iio: adc: ad7606: add offset and phase calibration support

Nuno Sá
2025-05-04T09:34:05Z

On Sun, 2025-05-04 at 09:30 +0100, Nuno Sá wrote:
> On Fri, 2025-05-02 at 15:27 +0200, Angelo Dureghello wrote:
> > From: Angelo Dureghello <adureghello@baylibre.com>
> >
> > Add support for offset and phase calibration, only for
> > devices that support software mode, that are:
> >
> > ad7606b
> > ad7606c-16
> > ad7606c-18
> >
> > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> > ---
>
> With the the proper type for the DT properties:
>

Disregard this... Feel free to add the tag. The comment is for other patch!
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>
> >  drivers/iio/adc/ad7606.c | 160 +++++++++++++++++++++++++++++++++++++++++++++++
> >  drivers/iio/adc/ad7606.h |   9 +++
> >  2 files changed, 169 insertions(+)
> >
> > diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
> > index
> > ad5e6b5e1d5d2edc7f8ac7ed9a8a4e6e43827b85..139d8b3f9bb39dd631a71c70539005d719fb5b7
> > b
> > 100644
> > --- a/drivers/iio/adc/ad7606.c
> > +++ b/drivers/iio/adc/ad7606.c
> > @@ -95,6 +95,22 @@ static const unsigned int ad7616_oversampling_avail[8] = {
> >   1, 2, 4, 8, 16, 32, 64, 128,
> >  };
> >
> > +static const int ad7606_calib_offset_avail[3] = {
> > + -128, 1, 127,
> > +};
> > +
> > +static const int ad7606c_18bit_calib_offset_avail[3] = {
> > + -512, 4, 508,
> > +};
> > +
> > +static const int ad7606b_calib_phase_avail[][2] = {
> > + { 0, 0 }, { 0, 1250 }, { 0, 318750 },
> > +};
> > +
> > +static const int ad7606c_calib_phase_avail[][2] = {
> > + { 0, 0 }, { 0, 1000 }, { 0, 255000 },
> > +};
> > +
> >  static int ad7606c_18bit_chan_scale_setup(struct iio_dev *indio_dev,
> >     struct iio_chan_spec *chan);
> >  static int ad7606c_16bit_chan_scale_setup(struct iio_dev *indio_dev,
> > @@ -164,6 +180,8 @@ const struct ad7606_chip_info ad7606b_info = {
> >   .scale_setup_cb = ad7606_16bit_chan_scale_setup,
> >   .sw_setup_cb = ad7606b_sw_mode_setup,
> >   .offload_storagebits = 32,
> > + .calib_offset_avail = ad7606_calib_offset_avail,
> > + .calib_phase_avail = ad7606b_calib_phase_avail,
> >  };
> >  EXPORT_SYMBOL_NS_GPL(ad7606b_info, "IIO_AD7606");
> >
> > @@ -177,6 +195,8 @@ const struct ad7606_chip_info ad7606c_16_info = {
> >   .scale_setup_cb = ad7606c_16bit_chan_scale_setup,
> >   .sw_setup_cb = ad7606b_sw_mode_setup,
> >   .offload_storagebits = 32,
> > + .calib_offset_avail = ad7606_calib_offset_avail,
> > + .calib_phase_avail = ad7606c_calib_phase_avail,
> >  };
> >  EXPORT_SYMBOL_NS_GPL(ad7606c_16_info, "IIO_AD7606");
> >
> > @@ -226,6 +246,8 @@ const struct ad7606_chip_info ad7606c_18_info = {
> >   .scale_setup_cb = ad7606c_18bit_chan_scale_setup,
> >   .sw_setup_cb = ad7606b_sw_mode_setup,
> >   .offload_storagebits = 32,
> > + .calib_offset_avail = ad7606c_18bit_calib_offset_avail,
> > + .calib_phase_avail = ad7606c_calib_phase_avail,
> >  };
> >  EXPORT_SYMBOL_NS_GPL(ad7606c_18_info, "IIO_AD7606");
> >
> > @@ -683,6 +705,40 @@ static int ad7606_scan_direct(struct iio_dev *indio_dev,
> > unsigned int ch,
> >   return ret;
> >  }
> >
> > +static int ad7606_get_calib_offset(struct ad7606_state *st, int ch, int *val)
> > +{
> > + int ret;
> > +
> > + ret = st->bops->reg_read(st, AD7606_CALIB_OFFSET(ch));
> > + if (ret < 0)
> > + return ret;
> > +
> > + *val = st->chip_info->calib_offset_avail[0] +
> > + ret * st->chip_info->calib_offset_avail[1];
> > +
> > + return 0;
> > +}
> > +
> > +static int ad7606_get_calib_phase(struct ad7606_state *st, int ch, int *val,
> > +   int *val2)
> > +{
> > + int ret;
> > +
> > + ret = st->bops->reg_read(st, AD7606_CALIB_PHASE(ch));
> > + if (ret < 0)
> > + return ret;
> > +
> > + *val = 0;
> > +
> > + /*
> > + * ad7606b: phase delay from 0 to 318.75 μs in steps of 1.25 μs.
> > + * ad7606c-16/18: phase delay from 0 µs to 255 µs in steps of 1 µs.
> > + */
> > + *val2 = ret * st->chip_info->calib_phase_avail[1][1];
> > +
> > + return 0;
> > +}
> > +
> >  static int ad7606_read_raw(struct iio_dev *indio_dev,
> >      struct iio_chan_spec const *chan,
> >      int *val,
> > @@ -717,6 +773,22 @@ static int ad7606_read_raw(struct iio_dev *indio_dev,
> >   pwm_get_state(st->cnvst_pwm, &cnvst_pwm_state);
> >   *val = DIV_ROUND_CLOSEST_ULL(NSEC_PER_SEC,
> > cnvst_pwm_state.period);
> >   return IIO_VAL_INT;
> > + case IIO_CHAN_INFO_CALIBBIAS:
> > + if (!iio_device_claim_direct(indio_dev))
> > + return -EBUSY;
> > + ret = ad7606_get_calib_offset(st, chan->scan_index, val);
> > + iio_device_release_direct(indio_dev);
> > + if (ret)
> > + return ret;
> > + return IIO_VAL_INT;
> > + case IIO_CHAN_INFO_CALIBCONV_DELAY:
> > + if (!iio_device_claim_direct(indio_dev))
> > + return -EBUSY;
> > + ret = ad7606_get_calib_phase(st, chan->scan_index, val, val2);
> > + iio_device_release_direct(indio_dev);
> > + if (ret)
> > + return ret;
> > + return IIO_VAL_INT_PLUS_NANO;
> >   }
> >   return -EINVAL;
> >  }
> > @@ -767,6 +839,64 @@ static int ad7606_write_os_hw(struct iio_dev *indio_dev, int
> > val)
> >   return 0;
> >  }
> >
> > +static int ad7606_set_calib_offset(struct ad7606_state *st, int ch, int val)
> > +{
> > + int start_val, step_val, stop_val;
> > +
> > + start_val = st->chip_info->calib_offset_avail[0];
> > + step_val = st->chip_info->calib_offset_avail[1];
> > + stop_val = st->chip_info->calib_offset_avail[2];
> > +
> > + if (val < start_val || val > stop_val)
> > + return -EINVAL;
> > +
> > + val -= start_val;
> > + val /= step_val;
> > +
> > + return st->bops->reg_write(st, AD7606_CALIB_OFFSET(ch), val);
> > +}
> > +
> > +static int ad7606_set_calib_phase(struct ad7606_state *st, int ch, int val,
> > +   int val2)
> > +{
> > + int wreg, start_ns, step_ns, stop_ns;
> > +
> > + if (val != 0)
> > + return -EINVAL;
> > +
> > + start_ns = st->chip_info->calib_phase_avail[0][1];
> > + step_ns = st->chip_info->calib_phase_avail[1][1];
> > + stop_ns = st->chip_info->calib_phase_avail[2][1];
> > +
> > + /*
> > + * ad7606b: phase dielay from 0 to 318.75 μs in steps of 1.25 μs.
> > + * ad7606c-16/18: phase delay from 0 µs to 255 µs in steps of 1 µs.
> > + */
> > + if (val2 < start_ns || val2 > stop_ns)
> > + return -EINVAL;
> > +
> > + wreg = val2 / step_ns;
> > +
> > + return st->bops->reg_write(st, AD7606_CALIB_PHASE(ch), wreg);
> > +}
> > +
> > +static int ad7606_write_raw_get_fmt(struct iio_dev *indio_dev,
> > +     struct iio_chan_spec const *chan, long info)
> > +{
> > + switch (info) {
> > + case IIO_CHAN_INFO_SCALE:
> > + return IIO_VAL_INT_PLUS_MICRO;
> > + case IIO_CHAN_INFO_SAMP_FREQ:
> > + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> > + case IIO_CHAN_INFO_CALIBBIAS:
> > + return IIO_VAL_INT;
> > + case IIO_CHAN_INFO_CALIBCONV_DELAY:
> > + return IIO_VAL_INT_PLUS_NANO;
> > + default:
> > + return -EINVAL;
> > + }
> > +}
> > +
> >  static int ad7606_write_raw(struct iio_dev *indio_dev,
> >       struct iio_chan_spec const *chan,
> >       int val,
> > @@ -820,6 +950,18 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
> >   if (val < 0 && val2 != 0)
> >   return -EINVAL;
> >   return ad7606_set_sampling_freq(st, val);
> > + case IIO_CHAN_INFO_CALIBBIAS:
> > + if (!iio_device_claim_direct(indio_dev))
> > + return -EBUSY;
> > + ret = ad7606_set_calib_offset(st, chan->scan_index, val);
> > + iio_device_release_direct(indio_dev);
> > + return ret;
> > + case IIO_CHAN_INFO_CALIBCONV_DELAY:
> > + if (!iio_device_claim_direct(indio_dev))
> > + return -EBUSY;
> > + ret = ad7606_set_calib_phase(st, chan->scan_index, val, val2);
> > + iio_device_release_direct(indio_dev);
> > + return ret;
> >   default:
> >   return -EINVAL;
> >   }
> > @@ -998,6 +1140,14 @@ static int ad7606_read_avail(struct iio_dev *indio_dev,
> >   *type = IIO_VAL_INT_PLUS_MICRO;
> >
> >   return IIO_AVAIL_LIST;
> > + case IIO_CHAN_INFO_CALIBBIAS:
> > + *vals = st->chip_info->calib_offset_avail;
> > + *type = IIO_VAL_INT;
> > + return IIO_AVAIL_RANGE;
> > + case IIO_CHAN_INFO_CALIBCONV_DELAY:
> > + *vals = (const int *)st->chip_info->calib_phase_avail;
> > + *type = IIO_VAL_INT_PLUS_NANO;
> > + return IIO_AVAIL_RANGE;
> >   }
> >   return -EINVAL;
> >  }
> > @@ -1060,6 +1210,7 @@ static const struct iio_info ad7606_info_sw_mode = {
> >   .read_raw = &ad7606_read_raw,
> >   .write_raw = &ad7606_write_raw,
> >   .read_avail = &ad7606_read_avail,
> > + .write_raw_get_fmt = ad7606_write_raw_get_fmt,
> >   .debugfs_reg_access = &ad7606_reg_access,
> >   .validate_trigger = &ad7606_validate_trigger,
> >   .update_scan_mode = &ad7606_update_scan_mode,
> > @@ -1252,6 +1403,15 @@ static int ad7606_probe_channels(struct iio_dev
> > *indio_dev)
> >   chan->info_mask_separate_available |=
> >   BIT(IIO_CHAN_INFO_SCALE);
> >
> > + if (st->chip_info->calib_offset_avail) {
> > + chan->info_mask_separate |=
> > + BIT(IIO_CHAN_INFO_CALIBBIAS) |
> > + BIT(IIO_CHAN_INFO_CALIBCONV_DELAY);
> > + chan->info_mask_separate_available |=
> > + BIT(IIO_CHAN_INFO_CALIBBIAS) |
> > + BIT(IIO_CHAN_INFO_CALIBCONV_DELAY);
> > + }
> > +
> >   /*
> >   * All chips with software mode support oversampling,
> >   * so we skip the oversampling_available check. And the
> > diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
> > index
> > 89d49551eaf515bab9706c12bff056dfcb707b67..4c39de36154ffb80dfbb01bb4f812826bdc5596
> > 7
> > 100644
> > --- a/drivers/iio/adc/ad7606.h
> > +++ b/drivers/iio/adc/ad7606.h
> > @@ -40,6 +40,11 @@
> >  #define AD7606_RANGE_CH_ADDR(ch) (0x03 + ((ch) >> 1))
> >  #define AD7606_OS_MODE 0x08
> >
> > +#define AD7606_CALIB_GAIN(ch) (0x09 + (ch))
> > +#define AD7606_CALIB_GAIN_MASK GENMASK(5, 0)
> > +#define AD7606_CALIB_OFFSET(ch) (0x11 + (ch))
> > +#define AD7606_CALIB_PHASE(ch) (0x19 + (ch))
> > +
> >  struct ad7606_state;
> >
> >  typedef int (*ad7606_scale_setup_cb_t)(struct iio_dev *indio_dev,
> > @@ -61,6 +66,8 @@ typedef int (*ad7606_sw_setup_cb_t)(struct iio_dev *indio_dev);
> >   * @init_delay_ms: required delay in milliseconds for initialization
> >   * after a restart
> >   * @offload_storagebits: storage bits used by the offload hw implementation
> > + * @calib_offset_avail: pointer to offset calibration range/limits array
> > + * @calib_phase_avail:  pointer to phase calibration range/limits array
> >   */
> >  struct ad7606_chip_info {
> >   unsigned int max_samplerate;
> > @@ -74,6 +81,8 @@ struct ad7606_chip_info {
> >   bool os_req_reset;
> >   unsigned long init_delay_ms;
> >   u8 offload_storagebits;
> > + const int *calib_offset_avail;
> > + const int (*calib_phase_avail)[2];
> >  };
> >
> >  /**
> >
>