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 | 53 ++++++++++++++++++++++++++++++++++++++++++++++++
drivers/iio/adc/ad7606.h | 4 ++++
2 files changed, 57 insertions(+)
diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index ec063dd4a67eb94610c41c473e2d8040c91974cf..1ebc7080d3d153a2ba02bc5c126ef9957dc782ab 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 65536
+
/*
* 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,46 @@ 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", ®);
+ 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)
+ return ret;
+
+ if (r_gain < AD7606_CALIB_GAIN_MIN ||
+ r_gain > AD7606_CALIB_GAIN_MAX)
+ return -EINVAL;
+
+ /* 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 +1459,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_CALIBPHASE_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.0On 4/29/25 8:06 AM, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
>
...
> +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", ®);
> + 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);
Instead of...
> + if (ret)
> + return ret;
... we need:
if (ret == -EINVAL)
r_gain = 0;
else if (ret)
return ret;
Otherwise driver fails to probe if adi,rfilter-ohms is missing.
> +
> + if (r_gain < AD7606_CALIB_GAIN_MIN ||
> + r_gain > AD7606_CALIB_GAIN_MAX)
> + return -EINVAL;
> +
Also, return dev_err_probe() on the returns above would have made debugging
easier.
> + /* 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;
> +}
> +
On 29.04.2025 17:46, David Lechner wrote:
> On 4/29/25 8:06 AM, Angelo Dureghello wrote:
> > From: Angelo Dureghello <adureghello@baylibre.com>
> >
>
> ...
>
> > +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", ®);
> > + 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);
>
> Instead of...
>
> > + if (ret)
> > + return ret;
>
> ... we need:
>
> if (ret == -EINVAL)
> r_gain = 0;
> else if (ret)
> return ret;
>
> Otherwise driver fails to probe if adi,rfilter-ohms is missing.
>
Correct, i changed this before sending and could not catch it.
But not totally sure of applying a 0.
We are here after chip reset. So conceptually, would not apply any default,
ince it is already set after reset. What about:
if (ret == -EINVAL)
contnue;
else if (ret)
return ret;
> > +
> > + if (r_gain < AD7606_CALIB_GAIN_MIN ||
> > + r_gain > AD7606_CALIB_GAIN_MAX)
> > + return -EINVAL;
> > +
>
> Also, return dev_err_probe() on the returns above would have made debugging
> easier.
>
ack
> > + /* 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;
> > +}
> > +
On 5/1/25 8:35 AM, Angelo Dureghello wrote:
> On 29.04.2025 17:46, David Lechner wrote:
>> On 4/29/25 8:06 AM, Angelo Dureghello wrote:
>>> From: Angelo Dureghello <adureghello@baylibre.com>
>>>
>>
>> ...
>>
>>> +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", ®);
>>> + 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);
>>
>> Instead of...
>>
>>> + if (ret)
>>> + return ret;
>>
>> ... we need:
>>
>> if (ret == -EINVAL)
>> r_gain = 0;
>> else if (ret)
>> return ret;
>>
>> Otherwise driver fails to probe if adi,rfilter-ohms is missing.
>>
>
> Correct, i changed this before sending and could not catch it.
> But not totally sure of applying a 0.
> We are here after chip reset. So conceptually, would not apply any default,
> ince it is already set after reset. What about:
>
> if (ret == -EINVAL)
> contnue;
> else if (ret)
> return ret;
Sure. A comment could help here and the continue makes else not needed.
if (ret == -EINVAL)
/* Keep the default register value. */
contnue;
if (ret)
return ret;
>
>>> +
>>> + if (r_gain < AD7606_CALIB_GAIN_MIN ||
>>> + r_gain > AD7606_CALIB_GAIN_MAX)
>>> + return -EINVAL;
>>> +
>>
>> Also, return dev_err_probe() on the returns above would have made debugging
>> easier.
>>
> ack
>
>>> + /* 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;
>>> +}
>>> +
On Tue, Apr 29, 2025 at 4:08 PM Angelo Dureghello <adureghello@baylibre.com> 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. ... > +#define AD7606_CALIB_GAIN_MIN 0 > +#define AD7606_CALIB_GAIN_STEP 1024 > +#define AD7606_CALIB_GAIN_MAX 65536 Are those values in decimal in the datasheet? It looks to me something like _MAX = (64 * _STEP) but is it for real? Usually values are limited by the amount of bits in the HW, here it spans over 65 steps, i.e. 7 bits would be needed... Confusing. -- With Best Regards, Andy Shevchenko
On 30.04.2025 01:34, Andy Shevchenko wrote: > On Tue, Apr 29, 2025 at 4:08 PM Angelo Dureghello > <adureghello@baylibre.com> 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. > > ... > > > +#define AD7606_CALIB_GAIN_MIN 0 > > +#define AD7606_CALIB_GAIN_STEP 1024 > > +#define AD7606_CALIB_GAIN_MAX 65536 > Hi Andy, > Are those values in decimal in the datasheet? > It looks to me something like > > _MAX = (64 * _STEP) > > but is it for real? Usually values are limited by the amount of bits > in the HW, here it spans over 65 steps, i.e. 7 bits would be needed... > Confusing. > true, thanks, there must be a typo in the datasheet that says 0 to 65536 with a step of 1024 with 6 bits. Only 0 to 63 are possbile here. step 0 = 0 step 63 = 64512 Will fix that. Regards, angelo > -- > With Best Regards, > Andy Shevchenko
© 2016 - 2026 Red Hat, Inc.