[PATCH v3 00/11] Add support for AD4080 ADC

[PATCH v3 00/11] Add support for AD4080 ADC

[Antoniu Miclaus]

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

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

Antoniu Miclaus (11):
  iio: backend: add support for filter config
  iio: backend: add support for data alignment
  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 type config
  iio: adc: adi-axi-adc: add sync enable/disable
  iio: adc: adi-axi-adc: add sync status
  iio: adc: adi-axi-adc: add num lanes support
  dt-bindings: iio: adc: add ad4080
  iio: adc: ad4080: add driver support

 .../bindings/iio/adc/adi,ad4080.yaml          |  96 +++
 .../bindings/iio/adc/adi,axi-adc.yaml         |   2 +
 drivers/iio/adc/Kconfig                       |  14 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/ad4080.c                      | 618 ++++++++++++++++++
 drivers/iio/adc/adi-axi-adc.c                 |  95 +++
 drivers/iio/industrialio-backend.c            |  72 ++
 include/linux/iio/backend.h                   |  25 +
 8 files changed, 923 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4080.yaml
 create mode 100644 drivers/iio/adc/ad4080.c

-- 
2.49.0

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

[Antoniu Miclaus]

Add backend support for digital filter type selection.

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

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

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v3:
 - update function to set the actual filter type instead of just enable/disable.
 drivers/iio/industrialio-backend.c | 15 +++++++++++++++
 include/linux/iio/backend.h        | 13 +++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index d4ad36f54090..2d28eabb1607 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -778,6 +778,21 @@ static int __devm_iio_backend_get(struct device *dev, struct iio_backend *back)
 	return 0;
 }
 
+/**
+ * iio_backend_filter_type_set - Set filter type
+ * @back: Backend device
+ * @type: Filter type.
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_filter_type_set(struct iio_backend *back,
+				enum iio_backend_filter_type type)
+{
+	return iio_backend_op_call(back, filter_type_set, type);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_filter_type_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 e45b7dfbec35..5526800f5d4a 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -76,6 +76,14 @@ enum iio_backend_interface_type {
 	IIO_BACKEND_INTERFACE_MAX
 };
 
+enum iio_backend_filter_type {
+	IIO_BACKEND_FILTER_TYPE_DISABLED,
+	IIO_BACKEND_FILTER_TYPE_SINC1,
+	IIO_BACKEND_FILTER_TYPE_SINC5,
+	IIO_BACKEND_FILTER_TYPE_SINC5_PLUS_COMP,
+	IIO_BACKEND_FILTER_TYPE_MAX
+};
+
 /**
  * struct iio_backend_ops - operations structure for an iio_backend
  * @enable: Enable backend.
@@ -100,6 +108,7 @@ 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_type_set: Set filter type.
  * @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 +159,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_type_set)(struct iio_backend *back,
+			       enum iio_backend_filter_type type);
 	int (*ddr_enable)(struct iio_backend *back);
 	int (*ddr_disable)(struct iio_backend *back);
 	int (*data_stream_enable)(struct iio_backend *back);
@@ -190,6 +201,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_type_set(struct iio_backend *back,
+				enum iio_backend_filter_type type);
 int iio_backend_ddr_enable(struct iio_backend *back);
 int iio_backend_ddr_disable(struct iio_backend *back);
 int iio_backend_data_stream_enable(struct iio_backend *back);
-- 
2.49.0

[PATCH v3 02/11] iio: backend: add support for data alignment

[Antoniu Miclaus]

Add backend support for enabling/disabling the capture synchronization.
When activated, it initates a proccess that aligns the sample's most
significant bit (MSB) based solely on the captured data, without
considering any other external signals.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v3:
 - update commit description and align with the new hdl changes.
 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 2d28eabb1607..b77a31b16aa5 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -793,6 +793,32 @@ int iio_backend_filter_type_set(struct iio_backend *back,
 }
 EXPORT_SYMBOL_NS_GPL(iio_backend_filter_type_set, "IIO_BACKEND");
 
+/**
+ * iio_backend_data_alignment_enable - Enable the sync process.
+ * @back: Backend device
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_data_alignment_enable(struct iio_backend *back)
+{
+	return iio_backend_op_call(back, data_alignment_enable);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_data_alignment_enable, "IIO_BACKEND");
+
+/**
+ * iio_backend_data_alignment_disable - Disable the sync process.
+ * @back: Backend device
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_data_alignment_disable(struct iio_backend *back)
+{
+	return iio_backend_op_call(back, data_alignment_disable);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_data_alignment_disable, "IIO_BACKEND");
+
 /**
  * iio_backend_ddr_enable - Enable interface DDR (Double Data Rate) mode
  * @back: Backend device
diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
index 5526800f5d4a..bd973de8cad9 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -109,6 +109,8 @@ enum iio_backend_filter_type {
  * @debugfs_print_chan_status: Print channel status into a buffer.
  * @debugfs_reg_access: Read or write register value of backend.
  * @filter_type_set: Set filter type.
+ * @data_alignment_enable: Enable sync process.
+ * @data_alignment_disable: Disable sync process.
  * @ddr_enable: Enable interface DDR (Double Data Rate) mode.
  * @ddr_disable: Disable interface DDR (Double Data Rate) mode.
  * @data_stream_enable: Enable data stream.
@@ -161,6 +163,8 @@ struct iio_backend_ops {
 				  unsigned int writeval, unsigned int *readval);
 	int (*filter_type_set)(struct iio_backend *back,
 			       enum iio_backend_filter_type type);
+	int (*data_alignment_enable)(struct iio_backend *back);
+	int (*data_alignment_disable)(struct iio_backend *back);
 	int (*ddr_enable)(struct iio_backend *back);
 	int (*ddr_disable)(struct iio_backend *back);
 	int (*data_stream_enable)(struct iio_backend *back);
@@ -203,6 +207,8 @@ int devm_iio_backend_request_buffer(struct device *dev,
 				    struct iio_dev *indio_dev);
 int iio_backend_filter_type_set(struct iio_backend *back,
 				enum iio_backend_filter_type type);
+int iio_backend_data_alignment_enable(struct iio_backend *back);
+int iio_backend_data_alignment_disable(struct iio_backend *back);
 int iio_backend_ddr_enable(struct iio_backend *back);
 int iio_backend_ddr_disable(struct iio_backend *back);
 int iio_backend_data_stream_enable(struct iio_backend *back);
-- 
2.49.0

[PATCH v3 03/11] 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 data alignment has been
correctly performed and capture is synchronized. If successful, the
return value will be true.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v3:
 - update commit description.
 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 b77a31b16aa5..f7c8167a248d 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -819,6 +819,20 @@ int iio_backend_data_alignment_disable(struct iio_backend *back)
 }
 EXPORT_SYMBOL_NS_GPL(iio_backend_data_alignment_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 bd973de8cad9..e93d1a98e066 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -111,6 +111,7 @@ enum iio_backend_filter_type {
  * @filter_type_set: Set filter type.
  * @data_alignment_enable: Enable sync process.
  * @data_alignment_disable: Disable sync process.
+ * @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.
@@ -165,6 +166,7 @@ struct iio_backend_ops {
 			       enum iio_backend_filter_type type);
 	int (*data_alignment_enable)(struct iio_backend *back);
 	int (*data_alignment_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);
@@ -209,6 +211,7 @@ int iio_backend_filter_type_set(struct iio_backend *back,
 				enum iio_backend_filter_type type);
 int iio_backend_data_alignment_enable(struct iio_backend *back);
 int iio_backend_data_alignment_disable(struct iio_backend *back);
+int iio_backend_sync_status_get(struct iio_backend *back, bool *sync_en);
 int iio_backend_ddr_enable(struct iio_backend *back);
 int iio_backend_ddr_disable(struct iio_backend *back);
 int iio_backend_data_stream_enable(struct iio_backend *back);
-- 
2.49.0

[PATCH v3 04/11] iio: backend: add support for number of lanes

[Antoniu Miclaus]

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

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
no changes in v3.
 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 f7c8167a248d..1063935cd808 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -833,6 +833,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 e93d1a98e066..c8bcfe96c542 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -112,6 +112,7 @@ enum iio_backend_filter_type {
  * @data_alignment_enable: Enable sync process.
  * @data_alignment_disable: Disable sync process.
  * @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.
@@ -167,6 +168,7 @@ struct iio_backend_ops {
 	int (*data_alignment_enable)(struct iio_backend *back);
 	int (*data_alignment_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);
@@ -212,6 +214,7 @@ int iio_backend_filter_type_set(struct iio_backend *back,
 int iio_backend_data_alignment_enable(struct iio_backend *back);
 int iio_backend_data_alignment_disable(struct iio_backend *back);
 int iio_backend_sync_status_get(struct iio_backend *back, bool *sync_en);
+int iio_backend_num_lanes_set(struct iio_backend *back, unsigned int num_lanes);
 int iio_backend_ddr_enable(struct iio_backend *back);
 int iio_backend_ddr_disable(struct iio_backend *back);
 int iio_backend_data_stream_enable(struct iio_backend *back);
-- 
2.49.0

[PATCH v3 05/11] dt-bindings: iio: adc: add ad408x axi variant

[Antoniu Miclaus]

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

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

Although there are some particularities added for extended
control of the ad408x devices such as the filter configuration.

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

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
no changes in v3.
 Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml | 2 ++
 1 file changed, 2 insertions(+)

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

[PATCH v3 06/11] iio: adc: adi-axi-adc: add filter type config

[Antoniu Miclaus]

Add support for enabling/disabling filter based on the filter type
provided.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v3:
 - update the function to adapt to the backend interface.
 drivers/iio/adc/adi-axi-adc.c | 38 +++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index 61ab7dce43be..2a3a6c3f5e59 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,19 @@ static int axi_adc_ad485x_oversampling_ratio_set(struct iio_backend *back,
 	}
 }
 
+static int axi_adc_ad408x_filter_type_set(struct iio_backend *back,
+					  enum iio_backend_filter_type type)
+{
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+
+	if (type)
+		return regmap_set_bits(st->regmap, ADI_AXI_ADC_REG_CNTRL_3,
+				       AXI_AD408X_CNTRL_3_FILTER_EN_MSK);
+
+	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 +596,24 @@ 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_type_set = axi_adc_ad408x_filter_type_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),
+};
+
+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 +729,15 @@ static const struct axi_adc_info adc_ad7606 = {
 	.has_child_nodes = true,
 };
 
+static const struct axi_adc_info adi_axi_ad408x = {
+	.version = ADI_AXI_PCORE_VER(10, 0, 'a'),
+	.backend_info = &axi_ad408x,
+};
+
 /* Match table for of_platform binding */
 static const struct of_device_id adi_axi_adc_of_match[] = {
 	{ .compatible = "adi,axi-adc-10.0.a", .data = &adc_generic },
+	{ .compatible = "adi,axi-ad408x", .data = &adi_axi_ad408x },
 	{ .compatible = "adi,axi-ad485x", .data = &adi_axi_ad485x },
 	{ .compatible = "adi,axi-ad7606x", .data = &adc_ad7606 },
 	{ /* end of list */ }
-- 
2.49.0

[PATCH v3 07/11] iio: adc: adi-axi-adc: add sync enable/disable

[Antoniu Miclaus]

Add support for enabling/disabling the sync process used for data
capture alignment.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v3:
 - update the function to match the new backend interface.
 drivers/iio/adc/adi-axi-adc.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index 2a3a6c3f5e59..9947be059f98 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_SYNC_MSK		BIT(3)
 #define    ADI_AXI_ADC_CTRL_DDR_EDGESEL_MASK	BIT(1)
 
 #define ADI_AXI_ADC_REG_CNTRL_3			0x004c
@@ -416,6 +417,22 @@ static int axi_adc_ad408x_filter_type_set(struct iio_backend *back,
 				 AXI_AD408X_CNTRL_3_FILTER_EN_MSK);
 }
 
+static int axi_adc_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_CTRL,
+			       AXI_AD408X_CTRL_SYNC_MSK);
+}
+
+static int axi_adc_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_CTRL,
+				 AXI_AD408X_CTRL_SYNC_MSK);
+}
+
 static struct iio_buffer *axi_adc_request_buffer(struct iio_backend *back,
 						 struct iio_dev *indio_dev)
 {
@@ -559,6 +576,8 @@ static const struct iio_backend_ops adi_axi_adc_ops = {
 	.request_buffer = axi_adc_request_buffer,
 	.free_buffer = axi_adc_free_buffer,
 	.data_sample_trigger = axi_adc_data_sample_trigger,
+	.data_alignment_enable = axi_adc_sync_enable,
+	.data_alignment_disable = axi_adc_sync_disable,
 	.iodelay_set = axi_adc_iodelays_set,
 	.test_pattern_set = axi_adc_test_pattern_set,
 	.chan_status = axi_adc_chan_status,
@@ -605,6 +624,8 @@ static const struct iio_backend_ops adi_ad408x_ops = {
 	.free_buffer = axi_adc_free_buffer,
 	.data_sample_trigger = axi_adc_data_sample_trigger,
 	.filter_type_set = axi_adc_ad408x_filter_type_set,
+	.data_alignment_enable = axi_adc_sync_enable,
+	.data_alignment_disable = axi_adc_sync_disable,
 	.debugfs_reg_access = iio_backend_debugfs_ptr(axi_adc_reg_access),
 	.debugfs_print_chan_status = iio_backend_debugfs_ptr(axi_adc_debugfs_print_chan_status),
 };
-- 
2.49.0

[PATCH v3 08/11] iio: adc: adi-axi-adc: add sync status

[Antoniu Miclaus]

Add support for checking the ADC sync status. A true value is obtained
if the data capture is synchronized.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
no changes in v3.
 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 9947be059f98..bf0155830d87 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -55,6 +55,9 @@
 #define   AXI_AD485X_PACKET_FORMAT_32BIT	0x2
 #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)
 
@@ -433,6 +436,21 @@ static int axi_adc_sync_disable(struct iio_backend *back)
 				 AXI_AD408X_CTRL_SYNC_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 = 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)
 {
@@ -582,6 +600,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),
 };
@@ -626,6 +645,7 @@ static const struct iio_backend_ops adi_ad408x_ops = {
 	.filter_type_set = axi_adc_ad408x_filter_type_set,
 	.data_alignment_enable = axi_adc_sync_enable,
 	.data_alignment_disable = axi_adc_sync_disable,
+	.sync_status_get = axi_adc_sync_status_get,
 	.debugfs_reg_access = iio_backend_debugfs_ptr(axi_adc_reg_access),
 	.debugfs_print_chan_status = iio_backend_debugfs_ptr(axi_adc_debugfs_print_chan_status),
 };
-- 
2.49.0

[PATCH v3 09/11] iio: adc: adi-axi-adc: add num lanes support

[Antoniu Miclaus]

Add support for setting the number of lanes enabled.

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

diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index bf0155830d87..8ff781ab5ec3 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_SYNC_MSK		BIT(3)
 #define    ADI_AXI_ADC_CTRL_DDR_EDGESEL_MASK	BIT(1)
 
@@ -451,6 +452,19 @@ static int axi_adc_sync_status_get(struct iio_backend *back, bool *sync_en)
 	return 0;
 }
 
+static int axi_adc_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)
 {
@@ -601,6 +615,7 @@ static const struct iio_backend_ops adi_axi_adc_ops = {
 	.chan_status = axi_adc_chan_status,
 	.interface_type_get = axi_adc_interface_type_get,
 	.sync_status_get = axi_adc_sync_status_get,
+	.num_lanes_set = axi_adc_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),
 };
@@ -646,6 +661,7 @@ static const struct iio_backend_ops adi_ad408x_ops = {
 	.data_alignment_enable = axi_adc_sync_enable,
 	.data_alignment_disable = axi_adc_sync_disable,
 	.sync_status_get = axi_adc_sync_status_get,
+	.num_lanes_set = axi_adc_num_lanes_set,
 	.debugfs_reg_access = iio_backend_debugfs_ptr(axi_adc_reg_access),
 	.debugfs_print_chan_status = iio_backend_debugfs_ptr(axi_adc_debugfs_print_chan_status),
 };
-- 
2.49.0

[PATCH v3 10/11] dt-bindings: iio: adc: add ad4080

[Antoniu Miclaus]

Add devicetree bindings for ad4080 family.

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
 .../bindings/iio/adc/adi,ad4080.yaml          | 96 +++++++++++++++++++
 1 file changed, 96 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4080.yaml

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

[PATCH v3 11/11] iio: adc: ad4080: add driver support

[Antoniu Miclaus]

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

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v3:
 - name the defines to make associacion with register name.
 - drop redundant defines.
 - add trailing comma when needed.
 - drop explicit masking and use field_prep instead
 - add fsleep during sync process.
 - do not wrap where 80 chars is not exceeded.
 - use space for { }
 - drop channel definition macro
 - return dev_info on chip id mismatch.
 - flip expression to `if (!st->lvds_cnv_en)`
 - rework num_lanes property parse.
 - update the driver based on the new edits on the backend interface related to
   this part and the last disscussions in v2.
 drivers/iio/adc/Kconfig  |  14 +
 drivers/iio/adc/Makefile |   1 +
 drivers/iio/adc/ad4080.c | 618 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 633 insertions(+)
 create mode 100644 drivers/iio/adc/ad4080.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 27413516216c..17df328f5322 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -47,6 +47,20 @@ config AD4030
 	  To compile this driver as a module, choose M here: the module will be
 	  called ad4030.
 
+config AD4080
+	tristate "Analog Devices AD4080 high speed ADC"
+	depends on SPI
+	select REGMAP_SPI
+	select IIO_BACKEND
+	help
+	  Say yes here to build support for Analog Devices AD4080
+	  high speed, low noise, low distortion, 20-bit, Easy Drive,
+	  successive approximation register (SAR) analog-to-digital
+	  converter (ADC). Supports iio_backended devices for AD4080.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called ad4080.
+
 config AD4130
 	tristate "Analog Device AD4130 ADC Driver"
 	depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 9f26d5eca822..e6efed5b4e7a 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
 obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
 obj-$(CONFIG_AD4000) += ad4000.o
 obj-$(CONFIG_AD4030) += ad4030.o
+obj-$(CONFIG_AD4080) += ad4080.o
 obj-$(CONFIG_AD4130) += ad4130.o
 obj-$(CONFIG_AD4695) += ad4695.o
 obj-$(CONFIG_AD4851) += ad4851.o
diff --git a/drivers/iio/adc/ad4080.c b/drivers/iio/adc/ad4080.c
new file mode 100644
index 000000000000..b51893253941
--- /dev/null
+++ b/drivers/iio/adc/ad4080.c
@@ -0,0 +1,618 @@
+// 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_INTERFACE_CONFIG_A_SW_RESET_MSK			(BIT(7) | BIT(0))
+#define AD4080_INTERFACE_CONFIG_A_ADDR_ASC_MSK			BIT(5)
+#define AD4080_INTERFACE_CONFIG_A_SDO_ENABLE_MSK		BIT(4)
+
+/* AD4080_REG_INTERFACE_CONFIG_B Bit Definition */
+#define AD4080_INTERFACE_CONFIG_B_SINGLE_INST_MSK		BIT(7)
+#define AD4080_INTERFACE_CONFIG_B_SHORT_INST_MSK		BIT(3)
+
+/* AD4080_REG_DEVICE_CONFIG Bit Definition */
+#define AD4080_DEVICE_CONFIG_OPERATING_MODES_MSK		GENMASK(1, 0)
+
+/* AD4080_REG_TRANSFER_CONFIG Bit Definition */
+#define AD4080_TRANSFER_CONFIG_KEEP_STREAM_LENGTH_VAL_MSK	BIT(2)
+
+/* AD4080_REG_INTERFACE_CONFIG_C Bit Definition */
+#define AD4080_INTERFACE_CONFIG_C_STRICT_REG_ACCESS_MSK		BIT(5)
+
+/* AD4080_REG_ADC_DATA_INTF_CONFIG_A Bit Definition */
+#define AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK	BIT(6)
+#define AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK		BIT(4)
+#define AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK	BIT(2)
+#define AD4080_ADC_DATA_INTF_CONFIG_A_DATA_INTF_MODE_MSK	BIT(0)
+
+/* AD4080_REG_ADC_DATA_INTF_CONFIG_B Bit Definition */
+#define AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_CLK_CNT_MSK	GENMASK(7, 4)
+#define AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_SELF_CLK_MODE_MSK	BIT(3)
+#define AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_EN_MSK		BIT(0)
+
+/* AD4080_REG_ADC_DATA_INTF_CONFIG_C Bit Definition */
+#define AD4080_ADC_DATA_INTF_CONFIG_C_LVDS_VOD_MSK		GENMASK(6, 4)
+
+/* AD4080_REG_PWR_CTRL Bit Definition */
+#define AD4080_PWR_CTRL_ANA_DIG_LDO_PD_MSK			BIT(1)
+#define AD4080_PWR_CTRL_INTF_LDO_PD_MSK				BIT(0)
+
+/* AD4080_REG_GPIO_CONFIG_A Bit Definition */
+#define AD4080_GPIO_CONFIG_A_GPO_1_EN_MSK			BIT(1)
+#define AD4080_GPIO_CONFIG_A_GPO_0_EN_MSK			BIT(0)
+
+/* AD4080_REG_GPIO_CONFIG_B Bit Definition */
+#define AD4080_GPIO_CONFIG_B_GPIO_1_SEL				GENMASK(7, 4)
+#define AD4080_GPIO_CONFIG_B_GPIO_0_SEL				GENMASK(3, 0)
+
+/* AD4080_REG_FIFO_CONFIG Bit Definition */
+#define AD4080_FIFO_CONFIG_FIFO_MODE_MSK			GENMASK(1, 0)
+
+/* AD4080_REG_FILTER_CONFIG Bit Definition */
+#define AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK			GENMASK(6, 3)
+#define AD4080_FILTER_CONFIG_FILTER_SEL_MSK			GENMASK(1, 0)
+
+/* Miscellaneous Definitions */
+#define AD4080_SPI_READ						BIT(7)
+#define AD4080_CHIP_ID						GENMASK(2, 0)
+
+#define AD4080_MAX_SAMP_FREQ					40000000
+#define AD4080_MIN_SAMP_FREQ					1250000
+
+enum ad4080_filter_type {
+	FILTER_DISABLE,
+	SINC_1,
+	SINC_5,
+	SINC_5_COMP
+};
+
+static const unsigned int ad4080_scale_table[][2] = {
+	{ 6000, 0},
+};
+
+static const char *const ad4080_filter_type_iio_enum[] = {
+	[FILTER_DISABLE]   = "disabled",
+	[SINC_1]           = "sinc1",
+	[SINC_5]           = "sinc5",
+	[SINC_5_COMP]      = "sinc5_plus_compensation",
+};
+
+static const int ad4080_dec_rate_iio_enum[] = {
+	2, 4, 8, 16, 32, 64, 128, 256, 512, 1024,
+};
+
+static const char * const ad4080_power_supplies[] = {
+	"vdd33", "vdd11", "vddldo", "iovdd", "vrefin",
+};
+
+struct ad4080_chip_info {
+	const char *name;
+	unsigned int product_id;
+	int num_scales;
+	const unsigned int (*scale_table)[2];
+	const struct iio_chan_spec *channels;
+	unsigned int num_channels;
+};
+
+struct ad4080_state {
+	struct spi_device		*spi;
+	struct regmap			*regmap;
+	struct clk			*clk;
+	struct iio_backend		*back;
+	const struct ad4080_chip_info	*info;
+	/*
+	 * Synchronize access to members the of driver state, and ensure
+	 * atomicity of consecutive regmap operations.
+	 */
+	struct mutex			lock;
+	unsigned int			num_lanes;
+	unsigned int			dec_rate;
+	enum ad4080_filter_type		filter_type;
+	bool				lvds_cnv_en;
+};
+
+static const struct regmap_config ad4080_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	.read_flag_mask = BIT(7),
+	.max_register = 0x29,
+};
+
+static int ad4080_reg_access(struct iio_dev *indio_dev, unsigned int reg,
+			     unsigned int writeval, unsigned int *readval)
+{
+	struct ad4080_state *st = iio_priv(indio_dev);
+
+	if (readval)
+		return regmap_read(st->regmap, reg, readval);
+
+	return regmap_write(st->regmap, reg, writeval);
+}
+
+static int ad4080_get_scale(struct ad4080_state *st, int *val, int *val2)
+{
+	unsigned int tmp;
+
+	tmp = (st->info->scale_table[0][0] * 1000000ULL) >>
+		    st->info->channels[0].scan_type.realbits;
+	*val = tmp / 1000000;
+	*val2 = tmp % 1000000;
+
+	return IIO_VAL_INT_PLUS_NANO;
+}
+
+static unsigned int ad4080_get_dec_rate(struct iio_dev *dev,
+					const struct iio_chan_spec *chan)
+{
+	struct ad4080_state *st = iio_priv(dev);
+	int ret;
+	unsigned int data;
+
+	ret = regmap_read(st->regmap, AD4080_REG_FILTER_CONFIG, &data);
+	if (ret)
+		return ret;
+
+	return (1 << (FIELD_GET(AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK, data) + 1));
+}
+
+static int ad4080_set_dec_rate(struct iio_dev *dev,
+			       const struct iio_chan_spec *chan,
+			       unsigned int mode)
+{
+	struct ad4080_state *st = iio_priv(dev);
+	int ret;
+
+	if (st->filter_type >= SINC_5 && mode >= 512)
+		return -EINVAL;
+
+	guard(mutex)(&st->lock);
+	ret = regmap_update_bits(st->regmap, AD4080_REG_FILTER_CONFIG,
+				 AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK,
+				 FIELD_PREP(AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK,
+					    (ilog2(mode) - 1)));
+	if (ret)
+		return ret;
+
+	st->dec_rate = mode;
+
+	return 0;
+}
+
+static int ad4080_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long m)
+{
+	struct ad4080_state *st = iio_priv(indio_dev);
+	int dec_rate;
+
+	switch (m) {
+	case IIO_CHAN_INFO_SCALE:
+		return ad4080_get_scale(st, val, val2);
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		if (st->filter_type == SINC_5_COMP)
+			dec_rate = st->dec_rate * 2;
+		else
+			dec_rate = st->dec_rate;
+		if (st->filter_type)
+			*val = DIV_ROUND_CLOSEST(clk_get_rate(st->clk), dec_rate);
+		else
+			*val = clk_get_rate(st->clk);
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		*val = ad4080_get_dec_rate(indio_dev, chan);
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad4080_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		return -EINVAL;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return -EINVAL;
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		return ad4080_set_dec_rate(indio_dev, chan, val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad4080_lvds_sync_write(struct ad4080_state *st)
+{
+	unsigned int timeout = 100;
+	bool sync_en;
+	int ret;
+
+	guard(mutex)(&st->lock);
+	if (st->num_lanes == 1)
+		ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
+				   AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
+				   AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK);
+	else
+		ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
+				   AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
+				   AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK |
+				   AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
+	if (ret)
+		return ret;
+
+	ret = iio_backend_data_alignment_enable(st->back);
+	if (ret)
+		return ret;
+
+	do {
+		ret = iio_backend_sync_status_get(st->back, &sync_en);
+		if (ret)
+			return ret;
+
+		if (!sync_en)
+			dev_dbg(&st->spi->dev, "Not Locked: Running Bit Slip\n");
+
+		fsleep(500);
+	} while (--timeout && !sync_en);
+
+	if (timeout) {
+		dev_info(&st->spi->dev, "Success: Pattern correct and Locked!\n");
+		if (st->num_lanes == 1)
+			return regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
+					    AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK);
+		else
+			return regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
+					    AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
+					    AD4080_ADC_DATA_INTF_CONFIG_A_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_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK);
+			if (ret)
+				return ret;
+		} else {
+			ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
+					   AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
+					   AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
+			if (ret)
+				return ret;
+		}
+
+		return -ETIMEDOUT;
+	}
+}
+
+static ssize_t ad4080_get_filter_type(struct iio_dev *dev,
+				      const struct iio_chan_spec *chan)
+{
+	struct ad4080_state *st = iio_priv(dev);
+	unsigned int data;
+	int ret;
+
+	ret = regmap_read(st->regmap, AD4080_REG_FILTER_CONFIG, &data);
+	if (ret)
+		return ret;
+
+	return FIELD_GET(AD4080_FILTER_CONFIG_FILTER_SEL_MSK, data);
+}
+
+static int ad4080_set_filter_type(struct iio_dev *dev,
+				  const struct iio_chan_spec *chan,
+				  unsigned int mode)
+{
+	struct ad4080_state *st = iio_priv(dev);
+	int ret;
+
+	if (mode >= SINC_5 && st->dec_rate >= 512)
+		return -EINVAL;
+
+	guard(mutex)(&st->lock);
+	ret = iio_backend_filter_type_set(st->back, mode);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(st->regmap, AD4080_REG_FILTER_CONFIG,
+				 AD4080_FILTER_CONFIG_FILTER_SEL_MSK,
+				 FIELD_PREP(AD4080_FILTER_CONFIG_FILTER_SEL_MSK,
+					    mode));
+	if (ret)
+		return ret;
+
+	st->filter_type = mode;
+
+	return 0;
+}
+
+static int ad4080_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     const int **vals, int *type, int *length,
+			     long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		*vals = ad4080_dec_rate_iio_enum;
+		*length = ARRAY_SIZE(ad4080_dec_rate_iio_enum);
+		*type = IIO_VAL_INT;
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info ad4080_iio_info = {
+	.debugfs_reg_access = ad4080_reg_access,
+	.read_raw = ad4080_read_raw,
+	.write_raw = ad4080_write_raw,
+	.read_avail = ad4080_read_avail,
+};
+
+static const struct iio_enum ad4080_filter_type_enum = {
+	.items = ad4080_filter_type_iio_enum,
+	.num_items = ARRAY_SIZE(ad4080_filter_type_iio_enum),
+	.set = ad4080_set_filter_type,
+	.get = ad4080_get_filter_type,
+};
+
+static struct iio_chan_spec_ext_info ad4080_ext_info[] = {
+	IIO_ENUM("filter_type", IIO_SHARED_BY_ALL, &ad4080_filter_type_enum),
+	IIO_ENUM_AVAILABLE("filter_type", IIO_SHARED_BY_ALL,
+			   &ad4080_filter_type_enum),
+	{ }
+};
+
+static const struct iio_chan_spec ad4080_channels[] = {
+	{
+		.type = IIO_VOLTAGE,
+		.indexed = 1,
+		.channel = 0,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+				BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+		.info_mask_shared_by_all_available =
+				BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+		.ext_info = ad4080_ext_info,
+		.scan_index = 0,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 20,
+			.storagebits = 32,
+			.shift = 0,
+		},
+	}
+};
+
+static const struct ad4080_chip_info ad4080_chip_info = {
+	.name = "AD4080",
+	.product_id = AD4080_CHIP_ID,
+	.scale_table = ad4080_scale_table,
+	.num_scales = ARRAY_SIZE(ad4080_scale_table),
+	.num_channels = 1,
+	.channels = ad4080_channels,
+};
+
+static int ad4080_setup(struct iio_dev *indio_dev)
+{
+	struct ad4080_state *st = iio_priv(indio_dev);
+	unsigned int id;
+	int ret;
+
+	ret = regmap_write(st->regmap, AD4080_REG_INTERFACE_CONFIG_A,
+			   AD4080_INTERFACE_CONFIG_A_SW_RESET_MSK);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, AD4080_REG_INTERFACE_CONFIG_A,
+			   AD4080_INTERFACE_CONFIG_A_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_info(&st->spi->dev, "Unrecognized CHIP_ID 0x%X\n", id);
+
+	ret = regmap_set_bits(st->regmap, AD4080_REG_GPIO_CONFIG_A,
+			      AD4080_GPIO_CONFIG_A_GPO_1_EN_MSK);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, AD4080_REG_GPIO_CONFIG_B,
+			   FIELD_PREP(AD4080_GPIO_CONFIG_B_GPIO_1_SEL, 3));
+	if (ret)
+		return ret;
+
+	ret = iio_backend_num_lanes_set(st->back, st->num_lanes);
+	if (ret)
+		return ret;
+
+	if (!st->lvds_cnv_en)
+		return 0;
+
+	if (st->num_lanes) {
+		ret = regmap_update_bits(st->regmap,
+					 AD4080_REG_ADC_DATA_INTF_CONFIG_B,
+					 AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_CLK_CNT_MSK,
+					 FIELD_PREP(AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_CLK_CNT_MSK,
+						    7));
+		if (ret)
+			return ret;
+	}
+
+	ret = regmap_set_bits(st->regmap,
+			      AD4080_REG_ADC_DATA_INTF_CONFIG_B,
+			      AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_EN_MSK);
+	if (ret)
+		return ret;
+
+	return ad4080_lvds_sync_write(st);
+}
+
+static void ad4080_properties_parse(struct ad4080_state *st)
+{
+	st->lvds_cnv_en = device_property_read_bool(&st->spi->dev,
+						    "adi,lvds-cnv-enable");
+
+	st->num_lanes = 1;
+	device_property_read_u32(&st->spi->dev, "adi,num_lanes", &st->num_lanes);
+}
+
+static int ad4080_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct device *dev = &spi->dev;
+	struct ad4080_state *st;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+
+	ret = devm_regulator_bulk_get_enable(dev,
+					     ARRAY_SIZE(ad4080_power_supplies),
+					     ad4080_power_supplies);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to get and enable supplies\n");
+
+	st->regmap = devm_regmap_init_spi(spi, &ad4080_regmap_config);
+	if (IS_ERR(st->regmap))
+		return PTR_ERR(st->regmap);
+
+	st->info = spi_get_device_match_data(spi);
+	if (!st->info)
+		return -ENODEV;
+
+	ret = devm_mutex_init(dev, &st->lock);
+	if (ret)
+		return ret;
+
+	st->info = spi_get_device_match_data(spi);
+	if (!st->info)
+		return -ENODEV;
+
+	indio_dev->name = st->info->name;
+	indio_dev->channels = st->info->channels;
+	indio_dev->num_channels = st->info->num_channels;
+	indio_dev->info = &ad4080_iio_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	ad4080_properties_parse(st);
+
+	st->clk = devm_clk_get_enabled(&spi->dev, "cnv");
+	if (IS_ERR(st->clk))
+		return PTR_ERR(st->clk);
+
+	st->back = devm_iio_backend_get(dev, NULL);
+	if (IS_ERR(st->back))
+		return PTR_ERR(st->back);
+
+	ret = devm_iio_backend_request_buffer(dev, st->back, indio_dev);
+	if (ret)
+		return ret;
+
+	ret = devm_iio_backend_enable(dev, st->back);
+	if (ret)
+		return ret;
+
+	ret = ad4080_setup(indio_dev);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct spi_device_id ad4080_id[] = {
+	{ "ad4080", (kernel_ulong_t)&ad4080_chip_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ad4080_id);
+
+static const struct of_device_id ad4080_of_match[] = {
+	{ .compatible = "adi,ad4080", &ad4080_chip_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad4080_of_match);
+
+static struct spi_driver ad4080_driver = {
+	.driver = {
+		.name = "ad4080",
+		.of_match_table = ad4080_of_match,
+	},
+	.probe = ad4080_probe,
+	.id_table = ad4080_id,
+};
+module_spi_driver(ad4080_driver);
+
+MODULE_AUTHOR("Antoniu Miclaus <antoniu.miclaus@analog.com");
+MODULE_DESCRIPTION("Analog Devices AD4080");
+MODULE_LICENSE("GPL");
-- 
2.49.0

Re: [PATCH v3 11/11] iio: adc: ad4080: add driver support

[Jonathan Cameron]

On Fri, 25 Apr 2025 14:25:38 +0300
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> Add support for AD4080 high-speed, low noise, low distortion,
> 20-bit, Easy Drive, successive approximation register (SAR)
> analog-to-digital converter (ADC).
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
Hi Antoniu

I noticed a few things on a fresh read through I'd missed on earlier versions.
Sorry about that!

In particular I think moving to explicitly just updating the bits you need
to in a few paths around sync will simplify the code quite a bit and make
sure the stuff related to sync is all that happens in there.

The use of _MSK defines for single bits is fine if you also use FIELD_PREP()
to set them to 0 / 1 as appropriate, but it is confusing if you just
use them directly to write the values as we can't see at that point in the
code that they are single bit masks.  I'd drop the _MSK postfix on those.

Thanks

Jonathan

> 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..b51893253941
> --- /dev/null
> +++ b/drivers/iio/adc/ad4080.c
> @@ -0,0 +1,618 @@

> +/* AD4080_REG_INTERFACE_CONFIG_A Bit Definition */
> +#define AD4080_INTERFACE_CONFIG_A_SW_RESET_MSK			(BIT(7) | BIT(0))
> +#define AD4080_INTERFACE_CONFIG_A_ADDR_ASC_MSK			BIT(5)
> +#define AD4080_INTERFACE_CONFIG_A_SDO_ENABLE_MSK		BIT(4)
> +
> +/* AD4080_REG_INTERFACE_CONFIG_B Bit Definition */
> +#define AD4080_INTERFACE_CONFIG_B_SINGLE_INST_MSK		BIT(7)
> +#define AD4080_INTERFACE_CONFIG_B_SHORT_INST_MSK		BIT(3)
> +
> +/* AD4080_REG_DEVICE_CONFIG Bit Definition */
> +#define AD4080_DEVICE_CONFIG_OPERATING_MODES_MSK		GENMASK(1, 0)
> +
> +/* AD4080_REG_TRANSFER_CONFIG Bit Definition */
> +#define AD4080_TRANSFER_CONFIG_KEEP_STREAM_LENGTH_VAL_MSK	BIT(2)
> +
> +/* AD4080_REG_INTERFACE_CONFIG_C Bit Definition */
> +#define AD4080_INTERFACE_CONFIG_C_STRICT_REG_ACCESS_MSK		BIT(5)
> +
> +/* AD4080_REG_ADC_DATA_INTF_CONFIG_A Bit Definition */
> +#define AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK	BIT(6)

If we are going to call these masks we should be using field prep to specify
the values.   Dropping the _MSK postfix is also fine for single bit values
ant which point just | them into register values looks less odd.

> +#define AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK		BIT(4)
> +#define AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK	BIT(2)
> +#define AD4080_ADC_DATA_INTF_CONFIG_A_DATA_INTF_MODE_MSK	BIT(0)
> +
> +/* AD4080_REG_ADC_DATA_INTF_CONFIG_B Bit Definition */
> +#define AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_CLK_CNT_MSK	GENMASK(7, 4)
> +#define AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_SELF_CLK_MODE_MSK	BIT(3)
> +#define AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_EN_MSK		BIT(0)
> +
> +/* AD4080_REG_ADC_DATA_INTF_CONFIG_C Bit Definition */
> +#define AD4080_ADC_DATA_INTF_CONFIG_C_LVDS_VOD_MSK		GENMASK(6, 4)
> +
> +/* AD4080_REG_PWR_CTRL Bit Definition */
> +#define AD4080_PWR_CTRL_ANA_DIG_LDO_PD_MSK			BIT(1)
> +#define AD4080_PWR_CTRL_INTF_LDO_PD_MSK				BIT(0)
> +
> +/* AD4080_REG_GPIO_CONFIG_A Bit Definition */
> +#define AD4080_GPIO_CONFIG_A_GPO_1_EN_MSK			BIT(1)
> +#define AD4080_GPIO_CONFIG_A_GPO_0_EN_MSK			BIT(0)
> +
> +/* AD4080_REG_GPIO_CONFIG_B Bit Definition */
> +#define AD4080_GPIO_CONFIG_B_GPIO_1_SEL				GENMASK(7, 4)
> +#define AD4080_GPIO_CONFIG_B_GPIO_0_SEL				GENMASK(3, 0)
> +
> +/* AD4080_REG_FIFO_CONFIG Bit Definition */
> +#define AD4080_FIFO_CONFIG_FIFO_MODE_MSK			GENMASK(1, 0)
> +
> +/* AD4080_REG_FILTER_CONFIG Bit Definition */
> +#define AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK			GENMASK(6, 3)
> +#define AD4080_FILTER_CONFIG_FILTER_SEL_MSK			GENMASK(1, 0)
> +
> +/* Miscellaneous Definitions */
> +#define AD4080_SPI_READ						BIT(7)
> +#define AD4080_CHIP_ID						GENMASK(2, 0)
> +
> +#define AD4080_MAX_SAMP_FREQ					40000000
> +#define AD4080_MIN_SAMP_FREQ					1250000
> +
> +enum ad4080_filter_type {
> +	FILTER_DISABLE,
> +	SINC_1,
> +	SINC_5,
> +	SINC_5_COMP
> +};
> +
> +static const unsigned int ad4080_scale_table[][2] = {
> +	{ 6000, 0},
space before } 

Check for other instances of this.

Also, why does this exist?  If you have other device support on the way where
this will change I don't mind having this table, but if not then just encode the
values inline.

> +};
> +
> +static const char *const ad4080_filter_type_iio_enum[] = {
> +	[FILTER_DISABLE]   = "disabled",
> +	[SINC_1]           = "sinc1",
> +	[SINC_5]           = "sinc5",
> +	[SINC_5_COMP]      = "sinc5_plus_compensation",
> +};
> +
> +static const int ad4080_dec_rate_iio_enum[] = {
> +	2, 4, 8, 16, 32, 64, 128, 256, 512, 1024,
> +};
> +
> +static const char * const ad4080_power_supplies[] = {
> +	"vdd33", "vdd11", "vddldo", "iovdd", "vrefin",
> +};
> +
> +struct ad4080_chip_info {
> +	const char *name;
> +	unsigned int product_id;
> +	int num_scales;
> +	const unsigned int (*scale_table)[2];
> +	const struct iio_chan_spec *channels;
> +	unsigned int num_channels;
> +};
> +
> +struct ad4080_state {
> +	struct spi_device		*spi;
> +	struct regmap			*regmap;
> +	struct clk			*clk;
> +	struct iio_backend		*back;
> +	const struct ad4080_chip_info	*info;
> +	/*
> +	 * Synchronize access to members the of driver state, and ensure
> +	 * atomicity of consecutive regmap operations.
> +	 */
> +	struct mutex			lock;
> +	unsigned int			num_lanes;
> +	unsigned int			dec_rate;
> +	enum ad4080_filter_type		filter_type;
> +	bool				lvds_cnv_en;
> +};

> +
> +static int ad4080_lvds_sync_write(struct ad4080_state *st)
> +{
> +	unsigned int timeout = 100;
> +	bool sync_en;
> +	int ret;
> +
> +	guard(mutex)(&st->lock);
> +	if (st->num_lanes == 1)
> +		ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +				   AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
Writing a reserved bit is unusual... (though I see it defaults to on)

Maybe you are better off just not touching that bit in the driver explicitly and instead
use.
		ret = regmap_set_bits(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
				      AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK);   

> +				   AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK);
> +	else
> +		ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +				   AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> +				   AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK |
> +				   AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);

The lanes field doesn't seem to have anything directly to do with sync.
So should that perhaps be a separate regmap_set_bits() call before this function?
There is already some num_lanes related code in ad4080_setup().

> +	if (ret)
> +		return ret;
> +
> +	ret = iio_backend_data_alignment_enable(st->back);
> +	if (ret)
> +		return ret;
> +
> +	do {
> +		ret = iio_backend_sync_status_get(st->back, &sync_en);
> +		if (ret)
> +			return ret;
> +
> +		if (!sync_en)
> +			dev_dbg(&st->spi->dev, "Not Locked: Running Bit Slip\n");
> +
> +		fsleep(500);
> +	} while (--timeout && !sync_en);
> +
> +	if (timeout) {
> +		dev_info(&st->spi->dev, "Success: Pattern correct and Locked!\n");
dev_dbg() on success tings.

> +		if (st->num_lanes == 1)
> +			return regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					    AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK);

These triggered comment above on using FIELD_PREP() for things called _MSK even
if they are single bit. The _MSK naming normally implies they are multi bit making this
code look somewhat odd.

If you were to use regmap_clear_bits(st->regmap, AD4080_REG_ADC_DATA_INF_CONFIG_A,
				     AD4080_DATA_INF_CONFIG_A_INTF_CHK_EN_MSK);

then you don't need to make it depend on the number of lanes.


> +		else
> +			return regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					    AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> +					    AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
> +	} else {
> +		dev_info(&st->spi->dev, "LVDS Sync Timeout.\n");

dev_err() given it's a failure is probably appropriate.

> +		if (st->num_lanes == 1) {
> +			ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					   AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK);
> +			if (ret)
> +				return ret;
> +		} else {
> +			ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					   AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> +					   AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
Here you can use regmap_clear_bits() as well.  That will also reduce the amount of duplicate
code so there is less reason to try and share it.

> +			if (ret)
> +				return ret;

Not clear to me we should return this error rather than -ETIMEDOUT in these cases
(as that was the first thing to go wrong).

> +		}
> +
> +		return -ETIMEDOUT;
> +	}
> +}

> +static const struct iio_chan_spec ad4080_channels[] = {
> +	{
> +		.type = IIO_VOLTAGE,
> +		.indexed = 1,
> +		.channel = 0,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +				BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +		.info_mask_shared_by_all_available =
> +				BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +		.ext_info = ad4080_ext_info,
> +		.scan_index = 0,
> +		.scan_type = {
> +			.sign = 's',
> +			.realbits = 20,
> +			.storagebits = 32,
> +			.shift = 0,

Trivial but we normally don't bother specifying shift explicitly if it is 0
as that's (kind of) the obvious default value.

> +		},
> +	}
> +};

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

[Jonathan Cameron]

On Fri, 25 Apr 2025 14:25:28 +0300
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> Add backend support for digital filter type selection.
> 
> This setting can be adjusted within the IP cores interfacing devices.
> 
> The IP core can be configured based on the state of the actual
> digital filter configuration of the part.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
All these backend things look fine to me. However, as for all backend
stuff I'm not the expert so will be looking for other reviews on these.

+CC Nuno

> ---
> changes in v3:
>  - update function to set the actual filter type instead of just enable/disable.
>  drivers/iio/industrialio-backend.c | 15 +++++++++++++++
>  include/linux/iio/backend.h        | 13 +++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
> index d4ad36f54090..2d28eabb1607 100644
> --- a/drivers/iio/industrialio-backend.c
> +++ b/drivers/iio/industrialio-backend.c
> @@ -778,6 +778,21 @@ static int __devm_iio_backend_get(struct device *dev, struct iio_backend *back)
>  	return 0;
>  }
>  
> +/**
> + * iio_backend_filter_type_set - Set filter type
> + * @back: Backend device
> + * @type: Filter type.
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_filter_type_set(struct iio_backend *back,
> +				enum iio_backend_filter_type type)
> +{
> +	return iio_backend_op_call(back, filter_type_set, type);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_filter_type_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 e45b7dfbec35..5526800f5d4a 100644
> --- a/include/linux/iio/backend.h
> +++ b/include/linux/iio/backend.h
> @@ -76,6 +76,14 @@ enum iio_backend_interface_type {
>  	IIO_BACKEND_INTERFACE_MAX
>  };
>  
> +enum iio_backend_filter_type {
> +	IIO_BACKEND_FILTER_TYPE_DISABLED,
> +	IIO_BACKEND_FILTER_TYPE_SINC1,
> +	IIO_BACKEND_FILTER_TYPE_SINC5,
> +	IIO_BACKEND_FILTER_TYPE_SINC5_PLUS_COMP,
> +	IIO_BACKEND_FILTER_TYPE_MAX
> +};
> +
>  /**
>   * struct iio_backend_ops - operations structure for an iio_backend
>   * @enable: Enable backend.
> @@ -100,6 +108,7 @@ 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_type_set: Set filter type.
>   * @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 +159,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_type_set)(struct iio_backend *back,
> +			       enum iio_backend_filter_type type);
>  	int (*ddr_enable)(struct iio_backend *back);
>  	int (*ddr_disable)(struct iio_backend *back);
>  	int (*data_stream_enable)(struct iio_backend *back);
> @@ -190,6 +201,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_type_set(struct iio_backend *back,
> +				enum iio_backend_filter_type type);
>  int iio_backend_ddr_enable(struct iio_backend *back);
>  int iio_backend_ddr_disable(struct iio_backend *back);
>  int iio_backend_data_stream_enable(struct iio_backend *back);

Re: [PATCH v3 11/11] iio: adc: ad4080: add driver support

[kernel test robot]

Hi Antoniu,

kernel test robot noticed the following build warnings:

[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v6.15-rc3 next-20250424]
[cannot apply to jic23-iio/togreg]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Antoniu-Miclaus/iio-backend-add-support-for-filter-config/20250425-192951
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20250425112538.59792-12-antoniu.miclaus%40analog.com
patch subject: [PATCH v3 11/11] iio: adc: ad4080: add driver support
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20250427/202504270010.w3ZsLDZR-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250427/202504270010.w3ZsLDZR-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504270010.w3ZsLDZR-lkp@intel.com/

All warnings (new ones prefixed by >>, old ones prefixed by <<):

WARNING: modpost: missing MODULE_DESCRIPTION() in lib/tests/slub_kunit.o
>> WARNING: modpost: module ad4080 uses symbol iio_backend_filter_type_set from namespace IIO_BACKEND, but does not import it.
>> WARNING: modpost: module ad4080 uses symbol iio_backend_data_alignment_enable from namespace IIO_BACKEND, but does not import it.
>> WARNING: modpost: module ad4080 uses symbol iio_backend_sync_status_get from namespace IIO_BACKEND, but does not import it.
>> WARNING: modpost: module ad4080 uses symbol iio_backend_num_lanes_set from namespace IIO_BACKEND, but does not import it.
>> WARNING: modpost: module ad4080 uses symbol devm_iio_backend_get from namespace IIO_BACKEND, but does not import it.
>> WARNING: modpost: module ad4080 uses symbol devm_iio_backend_request_buffer from namespace IIO_BACKEND, but does not import it.
>> WARNING: modpost: module ad4080 uses symbol devm_iio_backend_enable from namespace IIO_BACKEND, but does not import it.

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

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

[Nuno Sá]

On Fri, 2025-04-25 at 14:25 +0300, Antoniu Miclaus wrote:
> Add backend support for digital filter type selection.
> 
> This setting can be adjusted within the IP cores interfacing devices.
> 
> The IP core can be configured based on the state of the actual
> digital filter configuration of the part.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---

This one LGTM:

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

> changes in v3:
>  - update function to set the actual filter type instead of just
> enable/disable.
>  drivers/iio/industrialio-backend.c | 15 +++++++++++++++
>  include/linux/iio/backend.h        | 13 +++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-
> backend.c
> index d4ad36f54090..2d28eabb1607 100644
> --- a/drivers/iio/industrialio-backend.c
> +++ b/drivers/iio/industrialio-backend.c
> @@ -778,6 +778,21 @@ static int __devm_iio_backend_get(struct device *dev,
> struct iio_backend *back)
>  	return 0;
>  }
>  
> +/**
> + * iio_backend_filter_type_set - Set filter type
> + * @back: Backend device
> + * @type: Filter type.
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_filter_type_set(struct iio_backend *back,
> +				enum iio_backend_filter_type type)
> +{
> +	return iio_backend_op_call(back, filter_type_set, type);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_filter_type_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 e45b7dfbec35..5526800f5d4a 100644
> --- a/include/linux/iio/backend.h
> +++ b/include/linux/iio/backend.h
> @@ -76,6 +76,14 @@ enum iio_backend_interface_type {
>  	IIO_BACKEND_INTERFACE_MAX
>  };
>  
> +enum iio_backend_filter_type {
> +	IIO_BACKEND_FILTER_TYPE_DISABLED,
> +	IIO_BACKEND_FILTER_TYPE_SINC1,
> +	IIO_BACKEND_FILTER_TYPE_SINC5,
> +	IIO_BACKEND_FILTER_TYPE_SINC5_PLUS_COMP,
> +	IIO_BACKEND_FILTER_TYPE_MAX
> +};
> +
>  /**
>   * struct iio_backend_ops - operations structure for an iio_backend
>   * @enable: Enable backend.
> @@ -100,6 +108,7 @@ 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_type_set: Set filter type.
>   * @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 +159,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_type_set)(struct iio_backend *back,
> +			       enum iio_backend_filter_type type);
>  	int (*ddr_enable)(struct iio_backend *back);
>  	int (*ddr_disable)(struct iio_backend *back);
>  	int (*data_stream_enable)(struct iio_backend *back);
> @@ -190,6 +201,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_type_set(struct iio_backend *back,
> +				enum iio_backend_filter_type type);
>  int iio_backend_ddr_enable(struct iio_backend *back);
>  int iio_backend_ddr_disable(struct iio_backend *back);
>  int iio_backend_data_stream_enable(struct iio_backend *back);

Re: [PATCH v3 06/11] iio: adc: adi-axi-adc: add filter type config

[Nuno Sá]

On Fri, 2025-04-25 at 14:25 +0300, Antoniu Miclaus wrote:
> Add support for enabling/disabling filter based on the filter type
> provided.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---

You should either mention in the commit message that a new compatible (and thus
iio_backend_ops) is being added or split this into two patches. This is simple
enough that I don't really mind being in the same patch but still...

With the above (and a nit below):

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

> changes in v3:
>  - update the function to adapt to the backend interface.
>  drivers/iio/adc/adi-axi-adc.c | 38 +++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
> index 61ab7dce43be..2a3a6c3f5e59 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,19 @@ static int axi_adc_ad485x_oversampling_ratio_set(struct
> iio_backend *back,
>  	}
>  }
>  
> +static int axi_adc_ad408x_filter_type_set(struct iio_backend *back,
> +					  enum iio_backend_filter_type type)
> +{
> +	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
> +
> +	if (type)

We could check for a max value and return -EINVAL...

> +		return regmap_set_bits(st->regmap, ADI_AXI_ADC_REG_CNTRL_3,
> +				       AXI_AD408X_CNTRL_3_FILTER_EN_MSK);
> +
> +	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 +596,24 @@ 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_type_set = axi_adc_ad408x_filter_type_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),
> +};
> +
> +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 +729,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 */ }

Re: [PATCH v3 04/11] iio: backend: add support for number of lanes

[Nuno Sá]

On Fri, 2025-04-25 at 14:25 +0300, Antoniu Miclaus wrote:
> Add iio backend support for number of lanes to be enabled.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

> no changes in v3.
>  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 f7c8167a248d..1063935cd808 100644
> --- a/drivers/iio/industrialio-backend.c
> +++ b/drivers/iio/industrialio-backend.c
> @@ -833,6 +833,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 e93d1a98e066..c8bcfe96c542 100644
> --- a/include/linux/iio/backend.h
> +++ b/include/linux/iio/backend.h
> @@ -112,6 +112,7 @@ enum iio_backend_filter_type {
>   * @data_alignment_enable: Enable sync process.
>   * @data_alignment_disable: Disable sync process.
>   * @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.
> @@ -167,6 +168,7 @@ struct iio_backend_ops {
>  	int (*data_alignment_enable)(struct iio_backend *back);
>  	int (*data_alignment_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);
> @@ -212,6 +214,7 @@ int iio_backend_filter_type_set(struct iio_backend *back,
>  int iio_backend_data_alignment_enable(struct iio_backend *back);
>  int iio_backend_data_alignment_disable(struct iio_backend *back);
>  int iio_backend_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);

Re: [PATCH v3 09/11] iio: adc: adi-axi-adc: add num lanes support

[Nuno Sá]

On Fri, 2025-04-25 at 14:25 +0300, Antoniu Miclaus wrote:
> Add support for setting the number of lanes enabled.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---

Just one small comment below... With that addressed:

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

> no changes in v3.
>  drivers/iio/adc/adi-axi-adc.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
> index bf0155830d87..8ff781ab5ec3 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_SYNC_MSK		BIT(3)
>  #define    ADI_AXI_ADC_CTRL_DDR_EDGESEL_MASK	BIT(1)
>  
> @@ -451,6 +452,19 @@ static int axi_adc_sync_status_get(struct iio_backend
> *back, bool *sync_en)
>  	return 0;
>  }
>  
> +static int axi_adc_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)
>  {
> @@ -601,6 +615,7 @@ static const struct iio_backend_ops adi_axi_adc_ops = {
>  	.chan_status = axi_adc_chan_status,
>  	.interface_type_get = axi_adc_interface_type_get,
>  	.sync_status_get = axi_adc_sync_status_get,
> +	.num_lanes_set = axi_adc_num_lanes_set,

Not sure if we should set this. Although it might be in the regular/default
register map, I suppose that this not a generic feature all axi-adc backends
inherit from the base design... So, if wrongly used, I guess it would result in
a no-op on the backend side.

>  	.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),
>  };
> @@ -646,6 +661,7 @@ static const struct iio_backend_ops adi_ad408x_ops = {
>  	.data_alignment_enable = axi_adc_sync_enable,
>  	.data_alignment_disable = axi_adc_sync_disable,
>  	.sync_status_get = axi_adc_sync_status_get,
> +	.num_lanes_set = axi_adc_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),
>  };

Re: [PATCH v3 07/11] iio: adc: adi-axi-adc: add sync enable/disable

[Nuno Sá]

On Fri, 2025-04-25 at 14:25 +0300, Antoniu Miclaus wrote:
> Add support for enabling/disabling the sync process used for data
> capture alignment.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---

Some comments from me... I'm also still not convinced this API can't be merged
with the sync get. More comments on the frontend driver patch.

> changes in v3:
>  - update the function to match the new backend interface.
>  drivers/iio/adc/adi-axi-adc.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
> index 2a3a6c3f5e59..9947be059f98 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_SYNC_MSK		BIT(3)
>  #define    ADI_AXI_ADC_CTRL_DDR_EDGESEL_MASK	BIT(1)
>  
>  #define ADI_AXI_ADC_REG_CNTRL_3			0x004c
> @@ -416,6 +417,22 @@ static int axi_adc_ad408x_filter_type_set(struct
> iio_backend *back,
>  				 AXI_AD408X_CNTRL_3_FILTER_EN_MSK);
>  }
>  
> +static int axi_adc_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_CTRL,
> +			       AXI_AD408X_CTRL_SYNC_MSK);
> +}
> +
> +static int axi_adc_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_CTRL,
> +				 AXI_AD408X_CTRL_SYNC_MSK);
> +}
> +
>  static struct iio_buffer *axi_adc_request_buffer(struct iio_backend *back,
>  						 struct iio_dev *indio_dev)
>  {
> @@ -559,6 +576,8 @@ static const struct iio_backend_ops adi_axi_adc_ops = {
>  	.request_buffer = axi_adc_request_buffer,
>  	.free_buffer = axi_adc_free_buffer,
>  	.data_sample_trigger = axi_adc_data_sample_trigger,
> +	.data_alignment_enable = axi_adc_sync_enable,
> +	.data_alignment_disable = axi_adc_sync_disable,

Same comment as for the number of lanes op...
>  	.iodelay_set = axi_adc_iodelays_set,
>  	.test_pattern_set = axi_adc_test_pattern_set,
>  	.chan_status = axi_adc_chan_status,
> @@ -605,6 +624,8 @@ static const struct iio_backend_ops adi_ad408x_ops = {
>  	.free_buffer = axi_adc_free_buffer,
>  	.data_sample_trigger = axi_adc_data_sample_trigger,
>  	.filter_type_set = axi_adc_ad408x_filter_type_set,
> +	.data_alignment_enable = axi_adc_sync_enable,
> +	.data_alignment_disable = axi_adc_sync_disable,

Are we using the disable op at all?

>  	.debugfs_reg_access = iio_backend_debugfs_ptr(axi_adc_reg_access),
>  	.debugfs_print_chan_status =
> iio_backend_debugfs_ptr(axi_adc_debugfs_print_chan_status),
>  };

Re: [PATCH v3 11/11] iio: adc: ad4080: add driver support

[Nuno Sá]

Hi Antoniu,

On Fri, 2025-04-25 at 14:25 +0300, Antoniu Miclaus wrote:
> Add support for AD4080 high-speed, low noise, low distortion,
> 20-bit, Easy Drive, successive approximation register (SAR)
> analog-to-digital converter (ADC).
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> changes in v3:
>  - name the defines to make associacion with register name.
>  - drop redundant defines.
>  - add trailing comma when needed.
>  - drop explicit masking and use field_prep instead
>  - add fsleep during sync process.
>  - do not wrap where 80 chars is not exceeded.
>  - use space for { }
>  - drop channel definition macro
>  - return dev_info on chip id mismatch.
>  - flip expression to `if (!st->lvds_cnv_en)`
>  - rework num_lanes property parse.
>  - update the driver based on the new edits on the backend interface related
> to
>    this part and the last disscussions in v2.
>  drivers/iio/adc/Kconfig  |  14 +
>  drivers/iio/adc/Makefile |   1 +
>  drivers/iio/adc/ad4080.c | 618 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 633 insertions(+)
>  create mode 100644 drivers/iio/adc/ad4080.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 27413516216c..17df328f5322 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -47,6 +47,20 @@ config AD4030
>  	  To compile this driver as a module, choose M here: the module will
> be
>  	  called ad4030.
>  
> +config AD4080
> +	tristate "Analog Devices AD4080 high speed ADC"
> +	depends on SPI
> +	select REGMAP_SPI
> +	select IIO_BACKEND
> +	help
> +	  Say yes here to build support for Analog Devices AD4080
> +	  high speed, low noise, low distortion, 20-bit, Easy Drive,
> +	  successive approximation register (SAR) analog-to-digital
> +	  converter (ADC). Supports iio_backended devices for AD4080.
> +
> +	  To compile this driver as a module, choose M here: the module will
> be
> +	  called ad4080.
> +
>  config AD4130
>  	tristate "Analog Device AD4130 ADC Driver"
>  	depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 9f26d5eca822..e6efed5b4e7a 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
>  obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
>  obj-$(CONFIG_AD4000) += ad4000.o
>  obj-$(CONFIG_AD4030) += ad4030.o
> +obj-$(CONFIG_AD4080) += ad4080.o
>  obj-$(CONFIG_AD4130) += ad4130.o
>  obj-$(CONFIG_AD4695) += ad4695.o
>  obj-$(CONFIG_AD4851) += ad4851.o
> diff --git a/drivers/iio/adc/ad4080.c b/drivers/iio/adc/ad4080.c
> new file mode 100644
> index 000000000000..b51893253941
> --- /dev/null
> +++ b/drivers/iio/adc/ad4080.c
> @@ -0,0 +1,618 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Analog Devices AD4080 SPI ADC driver
> + *
> + * Copyright 2025 Analog Devices Inc.
> + */
> +
> 

...

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

Kind of a nit comment (and likely personal preference) but I would rather get
the clock rate during probe. Most of the times, the clock never changes so I
rather prefer doing the above when actually needed. Or is this one those cases
were it actually changes?

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

Why not handle this in 'default'?

> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		return ad4080_set_dec_rate(indio_dev, chan, val);

IIRC, you mentioned at some point that after changing the sampling frequency we
should align the interface again. Isn't this setting affecting it?

> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ad4080_lvds_sync_write(struct ad4080_state *st)
> +{
> +	unsigned int timeout = 100;
> +	bool sync_en;
> +	int ret;
> +
> +	guard(mutex)(&st->lock);
> +	if (st->num_lanes == 1)
> +		ret = regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +				  
> AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> +				  
> AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK);
> +	else
> +		ret = regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +				  
> AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> +				  
> AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK |
> +				  
> AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
> +	if (ret)
> +		return ret;
> +
> +	ret = iio_backend_data_alignment_enable(st->back);
> +	if (ret)
> +		return ret;

So it looks like you never disable the internal alignment. Is that on purpose? I
also failed to reply to your comments in the previous version so I'll bring them
back:

"In this particular case, yes, one backend would fit for the sync process. But
taking into account that these two features are part also from the common core
of the AXI ADC, in other cases they might be used separately."

Not sure if that is true... I guess these are in the default register map but
are they always implemented for all the designs? They might just be no-ops for
some designs. Example, I do not think bitslip is always implemented. But even in
that case, the goal here is to align the interface for data sampling. So,
really, something like:

iio_backend_interface_data_align(st->back, u32 timeout);

looks fairly generic to me (given that the process is for the complete interface
at once. IOW, there's no specific timing points or stuff like that that we need
to probe independently)... So doing the polling on the backend side seems
reasonable to me (and there you can use regmap_poll APIs).

And as Jonathan puts it, this is all in kernel APIs so we can easily change it
afterwards :)

"The CNV signal is mainly used for sampling (an input pin according to the
datasheet - 
conversion is initiated on the rising edge of the convert signal).
We use it only for determining the sampling frequency."

Ok, from your previous versions I got the impression this pin could also be used
for aligning the interface.

> +
> +	do {
> +		ret = iio_backend_sync_status_get(st->back, &sync_en);
> +		if (ret)
> +			return ret;
> +
> +		if (!sync_en)
> +			dev_dbg(&st->spi->dev, "Not Locked: Running Bit
> Slip\n");
> +
> +		fsleep(500);
> +	} while (--timeout && !sync_en);
> +
> +	if (timeout) {
> +		dev_info(&st->spi->dev, "Success: Pattern correct and
> Locked!\n");
> +		if (st->num_lanes == 1)
> +			return regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					   
> AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK);
> +		else
> +			return regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					   
> AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> +					   
> AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);

redundant else

> +	} 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_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK);
> +			if (ret)
> +				return ret;
> +		} else {
> +			ret = regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					  
> AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> +					  
> AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
> +			if (ret)
> +				return ret;
> +		}
> +
> +		return -ETIMEDOUT;
> +	}
> +}
> 

Re: [PATCH v3 07/11] iio: adc: adi-axi-adc: add sync enable/disable

[David Lechner]

On 4/25/25 6:25 AM, Antoniu Miclaus wrote:
> Add support for enabling/disabling the sync process used for data
> capture alignment.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> changes in v3:
>  - update the function to match the new backend interface.
>  drivers/iio/adc/adi-axi-adc.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
> index 2a3a6c3f5e59..9947be059f98 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_SYNC_MSK		BIT(3)

If this bit applies to AXI ADC in general, then it shouldn't have AD408X in the
name. Or, if this is really specific to AD408X, then...

>  #define    ADI_AXI_ADC_CTRL_DDR_EDGESEL_MASK	BIT(1)
>  
>  #define ADI_AXI_ADC_REG_CNTRL_3			0x004c
> @@ -416,6 +417,22 @@ static int axi_adc_ad408x_filter_type_set(struct iio_backend *back,
>  				 AXI_AD408X_CNTRL_3_FILTER_EN_MSK);
>  }
>  
> +static int axi_adc_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_CTRL,
> +			       AXI_AD408X_CTRL_SYNC_MSK);
> +}
> +
> +static int axi_adc_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_CTRL,
> +				 AXI_AD408X_CTRL_SYNC_MSK);
> +}

... these functions should have ad408x in the name to make that clear and ...

> +
>  static struct iio_buffer *axi_adc_request_buffer(struct iio_backend *back,
>  						 struct iio_dev *indio_dev)
>  {
> @@ -559,6 +576,8 @@ static const struct iio_backend_ops adi_axi_adc_ops = {
>  	.request_buffer = axi_adc_request_buffer,
>  	.free_buffer = axi_adc_free_buffer,
>  	.data_sample_trigger = axi_adc_data_sample_trigger,
> +	.data_alignment_enable = axi_adc_sync_enable,
> +	.data_alignment_disable = axi_adc_sync_disable,

... we shouldn't be adding them to the generic core ops.

>  	.iodelay_set = axi_adc_iodelays_set,
>  	.test_pattern_set = axi_adc_test_pattern_set,
>  	.chan_status = axi_adc_chan_status,
> @@ -605,6 +624,8 @@ static const struct iio_backend_ops adi_ad408x_ops = {
>  	.free_buffer = axi_adc_free_buffer,
>  	.data_sample_trigger = axi_adc_data_sample_trigger,
>  	.filter_type_set = axi_adc_ad408x_filter_type_set,
> +	.data_alignment_enable = axi_adc_sync_enable,
> +	.data_alignment_disable = axi_adc_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),
>  };

Re: [PATCH v3 09/11] iio: adc: adi-axi-adc: add num lanes support

[David Lechner]

On 4/25/25 6:25 AM, Antoniu Miclaus wrote:
> Add support for setting the number of lanes enabled.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> no changes in v3.
>  drivers/iio/adc/adi-axi-adc.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
> index bf0155830d87..8ff781ab5ec3 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)

Same comment applies here. It looks like this is common to all cores, so no
AD408X in the name please.

>  #define    AXI_AD408X_CTRL_SYNC_MSK		BIT(3)
>  #define    ADI_AXI_ADC_CTRL_DDR_EDGESEL_MASK	BIT(1)
>  
> @@ -451,6 +452,19 @@ static int axi_adc_sync_status_get(struct iio_backend *back, bool *sync_en)
>  	return 0;
>  }

Re: [PATCH v3 11/11] iio: adc: ad4080: add driver support

[David Lechner]

On 4/25/25 6:25 AM, 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>
> ---

...

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

Should be grouped with the others.

> +
> +/* Register Definition */

...

> +
> +enum ad4080_filter_type {
> +	FILTER_DISABLE,
> +	SINC_1,
> +	SINC_5,
> +	SINC_5_COMP
> +};
> +
> +static const unsigned int ad4080_scale_table[][2] = {
> +	{ 6000, 0},
> +};
> +
> +static const char *const ad4080_filter_type_iio_enum[] = {

So far, only "sinc5" is documented in Documentation/ABI/testing/sysfs-bus-iio
so we will to add the rest there.

> +	[FILTER_DISABLE]   = "disabled",

IMHO, "disabled" doesn't make sense as a "type". I would call it "none" instead.

> +	[SINC_1]           = "sinc1",
> +	[SINC_5]           = "sinc5",
> +	[SINC_5_COMP]      = "sinc5_plus_compensation",

To follow the existing naming patterns it would make sense to call this one:

"sinc5+compensation" - Sinc5 + ???

Or even more generic like the existing sinc3+pfX options:

"sinc5+pf1" - Sinc5 + device specific Post Filter 1.

> +};
> +
> +static const int ad4080_dec_rate_iio_enum[] = {
> +	2, 4, 8, 16, 32, 64, 128, 256, 512, 1024,
> +};

The datasheet says that 512 and 1024 only apply to sinc1 and that for
sinc5+compensation, the values are N * 2. And I would assume with the filter
disabled, the only option would be 1.

So I think we need 4 different arrays for this with the selection depending
on the filter type.

> +
> +static const char * const ad4080_power_supplies[] = {
> +	"vdd33", "vdd11", "vddldo", "iovdd", "vrefin",
> +};

From the datasheet, it sounds like VDDLDO is tecnically optional. Given the
way regulators work in Linux though, I guess this is OK for simplicity.

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

I guess this is preparing the driver to support more than one chip?

> +
> +struct ad4080_state {
> +	struct spi_device		*spi;

It looks like this is only ever used to get &spi->dev. We could drop this and
get dev from regmap instead.

> +	struct regmap			*regmap;
> +	struct clk			*clk;
> +	struct iio_backend		*back;
> +	const struct ad4080_chip_info	*info;
> +	/*
> +	 * Synchronize access to members the of driver state, and ensure
> +	 * atomicity of consecutive regmap operations.
> +	 */
> +	struct mutex			lock;
> +	unsigned int			num_lanes;
> +	unsigned int			dec_rate;
> +	enum ad4080_filter_type		filter_type;
> +	bool				lvds_cnv_en;
> +};
> +
> +static const struct regmap_config ad4080_regmap_config = {
> +	.reg_bits = 16,
> +	.val_bits = 8,
> +	.read_flag_mask = BIT(7),
> +	.max_register = 0x29,
> +};
> +
> +static int ad4080_reg_access(struct iio_dev *indio_dev, unsigned int reg,
> +			     unsigned int writeval, unsigned int *readval)
> +{
> +	struct ad4080_state *st = iio_priv(indio_dev);
> +

Missing guard(mutex)(&st->lock); ?

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

Seems like this could be simplifed by using IIO_VAL_FRACTIONAL_LOG2 instead.

> +}
> +
> +static unsigned int ad4080_get_dec_rate(struct iio_dev *dev,
> +					const struct iio_chan_spec *chan)
> +{
> +	struct ad4080_state *st = iio_priv(dev);
> +	int ret;
> +	unsigned int data;
> +

Missing guard(mutex)(&st->lock); ?

> +	ret = regmap_read(st->regmap, AD4080_REG_FILTER_CONFIG, &data);
> +	if (ret)
> +		return ret;
> +
> +	return (1 << (FIELD_GET(AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK, data) + 1));

nit: doen't need outermost ().

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

Don't we need to check for < 2 as well?

> +	if (st->filter_type >= SINC_5 && mode >= 512)
> +		return -EINVAL;
> +
> +	guard(mutex)(&st->lock);
> +	ret = regmap_update_bits(st->regmap, AD4080_REG_FILTER_CONFIG,
> +				 AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK,
> +				 FIELD_PREP(AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK,
> +					    (ilog2(mode) - 1)));

Otherwise ilog2(mode) - 1 could be < 0.

> +	if (ret)
> +		return ret;
> +
> +	st->dec_rate = mode;

This saves the value the user entered, not what the hardware is actually doing.
It should be saving the power of 2 value instead.

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

As a concequence of the above, this will return incorrect information if the
user didn't enter an exact value.

> +		if (st->filter_type)
> +			*val = DIV_ROUND_CLOSEST(clk_get_rate(st->clk), dec_rate);
> +		else
> +			*val = clk_get_rate(st->clk);
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		*val = ad4080_get_dec_rate(indio_dev, chan);
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ad4080_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int val, int val2, long mask)
> +{
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		return -EINVAL;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		return -EINVAL;

Can leave these 2 out and just let them fall through to the default.

> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		return ad4080_set_dec_rate(indio_dev, chan, val);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ad4080_lvds_sync_write(struct ad4080_state *st)
> +{
> +	unsigned int timeout = 100;
> +	bool sync_en;
> +	int ret;

nit: some comments in this function would be helpful to readers not familiar
with the part. 

> +
> +	guard(mutex)(&st->lock);
> +	if (st->num_lanes == 1)
> +		ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +				   AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> +				   AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK);
> +	else
> +		ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +				   AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> +				   AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK |
> +				   AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
> +	if (ret)
> +		return ret;
> +
> +	ret = iio_backend_data_alignment_enable(st->back);
> +	if (ret)
> +		return ret;
> +
> +	do {
> +		ret = iio_backend_sync_status_get(st->back, &sync_en);
> +		if (ret)
> +			return ret;
> +
> +		if (!sync_en)
> +			dev_dbg(&st->spi->dev, "Not Locked: Running Bit Slip\n");
> +
> +		fsleep(500);
> +	} while (--timeout && !sync_en);
> +
> +	if (timeout) {
> +		dev_info(&st->spi->dev, "Success: Pattern correct and Locked!\n");
> +		if (st->num_lanes == 1)
> +			return regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					    AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK);
> +		else
> +			return regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					    AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> +					    AD4080_ADC_DATA_INTF_CONFIG_A_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_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK);
> +			if (ret)
> +				return ret;
> +		} else {
> +			ret = regmap_write(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> +					   AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> +					   AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
> +			if (ret)
> +				return ret;
> +		}
> +
> +		return -ETIMEDOUT;
> +	}
> +}
> +
> +static ssize_t ad4080_get_filter_type(struct iio_dev *dev,
> +				      const struct iio_chan_spec *chan)
> +{
> +	struct ad4080_state *st = iio_priv(dev);
> +	unsigned int data;
> +	int ret;
> +

Missing guard(mutex)(&st->lock); ?

> +	ret = regmap_read(st->regmap, AD4080_REG_FILTER_CONFIG, &data);
> +	if (ret)
> +		return ret;
> +
> +	return FIELD_GET(AD4080_FILTER_CONFIG_FILTER_SEL_MSK, data);
> +}
> +

...

> +static struct iio_chan_spec_ext_info ad4080_ext_info[] = {
> +	IIO_ENUM("filter_type", IIO_SHARED_BY_ALL, &ad4080_filter_type_enum),
> +	IIO_ENUM_AVAILABLE("filter_type", IIO_SHARED_BY_ALL,
> +			   &ad4080_filter_type_enum),
> +	{ }
> +};
> +
> +static const struct iio_chan_spec ad4080_channels[] = {

Array with one element doesn't make sense. It can just be a single struct.

> +	{
> +		.type = IIO_VOLTAGE,
> +		.indexed = 1,
> +		.channel = 0,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +				BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +		.info_mask_shared_by_all_available =
> +				BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +		.ext_info = ad4080_ext_info,
> +		.scan_index = 0,
> +		.scan_type = {
> +			.sign = 's',
> +			.realbits = 20,
> +			.storagebits = 32,
> +			.shift = 0,
> +		},
> +	}
> +};
> +
> +static const struct ad4080_chip_info ad4080_chip_info = {
> +	.name = "AD4080",
> +	.product_id = AD4080_CHIP_ID,
> +	.scale_table = ad4080_scale_table,
> +	.num_scales = ARRAY_SIZE(ad4080_scale_table),
> +	.num_channels = 1,
> +	.channels = ad4080_channels,
> +};
> +
> +static int ad4080_setup(struct iio_dev *indio_dev)
> +{
> +	struct ad4080_state *st = iio_priv(indio_dev);
> +	unsigned int id;
> +	int ret;
> +
> +	ret = regmap_write(st->regmap, AD4080_REG_INTERFACE_CONFIG_A,
> +			   AD4080_INTERFACE_CONFIG_A_SW_RESET_MSK);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, AD4080_REG_INTERFACE_CONFIG_A,
> +			   AD4080_INTERFACE_CONFIG_A_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_info(&st->spi->dev, "Unrecognized CHIP_ID 0x%X\n", id);
> +
> +	ret = regmap_set_bits(st->regmap, AD4080_REG_GPIO_CONFIG_A,
> +			      AD4080_GPIO_CONFIG_A_GPO_1_EN_MSK);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, AD4080_REG_GPIO_CONFIG_B,
> +			   FIELD_PREP(AD4080_GPIO_CONFIG_B_GPIO_1_SEL, 3));
> +	if (ret)
> +		return ret;
> +
> +	ret = iio_backend_num_lanes_set(st->back, st->num_lanes);
> +	if (ret)
> +		return ret;
> +
> +	if (!st->lvds_cnv_en)
> +		return 0;
> +
> +	if (st->num_lanes) {

Since the defualt is st->num_lanes = 1, it seems like this would always be
true, so we can leave out the "if".

> +		ret = regmap_update_bits(st->regmap,
> +					 AD4080_REG_ADC_DATA_INTF_CONFIG_B,
> +					 AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_CLK_CNT_MSK,
> +					 FIELD_PREP(AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_CLK_CNT_MSK,
> +						    7));
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = regmap_set_bits(st->regmap,
> +			      AD4080_REG_ADC_DATA_INTF_CONFIG_B,
> +			      AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_EN_MSK);
> +	if (ret)
> +		return ret;
> +
> +	return ad4080_lvds_sync_write(st);
> +}
> +
> +static void ad4080_properties_parse(struct ad4080_state *st)
> +{
> +	st->lvds_cnv_en = device_property_read_bool(&st->spi->dev,
> +						    "adi,lvds-cnv-enable");
> +
> +	st->num_lanes = 1;
> +	device_property_read_u32(&st->spi->dev, "adi,num_lanes", &st->num_lanes);

nit: odd that other property names use "-" but this one uses "_". Typical would
be "adi,num-lanes".

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

reduandant assignement

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

There is not IIO_CHAN_INFO_RAW (or _PROCESSED), so INDIO_DIRECT_MODE does not
apply.

> +
> +	ad4080_properties_parse(st);
> +
> +	st->clk = devm_clk_get_enabled(&spi->dev, "cnv");
> +	if (IS_ERR(st->clk))
> +		return PTR_ERR(st->clk);
> +
> +	st->back = devm_iio_backend_get(dev, NULL);
> +	if (IS_ERR(st->back))
> +		return PTR_ERR(st->back);
> +
> +	ret = devm_iio_backend_request_buffer(dev, st->back, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_iio_backend_enable(dev, st->back);
> +	if (ret)
> +		return ret;
> +
> +	ret = ad4080_setup(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	return devm_iio_device_register(&spi->dev, indio_dev);
> +}

RE: [PATCH v3 11/11] iio: adc: ad4080: add driver support

[Miclaus, Antoniu]

...
> > +	unsigned int num_channels;
> > +};
> 
> I guess this is preparing the driver to support more than one chip?
> 
Yes. It is stated also in the cover letter.
> > +
> > +struct ad4080_state {
> > +	struct spi_device		*spi;
> 
> It looks like this is only ever used to get &spi->dev. We could drop this and
> get dev from regmap instead.
How can I get the dev from regmap?
> > +	struct regmap			*regmap;
> > +	struct clk			*clk;
> > +	struct iio_backend		*back;
> > +	const struct ad4080_chip_info	*info;
> > +	/*
> > +	 * Synchronize access to members the of driver state, and ensure
> > +	 * atomicity of consecutive regmap operations.
> > +	 */
> > +	struct mutex			lock;
> > +	unsigned int			num_lanes;
> > +	unsigned int			dec_rate;
> > +	enum ad4080_filter_type		filter_type;
> > +	bool				lvds_cnv_en;
> > +};
> > +
> > +static const struct regmap_config ad4080_regmap_config = {
> > +	.reg_bits = 16,
> > +	.val_bits = 8,
> > +	.read_flag_mask = BIT(7),
> > +	.max_register = 0x29,
> > +};
> > +
> > +static int ad4080_reg_access(struct iio_dev *indio_dev, unsigned int reg,
> > +			     unsigned int writeval, unsigned int *readval)
> > +{
> > +	struct ad4080_state *st = iio_priv(indio_dev);
> > +
> 
> Missing guard(mutex)(&st->lock); ?
Aren't regmap operations thread safe? (own internal locking).
> > +	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;
> 
> Seems like this could be simplifed by using IIO_VAL_FRACTIONAL_LOG2
> instead.
> 
> > +}
> > +
> > +static unsigned int ad4080_get_dec_rate(struct iio_dev *dev,
> > +					const struct iio_chan_spec *chan)
> > +{
> > +	struct ad4080_state *st = iio_priv(dev);
> > +	int ret;
> > +	unsigned int data;
> > +
> 
> Missing guard(mutex)(&st->lock); ?
> 
> > +	ret = regmap_read(st->regmap, AD4080_REG_FILTER_CONFIG,
> &data);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return (1 <<
> (FIELD_GET(AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK, data) + 1));
> 
> nit: doen't need outermost ().
> 
> > +}
> > +
> > +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;
> > +
> 
> Don't we need to check for < 2 as well?
> 
> > +	if (st->filter_type >= SINC_5 && mode >= 512)
> > +		return -EINVAL;
> > +
> > +	guard(mutex)(&st->lock);
> > +	ret = regmap_update_bits(st->regmap, AD4080_REG_FILTER_CONFIG,
> > +
> AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK,
> > +
> FIELD_PREP(AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK,
> > +					    (ilog2(mode) - 1)));
> 
> Otherwise ilog2(mode) - 1 could be < 0.
> 
> > +	if (ret)
> > +		return ret;
> > +
> > +	st->dec_rate = mode;
> 
> This saves the value the user entered, not what the hardware is actually doing.
> It should be saving the power of 2 value instead.
> 
> > +
> > +	return 0;
> > +}
> > +
> > +static int ad4080_read_raw(struct iio_dev *indio_dev,
> > +			   struct iio_chan_spec const *chan,
> > +			   int *val, int *val2, long m)
> > +{
> > +	struct ad4080_state *st = iio_priv(indio_dev);
> > +	int dec_rate;
> > +
> > +	switch (m) {
> > +	case IIO_CHAN_INFO_SCALE:
> > +		return ad4080_get_scale(st, val, val2);
> > +	case IIO_CHAN_INFO_SAMP_FREQ:
> > +		if (st->filter_type == SINC_5_COMP)
> > +			dec_rate = st->dec_rate * 2;
> > +		else
> > +			dec_rate = st->dec_rate;
> 
> As a concequence of the above, this will return incorrect information if the
> user didn't enter an exact value.
Isn't the user constrained by the ad4080_read_avail for entering the dec_rate values?
The user both writes and reads the actual decimation rate value. The conversions are done inside the functions.
> 
> > +		if (st->filter_type)
> > +			*val = DIV_ROUND_CLOSEST(clk_get_rate(st->clk),
> dec_rate);
> > +		else
> > +			*val = clk_get_rate(st->clk);
> > +		return IIO_VAL_INT;
> > +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> > +		*val = ad4080_get_dec_rate(indio_dev, chan);
> > +		return IIO_VAL_INT;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +
> > +static int ad4080_write_raw(struct iio_dev *indio_dev,
> > +			    struct iio_chan_spec const *chan,
> > +			    int val, int val2, long mask)
> > +{
> > +	switch (mask) {
> > +	case IIO_CHAN_INFO_SCALE:
> > +		return -EINVAL;
> > +	case IIO_CHAN_INFO_SAMP_FREQ:
> > +		return -EINVAL;
> 
> Can leave these 2 out and just let them fall through to the default.
> 
> > +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> > +		return ad4080_set_dec_rate(indio_dev, chan, val);
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +
> > +static int ad4080_lvds_sync_write(struct ad4080_state *st)
> > +{
> > +	unsigned int timeout = 100;
> > +	bool sync_en;
> > +	int ret;
> 
> nit: some comments in this function would be helpful to readers not familiar
> with the part.
> 
> > +
> > +	guard(mutex)(&st->lock);
> > +	if (st->num_lanes == 1)
> > +		ret = regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > +
> AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> > +
> AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK);
> > +	else
> > +		ret = regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > +
> AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> > +
> AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN_MSK |
> > +
> AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = iio_backend_data_alignment_enable(st->back);
> > +	if (ret)
> > +		return ret;
> > +
> > +	do {
> > +		ret = iio_backend_sync_status_get(st->back, &sync_en);
> > +		if (ret)
> > +			return ret;
> > +
> > +		if (!sync_en)
> > +			dev_dbg(&st->spi->dev, "Not Locked: Running Bit
> Slip\n");
> > +
> > +		fsleep(500);
> > +	} while (--timeout && !sync_en);
> > +
> > +	if (timeout) {
> > +		dev_info(&st->spi->dev, "Success: Pattern correct and
> Locked!\n");
> > +		if (st->num_lanes == 1)
> > +			return regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > +
> AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK);
> > +		else
> > +			return regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > +
> AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> > +
> AD4080_ADC_DATA_INTF_CONFIG_A_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_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK);
> > +			if (ret)
> > +				return ret;
> > +		} else {
> > +			ret = regmap_write(st->regmap,
> AD4080_REG_ADC_DATA_INTF_CONFIG_A,
> > +
> AD4080_ADC_DATA_INTF_CONFIG_A_RESERVED_CONFIG_A_MSK |
> > +
> AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES_MSK);
> > +			if (ret)
> > +				return ret;
> > +		}
> > +
> > +		return -ETIMEDOUT;
> > +	}
> > +}
> > +
> > +static ssize_t ad4080_get_filter_type(struct iio_dev *dev,
> > +				      const struct iio_chan_spec *chan)
> > +{
> > +	struct ad4080_state *st = iio_priv(dev);
> > +	unsigned int data;
> > +	int ret;
> > +
> 
> Missing guard(mutex)(&st->lock); ?
> 
> > +	ret = regmap_read(st->regmap, AD4080_REG_FILTER_CONFIG,
> &data);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return FIELD_GET(AD4080_FILTER_CONFIG_FILTER_SEL_MSK, data);
> > +}
> > +
> 
> ...
> 
> > +static struct iio_chan_spec_ext_info ad4080_ext_info[] = {
> > +	IIO_ENUM("filter_type", IIO_SHARED_BY_ALL,
> &ad4080_filter_type_enum),
> > +	IIO_ENUM_AVAILABLE("filter_type", IIO_SHARED_BY_ALL,
> > +			   &ad4080_filter_type_enum),
> > +	{ }
> > +};
> > +
> > +static const struct iio_chan_spec ad4080_channels[] = {
> 
> Array with one element doesn't make sense. It can just be a single struct.
Isn't indio_dev->channels expecting an array?
> > +	{
> > +		.type = IIO_VOLTAGE,
> > +		.indexed = 1,
> > +		.channel = 0,
> > +		.info_mask_separate = BIT(IIO_CHAN_INFO_SCALE),
> > +		.info_mask_shared_by_all =
> BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> > +
> 	BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> > +		.info_mask_shared_by_all_available =
> > +
> 	BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> > +		.ext_info = ad4080_ext_info,
> > +		.scan_index = 0,
> > +		.scan_type = {
> > +			.sign = 's',
> > +			.realbits = 20,
> > +			.storagebits = 32,
> > +			.shift = 0,
> > +		},
> > +	}
> > +};
> > +

Re: [PATCH v3 11/11] iio: adc: ad4080: add driver support

[David Lechner]

On 4/30/25 7:31 AM, Miclaus, Antoniu wrote:
> ...
>>> +	unsigned int num_channels;
>>> +};
>>
>> I guess this is preparing the driver to support more than one chip?
>>
> Yes. It is stated also in the cover letter.
>>> +
>>> +struct ad4080_state {
>>> +	struct spi_device		*spi;
>>
>> It looks like this is only ever used to get &spi->dev. We could drop this and
>> get dev from regmap instead.
> How can I get the dev from regmap?

struct device *dev = regmap_get_device(regmap);

>>> +	struct regmap			*regmap;
>>> +	struct clk			*clk;
>>> +	struct iio_backend		*back;
>>> +	const struct ad4080_chip_info	*info;
>>> +	/*
>>> +	 * Synchronize access to members the of driver state, and ensure
>>> +	 * atomicity of consecutive regmap operations.
>>> +	 */
>>> +	struct mutex			lock;
>>> +	unsigned int			num_lanes;
>>> +	unsigned int			dec_rate;
>>> +	enum ad4080_filter_type		filter_type;
>>> +	bool				lvds_cnv_en;
>>> +};
>>> +
>>> +static const struct regmap_config ad4080_regmap_config = {
>>> +	.reg_bits = 16,
>>> +	.val_bits = 8,
>>> +	.read_flag_mask = BIT(7),
>>> +	.max_register = 0x29,
>>> +};
>>> +
>>> +static int ad4080_reg_access(struct iio_dev *indio_dev, unsigned int reg,
>>> +			     unsigned int writeval, unsigned int *readval)
>>> +{
>>> +	struct ad4080_state *st = iio_priv(indio_dev);
>>> +
>>
>> Missing guard(mutex)(&st->lock); ?
> Aren't regmap operations thread safe? (own internal locking).

For single operations, yes. But I assumed that you added the lock so that when
functions that do multiple regmap read/writes don't have another thread doing
a different regmap operation in the middle.

However, it looks like ad4080_lvds_sync_write() is currently the only function
like this and it is only called during probe. So it seems like the extra mutex
lock isn't currently needed and could be removed from the driver entirely.

>>> +	if (readval)
>>> +		return regmap_read(st->regmap, reg, readval);
>>> +
>>> +	return regmap_write(st->regmap, reg, writeval);
>>> +}


...

>>> +
>>> +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;
>>> +
>>
>> Don't we need to check for < 2 as well?
>>
>>> +	if (st->filter_type >= SINC_5 && mode >= 512)
>>> +		return -EINVAL;
>>> +
>>> +	guard(mutex)(&st->lock);
>>> +	ret = regmap_update_bits(st->regmap, AD4080_REG_FILTER_CONFIG,
>>> +
>> AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK,
>>> +
>> FIELD_PREP(AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK,
>>> +					    (ilog2(mode) - 1)));
>>
>> Otherwise ilog2(mode) - 1 could be < 0.
>>
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	st->dec_rate = mode;
>>
>> This saves the value the user entered, not what the hardware is actually doing.
>> It should be saving the power of 2 value instead.
>>
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int ad4080_read_raw(struct iio_dev *indio_dev,
>>> +			   struct iio_chan_spec const *chan,
>>> +			   int *val, int *val2, long m)
>>> +{
>>> +	struct ad4080_state *st = iio_priv(indio_dev);
>>> +	int dec_rate;
>>> +
>>> +	switch (m) {
>>> +	case IIO_CHAN_INFO_SCALE:
>>> +		return ad4080_get_scale(st, val, val2);
>>> +	case IIO_CHAN_INFO_SAMP_FREQ:
>>> +		if (st->filter_type == SINC_5_COMP)
>>> +			dec_rate = st->dec_rate * 2;
>>> +		else
>>> +			dec_rate = st->dec_rate;
>>
>> As a concequence of the above, this will return incorrect information if the
>> user didn't enter an exact value.
> Isn't the user constrained by the ad4080_read_avail for entering the dec_rate values?
> The user both writes and reads the actual decimation rate value. The conversions are done inside the functions.

Yes, the oversampling_ratio attribute is calling ad4080_get_dec_rate(), so
that one is OK, but this is the sampling_frequency attribute. Currently
st->dec_rate holds the user-requested value and isn't necessarily the same as
the value that would be returned by ad4080_get_dec_rate(indio_dev, chan).

If we dropped st->dec_rate and used ad4080_get_dec_rate(indio_dev, chan) here
too, that would be an acceptable solution too.

>>
>>> +		if (st->filter_type)
>>> +			*val = DIV_ROUND_CLOSEST(clk_get_rate(st->clk),
>> dec_rate);
>>> +		else
>>> +			*val = clk_get_rate(st->clk);
>>> +		return IIO_VAL_INT;
>>> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
>>> +		*val = ad4080_get_dec_rate(indio_dev, chan);
>>> +		return IIO_VAL_INT;
>>> +	default:
>>> +		return -EINVAL;
>>> +	}
>>> +}

...

>>> +static const struct iio_chan_spec ad4080_channels[] = {
>>
>> Array with one element doesn't make sense. It can just be a single struct.
> Isn't indio_dev->channels expecting an array?

No, it expects a pointer. So &ad4080_channel; can be used to get a pointer to
a single struct instance.