- 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     | 124 +++++++++++++++++++++++++++++++++++++------
 drivers/iio/adc/ad7606.h     |  15 ++++++
 drivers/iio/adc/ad7606_par.c |  94 +++++++++++++++++++++++++++++++-
 4 files changed, 219 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 4ab1a3092d88..9b52d5b2c592 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 3666a58f8a6f..d86eb7c3e4f7 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,
@@ -496,6 +498,20 @@ 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;
+
+	if (!st->cnvst_pwm)
+		return -EINVAL;
+
+	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;
@@ -580,11 +596,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);
@@ -625,6 +652,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:
@@ -645,6 +673,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;
 }
@@ -737,6 +773,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;
 	}
@@ -906,14 +946,50 @@ static int ad7606_read_avail(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
+static int ad7606_pwm_buffer_postenable(struct iio_dev *indio_dev)
+{
+	struct ad7606_state *st = iio_priv(indio_dev);
+
+	return ad7606_pwm_set_swing(st);
+}
+
+static int ad7606_pwm_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_pwm_buffer_ops = {
+	.postenable = &ad7606_pwm_buffer_postenable,
+	.predisable = &ad7606_pwm_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 = {
@@ -921,6 +997,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 = {
@@ -929,6 +1006,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 = {
@@ -936,6 +1014,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 = {
@@ -943,6 +1022,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 = {
@@ -1062,8 +1142,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");
@@ -1108,7 +1186,24 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
 					       st->cnvst_pwm);
 		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->setup_ops = &ad7606_pwm_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));
@@ -1126,15 +1221,14 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
 						      &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_request_threaded_irq(dev, irq,
-					NULL,
-					&ad7606_interrupt,
-					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
-					chip_info->name, indio_dev);
-	if (ret)
-		return ret;
-
 	return devm_iio_device_register(dev, indio_dev);
 }
 EXPORT_SYMBOL_NS_GPL(ad7606_probe, IIO_AD7606);
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..6042f6799272 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,83 @@
 #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;
+}
+
+const struct ad7606_bus_ops ad7606_bi_bops = {
+	.iio_backend_config = ad7606_bi_setup_iio_backend,
+	.update_scan_mode = ad7606_bi_update_scan_mode,
+};
+EXPORT_SYMBOL_NS_GPL(ad7606_bi_bops, IIO_AD7606);
+
 static int ad7606_par16_read_block(struct device *dev,
 				   int count, void *buf)
 {
@@ -96,9 +171,23 @@ 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);
+		back = devm_iio_backend_get(&pdev->dev, NULL);
+		if (!IS_ERR(back))
+			/*
+			 * 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 +213,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 +223,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 +243,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);

-- 
2.34.1

Hi Guillaume,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 35307f34d6fef8f9d41a1e8f4f532e4b0a7ee422]

url:    https://github.com/intel-lab-lkp/linux/commits/Guillaume-Stols/iio-adc-ad7606-Fix-typo-in-the-driver-name/20241005-055256
base:   35307f34d6fef8f9d41a1e8f4f532e4b0a7ee422
patch link:    https://lore.kernel.org/r/20241004-ad7606_add_iio_backend_support-v3-9-38757012ce82%40baylibre.com
patch subject: [PATCH v3 09/10] iio: adc: ad7606: Add iio-backend support
config: x86_64-randconfig-123-20241006 (https://download.01.org/0day-ci/archive/20241006/202410061307.IHo3Eizh-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/20241006/202410061307.IHo3Eizh-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/202410061307.IHo3Eizh-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/iio/adc/ad7606_par.c:89:29: sparse: sparse: symbol 'ad7606_bi_bops' was not declared. Should it be static?

vim +/ad7606_bi_bops +89 drivers/iio/adc/ad7606_par.c

    88	
  > 89	const struct ad7606_bus_ops ad7606_bi_bops = {
    90		.iio_backend_config = ad7606_bi_setup_iio_backend,
    91		.update_scan_mode = ad7606_bi_update_scan_mode,
    92	};
    93	EXPORT_SYMBOL_NS_GPL(ad7606_bi_bops, IIO_AD7606);
    94	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

On Fri, 04 Oct 2024 21:48:43 +0000
Guillaume Stols <gstols@baylibre.com> 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.
I don't much like the trivial window between this patch and the next
where the emulated mode is still there but the sleeps aren't adapting with sampling frequency.

Maybe it's worth a dance of leaving the write_raw support
until after this one so the frequency remains fixed until after
the fsleep(2) calls are gone?

There is another bit that I'm unsure is technically correct until after
the next patch.  Maybe I'm reading the diff wrong though!

Thanks,

J

> 
> Signed-off-by: Guillaume Stols <gstols@baylibre.com>
> ---
>  drivers/iio/adc/Kconfig      |   2 +
>  drivers/iio/adc/ad7606.c     | 124 +++++++++++++++++++++++++++++++++++++------
>  drivers/iio/adc/ad7606.h     |  15 ++++++
>  drivers/iio/adc/ad7606_par.c |  94 +++++++++++++++++++++++++++++++-
>  4 files changed, 219 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 4ab1a3092d88..9b52d5b2c592 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 3666a58f8a6f..d86eb7c3e4f7 100644
> --- a/drivers/iio/adc/ad7606.c
> +++ b/drivers/iio/adc/ad7606.c
> @@ -21,6 +21,7 @@

> @@ -737,6 +773,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);

Currently I think  for the !backend + pwm case this can go out of
range for which that code works (fsleep removed in next patch).
Perhaps delay adding this until after that patch.
>  	default:
>  		return -EINVAL;
>  	}

> @@ -1108,7 +1186,24 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
>  					       st->cnvst_pwm);
>  		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->setup_ops = &ad7606_pwm_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));
It's a little hard to unwind the patches, but this was previously in the !pwm case.
At this point in the series we still allow the pwm case to work with ! backend.
So is this now running in that case?   Do we need a temporary additional check
on !pwm


> @@ -1126,15 +1221,14 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
>  						      &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_request_threaded_irq(dev, irq,
> -					NULL,
> -					&ad7606_interrupt,
> -					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> -					chip_info->name, indio_dev);
> -	if (ret)
> -		return ret;
> -
>  	return devm_iio_device_register(dev, indio_dev);
>  }
>  EXPORT_SYMBOL_NS_GPL(ad7606_probe, IIO_AD7606);

> diff --git a/drivers/iio/adc/ad7606_par.c b/drivers/iio/adc/ad7606_par.c
> index b87be2f1ca04..6042f6799272 100644
> --- a/drivers/iio/adc/ad7606_par.c
> +++ b/drivers/iio/adc/ad7606_par.c
> @@ -2,7 +2,8 @@
> +
> +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;

Ultimately this may want to move into the chip_info structures as more devices are added
but this is fine for now I suppose.

> +	indio_dev->num_channels = 8;
> +
> +	return 0;
> +}

On 10/5/24 13:53, Jonathan Cameron wrote:
> On Fri, 04 Oct 2024 21:48:43 +0000
> Guillaume Stols <gstols@baylibre.com> 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.
> I don't much like the trivial window between this patch and the next
> where the emulated mode is still there but the sleeps aren't adapting with sampling frequency.
>
> Maybe it's worth a dance of leaving the write_raw support
> until after this one so the frequency remains fixed until after
> the fsleep(2) calls are gone?
>
> There is another bit that I'm unsure is technically correct until after
> the next patch.  Maybe I'm reading the diff wrong though!
>
> Thanks,
>
> J
>
>> Signed-off-by: Guillaume Stols <gstols@baylibre.com>
>> ---
>>   drivers/iio/adc/Kconfig      |   2 +
>>   drivers/iio/adc/ad7606.c     | 124 +++++++++++++++++++++++++++++++++++++------
>>   drivers/iio/adc/ad7606.h     |  15 ++++++
>>   drivers/iio/adc/ad7606_par.c |  94 +++++++++++++++++++++++++++++++-
>>   4 files changed, 219 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>> index 4ab1a3092d88..9b52d5b2c592 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 3666a58f8a6f..d86eb7c3e4f7 100644
>> --- a/drivers/iio/adc/ad7606.c
>> +++ b/drivers/iio/adc/ad7606.c
>> @@ -21,6 +21,7 @@
>> @@ -737,6 +773,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);
> Currently I think  for the !backend + pwm case this can go out of
> range for which that code works (fsleep removed in next patch).
> Perhaps delay adding this until after that patch.

Hi Jonathan,

The sampling frequency can be adjusted only for the backend version, 
otherwise (including pwm+interrupt), there is no sysfs access to the 
sampling frequency (only available for AD7606_BI_CHANNEL).

>>   	default:
>>   		return -EINVAL;
>>   	}
>> @@ -1108,7 +1186,24 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
>>   					       st->cnvst_pwm);
>>   		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->setup_ops = &ad7606_pwm_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));
> It's a little hard to unwind the patches, but this was previously in the !pwm case.
> At this point in the series we still allow the pwm case to work with ! backend.
> So is this now running in that case?   Do we need a temporary additional check
> on !pwm

mmm actually this should not be in a condition in the PWM  patch. Will 
fix this directly there.

>
>
>> @@ -1126,15 +1221,14 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
>>   						      &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_request_threaded_irq(dev, irq,
>> -					NULL,
>> -					&ad7606_interrupt,
>> -					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
>> -					chip_info->name, indio_dev);
>> -	if (ret)
>> -		return ret;
>> -
>>   	return devm_iio_device_register(dev, indio_dev);
>>   }
>>   EXPORT_SYMBOL_NS_GPL(ad7606_probe, IIO_AD7606);
>> diff --git a/drivers/iio/adc/ad7606_par.c b/drivers/iio/adc/ad7606_par.c
>> index b87be2f1ca04..6042f6799272 100644
>> --- a/drivers/iio/adc/ad7606_par.c
>> +++ b/drivers/iio/adc/ad7606_par.c
>> @@ -2,7 +2,8 @@
>> +
>> +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;
> Ultimately this may want to move into the chip_info structures as more devices are added
> but this is fine for now I suppose.
Will do this in a next series where support is added for the other chips.
>
>> +	indio_dev->num_channels = 8;
>> +
>> +	return 0;
>> +}

> >> diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
> >> index 3666a58f8a6f..d86eb7c3e4f7 100644
> >> --- a/drivers/iio/adc/ad7606.c
> >> +++ b/drivers/iio/adc/ad7606.c
> >> @@ -21,6 +21,7 @@
> >> @@ -737,6 +773,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);  
> > Currently I think  for the !backend + pwm case this can go out of
> > range for which that code works (fsleep removed in next patch).
> > Perhaps delay adding this until after that patch.  
> 
> Hi Jonathan,
> 
> The sampling frequency can be adjusted only for the backend version, 
> otherwise (including pwm+interrupt), there is no sysfs access to the 
> sampling frequency (only available for AD7606_BI_CHANNEL).
Ah! That makes sense.
Thanks,

J