devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] Add support for AD7405/ADUM770x
@ 2025-03-24  9:07 Pop Ioan Daniel
  2025-03-24  9:07 ` [PATCH 1/5] iio: backend: add support for decimation ratio set Pop Ioan Daniel
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Pop Ioan Daniel @ 2025-03-24  9:07 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Nuno Sa,
	Olivier Moysan, David Lechner, Javier Carrasco, Andy Shevchenko,
	Trevor Gamblin, Guillaume Stols, Dumitru Ceclan, Matteo Martelli,
	AngeloGioacchino Del Regno, Alisa-Dariana Roman, Marcelo Schmitt,
	Ramona Alexandra Nechita, Herve Codina, Dragos Bogdan, linux-iio,
	devicetree, linux-kernel
  Cc: Pop Ioan Daniel

The AD7405 is a high performance, second-order, Σ-Δ modulator
that converts an analog input signal into a high speed, single-bit
LVDS data stream, with on-chip digital isolation based on Analog
Devices, Inc., iCoupler technology. The AD7405 operates from a
4.5 V to 5.5 V (VDD1) power supply and accepts a differential input
signal of ±250 mV (±320 mV full-scale). The differential input is ideally
suited to shunt voltage monitoring in high voltage applications
where galvanic isolation is required.

Pop Ioan Daniel (5):
  iio: backend: add support for decimation ratio set
  iio: adc: adi-axi-adc: add set decimation rate
  dt-bindings: iio: adc: add ad7405 axi variant
  dt-bindings: iio: adc: add ad7405
  iio: adc: ad7405: add ad7405 driver

 .../bindings/iio/adc/adi,ad7405.yaml          |  68 ++++
 .../bindings/iio/adc/adi,axi-adc.yaml         |   2 +
 drivers/iio/adc/Kconfig                       |  10 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/ad7405.c                      | 301 ++++++++++++++++++
 drivers/iio/adc/adi-axi-adc.c                 |  43 ++-
 drivers/iio/industrialio-backend.c            |  18 ++
 include/linux/iio/backend.h                   |   3 +
 8 files changed, 445 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7405.yaml
 create mode 100644 drivers/iio/adc/ad7405.c

-- 
2.34.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/5] iio: backend: add support for decimation ratio set
  2025-03-24  9:07 [PATCH 0/5] Add support for AD7405/ADUM770x Pop Ioan Daniel
@ 2025-03-24  9:07 ` Pop Ioan Daniel
  2025-03-24 10:13   ` Andy Shevchenko
  2025-03-24 18:53   ` David Lechner
  2025-03-24  9:07 ` [PATCH 2/5] iio: adc: adi-axi-adc: add set decimation rate Pop Ioan Daniel
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 18+ messages in thread
From: Pop Ioan Daniel @ 2025-03-24  9:07 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Nuno Sa,
	Olivier Moysan, David Lechner, Javier Carrasco, Andy Shevchenko,
	Guillaume Stols, Trevor Gamblin, Dumitru Ceclan, Matteo Martelli,
	João Paulo Gonçalves, Alisa-Dariana Roman,
	Thomas Bonnefille, Herve Codina, Marcelo Schmitt, Dragos Bogdan,
	linux-iio, devicetree, linux-kernel
  Cc: Pop Ioan Daniel

Add backend support for setting the decimation ratio used.

Signed-off-by: Pop Ioan Daniel <pop.ioan-daniel@analog.com>
---
 drivers/iio/industrialio-backend.c | 18 ++++++++++++++++++
 include/linux/iio/backend.h        |  3 +++
 2 files changed, 21 insertions(+)

diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index 363281272035..f4db6514944a 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -417,6 +417,24 @@ int iio_backend_test_pattern_set(struct iio_backend *back,
 }
 EXPORT_SYMBOL_NS_GPL(iio_backend_test_pattern_set, "IIO_BACKEND");
 
+/**
+ * iio_backend_set_dec_rate - set decimation ratio
+ * @back: Backend device
+ * @rate: Rate in decimal
+
+ * Return:
+ * 0 on success, negative error number on failure.
+ */
+
+int iio_backend_set_dec_rate(struct iio_backend *back, unsigned int rate)
+{
+	if (!rate)
+		return -EINVAL;
+
+	return iio_backend_op_call(back, set_dec_rate, rate);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_set_dec_rate, "IIO_BACKEND");
+
 /**
  * iio_backend_chan_status - Get the channel status
  * @back: Backend device
diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
index 10be00f3b120..e73d7d265a16 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -80,6 +80,7 @@ enum iio_backend_sample_trigger {
  * @data_source_set: Configure the data source for a specific channel.
  * @set_sample_rate: Configure the sampling rate for a specific channel.
  * @test_pattern_set: Configure a test pattern.
+ * @set_dec_rate: Set decimation ratio
  * @chan_status: Get the channel status.
  * @iodelay_set: Set digital I/O delay.
  * @data_sample_trigger: Control when to sample data.
@@ -111,6 +112,7 @@ struct iio_backend_ops {
 	int (*test_pattern_set)(struct iio_backend *back,
 				unsigned int chan,
 				enum iio_backend_test_pattern pattern);
+	int (*set_dec_rate)(struct iio_backend *back, unsigned int rate);
 	int (*chan_status)(struct iio_backend *back, unsigned int chan,
 			   bool *error);
 	int (*iodelay_set)(struct iio_backend *back, unsigned int chan,
@@ -167,6 +169,7 @@ int iio_backend_set_sampling_freq(struct iio_backend *back, unsigned int chan,
 int iio_backend_test_pattern_set(struct iio_backend *back,
 				 unsigned int chan,
 				 enum iio_backend_test_pattern pattern);
+int iio_backend_set_dec_rate(struct iio_backend *back, unsigned int rate);
 int iio_backend_chan_status(struct iio_backend *back, unsigned int chan,
 			    bool *error);
 int iio_backend_iodelay_set(struct iio_backend *back, unsigned int lane,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/5] iio: adc: adi-axi-adc: add set decimation rate
  2025-03-24  9:07 [PATCH 0/5] Add support for AD7405/ADUM770x Pop Ioan Daniel
  2025-03-24  9:07 ` [PATCH 1/5] iio: backend: add support for decimation ratio set Pop Ioan Daniel
@ 2025-03-24  9:07 ` Pop Ioan Daniel
  2025-03-24 10:15   ` Andy Shevchenko
  2025-03-24 18:54   ` David Lechner
  2025-03-24  9:07 ` [PATCH 3/5] dt-bindings: iio: adc: add ad7405 axi variant Pop Ioan Daniel
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 18+ messages in thread
From: Pop Ioan Daniel @ 2025-03-24  9:07 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Nuno Sa,
	Olivier Moysan, David Lechner, Javier Carrasco, Andy Shevchenko,
	Guillaume Stols, Trevor Gamblin, Dumitru Ceclan, Matteo Martelli,
	João Paulo Gonçalves, Alisa-Dariana Roman,
	Marcelo Schmitt, Herve Codina, Ramona Alexandra Nechita,
	Dragos Bogdan, linux-iio, devicetree, linux-kernel
  Cc: Pop Ioan Daniel

Add support for setting decimation rate.

Add separate compatible string for the custom AD7405 IP and implement
the necessary changes.

Signed-off-by: Pop Ioan Daniel <pop.ioan-daniel@analog.com>
---
 drivers/iio/adc/adi-axi-adc.c | 43 ++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index 766406f45396..0ed609e294ba 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -70,6 +70,9 @@
 #define ADI_AXI_ADC_REG_CHAN_CTRL_3(c)		(0x0418 + (c) * 0x40)
 #define   ADI_AXI_ADC_CHAN_PN_SEL_MASK		GENMASK(19, 16)
 
+#define ADI_AXI_ADC_REG_CHAN_USR_CTRL_2	0x0424
+#define   ADI_AXI_ADC_DEC_RATE_MASK		GENMASK(15, 0)
+
 /* IO Delays */
 #define ADI_AXI_ADC_REG_DELAY(l)		(0x0800 + (l) * 0x4)
 #define   AXI_ADC_DELAY_CTRL_MASK		GENMASK(4, 0)
@@ -232,6 +235,16 @@ static int axi_adc_test_pattern_set(struct iio_backend *back,
 	}
 }
 
+static int axi_adc_set_dec_rate(struct iio_backend *back,
+				unsigned int rate)
+{
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+
+	return regmap_update_bits(st->regmap, (ADI_AXI_ADC_REG_CHAN_USR_CTRL_2),
+					      ADI_AXI_ADC_DEC_RATE_MASK,
+					      FIELD_PREP(ADI_AXI_ADC_DEC_RATE_MASK, rate));
+}
+
 static int axi_adc_read_chan_status(struct adi_axi_adc_state *st, unsigned int chan,
 				    unsigned int *status)
 {
@@ -465,6 +478,28 @@ static const struct iio_backend_info adi_axi_adc_generic = {
 	.ops = &adi_axi_adc_ops,
 };
 
+static const struct iio_backend_ops adi_ad7405_ops = {
+	.enable = axi_adc_enable,
+	.disable = axi_adc_disable,
+	.data_format_set = axi_adc_data_format_set,
+	.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,
+	.iodelay_set = axi_adc_iodelays_set,
+	.test_pattern_set = axi_adc_test_pattern_set,
+	.set_dec_rate = axi_adc_set_dec_rate,
+	.chan_status = axi_adc_chan_status,
+	.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_ad7405 = {
+	.name = "axi-ad7405",
+	.ops = &adi_ad7405_ops,
+};
+
 static int adi_axi_adc_probe(struct platform_device *pdev)
 {
 	struct adi_axi_adc_state *st;
@@ -522,7 +557,7 @@ static int adi_axi_adc_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	ret = devm_iio_backend_register(&pdev->dev, &adi_axi_adc_generic, st);
+	ret = devm_iio_backend_register(&pdev->dev, st->info->backend_info, st);
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret,
 				     "failed to register iio backend\n");
@@ -575,10 +610,16 @@ static const struct axi_adc_info adc_ad7606 = {
 	.has_child_nodes = true,
 };
 
+static const struct axi_adc_info adi_axi_ad7405 = {
+	.version = ADI_AXI_PCORE_VER(10, 0, 'a'),
+	.backend_info = &axi_ad7405,
+};
+
 /* 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-ad7606x", .data = &adc_ad7606 },
+	{ .compatible = "adi,axi-ad7405", .data = &adi_axi_ad7405},
 	{ /* end of list */ }
 };
 MODULE_DEVICE_TABLE(of, adi_axi_adc_of_match);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/5] dt-bindings: iio: adc: add ad7405 axi variant
  2025-03-24  9:07 [PATCH 0/5] Add support for AD7405/ADUM770x Pop Ioan Daniel
  2025-03-24  9:07 ` [PATCH 1/5] iio: backend: add support for decimation ratio set Pop Ioan Daniel
  2025-03-24  9:07 ` [PATCH 2/5] iio: adc: adi-axi-adc: add set decimation rate Pop Ioan Daniel
@ 2025-03-24  9:07 ` Pop Ioan Daniel
  2025-03-24 18:55   ` David Lechner
  2025-03-24  9:07 ` [PATCH 4/5] dt-bindings: iio: adc: add ad7405 Pop Ioan Daniel
  2025-03-24  9:08 ` [PATCH 5/5] iio: adc: ad7405: add ad7405 driver Pop Ioan Daniel
  4 siblings, 1 reply; 18+ messages in thread
From: Pop Ioan Daniel @ 2025-03-24  9:07 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Nuno Sa,
	Olivier Moysan, David Lechner, Javier Carrasco, Andy Shevchenko,
	Trevor Gamblin, Guillaume Stols, Dumitru Ceclan, Matteo Martelli,
	Marcelo Schmitt, Alisa-Dariana Roman, Thomas Bonnefille,
	Ramona Alexandra Nechita, Pop Ioan Daniel, Dragos Bogdan,
	linux-iio, devicetree, linux-kernel

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

The AXI AD7405 IP is a very similar HDL (fpga) variant of the generic
AXI ADC IP, intended to control ad7405/adum770x family.

Signed-off-by: Pop Ioan Daniel <pop.ioan-daniel@analog.com>
---
 Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml b/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
index 4fa82dcf6fc9..1b02217ff8b5 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_ad7405/index.html
   http://analogdevicesinc.github.io/hdl/library/axi_ad7606x/index.html
 
 properties:
@@ -34,6 +35,7 @@ properties:
     enum:
       - adi,axi-adc-10.0.a
       - adi,axi-ad7606x
+      - adi,axi-ad7405
 
   reg:
     maxItems: 1
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/5] dt-bindings: iio: adc: add ad7405
  2025-03-24  9:07 [PATCH 0/5] Add support for AD7405/ADUM770x Pop Ioan Daniel
                   ` (2 preceding siblings ...)
  2025-03-24  9:07 ` [PATCH 3/5] dt-bindings: iio: adc: add ad7405 axi variant Pop Ioan Daniel
@ 2025-03-24  9:07 ` Pop Ioan Daniel
  2025-03-24 18:56   ` David Lechner
                     ` (2 more replies)
  2025-03-24  9:08 ` [PATCH 5/5] iio: adc: ad7405: add ad7405 driver Pop Ioan Daniel
  4 siblings, 3 replies; 18+ messages in thread
From: Pop Ioan Daniel @ 2025-03-24  9:07 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Nuno Sa,
	Olivier Moysan, David Lechner, Javier Carrasco, Andy Shevchenko,
	Trevor Gamblin, Guillaume Stols, Dumitru Ceclan, Matteo Martelli,
	Marcelo Schmitt, Alisa-Dariana Roman, Ramona Alexandra Nechita,
	Thomas Bonnefille, Dragos Bogdan, linux-iio, devicetree,
	linux-kernel
  Cc: Pop Ioan Daniel

Add devicetree bindings for ad7405/adum770x family.

Signed-off-by: Pop Ioan Daniel <pop.ioan-daniel@analog.com>
---
 .../bindings/iio/adc/adi,ad7405.yaml          | 68 +++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7405.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7405.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7405.yaml
new file mode 100644
index 000000000000..e312fa0cdb05
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7405.yaml
@@ -0,0 +1,68 @@
+# 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,ad7405.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD7405 family
+
+maintainers:
+  - Dragos Bogdan <dragos.bogdan@analog.com>
+
+description: |
+  Analog Devices AD7405 is a high performance isolated ADC, 1-channel,
+  16-bit with a second-order Σ-Δ modulator that converts an analog input signal
+  into a high speed, single-bit data stream.
+
+  https://www.analog.com/media/en/technical-documentation/data-sheets/ad7405.pdf
+  https://www.analog.com/media/en/technical-documentation/data-sheets/adum7701.pdf
+  https://www.analog.com/media/en/technical-documentation/data-sheets/adum7702.pdf
+  https://www.analog.com/media/en/technical-documentation/data-sheets/ADuM7703.pdf
+
+properties:
+  compatible:
+    enum:
+      - adi,ad7405
+      - adi,adum7701
+      - adi,adum7702
+      - adi,adum7703
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  vdd1-supply: true
+
+  vdd2-supply: true
+
+  clocks:
+    maxitems: 1
+
+  io-backends:
+    maxItems: 1
+
+required:
+  - compatible
+  - vdd1-supply
+  - vdd2-supply
+  - clocks
+  - io-backends
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    adc {
+       #address-cells = <1>;
+        #size-cells = <0>;
+        compatible = "adi,ad7405";
+        clocks = <&axi_clk_gen 0>;
+        vdd1-supply = <&vdd1>;
+        vdd2-supply = <&vdd2>;
+        io-backends = <&iio_backend>;
+    };
+...
\ No newline at end of file
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/5] iio: adc: ad7405: add ad7405 driver
  2025-03-24  9:07 [PATCH 0/5] Add support for AD7405/ADUM770x Pop Ioan Daniel
                   ` (3 preceding siblings ...)
  2025-03-24  9:07 ` [PATCH 4/5] dt-bindings: iio: adc: add ad7405 Pop Ioan Daniel
@ 2025-03-24  9:08 ` Pop Ioan Daniel
  2025-03-24 10:21   ` Andy Shevchenko
                     ` (2 more replies)
  4 siblings, 3 replies; 18+ messages in thread
From: Pop Ioan Daniel @ 2025-03-24  9:08 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Nuno Sa,
	Olivier Moysan, David Lechner, Javier Carrasco, Andy Shevchenko,
	Guillaume Stols, Trevor Gamblin, Dumitru Ceclan, Matteo Martelli,
	AngeloGioacchino Del Regno, Alisa-Dariana Roman, Michael Walle,
	Herve Codina, Thomas Bonnefille, Pop Ioan Daniel, Dragos Bogdan,
	linux-iio, devicetree, linux-kernel

Add support for the AD7405/ADUM770x, a high performance isolated ADC,
1-channel, 16-bit with a second-order Σ-Δ modulator that converts an
analog input signal into a high speed, single-bit data stream.

Signed-off-by: Pop Ioan Daniel <pop.ioan-daniel@analog.com>
---
 drivers/iio/adc/Kconfig  |  10 ++
 drivers/iio/adc/Makefile |   1 +
 drivers/iio/adc/ad7405.c | 301 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 312 insertions(+)
 create mode 100644 drivers/iio/adc/ad7405.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index f64b5faeb257..321a1ee7304f 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -203,6 +203,16 @@ config AD7380
 	  To compile this driver as a module, choose M here: the module will be
 	  called ad7380.
 
+config AD7405
+	tristate "Analog Device AD7405 ADC Driver"
+	select IIO_BACKEND
+	help
+	  Say yes here to build support for Analog Devices AD7405, ADUM7701,
+	  ADUM7702, ADUM7703 analog to digital converters (ADC).
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called ad7405.
+
 config AD7476
 	tristate "Analog Devices AD7476 1-channel ADCs driver and other similar devices from AD and TI"
 	depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index ee19afba62b7..0c3c1c69b6b4 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_AD7291) += ad7291.o
 obj-$(CONFIG_AD7292) += ad7292.o
 obj-$(CONFIG_AD7298) += ad7298.o
 obj-$(CONFIG_AD7380) += ad7380.o
+obj-$(CONFIG_AD7405) += ad7405.o
 obj-$(CONFIG_AD7476) += ad7476.o
 obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o
 obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o
diff --git a/drivers/iio/adc/ad7405.c b/drivers/iio/adc/ad7405.c
new file mode 100644
index 000000000000..40fe072369d5
--- /dev/null
+++ b/drivers/iio/adc/ad7405.c
@@ -0,0 +1,301 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Analog Devices AD7405 driver
+ *
+ * Copyright 2025 Analog Devices Inc.
+ */
+
+#include <linux/module.h>
+#include <linux/log2.h>
+#include <linux/clk.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/backend.h>
+#include <linux/util_macros.h>
+#include <linux/regulator/consumer.h>
+
+#define AD7405_DEFAULT_DEC_RATE 1024
+
+const unsigned int ad7405_dec_rates[] = {
+		4096, 2048, 1024, 512, 256, 128, 64, 32,
+};
+
+struct ad7405_chip_info {
+	const char *name;
+	unsigned int num_channels;
+	unsigned int max_rate;
+	unsigned int min_rate;
+	struct iio_chan_spec channel[3];
+	const unsigned long *available_mask;
+};
+
+struct ad7405_state {
+	struct iio_backend *back;
+	struct clk *axi_clk_gen;
+	/* lock to protect multiple accesses to the device registers */
+	struct mutex lock;
+	struct regmap *regmap;
+	struct iio_info iio_info;
+	const struct ad7405_chip_info *info;
+	unsigned int sample_frequency_tbl[ARRAY_SIZE(ad7405_dec_rates)];
+	unsigned int sample_frequency;
+	unsigned int ref_frequency;
+};
+
+static void ad7405_fill_samp_freq_table(struct ad7405_state *st)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(ad7405_dec_rates); i++)
+		st->sample_frequency_tbl[i] = DIV_ROUND_CLOSEST_ULL(st->ref_frequency, ad7405_dec_rates[i]);
+}
+
+static int ad7405_set_sampling_rate(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan,
+				    unsigned int samp_rate)
+{
+	struct ad7405_state *st = iio_priv(indio_dev);
+	unsigned int dec_rate, idx;
+	int ret;
+
+	dec_rate = DIV_ROUND_CLOSEST_ULL(st->ref_frequency, samp_rate);
+
+	idx = find_closest_descending(dec_rate, ad7405_dec_rates,
+				      ARRAY_SIZE(ad7405_dec_rates));
+
+	    dec_rate = ad7405_dec_rates[idx];
+
+	ret = iio_backend_set_dec_rate(st->back, dec_rate);
+	if (ret)
+		return ret;
+
+	st->sample_frequency = DIV_ROUND_CLOSEST_ULL(st->ref_frequency, dec_rate);
+
+	return 0;
+}
+
+static int ad7405_update_scan_mode(struct iio_dev *indio_dev,
+				   const unsigned long *scan_mask)
+{
+	struct ad7405_state *st = iio_priv(indio_dev);
+	unsigned int c;
+	int ret;
+
+	for (c = 0; c < indio_dev->num_channels; c++) {
+		if (test_bit(c, scan_mask))
+			ret = iio_backend_chan_enable(st->back, c);
+		else
+			ret = iio_backend_chan_disable(st->back, c);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int ad7405_read_raw(struct iio_dev *indio_dev,
+			   const struct iio_chan_spec *chan, int *val,
+			   int *val2, long info)
+{
+	struct ad7405_state *st = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+			*val = st->sample_frequency;
+
+			return IIO_VAL_INT;
+	default:
+			return -EINVAL;
+	}
+}
+
+static int ad7405_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan, int val,
+			    int val2, long info)
+{
+	switch (info) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+
+			return ad7405_set_sampling_rate(indio_dev, chan, val);
+
+	default:
+			return -EINVAL;
+	}
+}
+
+static int ad7405_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+				 const int **vals, int *type, int *length,
+				 long info)
+{
+	struct ad7405_state *st = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+			*vals = st->sample_frequency_tbl;
+			*length = ARRAY_SIZE(st->sample_frequency_tbl);
+			*type = IIO_VAL_INT;
+			return IIO_AVAIL_LIST;
+	default:
+			return -EINVAL;
+	}
+}
+
+static const struct iio_info ad7405_iio_info = {
+	.read_raw = &ad7405_read_raw,
+	.write_raw = &ad7405_write_raw,
+	.read_avail = &ad7405_read_avail,
+	.update_scan_mode = ad7405_update_scan_mode,
+};
+
+#define AD7405_IIO_CHANNEL(_chan, _bits, _sign)		  \
+	{ .type = IIO_VOLTAGE,					  \
+	  .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+	  .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+	  .indexed = 1,						 \
+	  .channel = _chan,					 \
+	  .scan_type = {				\
+		.sign = _sign,				\
+		.realbits = _bits,			\
+		.storagebits = 16,			\
+		.shift = 0,				\
+	  },						\
+	}
+
+static const unsigned long ad7405_channel_masks[] = {
+		BIT(0),
+		0,
+};
+
+static const struct ad7405_chip_info ad7405_chip_info = {
+		.name = "AD7405",
+		.max_rate = 625000UL,
+		.min_rate = 4883UL,
+		.num_channels = 1,
+		.channel = {
+			AD7405_IIO_CHANNEL(0, 16, 'u'),
+		},
+		.available_mask = ad7405_channel_masks,
+};
+
+static const struct ad7405_chip_info adum7701_chip_info = {
+		.name = "ADUM7701",
+		.max_rate = 656250UL,
+		.min_rate = 5127UL,
+		.num_channels = 1,
+		.channel = {
+			AD7405_IIO_CHANNEL(0, 16, 'u'),
+		},
+		.available_mask = ad7405_channel_masks,
+};
+
+static const char * const ad7405_power_supplies[] = {
+	"vdd1",	"vdd2",
+};
+
+static int ad7405_probe(struct platform_device *pdev)
+{
+	const struct ad7405_chip_info *chip_info;
+	struct device *dev = &pdev->dev;
+	struct iio_dev *indio_dev;
+	struct ad7405_state *st;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+
+	ret = devm_mutex_init(dev, &st->lock);
+	if (ret)
+		return ret;
+
+	chip_info = &ad7405_chip_info;
+
+	platform_set_drvdata(pdev, indio_dev);
+
+	st->axi_clk_gen = devm_clk_get(dev, NULL);
+	if (IS_ERR(st->axi_clk_gen))
+		return PTR_ERR(st->axi_clk_gen);
+
+	ret = clk_prepare_enable(st->axi_clk_gen);
+	if (ret)
+		return ret;
+
+	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(ad7405_power_supplies),
+					     ad7405_power_supplies);
+
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to get and enable supplies");
+
+	st->ref_frequency = clk_get_rate(st->axi_clk_gen);
+
+	ad7405_fill_samp_freq_table(st);
+
+	indio_dev->dev.parent = dev;
+	indio_dev->name = pdev->dev.of_node->name;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	indio_dev->channels = chip_info->channel;
+	indio_dev->num_channels = chip_info->num_channels;
+
+	st->iio_info = ad7405_iio_info;
+	indio_dev->info = &st->iio_info;
+
+	st->back = devm_iio_backend_get(dev, NULL);
+	if (IS_ERR(st->back))
+		return dev_err_probe(dev, PTR_ERR(st->back),
+				     "failed to get IIO backend");
+
+	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;
+
+	/* Reset all HDL Cores */
+	iio_backend_disable(st->back);
+	iio_backend_enable(st->back);
+
+	ret = ad7405_set_sampling_rate(indio_dev, &indio_dev->channels[0],
+				       chip_info->max_rate);
+	if (ret)
+		return ret;
+
+	ret = devm_iio_device_register(dev, indio_dev);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/* Match table for of_platform binding */
+static const struct of_device_id ad7405_of_match[] = {
+	{ .compatible = "adi,ad7405", .data = &ad7405_chip_info, },
+	{ .compatible = "adi,adum7701", .data = &adum7701_chip_info, },
+	{ .compatible = "adi,adum7702", .data = &adum7701_chip_info, },
+	{ .compatible = "adi,adum7703", .data = &adum7701_chip_info, },
+	{ /* end of list */ },
+};
+
+MODULE_DEVICE_TABLE(of, ad7405_of_match);
+
+static struct platform_driver ad7405_driver = {
+	.driver = {
+		.name = "ad7405",
+		.owner = THIS_MODULE,
+		.of_match_table = ad7405_of_match,
+	},
+	.probe = ad7405_probe,
+};
+
+module_platform_driver(ad7405_driver);
+
+MODULE_AUTHOR("Dragos Bogdan <dragos.bogdan@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD7405 driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("IIO_BACKEND");
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/5] iio: backend: add support for decimation ratio set
  2025-03-24  9:07 ` [PATCH 1/5] iio: backend: add support for decimation ratio set Pop Ioan Daniel
@ 2025-03-24 10:13   ` Andy Shevchenko
  2025-03-24 18:53   ` David Lechner
  1 sibling, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2025-03-24 10:13 UTC (permalink / raw)
  To: Pop Ioan Daniel
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Nuno Sa,
	Olivier Moysan, David Lechner, Javier Carrasco, Guillaume Stols,
	Trevor Gamblin, Dumitru Ceclan, Matteo Martelli,
	João Paulo Gonçalves, Alisa-Dariana Roman,
	Thomas Bonnefille, Herve Codina, Marcelo Schmitt, Dragos Bogdan,
	linux-iio, devicetree, linux-kernel

On Mon, Mar 24, 2025 at 11:07:56AM +0200, Pop Ioan Daniel wrote:
> Add backend support for setting the decimation ratio used.

...

> +/**
> + * iio_backend_set_dec_rate - set decimation ratio
> + * @back: Backend device
> + * @rate: Rate in decimal
> +

Something is missing here...

> + * Return:
> + * 0 on success, negative error number on failure.

Please, double check that the style of Return section is the same as for the
rest of the functions in this file. If there are different styles choose one
which is simultaneously more recent and has more common sense in it (like
Returns: vs. Return, the preferred is the latter).

> + */

> +

Here is an uneeded blank line.

> +int iio_backend_set_dec_rate(struct iio_backend *back, unsigned int rate)
> +{

> +	if (!rate)
> +		return -EINVAL;

Yeah, besides missing style comment above, the function is undescribed. You
really need to add something meaningful in the kernel-doc and esp. explain
corner cases like this and choices made (why we fail it, what's wrong with 0?).

> +	return iio_backend_op_call(back, set_dec_rate, rate);
> +}

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/5] iio: adc: adi-axi-adc: add set decimation rate
  2025-03-24  9:07 ` [PATCH 2/5] iio: adc: adi-axi-adc: add set decimation rate Pop Ioan Daniel
@ 2025-03-24 10:15   ` Andy Shevchenko
  2025-03-24 18:54   ` David Lechner
  1 sibling, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2025-03-24 10:15 UTC (permalink / raw)
  To: Pop Ioan Daniel
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Nuno Sa,
	Olivier Moysan, David Lechner, Javier Carrasco, Guillaume Stols,
	Trevor Gamblin, Dumitru Ceclan, Matteo Martelli,
	João Paulo Gonçalves, Alisa-Dariana Roman,
	Marcelo Schmitt, Herve Codina, Ramona Alexandra Nechita,
	Dragos Bogdan, linux-iio, devicetree, linux-kernel

On Mon, Mar 24, 2025 at 11:07:57AM +0200, Pop Ioan Daniel wrote:
> Add support for setting decimation rate.
> 
> Add separate compatible string for the custom AD7405 IP and implement
> the necessary changes.

...

> +static int axi_adc_set_dec_rate(struct iio_backend *back,
> +				unsigned int rate)
> +{
> +	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
> +
> +	return regmap_update_bits(st->regmap, (ADI_AXI_ADC_REG_CHAN_USR_CTRL_2),

What' the purpose of the parentheses, please?

> +					      ADI_AXI_ADC_DEC_RATE_MASK,
> +					      FIELD_PREP(ADI_AXI_ADC_DEC_RATE_MASK, rate));
> +}

...

>  /* 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-ad7606x", .data = &adc_ad7606 },
> +	{ .compatible = "adi,axi-ad7405", .data = &adi_axi_ad7405},

You really need to be as much as possible consistent with the style in the
	current code.

>  	{ /* end of list */ }
>  };

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/5] iio: adc: ad7405: add ad7405 driver
  2025-03-24  9:08 ` [PATCH 5/5] iio: adc: ad7405: add ad7405 driver Pop Ioan Daniel
@ 2025-03-24 10:21   ` Andy Shevchenko
  2025-03-24 19:09   ` David Lechner
  2025-03-30 16:01   ` Jonathan Cameron
  2 siblings, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2025-03-24 10:21 UTC (permalink / raw)
  To: Pop Ioan Daniel
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Nuno Sa,
	Olivier Moysan, David Lechner, Javier Carrasco, Guillaume Stols,
	Trevor Gamblin, Dumitru Ceclan, Matteo Martelli,
	AngeloGioacchino Del Regno, Alisa-Dariana Roman, Michael Walle,
	Herve Codina, Thomas Bonnefille, Dragos Bogdan, linux-iio,
	devicetree, linux-kernel

On Mon, Mar 24, 2025 at 11:08:00AM +0200, Pop Ioan Daniel wrote:
> Add support for the AD7405/ADUM770x, a high performance isolated ADC,
> 1-channel, 16-bit with a second-order Σ-Δ modulator that converts an
> analog input signal into a high speed, single-bit data stream.

Datasheet: tag?

> Signed-off-by: Pop Ioan Daniel <pop.ioan-daniel@analog.com>

...

The list of headers is semi-random and unordered. Please, follow IWYU principle
and fix the ordering as well.

> +#include <linux/module.h>
> +#include <linux/log2.h>
> +#include <linux/clk.h>
> +#include <linux/platform_device.h>

> +#include <linux/of.h>

No of.h in the new code, please.

> +#include <linux/iio/iio.h>
> +#include <linux/iio/backend.h>

Move this group...

> +#include <linux/util_macros.h>
> +#include <linux/regulator/consumer.h>
> +

...somewhere here as other (recent) drivers do.

...

> +#define AD7405_DEFAULT_DEC_RATE 1024

What is the units?

...

> +const unsigned int ad7405_dec_rates[] = {
> +		4096, 2048, 1024, 512, 256, 128, 64, 32,

Too much TABbed.

> +};

...

> +struct ad7405_chip_info {
> +	const char *name;
> +	unsigned int num_channels;
> +	unsigned int max_rate;
> +	unsigned int min_rate;
> +	struct iio_chan_spec channel[3];
> +	const unsigned long *available_mask;

`pahole` has been run and this is the best choice, right?

> +};
> +
> +struct ad7405_state {

Ditto.

> +	struct iio_backend *back;
> +	struct clk *axi_clk_gen;
> +	/* lock to protect multiple accesses to the device registers */
> +	struct mutex lock;
> +	struct regmap *regmap;
> +	struct iio_info iio_info;
> +	const struct ad7405_chip_info *info;
> +	unsigned int sample_frequency_tbl[ARRAY_SIZE(ad7405_dec_rates)];
> +	unsigned int sample_frequency;
> +	unsigned int ref_frequency;
> +};

...

> +static void ad7405_fill_samp_freq_table(struct ad7405_state *st)
> +{
> +	int i;

Why signed.

> +
> +	for (i = 0; i < ARRAY_SIZE(ad7405_dec_rates); i++)
> +		st->sample_frequency_tbl[i] = DIV_ROUND_CLOSEST_ULL(st->ref_frequency, ad7405_dec_rates[i]);

This is too long even for relaxed mode...

> +}

...

> +static int ad7405_set_sampling_rate(struct iio_dev *indio_dev,
> +				    const struct iio_chan_spec *chan,
> +				    unsigned int samp_rate)
> +{
> +	struct ad7405_state *st = iio_priv(indio_dev);
> +	unsigned int dec_rate, idx;
> +	int ret;
> +
> +	dec_rate = DIV_ROUND_CLOSEST_ULL(st->ref_frequency, samp_rate);
> +
> +	idx = find_closest_descending(dec_rate, ad7405_dec_rates,
> +				      ARRAY_SIZE(ad7405_dec_rates));
> +
> +	    dec_rate = ad7405_dec_rates[idx];

Something happened there...

> +	ret = iio_backend_set_dec_rate(st->back, dec_rate);
> +	if (ret)
> +		return ret;
> +
> +	st->sample_frequency = DIV_ROUND_CLOSEST_ULL(st->ref_frequency, dec_rate);
> +
> +	return 0;
> +}

...

Okay, I'll stop here, this driver is not ready for upstream. Please, consult
with your colleagues and do round of internal review before sending a new
version.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/5] iio: backend: add support for decimation ratio set
  2025-03-24  9:07 ` [PATCH 1/5] iio: backend: add support for decimation ratio set Pop Ioan Daniel
  2025-03-24 10:13   ` Andy Shevchenko
@ 2025-03-24 18:53   ` David Lechner
  2025-03-28  8:31     ` Nuno Sá
  1 sibling, 1 reply; 18+ messages in thread
From: David Lechner @ 2025-03-24 18:53 UTC (permalink / raw)
  To: Pop Ioan Daniel, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Nuno Sa, Olivier Moysan, Javier Carrasco, Andy Shevchenko,
	Guillaume Stols, Trevor Gamblin, Dumitru Ceclan, Matteo Martelli,
	João Paulo Gonçalves, Alisa-Dariana Roman,
	Thomas Bonnefille, Herve Codina, Marcelo Schmitt, Dragos Bogdan,
	linux-iio, devicetree, linux-kernel

On 3/24/25 4:07 AM, Pop Ioan Daniel wrote:
> Add backend support for setting the decimation ratio used.
> 
> Signed-off-by: Pop Ioan Daniel <pop.ioan-daniel@analog.com>
> ---
>  drivers/iio/industrialio-backend.c | 18 ++++++++++++++++++
>  include/linux/iio/backend.h        |  3 +++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
> index 363281272035..f4db6514944a 100644
> --- a/drivers/iio/industrialio-backend.c
> +++ b/drivers/iio/industrialio-backend.c
> @@ -417,6 +417,24 @@ int iio_backend_test_pattern_set(struct iio_backend *back,
>  }
>  EXPORT_SYMBOL_NS_GPL(iio_backend_test_pattern_set, "IIO_BACKEND");
>  
> +/**
> + * iio_backend_set_dec_rate - set decimation ratio

In [1], we decided that for a similar chip we could use "decimation rate" and
"oversampling ratio" interchangeably. And in [2], we recently added a backend
op for setting the oversampling ratio. Would it make sense to use that here
as well instead of introducing a new function? If not, we'll want more of an
explanation here on what the difference is. But from what I can tell, this
sounds very similar to the other case where they are essentially the same thing.
On the other hand, this is internal API and not userspace ABI, so having a
separate name might not be so bad, especially if there is a chance we would ever
have both at the same time.

[1]: https://lore.kernel.org/linux-iio/20250112122047.1e1978e0@jic23-huawei/
[2]: https://web.git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/drivers/iio/industrialio-backend.c?h=testing&id=22894e0be908791e3df011cdfac02589c2f340bd

> + * @back: Backend device
> + * @rate: Rate in decimal
> +
> + * Return:
> + * 0 on success, negative error number on failure.
> + */
> +
> +int iio_backend_set_dec_rate(struct iio_backend *back, unsigned int rate)
> +{
> +	if (!rate)
> +		return -EINVAL;
> +
> +	return iio_backend_op_call(back, set_dec_rate, rate);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_set_dec_rate, "IIO_BACKEND");
> +

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/5] iio: adc: adi-axi-adc: add set decimation rate
  2025-03-24  9:07 ` [PATCH 2/5] iio: adc: adi-axi-adc: add set decimation rate Pop Ioan Daniel
  2025-03-24 10:15   ` Andy Shevchenko
@ 2025-03-24 18:54   ` David Lechner
  1 sibling, 0 replies; 18+ messages in thread
From: David Lechner @ 2025-03-24 18:54 UTC (permalink / raw)
  To: Pop Ioan Daniel, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Nuno Sa, Olivier Moysan, Javier Carrasco, Andy Shevchenko,
	Guillaume Stols, Trevor Gamblin, Dumitru Ceclan, Matteo Martelli,
	João Paulo Gonçalves, Alisa-Dariana Roman,
	Marcelo Schmitt, Herve Codina, Ramona Alexandra Nechita,
	Dragos Bogdan, linux-iio, devicetree, linux-kernel

On 3/24/25 4:07 AM, Pop Ioan Daniel wrote:
> Add support for setting decimation rate.
> 
> Add separate compatible string for the custom AD7405 IP and implement
> the necessary changes.
> 
> Signed-off-by: Pop Ioan Daniel <pop.ioan-daniel@analog.com>
> ---
>  drivers/iio/adc/adi-axi-adc.c | 43 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
> index 766406f45396..0ed609e294ba 100644
> --- a/drivers/iio/adc/adi-axi-adc.c
> +++ b/drivers/iio/adc/adi-axi-adc.c
> @@ -70,6 +70,9 @@
>  #define ADI_AXI_ADC_REG_CHAN_CTRL_3(c)		(0x0418 + (c) * 0x40)
>  #define   ADI_AXI_ADC_CHAN_PN_SEL_MASK		GENMASK(19, 16)
>  
> +#define ADI_AXI_ADC_REG_CHAN_USR_CTRL_2	0x0424

This is a per-channel register, so expect to have a c parameter times offset
like the other channel registers.

> +#define   ADI_AXI_ADC_DEC_RATE_MASK		GENMASK(15, 0)

Usually, we try to include the register name in these as well even if the name
does get a bit long.

ADI_AXI_ADC_CHAN_USR_CTRL_2_DEC_RATE_N_MASK

> +
>  /* IO Delays */
>  #define ADI_AXI_ADC_REG_DELAY(l)		(0x0800 + (l) * 0x4)
>  #define   AXI_ADC_DELAY_CTRL_MASK		GENMASK(4, 0)
> @@ -232,6 +235,16 @@ static int axi_adc_test_pattern_set(struct iio_backend *back,
>  	}
>  }
>  
> +static int axi_adc_set_dec_rate(struct iio_backend *back,
> +				unsigned int rate)

This needs to take a channel index parameter as well to keep it generic.

> +{
> +	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
> +

Might be helpful for future users if we add a comment explaining why we are
setting the denominator but not the numerator.

> +	return regmap_update_bits(st->regmap, (ADI_AXI_ADC_REG_CHAN_USR_CTRL_2),
> +					      ADI_AXI_ADC_DEC_RATE_MASK,
> +					      FIELD_PREP(ADI_AXI_ADC_DEC_RATE_MASK, rate));
> +}
> +
>  static int axi_adc_read_chan_status(struct adi_axi_adc_state *st, unsigned int chan,
>  				    unsigned int *status)
>  {
> @@ -465,6 +478,28 @@ static const struct iio_backend_info adi_axi_adc_generic = {
>  	.ops = &adi_axi_adc_ops,
>  };
>  
> +static const struct iio_backend_ops adi_ad7405_ops = {
> +	.enable = axi_adc_enable,
> +	.disable = axi_adc_disable,
> +	.data_format_set = axi_adc_data_format_set,
> +	.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,
> +	.iodelay_set = axi_adc_iodelays_set,
> +	.test_pattern_set = axi_adc_test_pattern_set,
> +	.set_dec_rate = axi_adc_set_dec_rate,
> +	.chan_status = axi_adc_chan_status,
> +	.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),
> +};

As mentioned in my DT bindings patch reply, it looks like this register is part
of the generic AXI ADC IP block, so we shouldn't need to add a chip-specific
struct in this case.

> +
> +static const struct iio_backend_info axi_ad7405 = {
> +	.name = "axi-ad7405",
> +	.ops = &adi_ad7405_ops,
> +};
> +
>  static int adi_axi_adc_probe(struct platform_device *pdev)
>  {
>  	struct adi_axi_adc_state *st;
> @@ -522,7 +557,7 @@ static int adi_axi_adc_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  	}
>  
> -	ret = devm_iio_backend_register(&pdev->dev, &adi_axi_adc_generic, st);
> +	ret = devm_iio_backend_register(&pdev->dev, st->info->backend_info, st);

I'm guessing you haven't rebased on the iio tree recently. This line was already
changed in [1].

[1]: https://web.git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/drivers/iio/adc/adi-axi-adc.c?h=testing&id=7a794e3a0dc5b1df525bf72260b641d70e337784

>  	if (ret)
>  		return dev_err_probe(&pdev->dev, ret,
>  				     "failed to register iio backend\n");
> @@ -575,10 +610,16 @@ static const struct axi_adc_info adc_ad7606 = {
>  	.has_child_nodes = true,
>  };
>  
> +static const struct axi_adc_info adi_axi_ad7405 = {
> +	.version = ADI_AXI_PCORE_VER(10, 0, 'a'),
> +	.backend_info = &axi_ad7405,
> +};
> +
>  /* 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-ad7606x", .data = &adc_ad7606 },
> +	{ .compatible = "adi,axi-ad7405", .data = &adi_axi_ad7405},
>  	{ /* end of list */ }
>  };
>  MODULE_DEVICE_TABLE(of, adi_axi_adc_of_match);


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/5] dt-bindings: iio: adc: add ad7405 axi variant
  2025-03-24  9:07 ` [PATCH 3/5] dt-bindings: iio: adc: add ad7405 axi variant Pop Ioan Daniel
@ 2025-03-24 18:55   ` David Lechner
  0 siblings, 0 replies; 18+ messages in thread
From: David Lechner @ 2025-03-24 18:55 UTC (permalink / raw)
  To: Pop Ioan Daniel, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Nuno Sa, Olivier Moysan, Javier Carrasco, Andy Shevchenko,
	Trevor Gamblin, Guillaume Stols, Dumitru Ceclan, Matteo Martelli,
	Marcelo Schmitt, Alisa-Dariana Roman, Thomas Bonnefille,
	Ramona Alexandra Nechita, Dragos Bogdan, linux-iio, devicetree,
	linux-kernel

On 3/24/25 4:07 AM, Pop Ioan Daniel wrote:
> Add a new compatible and related bindings for the fpga-based ad7405 AXI

It's just a new compatible string. There don't seem to be any other related
bindings. So we don't need to say that in the description. 

And typically, FGPA is written in all caps.

> IP core, a variant of the generic AXI ADC IP.
> 
> The AXI AD7405 IP is a very similar HDL (fpga) variant of the generic
> AXI ADC IP, intended to control ad7405/adum770x family.

It helps the DT maintainers review if we can say specifically what the
difference is. In this case, I don't actually seen any registers that are
different from the generic AXI ADC IP block, so I'm not sure we actually need
a new compatible string in this case.

The REG_CHAN_USR_CNTRL_2 that you add in this series for the decimation rate
is documented in the generic IP block [1]. So the generic "adi,axi-adc-10.0.a"
should work. The chips that needed their own compatible have conflicting uses
for CUSTOM_CONTROL in the CNTRL_3 register, but it looks like this isn't used
for ad7405. 

[1]: https://wiki.analog.com/resources/fpga/docs/axi_adc_ip

> 
> Signed-off-by: Pop Ioan Daniel <pop.ioan-daniel@analog.com>
> ---
>  Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml b/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
> index 4fa82dcf6fc9..1b02217ff8b5 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_ad7405/index.html
>    http://analogdevicesinc.github.io/hdl/library/axi_ad7606x/index.html
>  
>  properties:
> @@ -34,6 +35,7 @@ properties:
>      enum:
>        - adi,axi-adc-10.0.a
>        - adi,axi-ad7606x
> +      - adi,axi-ad7405
>  
>    reg:
>      maxItems: 1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/5] dt-bindings: iio: adc: add ad7405
  2025-03-24  9:07 ` [PATCH 4/5] dt-bindings: iio: adc: add ad7405 Pop Ioan Daniel
@ 2025-03-24 18:56   ` David Lechner
  2025-03-24 20:04   ` Rob Herring
  2025-03-30 15:40   ` Jonathan Cameron
  2 siblings, 0 replies; 18+ messages in thread
From: David Lechner @ 2025-03-24 18:56 UTC (permalink / raw)
  To: Pop Ioan Daniel, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Nuno Sa, Olivier Moysan, Javier Carrasco, Andy Shevchenko,
	Trevor Gamblin, Guillaume Stols, Dumitru Ceclan, Matteo Martelli,
	Marcelo Schmitt, Alisa-Dariana Roman, Ramona Alexandra Nechita,
	Thomas Bonnefille, Dragos Bogdan, linux-iio, devicetree,
	linux-kernel

On 3/24/25 4:07 AM, Pop Ioan Daniel wrote:
> Add devicetree bindings for ad7405/adum770x family.
> 
> Signed-off-by: Pop Ioan Daniel <pop.ioan-daniel@analog.com>
> ---
>  .../bindings/iio/adc/adi,ad7405.yaml          | 68 +++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7405.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7405.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7405.yaml
> new file mode 100644
> index 000000000000..e312fa0cdb05
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7405.yaml
> @@ -0,0 +1,68 @@
> +# 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,ad7405.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD7405 family
> +
> +maintainers:
> +  - Dragos Bogdan <dragos.bogdan@analog.com>
> +
> +description: |
> +  Analog Devices AD7405 is a high performance isolated ADC, 1-channel,
> +  16-bit with a second-order Σ-Δ modulator that converts an analog input signal
> +  into a high speed, single-bit data stream.
> +
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/ad7405.pdf
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/adum7701.pdf
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/adum7702.pdf
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/ADuM7703.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad7405
> +      - adi,adum7701
> +      - adi,adum7702
> +      - adi,adum7703
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +

There are no child nodes, so we don't need #address-cells or #size-cells.


> +  vdd1-supply: true
> +
> +  vdd2-supply: true
> +
> +  clocks:
> +    maxitems: 1
> +
> +  io-backends:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - vdd1-supply
> +  - vdd2-supply
> +  - clocks
> +  - io-backends
> +
> +unevaluatedProperties: false

Should be able to use additionalProperties: false here instead.

> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>

This include isn't used and can be removed.

> +    adc {
> +       #address-cells = <1>;
> +        #size-cells = <0>;

Not needed as above.

> +        compatible = "adi,ad7405";
> +        clocks = <&axi_clk_gen 0>;
> +        vdd1-supply = <&vdd1>;
> +        vdd2-supply = <&vdd2>;
> +        io-backends = <&iio_backend>;
> +    };
> +...
> \ No newline at end of file


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/5] iio: adc: ad7405: add ad7405 driver
  2025-03-24  9:08 ` [PATCH 5/5] iio: adc: ad7405: add ad7405 driver Pop Ioan Daniel
  2025-03-24 10:21   ` Andy Shevchenko
@ 2025-03-24 19:09   ` David Lechner
  2025-03-30 16:01   ` Jonathan Cameron
  2 siblings, 0 replies; 18+ messages in thread
From: David Lechner @ 2025-03-24 19:09 UTC (permalink / raw)
  To: Pop Ioan Daniel, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Nuno Sa, Olivier Moysan, Javier Carrasco, Andy Shevchenko,
	Guillaume Stols, Trevor Gamblin, Dumitru Ceclan, Matteo Martelli,
	AngeloGioacchino Del Regno, Alisa-Dariana Roman, Michael Walle,
	Herve Codina, Thomas Bonnefille, Dragos Bogdan, linux-iio,
	devicetree, linux-kernel

On 3/24/25 4:08 AM, Pop Ioan Daniel wrote:
> Add support for the AD7405/ADUM770x, a high performance isolated ADC,
> 1-channel, 16-bit with a second-order Σ-Δ modulator that converts an
> analog input signal into a high speed, single-bit data stream.
> 

Dragos is listed as the MODULE_AUTHOR, so would expect to see Co-developed-by:
and Signed-off-by: tags for him as well, assuming he wrote some of this code.

More info: https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

> Signed-off-by: Pop Ioan Daniel <pop.ioan-daniel@analog.com>
> ---
>  drivers/iio/adc/Kconfig  |  10 ++
>  drivers/iio/adc/Makefile |   1 +
>  drivers/iio/adc/ad7405.c | 301 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 312 insertions(+)
>  create mode 100644 drivers/iio/adc/ad7405.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index f64b5faeb257..321a1ee7304f 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -203,6 +203,16 @@ config AD7380
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called ad7380.
>  
> +config AD7405
> +	tristate "Analog Device AD7405 ADC Driver"
> +	select IIO_BACKEND
> +	help
> +	  Say yes here to build support for Analog Devices AD7405, ADUM7701,
> +	  ADUM7702, ADUM7703 analog to digital converters (ADC).
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called ad7405.
> +
>  config AD7476
>  	tristate "Analog Devices AD7476 1-channel ADCs driver and other similar devices from AD and TI"
>  	depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index ee19afba62b7..0c3c1c69b6b4 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_AD7291) += ad7291.o
>  obj-$(CONFIG_AD7292) += ad7292.o
>  obj-$(CONFIG_AD7298) += ad7298.o
>  obj-$(CONFIG_AD7380) += ad7380.o
> +obj-$(CONFIG_AD7405) += ad7405.o
>  obj-$(CONFIG_AD7476) += ad7476.o
>  obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o
>  obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o
> diff --git a/drivers/iio/adc/ad7405.c b/drivers/iio/adc/ad7405.c
> new file mode 100644
> index 000000000000..40fe072369d5
> --- /dev/null
> +++ b/drivers/iio/adc/ad7405.c
> @@ -0,0 +1,301 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Analog Devices AD7405 driver
> + *
> + * Copyright 2025 Analog Devices Inc.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/log2.h>
> +#include <linux/clk.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/backend.h>
> +#include <linux/util_macros.h>
> +#include <linux/regulator/consumer.h>

Sort the includes in alphabetical order. And prune headers that aren't used
like log2.h and of.h.

> +
> +#define AD7405_DEFAULT_DEC_RATE 1024
> +
> +const unsigned int ad7405_dec_rates[] = {
> +		4096, 2048, 1024, 512, 256, 128, 64, 32,
> +};
> +
> +struct ad7405_chip_info {
> +	const char *name;
> +	unsigned int num_channels;
> +	unsigned int max_rate;
> +	unsigned int min_rate;
> +	struct iio_chan_spec channel[3];

Currently, all chips only have one channel, so we can leave out num_channels
and not use an array for the single struct iio_chan_spec.

> +	const unsigned long *available_mask;
> +};
> +
> +struct ad7405_state {
> +	struct iio_backend *back;
> +	struct clk *axi_clk_gen;

Just call it clk. Also, if we don't need to access it outside of probe, we
don't need it in this struct.

> +	/* lock to protect multiple accesses to the device registers */
> +	struct mutex lock;
> +	struct regmap *regmap;

These are not used, so should be removed.

> +	struct iio_info iio_info;

Don't need to have a copy in this struct.

> +	const struct ad7405_chip_info *info;
> +	unsigned int sample_frequency_tbl[ARRAY_SIZE(ad7405_dec_rates)];
> +	unsigned int sample_frequency;
> +	unsigned int ref_frequency;
> +};
> +
> +static void ad7405_fill_samp_freq_table(struct ad7405_state *st)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(ad7405_dec_rates); i++)
> +		st->sample_frequency_tbl[i] = DIV_ROUND_CLOSEST_ULL(st->ref_frequency, ad7405_dec_rates[i]);

Wrap to 80 chars.

> +}
> +
> +static int ad7405_set_sampling_rate(struct iio_dev *indio_dev,
> +				    const struct iio_chan_spec *chan,
> +				    unsigned int samp_rate)
> +{
> +	struct ad7405_state *st = iio_priv(indio_dev);
> +	unsigned int dec_rate, idx;
> +	int ret;
> +
> +	dec_rate = DIV_ROUND_CLOSEST_ULL(st->ref_frequency, samp_rate);
> +
> +	idx = find_closest_descending(dec_rate, ad7405_dec_rates,
> +				      ARRAY_SIZE(ad7405_dec_rates));
> +
> +	    dec_rate = ad7405_dec_rates[idx];
> +
> +	ret = iio_backend_set_dec_rate(st->back, dec_rate);
> +	if (ret)
> +		return ret;
> +
> +	st->sample_frequency = DIV_ROUND_CLOSEST_ULL(st->ref_frequency, dec_rate);
> +
> +	return 0;
> +}
> +
> +static int ad7405_update_scan_mode(struct iio_dev *indio_dev,
> +				   const unsigned long *scan_mask)
> +{
> +	struct ad7405_state *st = iio_priv(indio_dev);
> +	unsigned int c;
> +	int ret;
> +
> +	for (c = 0; c < indio_dev->num_channels; c++) {
> +		if (test_bit(c, scan_mask))
> +			ret = iio_backend_chan_enable(st->back, c);
> +		else
> +			ret = iio_backend_chan_disable(st->back, c);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ad7405_read_raw(struct iio_dev *indio_dev,
> +			   const struct iio_chan_spec *chan, int *val,
> +			   int *val2, long info)
> +{
> +	struct ad7405_state *st = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +			*val = st->sample_frequency;
> +
> +			return IIO_VAL_INT;
> +	default:
> +			return -EINVAL;
> +	}
> +}
> +
> +static int ad7405_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan, int val,
> +			    int val2, long info)
> +{
> +	switch (info) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +

Need to return -EINVAL on val = 0 to avoid divide by zero crash.

> +			return ad7405_set_sampling_rate(indio_dev, chan, val);
> +
> +	default:
> +			return -EINVAL;
> +	}
> +}
> +
> +static int ad7405_read_avail(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +				 const int **vals, int *type, int *length,
> +				 long info)
> +{
> +	struct ad7405_state *st = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +			*vals = st->sample_frequency_tbl;
> +			*length = ARRAY_SIZE(st->sample_frequency_tbl);
> +			*type = IIO_VAL_INT;
> +			return IIO_AVAIL_LIST;
> +	default:
> +			return -EINVAL;
> +	}
> +}

./scripts/checkpatch.pl should be catching issues with indentation style in the
functions above.

> +
> +static const struct iio_info ad7405_iio_info = {
> +	.read_raw = &ad7405_read_raw,
> +	.write_raw = &ad7405_write_raw,
> +	.read_avail = &ad7405_read_avail,
> +	.update_scan_mode = ad7405_update_scan_mode,
> +};
> +
> +#define AD7405_IIO_CHANNEL(_chan, _bits, _sign)		  \

chan, bits and sign are always the same, so we could omit these paramters.

> +	{ .type = IIO_VOLTAGE,					  \

We need info_mask_shared_by_type (or _separate) with IIO_CHAN_INFO_SCALE and
IIO_CHAN_INFO_OFFSET flags so that userspace knows how to convert raw data to
the standard unit of millivolts.

> +	  .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> +	  .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> +	  .indexed = 1,						 \
> +	  .channel = _chan,					 \

Also needs .scan_index = _chan, .differential = 1, .channel2 = _chan + 1,

> +	  .scan_type = {				\
> +		.sign = _sign,				\
> +		.realbits = _bits,			\
> +		.storagebits = 16,			\
> +		.shift = 0,				\
> +	  },						\
> +	}
> +
> +static const unsigned long ad7405_channel_masks[] = {
> +		BIT(0),
> +		0,
> +};

This should not be need since there is only one channel.

> +
> +static const struct ad7405_chip_info ad7405_chip_info = {
> +		.name = "AD7405",
> +		.max_rate = 625000UL,
> +		.min_rate = 4883UL,

Doesn't the max rate depend on the clock frequency? So not sure how useful it
is to specify this.

min_rate is not used anywhere, so can be omitted.

> +		.num_channels = 1,
> +		.channel = {
> +			AD7405_IIO_CHANNEL(0, 16, 'u'),
> +		},
> +		.available_mask = ad7405_channel_masks,
> +};
> +
> +static const struct ad7405_chip_info adum7701_chip_info = {
> +		.name = "ADUM7701",
> +		.max_rate = 656250UL,
> +		.min_rate = 5127UL,
> +		.num_channels = 1,
> +		.channel = {
> +			AD7405_IIO_CHANNEL(0, 16, 'u'),
> +		},
> +		.available_mask = ad7405_channel_masks,
> +};
> +
> +static const char * const ad7405_power_supplies[] = {
> +	"vdd1",	"vdd2",
> +};
> +
> +static int ad7405_probe(struct platform_device *pdev)
> +{
> +	const struct ad7405_chip_info *chip_info;
> +	struct device *dev = &pdev->dev;
> +	struct iio_dev *indio_dev;
> +	struct ad7405_state *st;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +
> +	ret = devm_mutex_init(dev, &st->lock);
> +	if (ret)
> +		return ret;
> +
> +	chip_info = &ad7405_chip_info;

This uses the same chip info for all chips and ignores the .data in the module
device table.

> +
> +	platform_set_drvdata(pdev, indio_dev);

There is no platform_get_drvdata(), so this is unnecessary.

> +
> +	st->axi_clk_gen = devm_clk_get(dev, NULL);

Can be simplified to devm_clk_get_enabled().

> +	if (IS_ERR(st->axi_clk_gen))
> +		return PTR_ERR(st->axi_clk_gen);
> +
> +	ret = clk_prepare_enable(st->axi_clk_gen);
> +	if (ret)
> +		return ret;

Otherwise we also need to add something to disable_unprepare the clock when
the driver is removed.

> +
> +	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(ad7405_power_supplies),
> +					     ad7405_power_supplies);

I didn't see anything in the datasheet about power up sequencing, but typically
we would turn on power to the chip first before applying any other signals, like
the clock.

> +
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to get and enable supplies");
> +
> +	st->ref_frequency = clk_get_rate(st->axi_clk_gen);

Should check for return value of 0 and raise an error, otherwise we would get
divide by zero crash later.

> +
> +	ad7405_fill_samp_freq_table(st);
> +
> +	indio_dev->dev.parent = dev;
> +	indio_dev->name = pdev->dev.of_node->name;

I think usually this is chip_info->name rather than the DT node name.

> +	indio_dev->modes = INDIO_DIRECT_MODE;

IIO_CHAN_INFO_RAW isn't implemented, so INDIO_DIRECT_MODE should not be set.

> +
> +	indio_dev->channels = chip_info->channel;
> +	indio_dev->num_channels = chip_info->num_channels;
> +
> +	st->iio_info = ad7405_iio_info;
> +	indio_dev->info = &st->iio_info;
> +
> +	st->back = devm_iio_backend_get(dev, NULL);
> +	if (IS_ERR(st->back))
> +		return dev_err_probe(dev, PTR_ERR(st->back),
> +				     "failed to get IIO backend");
> +
> +	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;
> +
> +	/* Reset all HDL Cores */
> +	iio_backend_disable(st->back);
> +	iio_backend_enable(st->back);

Seems like this would be redunant and should be done implicitly by
devm_iio_backend_enable() (i.e. disable in adi_axi_adc_probe() so that 
devm_iio_backend_enable() brings it out of reset).

> +
> +	ret = ad7405_set_sampling_rate(indio_dev, &indio_dev->channels[0],
> +				       chip_info->max_rate);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_iio_device_register(dev, indio_dev);

Can just return directly here.

> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +/* Match table for of_platform binding */
> +static const struct of_device_id ad7405_of_match[] = {
> +	{ .compatible = "adi,ad7405", .data = &ad7405_chip_info, },
> +	{ .compatible = "adi,adum7701", .data = &adum7701_chip_info, },
> +	{ .compatible = "adi,adum7702", .data = &adum7701_chip_info, },
> +	{ .compatible = "adi,adum7703", .data = &adum7701_chip_info, },
> +	{ /* end of list */ },
> +};
> +
> +MODULE_DEVICE_TABLE(of, ad7405_of_match);
> +
> +static struct platform_driver ad7405_driver = {
> +	.driver = {
> +		.name = "ad7405",
> +		.owner = THIS_MODULE,
> +		.of_match_table = ad7405_of_match,
> +	},
> +	.probe = ad7405_probe,
> +};
> +
> +module_platform_driver(ad7405_driver);
> +
> +MODULE_AUTHOR("Dragos Bogdan <dragos.bogdan@analog.com>");
> +MODULE_DESCRIPTION("Analog Devices AD7405 driver");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("IIO_BACKEND");


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/5] dt-bindings: iio: adc: add ad7405
  2025-03-24  9:07 ` [PATCH 4/5] dt-bindings: iio: adc: add ad7405 Pop Ioan Daniel
  2025-03-24 18:56   ` David Lechner
@ 2025-03-24 20:04   ` Rob Herring
  2025-03-30 15:40   ` Jonathan Cameron
  2 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2025-03-24 20:04 UTC (permalink / raw)
  To: Pop Ioan Daniel
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Krzysztof Kozlowski, Conor Dooley, Nuno Sa, Olivier Moysan,
	David Lechner, Javier Carrasco, Andy Shevchenko, Trevor Gamblin,
	Guillaume Stols, Dumitru Ceclan, Matteo Martelli, Marcelo Schmitt,
	Alisa-Dariana Roman, Ramona Alexandra Nechita, Thomas Bonnefille,
	Dragos Bogdan, linux-iio, devicetree, linux-kernel

On Mon, Mar 24, 2025 at 11:07:59AM +0200, Pop Ioan Daniel wrote:
> Add devicetree bindings for ad7405/adum770x family.
> 
> Signed-off-by: Pop Ioan Daniel <pop.ioan-daniel@analog.com>
> ---
>  .../bindings/iio/adc/adi,ad7405.yaml          | 68 +++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7405.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7405.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7405.yaml
> new file mode 100644
> index 000000000000..e312fa0cdb05
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7405.yaml
> @@ -0,0 +1,68 @@
> +# 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,ad7405.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD7405 family
> +
> +maintainers:
> +  - Dragos Bogdan <dragos.bogdan@analog.com>
> +
> +description: |
> +  Analog Devices AD7405 is a high performance isolated ADC, 1-channel,
> +  16-bit with a second-order Σ-Δ modulator that converts an analog input signal
> +  into a high speed, single-bit data stream.
> +
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/ad7405.pdf
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/adum7701.pdf
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/adum7702.pdf
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/ADuM7703.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad7405
> +      - adi,adum7701
> +      - adi,adum7702
> +      - adi,adum7703
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +  vdd1-supply: true
> +
> +  vdd2-supply: true
> +
> +  clocks:
> +    maxitems: 1
> +
> +  io-backends:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - vdd1-supply
> +  - vdd2-supply
> +  - clocks
> +  - io-backends
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    adc {
> +       #address-cells = <1>;
> +        #size-cells = <0>;
> +        compatible = "adi,ad7405";
> +        clocks = <&axi_clk_gen 0>;
> +        vdd1-supply = <&vdd1>;
> +        vdd2-supply = <&vdd2>;
> +        io-backends = <&iio_backend>;
> +    };
> +...
> \ No newline at end of file

Fix this.

With that,

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/5] iio: backend: add support for decimation ratio set
  2025-03-24 18:53   ` David Lechner
@ 2025-03-28  8:31     ` Nuno Sá
  0 siblings, 0 replies; 18+ messages in thread
From: Nuno Sá @ 2025-03-28  8:31 UTC (permalink / raw)
  To: David Lechner, Pop Ioan Daniel, Lars-Peter Clausen,
	Michael Hennerich, Jonathan Cameron, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Nuno Sa, Olivier Moysan,
	Javier Carrasco, Andy Shevchenko, Guillaume Stols, Trevor Gamblin,
	Dumitru Ceclan, Matteo Martelli, João Paulo Gonçalves,
	Alisa-Dariana Roman, Thomas Bonnefille, Herve Codina,
	Marcelo Schmitt, Dragos Bogdan, linux-iio, devicetree,
	linux-kernel

On Mon, 2025-03-24 at 13:53 -0500, David Lechner wrote:
> On 3/24/25 4:07 AM, Pop Ioan Daniel wrote:
> > Add backend support for setting the decimation ratio used.
> > 
> > Signed-off-by: Pop Ioan Daniel <pop.ioan-daniel@analog.com>
> > ---
> >  drivers/iio/industrialio-backend.c | 18 ++++++++++++++++++
> >  include/linux/iio/backend.h        |  3 +++
> >  2 files changed, 21 insertions(+)
> > 
> > diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-
> > backend.c
> > index 363281272035..f4db6514944a 100644
> > --- a/drivers/iio/industrialio-backend.c
> > +++ b/drivers/iio/industrialio-backend.c
> > @@ -417,6 +417,24 @@ int iio_backend_test_pattern_set(struct iio_backend *back,
> >  }
> >  EXPORT_SYMBOL_NS_GPL(iio_backend_test_pattern_set, "IIO_BACKEND");
> >  
> > +/**
> > + * iio_backend_set_dec_rate - set decimation ratio
> 
> In [1], we decided that for a similar chip we could use "decimation rate" and
> "oversampling ratio" interchangeably. And in [2], we recently added a backend
> op for setting the oversampling ratio. Would it make sense to use that here
> as well instead of introducing a new function? If not, we'll want more of an
> explanation here on what the difference is. But from what I can tell, this
> sounds very similar to the other case where they are essentially the same thing.
> On the other hand, this is internal API and not userspace ABI, so having a
> separate name might not be so bad, especially if there is a chance we would ever
> have both at the same time.

If it fits, I have preference for using existent interfaces...

- Nuno Sá
> 
> [1]: https://lore.kernel.org/linux-iio/20250112122047.1e1978e0@jic23-huawei/
> [2]:
> https://web.git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/drivers/iio/industrialio-backend.c?h=testing&id=22894e0be908791e3df011cdfac02589c2f340bd
> 
> > + * @back: Backend device
> > + * @rate: Rate in decimal
> > +
> > + * Return:
> > + * 0 on success, negative error number on failure.
> > + */
> > +
> > +int iio_backend_set_dec_rate(struct iio_backend *back, unsigned int rate)
> > +{
> > +	if (!rate)
> > +		return -EINVAL;
> > +
> > +	return iio_backend_op_call(back, set_dec_rate, rate);
> > +}
> > +EXPORT_SYMBOL_NS_GPL(iio_backend_set_dec_rate, "IIO_BACKEND");
> > +


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/5] dt-bindings: iio: adc: add ad7405
  2025-03-24  9:07 ` [PATCH 4/5] dt-bindings: iio: adc: add ad7405 Pop Ioan Daniel
  2025-03-24 18:56   ` David Lechner
  2025-03-24 20:04   ` Rob Herring
@ 2025-03-30 15:40   ` Jonathan Cameron
  2 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2025-03-30 15:40 UTC (permalink / raw)
  To: Pop Ioan Daniel
  Cc: Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Nuno Sa, Olivier Moysan,
	David Lechner, Javier Carrasco, Andy Shevchenko, Trevor Gamblin,
	Guillaume Stols, Dumitru Ceclan, Matteo Martelli, Marcelo Schmitt,
	Alisa-Dariana Roman, Ramona Alexandra Nechita, Thomas Bonnefille,
	Dragos Bogdan, linux-iio, devicetree, linux-kernel

On Mon, 24 Mar 2025 11:07:59 +0200
Pop Ioan Daniel <pop.ioan-daniel@analog.com> wrote:

> Add devicetree bindings for ad7405/adum770x family.
> 
> Signed-off-by: Pop Ioan Daniel <pop.ioan-daniel@analog.com>
Hi Pop,

One more trivial thing inline.
> ---
>  .../bindings/iio/adc/adi,ad7405.yaml          | 68 +++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7405.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7405.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7405.yaml
> new file mode 100644
> index 000000000000..e312fa0cdb05
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7405.yaml
> @@ -0,0 +1,68 @@
> +# 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,ad7405.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#

> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    adc {
> +       #address-cells = <1>;

Odd alignment going on here. 1 more space I think.

> +        #size-cells = <0>;
> +        compatible = "adi,ad7405";
> +        clocks = <&axi_clk_gen 0>;
> +        vdd1-supply = <&vdd1>;
> +        vdd2-supply = <&vdd2>;
> +        io-backends = <&iio_backend>;
> +    };
> +...
> \ No newline at end of file


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/5] iio: adc: ad7405: add ad7405 driver
  2025-03-24  9:08 ` [PATCH 5/5] iio: adc: ad7405: add ad7405 driver Pop Ioan Daniel
  2025-03-24 10:21   ` Andy Shevchenko
  2025-03-24 19:09   ` David Lechner
@ 2025-03-30 16:01   ` Jonathan Cameron
  2 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2025-03-30 16:01 UTC (permalink / raw)
  To: Pop Ioan Daniel
  Cc: Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Nuno Sa, Olivier Moysan,
	David Lechner, Javier Carrasco, Andy Shevchenko, Guillaume Stols,
	Trevor Gamblin, Dumitru Ceclan, Matteo Martelli,
	AngeloGioacchino Del Regno, Alisa-Dariana Roman, Michael Walle,
	Herve Codina, Thomas Bonnefille, Dragos Bogdan, linux-iio,
	devicetree, linux-kernel

On Mon, 24 Mar 2025 11:08:00 +0200
Pop Ioan Daniel <pop.ioan-daniel@analog.com> wrote:

> Add support for the AD7405/ADUM770x, a high performance isolated ADC,
> 1-channel, 16-bit with a second-order Σ-Δ modulator that converts an
> analog input signal into a high speed, single-bit data stream.
> 
> Signed-off-by: Pop Ioan Daniel <pop.ioan-daniel@analog.com>
Hi Pop,

I'll probably overlap with existing reviews but maybe there will
be other stuff.

Dragos is listed as the maintainer. If that's accurate should have a
Co-developed by and sign off.

> ---
>  drivers/iio/adc/Kconfig  |  10 ++
>  drivers/iio/adc/Makefile |   1 +
>  drivers/iio/adc/ad7405.c | 301 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 312 insertions(+)
>  create mode 100644 drivers/iio/adc/ad7405.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index f64b5faeb257..321a1ee7304f 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -203,6 +203,16 @@ config AD7380
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called ad7380.
>  
> +config AD7405
> +	tristate "Analog Device AD7405 ADC Driver"
> +	select IIO_BACKEND
> +	help
> +	  Say yes here to build support for Analog Devices AD7405, ADUM7701,
> +	  ADUM7702, ADUM7703 analog to digital converters (ADC).

Maybe mention the bus?

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

> diff --git a/drivers/iio/adc/ad7405.c b/drivers/iio/adc/ad7405.c
> new file mode 100644
> index 000000000000..40fe072369d5
> --- /dev/null
> +++ b/drivers/iio/adc/ad7405.c
> @@ -0,0 +1,301 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Analog Devices AD7405 driver
> + *
> + * Copyright 2025 Analog Devices Inc.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/log2.h>
> +#include <linux/clk.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>

Use property.h if you need to access firmware properties.
For the table,  use
mod_devicetable.h not of.h.


> +#include <linux/iio/iio.h>
> +#include <linux/iio/backend.h>
> +#include <linux/util_macros.h>
> +#include <linux/regulator/consumer.h>
> +
> +#define AD7405_DEFAULT_DEC_RATE 1024
> +
> +const unsigned int ad7405_dec_rates[] = {
> +		4096, 2048, 1024, 512, 256, 128, 64, 32,

1 tab only is enough here.

> +};
> +
> +struct ad7405_chip_info {
> +	const char *name;
> +	unsigned int num_channels;
> +	unsigned int max_rate;
> +	unsigned int min_rate;
> +	struct iio_chan_spec channel[3];
Why 3? 

> +	const unsigned long *available_mask;
As below - this makes little sense. Drop it.

> +};
> +
> +struct ad7405_state {
> +	struct iio_backend *back;
> +	struct clk *axi_clk_gen;
> +	/* lock to protect multiple accesses to the device registers */

Why  I assume this is about read modify update sequences? 
If so say something more about that.  For now I can't see any
happening. All we have is an update of the backend sampling frequency.

> +	struct mutex lock;
> +	struct regmap *regmap;
Note used. Drop it. 
> +	struct iio_info iio_info;
As noted below, not obvious what this is for.

> +	const struct ad7405_chip_info *info;
> +	unsigned int sample_frequency_tbl[ARRAY_SIZE(ad7405_dec_rates)];
> +	unsigned int sample_frequency;
> +	unsigned int ref_frequency;
> +};
> +
> +static void ad7405_fill_samp_freq_table(struct ad7405_state *st)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(ad7405_dec_rates); i++)
> +		st->sample_frequency_tbl[i] = DIV_ROUND_CLOSEST_ULL(st->ref_frequency, ad7405_dec_rates[i]);
Wrap this line.
		st->sample_frequency_tbl[i] =
			DIV_ROUND_CLOSEST_ULL(st->ref_frequency, ad7405_dec_rates[i]);

> +}
> +
> +static int ad7405_set_sampling_rate(struct iio_dev *indio_dev,
> +				    const struct iio_chan_spec *chan,
> +				    unsigned int samp_rate)
> +{
> +	struct ad7405_state *st = iio_priv(indio_dev);
> +	unsigned int dec_rate, idx;
> +	int ret;
> +
> +	dec_rate = DIV_ROUND_CLOSEST_ULL(st->ref_frequency, samp_rate);
> +
> +	idx = find_closest_descending(dec_rate, ad7405_dec_rates,
> +				      ARRAY_SIZE(ad7405_dec_rates));
> +
> +	    dec_rate = ad7405_dec_rates[idx];

Odd indent.

> +
> +	ret = iio_backend_set_dec_rate(st->back, dec_rate);
> +	if (ret)
> +		return ret;
> +
> +	st->sample_frequency = DIV_ROUND_CLOSEST_ULL(st->ref_frequency, dec_rate);
> +
> +	return 0;
> +}
> +
> +static int ad7405_update_scan_mode(struct iio_dev *indio_dev,
> +				   const unsigned long *scan_mask)
> +{
> +	struct ad7405_state *st = iio_priv(indio_dev);
> +	unsigned int c;
> +	int ret;
> +

This is odd given you only have one channel.
Either do the enable in probe() or in postenable callback and
disable in predisable callback.


> +	for (c = 0; c < indio_dev->num_channels; c++) {
> +		if (test_bit(c, scan_mask))
> +			ret = iio_backend_chan_enable(st->back, c);
> +		else
> +			ret = iio_backend_chan_disable(st->back, c);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ad7405_read_raw(struct iio_dev *indio_dev,
> +			   const struct iio_chan_spec *chan, int *val,
> +			   int *val2, long info)
> +{
> +	struct ad7405_state *st = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +			*val = st->sample_frequency;
> +
> +			return IIO_VAL_INT;
> +	default:
> +			return -EINVAL;
> +	}
> +}
> +
> +static int ad7405_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan, int val,
> +			    int val2, long info)
> +{
> +	switch (info) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +
> +			return ad7405_set_sampling_rate(indio_dev, chan, val);
> +
> +	default:
> +			return -EINVAL;
> +	}
> +}
> +
> +static int ad7405_read_avail(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +				 const int **vals, int *type, int *length,
> +				 long info)
> +{
> +	struct ad7405_state *st = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +			*vals = st->sample_frequency_tbl;
> +			*length = ARRAY_SIZE(st->sample_frequency_tbl);
> +			*type = IIO_VAL_INT;
> +			return IIO_AVAIL_LIST;
> +	default:
> +			return -EINVAL;
> +	}
> +}
> +
> +static const struct iio_info ad7405_iio_info = {
> +	.read_raw = &ad7405_read_raw,
> +	.write_raw = &ad7405_write_raw,
> +	.read_avail = &ad7405_read_avail,
> +	.update_scan_mode = ad7405_update_scan_mode,
> +};
> +
> +#define AD7405_IIO_CHANNEL(_chan, _bits, _sign)		  \
> +	{ .type = IIO_VOLTAGE,					  \
	{
		.type = IIO_VOLTAGE,					\
etc.

That is keep to normal formatting.  If you go over 80 chars for readability
reasons that is fine.
> +	  .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> +	  .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> +	  .indexed = 1,						 \
> +	  .channel = _chan,					 \

For now channel always 0 so just set to 0.
Bring in the parameter only when you need it.

> +	  .scan_type = {				\
> +		.sign = _sign,				\
> +		.realbits = _bits,			\
> +		.storagebits = 16,			\
> +		.shift = 0,				\

Never any need to set shift to 0. It's the obvious default and the c spec
ensures it is set to 0 if you leave it unspecified.

> +	  },						\
> +	}
> +
> +static const unsigned long ad7405_channel_masks[] = {
> +		BIT(0),

If there is only one channel this serves no useful purpose.
Drop it.

> +		0,
> +};
> +
> +static const struct ad7405_chip_info ad7405_chip_info = {
> +		.name = "AD7405",
> +		.max_rate = 625000UL,
> +		.min_rate = 4883UL,
> +		.num_channels = 1,
> +		.channel = {
> +			AD7405_IIO_CHANNEL(0, 16, 'u'),
> +		},
> +		.available_mask = ad7405_channel_masks,
> +};
> +
> +static const struct ad7405_chip_info adum7701_chip_info = {
> +		.name = "ADUM7701",
> +		.max_rate = 656250UL,
> +		.min_rate = 5127UL,
> +		.num_channels = 1,
> +		.channel = {
> +			AD7405_IIO_CHANNEL(0, 16, 'u'),
> +		},
> +		.available_mask = ad7405_channel_masks,
> +};

> +static int ad7405_probe(struct platform_device *pdev)
> +{
> +	const struct ad7405_chip_info *chip_info;
> +	struct device *dev = &pdev->dev;
> +	struct iio_dev *indio_dev;
> +	struct ad7405_state *st;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +
> +	ret = devm_mutex_init(dev, &st->lock);
> +	if (ret)
> +		return ret;
> +
> +	chip_info = &ad7405_chip_info;
> +
> +	platform_set_drvdata(pdev, indio_dev);
Is this used anywhere? If not drop it.

> +
> +	st->axi_clk_gen = devm_clk_get(dev, NULL);
> +	if (IS_ERR(st->axi_clk_gen))
> +		return PTR_ERR(st->axi_clk_gen);
> +
> +	ret = clk_prepare_enable(st->axi_clk_gen);

why not dev_clk_get_enabled()?

> +	if (ret)
> +		return ret;
> +
> +	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(ad7405_power_supplies),
> +					     ad7405_power_supplies);
> +
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to get and enable supplies");
> +
> +	st->ref_frequency = clk_get_rate(st->axi_clk_gen);
> +
> +	ad7405_fill_samp_freq_table(st);
> +
> +	indio_dev->dev.parent = dev;

Set by the IIO core for you, so no need to do it explicitly unless you want
to modify it for a non obvious parent.


> +	indio_dev->name = pdev->dev.of_node->name;

First of all, don't use of_node directly. Always use the property.h accessors.
Secondly don't get the name from there - just hard code it so we can immediate
see what it is here. Given you are supporting multiple parts, and already
have it in the chip specific structure, just get it from there.


> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	indio_dev->channels = chip_info->channel;
> +	indio_dev->num_channels = chip_info->num_channels;
> +
> +	st->iio_info = ad7405_iio_info;
Why is the extra copy needed?  
> +	indio_dev->info = &st->iio_info;
> +
> +	st->back = devm_iio_backend_get(dev, NULL);
> +	if (IS_ERR(st->back))
> +		return dev_err_probe(dev, PTR_ERR(st->back),
> +				     "failed to get IIO backend");
> +
> +	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;
> +
> +	/* Reset all HDL Cores */

Needs more info.  Why is that needed for this specific part but not
for others?  I'd also expect the final one to be the devm case
if this sequence is needed.


> +	iio_backend_disable(st->back);
> +	iio_backend_enable(st->back);
> +
> +	ret = ad7405_set_sampling_rate(indio_dev, &indio_dev->channels[0],
> +				       chip_info->max_rate);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_iio_device_register(dev, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	return 0;

	return devm_iio...

> +}
> +
> +/* Match table for of_platform binding */
> +static const struct of_device_id ad7405_of_match[] = {
> +	{ .compatible = "adi,ad7405", .data = &ad7405_chip_info, },
> +	{ .compatible = "adi,adum7701", .data = &adum7701_chip_info, },
> +	{ .compatible = "adi,adum7702", .data = &adum7701_chip_info, },
> +	{ .compatible = "adi,adum7703", .data = &adum7701_chip_info, },
> +	{ /* end of list */ },
> +};

Similar to below. Common to not have a blank line here given
close association of macro and table.

> +
> +MODULE_DEVICE_TABLE(of, ad7405_of_match);
> +
> +static struct platform_driver ad7405_driver = {
> +	.driver = {
> +		.name = "ad7405",
> +		.owner = THIS_MODULE,
> +		.of_match_table = ad7405_of_match,
> +	},
> +	.probe = ad7405_probe,
> +};
> +
Common practice to not have a blank line here as the association
between the following macro and the platform_driver structure is
very close.
> +module_platform_driver(ad7405_driver);
> +
> +MODULE_AUTHOR("Dragos Bogdan <dragos.bogdan@analog.com>");

As above. If this is correct then from + sign-offs above may be inapproprite.

> +MODULE_DESCRIPTION("Analog Devices AD7405 driver");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("IIO_BACKEND");


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2025-03-30 16:01 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-24  9:07 [PATCH 0/5] Add support for AD7405/ADUM770x Pop Ioan Daniel
2025-03-24  9:07 ` [PATCH 1/5] iio: backend: add support for decimation ratio set Pop Ioan Daniel
2025-03-24 10:13   ` Andy Shevchenko
2025-03-24 18:53   ` David Lechner
2025-03-28  8:31     ` Nuno Sá
2025-03-24  9:07 ` [PATCH 2/5] iio: adc: adi-axi-adc: add set decimation rate Pop Ioan Daniel
2025-03-24 10:15   ` Andy Shevchenko
2025-03-24 18:54   ` David Lechner
2025-03-24  9:07 ` [PATCH 3/5] dt-bindings: iio: adc: add ad7405 axi variant Pop Ioan Daniel
2025-03-24 18:55   ` David Lechner
2025-03-24  9:07 ` [PATCH 4/5] dt-bindings: iio: adc: add ad7405 Pop Ioan Daniel
2025-03-24 18:56   ` David Lechner
2025-03-24 20:04   ` Rob Herring
2025-03-30 15:40   ` Jonathan Cameron
2025-03-24  9:08 ` [PATCH 5/5] iio: adc: ad7405: add ad7405 driver Pop Ioan Daniel
2025-03-24 10:21   ` Andy Shevchenko
2025-03-24 19:09   ` David Lechner
2025-03-30 16:01   ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).