diff mbox series

[v5,7/8] iio: adc: ad7606: Add iio-backend support

Message ID 20241015-ad7606_add_iio_backend_support-v5-7-654faf1ae08c@baylibre.com
State Handled Elsewhere
Headers show
Series [v5,1/8] dt-bindings: iio: adc: ad7606: Remove spi-cpha from required | expand

Commit Message

Guillaume Stols Oct. 15, 2024, 1:56 p.m. UTC
- Basic support for iio backend.
- Supports IIO_CHAN_INFO_SAMP_FREQ R/W.
- Only hardware mode is available, and that IIO_CHAN_INFO_RAW is not
  supported if iio-backend mode is selected.

Signed-off-by: Guillaume Stols <gstols@baylibre.com>
---
 drivers/iio/adc/Kconfig      |   2 +
 drivers/iio/adc/ad7606.c     | 154 ++++++++++++++++++++++++++++++++++---------
 drivers/iio/adc/ad7606.h     |  15 +++++
 drivers/iio/adc/ad7606_par.c |  92 +++++++++++++++++++++++++-
 4 files changed, 231 insertions(+), 32 deletions(-)

Comments

Guillaume Stols Oct. 18, 2024, 2:07 p.m. UTC | #1
On 10/15/24 15:56, Guillaume Stols wrote:
> - Basic support for iio backend.
> - Supports IIO_CHAN_INFO_SAMP_FREQ R/W.
> - Only hardware mode is available, and that IIO_CHAN_INFO_RAW is not
>    supported if iio-backend mode is selected.
>
> Signed-off-by: Guillaume Stols <gstols@baylibre.com>
> ---
>   drivers/iio/adc/Kconfig      |   2 +
>   drivers/iio/adc/ad7606.c     | 154 ++++++++++++++++++++++++++++++++++---------
>   drivers/iio/adc/ad7606.h     |  15 +++++
>   drivers/iio/adc/ad7606_par.c |  92 +++++++++++++++++++++++++-
>   4 files changed, 231 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 91873f60322d..a57b5f0bc070 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -224,9 +224,11 @@ config AD7606_IFACE_PARALLEL
>   	tristate "Analog Devices AD7606 ADC driver with parallel interface support"
>   	depends on HAS_IOPORT
>   	select AD7606
> +	select IIO_BACKEND
>   	help
>   	  Say yes here to build parallel interface support for Analog Devices:
>   	  ad7605-4, ad7606, ad7606-6, ad7606-4 analog to digital converters (ADC).
> +	  It also support iio_backended devices for AD7606B.
>   
>   	  To compile this driver as a module, choose M here: the
>   	  module will be called ad7606_par.
> diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
> index 34d377e9ac79..7871552ce5ac 100644
> --- a/drivers/iio/adc/ad7606.c
> +++ b/drivers/iio/adc/ad7606.c
> @@ -21,6 +21,7 @@
>   #include <linux/units.h>
>   #include <linux/util_macros.h>
>   
> +#include <linux/iio/backend.h>
>   #include <linux/iio/buffer.h>
>   #include <linux/iio/iio.h>
>   #include <linux/iio/sysfs.h>
> @@ -191,6 +192,7 @@ EXPORT_SYMBOL_NS_GPL(ad7606_4_info, IIO_AD7606);
>   
>   const struct ad7606_chip_info ad7606b_info = {
>   	.channels = ad7606_channels_16bit,
> +	.max_samplerate = 800 * KILO,
>   	.name = "ad7606b",
>   	.num_adc_channels = 8,
>   	.num_channels = 9,
> @@ -490,6 +492,17 @@ static int ad7606_pwm_set_low(struct ad7606_state *st)
>   	return ret;
>   }
>   
> +static int ad7606_pwm_set_swing(struct ad7606_state *st)
> +{
> +	struct pwm_state cnvst_pwm_state;
> +
> +	pwm_get_state(st->cnvst_pwm, &cnvst_pwm_state);
> +	cnvst_pwm_state.enabled = true;
> +	cnvst_pwm_state.duty_cycle = cnvst_pwm_state.period / 2;
> +
> +	return pwm_apply_might_sleep(st->cnvst_pwm, &cnvst_pwm_state);
> +}
> +
>   static bool ad7606_pwm_is_swinging(struct ad7606_state *st)
>   {
>   	struct pwm_state cnvst_pwm_state;
> @@ -576,11 +589,22 @@ static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch,
>   		if (ret < 0)
>   			return ret;
>   	}
> -	ret = wait_for_completion_timeout(&st->completion,
> -					  msecs_to_jiffies(1000));
> -	if (!ret) {
> -		ret = -ETIMEDOUT;
> -		goto error_ret;
> +
> +	/*
> +	 * If no backend, wait for the interruption on busy pin, otherwise just add
> +	 * a delay to leave time for the data to be available. For now, the latter
> +	 * will not happen because IIO_CHAN_INFO_RAW is not supported for the backend.
> +	 * TODO: Add support for reading a single value when the backend is used.
> +	 */
> +	if (!st->back) {
> +		ret = wait_for_completion_timeout(&st->completion,
> +						  msecs_to_jiffies(1000));
> +		if (!ret) {
> +			ret = -ETIMEDOUT;
> +			goto error_ret;
> +		}
> +	} else {
> +		fsleep(1);
>   	}
>   
>   	ret = ad7606_read_samples(st);
> @@ -620,6 +644,7 @@ static int ad7606_read_raw(struct iio_dev *indio_dev,
>   	int ret, ch = 0;
>   	struct ad7606_state *st = iio_priv(indio_dev);
>   	struct ad7606_chan_scale *cs;
> +	struct pwm_state cnvst_pwm_state;
>   
>   	switch (m) {
>   	case IIO_CHAN_INFO_RAW:
> @@ -640,6 +665,14 @@ static int ad7606_read_raw(struct iio_dev *indio_dev,
>   	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
>   		*val = st->oversampling;
>   		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		/*
> +		 * TODO: return the real frequency intead of the requested one once
> +		 * pwm_get_state_hw comes upstream.
> +		 */
> +		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;
>   	}
>   	return -EINVAL;
>   }
> @@ -732,6 +765,10 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
>   			return ret;
>   
>   		return 0;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		if (val < 0 && val2 != 0)
> +			return -EINVAL;
> +		return ad7606_set_sampling_freq(st, val);
>   	default:
>   		return -EINVAL;
>   	}
> @@ -914,14 +951,50 @@ static int ad7606_read_avail(struct iio_dev *indio_dev,
>   	return -EINVAL;
>   }
>   
> +static int ad7606_backend_buffer_postenable(struct iio_dev *indio_dev)
> +{
> +	struct ad7606_state *st = iio_priv(indio_dev);
> +
> +	return ad7606_pwm_set_swing(st);
> +}
> +
> +static int ad7606_backend_buffer_predisable(struct iio_dev *indio_dev)
> +{
> +	struct ad7606_state *st = iio_priv(indio_dev);
> +
> +	return ad7606_pwm_set_low(st);
> +}
> +
> +static int ad7606_update_scan_mode(struct iio_dev *indio_dev,
> +				   const unsigned long *scan_mask)
> +{
> +	struct ad7606_state *st = iio_priv(indio_dev);
> +
> +	/*
> +	 * The update scan mode is only for iio backend compatible drivers.
> +	 * If the specific update_scan_mode is not defined in the bus ops,
> +	 * just do nothing and return 0.
> +	 */
> +	if (!st->bops->update_scan_mode)
> +		return 0;
> +
> +	return st->bops->update_scan_mode(indio_dev, scan_mask);
> +}
> +
>   static const struct iio_buffer_setup_ops ad7606_buffer_ops = {
>   	.postenable = &ad7606_buffer_postenable,
>   	.predisable = &ad7606_buffer_predisable,
>   };
>   
> +static const struct iio_buffer_setup_ops ad7606_backend_buffer_ops = {
> +	.postenable = &ad7606_backend_buffer_postenable,
> +	.predisable = &ad7606_backend_buffer_predisable,
> +};
> +
>   static const struct iio_info ad7606_info_no_os_or_range = {
>   	.read_raw = &ad7606_read_raw,
>   	.validate_trigger = &ad7606_validate_trigger,
> +	.update_scan_mode = &ad7606_update_scan_mode,
>   };
>   
>   static const struct iio_info ad7606_info_os_and_range = {
> @@ -929,6 +1002,7 @@ static const struct iio_info ad7606_info_os_and_range = {
>   	.write_raw = &ad7606_write_raw,
>   	.attrs = &ad7606_attribute_group_os_and_range,
>   	.validate_trigger = &ad7606_validate_trigger,
> +	.update_scan_mode = &ad7606_update_scan_mode,
>   };
>   
>   static const struct iio_info ad7606_info_sw_mode = {
> @@ -937,6 +1011,7 @@ static const struct iio_info ad7606_info_sw_mode = {
>   	.read_avail = &ad7606_read_avail,
>   	.debugfs_reg_access = &ad7606_reg_access,
>   	.validate_trigger = &ad7606_validate_trigger,
> +	.update_scan_mode = &ad7606_update_scan_mode,
>   };
>   
>   static const struct iio_info ad7606_info_os = {
> @@ -944,6 +1019,7 @@ static const struct iio_info ad7606_info_os = {
>   	.write_raw = &ad7606_write_raw,
>   	.attrs = &ad7606_attribute_group_os,
>   	.validate_trigger = &ad7606_validate_trigger,
> +	.update_scan_mode = &ad7606_update_scan_mode,
>   };
>   
>   static const struct iio_info ad7606_info_range = {
> @@ -951,6 +1027,7 @@ static const struct iio_info ad7606_info_range = {
>   	.write_raw = &ad7606_write_raw,
>   	.attrs = &ad7606_attribute_group_range,
>   	.validate_trigger = &ad7606_validate_trigger,
> +	.update_scan_mode = &ad7606_update_scan_mode,
>   };
>   
>   static const struct iio_trigger_ops ad7606_trigger_ops = {
> @@ -1070,8 +1147,6 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
>   	indio_dev->channels = st->chip_info->channels;
>   	indio_dev->num_channels = st->chip_info->num_channels;
>   
> -	init_completion(&st->completion);
> -
>   	ret = ad7606_reset(st);
>   	if (ret)
>   		dev_warn(st->dev, "failed to RESET: no RESET GPIO specified\n");
> @@ -1118,34 +1193,51 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
>   		if (ret)
>   			return ret;
>   	}
> -	st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d",
> -					  indio_dev->name,
> -					  iio_device_id(indio_dev));
> -	if (!st->trig)
> -		return -ENOMEM;
>   
> -	st->trig->ops = &ad7606_trigger_ops;
> -	iio_trigger_set_drvdata(st->trig, indio_dev);
> -	ret = devm_iio_trigger_register(dev, st->trig);
> -	if (ret)
> -		return ret;
> +	if (st->bops->iio_backend_config) {
> +		/*
> +		 * If there is a backend, the PWM should not overpass the maximum sampling
> +		 * frequency the chip supports.
> +		 */
> +		ret = ad7606_set_sampling_freq(st,
> +					       chip_info->max_samplerate ? : 2 * KILO);
> +		if (ret)
> +			return ret;
> +
> +		ret = st->bops->iio_backend_config(dev, indio_dev);
> +		if (ret)
> +			return ret;
>   
> -	indio_dev->trig = iio_trigger_get(st->trig);
> +		indio_dev->setup_ops = &ad7606_backend_buffer_ops;
> +	} else {
> +		init_completion(&st->completion);
> +		st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d",
> +						  indio_dev->name,
> +						  iio_device_id(indio_dev));
> +		if (!st->trig)
> +			return -ENOMEM;
> +
> +		st->trig->ops = &ad7606_trigger_ops;
> +		iio_trigger_set_drvdata(st->trig, indio_dev);
> +		ret = devm_iio_trigger_register(dev, st->trig);
> +		if (ret)
> +			return ret;
>   
> -	ret = devm_request_threaded_irq(dev, irq,
> -					NULL,
> -					&ad7606_interrupt,
> -					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> -					chip_info->name, indio_dev);
> -	if (ret)
> -		return ret;
> +		indio_dev->trig = iio_trigger_get(st->trig);
>   
> -	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
> -					      &iio_pollfunc_store_time,
> -					      &ad7606_trigger_handler,
> -					      &ad7606_buffer_ops);
> -	if (ret)
> -		return ret;
> +		ret = devm_request_threaded_irq(dev, irq, NULL, &ad7606_interrupt,
> +						IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +						chip_info->name, indio_dev);
> +		if (ret)
> +			return ret;
> +
> +		ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
> +						      &iio_pollfunc_store_time,
> +						      &ad7606_trigger_handler,
> +						      &ad7606_buffer_ops);
> +		if (ret)
> +			return ret;
> +	}
>   
>   	return devm_iio_device_register(dev, indio_dev);
>   }
> diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
> index b26a11b2eba1..2c629a15cc33 100644
> --- a/drivers/iio/adc/ad7606.h
> +++ b/drivers/iio/adc/ad7606.h
> @@ -61,6 +61,12 @@
>   
>   #define AD7616_CHANNEL(num)	AD7606_SW_CHANNEL(num, 16)
>   
> +#define AD7606_BI_CHANNEL(num)				\
> +	AD760X_CHANNEL(num, 0,				\
> +		BIT(IIO_CHAN_INFO_SCALE),		\
> +		BIT(IIO_CHAN_INFO_SAMP_FREQ) |		\
> +		BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), 16)
> +
>   struct ad7606_state;
>   
>   typedef int (*ad7606_scale_setup_cb_t)(struct ad7606_state *st,
> @@ -69,6 +75,7 @@ typedef int (*ad7606_scale_setup_cb_t)(struct ad7606_state *st,
>   /**
>    * struct ad7606_chip_info - chip specific information
>    * @channels:		channel specification
> + * @max_samplerate:	maximum supported samplerate
>    * @name		device name
>    * @num_channels:	number of channels
>    * @num_adc_channels	the number of channels the ADC actually inputs.
> @@ -82,6 +89,7 @@ typedef int (*ad7606_scale_setup_cb_t)(struct ad7606_state *st,
>    */
>   struct ad7606_chip_info {
>   	const struct iio_chan_spec	*channels;
> +	unsigned int			max_samplerate;
>   	const char			*name;
>   	unsigned int			num_adc_channels;
>   	unsigned int			num_channels;
> @@ -152,6 +160,7 @@ struct ad7606_state {
>   	bool				sw_mode_en;
>   	const unsigned int		*oversampling_avail;
>   	unsigned int			num_os_ratios;
> +	struct iio_backend		*back;
>   	int (*write_scale)(struct iio_dev *indio_dev, int ch, int val);
>   	int (*write_os)(struct iio_dev *indio_dev, int val);
>   
> @@ -180,16 +189,21 @@ struct ad7606_state {
>   
>   /**
>    * struct ad7606_bus_ops - driver bus operations
> + * @iio_backend_config	function pointer for configuring the iio_backend for
> + *			the compatibles that use it
>    * @read_block		function pointer for reading blocks of data
>    * @sw_mode_config:	pointer to a function which configured the device
>    *			for software mode
>    * @reg_read	function pointer for reading spi register
>    * @reg_write	function pointer for writing spi register
>    * @write_mask	function pointer for write spi register with mask
> + * @update_scan_mode	function pointer for handling the calls to iio_info's update_scan
> + *			mode when enabling/disabling channels.
>    * @rd_wr_cmd	pointer to the function which calculates the spi address
>    */
>   struct ad7606_bus_ops {
>   	/* more methods added in future? */
> +	int (*iio_backend_config)(struct device *dev, struct iio_dev *indio_dev);
>   	int (*read_block)(struct device *dev, int num, void *data);
>   	int (*sw_mode_config)(struct iio_dev *indio_dev);
>   	int (*reg_read)(struct ad7606_state *st, unsigned int addr);
> @@ -200,6 +214,7 @@ struct ad7606_bus_ops {
>   				 unsigned int addr,
>   				 unsigned long mask,
>   				 unsigned int val);
> +	int (*update_scan_mode)(struct iio_dev *indio_dev, const unsigned long *scan_mask);
>   	u16 (*rd_wr_cmd)(int addr, char isWriteOp);
>   };
>   
> diff --git a/drivers/iio/adc/ad7606_par.c b/drivers/iio/adc/ad7606_par.c
> index b87be2f1ca04..6946ff00e4cb 100644
> --- a/drivers/iio/adc/ad7606_par.c
> +++ b/drivers/iio/adc/ad7606_par.c
> @@ -2,7 +2,8 @@
>   /*
>    * AD7606 Parallel Interface ADC driver
>    *
> - * Copyright 2011 Analog Devices Inc.
> + * Copyright 2011 - 2024 Analog Devices Inc.
> + * Copyright 2024 BayLibre SAS.
>    */
>   
>   #include <linux/err.h>
> @@ -14,9 +15,82 @@
>   #include <linux/property.h>
>   #include <linux/types.h>
>   
> +#include <linux/iio/backend.h>
>   #include <linux/iio/iio.h>
> +
>   #include "ad7606.h"
>   
> +static const struct iio_chan_spec ad7606b_bi_channels[] = {
> +	AD7606_BI_CHANNEL(0),
> +	AD7606_BI_CHANNEL(1),
> +	AD7606_BI_CHANNEL(2),
> +	AD7606_BI_CHANNEL(3),
> +	AD7606_BI_CHANNEL(4),
> +	AD7606_BI_CHANNEL(5),
> +	AD7606_BI_CHANNEL(6),
> +	AD7606_BI_CHANNEL(7),
> +};
> +
> +static int ad7606_bi_update_scan_mode(struct iio_dev *indio_dev, const unsigned long *scan_mask)
> +{
> +	struct ad7606_state *st = iio_priv(indio_dev);
> +	unsigned int c, ret;
> +
> +	for (c = 0; c < indio_dev->num_channels; c++) {
> +		if (test_bit(c, scan_mask))
> +			ret = iio_backend_chan_enable(st->back, c);
> +		else
> +			ret = iio_backend_chan_disable(st->back, c);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ad7606_bi_setup_iio_backend(struct device *dev, struct iio_dev *indio_dev)
> +{
> +	struct ad7606_state *st = iio_priv(indio_dev);
> +	unsigned int ret, c;
> +	struct iio_backend_data_fmt data = {
> +		.sign_extend = true,
> +		.enable = true,
> +	};
> +
> +	st->back = devm_iio_backend_get(dev, NULL);
> +	if (IS_ERR(st->back))
> +		return PTR_ERR(st->back);
> +
> +	/* If the device is iio_backend powered the PWM is mandatory */
> +	if (!st->cnvst_pwm)
> +		return dev_err_probe(st->dev, -EINVAL,
> +				     "A PWM is mandatory when using backend.\n");
> +
> +	ret = devm_iio_backend_request_buffer(dev, st->back, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_iio_backend_enable(dev, st->back);
> +	if (ret)
> +		return ret;
> +
> +	for (c = 0; c < indio_dev->num_channels; c++) {
> +		ret = iio_backend_data_format_set(st->back, c, &data);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	indio_dev->channels = ad7606b_bi_channels;
> +	indio_dev->num_channels = 8;
> +
> +	return 0;
> +}
> +
> +static const struct ad7606_bus_ops ad7606_bi_bops = {
> +	.iio_backend_config = ad7606_bi_setup_iio_backend,
> +	.update_scan_mode = ad7606_bi_update_scan_mode,
> +};
> +
>   static int ad7606_par16_read_block(struct device *dev,
>   				   int count, void *buf)
>   {
> @@ -96,9 +170,22 @@ static int ad7606_par_probe(struct platform_device *pdev)
>   	void __iomem *addr;
>   	resource_size_t remap_size;
>   	int irq;
> +	struct iio_backend *back;

Just spotted that I forgot to remove the declaration. Shall I send 
another series with this correction ?

>   
> +	/*
> +	 * If a firmware node is available (ACPI or DT), platform_device_id is null
> +	 * and we must use get_match_data.
> +	 */
>   	if (dev_fwnode(&pdev->dev)) {
>   		chip_info = device_get_match_data(&pdev->dev);
> +		if (device_property_present(&pdev->dev, "io-backends"))
> +			/*
> +			 * If a backend is available ,call the core probe with backend
> +			 * bops, otherwise use the former bops.
> +			 */
> +			return ad7606_probe(&pdev->dev, 0, NULL,
> +					    chip_info,
> +					    &ad7606_bi_bops);
>   	} else {
>   		id = platform_get_device_id(pdev);
>   		chip_info = (const struct ad7606_chip_info *)id->driver_data;
> @@ -124,6 +211,7 @@ static const struct platform_device_id ad7606_driver_ids[] = {
>   	{ .name	= "ad7606-4", .driver_data = (kernel_ulong_t)&ad7606_4_info, },
>   	{ .name	= "ad7606-6", .driver_data = (kernel_ulong_t)&ad7606_6_info, },
>   	{ .name	= "ad7606-8", .driver_data = (kernel_ulong_t)&ad7606_8_info, },
> +	{ .name	= "ad7606b", .driver_data = (kernel_ulong_t)&ad7606b_info, },
>   	{ }
>   };
>   MODULE_DEVICE_TABLE(platform, ad7606_driver_ids);
> @@ -133,6 +221,7 @@ static const struct of_device_id ad7606_of_match[] = {
>   	{ .compatible = "adi,ad7606-4", .data = &ad7606_4_info },
>   	{ .compatible = "adi,ad7606-6", .data = &ad7606_6_info },
>   	{ .compatible = "adi,ad7606-8", .data = &ad7606_8_info },
> +	{ .compatible = "adi,ad7606b", .data = &ad7606b_info },
>   	{ }
>   };
>   MODULE_DEVICE_TABLE(of, ad7606_of_match);
> @@ -152,3 +241,4 @@ MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
>   MODULE_DESCRIPTION("Analog Devices AD7606 ADC");
>   MODULE_LICENSE("GPL v2");
>   MODULE_IMPORT_NS(IIO_AD7606);
> +MODULE_IMPORT_NS(IIO_BACKEND);
>
kernel test robot Oct. 19, 2024, 12:36 a.m. UTC | #2
Hi Guillaume,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 465644ac29536d10178b5ca4684d0b84765b9fa4]

url:    https://github.com/intel-lab-lkp/linux/commits/Guillaume-Stols/dt-bindings-iio-adc-ad7606-Remove-spi-cpha-from-required/20241015-215831
base:   465644ac29536d10178b5ca4684d0b84765b9fa4
patch link:    https://lore.kernel.org/r/20241015-ad7606_add_iio_backend_support-v5-7-654faf1ae08c%40baylibre.com
patch subject: [PATCH v5 7/8] iio: adc: ad7606: Add iio-backend support
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20241019/202410190802.CLaySBOq-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241019/202410190802.CLaySBOq-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410190802.CLaySBOq-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/iio/adc/ad7606_par.c:173:22: warning: unused variable 'back' [-Wunused-variable]
     173 |         struct iio_backend *back;
         |                             ^~~~
   1 warning generated.


vim +/back +173 drivers/iio/adc/ad7606_par.c

   164	
   165	static int ad7606_par_probe(struct platform_device *pdev)
   166	{
   167		const struct ad7606_chip_info *chip_info;
   168		const struct platform_device_id *id;
   169		struct resource *res;
   170		void __iomem *addr;
   171		resource_size_t remap_size;
   172		int irq;
 > 173		struct iio_backend *back;
   174	
   175		/*
   176		 * If a firmware node is available (ACPI or DT), platform_device_id is null
   177		 * and we must use get_match_data.
   178		 */
   179		if (dev_fwnode(&pdev->dev)) {
   180			chip_info = device_get_match_data(&pdev->dev);
   181			if (device_property_present(&pdev->dev, "io-backends"))
   182				/*
   183				 * If a backend is available ,call the core probe with backend
   184				 * bops, otherwise use the former bops.
   185				 */
   186				return ad7606_probe(&pdev->dev, 0, NULL,
   187						    chip_info,
   188						    &ad7606_bi_bops);
   189		} else {
   190			id = platform_get_device_id(pdev);
   191			chip_info = (const struct ad7606_chip_info *)id->driver_data;
   192		}
   193	
   194		irq = platform_get_irq(pdev, 0);
   195		if (irq < 0)
   196			return irq;
   197	
   198		addr = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
   199		if (IS_ERR(addr))
   200			return PTR_ERR(addr);
   201	
   202		remap_size = resource_size(res);
   203	
   204		return ad7606_probe(&pdev->dev, irq, addr, chip_info,
   205				    remap_size > 1 ? &ad7606_par16_bops :
   206				    &ad7606_par8_bops);
   207	}
   208
Uwe Kleine-König Nov. 22, 2024, 4:39 p.m. UTC | #3
On Tue, Oct 15, 2024 at 01:56:20PM +0000, Guillaume Stols wrote:
> @@ -640,6 +665,14 @@ static int ad7606_read_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
>  		*val = st->oversampling;
>  		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		/*
> +		 * TODO: return the real frequency intead of the requested one once
> +		 * pwm_get_state_hw comes upstream.
> +		 */
> +		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;
>  	}
>  	return -EINVAL;
>  }

Being late to the party as the patch is already applied:

ad7606_set_sampling_freq() uses DIV_ROUND_UP_ULL to determine the period
from freq. So I guess you should a down-rounding div here to calculate
freq from period.

Having said that, pwm_get_state_hw() is in mainline and will be included
in v6.13-rc1.

Best regards
Uwe
diff mbox series

Patch

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 91873f60322d..a57b5f0bc070 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -224,9 +224,11 @@  config AD7606_IFACE_PARALLEL
 	tristate "Analog Devices AD7606 ADC driver with parallel interface support"
 	depends on HAS_IOPORT
 	select AD7606
+	select IIO_BACKEND
 	help
 	  Say yes here to build parallel interface support for Analog Devices:
 	  ad7605-4, ad7606, ad7606-6, ad7606-4 analog to digital converters (ADC).
+	  It also support iio_backended devices for AD7606B.
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad7606_par.
diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index 34d377e9ac79..7871552ce5ac 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -21,6 +21,7 @@ 
 #include <linux/units.h>
 #include <linux/util_macros.h>
 
+#include <linux/iio/backend.h>
 #include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
@@ -191,6 +192,7 @@  EXPORT_SYMBOL_NS_GPL(ad7606_4_info, IIO_AD7606);
 
 const struct ad7606_chip_info ad7606b_info = {
 	.channels = ad7606_channels_16bit,
+	.max_samplerate = 800 * KILO,
 	.name = "ad7606b",
 	.num_adc_channels = 8,
 	.num_channels = 9,
@@ -490,6 +492,17 @@  static int ad7606_pwm_set_low(struct ad7606_state *st)
 	return ret;
 }
 
+static int ad7606_pwm_set_swing(struct ad7606_state *st)
+{
+	struct pwm_state cnvst_pwm_state;
+
+	pwm_get_state(st->cnvst_pwm, &cnvst_pwm_state);
+	cnvst_pwm_state.enabled = true;
+	cnvst_pwm_state.duty_cycle = cnvst_pwm_state.period / 2;
+
+	return pwm_apply_might_sleep(st->cnvst_pwm, &cnvst_pwm_state);
+}
+
 static bool ad7606_pwm_is_swinging(struct ad7606_state *st)
 {
 	struct pwm_state cnvst_pwm_state;
@@ -576,11 +589,22 @@  static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch,
 		if (ret < 0)
 			return ret;
 	}
-	ret = wait_for_completion_timeout(&st->completion,
-					  msecs_to_jiffies(1000));
-	if (!ret) {
-		ret = -ETIMEDOUT;
-		goto error_ret;
+
+	/*
+	 * If no backend, wait for the interruption on busy pin, otherwise just add
+	 * a delay to leave time for the data to be available. For now, the latter
+	 * will not happen because IIO_CHAN_INFO_RAW is not supported for the backend.
+	 * TODO: Add support for reading a single value when the backend is used.
+	 */
+	if (!st->back) {
+		ret = wait_for_completion_timeout(&st->completion,
+						  msecs_to_jiffies(1000));
+		if (!ret) {
+			ret = -ETIMEDOUT;
+			goto error_ret;
+		}
+	} else {
+		fsleep(1);
 	}
 
 	ret = ad7606_read_samples(st);
@@ -620,6 +644,7 @@  static int ad7606_read_raw(struct iio_dev *indio_dev,
 	int ret, ch = 0;
 	struct ad7606_state *st = iio_priv(indio_dev);
 	struct ad7606_chan_scale *cs;
+	struct pwm_state cnvst_pwm_state;
 
 	switch (m) {
 	case IIO_CHAN_INFO_RAW:
@@ -640,6 +665,14 @@  static int ad7606_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
 		*val = st->oversampling;
 		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		/*
+		 * TODO: return the real frequency intead of the requested one once
+		 * pwm_get_state_hw comes upstream.
+		 */
+		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;
 	}
 	return -EINVAL;
 }
@@ -732,6 +765,10 @@  static int ad7606_write_raw(struct iio_dev *indio_dev,
 			return ret;
 
 		return 0;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		if (val < 0 && val2 != 0)
+			return -EINVAL;
+		return ad7606_set_sampling_freq(st, val);
 	default:
 		return -EINVAL;
 	}
@@ -914,14 +951,50 @@  static int ad7606_read_avail(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
+static int ad7606_backend_buffer_postenable(struct iio_dev *indio_dev)
+{
+	struct ad7606_state *st = iio_priv(indio_dev);
+
+	return ad7606_pwm_set_swing(st);
+}
+
+static int ad7606_backend_buffer_predisable(struct iio_dev *indio_dev)
+{
+	struct ad7606_state *st = iio_priv(indio_dev);
+
+	return ad7606_pwm_set_low(st);
+}
+
+static int ad7606_update_scan_mode(struct iio_dev *indio_dev,
+				   const unsigned long *scan_mask)
+{
+	struct ad7606_state *st = iio_priv(indio_dev);
+
+	/*
+	 * The update scan mode is only for iio backend compatible drivers.
+	 * If the specific update_scan_mode is not defined in the bus ops,
+	 * just do nothing and return 0.
+	 */
+	if (!st->bops->update_scan_mode)
+		return 0;
+
+	return st->bops->update_scan_mode(indio_dev, scan_mask);
+}
+
 static const struct iio_buffer_setup_ops ad7606_buffer_ops = {
 	.postenable = &ad7606_buffer_postenable,
 	.predisable = &ad7606_buffer_predisable,
 };
 
+static const struct iio_buffer_setup_ops ad7606_backend_buffer_ops = {
+	.postenable = &ad7606_backend_buffer_postenable,
+	.predisable = &ad7606_backend_buffer_predisable,
+};
+
 static const struct iio_info ad7606_info_no_os_or_range = {
 	.read_raw = &ad7606_read_raw,
 	.validate_trigger = &ad7606_validate_trigger,
+	.update_scan_mode = &ad7606_update_scan_mode,
 };
 
 static const struct iio_info ad7606_info_os_and_range = {
@@ -929,6 +1002,7 @@  static const struct iio_info ad7606_info_os_and_range = {
 	.write_raw = &ad7606_write_raw,
 	.attrs = &ad7606_attribute_group_os_and_range,
 	.validate_trigger = &ad7606_validate_trigger,
+	.update_scan_mode = &ad7606_update_scan_mode,
 };
 
 static const struct iio_info ad7606_info_sw_mode = {
@@ -937,6 +1011,7 @@  static const struct iio_info ad7606_info_sw_mode = {
 	.read_avail = &ad7606_read_avail,
 	.debugfs_reg_access = &ad7606_reg_access,
 	.validate_trigger = &ad7606_validate_trigger,
+	.update_scan_mode = &ad7606_update_scan_mode,
 };
 
 static const struct iio_info ad7606_info_os = {
@@ -944,6 +1019,7 @@  static const struct iio_info ad7606_info_os = {
 	.write_raw = &ad7606_write_raw,
 	.attrs = &ad7606_attribute_group_os,
 	.validate_trigger = &ad7606_validate_trigger,
+	.update_scan_mode = &ad7606_update_scan_mode,
 };
 
 static const struct iio_info ad7606_info_range = {
@@ -951,6 +1027,7 @@  static const struct iio_info ad7606_info_range = {
 	.write_raw = &ad7606_write_raw,
 	.attrs = &ad7606_attribute_group_range,
 	.validate_trigger = &ad7606_validate_trigger,
+	.update_scan_mode = &ad7606_update_scan_mode,
 };
 
 static const struct iio_trigger_ops ad7606_trigger_ops = {
@@ -1070,8 +1147,6 @@  int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
 	indio_dev->channels = st->chip_info->channels;
 	indio_dev->num_channels = st->chip_info->num_channels;
 
-	init_completion(&st->completion);
-
 	ret = ad7606_reset(st);
 	if (ret)
 		dev_warn(st->dev, "failed to RESET: no RESET GPIO specified\n");
@@ -1118,34 +1193,51 @@  int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
 		if (ret)
 			return ret;
 	}
-	st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d",
-					  indio_dev->name,
-					  iio_device_id(indio_dev));
-	if (!st->trig)
-		return -ENOMEM;
 
-	st->trig->ops = &ad7606_trigger_ops;
-	iio_trigger_set_drvdata(st->trig, indio_dev);
-	ret = devm_iio_trigger_register(dev, st->trig);
-	if (ret)
-		return ret;
+	if (st->bops->iio_backend_config) {
+		/*
+		 * If there is a backend, the PWM should not overpass the maximum sampling
+		 * frequency the chip supports.
+		 */
+		ret = ad7606_set_sampling_freq(st,
+					       chip_info->max_samplerate ? : 2 * KILO);
+		if (ret)
+			return ret;
+
+		ret = st->bops->iio_backend_config(dev, indio_dev);
+		if (ret)
+			return ret;
 
-	indio_dev->trig = iio_trigger_get(st->trig);
+		indio_dev->setup_ops = &ad7606_backend_buffer_ops;
+	} else {
+		init_completion(&st->completion);
+		st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d",
+						  indio_dev->name,
+						  iio_device_id(indio_dev));
+		if (!st->trig)
+			return -ENOMEM;
+
+		st->trig->ops = &ad7606_trigger_ops;
+		iio_trigger_set_drvdata(st->trig, indio_dev);
+		ret = devm_iio_trigger_register(dev, st->trig);
+		if (ret)
+			return ret;
 
-	ret = devm_request_threaded_irq(dev, irq,
-					NULL,
-					&ad7606_interrupt,
-					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
-					chip_info->name, indio_dev);
-	if (ret)
-		return ret;
+		indio_dev->trig = iio_trigger_get(st->trig);
 
-	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
-					      &iio_pollfunc_store_time,
-					      &ad7606_trigger_handler,
-					      &ad7606_buffer_ops);
-	if (ret)
-		return ret;
+		ret = devm_request_threaded_irq(dev, irq, NULL, &ad7606_interrupt,
+						IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+						chip_info->name, indio_dev);
+		if (ret)
+			return ret;
+
+		ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
+						      &iio_pollfunc_store_time,
+						      &ad7606_trigger_handler,
+						      &ad7606_buffer_ops);
+		if (ret)
+			return ret;
+	}
 
 	return devm_iio_device_register(dev, indio_dev);
 }
diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
index b26a11b2eba1..2c629a15cc33 100644
--- a/drivers/iio/adc/ad7606.h
+++ b/drivers/iio/adc/ad7606.h
@@ -61,6 +61,12 @@ 
 
 #define AD7616_CHANNEL(num)	AD7606_SW_CHANNEL(num, 16)
 
+#define AD7606_BI_CHANNEL(num)				\
+	AD760X_CHANNEL(num, 0,				\
+		BIT(IIO_CHAN_INFO_SCALE),		\
+		BIT(IIO_CHAN_INFO_SAMP_FREQ) |		\
+		BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), 16)
+
 struct ad7606_state;
 
 typedef int (*ad7606_scale_setup_cb_t)(struct ad7606_state *st,
@@ -69,6 +75,7 @@  typedef int (*ad7606_scale_setup_cb_t)(struct ad7606_state *st,
 /**
  * struct ad7606_chip_info - chip specific information
  * @channels:		channel specification
+ * @max_samplerate:	maximum supported samplerate
  * @name		device name
  * @num_channels:	number of channels
  * @num_adc_channels	the number of channels the ADC actually inputs.
@@ -82,6 +89,7 @@  typedef int (*ad7606_scale_setup_cb_t)(struct ad7606_state *st,
  */
 struct ad7606_chip_info {
 	const struct iio_chan_spec	*channels;
+	unsigned int			max_samplerate;
 	const char			*name;
 	unsigned int			num_adc_channels;
 	unsigned int			num_channels;
@@ -152,6 +160,7 @@  struct ad7606_state {
 	bool				sw_mode_en;
 	const unsigned int		*oversampling_avail;
 	unsigned int			num_os_ratios;
+	struct iio_backend		*back;
 	int (*write_scale)(struct iio_dev *indio_dev, int ch, int val);
 	int (*write_os)(struct iio_dev *indio_dev, int val);
 
@@ -180,16 +189,21 @@  struct ad7606_state {
 
 /**
  * struct ad7606_bus_ops - driver bus operations
+ * @iio_backend_config	function pointer for configuring the iio_backend for
+ *			the compatibles that use it
  * @read_block		function pointer for reading blocks of data
  * @sw_mode_config:	pointer to a function which configured the device
  *			for software mode
  * @reg_read	function pointer for reading spi register
  * @reg_write	function pointer for writing spi register
  * @write_mask	function pointer for write spi register with mask
+ * @update_scan_mode	function pointer for handling the calls to iio_info's update_scan
+ *			mode when enabling/disabling channels.
  * @rd_wr_cmd	pointer to the function which calculates the spi address
  */
 struct ad7606_bus_ops {
 	/* more methods added in future? */
+	int (*iio_backend_config)(struct device *dev, struct iio_dev *indio_dev);
 	int (*read_block)(struct device *dev, int num, void *data);
 	int (*sw_mode_config)(struct iio_dev *indio_dev);
 	int (*reg_read)(struct ad7606_state *st, unsigned int addr);
@@ -200,6 +214,7 @@  struct ad7606_bus_ops {
 				 unsigned int addr,
 				 unsigned long mask,
 				 unsigned int val);
+	int (*update_scan_mode)(struct iio_dev *indio_dev, const unsigned long *scan_mask);
 	u16 (*rd_wr_cmd)(int addr, char isWriteOp);
 };
 
diff --git a/drivers/iio/adc/ad7606_par.c b/drivers/iio/adc/ad7606_par.c
index b87be2f1ca04..6946ff00e4cb 100644
--- a/drivers/iio/adc/ad7606_par.c
+++ b/drivers/iio/adc/ad7606_par.c
@@ -2,7 +2,8 @@ 
 /*
  * AD7606 Parallel Interface ADC driver
  *
- * Copyright 2011 Analog Devices Inc.
+ * Copyright 2011 - 2024 Analog Devices Inc.
+ * Copyright 2024 BayLibre SAS.
  */
 
 #include <linux/err.h>
@@ -14,9 +15,82 @@ 
 #include <linux/property.h>
 #include <linux/types.h>
 
+#include <linux/iio/backend.h>
 #include <linux/iio/iio.h>
+
 #include "ad7606.h"
 
+static const struct iio_chan_spec ad7606b_bi_channels[] = {
+	AD7606_BI_CHANNEL(0),
+	AD7606_BI_CHANNEL(1),
+	AD7606_BI_CHANNEL(2),
+	AD7606_BI_CHANNEL(3),
+	AD7606_BI_CHANNEL(4),
+	AD7606_BI_CHANNEL(5),
+	AD7606_BI_CHANNEL(6),
+	AD7606_BI_CHANNEL(7),
+};
+
+static int ad7606_bi_update_scan_mode(struct iio_dev *indio_dev, const unsigned long *scan_mask)
+{
+	struct ad7606_state *st = iio_priv(indio_dev);
+	unsigned int c, ret;
+
+	for (c = 0; c < indio_dev->num_channels; c++) {
+		if (test_bit(c, scan_mask))
+			ret = iio_backend_chan_enable(st->back, c);
+		else
+			ret = iio_backend_chan_disable(st->back, c);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int ad7606_bi_setup_iio_backend(struct device *dev, struct iio_dev *indio_dev)
+{
+	struct ad7606_state *st = iio_priv(indio_dev);
+	unsigned int ret, c;
+	struct iio_backend_data_fmt data = {
+		.sign_extend = true,
+		.enable = true,
+	};
+
+	st->back = devm_iio_backend_get(dev, NULL);
+	if (IS_ERR(st->back))
+		return PTR_ERR(st->back);
+
+	/* If the device is iio_backend powered the PWM is mandatory */
+	if (!st->cnvst_pwm)
+		return dev_err_probe(st->dev, -EINVAL,
+				     "A PWM is mandatory when using backend.\n");
+
+	ret = devm_iio_backend_request_buffer(dev, st->back, indio_dev);
+	if (ret)
+		return ret;
+
+	ret = devm_iio_backend_enable(dev, st->back);
+	if (ret)
+		return ret;
+
+	for (c = 0; c < indio_dev->num_channels; c++) {
+		ret = iio_backend_data_format_set(st->back, c, &data);
+		if (ret)
+			return ret;
+	}
+
+	indio_dev->channels = ad7606b_bi_channels;
+	indio_dev->num_channels = 8;
+
+	return 0;
+}
+
+static const struct ad7606_bus_ops ad7606_bi_bops = {
+	.iio_backend_config = ad7606_bi_setup_iio_backend,
+	.update_scan_mode = ad7606_bi_update_scan_mode,
+};
+
 static int ad7606_par16_read_block(struct device *dev,
 				   int count, void *buf)
 {
@@ -96,9 +170,22 @@  static int ad7606_par_probe(struct platform_device *pdev)
 	void __iomem *addr;
 	resource_size_t remap_size;
 	int irq;
+	struct iio_backend *back;
 
+	/*
+	 * If a firmware node is available (ACPI or DT), platform_device_id is null
+	 * and we must use get_match_data.
+	 */
 	if (dev_fwnode(&pdev->dev)) {
 		chip_info = device_get_match_data(&pdev->dev);
+		if (device_property_present(&pdev->dev, "io-backends"))
+			/*
+			 * If a backend is available ,call the core probe with backend
+			 * bops, otherwise use the former bops.
+			 */
+			return ad7606_probe(&pdev->dev, 0, NULL,
+					    chip_info,
+					    &ad7606_bi_bops);
 	} else {
 		id = platform_get_device_id(pdev);
 		chip_info = (const struct ad7606_chip_info *)id->driver_data;
@@ -124,6 +211,7 @@  static const struct platform_device_id ad7606_driver_ids[] = {
 	{ .name	= "ad7606-4", .driver_data = (kernel_ulong_t)&ad7606_4_info, },
 	{ .name	= "ad7606-6", .driver_data = (kernel_ulong_t)&ad7606_6_info, },
 	{ .name	= "ad7606-8", .driver_data = (kernel_ulong_t)&ad7606_8_info, },
+	{ .name	= "ad7606b", .driver_data = (kernel_ulong_t)&ad7606b_info, },
 	{ }
 };
 MODULE_DEVICE_TABLE(platform, ad7606_driver_ids);
@@ -133,6 +221,7 @@  static const struct of_device_id ad7606_of_match[] = {
 	{ .compatible = "adi,ad7606-4", .data = &ad7606_4_info },
 	{ .compatible = "adi,ad7606-6", .data = &ad7606_6_info },
 	{ .compatible = "adi,ad7606-8", .data = &ad7606_8_info },
+	{ .compatible = "adi,ad7606b", .data = &ad7606b_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ad7606_of_match);
@@ -152,3 +241,4 @@  MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
 MODULE_DESCRIPTION("Analog Devices AD7606 ADC");
 MODULE_LICENSE("GPL v2");
 MODULE_IMPORT_NS(IIO_AD7606);
+MODULE_IMPORT_NS(IIO_BACKEND);