diff mbox series

[v10,09/12] iio: adc: ad7768-1: add support for Synchronization over SPI

Message ID 4c8cce58c3f5d84423dffee23c260b94c4cb6c36.1749063024.git.Jonathan.Santos@analog.com
State New
Headers show
Series iio: adc: ad7768-1: Add features, improvements, and fixes | expand

Commit Message

Jonathan Santos June 4, 2025, 7:37 p.m. UTC
The synchronization method using GPIO requires the generated pulse to be
truly synchronous with the base MCLK signal. When it is not possible to
do that in hardware, the datasheet recommends using synchronization over
SPI, where the generated pulse is already synchronous with MCLK. This
requires the SYNC_OUT pin to be connected to the SYNC_IN pin.

Use trigger-sources property to enable device synchronization over SPI
and multi-device synchronization while replacing sync-in-gpios property.

Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
---
v10 Changes:
* Moved gpio delay related changes to the first patch.
* Replaced fwnode_find_reference_args() for  
  fwnode_property_get_reference_args() as discussed in the last version.
  This allows us to discard the new wrapper patch.

v9 Changes:
* Refactored ad7768_trigger_sources_get_sync() again to split the
  trigger-sources setup and to remove the labels/jumps.
* used new fwnode_find_reference_args() to get the trigger-sources
  property with proper cleanup, as recommended.

v8 Changes:
* Putted ad7768_trigger_source_get_gpio() code inline to fix the compatible
  issue.

v7 Changes:
* Added delay in the synchronization pulse via GPIO.
* replaced device_property_present() for fwnode_property_present().
* Refactored ad7768_trigger_sources_get_sync() to avoid excessive jumps.
* Self triggering is enabled only when the trigger-sources property is
  not defined. Added TODO to support other trigger sources when the subsystem
  is available.

v6 Changes:
* Created macro for the SYNC index from trigger-sources.
* Check trigger source by the compatible string (and the dev node for the
  self triggering).
* Check nargs before accessing the args array.
* Use `trigger-sources` as an alternative to `adi,sync-in-gpios`
  (now optional), instead of replacing it. 

v5 Changes:
* Allow omitting trigger-sources property.
* include gpio-trigger to trigger-sources to replace adi,sync-in-gpios
  property.
* Read trigger-sources cell value to differentiate the trigger type.

v4 Changes:
* None.

v3 Changes:
* Fixed args.fwnode leakage in the trigger-sources parsing.
* Synchronization over spi is enabled when the trigger-sources
  references the own device.
* Synchronization is kept within the device, and return error if the
  gpio is not defined and the trigger-sources reference does not match
  the current device. 

v2 Changes:
* Synchronization via SPI is enabled when the Sync GPIO is not defined.
* now trigger-sources property indicates the synchronization provider or
  main device. The main device will be used to drive the SYNC_IN when
  requested (via GPIO or SPI).
---
 drivers/iio/adc/ad7768-1.c | 81 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 79 insertions(+), 2 deletions(-)

Comments

Andy Shevchenko June 5, 2025, 6:10 a.m. UTC | #1
On Wed, Jun 04, 2025 at 04:37:09PM -0300, Jonathan Santos wrote:
> The synchronization method using GPIO requires the generated pulse to be
> truly synchronous with the base MCLK signal. When it is not possible to
> do that in hardware, the datasheet recommends using synchronization over
> SPI, where the generated pulse is already synchronous with MCLK. This
> requires the SYNC_OUT pin to be connected to the SYNC_IN pin.
> 
> Use trigger-sources property to enable device synchronization over SPI
> and multi-device synchronization while replacing sync-in-gpios property.

...

> +static int ad7768_trigger_sources_sync_setup(struct device *dev,
> +					     struct fwnode_handle *fwnode,
> +					     struct ad7768_state *st)
> +{
> +	struct fwnode_reference_args args;
> +	struct fwnode_handle *ref __free(fwnode_handle) = NULL;
> +	int ret;
> +
> +	ret = fwnode_property_get_reference_args(fwnode, "trigger-sources",
> +						 "#trigger-source-cells", 0,
> +						 AD7768_TRIGGER_SOURCE_SYNC_IDX, &args);
> +	if (ret)
> +		return ret;
> +
> +	ref = args.fwnode;
> +	/* First, try getting the GPIO trigger source */
> +	if (fwnode_device_is_compatible(ref, "gpio-trigger")) {

> +		st->gpio_sync_in = devm_fwnode_gpiod_get_index(dev, ref, NULL,
> +							       0,

This 0 is close to NULL semantically, please move it to the above line to make
the split more logical.

> +							       GPIOD_OUT_LOW,
> +							       "sync-in");
> +		return PTR_ERR_OR_ZERO(st->gpio_sync_in);
> +	}
> +
> +	/*
> +	 * TODO: Support the other cases when we have a trigger subsystem
> +	 * to reliably handle other types of devices as trigger sources.
> +	 *
> +	 * For now, return an error message. For self triggering, omit the
> +	 * trigger-sources property.
> +	 */
> +	return dev_err_probe(dev, -EOPNOTSUPP, "Invalid synchronization trigger source\n");
> +}
diff mbox series

Patch

diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index 55913763313d..b2f69bb8195f 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -29,6 +29,8 @@ 
 #include <linux/iio/triggered_buffer.h>
 #include <linux/iio/trigger_consumer.h>
 
+#include <dt-bindings/iio/adc/adi,ad7768-1.h>
+
 /* AD7768 registers definition */
 #define AD7768_REG_CHIP_TYPE		0x3
 #define AD7768_REG_PROD_ID_L		0x4
@@ -101,6 +103,8 @@ 
 
 #define AD7768_VCM_OFF			0x07
 
+#define AD7768_TRIGGER_SOURCE_SYNC_IDX 0
+
 enum ad7768_conv_mode {
 	AD7768_CONTINUOUS,
 	AD7768_ONE_SHOT,
@@ -211,6 +215,7 @@  struct ad7768_state {
 	struct gpio_desc *gpio_reset;
 	const char *labels[ARRAY_SIZE(ad7768_channels)];
 	struct gpio_chip gpiochip;
+	bool en_spi_sync;
 	/*
 	 * DMA (thus cache coherency maintenance) may require the
 	 * transfer buffers to live in their own cache lines.
@@ -298,6 +303,9 @@  static const struct regmap_config ad7768_regmap24_config = {
 
 static int ad7768_send_sync_pulse(struct ad7768_state *st)
 {
+	if (st->en_spi_sync)
+		return regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x00);
+
 	/*
 	 * The datasheet specifies a minimum SYNC_IN pulse width of 1.5 × Tmclk,
 	 * where Tmclk is the MCLK period. The supported MCLK frequencies range
@@ -687,6 +695,64 @@  static const struct iio_info ad7768_info = {
 	.debugfs_reg_access = &ad7768_reg_access,
 };
 
+static int ad7768_trigger_sources_sync_setup(struct device *dev,
+					     struct fwnode_handle *fwnode,
+					     struct ad7768_state *st)
+{
+	struct fwnode_reference_args args;
+	struct fwnode_handle *ref __free(fwnode_handle) = NULL;
+	int ret;
+
+	ret = fwnode_property_get_reference_args(fwnode, "trigger-sources",
+						 "#trigger-source-cells", 0,
+						 AD7768_TRIGGER_SOURCE_SYNC_IDX, &args);
+	if (ret)
+		return ret;
+
+	ref = args.fwnode;
+	/* First, try getting the GPIO trigger source */
+	if (fwnode_device_is_compatible(ref, "gpio-trigger")) {
+		st->gpio_sync_in = devm_fwnode_gpiod_get_index(dev, ref, NULL,
+							       0,
+							       GPIOD_OUT_LOW,
+							       "sync-in");
+		return PTR_ERR_OR_ZERO(st->gpio_sync_in);
+	}
+
+	/*
+	 * TODO: Support the other cases when we have a trigger subsystem
+	 * to reliably handle other types of devices as trigger sources.
+	 *
+	 * For now, return an error message. For self triggering, omit the
+	 * trigger-sources property.
+	 */
+	return dev_err_probe(dev, -EOPNOTSUPP, "Invalid synchronization trigger source\n");
+}
+
+static int ad7768_trigger_sources_get_sync(struct device *dev,
+					   struct ad7768_state *st)
+{
+	struct fwnode_handle *fwnode = dev_fwnode(dev);
+
+	/*
+	 * The AD7768-1 allows two primary methods for driving the SYNC_IN pin
+	 * to synchronize one or more devices:
+	 * 1. Using an external GPIO.
+	 * 2. Using a SPI command, where the SYNC_OUT pin generates a
+	 *    synchronization pulse that drives the SYNC_IN pin.
+	 */
+	if (fwnode_property_present(fwnode, "trigger-sources"))
+		return ad7768_trigger_sources_sync_setup(dev, fwnode, st);
+
+	/*
+	 * In the absence of trigger-sources property, enable self
+	 * synchronization over SPI (SYNC_OUT).
+	 */
+	st->en_spi_sync = true;
+
+	return 0;
+}
+
 static int ad7768_setup(struct iio_dev *indio_dev)
 {
 	struct ad7768_state *st = iio_priv(indio_dev);
@@ -717,11 +783,22 @@  static int ad7768_setup(struct iio_dev *indio_dev)
 			return ret;
 	}
 
-	st->gpio_sync_in = devm_gpiod_get(&st->spi->dev, "adi,sync-in",
-					  GPIOD_OUT_LOW);
+	/* For backwards compatibility, try the adi,sync-in-gpios property */
+	st->gpio_sync_in = devm_gpiod_get_optional(&st->spi->dev, "adi,sync-in",
+						   GPIOD_OUT_LOW);
 	if (IS_ERR(st->gpio_sync_in))
 		return PTR_ERR(st->gpio_sync_in);
 
+	/*
+	 * If the synchronization is not defined by adi,sync-in-gpios, try the
+	 * trigger-sources.
+	 */
+	if (!st->gpio_sync_in) {
+		ret = ad7768_trigger_sources_get_sync(&st->spi->dev, st);
+		if (ret)
+			return ret;
+	}
+
 	/* Only create a Chip GPIO if flagged for it */
 	if (device_property_read_bool(&st->spi->dev, "gpio-controller")) {
 		ret = ad7768_gpio_init(indio_dev);