[PATCH v2 0/5] iio: adc: add ad7606 calibration support

[PATCH v2 0/5] iio: adc: add ad7606 calibration support

[Angelo Dureghello]

Add gain, offset and phase (as a delay) calibration support, for
ad7606b, ad7606c16 and ad7606c18.

Calibration is available for devices with software mode capability. 

Offset and phase calibration is configurable by sysfs attributes, while
gain calibration value in ohms must match the external RFilter value,
when an external RFilter is available, so implemented through a specific
devicetree "adi,rfilter-ohms" property.

Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
Changes in v2:
- change phase_delay to calib_delay,
- fix dt_bindings,
- fix gain calibarion fdt parsing,
- fix ad7606c-18 calib offset range,
- fix calib offset calculation,
- fix calib gain range,
- Link to v1: https://lore.kernel.org/r/20250429-wip-bl-ad7606-calibration-v1-0-eb4d4821b172@baylibre.com

---
Angelo Dureghello (5):
      Documentation: ABI: IIO: add calibconv_delay documentation
      iio: core: add ADC delay calibration definition
      iio: adc: ad7606: add offset and phase calibration support
      dt-bindings: iio: adc: adi,ad7606: add gain calibration support
      iio: adc: ad7606: add gain calibration support

 Documentation/ABI/testing/sysfs-bus-iio            |  20 ++
 .../devicetree/bindings/iio/adc/adi,ad7606.yaml    |  29 +++
 drivers/iio/adc/ad7606.c                           | 217 +++++++++++++++++++++
 drivers/iio/adc/ad7606.h                           |  13 ++
 drivers/iio/industrialio-core.c                    |   1 +
 include/linux/iio/types.h                          |   1 +
 6 files changed, 281 insertions(+)
---
base-commit: e22e3d5089987cb4250801623026992b2ba4645d
change-id: 20250429-wip-bl-ad7606-calibration-20a396a60352

Best regards,
-- 
Angelo Dureghello <adureghello@baylibre.com>

[PATCH v2 1/5] Documentation: ABI: IIO: add calibconv_delay documentation

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

Add new IIO calibconv_delay documentation.

The ad7606 implements a phase calibation feature, in nanoseconds.
Being this a time delay, using the conv_delay suffix.

Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 33c09c4ac60a4feec82308461643134f5ba84b66..56eb42f88999660b5f93f2311b7d57e0303b0647 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -559,6 +559,26 @@ Description:
 		- a small discrete set of values like "0 2 4 6 8"
 		- a range specified as "[min step max]"
 
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_calibconv_delay
+KernelVersion:  6.16
+Contact:        linux-iio@vger.kernel.org
+Description:
+		Hardware applied calbiration delay (assumed to fix errors that are
+		introduced from external circuitry).
+		For the ad7606 ADC series, this value is intended as a time delay,
+		as an integer plus nanoseconds.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_calibconv_delay_available
+KernelVersion:	6.16
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Available values of calibconv_delay. Maybe expressed as:
+
+		- a range specified as "[min step max]"
+
+		If shared across all channels, <type>_calibconv_delay_available
+		is used.
+
 What:		/sys/bus/iio/devices/iio:deviceX/in_accel_x_calibscale
 What:		/sys/bus/iio/devices/iio:deviceX/in_accel_y_calibscale
 What:		/sys/bus/iio/devices/iio:deviceX/in_accel_z_calibscale

-- 
2.49.0

Re: [PATCH v2 1/5] Documentation: ABI: IIO: add calibconv_delay documentation

[Jonathan Cameron]

On Fri, 02 May 2025 15:26:58 +0200
Angelo Dureghello <adureghello@baylibre.com> wrote:

> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Add new IIO calibconv_delay documentation.
> 
> The ad7606 implements a phase calibation feature, in nanoseconds.
> Being this a time delay, using the conv_delay suffix.
I made a late reply to v1...

Key point being that, in the general sense this is only a calibration
thing if it is both writeable and we are using it for filter phase correction.
In more general terms it's just a conversion sampling time offset (and as you have
it here in seconds).  I'm keen we define this to incorporate more general
cases including extra read only info on sequencer timing - that can be useful
if we have something like 
                 _____________
Input 0 --------|             |
Input 1 --------| 4 in, 2 out |-----  ADC0
Input 2 --------|  MUX        |
Input 3 --------|_____________|-----  ADC1

That is the ability to schedule more channels across a small number of
simultaneous sampling ADCs.  In these cases we've never had a way to
express what was done together.  Mostly there have been obvious
combinations (i.e. voltage and current at same time on a given wire for
power measurement), but it would still be nice to use your new interface
to allow us to describe what is running here (though probably not control
it as that would be hard to do!)

> 
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 33c09c4ac60a4feec82308461643134f5ba84b66..56eb42f88999660b5f93f2311b7d57e0303b0647 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -559,6 +559,26 @@ Description:
>  		- a small discrete set of values like "0 2 4 6 8"
>  		- a range specified as "[min step max]"
>  
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_calibconv_delay

I wonder if simply in_voltageY_delay is enough?  I don't mind
in_voltageY_convdelay  but don't like the calib bit.

I have had requests to stop using underscores in middle of modifiers as they are a pain
to parse - hence convdelay rather than conv_delay

> +KernelVersion:  6.16
> +Contact:        linux-iio@vger.kernel.org
> +Description:
> +		Hardware applied calbiration delay (assumed to fix errors that are
> +		introduced from external circuitry).

Use this a an example of why this might be controllable.  It might be read only
if all it is doing is giving us a richer description of a sequencer.
Something like

		Delay of start of conversion in seconds from common reference point
		shared by all channels.  When used to compensate for delay variation
		in external filters feeding a simultaneous sampling ADC this may
		be referred to as a ...

> +		For the ad7606 ADC series, this value is intended as a time delay,
> +		as an integer plus nanoseconds.
Just call it seconds. Once it reaches userspace it might have different scaling but we 
fix that up with the right number of zeros.
No part specific units...  They all need to be seconds.

> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_calibconv_delay_available
> +KernelVersion:	6.16
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Available values of calibconv_delay. Maybe expressed as:
> +
> +		- a range specified as "[min step max]"
> +
> +		If shared across all channels, <type>_calibconv_delay_available
> +		is used.
> +
>  What:		/sys/bus/iio/devices/iio:deviceX/in_accel_x_calibscale
>  What:		/sys/bus/iio/devices/iio:deviceX/in_accel_y_calibscale
>  What:		/sys/bus/iio/devices/iio:deviceX/in_accel_z_calibscale
> 

Re: [PATCH v2 1/5] Documentation: ABI: IIO: add calibconv_delay documentation

[David Lechner]

On 5/4/25 10:16 AM, Jonathan Cameron wrote:
> On Fri, 02 May 2025 15:26:58 +0200
> Angelo Dureghello <adureghello@baylibre.com> wrote:
> 
>> From: Angelo Dureghello <adureghello@baylibre.com>
>>
>> Add new IIO calibconv_delay documentation.
>>
>> The ad7606 implements a phase calibation feature, in nanoseconds.
>> Being this a time delay, using the conv_delay suffix.
> I made a late reply to v1...
> 
> Key point being that, in the general sense this is only a calibration
> thing if it is both writeable and we are using it for filter phase correction.
> In more general terms it's just a conversion sampling time offset (and as you have
> it here in seconds).  I'm keen we define this to incorporate more general
> cases including extra read only info on sequencer timing - that can be useful
> if we have something like 
>                  _____________
> Input 0 --------|             |
> Input 1 --------| 4 in, 2 out |-----  ADC0
> Input 2 --------|  MUX        |
> Input 3 --------|_____________|-----  ADC1
> 
> That is the ability to schedule more channels across a small number of
> simultaneous sampling ADCs.  In these cases we've never had a way to
> express what was done together.  Mostly there have been obvious
> combinations (i.e. voltage and current at same time on a given wire for
> power measurement), but it would still be nice to use your new interface
> to allow us to describe what is running here (though probably not control
> it as that would be hard to do!)
> 
I'm totally on board with making this more general than just calibration, but
having worked on a couple of multiplexed simultaneous sampling ADCs like this,
I'm scratching my head a bit trying to figure out how we would be able to know
what the delay was between the conversions, at least in cases where we don't
have a hardware conversion trigger based on a clock/pwm. Generally, it is as
fast as the SPI bus can bang it out, but that isn't a fixed or predictable
amount of time.

Re: [PATCH v2 1/5] Documentation: ABI: IIO: add calibconv_delay documentation

[Jonathan Cameron]

On Sun, 4 May 2025 14:48:32 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 5/4/25 10:16 AM, Jonathan Cameron wrote:
> > On Fri, 02 May 2025 15:26:58 +0200
> > Angelo Dureghello <adureghello@baylibre.com> wrote:
> >   
> >> From: Angelo Dureghello <adureghello@baylibre.com>
> >>
> >> Add new IIO calibconv_delay documentation.
> >>
> >> The ad7606 implements a phase calibation feature, in nanoseconds.
> >> Being this a time delay, using the conv_delay suffix.  
> > I made a late reply to v1...
> > 
> > Key point being that, in the general sense this is only a calibration
> > thing if it is both writeable and we are using it for filter phase correction.
> > In more general terms it's just a conversion sampling time offset (and as you have
> > it here in seconds).  I'm keen we define this to incorporate more general
> > cases including extra read only info on sequencer timing - that can be useful
> > if we have something like 
> >                  _____________
> > Input 0 --------|             |
> > Input 1 --------| 4 in, 2 out |-----  ADC0
> > Input 2 --------|  MUX        |
> > Input 3 --------|_____________|-----  ADC1
> > 
> > That is the ability to schedule more channels across a small number of
> > simultaneous sampling ADCs.  In these cases we've never had a way to
> > express what was done together.  Mostly there have been obvious
> > combinations (i.e. voltage and current at same time on a given wire for
> > power measurement), but it would still be nice to use your new interface
> > to allow us to describe what is running here (though probably not control
> > it as that would be hard to do!)
> >   
> I'm totally on board with making this more general than just calibration, but
> having worked on a couple of multiplexed simultaneous sampling ADCs like this,
> I'm scratching my head a bit trying to figure out how we would be able to know
> what the delay was between the conversions, at least in cases where we don't
> have a hardware conversion trigger based on a clock/pwm. Generally, it is as
> fast as the SPI bus can bang it out, but that isn't a fixed or predictable
> amount of time.

Yeah, this only applies to self clocking devices with a FIFO (possibly a very
short one with 1 register per channel in a scan).
The SPI hammering cases don't work for exactly the reason you mention.

For those we might be able to come up with a multi-baseline solution to
indicate that inputs 0 + 1 are together and 2 + 3 also together but it would
be fiddly.  So lets wait until we know we need that :)

Jonathan

[PATCH v2 2/5] iio: core: add ADC delay calibration definition

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

ADCs as ad7606 implement a phase calibration as a delay.
Add such definition, needed for ad7606.

Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
 drivers/iio/industrialio-core.c | 1 +
 include/linux/iio/types.h       | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 178e99b111debc59a247fcc3a6037e429db3bebf..45ce8603959e6d0985904b7feb79872bffde1126 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -188,6 +188,7 @@ static const char * const iio_chan_info_postfix[] = {
 	[IIO_CHAN_INFO_CALIBAMBIENT] = "calibambient",
 	[IIO_CHAN_INFO_ZEROPOINT] = "zeropoint",
 	[IIO_CHAN_INFO_TROUGH] = "trough_raw",
+	[IIO_CHAN_INFO_CALIBCONV_DELAY] = "calibconv_delay",
 };
 /**
  * iio_device_id() - query the unique ID for the device
diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
index d89982c98368cf72c0fc30fa66ab001e48af4e8b..d1137180a8b0bf2e34cb4186dceddad4978ca766 100644
--- a/include/linux/iio/types.h
+++ b/include/linux/iio/types.h
@@ -69,6 +69,7 @@ enum iio_chan_info_enum {
 	IIO_CHAN_INFO_CALIBAMBIENT,
 	IIO_CHAN_INFO_ZEROPOINT,
 	IIO_CHAN_INFO_TROUGH,
+	IIO_CHAN_INFO_CALIBCONV_DELAY,
 };
 
 #endif /* _IIO_TYPES_H_ */

-- 
2.49.0

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

[Angelo Dureghello]

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>
---
 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..139d8b3f9bb39dd631a71c70539005d719fb5b7b 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..4c39de36154ffb80dfbb01bb4f812826bdc55967 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];
 };
 
 /**

-- 
2.49.0

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

[Andy Shevchenko]

On Fri, May 02, 2025 at 03:27:00PM +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

...

> +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];

Something wrong with the indentation.

> +	return 0;
> +}

...

> +static int ad7606_set_calib_offset(struct ad7606_state *st, int ch, int val)
> +{
> +	int start_val, step_val, stop_val;

All need to be signed?

> +	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;

Ditto.

> +	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;

Wrong indentation. Please fix in all places where it happens.

> +	wreg = val2 / step_ns;
> +
> +	return st->bops->reg_write(st, AD7606_CALIB_PHASE(ch), wreg);
> +}

-- 
With Best Regards,
Andy Shevchenko

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

[Angelo Dureghello]

On 02.05.2025 16:39, Andy Shevchenko wrote:
> On Fri, May 02, 2025 at 03:27:00PM +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
> 
> ...
> 
> > +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];
> 
> Something wrong with the indentation.
> 
> > +	return 0;
> > +}
> 
> ...
> 
> > +static int ad7606_set_calib_offset(struct ad7606_state *st, int ch, int val)
> > +{
> > +	int start_val, step_val, stop_val;
> 
> All need to be signed?
> 
> > +	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;
> 
> Ditto.
> 
> > +	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;
> 
> Wrong indentation. Please fix in all places where it happens.

as already said, my code is correct, 1 tab after the if, anywone knows 
why git formats the patch this way ?  

> 
> > +	wreg = val2 / step_ns;
> > +
> > +	return st->bops->reg_write(st, AD7606_CALIB_PHASE(ch), wreg);
> > +}
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

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

[Andy Shevchenko]

On Tue, May 06, 2025 at 02:59:50PM +0200, Angelo Dureghello wrote:
> On 02.05.2025 16:39, Andy Shevchenko wrote:
> > On Fri, May 02, 2025 at 03:27:00PM +0200, Angelo Dureghello wrote:

...

> > > +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];
> > 
> > Something wrong with the indentation.
> > 
> > > +	return 0;
> > > +}

...

> > > +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;
> > 
> > Wrong indentation. Please fix in all places where it happens.
> 
> as already said, my code is correct,

Evidently no.

> 1 tab after the if, anywone knows why git formats the patch this way ?  

It's you, who should answer this and fix it.

> > > +	wreg = val2 / step_ns;
> > > +
> > > +	return st->bops->reg_write(st, AD7606_CALIB_PHASE(ch), wreg);
> > > +}

-- 
With Best Regards,
Andy Shevchenko

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

[Angelo Dureghello]

On 02.05.2025 16:39, Andy Shevchenko wrote:
> On Fri, May 02, 2025 at 03:27:00PM +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
> 
> ...
> 
> > +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];
> 
> Something wrong with the indentation.

Can eventually adjust to:

	*val = st->chip_info->calib_offset_avail[0] + ret *
		st->chip_info->calib_offset_avail[1];

this is genrelly ok.

> 
> > +	return 0;
> > +}
> 
> ...
> 
> > +static int ad7606_set_calib_offset(struct ad7606_state *st, int ch, int val)
> > +{
> > +	int start_val, step_val, stop_val;
> 
> All need to be signed?
> 

would keep them all as int, to avoid different type comparison later. 


> > +	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;
> 
> Ditto.
>
here too, val and val2 comes in as int, and subsequent operations
are int, so would stay as int.
 
> > +	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;
> 
> Wrong indentation. Please fix in all places where it happens.
> 
rechecked better now, tabs in this place was wrong. Fixed, thanks.

> > +	wreg = val2 / step_ns;
> > +
> > +	return st->bops->reg_write(st, AD7606_CALIB_PHASE(ch), wreg);
> > +}
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 
regards,
angelo

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

[Andy Shevchenko]

On Tue, May 06, 2025 at 04:46:35PM +0200, Angelo Dureghello wrote:
> On 02.05.2025 16:39, Andy Shevchenko wrote:
> > On Fri, May 02, 2025 at 03:27:00PM +0200, Angelo Dureghello wrote:

...

> > > +	*val = st->chip_info->calib_offset_avail[0] +
> > > +		ret * st->chip_info->calib_offset_avail[1];
> > 
> > Something wrong with the indentation.
> 
> Can eventually adjust to:
> 
> 	*val = st->chip_info->calib_offset_avail[0] + ret *
> 		st->chip_info->calib_offset_avail[1];
> 
> this is genrelly ok.

This will be illogical split. The idea is to replace second TAB with 7 spaces
(instead of 8).

-- 
With Best Regards,
Andy Shevchenko

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

[David Lechner]

On 5/2/25 8:27 AM, 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>
> ---
Tested-by: David Lechner <dlechner@baylibre.com>

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

[Nuno Sá]

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:

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..139d8b3f9bb39dd631a71c70539005d719fb5b7b
> 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..4c39de36154ffb80dfbb01bb4f812826bdc55967
> 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];
>  };
>  
>  /**
> 

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

[Nuno Sá]

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];
> >  };
> >  
> >  /**
> > 
> 

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

[Jonathan Cameron]

On Fri, 02 May 2025 15:27:00 +0200
Angelo Dureghello <adureghello@baylibre.com> 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>

One trivial typo.

> +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.

Spell check. delay



> +	 * ad7606c-16/18: phase delay from 0 µs to 255 µs in steps of 1 µs.
> +	 */

[PATCH v2 4/5] dt-bindings: iio: adc: adi,ad7606: add gain calibration support

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

Add gain calibration support by a per-channel resistor value.

Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
 .../devicetree/bindings/iio/adc/adi,ad7606.yaml    | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
index 29f12d650442b8ff2eb455306ce59a0e87867ddd..d4b8ea51f60be367e79a4db18d932cbca9c7dc91 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
@@ -204,6 +204,15 @@ patternProperties:
           considered a bipolar differential channel. Otherwise it is bipolar
           single-ended.
 
+      adi,rfilter-ohms:
+        description:
+          For ADCs that supports gain calibration, this property must be set to
+          the value of the external RFilter resistor. Proper gain error
+          correction is applied based on this value.
+        default: 0
+        minimum: 0
+        maximum: 65536
+
     required:
       - reg
       - bipolar
@@ -256,6 +265,25 @@ allOf:
       properties:
         adi,oversampling-ratio-gpios: false
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,ad7605-4
+              - adi,ad7606-4
+              - adi,ad7606-6
+              - adi,ad7606-8
+              - adi,ad7607
+              - adi,ad7608
+              - adi,ad7609
+              - adi,ad7616
+    then:
+      patternProperties:
+        "^channel@[0-9a-f]+$":
+          properties:
+            adi,rfilter-ohms: false
+
   - if:
       properties:
         compatible:
@@ -398,6 +426,7 @@ examples:
                 reg = <8>;
                 diff-channels = <8 8>;
                 bipolar;
+                adi,rfilter-ohms = <2048>;
             };
 
         };

-- 
2.49.0

Re: [PATCH v2 4/5] dt-bindings: iio: adc: adi,ad7606: add gain calibration support

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2431 bytes --]

On Fri, May 02, 2025 at 03:27:01PM +0200, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Add gain calibration support by a per-channel resistor value.
> 
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
>  .../devicetree/bindings/iio/adc/adi,ad7606.yaml    | 29 ++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> index 29f12d650442b8ff2eb455306ce59a0e87867ddd..d4b8ea51f60be367e79a4db18d932cbca9c7dc91 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> @@ -204,6 +204,15 @@ patternProperties:
>            considered a bipolar differential channel. Otherwise it is bipolar
>            single-ended.
>  
> +      adi,rfilter-ohms:
> +        description:
> +          For ADCs that supports gain calibration, this property must be set to
> +          the value of the external RFilter resistor. Proper gain error
> +          correction is applied based on this value.
> +        default: 0
> +        minimum: 0

Perhaps obvious to those working on these ADCs, but this is a series
resistor so a default of zero for the existing devices makes sense.
Acked-by: Conor Dooley <conor.dooley@microchip.com>

> +        maximum: 65536
> +
>      required:
>        - reg
>        - bipolar
> @@ -256,6 +265,25 @@ allOf:
>        properties:
>          adi,oversampling-ratio-gpios: false
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - adi,ad7605-4
> +              - adi,ad7606-4
> +              - adi,ad7606-6
> +              - adi,ad7606-8
> +              - adi,ad7607
> +              - adi,ad7608
> +              - adi,ad7609
> +              - adi,ad7616
> +    then:
> +      patternProperties:
> +        "^channel@[0-9a-f]+$":
> +          properties:
> +            adi,rfilter-ohms: false
> +
>    - if:
>        properties:
>          compatible:
> @@ -398,6 +426,7 @@ examples:
>                  reg = <8>;
>                  diff-channels = <8 8>;
>                  bipolar;
> +                adi,rfilter-ohms = <2048>;
>              };
>  
>          };
> 
> -- 
> 2.49.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 4/5] dt-bindings: iio: adc: adi,ad7606: add gain calibration support

[David Lechner]

On 5/2/25 8:27 AM, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Add gain calibration support by a per-channel resistor value.
> 
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
>  .../devicetree/bindings/iio/adc/adi,ad7606.yaml    | 29 ++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> index 29f12d650442b8ff2eb455306ce59a0e87867ddd..d4b8ea51f60be367e79a4db18d932cbca9c7dc91 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> @@ -204,6 +204,15 @@ patternProperties:
>            considered a bipolar differential channel. Otherwise it is bipolar
>            single-ended.
>  
> +      adi,rfilter-ohms:
> +        description:
> +          For ADCs that supports gain calibration, this property must be set to
> +          the value of the external RFilter resistor. Proper gain error
> +          correction is applied based on this value.
> +        default: 0
> +        minimum: 0
> +        maximum: 65536
> +
Max should be 64512.

[PATCH v2 5/5] iio: adc: ad7606: add gain calibration support

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

Add gain calibration support, using resistor values set on devicetree,
values to be set accordingly with ADC external RFilter, as explained in
the ad7606c-16 datasheet, rev0, page 37.

Usage example in the fdt yaml documentation.

Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
 drivers/iio/adc/ad7606.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/iio/adc/ad7606.h |  4 ++++
 2 files changed, 61 insertions(+)

diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index 139d8b3f9bb39dd631a71c70539005d719fb5b7b..a167f080e89c8a8d8accaff5904cce31d860edf9 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -33,6 +33,10 @@
 
 #include "ad7606.h"
 
+#define AD7606_CALIB_GAIN_MIN	0
+#define AD7606_CALIB_GAIN_STEP	1024
+#define AD7606_CALIB_GAIN_MAX	(63 * AD7606_CALIB_GAIN_STEP)
+
 /*
  * Scales are computed as 5000/32768 and 10000/32768 respectively,
  * so that when applied to the raw values they provide mV values.
@@ -125,6 +129,8 @@ static int ad7609_chan_scale_setup(struct iio_dev *indio_dev,
 				   struct iio_chan_spec *chan);
 static int ad7616_sw_mode_setup(struct iio_dev *indio_dev);
 static int ad7606b_sw_mode_setup(struct iio_dev *indio_dev);
+static int ad7606_chan_calib_gain_setup(struct iio_dev *indio_dev,
+					struct iio_chan_spec *chan);
 
 const struct ad7606_chip_info ad7605_4_info = {
 	.max_samplerate = 300 * KILO,
@@ -180,6 +186,7 @@ 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_gain_setup_cb = ad7606_chan_calib_gain_setup,
 	.calib_offset_avail = ad7606_calib_offset_avail,
 	.calib_phase_avail = ad7606b_calib_phase_avail,
 };
@@ -195,6 +202,7 @@ 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_gain_setup_cb = ad7606_chan_calib_gain_setup,
 	.calib_offset_avail = ad7606_calib_offset_avail,
 	.calib_phase_avail = ad7606c_calib_phase_avail,
 };
@@ -246,6 +254,7 @@ 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_gain_setup_cb = ad7606_chan_calib_gain_setup,
 	.calib_offset_avail = ad7606c_18bit_calib_offset_avail,
 	.calib_phase_avail = ad7606c_calib_phase_avail,
 };
@@ -357,6 +366,50 @@ static int ad7606_get_chan_config(struct iio_dev *indio_dev, int ch,
 	return 0;
 }
 
+static int ad7606_chan_calib_gain_setup(struct iio_dev *indio_dev,
+					struct iio_chan_spec *chan)
+{
+	struct ad7606_state *st = iio_priv(indio_dev);
+	unsigned int num_channels = st->chip_info->num_adc_channels;
+	struct device *dev = st->dev;
+	int ret;
+
+	device_for_each_child_node_scoped(dev, child) {
+		int reg, r_gain;
+
+		ret = fwnode_property_read_u32(child, "reg", &reg);
+		if (ret)
+			return ret;
+
+		/* channel number (here) is from 1 to num_channels */
+		if (reg < 1 || reg > num_channels) {
+			dev_warn(dev, "invalid ch number (ignoring): %d\n", reg);
+			continue;
+		}
+
+		ret = fwnode_property_read_u32(child, "adi,rfilter-ohms",
+					       &r_gain);
+		if (ret == -EINVAL)
+			/* Keep the default register value. */
+			continue;
+		if (ret)
+			return ret;
+
+		if (r_gain < AD7606_CALIB_GAIN_MIN ||
+		    r_gain > AD7606_CALIB_GAIN_MAX)
+			return dev_err_probe(st->dev, -EINVAL,
+					     "wrong gain calibration value.");
+
+		/* Chan reg is 1-based index. */
+		ret = st->bops->reg_write(st, AD7606_CALIB_GAIN(reg - 1),
+					  r_gain / AD7606_CALIB_GAIN_STEP);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int ad7606c_18bit_chan_scale_setup(struct iio_dev *indio_dev,
 					  struct iio_chan_spec *chan)
 {
@@ -1410,6 +1463,10 @@ static int ad7606_probe_channels(struct iio_dev *indio_dev)
 				chan->info_mask_separate_available |=
 					BIT(IIO_CHAN_INFO_CALIBBIAS) |
 					BIT(IIO_CHAN_INFO_CALIBCONV_DELAY);
+				ret = st->chip_info->calib_gain_setup_cb(
+					indio_dev, chan);
+				if (ret)
+					return ret;
 			}
 
 			/*
diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
index 4c39de36154ffb80dfbb01bb4f812826bdc55967..e9a59d2afafd43e66137599dbd8220cd6b641e61 100644
--- a/drivers/iio/adc/ad7606.h
+++ b/drivers/iio/adc/ad7606.h
@@ -50,6 +50,8 @@ struct ad7606_state;
 typedef int (*ad7606_scale_setup_cb_t)(struct iio_dev *indio_dev,
 				       struct iio_chan_spec *chan);
 typedef int (*ad7606_sw_setup_cb_t)(struct iio_dev *indio_dev);
+typedef int (*ad7606_calib_gain_setup_cb_t)(struct iio_dev *indio_dev,
+					    struct iio_chan_spec *chan);
 
 /**
  * struct ad7606_chip_info - chip specific information
@@ -66,6 +68,7 @@ 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_gain_setup_cb: callback to setup of gain calibration for each channel
  * @calib_offset_avail: pointer to offset calibration range/limits array
  * @calib_phase_avail:  pointer to phase calibration range/limits array
  */
@@ -81,6 +84,7 @@ struct ad7606_chip_info {
 	bool				os_req_reset;
 	unsigned long			init_delay_ms;
 	u8				offload_storagebits;
+	ad7606_calib_gain_setup_cb_t	calib_gain_setup_cb;
 	const int			*calib_offset_avail;
 	const int			(*calib_phase_avail)[2];
 };

-- 
2.49.0

Re: [PATCH v2 5/5] iio: adc: ad7606: add gain calibration support

[Andy Shevchenko]

On Fri, May 02, 2025 at 03:27:02PM +0200, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Add gain calibration support, using resistor values set on devicetree,
> values to be set accordingly with ADC external RFilter, as explained in
> the ad7606c-16 datasheet, rev0, page 37.
> 
> Usage example in the fdt yaml documentation.

...

> +	device_for_each_child_node_scoped(dev, child) {
> +		int reg, r_gain;

Both are defined in DT as unsigned. Are you able to use int as u32 in DT compiler?

> +		ret = fwnode_property_read_u32(child, "reg", &reg);
> +		if (ret)
> +			return ret;
> +
> +		/* channel number (here) is from 1 to num_channels */
> +		if (reg < 1 || reg > num_channels) {
> +			dev_warn(dev, "invalid ch number (ignoring): %d\n", reg);
> +			continue;
> +		}
> +
> +		ret = fwnode_property_read_u32(child, "adi,rfilter-ohms",
> +					       &r_gain);
> +		if (ret == -EINVAL)
> +			/* Keep the default register value. */
> +			continue;
> +		if (ret)
> +			return ret;

> +		if (r_gain < AD7606_CALIB_GAIN_MIN ||
> +		    r_gain > AD7606_CALIB_GAIN_MAX)

Seems like minimum check is not needed. See above why.

> +			return dev_err_probe(st->dev, -EINVAL,
> +					     "wrong gain calibration value.");
> +
> +		/* Chan reg is 1-based index. */
> +		ret = st->bops->reg_write(st, AD7606_CALIB_GAIN(reg - 1),
> +					  r_gain / AD7606_CALIB_GAIN_STEP);

Wonder if we need DIV_ROUND_CLOSEST() instead...

> +		if (ret)
> +			return ret;
> +	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 5/5] iio: adc: ad7606: add gain calibration support

[David Lechner]

On 5/2/25 8:27 AM, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Add gain calibration support, using resistor values set on devicetree,
> values to be set accordingly with ADC external RFilter, as explained in
> the ad7606c-16 datasheet, rev0, page 37.
> 
> Usage example in the fdt yaml documentation.
> 
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---

Tested-by: David Lechner <dlechner@baylibre.com>

Re: [PATCH v2 5/5] iio: adc: ad7606: add gain calibration support

[Nuno Sá]

On Fri, 2025-05-02 at 15:27 +0200, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Add gain calibration support, using resistor values set on devicetree,
> values to be set accordingly with ADC external RFilter, as explained in
> the ad7606c-16 datasheet, rev0, page 37.
> 
> Usage example in the fdt yaml documentation.
> 
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---

Here!

With the the proper type for the DT properties:

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/adc/ad7606.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/iio/adc/ad7606.h |  4 ++++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
> index
> 139d8b3f9bb39dd631a71c70539005d719fb5b7b..a167f080e89c8a8d8accaff5904cce31d860edf9
> 100644
> --- a/drivers/iio/adc/ad7606.c
> +++ b/drivers/iio/adc/ad7606.c
> @@ -33,6 +33,10 @@
>  
>  #include "ad7606.h"
>  
> +#define AD7606_CALIB_GAIN_MIN	0
> +#define AD7606_CALIB_GAIN_STEP	1024
> +#define AD7606_CALIB_GAIN_MAX	(63 * AD7606_CALIB_GAIN_STEP)
> +
>  /*
>   * Scales are computed as 5000/32768 and 10000/32768 respectively,
>   * so that when applied to the raw values they provide mV values.
> @@ -125,6 +129,8 @@ static int ad7609_chan_scale_setup(struct iio_dev *indio_dev,
>  				   struct iio_chan_spec *chan);
>  static int ad7616_sw_mode_setup(struct iio_dev *indio_dev);
>  static int ad7606b_sw_mode_setup(struct iio_dev *indio_dev);
> +static int ad7606_chan_calib_gain_setup(struct iio_dev *indio_dev,
> +					struct iio_chan_spec *chan);
>  
>  const struct ad7606_chip_info ad7605_4_info = {
>  	.max_samplerate = 300 * KILO,
> @@ -180,6 +186,7 @@ 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_gain_setup_cb = ad7606_chan_calib_gain_setup,
>  	.calib_offset_avail = ad7606_calib_offset_avail,
>  	.calib_phase_avail = ad7606b_calib_phase_avail,
>  };
> @@ -195,6 +202,7 @@ 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_gain_setup_cb = ad7606_chan_calib_gain_setup,
>  	.calib_offset_avail = ad7606_calib_offset_avail,
>  	.calib_phase_avail = ad7606c_calib_phase_avail,
>  };
> @@ -246,6 +254,7 @@ 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_gain_setup_cb = ad7606_chan_calib_gain_setup,
>  	.calib_offset_avail = ad7606c_18bit_calib_offset_avail,
>  	.calib_phase_avail = ad7606c_calib_phase_avail,
>  };
> @@ -357,6 +366,50 @@ static int ad7606_get_chan_config(struct iio_dev *indio_dev,
> int ch,
>  	return 0;
>  }
>  
> +static int ad7606_chan_calib_gain_setup(struct iio_dev *indio_dev,
> +					struct iio_chan_spec *chan)
> +{
> +	struct ad7606_state *st = iio_priv(indio_dev);
> +	unsigned int num_channels = st->chip_info->num_adc_channels;
> +	struct device *dev = st->dev;
> +	int ret;
> +
> +	device_for_each_child_node_scoped(dev, child) {
> +		int reg, r_gain;
> +
> +		ret = fwnode_property_read_u32(child, "reg", &reg);
> +		if (ret)
> +			return ret;
> +
> +		/* channel number (here) is from 1 to num_channels */
> +		if (reg < 1 || reg > num_channels) {
> +			dev_warn(dev, "invalid ch number (ignoring): %d\n", reg);
> +			continue;
> +		}
> +
> +		ret = fwnode_property_read_u32(child, "adi,rfilter-ohms",
> +					       &r_gain);
> +		if (ret == -EINVAL)
> +			/* Keep the default register value. */
> +			continue;
> +		if (ret)
> +			return ret;
> +
> +		if (r_gain < AD7606_CALIB_GAIN_MIN ||
> +		    r_gain > AD7606_CALIB_GAIN_MAX)
> +			return dev_err_probe(st->dev, -EINVAL,
> +					     "wrong gain calibration value.");
> +
> +		/* Chan reg is 1-based index. */
> +		ret = st->bops->reg_write(st, AD7606_CALIB_GAIN(reg - 1),
> +					  r_gain / AD7606_CALIB_GAIN_STEP);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static int ad7606c_18bit_chan_scale_setup(struct iio_dev *indio_dev,
>  					  struct iio_chan_spec *chan)
>  {
> @@ -1410,6 +1463,10 @@ static int ad7606_probe_channels(struct iio_dev *indio_dev)
>  				chan->info_mask_separate_available |=
>  					BIT(IIO_CHAN_INFO_CALIBBIAS) |
>  					BIT(IIO_CHAN_INFO_CALIBCONV_DELAY);
> +				ret = st->chip_info->calib_gain_setup_cb(
> +					indio_dev, chan);
> +				if (ret)
> +					return ret;
>  			}
>  
>  			/*
> diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
> index
> 4c39de36154ffb80dfbb01bb4f812826bdc55967..e9a59d2afafd43e66137599dbd8220cd6b641e61
> 100644
> --- a/drivers/iio/adc/ad7606.h
> +++ b/drivers/iio/adc/ad7606.h
> @@ -50,6 +50,8 @@ struct ad7606_state;
>  typedef int (*ad7606_scale_setup_cb_t)(struct iio_dev *indio_dev,
>  				       struct iio_chan_spec *chan);
>  typedef int (*ad7606_sw_setup_cb_t)(struct iio_dev *indio_dev);
> +typedef int (*ad7606_calib_gain_setup_cb_t)(struct iio_dev *indio_dev,
> +					    struct iio_chan_spec *chan);
>  
>  /**
>   * struct ad7606_chip_info - chip specific information
> @@ -66,6 +68,7 @@ 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_gain_setup_cb: callback to setup of gain calibration for each channel
>   * @calib_offset_avail: pointer to offset calibration range/limits array
>   * @calib_phase_avail:  pointer to phase calibration range/limits array
>   */
> @@ -81,6 +84,7 @@ struct ad7606_chip_info {
>  	bool				os_req_reset;
>  	unsigned long			init_delay_ms;
>  	u8				offload_storagebits;
> +	ad7606_calib_gain_setup_cb_t	calib_gain_setup_cb;
>  	const int			*calib_offset_avail;
>  	const int			(*calib_phase_avail)[2];
>  };
>