Re: [PATCH v3 09/10] iio: adc: ad7606: Add iio-backend support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 10/5/24 13:53, Jonathan Cameron wrote:
On Fri, 04 Oct 2024 21:48:43 +0000
Guillaume Stols <gstols@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
---
  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;
+}




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux