[PATCH 00/14] Add support for AD4080 ADC

[PATCH 00/14] 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. Simplification of the
input anti-alias filter design can be accomplished by applying over-
sampling along with the integrated digital filtering and decimation to
reduce noise and lower the output data rate for applications that do
not require the lowest latency of the AD4080.

Antoniu Miclaus (14):
  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
  Documentation: ABI: testing: ad4080 docs

 .../ABI/testing/sysfs-bus-iio-adc-ad4080      |  55 ++
 .../bindings/iio/adc/adi,ad4080.yaml          |  92 +++
 .../bindings/iio/adc/adi,axi-adc.yaml         |   2 +
 drivers/iio/adc/Kconfig                       |  15 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/ad4080.c                      | 768 ++++++++++++++++++
 drivers/iio/adc/adi-axi-adc.c                 | 115 +++
 drivers/iio/industrialio-backend.c            | 113 +++
 include/linux/iio/backend.h                   |  24 +
 9 files changed, 1185 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad4080
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4080.yaml
 create mode 100644 drivers/iio/adc/ad4080.c

-- 
2.48.1

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

[Antoniu Miclaus]

Add backend support for filter enable/disable.
This setting can be adjusted within the IP cores interfacing devices.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
 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.48.1

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

[Jonathan Cameron]

On Thu, 20 Feb 2025 15:54:11 +0200
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> Add backend support for filter enable/disable.
> This setting can be adjusted within the IP cores interfacing devices.

Hi Antoniu,

Please add some details on what this filtering actually is.
There are lots of possibilities. I'm not sure why you'd have
just an enable on the backend rather than a richer set of controls?

Thanks,

Jonathan


> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
>  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);

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

[Antoniu Miclaus]

Add backend support for enabling/disabling the sync process.
This setting can be adjusted within the IP cores interfacing devices.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
 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..b01db8a65b1c 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_bitslip_enable - Enable the sync process.
+ * @back: Backend device
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_bitslip_enable(struct iio_backend *back)
+{
+	return iio_backend_op_call(back, bitslip_enable);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_bitslip_enable, "IIO_BACKEND");
+
+/**
+ * iio_backend_bitslip_disable - Disable the sync process.
+ * @back: Backend device
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_bitslip_disable(struct iio_backend *back)
+{
+	return iio_backend_op_call(back, bitslip_disable);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_bitslip_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..c7b4631b524c 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.
+ * @bitslip_enable: Enable sync process.
+ * @bitslip_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 (*bitslip_enable)(struct iio_backend *back);
+	int (*bitslip_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_bitslip_enable(struct iio_backend *back);
+int iio_backend_bitslip_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.48.1

Re: [PATCH 02/14] iio: backend: add support for sync process

[Jonathan Cameron]

On Thu, 20 Feb 2025 15:54:12 +0200
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> Add backend support for enabling/disabling the sync process.
> This setting can be adjusted within the IP cores interfacing devices.

More details needed here as well.  Not obvious what sync we are talking
about and what bitslip might be? I'd guess it's a skid control on the serial
interface but good to have something describing it here.

[PATCH 03/14] 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>
---
 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 b01db8a65b1c..a81780f569e3 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -830,6 +830,36 @@ int iio_backend_bitslip_disable(struct iio_backend *back)
 }
 EXPORT_SYMBOL_NS_GPL(iio_backend_bitslip_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 c7b4631b524c..8e663c838974 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.
  * @bitslip_enable: Enable sync process.
  * @bitslip_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 (*bitslip_enable)(struct iio_backend *back);
 	int (*bitslip_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_bitslip_enable(struct iio_backend *back);
 int iio_backend_bitslip_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.48.1

[PATCH 04/14] 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>
---
 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 a81780f569e3..99c46b79e373 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 8e663c838974..38322e808ee2 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -106,6 +106,7 @@ enum iio_backend_interface_type {
  * @bitslip_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 (*bitslip_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_bitslip_enable(struct iio_backend *back);
 int iio_backend_bitslip_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.48.1

[PATCH 05/14] 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>
---
 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 99c46b79e373..64ac3c6d9eb4 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 38322e808ee2..67e062cabbea 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_bitslip_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.48.1

Re: [PATCH 05/14] iio: backend: add support for number of lanes

[David Lechner]

On 2/20/25 7:54 AM, Antoniu Miclaus wrote:
> Add iio backend support for number of lanes to be enabled.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
This is why I was pushing for a similar function to be an iio_backend
function in [1]. :-)

[1]: https://lore.kernel.org/linux-iio/94efa413-5fa9-4014-86c2-331442e9d42e@baylibre.com/

Not sure if that changes what we want to do here, but just pointing it
out as a similar case to help us decide what the most useful generic
function would be for this.

Re: [PATCH 05/14] iio: backend: add support for number of lanes

[Nuno Sá]

On Thu, 2025-02-20 at 14:20 -0600, David Lechner wrote:
> On 2/20/25 7:54 AM, Antoniu Miclaus wrote:
> > Add iio backend support for number of lanes to be enabled.
> > 
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > ---
> This is why I was pushing for a similar function to be an iio_backend
> function in [1]. :-)
> 
> [1]:
> https://lore.kernel.org/linux-iio/94efa413-5fa9-4014-86c2-331442e9d42e@baylibre.com/
> 
> Not sure if that changes what we want to do here, but just pointing it
> out as a similar case to help us decide what the most useful generic
> function would be for this.
> 
> 

Hmm, yeah, I guess we could have them both in this interface or the API
suggested in discussion you linked (is octal spi a thing?!) even though it gets
a bit confusing in the AD3552R case where we have the other interface for bus
related things.

- Nuno Sá

[PATCH 06/14] 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.

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

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
 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.48.1

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1645 bytes --]

On Thu, Feb 20, 2025 at 03:54:16PM +0200, 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.

Very similar isn't very specific. Is it functionally different and needs
to be an enum member, or a fallback to the generic compatible is
sufficient for a driver to function?

> 
> Wildcard naming is used to match the naming of the published
> firmware.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
>  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.48.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 07/14] 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>
---
 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.48.1

[PATCH 08/14] 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>
---
 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..c7811667f853 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,
+	.bitslip_enable = axi_adc_ad408x_bitslip_enable,
+	.bitslip_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.48.1

[PATCH 09/14] 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>
---
 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 c7811667f853..7f6dc0ff7c54 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,
 	.bitslip_enable = axi_adc_ad408x_bitslip_enable,
 	.bitslip_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.48.1

[PATCH 10/14] 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>
---
 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 7f6dc0ff7c54..b5c2ba3a44c8 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 = {
 	.bitslip_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.48.1

[PATCH 11/14] 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>
---
 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 b5c2ba3a44c8..c39f9ac7797f 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.48.1

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

[Antoniu Miclaus]

Add devicetree bindings for ad4080 family.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
 .../bindings/iio/adc/adi,ad4080.yaml          | 92 +++++++++++++++++++
 1 file changed, 92 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..e0ea712b8457
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4080.yaml
@@ -0,0 +1,92 @@
+# 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:
+    maximum: 50000000
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: adc-clk
+
+  vdd33-supply: true
+
+  vdd11-supply: true
+
+  vddldo-supply: true
+
+  iovdd-supply: true
+
+  vrefin-supply: true
+
+  adi,num-lanes:
+    description:
+      Nmber 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
+  - vdd11-supply
+  - vddldo-supply
+  - iovdd-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>;
+          vdd11-supply = <&vdd11>;
+          vddldo-supply = <&vddldo>;
+          iovdd-supply = <&iovdd>;
+          vrefin-supply = <&vrefin>;
+          clocks = <&adc_clk>;
+          clock-names = "adc-clk";
+        };
+    };
+...
-- 
2.48.1

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

[David Lechner]

On 2/20/25 7:54 AM, Antoniu Miclaus wrote:
> Add devicetree bindings for ad4080 family.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
>  .../bindings/iio/adc/adi,ad4080.yaml          | 92 +++++++++++++++++++
>  1 file changed, 92 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..e0ea712b8457
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4080.yaml
> @@ -0,0 +1,92 @@
> +# 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:
> +    maximum: 50000000

Since there are potentially two independent SPI busses on this chip
(one for configuration, one for data) it might be a good idea to put
in a description somewhere that these SPI properties are for the
configuration SPI bus.

> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: adc-clk

Clocks don't need a name if there is only one clock.

But the description of clocks should say which pin you mean.
Is the the CLK+/- pins or the CNV+/- pins?

> +
> +  vdd33-supply: true
> +
> +  vdd11-supply: true
> +
> +  vddldo-supply: true
> +
> +  iovdd-supply: true
> +
> +  vrefin-supply: true

I would expect we need a vendor boolean property to say if the
DCO+/- pins are wired or not for the echoed clock.

And what does the CNV trigger get wired to? We probably need a
vendor boolean property to say if it is wired to something CMOS
or LVDS. Plus maybe a pwms property or whatever makes sense for
whatever kind of signal generator it is connected to.

> +
> +  adi,num-lanes:
> +    description:
> +      Nmber of lanes on which the data is sent on the output (DA, DB pins).

s/Nmber/Number/

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 2]
> +    default: 1
> +

Also, don't we need io-backends for the data interface?

We can also add gpio-controller and #gpio-cells since this
chips provides GPIOs.

> +required:
> +  - compatible
> +  - reg
> +  - clocks

Assuming clocks is CLK+/- pins, this should be optional since
the pins are used for a different function in SPI mode.

> +  - clock-names
> +  - vdd33-supply
> +  - vdd11-supply
> +  - vddldo-supply
> +  - iovdd-supply
> +  - vrefin-supply

The datasheet says things like, "If VDDLDO is connected to a
voltage source, neither VDD11 nor IOVDD should be connected to
any external voltage source.". So making all supplies required
doesn't seem correct.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        adc@0 {
> +          compatible = "adi,ad4080";
> +          reg = <0>;
> +          spi-max-frequency = <10000000>;
> +          vdd33-supply = <&vdd33>;
> +          vdd11-supply = <&vdd11>;
> +          vddldo-supply = <&vddldo>;
> +          iovdd-supply = <&iovdd>;
> +          vrefin-supply = <&vrefin>;
> +          clocks = <&adc_clk>;
> +          clock-names = "adc-clk";
> +        };
> +    };
> +...

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 376 bytes --]

On Thu, Feb 20, 2025 at 01:30:11PM -0600, David Lechner wrote:
> On 2/20/25 7:54 AM, Antoniu Miclaus wrote:

> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    items:
> > +      - const: adc-clk
> 
> Clocks don't need a name if there is only one clock.

And if they do need a name, putting "clk" in the name of a clock is
pointless!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 13/14] 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>
---
 drivers/iio/adc/Kconfig  |  15 +
 drivers/iio/adc/Makefile |   1 +
 drivers/iio/adc/ad4080.c | 768 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 784 insertions(+)
 create mode 100644 drivers/iio/adc/ad4080.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 27413516216c..b198a93c10b7 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -47,6 +47,21 @@ 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).
+
+	  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..71c443965e10
--- /dev/null
+++ b/drivers/iio/adc/ad4080.c
@@ -0,0 +1,768 @@
+// 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_sel {
+	FILTER_DISABLE,
+	SINC_1,
+	SINC_5,
+	SINC_5_COMP
+};
+
+enum ad4080_dec_rate {
+	DEC_2,
+	DEC_4,
+	DEC_8,
+	DEC_16,
+	DEC_32,
+	DEC_64,
+	DEC_128,
+	DEC_256,
+	DEC_512,
+	DEC_1024
+};
+
+static const unsigned int ad4080_scale_table[][2] = {
+	{6000, 0},
+};
+
+static const int ad4080_dec_rate_value[] = {
+	[DEC_2]    = 2,
+	[DEC_4]    = 4,
+	[DEC_8]    = 8,
+	[DEC_16]   = 16,
+	[DEC_32]   = 32,
+	[DEC_64]   = 64,
+	[DEC_128]  = 128,
+	[DEC_256]  = 256,
+	[DEC_512]  = 512,
+	[DEC_1024] = 1024,
+};
+
+static const char *const ad4080_filter_mode_iio_enum[] = {
+	[FILTER_DISABLE]   = "disabled",
+	[SINC_1]           = "sinc1",
+	[SINC_5]           = "sinc5",
+	[SINC_5_COMP]      = "sinc5_plus_compensation",
+};
+
+static const char *const ad4080_dec_rate_iio_enum[] = {
+	[DEC_2]    = "2",
+	[DEC_4]    = "4",
+	[DEC_8]    = "8",
+	[DEC_16]   = "16",
+	[DEC_32]   = "32",
+	[DEC_64]   = "64",
+	[DEC_128]  = "128",
+	[DEC_256]  = "256",
+	[DEC_512]  = "512",
+	[DEC_1024] = "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;
+	enum ad4080_dec_rate		dec_rate;
+	enum ad4080_filter_sel		filter_mode;
+	bool				filter_enabled;
+};
+
+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 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_mode == SINC_5_COMP)
+			dec_rate = ad4080_dec_rate_value[st->dec_rate] * 2;
+		else
+			dec_rate = ad4080_dec_rate_value[st->dec_rate];
+		if (st->filter_enabled)
+			*val = DIV_ROUND_CLOSEST(clk_get_rate(st->clk), dec_rate);
+		else
+			*val = clk_get_rate(st->clk);
+		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)
+{
+	struct ad4080_state *st = iio_priv(indio_dev);
+	unsigned long s_clk;
+	int dec_rate = 1;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		return -EINVAL;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		s_clk = clk_round_rate(st->clk, val);
+
+		if (st->filter_enabled) {
+			if (st->filter_mode == SINC_5_COMP)
+				dec_rate = ad4080_dec_rate_value[st->dec_rate] * 2;
+			else
+				dec_rate = ad4080_dec_rate_value[st->dec_rate];
+		}
+
+		s_clk *= dec_rate;
+
+		if (s_clk < AD4080_MIN_SAMP_FREQ)
+			s_clk = AD4080_MIN_SAMP_FREQ;
+		if (s_clk > AD4080_MAX_SAMP_FREQ)
+			s_clk = AD4080_MAX_SAMP_FREQ;
+
+		return clk_set_rate(st->clk, s_clk);
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static ssize_t ad4080_lvds_sync_write(struct iio_dev *indio_dev,
+				      uintptr_t private,
+				      const struct iio_chan_spec *chan,
+				      const char *buf, size_t len)
+{
+	struct ad4080_state *st = iio_priv(indio_dev);
+	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_self_sync_enable(st->back);
+	if (ret)
+		return ret;
+
+	ret = iio_backend_num_lanes_set(st->back, st->num_lanes);
+	if (ret)
+		return ret;
+
+	ret = iio_backend_bitslip_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_info(&st->spi->dev, "Not Locked: Running Bit Slip\n");
+		else
+			break;
+	} while (--timeout);
+
+	if (timeout) {
+		dev_info(&st->spi->dev, "Success: Pattern correct and Locked!\n");
+		if (st->num_lanes == 1)
+			ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
+					   AD4080_RESERVED_CONFIG_A_MSK);
+		else
+			ret = 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);
+		else
+			ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
+					   AD4080_RESERVED_CONFIG_A_MSK |
+					   AD4080_SPI_LVDS_LANES_MSK);
+		ret = -ETIME;
+	}
+
+	return ret ? ret : len;
+}
+
+static ssize_t ad4080_lvds_cnv_en_read(struct iio_dev *indio_dev,
+				       uintptr_t private,
+				       const struct iio_chan_spec *chan,
+				       char *buf)
+{
+	struct ad4080_state *st = iio_priv(indio_dev);
+	unsigned int data;
+	int ret;
+
+	ret = regmap_read(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_B, &data);
+	if (ret)
+		return ret;
+
+	data = FIELD_GET(AD4080_LVDS_CNV_EN_MSK, data);
+
+	return ret ? ret : sysfs_emit(buf, "%u\n", data);
+}
+
+static ssize_t ad4080_lvds_cnv_en_write(struct iio_dev *indio_dev,
+					uintptr_t private,
+					const struct iio_chan_spec *chan,
+					const char *buf, size_t len)
+{
+	struct ad4080_state *st = iio_priv(indio_dev);
+	bool en;
+	unsigned int data;
+	int ret;
+
+	ret = kstrtobool(buf, &en);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&st->lock);
+
+	ret = regmap_read(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_B, &data);
+	if (ret)
+		return ret;
+
+	data &= ~AD4080_LVDS_CNV_EN_MSK;
+	data |= en;
+
+	ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_B, data);
+
+	return ret ? ret : len;
+}
+
+static ssize_t ad4080_get_filter_mode(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;
+
+	data = FIELD_GET(GENMASK(1, 0), data);
+
+	return data;
+}
+
+static int ad4080_set_filter_mode(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 >= DEC_512)
+		return -EINVAL;
+
+	guard(mutex)(&st->lock);
+	if (mode) {
+		ret = iio_backend_filter_enable(st->back);
+		if (ret)
+			return ret;
+
+		st->filter_enabled = true;
+	} else {
+		ret = iio_backend_filter_enable(st->back);
+		if (ret)
+			return ret;
+
+		st->filter_enabled = false;
+	}
+	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_mode = mode;
+
+	return ret;
+}
+
+static ssize_t 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;
+
+	data = FIELD_GET(AD4080_SINC_DEC_RATE_MSK, data);
+
+	return data;
+}
+
+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_mode >= SINC_5 && mode >= DEC_512)
+		return -EINVAL;
+
+	guard(mutex)(&st->lock);
+	ret = regmap_read(st->regmap, AD4080_REG_FILTER_CONFIG, &reg_val);
+	if (ret)
+		return ret;
+
+	data = (mode << 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 const struct iio_info ad4080_iio_info = {
+	.debugfs_reg_access = ad4080_reg_access,
+	.read_raw = ad4080_read_raw,
+	.write_raw = ad4080_write_raw,
+};
+
+static const struct iio_enum ad4080_dec_rate_enum = {
+	.items = ad4080_dec_rate_iio_enum,
+	.num_items = ARRAY_SIZE(ad4080_dec_rate_iio_enum),
+	.set = ad4080_set_dec_rate,
+	.get = ad4080_get_dec_rate,
+};
+
+static const struct iio_enum ad4080_filter_mode_enum = {
+	.items = ad4080_filter_mode_iio_enum,
+	.num_items = ARRAY_SIZE(ad4080_filter_mode_iio_enum),
+	.set = ad4080_set_filter_mode,
+	.get = ad4080_get_filter_mode,
+};
+
+static struct iio_chan_spec_ext_info ad4080_ext_info[] = {
+	{
+	 .name = "lvds_sync",
+	 .write = ad4080_lvds_sync_write,
+	 .shared = IIO_SHARED_BY_ALL,
+	 },
+	{
+	 .name = "lvds_cnv",
+	 .read = ad4080_lvds_cnv_en_read,
+	 .write = ad4080_lvds_cnv_en_write,
+	 .shared = IIO_SHARED_BY_ALL,
+	},
+	IIO_ENUM("sinc_dec_rate",
+		 IIO_SHARED_BY_ALL,
+		 &ad4080_dec_rate_enum),
+	IIO_ENUM_AVAILABLE("sinc_dec_rate",
+			   IIO_SHARED_BY_ALL,
+			   &ad4080_dec_rate_enum),
+	IIO_ENUM("filter_sel",
+		 IIO_SHARED_BY_ALL,
+		 &ad4080_filter_mode_enum),
+	IIO_ENUM_AVAILABLE("filter_sel",
+			   IIO_SHARED_BY_ALL,
+			   &ad4080_filter_mode_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),	\
+	  .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_post_setup(struct iio_dev *indio_dev)
+{
+	struct ad4080_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = iio_backend_num_lanes_set(st->back, st->num_lanes);
+	if (ret)
+		return ret;
+
+	return iio_backend_self_sync_enable(st->back);
+}
+
+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) {
+		dev_err(&st->spi->dev, "Unrecognized CHIP_ID 0x%X\n", id);
+		return -EINVAL;
+	}
+
+	if (st->num_lanes)
+		regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_B,
+			     FIELD_PREP(AD4080_LVDS_CNV_CLK_CNT_MSK, 7) |
+			     AD4080_LVDS_CNV_EN_MSK);
+	else
+		regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_B,
+			     AD4080_LVDS_CNV_EN_MSK);
+
+	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;
+
+	return ad4080_post_setup(indio_dev);
+}
+
+static int ad4080_properties_parse(struct ad4080_state *st)
+{
+	unsigned int val;
+	int ret;
+
+	ret = device_property_read_u32(&st->spi->dev, "adi,num_lanes", &val);
+	if (!ret)
+		st->num_lanes = val;
+	else
+		st->num_lanes = 1;
+
+	return 0;
+}
+
+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;
+
+	ret = ad4080_properties_parse(st);
+	if (ret)
+		return ret;
+
+	st->clk = devm_clk_get_enabled(&spi->dev, "adc-clk");
+	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.48.1

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

[Nuno Sá]

Hi Antoniu,

A first, not in depth, review from my side...

On Thu, 2025-02-20 at 15:54 +0200, 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>
> ---
>  drivers/iio/adc/Kconfig  |  15 +
>  drivers/iio/adc/Makefile |   1 +
>  drivers/iio/adc/ad4080.c | 768 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 784 insertions(+)
>  create mode 100644 drivers/iio/adc/ad4080.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 27413516216c..b198a93c10b7 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -47,6 +47,21 @@ 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).
> +
> +	  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..71c443965e10
> --- /dev/null
> +++ b/drivers/iio/adc/ad4080.c
> @@ -0,0 +1,768 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Analog Devices AD4080 SPI ADC driver
> + *
> + * Copyright 2025 Analog Devices Inc.
> + */

...

> 
> +static int ad4080_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int val, int val2, long mask)
> +{
> +	struct ad4080_state *st = iio_priv(indio_dev);
> +	unsigned long s_clk;
> +	int dec_rate = 1;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		return -EINVAL;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		s_clk = clk_round_rate(st->clk, val);
> +
> +		if (st->filter_enabled) {
> +			if (st->filter_mode == SINC_5_COMP)
> +				dec_rate = ad4080_dec_rate_value[st-
> >dec_rate] * 2;
> +			else
> +				dec_rate = ad4080_dec_rate_value[st-
> >dec_rate];
> +		}
> +
> +		s_clk *= dec_rate;
> +
> +		if (s_clk < AD4080_MIN_SAMP_FREQ)
> +			s_clk = AD4080_MIN_SAMP_FREQ;
> +		if (s_clk > AD4080_MAX_SAMP_FREQ)
> +			s_clk = AD4080_MAX_SAMP_FREQ;
> +
> +		return clk_set_rate(st->clk, s_clk);

It seems to me that we could skip the dec_rate attribute. Given the available
values we have, can't we compute the available sampling frequencies during
.probe() after getting our ref clock? 
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static ssize_t ad4080_lvds_sync_write(struct iio_dev *indio_dev,
> +				      uintptr_t private,
> +				      const struct iio_chan_spec *chan,
> +				      const char *buf, size_t len)
> +{
> +	struct ad4080_state *st = iio_priv(indio_dev);
> +	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_self_sync_enable(st->back);
> +	if (ret)
> +		return ret;
> +
> +	ret = iio_backend_num_lanes_set(st->back, st->num_lanes);
> +	if (ret)
> +		return ret;

The number of lanes is a DT property so it seems to me that we can do the above
only once during probe. Otherwise I would expect a comment on why we need to do
this again...


> +
> +	ret = iio_backend_bitslip_enable(st->back);
> +	if (ret)
> +		return ret;

AFAIU, bit slip seems to be something very specific to AMD/Xilinx isn't it? It
also looks like this is mostly about adjusting the alignment of incoming data at
the interface level for data integrity. 

So we could maybe rename the API for something more generic like
iio_backend_data_alignment_enable()? I'm open for suggestions here :)

> +
> +	do {
> +		ret = iio_backend_sync_status_get(st->back, &sync_en);
> +		if (ret)
> +			return ret;

Since this is about waiting on some alignment/synchronization process to
complete, we could also think on a better name and API for this. I think we
could pass a timeout parameter and do the waiting (with some regmap polling) in
the backend returning 0 on success. Thoughts?

> +
> +		if (!sync_en)
> +			dev_info(&st->spi->dev, "Not Locked: Running Bit
> Slip\n");
> +		else
> +			break;
> +	} while (--timeout);
> 

Now comes my question about this process. This looks like some kind of interface
calibration or am I completely wrong? I wonder if this is something that really
needs to be a control that userspace can randomly use? Or is this something we
should only do once during probe? Or every time before buffering? Or everytime
we change the sampling frequency? 

Gut feeling is that we should do this after changing the sampling frequency but
I can be totally wrong.

Other thing that I'm confused about is that during probe we do
iio_backend_self_sync_enable() and we can start buffering without running the
lvds_sync code. My point is that after running lvds_sync we are left with  
iio_backend_bitslip_enable() so I'm wondering how this all should work? Should
bitslip be always enabled or only for this process?

The Docs are also a bit confusing:

BITSLIP_ENABLE - Enables the sync process.
SELF_SYNC - Controls if the data capture synchronization is done through CNV
signal or bit-slip.

From what I can tell we always set SELF_SYNC to 1 which is not clear what it is?
I guess bit slip?

> +	if (timeout) {
> +		dev_info(&st->spi->dev, "Success: Pattern correct and
> Locked!\n");
> +		if (st->num_lanes == 1)
> +			ret = regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					   AD4080_RESERVED_CONFIG_A_MSK);
> +		else
> +			ret = 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);
> +		else
> +			ret = regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					   AD4080_RESERVED_CONFIG_A_MSK |
> +					   AD4080_SPI_LVDS_LANES_MSK);
> +		ret = -ETIME;

ETIMEDOUT (might have a typo)

> +	}
> +
> +	return ret ? ret : len;
> +}
> +

...

> 
> +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;
> +
> +	ret = ad4080_properties_parse(st);
> +	if (ret)
> +		return ret;
> +
> +	st->clk = devm_clk_get_enabled(&spi->dev, "adc-clk");
> +	if (IS_ERR(st->clk))
> +		return PTR_ERR(st->clk);

Is this the CNV pin? If so, cnv-clk could be a better name or since we only have
one clock, we don't really need a name...

- Nuno Sá
> 

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

[Jonathan Cameron]

On Thu, 20 Feb 2025 15:54:23 +0200
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>
Hi Antoniu,

Some fairly superficial comments inline.  The bigger questions have
been raised by others.

Jonathan

> ---
>  drivers/iio/adc/Kconfig  |  15 +
>  drivers/iio/adc/Makefile |   1 +
>  drivers/iio/adc/ad4080.c | 768 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 784 insertions(+)
>  create mode 100644 drivers/iio/adc/ad4080.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 27413516216c..b198a93c10b7 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -47,6 +47,21 @@ 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).

Good to give some clue on what sort of bus / backend this connects to in this
description.

> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called ad4080.
> +

One blank line only.

> +
>  config AD4130
>  	tristate "Analog Device AD4130 ADC Driver"
>  	depends on SPI


> diff --git a/drivers/iio/adc/ad4080.c b/drivers/iio/adc/ad4080.c
> new file mode 100644
> index 000000000000..71c443965e10
> --- /dev/null
> +++ b/drivers/iio/adc/ad4080.c
> @@ -0,0 +1,768 @@

> +
> +/** Miscellaneous Definitions */

Not kernel-doc I think, so /* only

> +#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)
Makes little sense as a define and isn't used.


> +#define AD4080_CHIP_ID				GENMASK(2, 0)

> +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.

Check for those if you can use the regmap provided update functions.

> +	 */
> +	struct mutex			lock;
> +	unsigned int			num_lanes;
> +	enum ad4080_dec_rate		dec_rate;
> +	enum ad4080_filter_sel		filter_mode;
> +	bool				filter_enabled;
> +};

> +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_mode == SINC_5_COMP)
> +			dec_rate = ad4080_dec_rate_value[st->dec_rate] * 2;
> +		else
> +			dec_rate = ad4080_dec_rate_value[st->dec_rate];
> +		if (st->filter_enabled)
> +			*val = DIV_ROUND_CLOSEST(clk_get_rate(st->clk), dec_rate);
> +		else
> +			*val = clk_get_rate(st->clk);
> +		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)
> +{
> +	struct ad4080_state *st = iio_priv(indio_dev);
> +	unsigned long s_clk;
> +	int dec_rate = 1;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		return -EINVAL;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		s_clk = clk_round_rate(st->clk, val);
> +
> +		if (st->filter_enabled) {
> +			if (st->filter_mode == SINC_5_COMP)
> +				dec_rate = ad4080_dec_rate_value[st->dec_rate] * 2;
> +			else
> +				dec_rate = ad4080_dec_rate_value[st->dec_rate];
> +		}
> +
> +		s_clk *= dec_rate;
> +
> +		if (s_clk < AD4080_MIN_SAMP_FREQ)
> +			s_clk = AD4080_MIN_SAMP_FREQ;
> +		if (s_clk > AD4080_MAX_SAMP_FREQ)
> +			s_clk = AD4080_MAX_SAMP_FREQ;
> +
> +		return clk_set_rate(st->clk, s_clk);
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
can't get here so drop this.

> +}
> +
> +static ssize_t ad4080_lvds_sync_write(struct iio_dev *indio_dev,
> +				      uintptr_t private,
> +				      const struct iio_chan_spec *chan,
> +				      const char *buf, size_t len)
> +{
> +	struct ad4080_state *st = iio_priv(indio_dev);
> +	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_self_sync_enable(st->back);
> +	if (ret)
> +		return ret;
> +
> +	ret = iio_backend_num_lanes_set(st->back, st->num_lanes);
> +	if (ret)
> +		return ret;
> +
> +	ret = iio_backend_bitslip_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_info(&st->spi->dev, "Not Locked: Running Bit Slip\n");

too noisy. dev_dbg() for something like this.


> +		else
> +			break;
> +	} while (--timeout);
whilst (--timeout && !sync_en) maybe?

> +
> +	if (timeout) {
> +		dev_info(&st->spi->dev, "Success: Pattern correct and Locked!\n");
> +		if (st->num_lanes == 1)
> +			ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					   AD4080_RESERVED_CONFIG_A_MSK);
> +		else
> +			ret = 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);
> +		else
> +			ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					   AD4080_RESERVED_CONFIG_A_MSK |
> +					   AD4080_SPI_LVDS_LANES_MSK);
> +		ret = -ETIME;
return -ETIME;

However why are you overwriting the values you just put in ret?  Don't store
them into ret if you don't want them.  I'd do this case first before the good one
and return early.


> +	}
> +
> +	return ret ? ret : len;
> +}
.
> +
> +static ssize_t ad4080_lvds_cnv_en_write(struct iio_dev *indio_dev,
> +					uintptr_t private,
> +					const struct iio_chan_spec *chan,
> +					const char *buf, size_t len)
> +{
> +	struct ad4080_state *st = iio_priv(indio_dev);
> +	bool en;
> +	unsigned int data;
> +	int ret;
> +
> +	ret = kstrtobool(buf, &en);
> +	if (ret)
> +		return ret;
> +
> +	guard(mutex)(&st->lock);
> +
> +	ret = regmap_read(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_B, &data);
> +	if (ret)
> +		return ret;
> +
> +	data &= ~AD4080_LVDS_CNV_EN_MSK;
> +	data |= en;
> +
> +	ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_B, data);
regmap_update_bits()? and consider if the regmap lock is enough.

> +
> +	return ret ? ret : len;
> +}
> +
> +static ssize_t ad4080_get_filter_mode(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;
> +
> +	data = FIELD_GET(GENMASK(1, 0), data);
That GENMASK should probably be a define for the field in the register.

> +
> +	return data;
return FIELD_GET() 
In general not good to use same local variable for the register contents
and the field.  It doesn't help readability. Here you don't need a second
one but if you do, give it a name reflecting what it is.

> +}
> +
> +static int ad4080_set_filter_mode(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 >= DEC_512)
> +		return -EINVAL;
> +
> +	guard(mutex)(&st->lock);
> +	if (mode) {
> +		ret = iio_backend_filter_enable(st->back);
> +		if (ret)
> +			return ret;
> +
> +		st->filter_enabled = true;

st->filter_enabled = mode; and drop it out of the conditional part.

> +	} else {
> +		ret = iio_backend_filter_enable(st->back);

Enable in both paths?

> +		if (ret)
> +			return ret;
> +
> +		st->filter_enabled = false;
> +	}
> +	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);
Excess brackets on first line.
> +
> +	ret = regmap_write(st->regmap, AD4080_REG_FILTER_CONFIG, data);
> +	if (ret)
> +		return ret;
> +
> +	st->filter_mode = mode;
> +
> +	return ret;
> +}
> +
> +static ssize_t 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;
> +
> +	data = FIELD_GET(AD4080_SINC_DEC_RATE_MSK, data);
> +
> +	return data;
return FEILD_GET()

> +}
> +
> +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_mode >= SINC_5 && mode >= DEC_512)
> +		return -EINVAL;
> +
> +	guard(mutex)(&st->lock);
> +	ret = regmap_read(st->regmap, AD4080_REG_FILTER_CONFIG, &reg_val);

If it's a filter thing I'm guessing it's averaging multiple samples? If so
that's oversampling as David pointed out - so standard ABI.
Slight complexity is sampling frequency is after oversampling is applied
so they end up coupled.


> +	if (ret)
> +		return ret;
> +
> +	data = (mode << 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 struct iio_chan_spec_ext_info ad4080_ext_info[] = {
> +	{
> +	 .name = "lvds_sync",
1 tab further in than the {
> +	 .write = ad4080_lvds_sync_write,
> +	 .shared = IIO_SHARED_BY_ALL,
> +	 },
> +	{
> +	 .name = "lvds_cnv",
> +	 .read = ad4080_lvds_cnv_en_read,
> +	 .write = ad4080_lvds_cnv_en_write,
> +	 .shared = IIO_SHARED_BY_ALL,
> +	},
> +	IIO_ENUM("sinc_dec_rate",
> +		 IIO_SHARED_BY_ALL,
> +		 &ad4080_dec_rate_enum),

Wrap at 80 chars.

> +	IIO_ENUM_AVAILABLE("sinc_dec_rate",
> +			   IIO_SHARED_BY_ALL,
> +			   &ad4080_dec_rate_enum),
> +	IIO_ENUM("filter_sel",
> +		 IIO_SHARED_BY_ALL,
> +		 &ad4080_filter_mode_enum),
> +	IIO_ENUM_AVAILABLE("filter_sel",
> +			   IIO_SHARED_BY_ALL,
> +			   &ad4080_filter_mode_enum),
> +	{},
No comma on 'terminators' like this.

> +};

> +
> +static int ad4080_post_setup(struct iio_dev *indio_dev)
> +{
> +	struct ad4080_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = iio_backend_num_lanes_set(st->back, st->num_lanes);
> +	if (ret)
> +		return ret;
> +
> +	return iio_backend_self_sync_enable(st->back);
> +}
> +
> +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) {

If only called during probe just return dev_err_probe()

> +		dev_err(&st->spi->dev, "Unrecognized CHIP_ID 0x%X\n", id);
> +		return -EINVAL;
> +	}
> +
> +	if (st->num_lanes)
> +		regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_B,
> +			     FIELD_PREP(AD4080_LVDS_CNV_CLK_CNT_MSK, 7) |
> +			     AD4080_LVDS_CNV_EN_MSK);
Check return value on these
> +	else
> +		regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_B,
> +			     AD4080_LVDS_CNV_EN_MSK);
> +
> +	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;
> +
> +	return ad4080_post_setup(indio_dev);
What does that give you?  Just put those few lines here.
> +}
> +
> +static int ad4080_properties_parse(struct ad4080_state *st)
> +{
> +	unsigned int val;
> +	int ret;
> +
> +	ret = device_property_read_u32(&st->spi->dev, "adi,num_lanes", &val);
> +	if (!ret)
> +		st->num_lanes = val;
> +	else
> +		st->num_lanes = 1;
For cases with a default the pattern to use is:

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

No point in have a return type if you always return 0 - that just
implies a check is needed when it is not.

> +
> +	return 0;
> +}

> +static const struct spi_device_id ad4080_id[] = {
> +	{ "ad4080", (kernel_ulong_t)&ad4080_chip_info },

Unless you have support planned in the near future for more parts, we tend
not to add the flexibility of chip specific data until the series that
adds support for a second part.  Sometimes this has been added in the
past, no other part ever got supported, and we carry extra complexity
for no reason.   Good to mention if you are planning to add more
parts in this patches description.

> +	{ }
> +};
> +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);

[PATCH 14/14] Documentation: ABI: testing: ad4080 docs

[Antoniu Miclaus]

Add documentation for the ad4080 attributes.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
 .../ABI/testing/sysfs-bus-iio-adc-ad4080      | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad4080

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4080 b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4080
new file mode 100644
index 000000000000..e37bfba0e989
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4080
@@ -0,0 +1,55 @@
+What:		/sys/bus/iio/devices/iio:deviceX/lvds_sync
+Date:		February 2025
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		This attribute handles the data synchronization process.Because the CNV
+		signal is not taken into account by the FPGA when capturing the data, we
+		need a process that configures the ADC to output pattern data, writes the
+		SYNC bit in the axi_adc register map, waits until the custom HDL syncs the
+		data correctly, and then changes the output mode to analog data instead of
+		the fixed pattern.
+
+What:		/sys/bus/iio/devices/iio:deviceX/lvds_lvds
+Date:		February 2025
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Configures the signal type of the CNV signal. The value can be either CMOS
+		(lvds_cnv=0) or LVDS (lvds_cnv=1).
+
+What:		/sys/bus/iio/devices/iio:deviceX/filter_sel
+Date:		February 2025
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		This attribute enables the digital filter functionality of the AD4080.In
+		order to capture data correctly, the function must configure the ADC
+		through SPI to select the filter type and enable data capture in filter
+		mode through axi_adc(In this mode, data is gated by a signal generated by
+		the AD4080 (GPIO1 and is not continuous as it is when the filter is
+		disabled).
+
+What:		/sys/bus/iio/devices/iio:deviceX/filter_sel_available
+Date:		February 2025
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Return the available filter modes that can be set.
+
+What:		/sys/bus/iio/devices/iio:deviceX/sinc_dec_rate
+Date:		February 2025
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Set the filter’s decimation rate.
+
+What:		/sys/bus/iio/devices/iio:deviceX/sinc_dec_rate_available
+Date:		February 2025
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Return the available filter's decimation rates.
+
+
+
-- 
2.48.1

Re: [PATCH 14/14] Documentation: ABI: testing: ad4080 docs

[Nuno Sá]

On Thu, 2025-02-20 at 15:54 +0200, Antoniu Miclaus wrote:
> Add documentation for the ad4080 attributes.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
>  .../ABI/testing/sysfs-bus-iio-adc-ad4080      | 55 +++++++++++++++++++
>  1 file changed, 55 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad4080
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4080
> b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4080
> new file mode 100644
> index 000000000000..e37bfba0e989
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4080
> @@ -0,0 +1,55 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/lvds_sync
> +Date:		February 2025
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		This attribute handles the data synchronization
> process.Because the CNV
> +		signal is not taken into account by the FPGA when capturing
> the data, we
> +		need a process that configures the ADC to output pattern
> data, writes the
> +		SYNC bit in the axi_adc register map, waits until the custom
> HDL syncs the
> +		data correctly, and then changes the output mode to analog
> data instead of
> +		the fixed pattern.

I'll comment this one in the driver. I have some questions on how this works...

> +
> +What:		/sys/bus/iio/devices/iio:deviceX/lvds_lvds
> +Date:		February 2025
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Configures the signal type of the CNV signal. The value can
> be either CMOS
> +		(lvds_cnv=0) or LVDS (lvds_cnv=1).

The name seems to be wrong with what you have implemented. From this
description, I would think of this as a DT property? Can the signal type really
change at runtime?

> +
> +What:		/sys/bus/iio/devices/iio:deviceX/filter_sel
> +Date:		February 2025
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		This attribute enables the digital filter functionality of
> the AD4080.In
> +		order to capture data correctly, the function must configure
> the ADC
> +		through SPI to select the filter type and enable data capture
> in filter
> +		mode through axi_adc(In this mode, data is gated by a signal
> generated by
> +		the AD4080 (GPIO1 and is not continuous as it is when the
> filter is
> +		disabled).
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/filter_sel_available
> +Date:		February 2025
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Return the available filter modes that can be set.

There's a standard attr for this. I think we settled 
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/sinc_dec_rate
> +Date:		February 2025
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Set the filter’s decimation rate.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/sinc_dec_rate_available
> +Date:		February 2025
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Return the available filter's decimation rates.
> +
> +

I'm not yet convinced we need the dec_rate custom attr. I'll add more comments
in the driver.

- Nuno Sá

Re: [PATCH 14/14] Documentation: ABI: testing: ad4080 docs

[David Lechner]

On 2/20/25 8:53 AM, Nuno Sá wrote:
> On Thu, 2025-02-20 at 15:54 +0200, Antoniu Miclaus wrote:
>> Add documentation for the ad4080 attributes.
>>
>> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
>> ---
>>  .../ABI/testing/sysfs-bus-iio-adc-ad4080      | 55 +++++++++++++++++++
>>  1 file changed, 55 insertions(+)
>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad4080
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4080
>> b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4080
>> new file mode 100644
>> index 000000000000..e37bfba0e989
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4080
>> @@ -0,0 +1,55 @@
>> +What:		/sys/bus/iio/devices/iio:deviceX/lvds_sync
>> +Date:		February 2025
>> +KernelVersion:
>> +Contact:	linux-iio@vger.kernel.org
>> +Description:
>> +		This attribute handles the data synchronization
>> process.Because the CNV
>> +		signal is not taken into account by the FPGA when capturing
>> the data, we
>> +		need a process that configures the ADC to output pattern
>> data, writes the
>> +		SYNC bit in the axi_adc register map, waits until the custom
>> HDL syncs the
>> +		data correctly, and then changes the output mode to analog
>> data instead of
>> +		the fixed pattern.
> 
> I'll comment this one in the driver. I have some questions on how this works...
> 
>> +
>> +What:		/sys/bus/iio/devices/iio:deviceX/lvds_lvds
>> +Date:		February 2025
>> +KernelVersion:
>> +Contact:	linux-iio@vger.kernel.org
>> +Description:
>> +		Configures the signal type of the CNV signal. The value can
>> be either CMOS
>> +		(lvds_cnv=0) or LVDS (lvds_cnv=1).
> 
> The name seems to be wrong with what you have implemented. From this
> description, I would think of this as a DT property? Can the signal type really
> change at runtime?
> 
>> +
>> +What:		/sys/bus/iio/devices/iio:deviceX/filter_sel
>> +Date:		February 2025
>> +KernelVersion:
>> +Contact:	linux-iio@vger.kernel.org
>> +Description:
>> +		This attribute enables the digital filter functionality of
>> the AD4080.In
>> +		order to capture data correctly, the function must configure
>> the ADC
>> +		through SPI to select the filter type and enable data capture
>> in filter
>> +		mode through axi_adc(In this mode, data is gated by a signal
>> generated by
>> +		the AD4080 (GPIO1 and is not continuous as it is when the
>> filter is
>> +		disabled).
>> +
>> +What:		/sys/bus/iio/devices/iio:deviceX/filter_sel_available
>> +Date:		February 2025
>> +KernelVersion:
>> +Contact:	linux-iio@vger.kernel.org
>> +Description:
>> +		Return the available filter modes that can be set.
> 
> There's a standard attr for this. I think we settled 

Yup. filter_type and filter_type_available. 

>> +
>> +What:		/sys/bus/iio/devices/iio:deviceX/sinc_dec_rate
>> +Date:		February 2025
>> +KernelVersion:
>> +Contact:	linux-iio@vger.kernel.org
>> +Description:
>> +		Set the filter’s decimation rate.
>> +
>> +What:		/sys/bus/iio/devices/iio:deviceX/sinc_dec_rate_available
>> +Date:		February 2025
>> +KernelVersion:
>> +Contact:	linux-iio@vger.kernel.org
>> +Description:
>> +		Return the available filter's decimation rates.
>> +
>> +
> 
> I'm not yet convinced we need the dec_rate custom attr. I'll add more comments
> in the driver.

If we do need it, in another driver recently we concluded that
decimation rate is the same as oversampling ratio and there is
already a standard attribute for oversampling ratio, so we used
that.

> 
> - Nuno Sá
> 
> 

Re: [PATCH 14/14] Documentation: ABI: testing: ad4080 docs

[Nuno Sá]

On Thu, 2025-02-20 at 12:27 -0600, David Lechner wrote:
> On 2/20/25 8:53 AM, Nuno Sá wrote:
> > On Thu, 2025-02-20 at 15:54 +0200, Antoniu Miclaus wrote:
> > > Add documentation for the ad4080 attributes.
> > > 
> > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > > ---
> > >  .../ABI/testing/sysfs-bus-iio-adc-ad4080      | 55 +++++++++++++++++++
> > >  1 file changed, 55 insertions(+)
> > >  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad4080
> > > 
> > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4080
> > > b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4080
> > > new file mode 100644
> > > index 000000000000..e37bfba0e989
> > > --- /dev/null
> > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4080
> > > @@ -0,0 +1,55 @@
> > > +What:		/sys/bus/iio/devices/iio:deviceX/lvds_sync
> > > +Date:		February 2025
> > > +KernelVersion:
> > > +Contact:	linux-iio@vger.kernel.org
> > > +Description:
> > > +		This attribute handles the data synchronization
> > > process.Because the CNV
> > > +		signal is not taken into account by the FPGA when
> > > capturing
> > > the data, we
> > > +		need a process that configures the ADC to output pattern
> > > data, writes the
> > > +		SYNC bit in the axi_adc register map, waits until the
> > > custom
> > > HDL syncs the
> > > +		data correctly, and then changes the output mode to
> > > analog
> > > data instead of
> > > +		the fixed pattern.
> > 
> > I'll comment this one in the driver. I have some questions on how this
> > works...
> > 
> > > +
> > > +What:		/sys/bus/iio/devices/iio:deviceX/lvds_lvds
> > > +Date:		February 2025
> > > +KernelVersion:
> > > +Contact:	linux-iio@vger.kernel.org
> > > +Description:
> > > +		Configures the signal type of the CNV signal. The value
> > > can
> > > be either CMOS
> > > +		(lvds_cnv=0) or LVDS (lvds_cnv=1).
> > 
> > The name seems to be wrong with what you have implemented. From this
> > description, I would think of this as a DT property? Can the signal type
> > really
> > change at runtime?
> > 
> > > +
> > > +What:		/sys/bus/iio/devices/iio:deviceX/filter_sel
> > > +Date:		February 2025
> > > +KernelVersion:
> > > +Contact:	linux-iio@vger.kernel.org
> > > +Description:
> > > +		This attribute enables the digital filter functionality
> > > of
> > > the AD4080.In
> > > +		order to capture data correctly, the function must
> > > configure
> > > the ADC
> > > +		through SPI to select the filter type and enable data
> > > capture
> > > in filter
> > > +		mode through axi_adc(In this mode, data is gated by a
> > > signal
> > > generated by
> > > +		the AD4080 (GPIO1 and is not continuous as it is when the
> > > filter is
> > > +		disabled).
> > > +
> > > +What:		/sys/bus/iio/devices/iio:deviceX/filter_sel_available
> > > +Date:		February 2025
> > > +KernelVersion:
> > > +Contact:	linux-iio@vger.kernel.org
> > > +Description:
> > > +		Return the available filter modes that can be set.
> > 
> > There's a standard attr for this. I think we settled 
> 
> Yup. filter_type and filter_type_available. 
> 
> > > +
> > > +What:		/sys/bus/iio/devices/iio:deviceX/sinc_dec_rate
> > > +Date:		February 2025
> > > +KernelVersion:
> > > +Contact:	linux-iio@vger.kernel.org
> > > +Description:
> > > +		Set the filter’s decimation rate.
> > > +
> > > +What:		/sys/bus/iio/devices/iio:deviceX/sinc_dec_rate_available
> > > +Date:		February 2025
> > > +KernelVersion:
> > > +Contact:	linux-iio@vger.kernel.org
> > > +Description:
> > > +		Return the available filter's decimation rates.
> > > +
> > > +
> > 
> > I'm not yet convinced we need the dec_rate custom attr. I'll add more
> > comments
> > in the driver.
> 
> If we do need it, in another driver recently we concluded that
> decimation rate is the same as oversampling ratio and there is
> already a standard attribute for oversampling ratio, so we used
> that.
> 

Yeah, in theory decimation is about averaging samples. Makes sense to me even
though I never thought about using the oversampling ratio attr. I was biased by
the IMUs drivers where we configure the dec_rate as part of the sampling
frequency attr since these filters directly affect the chip ODR. 

Out of curiosity, how did you handled this in the other driver? I would be
tempted to only allow reading the sampling frequency attribute which means that
the oversampling ratio attr is the one we can write (which then directly affects
sampling frequency).

- Nuno Sá

Re: [PATCH 14/14] Documentation: ABI: testing: ad4080 docs

[David Lechner]

On 2/25/25 3:16 AM, Nuno Sá wrote:
> On Thu, 2025-02-20 at 12:27 -0600, David Lechner wrote:
>> On 2/20/25 8:53 AM, Nuno Sá wrote:
>>> On Thu, 2025-02-20 at 15:54 +0200, Antoniu Miclaus wrote:

...

>>>> +
>>>> +What:		/sys/bus/iio/devices/iio:deviceX/sinc_dec_rate
>>>> +Date:		February 2025
>>>> +KernelVersion:
>>>> +Contact:	linux-iio@vger.kernel.org
>>>> +Description:
>>>> +		Set the filter’s decimation rate.
>>>> +
>>>> +What:		/sys/bus/iio/devices/iio:deviceX/sinc_dec_rate_available
>>>> +Date:		February 2025
>>>> +KernelVersion:
>>>> +Contact:	linux-iio@vger.kernel.org
>>>> +Description:
>>>> +		Return the available filter's decimation rates.
>>>> +
>>>> +
>>>
>>> I'm not yet convinced we need the dec_rate custom attr. I'll add more
>>> comments
>>> in the driver.
>>
>> If we do need it, in another driver recently we concluded that
>> decimation rate is the same as oversampling ratio and there is
>> already a standard attribute for oversampling ratio, so we used
>> that.
>>
> 
> Yeah, in theory decimation is about averaging samples. Makes sense to me even
> though I never thought about using the oversampling ratio attr. I was biased by
> the IMUs drivers where we configure the dec_rate as part of the sampling
> frequency attr since these filters directly affect the chip ODR. 
> 
> Out of curiosity, how did you handled this in the other driver? I would be
> tempted to only allow reading the sampling frequency attribute which means that
> the oversampling ratio attr is the one we can write (which then directly affects
> sampling frequency).
> 
> - Nuno Sá

The other driver is still under review:

https://lore.kernel.org/linux-iio/2c3ce1701545e435238605342397e45657a0fb2a.1739368121.git.Jonathan.Santos@analog.com/

It is modifying an existing driver, so in that case, we still have to preserve
writing to sampling_frequency even if that isn't the ideal way to set it up.

Re: [PATCH 14/14] Documentation: ABI: testing: ad4080 docs

[Jonathan Cameron]

On Tue, 25 Feb 2025 09:26:56 -0600
David Lechner <dlechner@baylibre.com> wrote:

> On 2/25/25 3:16 AM, Nuno Sá wrote:
> > On Thu, 2025-02-20 at 12:27 -0600, David Lechner wrote:  
> >> On 2/20/25 8:53 AM, Nuno Sá wrote:  
> >>> On Thu, 2025-02-20 at 15:54 +0200, Antoniu Miclaus wrote:  
> 
> ...
> 
> >>>> +
> >>>> +What:		/sys/bus/iio/devices/iio:deviceX/sinc_dec_rate
> >>>> +Date:		February 2025
> >>>> +KernelVersion:
> >>>> +Contact:	linux-iio@vger.kernel.org
> >>>> +Description:
> >>>> +		Set the filter’s decimation rate.
> >>>> +
> >>>> +What:		/sys/bus/iio/devices/iio:deviceX/sinc_dec_rate_available
> >>>> +Date:		February 2025
> >>>> +KernelVersion:
> >>>> +Contact:	linux-iio@vger.kernel.org
> >>>> +Description:
> >>>> +		Return the available filter's decimation rates.
> >>>> +
> >>>> +  
> >>>
> >>> I'm not yet convinced we need the dec_rate custom attr. I'll add more
> >>> comments
> >>> in the driver.  
> >>
> >> If we do need it, in another driver recently we concluded that
> >> decimation rate is the same as oversampling ratio and there is
> >> already a standard attribute for oversampling ratio, so we used
> >> that.
> >>  
> > 
> > Yeah, in theory decimation is about averaging samples. Makes sense to me even
> > though I never thought about using the oversampling ratio attr. I was biased by
> > the IMUs drivers where we configure the dec_rate as part of the sampling
> > frequency attr since these filters directly affect the chip ODR. 
> > 
> > Out of curiosity, how did you handled this in the other driver? I would be
> > tempted to only allow reading the sampling frequency attribute which means that
> > the oversampling ratio attr is the one we can write (which then directly affects
> > sampling frequency).
> > 
> > - Nuno Sá  
> 
> The other driver is still under review:
> 
> https://lore.kernel.org/linux-iio/2c3ce1701545e435238605342397e45657a0fb2a.1739368121.git.Jonathan.Santos@analog.com/
> 
> It is modifying an existing driver, so in that case, we still have to preserve
> writing to sampling_frequency even if that isn't the ideal way to set it up.

It's pretty rare from what I recall to see a device sophisticated enough to do
over sampling that doesn't also support a sequencer that will allow configuring
the sampling frequency at a given oversampling ratio (by adding space, or more
settling time if a mux is involved which looks the same as space to software).

I guess there is always a first and to me in that case oversampling is the natural
control rather than sampling frequency.

Jonathan
Jonathan

Re: [PATCH 00/14] Add support for AD4080 ADC

[David Lechner]

On 2/20/25 7:54 AM, Antoniu Miclaus wrote:
> 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. Simplification of the
> input anti-alias filter design can be accomplished by applying over-
> sampling along with the integrated digital filtering and decimation to
> reduce noise and lower the output data rate for applications that do
> not require the lowest latency of the AD4080.
> 
It looks like this was just copied from the datasheet, so not useful
at all for a cover letter. We can read it in the datasheet.

Instead, please spend some time to explain the interesting and
unusual things about this driver that will help reviewers understand
*why* you are doing what you are doing. This is a very complex driver!

In particular, on this one, the documentation on the FPGA IP block isn't
very detailed. So it will be very helpful to know more about how all of
the sync stuff is supposed to work and what kind of filtering is the
FPGA doing in addition to the filtering done in the ADC chip.

Re: [PATCH 00/14] Add support for AD4080 ADC

[Jonathan Cameron]

On Thu, 20 Feb 2025 14:12:06 -0600
David Lechner <dlechner@baylibre.com> wrote:

> On 2/20/25 7:54 AM, Antoniu Miclaus wrote:
> > 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. Simplification of the
> > input anti-alias filter design can be accomplished by applying over-
> > sampling along with the integrated digital filtering and decimation to
> > reduce noise and lower the output data rate for applications that do
> > not require the lowest latency of the AD4080.
> >   
> It looks like this was just copied from the datasheet, so not useful
> at all for a cover letter. We can read it in the datasheet.
> 
> Instead, please spend some time to explain the interesting and
> unusual things about this driver that will help reviewers understand
> *why* you are doing what you are doing. This is a very complex driver!
> 
> In particular, on this one, the documentation on the FPGA IP block isn't
> very detailed. So it will be very helpful to know more about how all of
> the sync stuff is supposed to work and what kind of filtering is the
> FPGA doing in addition to the filtering done in the ADC chip.
> 
> 
> 
> 

This is a somewhat unusual device. I think a nice file with diagrams
etc under Documentation would also be good to have!

Jonathan