[PATCH v2 00/13] Add support for AD4080 ADC

[PATCH v2 00/13] Add support for AD4080 ADC

[Antoniu Miclaus]

The AD4080 is a high-speed, low noise, low distortion, 20-bit, Easy
Drive, successive approximation register (SAR) analog-to-digital
converter (ADC). Maintaining high performance (signal-to-noise and
distortion (SINAD) ratio > 90 dBFS) at signal frequencies in excess
of 1 MHz enables the AD4080 to service a wide variety of precision,
wide bandwidth data acquisition applications.

This driver aims to be extended in the future to support multiple parts that are
not released yet:
    AD4081
    AD4082
    AD4083
    AD4084
    AD4085
    AD4086
    AD4087
    AD4088


Antoniu Miclaus (13):
  iio: backend: add support for filter config
  iio: backend: add support for sync process
  iio: backend: add support for self sync
  iio: backend: add support for sync status
  iio: backend: add support for number of lanes
  dt-bindings: iio: adc: add ad408x axi variant
  iio: adc: adi-axi-adc: add filter enable/disable
  iio: adc: adi-axi-adc: add bitslip enable/disable
  iio: adc: adi-axi-adc: add self sync support
  iio: adc: adi-axi-adc: add sync status
  iio: adc: adi-axi-adc: add num lanes support
  dt-bindings: iio: adc: add ad4080
  iio: adc: ad4080: add driver support

 .../bindings/iio/adc/adi,ad4080.yaml          |  96 +++
 .../bindings/iio/adc/adi,axi-adc.yaml         |   2 +
 drivers/iio/adc/Kconfig                       |  14 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/ad4080.c                      | 653 ++++++++++++++++++
 drivers/iio/adc/adi-axi-adc.c                 | 115 +++
 drivers/iio/industrialio-backend.c            | 113 +++
 include/linux/iio/backend.h                   |  24 +
 8 files changed, 1018 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4080.yaml
 create mode 100644 drivers/iio/adc/ad4080.c

-- 
2.49.0

[PATCH v2 01/13] iio: backend: add support for filter config

[Antoniu Miclaus]

Add backend support for digital filter enable/disable.

This setting can be adjusted within the IP cores interfacing devices.

The IP core can be configured based on the state of the actual
digital filter configuration of the part.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v2:
 - improve commit description
 drivers/iio/industrialio-backend.c | 26 ++++++++++++++++++++++++++
 include/linux/iio/backend.h        |  6 ++++++
 2 files changed, 32 insertions(+)

diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index d4ad36f54090..ffafe7c73508 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -778,6 +778,32 @@ static int __devm_iio_backend_get(struct device *dev, struct iio_backend *back)
 	return 0;
 }
 
+/**
+ * iio_backend_filter_enable - Enable filter
+ * @back: Backend device
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_filter_enable(struct iio_backend *back)
+{
+	return iio_backend_op_call(back, filter_enable);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_filter_enable, "IIO_BACKEND");
+
+/**
+ * iio_backend_filter_disable - Disable filter
+ * @back: Backend device
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_filter_disable(struct iio_backend *back)
+{
+	return iio_backend_op_call(back, filter_disable);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_filter_disable, "IIO_BACKEND");
+
 /**
  * iio_backend_ddr_enable - Enable interface DDR (Double Data Rate) mode
  * @back: Backend device
diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
index e45b7dfbec35..7987d9f1cdb3 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -100,6 +100,8 @@ enum iio_backend_interface_type {
  * @read_raw: Read a channel attribute from a backend device
  * @debugfs_print_chan_status: Print channel status into a buffer.
  * @debugfs_reg_access: Read or write register value of backend.
+ * @filter_enable: Enable filter.
+ * @filter_disable: Disable filter.
  * @ddr_enable: Enable interface DDR (Double Data Rate) mode.
  * @ddr_disable: Disable interface DDR (Double Data Rate) mode.
  * @data_stream_enable: Enable data stream.
@@ -150,6 +152,8 @@ struct iio_backend_ops {
 					 size_t len);
 	int (*debugfs_reg_access)(struct iio_backend *back, unsigned int reg,
 				  unsigned int writeval, unsigned int *readval);
+	int (*filter_enable)(struct iio_backend *back);
+	int (*filter_disable)(struct iio_backend *back);
 	int (*ddr_enable)(struct iio_backend *back);
 	int (*ddr_disable)(struct iio_backend *back);
 	int (*data_stream_enable)(struct iio_backend *back);
@@ -190,6 +194,8 @@ int iio_backend_data_sample_trigger(struct iio_backend *back,
 int devm_iio_backend_request_buffer(struct device *dev,
 				    struct iio_backend *back,
 				    struct iio_dev *indio_dev);
+int iio_backend_filter_enable(struct iio_backend *back);
+int iio_backend_filter_disable(struct iio_backend *back);
 int iio_backend_ddr_enable(struct iio_backend *back);
 int iio_backend_ddr_disable(struct iio_backend *back);
 int iio_backend_data_stream_enable(struct iio_backend *back);
-- 
2.49.0

[PATCH v2 02/13] iio: backend: add support for sync process

[Antoniu Miclaus]

Add backend support for enabling/disabling the sync process for devices
that have data capture synchronization done through bit-slip (correct
data allignment) procedure and not through and external hardware pin.
This setting can be adjusted within the IP cores interfacing devices.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v2:
 - rename function for better clarity.
 - improve commit description
 drivers/iio/industrialio-backend.c | 26 ++++++++++++++++++++++++++
 include/linux/iio/backend.h        |  6 ++++++
 2 files changed, 32 insertions(+)

diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index ffafe7c73508..60578267643d 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -804,6 +804,32 @@ int iio_backend_filter_disable(struct iio_backend *back)
 }
 EXPORT_SYMBOL_NS_GPL(iio_backend_filter_disable, "IIO_BACKEND");
 
+/**
+ * iio_backend_data_alignment_enable - Enable the sync process.
+ * @back: Backend device
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_data_alignment_enable(struct iio_backend *back)
+{
+	return iio_backend_op_call(back, data_alignment_enable);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_data_alignment_enable, "IIO_BACKEND");
+
+/**
+ * iio_backend_data_alignment_disable - Disable the sync process.
+ * @back: Backend device
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_data_alignment_disable(struct iio_backend *back)
+{
+	return iio_backend_op_call(back, data_alignment_disable);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_data_alignment_disable, "IIO_BACKEND");
+
 /**
  * iio_backend_ddr_enable - Enable interface DDR (Double Data Rate) mode
  * @back: Backend device
diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
index 7987d9f1cdb3..beff66d18151 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -102,6 +102,8 @@ enum iio_backend_interface_type {
  * @debugfs_reg_access: Read or write register value of backend.
  * @filter_enable: Enable filter.
  * @filter_disable: Disable filter.
+ * @data_alignment_enable: Enable sync process.
+ * @data_alignment_disable: Disable sync process.
  * @ddr_enable: Enable interface DDR (Double Data Rate) mode.
  * @ddr_disable: Disable interface DDR (Double Data Rate) mode.
  * @data_stream_enable: Enable data stream.
@@ -154,6 +156,8 @@ struct iio_backend_ops {
 				  unsigned int writeval, unsigned int *readval);
 	int (*filter_enable)(struct iio_backend *back);
 	int (*filter_disable)(struct iio_backend *back);
+	int (*data_alignment_enable)(struct iio_backend *back);
+	int (*data_alignment_disable)(struct iio_backend *back);
 	int (*ddr_enable)(struct iio_backend *back);
 	int (*ddr_disable)(struct iio_backend *back);
 	int (*data_stream_enable)(struct iio_backend *back);
@@ -196,6 +200,8 @@ int devm_iio_backend_request_buffer(struct device *dev,
 				    struct iio_dev *indio_dev);
 int iio_backend_filter_enable(struct iio_backend *back);
 int iio_backend_filter_disable(struct iio_backend *back);
+int iio_backend_data_alignment_enable(struct iio_backend *back);
+int iio_backend_data_alignment_disable(struct iio_backend *back);
 int iio_backend_ddr_enable(struct iio_backend *back);
 int iio_backend_ddr_disable(struct iio_backend *back);
 int iio_backend_data_stream_enable(struct iio_backend *back);
-- 
2.49.0

[PATCH v2 03/13] iio: backend: add support for self sync

[Antoniu Miclaus]

Add iio backend support for self sync enable/disable.
When disabled data capture synchronization is done
through CNV signal, otherwise through bit-slip.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
no changes in v2.
 drivers/iio/industrialio-backend.c | 30 ++++++++++++++++++++++++++++++
 include/linux/iio/backend.h        |  6 ++++++
 2 files changed, 36 insertions(+)

diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index 60578267643d..cb23433b22c6 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -830,6 +830,36 @@ int iio_backend_data_alignment_disable(struct iio_backend *back)
 }
 EXPORT_SYMBOL_NS_GPL(iio_backend_data_alignment_disable, "IIO_BACKEND");
 
+/**
+ * iio_backend_self_sync_enable - Enable the self sync data capture.
+ * @back: Backend device
+ *
+ * Data capture synchronization is done through bit-slip.
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_self_sync_enable(struct iio_backend *back)
+{
+	return iio_backend_op_call(back, self_sync_enable);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_self_sync_enable, "IIO_BACKEND");
+
+/**
+ * iio_backend_self_sync_disable - Disable the self sync data capture.
+ * @back: Backend device
+ *
+ * Data capture synchronization is done through CNV signal.
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_self_sync_disable(struct iio_backend *back)
+{
+	return iio_backend_op_call(back, self_sync_disable);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_self_sync_disable, "IIO_BACKEND");
+
 /**
  * iio_backend_ddr_enable - Enable interface DDR (Double Data Rate) mode
  * @back: Backend device
diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
index beff66d18151..6d006cb0da5a 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -104,6 +104,8 @@ enum iio_backend_interface_type {
  * @filter_disable: Disable filter.
  * @data_alignment_enable: Enable sync process.
  * @data_alignment_disable: Disable sync process.
+ * @self_sync_enable: Enable the self sync data capture.
+ * @self_sync_disable: Disable the self sync data capture.
  * @ddr_enable: Enable interface DDR (Double Data Rate) mode.
  * @ddr_disable: Disable interface DDR (Double Data Rate) mode.
  * @data_stream_enable: Enable data stream.
@@ -158,6 +160,8 @@ struct iio_backend_ops {
 	int (*filter_disable)(struct iio_backend *back);
 	int (*data_alignment_enable)(struct iio_backend *back);
 	int (*data_alignment_disable)(struct iio_backend *back);
+	int (*self_sync_enable)(struct iio_backend *back);
+	int (*self_sync_disable)(struct iio_backend *back);
 	int (*ddr_enable)(struct iio_backend *back);
 	int (*ddr_disable)(struct iio_backend *back);
 	int (*data_stream_enable)(struct iio_backend *back);
@@ -202,6 +206,8 @@ int iio_backend_filter_enable(struct iio_backend *back);
 int iio_backend_filter_disable(struct iio_backend *back);
 int iio_backend_data_alignment_enable(struct iio_backend *back);
 int iio_backend_data_alignment_disable(struct iio_backend *back);
+int iio_backend_self_sync_enable(struct iio_backend *back);
+int iio_backend_self_sync_disable(struct iio_backend *back);
 int iio_backend_ddr_enable(struct iio_backend *back);
 int iio_backend_ddr_disable(struct iio_backend *back);
 int iio_backend_data_stream_enable(struct iio_backend *back);
-- 
2.49.0

[PATCH v2 04/13] iio: backend: add support for sync status

[Antoniu Miclaus]

Add iio backend support for synchronization status read.
The return value is a boolean stating if the synchronization is enabled
or disabled.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
no changes in v2.
 drivers/iio/industrialio-backend.c | 14 ++++++++++++++
 include/linux/iio/backend.h        |  3 +++
 2 files changed, 17 insertions(+)

diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index cb23433b22c6..0b27f88d6c27 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -860,6 +860,20 @@ int iio_backend_self_sync_disable(struct iio_backend *back)
 }
 EXPORT_SYMBOL_NS_GPL(iio_backend_self_sync_disable, "IIO_BACKEND");
 
+/**
+ * iio_backend_sync_status_get - Read the syncronization status
+ * @back: Backend device
+ * @sync_en: Synchronization status returned (enabled or disabled)
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_sync_status_get(struct iio_backend *back, bool *sync_en)
+{
+	return iio_backend_op_call(back, sync_status_get, sync_en);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_sync_status_get, "IIO_BACKEND");
+
 /**
  * iio_backend_ddr_enable - Enable interface DDR (Double Data Rate) mode
  * @back: Backend device
diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
index 6d006cb0da5a..9bf03181c5c1 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -106,6 +106,7 @@ enum iio_backend_interface_type {
  * @data_alignment_disable: Disable sync process.
  * @self_sync_enable: Enable the self sync data capture.
  * @self_sync_disable: Disable the self sync data capture.
+ * @sync_status_get: Get the syncronization status (enabled/disabled).
  * @ddr_enable: Enable interface DDR (Double Data Rate) mode.
  * @ddr_disable: Disable interface DDR (Double Data Rate) mode.
  * @data_stream_enable: Enable data stream.
@@ -162,6 +163,7 @@ struct iio_backend_ops {
 	int (*data_alignment_disable)(struct iio_backend *back);
 	int (*self_sync_enable)(struct iio_backend *back);
 	int (*self_sync_disable)(struct iio_backend *back);
+	int (*sync_status_get)(struct iio_backend *back, bool *sync_en);
 	int (*ddr_enable)(struct iio_backend *back);
 	int (*ddr_disable)(struct iio_backend *back);
 	int (*data_stream_enable)(struct iio_backend *back);
@@ -208,6 +210,7 @@ int iio_backend_data_alignment_enable(struct iio_backend *back);
 int iio_backend_data_alignment_disable(struct iio_backend *back);
 int iio_backend_self_sync_enable(struct iio_backend *back);
 int iio_backend_self_sync_disable(struct iio_backend *back);
+int iio_backend_sync_status_get(struct iio_backend *back, bool *sync_en);
 int iio_backend_ddr_enable(struct iio_backend *back);
 int iio_backend_ddr_disable(struct iio_backend *back);
 int iio_backend_data_stream_enable(struct iio_backend *back);
-- 
2.49.0

[PATCH v2 05/13] iio: backend: add support for number of lanes

[Antoniu Miclaus]

Add iio backend support for number of lanes to be enabled.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
no changes in v2.
 drivers/iio/industrialio-backend.c | 17 +++++++++++++++++
 include/linux/iio/backend.h        |  3 +++
 2 files changed, 20 insertions(+)

diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index 0b27f88d6c27..85cb8b0d2f09 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -874,6 +874,23 @@ int iio_backend_sync_status_get(struct iio_backend *back, bool *sync_en)
 }
 EXPORT_SYMBOL_NS_GPL(iio_backend_sync_status_get, "IIO_BACKEND");
 
+/**
+ * iio_backend_num_lanes_set - Number of lanes enabled.
+ * @back: Backend device
+ * @num_lanes: Number of lanes.
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_num_lanes_set(struct iio_backend *back, unsigned int num_lanes)
+{
+	if (!num_lanes)
+		return -EINVAL;
+
+	return iio_backend_op_call(back, num_lanes_set, num_lanes);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_num_lanes_set, "IIO_BACKEND");
+
 /**
  * iio_backend_ddr_enable - Enable interface DDR (Double Data Rate) mode
  * @back: Backend device
diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
index 9bf03181c5c1..cdd39c97ba1a 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -107,6 +107,7 @@ enum iio_backend_interface_type {
  * @self_sync_enable: Enable the self sync data capture.
  * @self_sync_disable: Disable the self sync data capture.
  * @sync_status_get: Get the syncronization status (enabled/disabled).
+ * @num_lanes_set: Set the number of lanes enabled.
  * @ddr_enable: Enable interface DDR (Double Data Rate) mode.
  * @ddr_disable: Disable interface DDR (Double Data Rate) mode.
  * @data_stream_enable: Enable data stream.
@@ -164,6 +165,7 @@ struct iio_backend_ops {
 	int (*self_sync_enable)(struct iio_backend *back);
 	int (*self_sync_disable)(struct iio_backend *back);
 	int (*sync_status_get)(struct iio_backend *back, bool *sync_en);
+	int (*num_lanes_set)(struct iio_backend *back, unsigned int num_lanes);
 	int (*ddr_enable)(struct iio_backend *back);
 	int (*ddr_disable)(struct iio_backend *back);
 	int (*data_stream_enable)(struct iio_backend *back);
@@ -211,6 +213,7 @@ int iio_backend_data_alignment_disable(struct iio_backend *back);
 int iio_backend_self_sync_enable(struct iio_backend *back);
 int iio_backend_self_sync_disable(struct iio_backend *back);
 int iio_backend_sync_status_get(struct iio_backend *back, bool *sync_en);
+int iio_backend_num_lanes_set(struct iio_backend *back, unsigned int num_lanes);
 int iio_backend_ddr_enable(struct iio_backend *back);
 int iio_backend_ddr_disable(struct iio_backend *back);
 int iio_backend_data_stream_enable(struct iio_backend *back);
-- 
2.49.0

[PATCH v2 06/13] dt-bindings: iio: adc: add ad408x axi variant

[Antoniu Miclaus]

Add a new compatible and related bindings for the fpga-based
AD408x AXI IP core, a variant of the generic AXI ADC IP.

The AXI AD408x IP is a very similar HDL (fpga) variant of the
generic AXI ADC IP, intended to control ad408x familiy.

Although there are some particularities added for extended
control of the ad408x devices such as the filter configuration,
data capture synchronization procedure and number of lanes enabled.

Wildcard naming is used to match the naming of the published
firmware.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v2:
 - improve commit description.
 Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml b/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
index cf74f84d6103..e91e421a3d6b 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
@@ -27,6 +27,7 @@ description: |
       the ad7606 family.
 
   https://wiki.analog.com/resources/fpga/docs/axi_adc_ip
+  https://analogdevicesinc.github.io/hdl/library/axi_ad408x/index.html
   https://analogdevicesinc.github.io/hdl/library/axi_ad485x/index.html
   http://analogdevicesinc.github.io/hdl/library/axi_ad7606x/index.html
 
@@ -34,6 +35,7 @@ properties:
   compatible:
     enum:
       - adi,axi-adc-10.0.a
+      - adi,axi-ad408x
       - adi,axi-ad7606x
       - adi,axi-ad485x
 
-- 
2.49.0

[PATCH v2 07/13] iio: adc: adi-axi-adc: add filter enable/disable

[Antoniu Miclaus]

Add support for enabling/disabling the filter.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
no changes in v2.
 drivers/iio/adc/adi-axi-adc.c | 42 +++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index 61ab7dce43be..d4466e02fc28 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -52,6 +52,7 @@
 #define   AXI_AD485X_PACKET_FORMAT_20BIT	0x0
 #define   AXI_AD485X_PACKET_FORMAT_24BIT	0x1
 #define   AXI_AD485X_PACKET_FORMAT_32BIT	0x2
+#define   AXI_AD408X_CNTRL_3_FILTER_EN_MSK	BIT(0)
 
 #define ADI_AXI_ADC_REG_DRP_STATUS		0x0074
 #define   ADI_AXI_ADC_DRP_LOCKED		BIT(17)
@@ -402,6 +403,22 @@ static int axi_adc_ad485x_oversampling_ratio_set(struct iio_backend *back,
 	}
 }
 
+static int axi_adc_ad408x_filter_enable(struct iio_backend *back)
+{
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+
+	return regmap_set_bits(st->regmap, ADI_AXI_ADC_REG_CNTRL_3,
+			       AXI_AD408X_CNTRL_3_FILTER_EN_MSK);
+}
+
+static int axi_adc_ad408x_filter_disable(struct iio_backend *back)
+{
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+
+	return regmap_clear_bits(st->regmap, ADI_AXI_ADC_REG_CNTRL_3,
+				 AXI_AD408X_CNTRL_3_FILTER_EN_MSK);
+}
+
 static struct iio_buffer *axi_adc_request_buffer(struct iio_backend *back,
 						 struct iio_dev *indio_dev)
 {
@@ -582,6 +599,25 @@ static const struct iio_backend_info axi_ad485x = {
 	.ops = &adi_ad485x_ops,
 };
 
+static const struct iio_backend_ops adi_ad408x_ops = {
+	.enable = axi_adc_enable,
+	.disable = axi_adc_disable,
+	.chan_enable = axi_adc_chan_enable,
+	.chan_disable = axi_adc_chan_disable,
+	.request_buffer = axi_adc_request_buffer,
+	.free_buffer = axi_adc_free_buffer,
+	.data_sample_trigger = axi_adc_data_sample_trigger,
+	.filter_enable = axi_adc_ad408x_filter_enable,
+	.filter_disable = axi_adc_ad408x_filter_disable,
+	.debugfs_reg_access = iio_backend_debugfs_ptr(axi_adc_reg_access),
+	.debugfs_print_chan_status = iio_backend_debugfs_ptr(axi_adc_debugfs_print_chan_status),
+};
+
+static const struct iio_backend_info axi_ad408x = {
+	.name = "axi-ad408x",
+	.ops = &adi_ad408x_ops,
+};
+
 static int adi_axi_adc_probe(struct platform_device *pdev)
 {
 	struct adi_axi_adc_state *st;
@@ -697,9 +733,15 @@ static const struct axi_adc_info adc_ad7606 = {
 	.has_child_nodes = true,
 };
 
+static const struct axi_adc_info adi_axi_ad408x = {
+	.version = ADI_AXI_PCORE_VER(10, 0, 'a'),
+	.backend_info = &axi_ad408x,
+};
+
 /* Match table for of_platform binding */
 static const struct of_device_id adi_axi_adc_of_match[] = {
 	{ .compatible = "adi,axi-adc-10.0.a", .data = &adc_generic },
+	{ .compatible = "adi,axi-ad408x", .data = &adi_axi_ad408x },
 	{ .compatible = "adi,axi-ad485x", .data = &adi_axi_ad485x },
 	{ .compatible = "adi,axi-ad7606x", .data = &adc_ad7606 },
 	{ /* end of list */ }
-- 
2.49.0

[PATCH v2 08/13] iio: adc: adi-axi-adc: add bitslip enable/disable

[Antoniu Miclaus]

Add support for enabling/disabling the sync process.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
no changes in v2.
 drivers/iio/adc/adi-axi-adc.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index d4466e02fc28..4625acc313c4 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -44,6 +44,7 @@
 #define   ADI_AXI_ADC_REG_CONFIG_CMOS_OR_LVDS_N	BIT(7)
 
 #define ADI_AXI_ADC_REG_CTRL			0x0044
+#define    AXI_AD408X_CTRL_BITSLIP_EN_MSK	BIT(3)
 #define    ADI_AXI_ADC_CTRL_DDR_EDGESEL_MASK	BIT(1)
 
 #define ADI_AXI_ADC_REG_CNTRL_3			0x004c
@@ -419,6 +420,22 @@ static int axi_adc_ad408x_filter_disable(struct iio_backend *back)
 				 AXI_AD408X_CNTRL_3_FILTER_EN_MSK);
 }
 
+static int axi_adc_ad408x_bitslip_enable(struct iio_backend *back)
+{
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+
+	return regmap_set_bits(st->regmap, ADI_AXI_ADC_REG_CTRL,
+			       AXI_AD408X_CTRL_BITSLIP_EN_MSK);
+}
+
+static int axi_adc_ad408x_bitslip_disable(struct iio_backend *back)
+{
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+
+	return regmap_clear_bits(st->regmap, ADI_AXI_ADC_REG_CTRL,
+				 AXI_AD408X_CTRL_BITSLIP_EN_MSK);
+}
+
 static struct iio_buffer *axi_adc_request_buffer(struct iio_backend *back,
 						 struct iio_dev *indio_dev)
 {
@@ -609,6 +626,8 @@ static const struct iio_backend_ops adi_ad408x_ops = {
 	.data_sample_trigger = axi_adc_data_sample_trigger,
 	.filter_enable = axi_adc_ad408x_filter_enable,
 	.filter_disable = axi_adc_ad408x_filter_disable,
+	.data_alignment_enable = axi_adc_ad408x_bitslip_enable,
+	.data_alignment_disable = axi_adc_ad408x_bitslip_disable,
 	.debugfs_reg_access = iio_backend_debugfs_ptr(axi_adc_reg_access),
 	.debugfs_print_chan_status = iio_backend_debugfs_ptr(axi_adc_debugfs_print_chan_status),
 };
-- 
2.49.0

[PATCH v2 09/13] iio: adc: adi-axi-adc: add self sync support

[Antoniu Miclaus]

Add support for data capture synchronization through CNV signal or
bit-slip.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
no changes in v2.
 drivers/iio/adc/adi-axi-adc.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index 4625acc313c4..017685854895 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -53,6 +53,7 @@
 #define   AXI_AD485X_PACKET_FORMAT_20BIT	0x0
 #define   AXI_AD485X_PACKET_FORMAT_24BIT	0x1
 #define   AXI_AD485X_PACKET_FORMAT_32BIT	0x2
+#define   AXI_AD408X_CNTRL_3_SELF_SYNC_EN_MSK	BIT(1)
 #define   AXI_AD408X_CNTRL_3_FILTER_EN_MSK	BIT(0)
 
 #define ADI_AXI_ADC_REG_DRP_STATUS		0x0074
@@ -436,6 +437,22 @@ static int axi_adc_ad408x_bitslip_disable(struct iio_backend *back)
 				 AXI_AD408X_CTRL_BITSLIP_EN_MSK);
 }
 
+static int axi_adc_ad408x_self_sync_enable(struct iio_backend *back)
+{
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+
+	return regmap_set_bits(st->regmap, ADI_AXI_ADC_REG_CNTRL_3,
+			       AXI_AD408X_CNTRL_3_SELF_SYNC_EN_MSK);
+}
+
+static int axi_adc_ad408x_self_sync_disable(struct iio_backend *back)
+{
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+
+	return regmap_clear_bits(st->regmap, ADI_AXI_ADC_REG_CNTRL_3,
+				 AXI_AD408X_CNTRL_3_SELF_SYNC_EN_MSK);
+}
+
 static struct iio_buffer *axi_adc_request_buffer(struct iio_backend *back,
 						 struct iio_dev *indio_dev)
 {
@@ -628,6 +645,8 @@ static const struct iio_backend_ops adi_ad408x_ops = {
 	.filter_disable = axi_adc_ad408x_filter_disable,
 	.data_alignment_enable = axi_adc_ad408x_bitslip_enable,
 	.data_alignment_disable = axi_adc_ad408x_bitslip_disable,
+	.self_sync_enable = axi_adc_ad408x_self_sync_enable,
+	.self_sync_disable = axi_adc_ad408x_self_sync_disable,
 	.debugfs_reg_access = iio_backend_debugfs_ptr(axi_adc_reg_access),
 	.debugfs_print_chan_status = iio_backend_debugfs_ptr(axi_adc_debugfs_print_chan_status),
 };
-- 
2.49.0

[PATCH v2 10/13] iio: adc: adi-axi-adc: add sync status

[Antoniu Miclaus]

Add support for checking the ADC sync status.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
no changes in v2.
 drivers/iio/adc/adi-axi-adc.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index 017685854895..0d12c0121bbc 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -56,6 +56,9 @@
 #define   AXI_AD408X_CNTRL_3_SELF_SYNC_EN_MSK	BIT(1)
 #define   AXI_AD408X_CNTRL_3_FILTER_EN_MSK	BIT(0)
 
+#define ADI_AXI_ADC_REG_SYNC_STATUS		0x0068
+#define   ADI_AXI_ADC_SYNC			BIT(0)
+
 #define ADI_AXI_ADC_REG_DRP_STATUS		0x0074
 #define   ADI_AXI_ADC_DRP_LOCKED		BIT(17)
 
@@ -453,6 +456,21 @@ static int axi_adc_ad408x_self_sync_disable(struct iio_backend *back)
 				 AXI_AD408X_CNTRL_3_SELF_SYNC_EN_MSK);
 }
 
+static int axi_adc_sync_status_get(struct iio_backend *back, bool *sync_en)
+{
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+	int ret;
+	u32 val;
+
+	ret = regmap_read(st->regmap, ADI_AXI_ADC_REG_SYNC_STATUS, &val);
+	if (ret)
+		return ret;
+
+	*sync_en = (bool)FIELD_GET(ADI_AXI_ADC_SYNC, val);
+
+	return 0;
+}
+
 static struct iio_buffer *axi_adc_request_buffer(struct iio_backend *back,
 						 struct iio_dev *indio_dev)
 {
@@ -600,6 +618,7 @@ static const struct iio_backend_ops adi_axi_adc_ops = {
 	.test_pattern_set = axi_adc_test_pattern_set,
 	.chan_status = axi_adc_chan_status,
 	.interface_type_get = axi_adc_interface_type_get,
+	.sync_status_get = axi_adc_sync_status_get,
 	.debugfs_reg_access = iio_backend_debugfs_ptr(axi_adc_reg_access),
 	.debugfs_print_chan_status = iio_backend_debugfs_ptr(axi_adc_debugfs_print_chan_status),
 };
@@ -647,6 +666,7 @@ static const struct iio_backend_ops adi_ad408x_ops = {
 	.data_alignment_disable = axi_adc_ad408x_bitslip_disable,
 	.self_sync_enable = axi_adc_ad408x_self_sync_enable,
 	.self_sync_disable = axi_adc_ad408x_self_sync_disable,
+	.sync_status_get = axi_adc_sync_status_get,
 	.debugfs_reg_access = iio_backend_debugfs_ptr(axi_adc_reg_access),
 	.debugfs_print_chan_status = iio_backend_debugfs_ptr(axi_adc_debugfs_print_chan_status),
 };
-- 
2.49.0

[PATCH v2 11/13] iio: adc: adi-axi-adc: add num lanes support

[Antoniu Miclaus]

Add support for setting the number of lanes enabled.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
no changes in v2.
 drivers/iio/adc/adi-axi-adc.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index 0d12c0121bbc..8576c0c1d024 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -44,6 +44,7 @@
 #define   ADI_AXI_ADC_REG_CONFIG_CMOS_OR_LVDS_N	BIT(7)
 
 #define ADI_AXI_ADC_REG_CTRL			0x0044
+#define    AXI_AD408X_CTRL_NUM_LANES_MSK	GENMASK(12, 8)
 #define    AXI_AD408X_CTRL_BITSLIP_EN_MSK	BIT(3)
 #define    ADI_AXI_ADC_CTRL_DDR_EDGESEL_MASK	BIT(1)
 
@@ -471,6 +472,19 @@ static int axi_adc_sync_status_get(struct iio_backend *back, bool *sync_en)
 	return 0;
 }
 
+static int axi_adc_ad408x_num_lanes_set(struct iio_backend *back,
+				 unsigned int num_lanes)
+{
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+
+	if (!num_lanes)
+		return -EINVAL;
+
+	return regmap_update_bits(st->regmap, ADI_AXI_ADC_REG_CTRL,
+				  AXI_AD408X_CTRL_NUM_LANES_MSK,
+				  FIELD_PREP(AXI_AD408X_CTRL_NUM_LANES_MSK, num_lanes));
+}
+
 static struct iio_buffer *axi_adc_request_buffer(struct iio_backend *back,
 						 struct iio_dev *indio_dev)
 {
@@ -667,6 +681,7 @@ static const struct iio_backend_ops adi_ad408x_ops = {
 	.self_sync_enable = axi_adc_ad408x_self_sync_enable,
 	.self_sync_disable = axi_adc_ad408x_self_sync_disable,
 	.sync_status_get = axi_adc_sync_status_get,
+	.num_lanes_set = axi_adc_ad408x_num_lanes_set,
 	.debugfs_reg_access = iio_backend_debugfs_ptr(axi_adc_reg_access),
 	.debugfs_print_chan_status = iio_backend_debugfs_ptr(axi_adc_debugfs_print_chan_status),
 };
-- 
2.49.0

[PATCH v2 12/13] dt-bindings: iio: adc: add ad4080

[Antoniu Miclaus]

Add devicetree bindings for ad4080 family.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v2:
 - add descripton for spi bus
 - use actual pin for clock name
 - fix typo in num lanes description
 - add iio-backends
 - don't make all supplies mandatory
 .../bindings/iio/adc/adi,ad4080.yaml          | 96 +++++++++++++++++++
 1 file changed, 96 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4080.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4080.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4080.yaml
new file mode 100644
index 000000000000..ed849ba1b77b
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4080.yaml
@@ -0,0 +1,96 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2025 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad4080.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD4080 20-Bit, 40 MSPS, Differential SAR ADC
+
+maintainers:
+  - Antoniu Miclaus <antoniu.miclaus@analog.com>
+
+description: |
+  The AD4080 is a high speed, low noise, low distortion, 20-bit, Easy Drive,
+  successive approximation register (SAR) analog-to-digital converter (ADC).
+  Maintaining high performance (signal-to-noise and distortion (SINAD) ratio
+  > 90 dBFS) at signal frequencies in excess of 1 MHz enables the AD4080 to
+  service a wide variety of precision, wide bandwidth data acquisition
+  applications.
+
+  https://www.analog.com/media/en/technical-documentation/data-sheets/ad4080.pdf
+
+$ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    enum:
+      - adi,ad4080
+
+  reg:
+    maxItems: 1
+
+  spi-max-frequency:
+    description: Configuration of the SPI bus.
+    maximum: 50000000
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: cnv
+
+  vdd33-supply: true
+
+  vdd11-supply: true
+
+  vddldo-supply: true
+
+  iovdd-supply: true
+
+  vrefin-supply: true
+
+  io-backends:
+    maxItems: 1
+
+  adi,lvds-cnv-enable:
+    description: Enable the LVDS signal type on the CNV pin. Default is CMOS.
+    type: boolean
+
+  adi,num-lanes:
+    description:
+      Number of lanes on which the data is sent on the output (DA, DB pins).
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [1, 2]
+    default: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - vdd33-supply
+  - vrefin-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0 {
+          compatible = "adi,ad4080";
+          reg = <0>;
+          spi-max-frequency = <10000000>;
+          vdd33-supply = <&vdd33>;
+          vddldo-supply = <&vddldo>;
+          vrefin-supply = <&vrefin>;
+          clocks = <&cnv>;
+          clock-names = "cnv";
+          io-backends = <&iio_backend>;
+        };
+    };
+...
-- 
2.49.0

[PATCH v2 13/13] iio: adc: ad4080: add driver support

[Antoniu Miclaus]

Add support for AD4080 high-speed, low noise, low distortion,
20-bit, Easy Drive, successive approximation register (SAR)
analog-to-digital converter (ADC).

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v2:
 - set num_lanes during probe.
 - rename bitslitp to iio_backend_data_alignment_enable
 - do ad4080_lvds_sync_write only once during probe.
 - use ETIMEDOUT
 - rename to `cnv` instead of `adc-clk`
 - update Kconfig help section
 - drop extra blank line.
 - replace `/**` with `/*` for comments
 - drop redundant return 0.
 - use `dev_dbg` during while(--timeout) procedure
 - use while (--timeout && !sync_en)
 - return -ETIME where applicable and check missing return codes.
 - regmap_update_bits used where applicable
 - use defines instead of GENMASK inline.
 - return FIELD_GET()
 - st->filter_enabled = mode and dropping the if else statement.
 - remove redundant brackets.
 - use OVERSAMPLING_RATIO attribute and drop custom ABI for it
 - use already existing filter_type attribute
 - fix indentation
 - remove comma on 'terminators'
 - use dev_err_probe instead of dev_err
 - check missing return values.
 - rework num_lanes property parse.
 - keep ad4080_chip_info since the driver will be extended for more parts
   in the future (also stated in cover letter).
 drivers/iio/adc/Kconfig  |  14 +
 drivers/iio/adc/Makefile |   1 +
 drivers/iio/adc/ad4080.c | 653 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 668 insertions(+)
 create mode 100644 drivers/iio/adc/ad4080.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 27413516216c..17df328f5322 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -47,6 +47,20 @@ config AD4030
 	  To compile this driver as a module, choose M here: the module will be
 	  called ad4030.
 
+config AD4080
+	tristate "Analog Devices AD4080 high speed ADC"
+	depends on SPI
+	select REGMAP_SPI
+	select IIO_BACKEND
+	help
+	  Say yes here to build support for Analog Devices AD4080
+	  high speed, low noise, low distortion, 20-bit, Easy Drive,
+	  successive approximation register (SAR) analog-to-digital
+	  converter (ADC). Supports iio_backended devices for AD4080.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called ad4080.
+
 config AD4130
 	tristate "Analog Device AD4130 ADC Driver"
 	depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 9f26d5eca822..e6efed5b4e7a 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
 obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
 obj-$(CONFIG_AD4000) += ad4000.o
 obj-$(CONFIG_AD4030) += ad4030.o
+obj-$(CONFIG_AD4080) += ad4080.o
 obj-$(CONFIG_AD4130) += ad4130.o
 obj-$(CONFIG_AD4695) += ad4695.o
 obj-$(CONFIG_AD4851) += ad4851.o
diff --git a/drivers/iio/adc/ad4080.c b/drivers/iio/adc/ad4080.c
new file mode 100644
index 000000000000..3a0b1ad13765
--- /dev/null
+++ b/drivers/iio/adc/ad4080.c
@@ -0,0 +1,653 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Analog Devices AD4080 SPI ADC driver
+ *
+ * Copyright 2025 Analog Devices Inc.
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+#include <linux/units.h>
+
+#include <linux/iio/backend.h>
+#include <linux/iio/iio.h>
+
+#include <linux/clk.h>
+
+/* Register Definition */
+#define AD4080_REG_INTERFACE_CONFIG_A		0x00
+#define AD4080_REG_INTERFACE_CONFIG_B		0x01
+#define AD4080_REG_DEVICE_CONFIG		0x02
+#define AD4080_REG_CHIP_TYPE			0x03
+#define AD4080_REG_PRODUCT_ID_L			0x04
+#define AD4080_REG_PRODUCT_ID_H			0x05
+#define AD4080_REG_CHIP_GRADE			0x06
+#define AD4080_REG_SCRATCH_PAD			0x0A
+#define AD4080_REG_SPI_REVISION			0x0B
+#define AD4080_REG_VENDOR_L			0x0C
+#define AD4080_REG_VENDOR_H			0x0D
+#define AD4080_REG_STREAM_MODE			0x0E
+#define AD4080_REG_TRANSFER_CONFIG		0x0F
+#define AD4080_REG_INTERFACE_CONFIG_C		0x10
+#define AD4080_REG_INTERFACE_STATUS_A		0x11
+#define AD4080_REG_DEVICE_STATUS		0x14
+#define AD4080_REG_ADC_DATA_INTF_CONFIG_A	0x15
+#define AD4080_REG_ADC_DATA_INTF_CONFIG_B	0x16
+#define AD4080_REG_ADC_DATA_INTF_CONFIG_C	0x17
+#define AD4080_REG_PWR_CTRL			0x18
+#define AD4080_REG_GPIO_CONFIG_A		0x19
+#define AD4080_REG_GPIO_CONFIG_B		0x1A
+#define AD4080_REG_GPIO_CONFIG_C		0x1B
+#define AD4080_REG_GENERAL_CONFIG		0x1C
+#define AD4080_REG_FIFO_WATERMARK_LSB		0x1D
+#define AD4080_REG_FIFO_WATERMARK_MSB		0x1E
+#define AD4080_REG_EVENT_HYSTERESIS_LSB		0x1F
+#define AD4080_REG_EVENT_HYSTERESIS_MSB		0x20
+#define AD4080_REG_EVENT_DETECTION_HI_LSB	0x21
+#define AD4080_REG_EVENT_DETECTION_HI_MSB	0x22
+#define AD4080_REG_EVENT_DETECTION_LO_LSB	0x23
+#define AD4080_REG_EVENT_DETECTION_LO_MSB	0x24
+#define AD4080_REG_OFFSET_LSB			0x25
+#define AD4080_REG_OFFSET_MSB			0x26
+#define AD4080_REG_GAIN_LSB			0x27
+#define AD4080_REG_GAIN_MSB			0x28
+#define AD4080_REG_FILTER_CONFIG		0x29
+
+/* AD4080_REG_INTERFACE_CONFIG_A Bit Definition */
+#define AD4080_SW_RESET_MSK			(BIT(7) | BIT(0))
+#define AD4080_ADDR_ASC_MSK			BIT(5)
+#define AD4080_SDO_ENABLE_MSK			BIT(4)
+
+/* AD4080_REG_INTERFACE_CONFIG_B Bit Definition */
+#define AD4080_SINGLE_INST_MSK			BIT(7)
+#define AD4080_SHORT_INST_MSK			BIT(3)
+
+/* AD4080_REG_DEVICE_CONFIG Bit Definition */
+#define AD4080_OPERATING_MODES_MSK		GENMASK(1, 0)
+
+/* AD4080_REG_TRANSFER_CONFIG Bit Definition */
+#define AD4080_KEEP_STREAM_LENGTH_VAL_MSK	BIT(2)
+
+/* AD4080_REG_INTERFACE_CONFIG_C Bit Definition */
+#define AD4080_STRICT_REG_ACCESS_MSK		BIT(5)
+
+/* AD4080_REG_ADC_DATA_INTF_CONFIG_A Bit Definition */
+#define AD4080_RESERVED_CONFIG_A_MSK		BIT(6)
+#define AD4080_INTF_CHK_EN_MSK			BIT(4)
+#define AD4080_SPI_LVDS_LANES_MSK		BIT(2)
+#define AD4080_DATA_INTF_MODE_MSK		BIT(0)
+
+/* AD4080_REG_ADC_DATA_INTF_CONFIG_B Bit Definition */
+#define AD4080_LVDS_CNV_CLK_CNT_MSK		GENMASK(7, 4)
+#define AD4080_LVDS_SELF_CLK_MODE_MSK		BIT(3)
+#define AD4080_LVDS_CNV_EN_MSK			BIT(0)
+
+/* AD4080_REG_ADC_DATA_INTF_CONFIG_C Bit Definition */
+#define AD4080_LVDS_VOD_MSK			GENMASK(6, 4)
+
+/* AD4080_REG_PWR_CTRL Bit Definition */
+#define AD4080_ANA_DIG_LDO_PD_MSK		BIT(1)
+#define AD4080_INTF_LDO_PD_MSK			BIT(0)
+
+/* AD4080_REG_GPIO_CONFIG_A Bit Definition */
+#define AD4080_GPO_1_EN				BIT(1)
+#define AD4080_GPO_0_EN				BIT(0)
+
+/* AD4080_REG_GPIO_CONFIG_B Bit Definition */
+#define AD4080_GPIO_1_SEL			GENMASK(7, 4)
+#define AD4080_GPIO_0_SEL			GENMASK(3, 0)
+
+/* AD4080_REG_FIFO_CONFIG Bit Definition */
+#define AD4080_FIFO_MODE_MSK			GENMASK(1, 0)
+
+/* AD4080_REG_FILTER_CONFIG Bit Definition */
+#define AD4080_SINC_DEC_RATE_MSK		GENMASK(6, 3)
+#define AD4080_FILTER_SEL_MSK			GENMASK(1, 0)
+
+/* Miscellaneous Definitions */
+#define AD4080_SW_RESET				(BIT(7) | BIT(0))
+#define AD4080_SPI_READ				BIT(7)
+#define BYTE_ADDR_H				GENMASK(15, 8)
+#define BYTE_ADDR_L				GENMASK(7, 0)
+#define AD4080_CHIP_ID				GENMASK(2, 0)
+
+#define AD4080_MAX_SAMP_FREQ			40000000
+#define AD4080_MIN_SAMP_FREQ			1250000
+
+#define AXI_AD4080_ENABLE_FILTER_BIT		BIT(0)
+#define AXI_AD4080_SELF_SYNC_BIT		BIT(1)
+
+enum ad4080_filter_type {
+	FILTER_DISABLE,
+	SINC_1,
+	SINC_5,
+	SINC_5_COMP
+};
+
+static const unsigned int ad4080_scale_table[][2] = {
+	{6000, 0},
+};
+
+static const char *const ad4080_filter_type_iio_enum[] = {
+	[FILTER_DISABLE]   = "disabled",
+	[SINC_1]           = "sinc1",
+	[SINC_5]           = "sinc5",
+	[SINC_5_COMP]      = "sinc5_plus_compensation",
+};
+
+static const int ad4080_dec_rate_iio_enum[] = {
+	2, 4, 8, 16, 32, 64, 128, 256, 512, 1024
+};
+
+static const char * const ad4080_power_supplies[] = {
+	"vdd33", "vdd11", "vddldo", "iovdd", "vrefin",
+};
+
+struct ad4080_chip_info {
+	const char *name;
+	unsigned int product_id;
+	int num_scales;
+	const unsigned int (*scale_table)[2];
+	const struct iio_chan_spec *channels;
+	unsigned int num_channels;
+};
+
+struct ad4080_state {
+	struct spi_device		*spi;
+	struct regmap			*regmap;
+	struct clk			*clk;
+	struct iio_backend		*back;
+	const struct ad4080_chip_info	*info;
+	/*
+	 * Synchronize access to members the of driver state, and ensure
+	 * atomicity of consecutive regmap operations.
+	 */
+	struct mutex			lock;
+	unsigned int			num_lanes;
+	unsigned int			dec_rate;
+	enum ad4080_filter_type		filter_type;
+	bool				filter_en;
+	bool				lvds_cnv_en;
+};
+
+static const struct regmap_config ad4080_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	.read_flag_mask = BIT(7),
+	.max_register = 0x29,
+};
+
+static int ad4080_reg_access(struct iio_dev *indio_dev, unsigned int reg,
+			     unsigned int writeval, unsigned int *readval)
+{
+	struct ad4080_state *st = iio_priv(indio_dev);
+
+	if (readval)
+		return regmap_read(st->regmap, reg, readval);
+
+	return regmap_write(st->regmap, reg, writeval);
+}
+
+static int ad4080_get_scale(struct ad4080_state *st, int *val, int *val2)
+{
+	unsigned int tmp;
+
+	tmp = (st->info->scale_table[0][0] * 1000000ULL) >>
+		    st->info->channels[0].scan_type.realbits;
+	*val = tmp / 1000000;
+	*val2 = tmp % 1000000;
+
+	return IIO_VAL_INT_PLUS_NANO;
+}
+
+static unsigned int ad4080_get_dec_rate(struct iio_dev *dev,
+					const struct iio_chan_spec *chan)
+{
+	struct ad4080_state *st = iio_priv(dev);
+	int ret;
+	unsigned int data;
+
+	ret = regmap_read(st->regmap, AD4080_REG_FILTER_CONFIG, &data);
+	if (ret)
+		return ret;
+
+	return (1 << (FIELD_GET(AD4080_SINC_DEC_RATE_MSK, data) + 1));
+}
+
+static int ad4080_set_dec_rate(struct iio_dev *dev,
+			       const struct iio_chan_spec *chan,
+			       unsigned int mode)
+{
+	struct ad4080_state *st = iio_priv(dev);
+	int ret;
+	unsigned int data;
+	unsigned int reg_val;
+
+	if (st->filter_type >= SINC_5 && mode >= 512)
+		return -EINVAL;
+
+	guard(mutex)(&st->lock);
+	ret = regmap_read(st->regmap, AD4080_REG_FILTER_CONFIG, &reg_val);
+	if (ret)
+		return ret;
+
+	data = ((ilog2(mode) - 1) << 3) | (reg_val & AD4080_FILTER_SEL_MSK);
+	ret = regmap_write(st->regmap, AD4080_REG_FILTER_CONFIG, data);
+	if (ret)
+		return ret;
+
+	st->dec_rate = mode;
+
+	return ret;
+}
+
+static int ad4080_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long m)
+{
+	struct ad4080_state *st = iio_priv(indio_dev);
+	int dec_rate;
+
+	switch (m) {
+	case IIO_CHAN_INFO_SCALE:
+		return ad4080_get_scale(st, val, val2);
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		if (st->filter_type == SINC_5_COMP)
+			dec_rate = st->dec_rate * 2;
+		else
+			dec_rate = st->dec_rate;
+		if (st->filter_en)
+			*val = DIV_ROUND_CLOSEST(clk_get_rate(st->clk), dec_rate);
+		else
+			*val = clk_get_rate(st->clk);
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		*val = ad4080_get_dec_rate(indio_dev, chan);
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad4080_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		return -EINVAL;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return -EINVAL;
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		return ad4080_set_dec_rate(indio_dev, chan, val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad4080_lvds_sync_write(struct ad4080_state *st)
+{
+	unsigned int timeout = 100;
+	bool sync_en;
+	int ret;
+
+	guard(mutex)(&st->lock);
+	if (st->num_lanes == 1)
+		ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
+				   AD4080_RESERVED_CONFIG_A_MSK |
+				   AD4080_INTF_CHK_EN_MSK);
+	else
+		ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
+				   AD4080_RESERVED_CONFIG_A_MSK |
+				   AD4080_INTF_CHK_EN_MSK |
+				   AD4080_SPI_LVDS_LANES_MSK);
+	if (ret)
+		return ret;
+
+	ret = iio_backend_data_alignment_enable(st->back);
+	if (ret)
+		return ret;
+
+	do {
+		ret = iio_backend_sync_status_get(st->back, &sync_en);
+		if (ret)
+			return ret;
+
+		if (!sync_en)
+			dev_dbg(&st->spi->dev, "Not Locked: Running Bit Slip\n");
+	} while (--timeout && !sync_en);
+
+	if (timeout) {
+		dev_info(&st->spi->dev, "Success: Pattern correct and Locked!\n");
+		if (st->num_lanes == 1)
+			return regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
+					    AD4080_RESERVED_CONFIG_A_MSK);
+		else
+			return regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
+					    AD4080_RESERVED_CONFIG_A_MSK |
+					    AD4080_SPI_LVDS_LANES_MSK);
+	} else {
+		dev_info(&st->spi->dev, "LVDS Sync Timeout.\n");
+		if (st->num_lanes == 1) {
+			ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
+					   AD4080_RESERVED_CONFIG_A_MSK);
+			if (ret)
+				return ret;
+		} else {
+			ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
+					   AD4080_RESERVED_CONFIG_A_MSK |
+					   AD4080_SPI_LVDS_LANES_MSK);
+			if (ret)
+				return ret;
+		}
+
+		return -ETIMEDOUT;
+	}
+}
+
+static ssize_t ad4080_get_filter_type(struct iio_dev *dev,
+				      const struct iio_chan_spec *chan)
+{
+	struct ad4080_state *st = iio_priv(dev);
+	unsigned int data;
+	int ret;
+
+	ret = regmap_read(st->regmap, AD4080_REG_FILTER_CONFIG, &data);
+	if (ret)
+		return ret;
+
+	return FIELD_GET(AD4080_FILTER_SEL_MSK, data);
+}
+
+static int ad4080_set_filter_type(struct iio_dev *dev,
+				  const struct iio_chan_spec *chan,
+				  unsigned int mode)
+{
+	struct ad4080_state *st = iio_priv(dev);
+	int ret;
+	unsigned int data;
+	unsigned int reg_val;
+
+	if (mode >= SINC_5 && st->dec_rate >= 512)
+		return -EINVAL;
+
+	guard(mutex)(&st->lock);
+	if (mode)
+		ret = iio_backend_filter_enable(st->back);
+	else
+		ret = iio_backend_filter_disable(st->back);
+	if (ret)
+		return ret;
+
+	st->filter_en = mode;
+
+	ret = regmap_read(st->regmap, AD4080_REG_FILTER_CONFIG, &reg_val);
+	if (ret)
+		return ret;
+
+	data = (reg_val & AD4080_SINC_DEC_RATE_MSK) |
+	       (mode & AD4080_FILTER_SEL_MSK);
+
+	ret = regmap_write(st->regmap, AD4080_REG_FILTER_CONFIG, data);
+	if (ret)
+		return ret;
+
+	st->filter_type = mode;
+
+	return ret;
+}
+
+static int ad4080_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     const int **vals, int *type, int *length,
+			     long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		*vals = ad4080_dec_rate_iio_enum;
+		*length = ARRAY_SIZE(ad4080_dec_rate_iio_enum);
+		*type = IIO_VAL_INT;
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info ad4080_iio_info = {
+	.debugfs_reg_access = ad4080_reg_access,
+	.read_raw = ad4080_read_raw,
+	.write_raw = ad4080_write_raw,
+	.read_avail = ad4080_read_avail,
+};
+
+static const struct iio_enum ad4080_filter_type_enum = {
+	.items = ad4080_filter_type_iio_enum,
+	.num_items = ARRAY_SIZE(ad4080_filter_type_iio_enum),
+	.set = ad4080_set_filter_type,
+	.get = ad4080_get_filter_type,
+};
+
+static struct iio_chan_spec_ext_info ad4080_ext_info[] = {
+	IIO_ENUM("filter_type",
+		 IIO_SHARED_BY_ALL,
+		 &ad4080_filter_type_enum),
+	IIO_ENUM_AVAILABLE("filter_type",
+			   IIO_SHARED_BY_ALL,
+			   &ad4080_filter_type_enum),
+	{}
+};
+
+#define AD4080_CHAN(_chan, _si, _bits, _sign, _shift)		\
+	{ .type = IIO_VOLTAGE,						\
+	  .indexed = 1,							\
+	  .channel = _chan,						\
+	  .info_mask_separate = BIT(IIO_CHAN_INFO_SCALE),		\
+	  .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) |	\
+		BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),			\
+	  .info_mask_shared_by_all_available =				\
+		BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),			\
+	  .ext_info = ad4080_ext_info,					\
+	  .scan_index = _si,						\
+	  .scan_type = {						\
+			.sign = _sign,					\
+			.realbits = _bits,				\
+			.storagebits = 32,				\
+			.shift = _shift,				\
+	  },								\
+	}
+
+static const struct iio_chan_spec ad4080_channels[] = {
+	AD4080_CHAN(0, 0, 20, 's', 0)
+};
+
+static const struct ad4080_chip_info ad4080_chip_info = {
+	.name = "AD4080",
+	.product_id = AD4080_CHIP_ID,
+	.scale_table = ad4080_scale_table,
+	.num_scales = ARRAY_SIZE(ad4080_scale_table),
+	.num_channels = 1,
+	.channels = ad4080_channels,
+};
+
+static int ad4080_setup(struct iio_dev *indio_dev)
+{
+	struct ad4080_state *st = iio_priv(indio_dev);
+	unsigned int id;
+	int ret;
+
+	ret = regmap_write(st->regmap, AD4080_REG_INTERFACE_CONFIG_A,
+			   AD4080_SW_RESET);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, AD4080_REG_INTERFACE_CONFIG_A,
+			   AD4080_SDO_ENABLE_MSK);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(st->regmap, AD4080_REG_CHIP_TYPE, &id);
+	if (ret)
+		return ret;
+
+	if (id != AD4080_CHIP_ID)
+		return dev_err_probe(&st->spi->dev, -EINVAL,
+				     "Unrecognized CHIP_ID 0x%X\n", id);
+
+	ret = regmap_set_bits(st->regmap, AD4080_REG_GPIO_CONFIG_A,
+			      AD4080_GPO_1_EN);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, AD4080_REG_GPIO_CONFIG_B,
+			   FIELD_PREP(AD4080_GPIO_1_SEL, 3));
+	if (ret)
+		return ret;
+
+	ret = iio_backend_num_lanes_set(st->back, st->num_lanes);
+	if (ret)
+		return ret;
+
+	ret = iio_backend_self_sync_enable(st->back);
+	if (ret)
+		return ret;
+
+	if (st->lvds_cnv_en) {
+		if (st->num_lanes) {
+			ret = regmap_update_bits(st->regmap,
+						 AD4080_REG_ADC_DATA_INTF_CONFIG_B,
+						 AD4080_LVDS_CNV_CLK_CNT_MSK,
+						 FIELD_PREP(AD4080_LVDS_CNV_CLK_CNT_MSK, 7));
+			if (ret)
+				return ret;
+		}
+
+		ret = regmap_set_bits(st->regmap,
+				      AD4080_REG_ADC_DATA_INTF_CONFIG_B,
+				      AD4080_LVDS_CNV_EN_MSK);
+		if (ret)
+			return ret;
+
+		return ad4080_lvds_sync_write(st);
+	}
+
+	return 0;
+}
+
+static void ad4080_properties_parse(struct ad4080_state *st)
+{
+	unsigned int val;
+	int ret;
+
+	st->lvds_cnv_en = device_property_read_bool(&st->spi->dev,
+						    "adi,lvds-cnv-enable");
+
+	st->num_lanes = 1;
+	ret = device_property_read_u32(&st->spi->dev, "adi,num_lanes", &val);
+	if (!ret)
+		st->num_lanes = val;
+}
+
+static int ad4080_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct device *dev = &spi->dev;
+	struct ad4080_state *st;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+
+	ret = devm_regulator_bulk_get_enable(dev,
+					     ARRAY_SIZE(ad4080_power_supplies),
+					     ad4080_power_supplies);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to get and enable supplies\n");
+
+	st->regmap = devm_regmap_init_spi(spi, &ad4080_regmap_config);
+	if (IS_ERR(st->regmap))
+		return PTR_ERR(st->regmap);
+
+	st->info = spi_get_device_match_data(spi);
+	if (!st->info)
+		return -ENODEV;
+
+	ret = devm_mutex_init(dev, &st->lock);
+	if (ret)
+		return ret;
+
+	st->info = spi_get_device_match_data(spi);
+	if (!st->info)
+		return -ENODEV;
+
+	indio_dev->name = st->info->name;
+	indio_dev->channels = st->info->channels;
+	indio_dev->num_channels = st->info->num_channels;
+	indio_dev->info = &ad4080_iio_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	ad4080_properties_parse(st);
+
+	st->clk = devm_clk_get_enabled(&spi->dev, "cnv");
+	if (IS_ERR(st->clk))
+		return PTR_ERR(st->clk);
+
+	st->back = devm_iio_backend_get(dev, NULL);
+	if (IS_ERR(st->back))
+		return PTR_ERR(st->back);
+
+	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;
+
+	ret = ad4080_setup(indio_dev);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct spi_device_id ad4080_id[] = {
+	{ "ad4080", (kernel_ulong_t)&ad4080_chip_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ad4080_id);
+
+static const struct of_device_id ad4080_of_match[] = {
+	{ .compatible = "adi,ad4080", &ad4080_chip_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad4080_of_match);
+
+static struct spi_driver ad4080_driver = {
+	.driver = {
+		.name = "ad4080",
+		.of_match_table = ad4080_of_match,
+	},
+	.probe = ad4080_probe,
+	.id_table = ad4080_id,
+};
+module_spi_driver(ad4080_driver);
+
+MODULE_AUTHOR("Antoniu Miclaus <antoniu.miclaus@analog.com");
+MODULE_DESCRIPTION("Analog Devices AD4080");
+MODULE_LICENSE("GPL");
-- 
2.49.0

Re: [PATCH v2 01/13] iio: backend: add support for filter config

[Nuno Sá]

Hi Antoniu,

I do not have time today for going through all the series but I'll already leave
my comment on this on..

On Fri, 2025-04-11 at 15:36 +0300, Antoniu Miclaus wrote:
> Add backend support for digital filter enable/disable.
> 
> This setting can be adjusted within the IP cores interfacing devices.
> 
> The IP core can be configured based on the state of the actual
> digital filter configuration of the part.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> changes in v2:
>  - improve commit description
>  drivers/iio/industrialio-backend.c | 26 ++++++++++++++++++++++++++
>  include/linux/iio/backend.h        |  6 ++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-
> backend.c
> index d4ad36f54090..ffafe7c73508 100644
> --- a/drivers/iio/industrialio-backend.c
> +++ b/drivers/iio/industrialio-backend.c
> @@ -778,6 +778,32 @@ static int __devm_iio_backend_get(struct device *dev,
> struct iio_backend *back)
>  	return 0;
>  }
>  
> +/**
> + * iio_backend_filter_enable - Enable filter
> + * @back: Backend device
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_filter_enable(struct iio_backend *back)
> +{
> +	return iio_backend_op_call(back, filter_enable);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_filter_enable, "IIO_BACKEND");
> +
> +/**
> + * iio_backend_filter_disable - Disable filter
> + * @back: Backend device
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_filter_disable(struct iio_backend *back)
> +{
> +	return iio_backend_op_call(back, filter_disable);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_filter_disable, "IIO_BACKEND");

This seems to resemble the filter_type IIO attr so I would likely be more
explicit in the API naming. Like 'iio_backend_filter_type_set()'. And that also
takes me into the more important point. I would consider having this API taking
an unsigned int filter_type (or an enum with the same possibilities as defined
in the ABI) argument rather than an enable vs disable thing. Like this, we're
just thinking about this particular usecase but it can very well happen that in
the future some backends might need to know the specific filter being
configured. Sure we could change things later on but doing it now is pretty
straight so why not :)?

- Nuno Sá

Re: [PATCH v2 06/13] dt-bindings: iio: adc: add ad408x axi variant

[Rob Herring (Arm)]

On Fri, 11 Apr 2025 15:36:20 +0300, Antoniu Miclaus wrote:
> Add a new compatible and related bindings for the fpga-based
> AD408x AXI IP core, a variant of the generic AXI ADC IP.
> 
> The AXI AD408x IP is a very similar HDL (fpga) variant of the
> generic AXI ADC IP, intended to control ad408x familiy.
> 
> Although there are some particularities added for extended
> control of the ad408x devices such as the filter configuration,
> data capture synchronization procedure and number of lanes enabled.
> 
> Wildcard naming is used to match the naming of the published
> firmware.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> changes in v2:
>  - improve commit description.
>  Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v2 12/13] dt-bindings: iio: adc: add ad4080

[Rob Herring (Arm)]

On Fri, 11 Apr 2025 15:36:26 +0300, Antoniu Miclaus wrote:
> Add devicetree bindings for ad4080 family.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> changes in v2:
>  - add descripton for spi bus
>  - use actual pin for clock name
>  - fix typo in num lanes description
>  - add iio-backends
>  - don't make all supplies mandatory
>  .../bindings/iio/adc/adi,ad4080.yaml          | 96 +++++++++++++++++++
>  1 file changed, 96 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4080.yaml
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v2 10/13] iio: adc: adi-axi-adc: add sync status

[Jonathan Cameron]

On Fri, 11 Apr 2025 15:36:24 +0300
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> Add support for checking the ADC sync status.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> no changes in v2.
>  drivers/iio/adc/adi-axi-adc.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
> index 017685854895..0d12c0121bbc 100644
> --- a/drivers/iio/adc/adi-axi-adc.c
> +++ b/drivers/iio/adc/adi-axi-adc.c
> @@ -56,6 +56,9 @@
>  #define   AXI_AD408X_CNTRL_3_SELF_SYNC_EN_MSK	BIT(1)
>  #define   AXI_AD408X_CNTRL_3_FILTER_EN_MSK	BIT(0)
>  
> +#define ADI_AXI_ADC_REG_SYNC_STATUS		0x0068
> +#define   ADI_AXI_ADC_SYNC			BIT(0)
> +
>  #define ADI_AXI_ADC_REG_DRP_STATUS		0x0074
>  #define   ADI_AXI_ADC_DRP_LOCKED		BIT(17)
>  
> @@ -453,6 +456,21 @@ static int axi_adc_ad408x_self_sync_disable(struct iio_backend *back)
>  				 AXI_AD408X_CNTRL_3_SELF_SYNC_EN_MSK);
>  }
>  
> +static int axi_adc_sync_status_get(struct iio_backend *back, bool *sync_en)
> +{
> +	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
> +	int ret;
> +	u32 val;
> +
> +	ret = regmap_read(st->regmap, ADI_AXI_ADC_REG_SYNC_STATUS, &val);
> +	if (ret)
> +		return ret;
> +
> +	*sync_en = (bool)FIELD_GET(ADI_AXI_ADC_SYNC, val);

Trivial but implicit casting from a bool to an int is fine.  I.e. drop
the (bool) as it doesn't add anything.

> +
> +	return 0;
> +}
> +
>  static struct iio_buffer *axi_adc_request_buffer(struct iio_backend *back,
>  						 struct iio_dev *indio_dev)
>  {
> @@ -600,6 +618,7 @@ static const struct iio_backend_ops adi_axi_adc_ops = {
>  	.test_pattern_set = axi_adc_test_pattern_set,
>  	.chan_status = axi_adc_chan_status,
>  	.interface_type_get = axi_adc_interface_type_get,
> +	.sync_status_get = axi_adc_sync_status_get,
>  	.debugfs_reg_access = iio_backend_debugfs_ptr(axi_adc_reg_access),
>  	.debugfs_print_chan_status = iio_backend_debugfs_ptr(axi_adc_debugfs_print_chan_status),
>  };
> @@ -647,6 +666,7 @@ static const struct iio_backend_ops adi_ad408x_ops = {
>  	.data_alignment_disable = axi_adc_ad408x_bitslip_disable,
>  	.self_sync_enable = axi_adc_ad408x_self_sync_enable,
>  	.self_sync_disable = axi_adc_ad408x_self_sync_disable,
> +	.sync_status_get = axi_adc_sync_status_get,
>  	.debugfs_reg_access = iio_backend_debugfs_ptr(axi_adc_reg_access),
>  	.debugfs_print_chan_status = iio_backend_debugfs_ptr(axi_adc_debugfs_print_chan_status),
>  };

Re: [PATCH v2 13/13] iio: adc: ad4080: add driver support

[Jonathan Cameron]

On Fri, 11 Apr 2025 15:36:27 +0300
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> Add support for AD4080 high-speed, low noise, low distortion,
> 20-bit, Easy Drive, successive approximation register (SAR)
> analog-to-digital converter (ADC).
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
I'll leave the backend stuff to Nuno who has a better feel than
me for what fits in that interface.  So this is just a review
of the rest of this driver.

Various minor comments inline

Thanks,

Jonathan

> diff --git a/drivers/iio/adc/ad4080.c b/drivers/iio/adc/ad4080.c
> new file mode 100644
> index 000000000000..3a0b1ad13765
> --- /dev/null
> +++ b/drivers/iio/adc/ad4080.c

> +/* AD4080_REG_GPIO_CONFIG_B Bit Definition */
> +#define AD4080_GPIO_1_SEL			GENMASK(7, 4)
> +#define AD4080_GPIO_0_SEL			GENMASK(3, 0)
> +
> +/* AD4080_REG_FIFO_CONFIG Bit Definition */
> +#define AD4080_FIFO_MODE_MSK			GENMASK(1, 0)
> +
> +/* AD4080_REG_FILTER_CONFIG Bit Definition */
Better to name the defines to make that association explicit.
#define AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK etc

> +#define AD4080_SINC_DEC_RATE_MSK		GENMASK(6, 3)
> +#define AD4080_FILTER_SEL_MSK			GENMASK(1, 0)
> +
> +/* Miscellaneous Definitions */
> +#define AD4080_SW_RESET				(BIT(7) | BIT(0))
> +#define AD4080_SPI_READ				BIT(7)
> +#define BYTE_ADDR_H				GENMASK(15, 8)
> +#define BYTE_ADDR_L				GENMASK(7, 0)
Definitely not on those two!

If you are going this you are probably reading into the wrong data type.

> +static const unsigned int ad4080_scale_table[][2] = {
> +	{6000, 0},
	{ 6000, 0 },
> +};
> +
> +static const char *const ad4080_filter_type_iio_enum[] = {
> +	[FILTER_DISABLE]   = "disabled",
> +	[SINC_1]           = "sinc1",
> +	[SINC_5]           = "sinc5",
> +	[SINC_5_COMP]      = "sinc5_plus_compensation",
> +};
> +
> +static const int ad4080_dec_rate_iio_enum[] = {
> +	2, 4, 8, 16, 32, 64, 128, 256, 512, 1024
Convention is keep a trailing comma except when we have
an explicit terminating entry (NULL etc)

> +
> +static int ad4080_set_dec_rate(struct iio_dev *dev,
> +			       const struct iio_chan_spec *chan,
> +			       unsigned int mode)
> +{
> +	struct ad4080_state *st = iio_priv(dev);
> +	int ret;
> +	unsigned int data;
> +	unsigned int reg_val;
> +
> +	if (st->filter_type >= SINC_5 && mode >= 512)
> +		return -EINVAL;
> +
> +	guard(mutex)(&st->lock);
> +	ret = regmap_read(st->regmap, AD4080_REG_FILTER_CONFIG, &reg_val);
> +	if (ret)
> +		return ret;
> +
> +	data = ((ilog2(mode) - 1) << 3) | (reg_val & AD4080_FILTER_SEL_MSK);

As below. Odd to keep stuff with explicit mask like this rather than more
normal masking out what we are placing. &= ~SINC_RET_DATA_MSK; etc
 

> +	ret = regmap_write(st->regmap, AD4080_REG_FILTER_CONFIG, data);
> +	if (ret)
> +		return ret;
> +
> +	st->dec_rate = mode;
> +
> +	return ret;
> +}

> +static int ad4080_lvds_sync_write(struct ad4080_state *st)
> +{
> +	unsigned int timeout = 100;
> +	bool sync_en;
> +	int ret;
> +
> +	guard(mutex)(&st->lock);
> +	if (st->num_lanes == 1)
> +		ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +				   AD4080_RESERVED_CONFIG_A_MSK |
> +				   AD4080_INTF_CHK_EN_MSK);
> +	else
> +		ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +				   AD4080_RESERVED_CONFIG_A_MSK |
> +				   AD4080_INTF_CHK_EN_MSK |
> +				   AD4080_SPI_LVDS_LANES_MSK);
> +	if (ret)
> +		return ret;
> +
> +	ret = iio_backend_data_alignment_enable(st->back);
> +	if (ret)
> +		return ret;
> +
> +	do {
> +		ret = iio_backend_sync_status_get(st->back, &sync_en);
> +		if (ret)
> +			return ret;
> +
> +		if (!sync_en)
> +			dev_dbg(&st->spi->dev, "Not Locked: Running Bit Slip\n");

Maybe sleep a bit before trying again?  Tight loops are very dependent on the
host CPU which is probably not what you want here.

> +	} while (--timeout && !sync_en);
> +
> +	if (timeout) {
> +		dev_info(&st->spi->dev, "Success: Pattern correct and Locked!\n");
> +		if (st->num_lanes == 1)
> +			return regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					    AD4080_RESERVED_CONFIG_A_MSK);
> +		else
> +			return regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					    AD4080_RESERVED_CONFIG_A_MSK |
> +					    AD4080_SPI_LVDS_LANES_MSK);
> +	} else {
> +		dev_info(&st->spi->dev, "LVDS Sync Timeout.\n");
> +		if (st->num_lanes == 1) {
> +			ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					   AD4080_RESERVED_CONFIG_A_MSK);
> +			if (ret)
> +				return ret;
> +		} else {
> +			ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					   AD4080_RESERVED_CONFIG_A_MSK |
> +					   AD4080_SPI_LVDS_LANES_MSK);
> +			if (ret)
> +				return ret;
> +		}
> +
> +		return -ETIMEDOUT;
> +	}
> +}

> +
> +static int ad4080_set_filter_type(struct iio_dev *dev,
> +				  const struct iio_chan_spec *chan,
> +				  unsigned int mode)
> +{
> +	struct ad4080_state *st = iio_priv(dev);
> +	int ret;
> +	unsigned int data;
> +	unsigned int reg_val;
> +
> +	if (mode >= SINC_5 && st->dec_rate >= 512)
> +		return -EINVAL;
> +
> +	guard(mutex)(&st->lock);
> +	if (mode)
> +		ret = iio_backend_filter_enable(st->back);
> +	else
> +		ret = iio_backend_filter_disable(st->back);
> +	if (ret)
> +		return ret;
> +
> +	st->filter_en = mode;
> +
> +	ret = regmap_read(st->regmap, AD4080_REG_FILTER_CONFIG, &reg_val);
> +	if (ret)
> +		return ret;
> +
> +	data = (reg_val & AD4080_SINC_DEC_RATE_MSK) |
> +	       (mode & AD4080_FILTER_SEL_MSK);

FIELD_PREP() for the second part.
The first is hanging on to one field.  Maybe just pull that out with
a FIELD_GET() and write it back again with FIELD_PREP?
Will be more code, but a little less subtle to read!


> +
> +	ret = regmap_write(st->regmap, AD4080_REG_FILTER_CONFIG, data);
> +	if (ret)
> +		return ret;
> +
> +	st->filter_type = mode;
> +
> +	return ret;
> +}

> +static struct iio_chan_spec_ext_info ad4080_ext_info[] = {
> +	IIO_ENUM("filter_type",
> +		 IIO_SHARED_BY_ALL,
> +		 &ad4080_filter_type_enum),
very short line wrap - aim for nearer 80 chars.

> +	IIO_ENUM_AVAILABLE("filter_type",
> +			   IIO_SHARED_BY_ALL,
> +			   &ad4080_filter_type_enum),
> +	{}
Trivial preference for 
	{ }

> +};
> +
> +#define AD4080_CHAN(_chan, _si, _bits, _sign, _shift)		\
> +	{ .type = IIO_VOLTAGE,	
Odd indent. Better perhaps as simpler
	{ \
		.indexed = 1, 
etc.					\
> +	  .indexed = 1,							\
> +	  .channel = _chan,						\
> +	  .info_mask_separate = BIT(IIO_CHAN_INFO_SCALE),		\
> +	  .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) |	\
> +		BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),			\
> +	  .info_mask_shared_by_all_available =				\
> +		BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),			\
> +	  .ext_info = ad4080_ext_info,					\
> +	  .scan_index = _si,						\
> +	  .scan_type = {						\
> +			.sign = _sign,					\
Current indent makes this look really wierd!

> +			.realbits = _bits,				\
> +			.storagebits = 32,				\
> +			.shift = _shift,				\
> +	  },								\
> +	}
> +
> +static const struct iio_chan_spec ad4080_channels[] = {
> +	AD4080_CHAN(0, 0, 20, 's', 0)
Don't bother with the macro as it doesn't add anything. Just put that
stuff all here. That will let you skip setting obvious defaults to 0
like the shift.

> +};
> +
> +static const struct ad4080_chip_info ad4080_chip_info = {
> +	.name = "AD4080",
> +	.product_id = AD4080_CHIP_ID,
> +	.scale_table = ad4080_scale_table,
> +	.num_scales = ARRAY_SIZE(ad4080_scale_table),
> +	.num_channels = 1,
> +	.channels = ad4080_channels,
> +};
> +
> +static int ad4080_setup(struct iio_dev *indio_dev)
> +{

> +	if (id != AD4080_CHIP_ID)
> +		return dev_err_probe(&st->spi->dev, -EINVAL,
> +				     "Unrecognized CHIP_ID 0x%X\n", id);
A mismatch on ID should not be treated as an error. That breaks the
use of fallback dt compatibles.  So convention on these is a dev_info()
and carry on anyway.  We've left breadcrumbs if things don't work but
not our role to make sure it is definitely the right hardware in the
firmware description.

> +
> +	ret = regmap_set_bits(st->regmap, AD4080_REG_GPIO_CONFIG_A,
> +			      AD4080_GPO_1_EN);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, AD4080_REG_GPIO_CONFIG_B,
> +			   FIELD_PREP(AD4080_GPIO_1_SEL, 3));
> +	if (ret)
> +		return ret;
> +
> +	ret = iio_backend_num_lanes_set(st->back, st->num_lanes);
> +	if (ret)
> +		return ret;
> +
> +	ret = iio_backend_self_sync_enable(st->back);
> +	if (ret)
> +		return ret;
> +
> +	if (st->lvds_cnv_en) {
I'd flip this unless you expect to shortly add more optional stuff after this
code

	if (!st->lvds_cnv_en)
		return 0;

	..

> +		if (st->num_lanes) {
> +			ret = regmap_update_bits(st->regmap,
> +						 AD4080_REG_ADC_DATA_INTF_CONFIG_B,
> +						 AD4080_LVDS_CNV_CLK_CNT_MSK,
> +						 FIELD_PREP(AD4080_LVDS_CNV_CLK_CNT_MSK, 7));
> +			if (ret)
> +				return ret;
> +		}
> +
> +		ret = regmap_set_bits(st->regmap,
> +				      AD4080_REG_ADC_DATA_INTF_CONFIG_B,
> +				      AD4080_LVDS_CNV_EN_MSK);
> +		if (ret)
> +			return ret;
> +
> +		return ad4080_lvds_sync_write(st);
> +	}
> +
> +	return 0;
> +}
> +
> +static void ad4080_properties_parse(struct ad4080_state *st)
> +{
> +	unsigned int val;
> +	int ret;
> +
> +	st->lvds_cnv_en = device_property_read_bool(&st->spi->dev,
> +						    "adi,lvds-cnv-enable");
> +
> +	st->num_lanes = 1;
> +	ret = device_property_read_u32(&st->spi->dev, "adi,num_lanes", &val);
> +	if (!ret)
> +		st->num_lanes = val;
Usual trick on these places were we have a default is to pick types correctly 
and do.

	st->num_lanes = 1;
	device_property_read_u32(&st->spi->dev, "adi,num_lanes", &st->num_lanes);

That is, rely on the call being side effect free on error.
 
> +}

Re: [PATCH v2 13/13] iio: adc: ad4080: add driver support

[Nuno Sá]

Hi Antoniu,

I still have doubts regarding the new backend API mostly because it's still
unclear how it should all work. I had some questions in the first version of the
series that were not clarified so I'll likely repeat myself... Please don't rush
into a v3 without having this sorted out.

Anyways, see below...
 
On Fri, 2025-04-11 at 15:36 +0300, Antoniu Miclaus wrote:
> Add support for AD4080 high-speed, low noise, low distortion,
> 20-bit, Easy Drive, successive approximation register (SAR)
> analog-to-digital converter (ADC).
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> changes in v2:
>  - set num_lanes during probe.
>  - rename bitslitp to iio_backend_data_alignment_enable
>  - do ad4080_lvds_sync_write only once during probe.
>  - use ETIMEDOUT
>  - rename to `cnv` instead of `adc-clk`
>  - update Kconfig help section
>  - drop extra blank line.
>  - replace `/**` with `/*` for comments
>  - drop redundant return 0.
>  - use `dev_dbg` during while(--timeout) procedure
>  - use while (--timeout && !sync_en)
>  - return -ETIME where applicable and check missing return codes.
>  - regmap_update_bits used where applicable
>  - use defines instead of GENMASK inline.
>  - return FIELD_GET()
>  - st->filter_enabled = mode and dropping the if else statement.
>  - remove redundant brackets.
>  - use OVERSAMPLING_RATIO attribute and drop custom ABI for it
>  - use already existing filter_type attribute
>  - fix indentation
>  - remove comma on 'terminators'
>  - use dev_err_probe instead of dev_err
>  - check missing return values.
>  - rework num_lanes property parse.
>  - keep ad4080_chip_info since the driver will be extended for more parts
>    in the future (also stated in cover letter).
>  drivers/iio/adc/Kconfig  |  14 +
>  drivers/iio/adc/Makefile |   1 +
>  drivers/iio/adc/ad4080.c | 653 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 668 insertions(+)
>  create mode 100644 drivers/iio/adc/ad4080.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 27413516216c..17df328f5322 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -47,6 +47,20 @@ config AD4030
>  	  To compile this driver as a module, choose M here: the module will
> be
>  	  called ad4030.
>  
> +config AD4080
> +	tristate "Analog Devices AD4080 high speed ADC"
> +	depends on SPI
> +	select REGMAP_SPI
> +	select IIO_BACKEND
> +	help
> +	  Say yes here to build support for Analog Devices AD4080
> +	  high speed, low noise, low distortion, 20-bit, Easy Drive,
> +	  successive approximation register (SAR) analog-to-digital
> +	  converter (ADC). Supports iio_backended devices for AD4080.
> +
> +	  To compile this driver as a module, choose M here: the module will
> be
> +	  called ad4080.
> +
>  config AD4130
>  	tristate "Analog Device AD4130 ADC Driver"
>  	depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 9f26d5eca822..e6efed5b4e7a 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
>  obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
>  obj-$(CONFIG_AD4000) += ad4000.o
>  obj-$(CONFIG_AD4030) += ad4030.o
> +obj-$(CONFIG_AD4080) += ad4080.o
>  obj-$(CONFIG_AD4130) += ad4130.o
>  obj-$(CONFIG_AD4695) += ad4695.o
>  obj-$(CONFIG_AD4851) += ad4851.o
> diff --git a/drivers/iio/adc/ad4080.c b/drivers/iio/adc/ad4080.c
> new file mode 100644
> index 000000000000..3a0b1ad13765
> --- /dev/null
> +++ b/drivers/iio/adc/ad4080.c
> @@ -0,0 +1,653 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Analog Devices AD4080 SPI ADC driver
> + *
> + * Copyright 2025 Analog Devices Inc.
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
> +#include <linux/units.h>
> +
> 

...

> +static int ad4080_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long m)
> +{
> +	struct ad4080_state *st = iio_priv(indio_dev);
> +	int dec_rate;
> +
> +	switch (m) {
> +	case IIO_CHAN_INFO_SCALE:
> +		return ad4080_get_scale(st, val, val2);
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		if (st->filter_type == SINC_5_COMP)
> +			dec_rate = st->dec_rate * 2;
> +		else
> +			dec_rate = st->dec_rate;
> +		if (st->filter_en)
> +			*val = DIV_ROUND_CLOSEST(clk_get_rate(st->clk),
> dec_rate);
> +		else
> +			*val = clk_get_rate(st->clk);

Are we expecting the clock rate to change at runtime. Typically, we can just
cache the rate during probe and do it "dynamically" if we ever need it.

> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		*val = ad4080_get_dec_rate(indio_dev, chan);
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ad4080_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int val, int val2, long mask)
> +{
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		return -EINVAL;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		return -EINVAL;
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		return ad4080_set_dec_rate(indio_dev, chan, val);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ad4080_lvds_sync_write(struct ad4080_state *st)
> +{

This whole function seems to depend on using the LVDS signal for cnv. Why? If we
have CMOS don't we also need some kind of interface alignment/calibration? If
not, that's sensible enough that should be stated in a comment.

> +	unsigned int timeout = 100;
> +	bool sync_en;
> +	int ret;
> +
> +	guard(mutex)(&st->lock);
> +	if (st->num_lanes == 1)
> +		ret = regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +				   AD4080_RESERVED_CONFIG_A_MSK |
> +				   AD4080_INTF_CHK_EN_MSK);
> +	else
> +		ret = regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +				   AD4080_RESERVED_CONFIG_A_MSK |
> +				   AD4080_INTF_CHK_EN_MSK |
> +				   AD4080_SPI_LVDS_LANES_MSK);
> +	if (ret)
> +		return ret;
> +
> +	ret = iio_backend_data_alignment_enable(st->back);
> +	if (ret)
> +		return ret;
> +
> +	do {
> +		ret = iio_backend_sync_status_get(st->back, &sync_en);
> +		if (ret)
> +			return ret;
> +
> +		if (!sync_en)
> +			dev_dbg(&st->spi->dev, "Not Locked: Running Bit
> Slip\n");
> +	} while (--timeout && !sync_en);
> +
> +	if (timeout) {
> +		dev_info(&st->spi->dev, "Success: Pattern correct and
> Locked!\n");
> +		if (st->num_lanes == 1)
> +			return regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					    AD4080_RESERVED_CONFIG_A_MSK);
> +		else
> +			return regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					    AD4080_RESERVED_CONFIG_A_MSK |
> +					    AD4080_SPI_LVDS_LANES_MSK);
> +	} else {
> +		dev_info(&st->spi->dev, "LVDS Sync Timeout.\n");
> +		if (st->num_lanes == 1) {
> +			ret = regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					   AD4080_RESERVED_CONFIG_A_MSK);
> +			if (ret)
> +				return ret;
> +		} else {
> +			ret = regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					   AD4080_RESERVED_CONFIG_A_MSK |
> +					   AD4080_SPI_LVDS_LANES_MSK);
> +			if (ret)
> +				return ret;
> +		}
> +
> +		return -ETIMEDOUT;

So, first the things that I don't really get (also the hdl docs need to be
improved FWIW):

* It seems that we can have data alignment or data capture synchronization
through an internal AMD FPGA technique called bit-slip or an external signal,
right? In here, it seems that we only use bit-slip and never disable it. Is that
really the goal?

* This whole process seems to be some kind of calibration at the interface
level, right?

* What's the point of the conv clock? Is it only used to get the sample rate? If
so, the hdl docs are misleading as it seems that it can be used instead of bit-
slip for data capture alignment?

Now, speaking about the backend API's, it looks like that
iio_backend_self_sync_enable() and iio_backend_data_alignment_enable() could be
merged into one API. From what I can tell, iio_backend_data_alignment_enable()
just enables the bit-slip process but that likely does nothing unless the
SELF_SYNC bit is also set to bit-slip, right? In fact, we could think about a
more generic (less flexible though) API like:

* iio_backend_intf_data_align(back, timeout_us), or
* iio_backend_intf_calib(back, timeout_us), or
* iio_backend_intf_data_capture_sync(back, timeout_us)

On the backend side, you could then enable bit-slip and do the status read (with
timeout) and return success or an error code.

The above seems to be pretty much what you're doing but in just one API that
makes sense to me.

Of course that the following questions still come to mind:

* Do we need to disable bit-slip after we're done (still fits into the one API
model)?
* Do we need a meaningful API to change between the syncing/alignment methods?
External signal vs bit-slip?

The above is key to better think of an API. Right now it feels that you're just
adding an API for every bit you want to control in this process...

If we end up needing more flexibility for this, we can also consider the
existing iio_backend_data_sample_trigger() API. I know is abusing a bit and a
stretch but in the end of the day the whole thing is related with aligning,
syncing, calibrating the interface for properly sampling data. Even bit-slip
(while not a traditional external trigger) looks like some kind of self-
adjusting, data-driven trigger mechanism to establish the correct starting point
for capturing data. So having two new enums like:

IIO_BACKEND_SAMPLE_TRIGGER_EXT_SIGNAL,
IIO_BACKEND_SAMPLE_TRIGGER_BIT_SLIP // or maybe a more generic name like
s/BIT_SLIP/INTERNAL

I do not think the above is that horrible :P... But I would really like to get
more opinions about this.

> +	}
> +}

...

> 
> +static const struct ad4080_chip_info ad4080_chip_info = {
> +	.name = "AD4080",
> +	.product_id = AD4080_CHIP_ID,
> +	.scale_table = ad4080_scale_table,
> +	.num_scales = ARRAY_SIZE(ad4080_scale_table),
> +	.num_channels = 1,
> +	.channels = ad4080_channels,
> +};
> +
> +static int ad4080_setup(struct iio_dev *indio_dev)
> +{
> +	struct ad4080_state *st = iio_priv(indio_dev);
> +	unsigned int id;
> +	int ret;
> +
> +	ret = regmap_write(st->regmap, AD4080_REG_INTERFACE_CONFIG_A,
> +			   AD4080_SW_RESET);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, AD4080_REG_INTERFACE_CONFIG_A,
> +			   AD4080_SDO_ENABLE_MSK);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(st->regmap, AD4080_REG_CHIP_TYPE, &id);
> +	if (ret)
> +		return ret;
> +
> +	if (id != AD4080_CHIP_ID)
> +		return dev_err_probe(&st->spi->dev, -EINVAL,
> +				     "Unrecognized CHIP_ID 0x%X\n", id);
> +
> +	ret = regmap_set_bits(st->regmap, AD4080_REG_GPIO_CONFIG_A,
> +			      AD4080_GPO_1_EN);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, AD4080_REG_GPIO_CONFIG_B,
> +			   FIELD_PREP(AD4080_GPIO_1_SEL, 3));
> +	if (ret)
> +		return ret;
> +
> +	ret = iio_backend_num_lanes_set(st->back, st->num_lanes);
> +	if (ret)
> +		return ret;
> +
> +	ret = iio_backend_self_sync_enable(st->back);
> +	if (ret)
> +		return ret;
> +

AFAIU, the above is enabling bit-slip?
 
> +	if (st->lvds_cnv_en) {
> +		if (st->num_lanes) {
> +			ret = regmap_update_bits(st->regmap,
> +						
> AD4080_REG_ADC_DATA_INTF_CONFIG_B,
> +						 AD4080_LVDS_CNV_CLK_CNT_MSK,
> +						
> FIELD_PREP(AD4080_LVDS_CNV_CLK_CNT_MSK, 7));
> +			if (ret)
> +				return ret;
> +		}
> +
> +		ret = regmap_set_bits(st->regmap,
> +				      AD4080_REG_ADC_DATA_INTF_CONFIG_B,
> +				      AD4080_LVDS_CNV_EN_MSK);
> +		if (ret)
> +			return ret;
> +
> +		return ad4080_lvds_sync_write(st);
> +	}
> +
> +	return 0;
> +}
> +
> +static void ad4080_properties_parse(struct ad4080_state *st)
> +{
> +	unsigned int val;
> +	int ret;
> +
> +	st->lvds_cnv_en = device_property_read_bool(&st->spi->dev,
> +						    "adi,lvds-cnv-enable");
> +

nit: I would probably drop the enable part. The property is about stating that
the signal is LVDS instead of CMOS. And IIUC, this should also depend on the
existence of `st->clk`
 
> +	st->num_lanes = 1;
> +	ret = device_property_read_u32(&st->spi->dev, "adi,num_lanes", &val);
> +	if (!ret)
> +		st->num_lanes = val;
> +}
> +
> +static int ad4080_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct device *dev = &spi->dev;
> +	struct ad4080_state *st;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	st->spi = spi;
> +
> +	ret = devm_regulator_bulk_get_enable(dev,
> +					    
> ARRAY_SIZE(ad4080_power_supplies),
> +					     ad4080_power_supplies);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "failed to get and enable supplies\n");
> +
> +	st->regmap = devm_regmap_init_spi(spi, &ad4080_regmap_config);
> +	if (IS_ERR(st->regmap))
> +		return PTR_ERR(st->regmap);
> +
> +	st->info = spi_get_device_match_data(spi);
> +	if (!st->info)
> +		return -ENODEV;
> +
> +	ret = devm_mutex_init(dev, &st->lock);
> +	if (ret)
> +		return ret;
> +
> +	st->info = spi_get_device_match_data(spi);
> +	if (!st->info)
> +		return -ENODEV;
> +

The above is duplicated...

> +	indio_dev->name = st->info->name;
> +	indio_dev->channels = st->info->channels;
> +	indio_dev->num_channels = st->info->num_channels;
> +	indio_dev->info = &ad4080_iio_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	ad4080_properties_parse(st);
> +
> +	st->clk = devm_clk_get_enabled(&spi->dev, "cnv");
> +	if (IS_ERR(st->clk))
> +		return PTR_ERR(st->clk);
> +

From the datasheet it looks like this clock is optional? Moreover in the IP docs
we have the following:


"SELF_SYNC: Controls if the data capture synchronization is done through CNV
signal or bit-slip."

So I wonder, is the cnv clock meaning that we want to have capture
sync/alignment through that external signal instead of bit-slip?

- Nuno Sá

Re: [PATCH v2 03/13] iio: backend: add support for self sync

[Nuno Sá]

On Fri, 2025-04-11 at 15:36 +0300, Antoniu Miclaus wrote:
> Add iio backend support for self sync enable/disable.
> When disabled data capture synchronization is done
> through CNV signal, otherwise through bit-slip.
> 

It is still not clear what API's will come out of this but you need to improve
the commit description (and the API docs). Note that the above is very focused
on your use case and you're just using the same names defined in the device docs
and hdl IP. While that might be ok for the device drivers commits, it is not
that nice for something that "tries" to be more generic.

This is true for all the new API's being introduced...

- Nuno Sá

> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> no changes in v2.
>  drivers/iio/industrialio-backend.c | 30 ++++++++++++++++++++++++++++++
>  include/linux/iio/backend.h        |  6 ++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-
> backend.c
> index 60578267643d..cb23433b22c6 100644
> --- a/drivers/iio/industrialio-backend.c
> +++ b/drivers/iio/industrialio-backend.c
> @@ -830,6 +830,36 @@ int iio_backend_data_alignment_disable(struct iio_backend
> *back)
>  }
>  EXPORT_SYMBOL_NS_GPL(iio_backend_data_alignment_disable, "IIO_BACKEND");
>  
> +/**
> + * iio_backend_self_sync_enable - Enable the self sync data capture.
> + * @back: Backend device
> + *
> + * Data capture synchronization is done through bit-slip.
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_self_sync_enable(struct iio_backend *back)
> +{
> +	return iio_backend_op_call(back, self_sync_enable);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_self_sync_enable, "IIO_BACKEND");
> +
> +/**
> + * iio_backend_self_sync_disable - Disable the self sync data capture.
> + * @back: Backend device
> + *
> + * Data capture synchronization is done through CNV signal.
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_self_sync_disable(struct iio_backend *back)
> +{
> +	return iio_backend_op_call(back, self_sync_disable);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_self_sync_disable, "IIO_BACKEND");
> +
>  /**
>   * iio_backend_ddr_enable - Enable interface DDR (Double Data Rate) mode
>   * @back: Backend device
> diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
> index beff66d18151..6d006cb0da5a 100644
> --- a/include/linux/iio/backend.h
> +++ b/include/linux/iio/backend.h
> @@ -104,6 +104,8 @@ enum iio_backend_interface_type {
>   * @filter_disable: Disable filter.
>   * @data_alignment_enable: Enable sync process.
>   * @data_alignment_disable: Disable sync process.
> + * @self_sync_enable: Enable the self sync data capture.
> + * @self_sync_disable: Disable the self sync data capture.
>   * @ddr_enable: Enable interface DDR (Double Data Rate) mode.
>   * @ddr_disable: Disable interface DDR (Double Data Rate) mode.
>   * @data_stream_enable: Enable data stream.
> @@ -158,6 +160,8 @@ struct iio_backend_ops {
>  	int (*filter_disable)(struct iio_backend *back);
>  	int (*data_alignment_enable)(struct iio_backend *back);
>  	int (*data_alignment_disable)(struct iio_backend *back);
> +	int (*self_sync_enable)(struct iio_backend *back);
> +	int (*self_sync_disable)(struct iio_backend *back);
>  	int (*ddr_enable)(struct iio_backend *back);
>  	int (*ddr_disable)(struct iio_backend *back);
>  	int (*data_stream_enable)(struct iio_backend *back);
> @@ -202,6 +206,8 @@ int iio_backend_filter_enable(struct iio_backend *back);
>  int iio_backend_filter_disable(struct iio_backend *back);
>  int iio_backend_data_alignment_enable(struct iio_backend *back);
>  int iio_backend_data_alignment_disable(struct iio_backend *back);
> +int iio_backend_self_sync_enable(struct iio_backend *back);
> +int iio_backend_self_sync_disable(struct iio_backend *back);
>  int iio_backend_ddr_enable(struct iio_backend *back);
>  int iio_backend_ddr_disable(struct iio_backend *back);
>  int iio_backend_data_stream_enable(struct iio_backend *back);

Re: [PATCH v2 13/13] iio: adc: ad4080: add driver support

[Jonathan Cameron]

			   AD4080_SPI_LVDS_LANES_MSK);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = iio_backend_data_alignment_enable(st->back);
> > +	if (ret)
> > +		return ret;
> > +
> > +	do {
> > +		ret = iio_backend_sync_status_get(st->back, &sync_en);
> > +		if (ret)
> > +			return ret;
> > +
> > +		if (!sync_en)
> > +			dev_dbg(&st->spi->dev, "Not Locked: Running Bit
> > Slip\n");
> > +	} while (--timeout && !sync_en);
> > +
> > +	if (timeout) {
> > +		dev_info(&st->spi->dev, "Success: Pattern correct and
> > Locked!\n");
> > +		if (st->num_lanes == 1)
> > +			return regmap_write(st->regmap,
> > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > +					    AD4080_RESERVED_CONFIG_A_MSK);
> > +		else
> > +			return regmap_write(st->regmap,
> > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > +					    AD4080_RESERVED_CONFIG_A_MSK |
> > +					    AD4080_SPI_LVDS_LANES_MSK);
> > +	} else {
> > +		dev_info(&st->spi->dev, "LVDS Sync Timeout.\n");
> > +		if (st->num_lanes == 1) {
> > +			ret = regmap_write(st->regmap,
> > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > +					   AD4080_RESERVED_CONFIG_A_MSK);
> > +			if (ret)
> > +				return ret;
> > +		} else {
> > +			ret = regmap_write(st->regmap,
> > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > +					   AD4080_RESERVED_CONFIG_A_MSK |
> > +					   AD4080_SPI_LVDS_LANES_MSK);
> > +			if (ret)
> > +				return ret;
> > +		}
> > +
> > +		return -ETIMEDOUT;  
> 
> So, first the things that I don't really get (also the hdl docs need to be
> improved FWIW):
> 
> * It seems that we can have data alignment or data capture synchronization
> through an internal AMD FPGA technique called bit-slip or an external signal,
> right? In here, it seems that we only use bit-slip and never disable it. Is that
> really the goal?
> 
> * This whole process seems to be some kind of calibration at the interface
> level, right?
> 
> * What's the point of the conv clock? Is it only used to get the sample rate? If
> so, the hdl docs are misleading as it seems that it can be used instead of bit-
> slip for data capture alignment?
> 
> Now, speaking about the backend API's, it looks like that
> iio_backend_self_sync_enable() and iio_backend_data_alignment_enable() could be
> merged into one API. From what I can tell, iio_backend_data_alignment_enable()
> just enables the bit-slip process but that likely does nothing unless the
> SELF_SYNC bit is also set to bit-slip, right? In fact, we could think about a
> more generic (less flexible though) API like:
> 
> * iio_backend_intf_data_align(back, timeout_us), or
> * iio_backend_intf_calib(back, timeout_us), or
> * iio_backend_intf_data_capture_sync(back, timeout_us)
> 
> On the backend side, you could then enable bit-slip and do the status read (with
> timeout) and return success or an error code.
> 
> The above seems to be pretty much what you're doing but in just one API that
> makes sense to me.
> 
> Of course that the following questions still come to mind:
> 
> * Do we need to disable bit-slip after we're done (still fits into the one API
> model)?
> * Do we need a meaningful API to change between the syncing/alignment methods?
> External signal vs bit-slip?
> 
> The above is key to better think of an API. Right now it feels that you're just
> adding an API for every bit you want to control in this process...
> 
> If we end up needing more flexibility for this, we can also consider the
> existing iio_backend_data_sample_trigger() API. I know is abusing a bit and a
> stretch but in the end of the day the whole thing is related with aligning,
> syncing, calibrating the interface for properly sampling data. Even bit-slip
> (while not a traditional external trigger) looks like some kind of self-
> adjusting, data-driven trigger mechanism to establish the correct starting point
> for capturing data. So having two new enums like:
> 
> IIO_BACKEND_SAMPLE_TRIGGER_EXT_SIGNAL,
> IIO_BACKEND_SAMPLE_TRIGGER_BIT_SLIP // or maybe a more generic name like
> s/BIT_SLIP/INTERNAL
> 
> I do not think the above is that horrible :P... But I would really like to get
> more opinions about this.

Seems reasonable to me to combine the two as long as we have good documentation.
Agreed it is kind of deriving a trigger from the signal so turning it on with
a trigger type selection doesn't seem unreasonable.

Name tricky though.

Jonathan

> 
> > +	}
> > +}  
> 
> ...
> 
> > 
> > +static const struct ad4080_chip_info ad4080_chip_info = {
> > +	.name = "AD4080",
> > +	.product_id = AD4080_CHIP_ID,
> > +	.scale_table = ad4080_scale_table,
> > +	.num_scales = ARRAY_SIZE(ad4080_scale_table),
> > +	.num_channels = 1,
> > +	.channels = ad4080_channels,
> > +};
> > +
> > +static int ad4080_setup(struct iio_dev *indio_dev)
> > +{
> > +	struct ad4080_state *st = iio_priv(indio_dev);
> > +	unsigned int id;
> > +	int ret;
> > +
> > +	ret = regmap_write(st->regmap, AD4080_REG_INTERFACE_CONFIG_A,
> > +			   AD4080_SW_RESET);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = regmap_write(st->regmap, AD4080_REG_INTERFACE_CONFIG_A,
> > +			   AD4080_SDO_ENABLE_MSK);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = regmap_read(st->regmap, AD4080_REG_CHIP_TYPE, &id);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (id != AD4080_CHIP_ID)
> > +		return dev_err_probe(&st->spi->dev, -EINVAL,
> > +				     "Unrecognized CHIP_ID 0x%X\n", id);
> > +
> > +	ret = regmap_set_bits(st->regmap, AD4080_REG_GPIO_CONFIG_A,
> > +			      AD4080_GPO_1_EN);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = regmap_write(st->regmap, AD4080_REG_GPIO_CONFIG_B,
> > +			   FIELD_PREP(AD4080_GPIO_1_SEL, 3));
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = iio_backend_num_lanes_set(st->back, st->num_lanes);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = iio_backend_self_sync_enable(st->back);
> > +	if (ret)
> > +		return ret;
> > +  
> 
> AFAIU, the above is enabling bit-slip?
>  
> > +	if (st->lvds_cnv_en) {
> > +		if (st->num_lanes) {
> > +			ret = regmap_update_bits(st->regmap,
> > +						
> > AD4080_REG_ADC_DATA_INTF_CONFIG_B,
> > +						 AD4080_LVDS_CNV_CLK_CNT_MSK,
> > +						
> > FIELD_PREP(AD4080_LVDS_CNV_CLK_CNT_MSK, 7));
> > +			if (ret)
> > +				return ret;
> > +		}
> > +
> > +		ret = regmap_set_bits(st->regmap,
> > +				      AD4080_REG_ADC_DATA_INTF_CONFIG_B,
> > +				      AD4080_LVDS_CNV_EN_MSK);
> > +		if (ret)
> > +			return ret;
> > +
> > +		return ad4080_lvds_sync_write(st);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void ad4080_properties_parse(struct ad4080_state *st)
> > +{
> > +	unsigned int val;
> > +	int ret;
> > +
> > +	st->lvds_cnv_en = device_property_read_bool(&st->spi->dev,
> > +						    "adi,lvds-cnv-enable");
> > +  
> 
> nit: I would probably drop the enable part. The property is about stating that
> the signal is LVDS instead of CMOS. And IIUC, this should also depend on the
> existence of `st->clk`
>  
> > +	st->num_lanes = 1;
> > +	ret = device_property_read_u32(&st->spi->dev, "adi,num_lanes", &val);
> > +	if (!ret)
> > +		st->num_lanes = val;
> > +}
> > +
> > +static int ad4080_probe(struct spi_device *spi)
> > +{
> > +	struct iio_dev *indio_dev;
> > +	struct device *dev = &spi->dev;
> > +	struct ad4080_state *st;
> > +	int ret;
> > +
> > +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> > +	if (!indio_dev)
> > +		return -ENOMEM;
> > +
> > +	st = iio_priv(indio_dev);
> > +	st->spi = spi;
> > +
> > +	ret = devm_regulator_bulk_get_enable(dev,
> > +					    
> > ARRAY_SIZE(ad4080_power_supplies),
> > +					     ad4080_power_supplies);
> > +	if (ret)
> > +		return dev_err_probe(dev, ret,
> > +				     "failed to get and enable supplies\n");
> > +
> > +	st->regmap = devm_regmap_init_spi(spi, &ad4080_regmap_config);
> > +	if (IS_ERR(st->regmap))
> > +		return PTR_ERR(st->regmap);
> > +
> > +	st->info = spi_get_device_match_data(spi);
> > +	if (!st->info)
> > +		return -ENODEV;
> > +
> > +	ret = devm_mutex_init(dev, &st->lock);
> > +	if (ret)
> > +		return ret;
> > +
> > +	st->info = spi_get_device_match_data(spi);
> > +	if (!st->info)
> > +		return -ENODEV;
> > +  
> 
> The above is duplicated...
> 
> > +	indio_dev->name = st->info->name;
> > +	indio_dev->channels = st->info->channels;
> > +	indio_dev->num_channels = st->info->num_channels;
> > +	indio_dev->info = &ad4080_iio_info;
> > +	indio_dev->modes = INDIO_DIRECT_MODE;
> > +
> > +	ad4080_properties_parse(st);
> > +
> > +	st->clk = devm_clk_get_enabled(&spi->dev, "cnv");
> > +	if (IS_ERR(st->clk))
> > +		return PTR_ERR(st->clk);
> > +  
> 
> From the datasheet it looks like this clock is optional? Moreover in the IP docs
> we have the following:
> 
> 
> "SELF_SYNC: Controls if the data capture synchronization is done through CNV
> signal or bit-slip."
> 
> So I wonder, is the cnv clock meaning that we want to have capture
> sync/alignment through that external signal instead of bit-slip?
> 
> - Nuno Sá

RE: [PATCH v2 13/13] iio: adc: ad4080: add driver support

[Miclaus, Antoniu]

> > +	int ret;
> > +
> > +	guard(mutex)(&st->lock);
> > +	if (st->num_lanes == 1)
> > +		ret = regmap_write(st->regmap,
> > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > +				   AD4080_RESERVED_CONFIG_A_MSK |
> > +				   AD4080_INTF_CHK_EN_MSK);
> > +	else
> > +		ret = regmap_write(st->regmap,
> > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > +				   AD4080_RESERVED_CONFIG_A_MSK |
> > +				   AD4080_INTF_CHK_EN_MSK |
> > +				   AD4080_SPI_LVDS_LANES_MSK);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = iio_backend_data_alignment_enable(st->back);
> > +	if (ret)
> > +		return ret;
> > +
> > +	do {
> > +		ret = iio_backend_sync_status_get(st->back, &sync_en);
> > +		if (ret)
> > +			return ret;
> > +
> > +		if (!sync_en)
> > +			dev_dbg(&st->spi->dev, "Not Locked: Running Bit
> > Slip\n");
> > +	} while (--timeout && !sync_en);
> > +
> > +	if (timeout) {
> > +		dev_info(&st->spi->dev, "Success: Pattern correct and
> > Locked!\n");
> > +		if (st->num_lanes == 1)
> > +			return regmap_write(st->regmap,
> > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > +
> 	    AD4080_RESERVED_CONFIG_A_MSK);
> > +		else
> > +			return regmap_write(st->regmap,
> > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > +
> 	    AD4080_RESERVED_CONFIG_A_MSK |
> > +					    AD4080_SPI_LVDS_LANES_MSK);
> > +	} else {
> > +		dev_info(&st->spi->dev, "LVDS Sync Timeout.\n");
> > +		if (st->num_lanes == 1) {
> > +			ret = regmap_write(st->regmap,
> > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > +
> 	   AD4080_RESERVED_CONFIG_A_MSK);
> > +			if (ret)
> > +				return ret;
> > +		} else {
> > +			ret = regmap_write(st->regmap,
> > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > +
> 	   AD4080_RESERVED_CONFIG_A_MSK |
> > +					   AD4080_SPI_LVDS_LANES_MSK);
> > +			if (ret)
> > +				return ret;
> > +		}
> > +
> > +		return -ETIMEDOUT;
> 
> So, first the things that I don't really get (also the hdl docs need to be
> improved FWIW):
> 
> * It seems that we can have data alignment or data capture synchronization
> through an internal AMD FPGA technique called bit-slip or an external signal,
> right? In here, it seems that we only use bit-slip and never disable it. Is that
> really the goal?
> 
> * This whole process seems to be some kind of calibration at the interface
> level, right?
> 
> * What's the point of the conv clock? Is it only used to get the sample rate? If
> so, the hdl docs are misleading as it seems that it can be used instead of bit-
> slip for data capture alignment?
> 
> Now, speaking about the backend API's, it looks like that
> iio_backend_self_sync_enable() and iio_backend_data_alignment_enable()
> could be
> merged into one API. From what I can tell,
> iio_backend_data_alignment_enable()
> just enables the bit-slip process but that likely does nothing unless the
> SELF_SYNC bit is also set to bit-slip, right? In fact, we could think about a
> more generic (less flexible though) API like:
> 
> * iio_backend_intf_data_align(back, timeout_us), or
> * iio_backend_intf_calib(back, timeout_us), or
> * iio_backend_intf_data_capture_sync(back, timeout_us)
> 
> On the backend side, you could then enable bit-slip and do the status read
> (with
> timeout) and return success or an error code.
> 
> The above seems to be pretty much what you're doing but in just one API that
> makes sense to me.
> 
> Of course that the following questions still come to mind:
> 
> * Do we need to disable bit-slip after we're done (still fits into the one API
> model)?
> * Do we need a meaningful API to change between the syncing/alignment
> methods?
> External signal vs bit-slip?
> 
> The above is key to better think of an API. Right now it feels that you're just
> adding an API for every bit you want to control in this process...
> 
> If we end up needing more flexibility for this, we can also consider the
> existing iio_backend_data_sample_trigger() API. I know is abusing a bit and a
> stretch but in the end of the day the whole thing is related with aligning,
> syncing, calibrating the interface for properly sampling data. Even bit-slip
> (while not a traditional external trigger) looks like some kind of self-
> adjusting, data-driven trigger mechanism to establish the correct starting
> point
> for capturing data. So having two new enums like:
> 
> IIO_BACKEND_SAMPLE_TRIGGER_EXT_SIGNAL,
> IIO_BACKEND_SAMPLE_TRIGGER_BIT_SLIP // or maybe a more generic name
> like
> s/BIT_SLIP/INTERNAL
> 
> I do not think the above is that horrible :P... But I would really like to get
> more opinions about this.

There've been some update on the HDL side changing a bit the behavior:
- bitslip_enable is removed, instead the `sync` bit is used which is already part
  of the adc common core.
 SYNC:
       This bit enables capture synchronization. When activated, it initiates
       an HDL process that aligns the sample's most significant bit (MSB) based
       solely on the captured data, without considering the AD4080's CNV signal.
       This bit is self-clearing and should be toggled whenever synchronization
       is needed (e.g., at boot or after updating the sampling rate).

The SELF_SYNC bit was removed.

SYNC_STATUS bit (which is also part of the common core) has the following behavior:
      This bit indicates whether the sample's MSB alignment has been correctly
      performed and the capture is synchronized. If successful, this bit will
      be set to 1.

Re: [PATCH v2 13/13] iio: adc: ad4080: add driver support

[Nuno Sá]

On Tue, 2025-04-22 at 08:12 +0000, Miclaus, Antoniu wrote:
> > > +	int ret;
> > > +
> > > +	guard(mutex)(&st->lock);
> > > +	if (st->num_lanes == 1)
> > > +		ret = regmap_write(st->regmap,
> > > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > > +				   AD4080_RESERVED_CONFIG_A_MSK |
> > > +				   AD4080_INTF_CHK_EN_MSK);
> > > +	else
> > > +		ret = regmap_write(st->regmap,
> > > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > > +				   AD4080_RESERVED_CONFIG_A_MSK |
> > > +				   AD4080_INTF_CHK_EN_MSK |
> > > +				   AD4080_SPI_LVDS_LANES_MSK);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	ret = iio_backend_data_alignment_enable(st->back);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	do {
> > > +		ret = iio_backend_sync_status_get(st->back, &sync_en);
> > > +		if (ret)
> > > +			return ret;
> > > +
> > > +		if (!sync_en)
> > > +			dev_dbg(&st->spi->dev, "Not Locked: Running Bit
> > > Slip\n");
> > > +	} while (--timeout && !sync_en);
> > > +
> > > +	if (timeout) {
> > > +		dev_info(&st->spi->dev, "Success: Pattern correct and
> > > Locked!\n");
> > > +		if (st->num_lanes == 1)
> > > +			return regmap_write(st->regmap,
> > > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > > +
> > 	    AD4080_RESERVED_CONFIG_A_MSK);
> > > +		else
> > > +			return regmap_write(st->regmap,
> > > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > > +
> > 	    AD4080_RESERVED_CONFIG_A_MSK |
> > > +					    AD4080_SPI_LVDS_LANES_MSK);
> > > +	} else {
> > > +		dev_info(&st->spi->dev, "LVDS Sync Timeout.\n");
> > > +		if (st->num_lanes == 1) {
> > > +			ret = regmap_write(st->regmap,
> > > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > > +
> > 	   AD4080_RESERVED_CONFIG_A_MSK);
> > > +			if (ret)
> > > +				return ret;
> > > +		} else {
> > > +			ret = regmap_write(st->regmap,
> > > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > > +
> > 	   AD4080_RESERVED_CONFIG_A_MSK |
> > > +					   AD4080_SPI_LVDS_LANES_MSK);
> > > +			if (ret)
> > > +				return ret;
> > > +		}
> > > +
> > > +		return -ETIMEDOUT;
> > 
> > So, first the things that I don't really get (also the hdl docs need to be
> > improved FWIW):
> > 
> > * It seems that we can have data alignment or data capture synchronization
> > through an internal AMD FPGA technique called bit-slip or an external
> > signal,
> > right? In here, it seems that we only use bit-slip and never disable it. Is
> > that
> > really the goal?
> > 
> > * This whole process seems to be some kind of calibration at the interface
> > level, right?
> > 
> > * What's the point of the conv clock? Is it only used to get the sample
> > rate? If
> > so, the hdl docs are misleading as it seems that it can be used instead of
> > bit-
> > slip for data capture alignment?
> > 
> > Now, speaking about the backend API's, it looks like that
> > iio_backend_self_sync_enable() and iio_backend_data_alignment_enable()
> > could be
> > merged into one API. From what I can tell,
> > iio_backend_data_alignment_enable()
> > just enables the bit-slip process but that likely does nothing unless the
> > SELF_SYNC bit is also set to bit-slip, right? In fact, we could think about
> > a
> > more generic (less flexible though) API like:
> > 
> > * iio_backend_intf_data_align(back, timeout_us), or
> > * iio_backend_intf_calib(back, timeout_us), or
> > * iio_backend_intf_data_capture_sync(back, timeout_us)
> > 
> > On the backend side, you could then enable bit-slip and do the status read
> > (with
> > timeout) and return success or an error code.
> > 
> > The above seems to be pretty much what you're doing but in just one API that
> > makes sense to me.
> > 
> > Of course that the following questions still come to mind:
> > 
> > * Do we need to disable bit-slip after we're done (still fits into the one
> > API
> > model)?
> > * Do we need a meaningful API to change between the syncing/alignment
> > methods?
> > External signal vs bit-slip?
> > 
> > The above is key to better think of an API. Right now it feels that you're
> > just
> > adding an API for every bit you want to control in this process...
> > 
> > If we end up needing more flexibility for this, we can also consider the
> > existing iio_backend_data_sample_trigger() API. I know is abusing a bit and
> > a
> > stretch but in the end of the day the whole thing is related with aligning,
> > syncing, calibrating the interface for properly sampling data. Even bit-slip
> > (while not a traditional external trigger) looks like some kind of self-
> > adjusting, data-driven trigger mechanism to establish the correct starting
> > point
> > for capturing data. So having two new enums like:
> > 
> > IIO_BACKEND_SAMPLE_TRIGGER_EXT_SIGNAL,
> > IIO_BACKEND_SAMPLE_TRIGGER_BIT_SLIP // or maybe a more generic name
> > like
> > s/BIT_SLIP/INTERNAL
> > 
> > I do not think the above is that horrible :P... But I would really like to
> > get
> > more opinions about this.
> 
> There've been some update on the HDL side changing a bit the behavior:
> - bitslip_enable is removed, instead the `sync` bit is used which is already
> part
>   of the adc common core.
>  SYNC:
>        This bit enables capture synchronization. When activated, it initiates
>        an HDL process that aligns the sample's most significant bit (MSB)
> based
>        solely on the captured data, without considering the AD4080's CNV
> signal.
>        This bit is self-clearing and should be toggled whenever
> synchronization
>        is needed (e.g., at boot or after updating the sampling rate).
> 
> The SELF_SYNC bit was removed.
> 
> SYNC_STATUS bit (which is also part of the common core) has the following
> behavior:
>       This bit indicates whether the sample's MSB alignment has been correctly
>       performed and the capture is synchronized. If successful, this bit will
>       be set to 1.

So this looks like it would fir in first approach of just one new backend API
right?

What about the CNV signal? Is that something we can fully control on the
frontend driver (though it also seems that signal is an output of the backend
device)?

Thx!
- Nuno Sá

RE: [PATCH v2 13/13] iio: adc: ad4080: add driver support

[Miclaus, Antoniu]

--
Antoniu Miclăuş

> -----Original Message-----
> From: Nuno Sá <noname.nuno@gmail.com>
> Sent: Tuesday, April 22, 2025 2:39 PM
> To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com>; jic23@kernel.org;
> robh@kernel.org; conor+dt@kernel.org; linux-iio@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2 13/13] iio: adc: ad4080: add driver support
> 
> [External]
> 
> On Tue, 2025-04-22 at 08:12 +0000, Miclaus, Antoniu wrote:
> > > > +	int ret;
> > > > +
> > > > +	guard(mutex)(&st->lock);
> > > > +	if (st->num_lanes == 1)
> > > > +		ret = regmap_write(st->regmap,
> > > > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > > > +				   AD4080_RESERVED_CONFIG_A_MSK |
> > > > +				   AD4080_INTF_CHK_EN_MSK);
> > > > +	else
> > > > +		ret = regmap_write(st->regmap,
> > > > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > > > +				   AD4080_RESERVED_CONFIG_A_MSK |
> > > > +				   AD4080_INTF_CHK_EN_MSK |
> > > > +				   AD4080_SPI_LVDS_LANES_MSK);
> > > > +	if (ret)
> > > > +		return ret;
> > > > +
> > > > +	ret = iio_backend_data_alignment_enable(st->back);
> > > > +	if (ret)
> > > > +		return ret;
> > > > +
> > > > +	do {
> > > > +		ret = iio_backend_sync_status_get(st->back, &sync_en);
> > > > +		if (ret)
> > > > +			return ret;
> > > > +
> > > > +		if (!sync_en)
> > > > +			dev_dbg(&st->spi->dev, "Not Locked: Running Bit
> > > > Slip\n");
> > > > +	} while (--timeout && !sync_en);
> > > > +
> > > > +	if (timeout) {
> > > > +		dev_info(&st->spi->dev, "Success: Pattern correct and
> > > > Locked!\n");
> > > > +		if (st->num_lanes == 1)
> > > > +			return regmap_write(st->regmap,
> > > > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > > > +
> > > 	    AD4080_RESERVED_CONFIG_A_MSK);
> > > > +		else
> > > > +			return regmap_write(st->regmap,
> > > > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > > > +
> > > 	    AD4080_RESERVED_CONFIG_A_MSK |
> > > > +					    AD4080_SPI_LVDS_LANES_MSK);
> > > > +	} else {
> > > > +		dev_info(&st->spi->dev, "LVDS Sync Timeout.\n");
> > > > +		if (st->num_lanes == 1) {
> > > > +			ret = regmap_write(st->regmap,
> > > > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > > > +
> > > 	   AD4080_RESERVED_CONFIG_A_MSK);
> > > > +			if (ret)
> > > > +				return ret;
> > > > +		} else {
> > > > +			ret = regmap_write(st->regmap,
> > > > AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > > > +
> > > 	   AD4080_RESERVED_CONFIG_A_MSK |
> > > > +					   AD4080_SPI_LVDS_LANES_MSK);
> > > > +			if (ret)
> > > > +				return ret;
> > > > +		}
> > > > +
> > > > +		return -ETIMEDOUT;
> > >
> > > So, first the things that I don't really get (also the hdl docs need to be
> > > improved FWIW):
> > >
> > > * It seems that we can have data alignment or data capture
> synchronization
> > > through an internal AMD FPGA technique called bit-slip or an external
> > > signal,
> > > right? In here, it seems that we only use bit-slip and never disable it. Is
> > > that
> > > really the goal?
> > >
> > > * This whole process seems to be some kind of calibration at the interface
> > > level, right?
> > >
> > > * What's the point of the conv clock? Is it only used to get the sample
> > > rate? If
> > > so, the hdl docs are misleading as it seems that it can be used instead of
> > > bit-
> > > slip for data capture alignment?
> > >
> > > Now, speaking about the backend API's, it looks like that
> > > iio_backend_self_sync_enable() and iio_backend_data_alignment_enable()
> > > could be
> > > merged into one API. From what I can tell,
> > > iio_backend_data_alignment_enable()
> > > just enables the bit-slip process but that likely does nothing unless the
> > > SELF_SYNC bit is also set to bit-slip, right? In fact, we could think about
> > > a
> > > more generic (less flexible though) API like:
> > >
> > > * iio_backend_intf_data_align(back, timeout_us), or
> > > * iio_backend_intf_calib(back, timeout_us), or
> > > * iio_backend_intf_data_capture_sync(back, timeout_us)
> > >
> > > On the backend side, you could then enable bit-slip and do the status read
> > > (with
> > > timeout) and return success or an error code.
> > >
> > > The above seems to be pretty much what you're doing but in just one API
> that
> > > makes sense to me.
> > >
> > > Of course that the following questions still come to mind:
> > >
> > > * Do we need to disable bit-slip after we're done (still fits into the one
> > > API
> > > model)?
> > > * Do we need a meaningful API to change between the syncing/alignment
> > > methods?
> > > External signal vs bit-slip?
> > >
> > > The above is key to better think of an API. Right now it feels that you're
> > > just
> > > adding an API for every bit you want to control in this process...
> > >
> > > If we end up needing more flexibility for this, we can also consider the
> > > existing iio_backend_data_sample_trigger() API. I know is abusing a bit and
> > > a
> > > stretch but in the end of the day the whole thing is related with aligning,
> > > syncing, calibrating the interface for properly sampling data. Even bit-slip
> > > (while not a traditional external trigger) looks like some kind of self-
> > > adjusting, data-driven trigger mechanism to establish the correct starting
> > > point
> > > for capturing data. So having two new enums like:
> > >
> > > IIO_BACKEND_SAMPLE_TRIGGER_EXT_SIGNAL,
> > > IIO_BACKEND_SAMPLE_TRIGGER_BIT_SLIP // or maybe a more generic
> name
> > > like
> > > s/BIT_SLIP/INTERNAL
> > >
> > > I do not think the above is that horrible :P... But I would really like to
> > > get
> > > more opinions about this.
> >
> > There've been some update on the HDL side changing a bit the behavior:
> > - bitslip_enable is removed, instead the `sync` bit is used which is already
> > part
> >   of the adc common core.
> >  SYNC:
> >        This bit enables capture synchronization. When activated, it initiates
> >        an HDL process that aligns the sample's most significant bit (MSB)
> > based
> >        solely on the captured data, without considering the AD4080's CNV
> > signal.
> >        This bit is self-clearing and should be toggled whenever
> > synchronization
> >        is needed (e.g., at boot or after updating the sampling rate).
> >
> > The SELF_SYNC bit was removed.
> >
> > SYNC_STATUS bit (which is also part of the common core) has the following
> > behavior:
> >       This bit indicates whether the sample's MSB alignment has been correctly
> >       performed and the capture is synchronized. If successful, this bit will
> >       be set to 1.
> 
> So this looks like it would fir in first approach of just one new backend API
> right?
In this particular case, yes, one backend would fit for the sync process. But
taking into account that these two features are part also from the common core
of the AXI ADC, in other cases they might be used separately.
> 
> What about the CNV signal? Is that something we can fully control on the
> frontend driver (though it also seems that signal is an output of the backend
> device)?
The CNV signal is mainly used for sampling (an input pin according to the datasheet - 
conversion is initiated on the rising edge of the convert signal).
We use it only for determining the sampling frequency.
> 
> Thx!
> - Nuno Sá