| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ygjn26b4g7ksxayfjs24iovndmgdrq5hvqdmanbsqpbzay3uir@zqmyrckzycps>
Date: Wed, 7 May 2025 10:52:04 +0200
From: Angelo Dureghello <adureghello@...libre.com>
To: Nuno Sá <noname.nuno@...il.com>
Cc: Jonathan Cameron <jic23@...nel.org>,
David Lechner <dlechner@...libre.com>, Nuno Sá <nuno.sa@...log.com>,
Andy Shevchenko <andy@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>,
Michael Hennerich <Michael.Hennerich@...log.com>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, linux-iio@...r.kernel.org,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v3 5/5] iio: adc: ad7606: add gain calibration support
On 07.05.2025 07:14, Nuno Sá wrote:
> On Tue, 2025-05-06 at 23:03 +0200, Angelo Dureghello wrote:
> > From: Angelo Dureghello <adureghello@...libre.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.
> >
> > Tested-by: David Lechner <dlechner@...libre.com>
> > Reviewed-by: Nuno Sá <nuno.sa@...log.com>
> > Signed-off-by: Angelo Dureghello <adureghello@...libre.com>
> > ---
> > drivers/iio/adc/ad7606.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++
> > drivers/iio/adc/ad7606.h | 4 ++++
> > 2 files changed, 60 insertions(+)
> >
> > diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
> > index
> > a986eb1284106da4980ac36cb0b5990e4e3bd948..049fd8616769d32778aa238b348b2fb82fa83745
> > 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,49 @@ 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) {
> > + u32 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, "wrong ch number (ignoring): %d\n", reg);
> > + continue;
> > + }
> > +
>
> Sorry Angelo, just realized this now. Any reason for not treating the above as a real
> invalid argument? It's minor and not a big deal but odd enough...
>
> - Nuno Sá
Hi Nuno,
no problem, feel free to comment,
Main reason is that on a wrong fdt channel node, continue does not stop
the parsing of other maybe correct channel nodes.
Regards,
angelo
>
> > + 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_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),
> > + DIV_ROUND_CLOSEST(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 +1462,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_CONVDELAY);
> > + 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
> > f613583a7fa4095115b0b28e3f8e51cd32b93524..94165d217b69d54cbce9109b8c0f9dc0237cf304
> > 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];
> > };
> >
>
Powered by blists - more mailing lists