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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c3e7ea15ea5c3ce14cdd4d99ffa98631e7ce2677.camel@gmail.com>
Date: Sun, 04 May 2025 09:30:31 +0100
From: Nuno Sá <noname.nuno@...il.com>
To: Angelo Dureghello <adureghello@...libre.com>, 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>
Cc: linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org, 
	devicetree@...r.kernel.org
Subject: Re: [PATCH v2 3/5] iio: adc: ad7606: add offset and phase
 calibration support

On Fri, 2025-05-02 at 15:27 +0200, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@...libre.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@...libre.com>
> ---

With the the proper type for the DT properties:

Reviewed-by: Nuno Sá <nuno.sa@...log.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];
>  };
>  
>  /**
> 


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ