[PATCH v2 0/8] iio: adc: dfsdm: add scaling support

[PATCH v2 0/8] iio: adc: dfsdm: add scaling support

[Olivier Moysan]

The aim of this serie is to add scaling support to STM32 DFSDM
peripheral in the analog context.

The DFSDM currently operates as a consumer of IIO channels
provided by a generic SD modulator. As previously discussed in RFC [1],
this topology is not suitable for implementing scaling.

This series brings the integration of the DFSDM driver with the new 
IIO backend framework [2], enabling the DFSDM IIO device to offer 
scaling feature based on reference voltage data obtained from the
IIO SD modulator backend. This generic SD modulator backend takes the
place of the former SD modulator, used with legacy implementation.

The DFSDM driver has been updated to adopt the generic ADC channel
binding [3]. The reasons for this include:
- Reducing the use of proprietary properties
- Simplifying the coexistence of legacy and new backend bindings
- Prepare the support of the MDF peripheral on STM32MP25 SoC

Backward compatibility is maintained through legacy support.

This series extends the backend framework with the following APIs:
- iio_backend_read_raw:
	This API is intented to retrieve the voltage information from the
	backend. It is based on IIO framework read_raw API.
- iio_backend_disable / iio_backend_enable:
	backend enable/disable to be used for PM management
- devm_iio_backend_fwnode_get
	Intended for parsing DT subnodes to allow generic channel binding
	support, as generic channel DT nodes are not populated as devices.

[1]: https://lore.kernel.org/lkml/20200204101008.11411-1-olivier.moysan@st.com/
[2]: https://lore.kernel.org/all/20240206-iio-backend-v9-0-df66d159c000@analog.com/
[3]: devicetree/bindings/iio/adc/adc.yaml

Changes in v2:
- Update enable/disable backend API
- Rename devm_iio_backend_subnode_get(), as devm_iio_backend_fwnode_get()
- Update iio_backend_read_raw() prototype to fully match IIO framework
  read_raw callback prototype.
- Change st,adc-channel-type property name and type in DFSDM binding
- Remove sd-backend and rename ads1201 compatibles in SD binding

Conor, in this v2, I left the SD modulator driver & binding unchanged,
regarding the naming issue you raised previously.

The problem here is that we have two versions of the generic sigma delta
modulator driver: one for legacy support and a new one to support new
binding. Maybe an alternate, is to rename former sd modulator as
"legacy" or something similar.
I will address this point in a v3 if necessary.

Olivier Moysan (8):
  iio: add read raw service to iio backend framework
  iio: add enable and disable services to iio backend framework
  iio: add child nodes support in iio backend framework
  dt-bindings: iio: dfsdm: move to backend framework
  dt-bindings: iio: add sigma delta modulator backend
  iio: adc: stm32-dfsdm: adopt generic channels bindings
  iio: add sd modulator generic iio backend
  iio: adc: stm32-dfsdm: add scaling support to dfsdm

 .../iio/adc/sd-modulator-backend.yaml         |  39 +++
 .../bindings/iio/adc/st,stm32-dfsdm-adc.yaml  | 157 ++++++++-
 drivers/iio/adc/Kconfig                       |  11 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/sd_adc_backend.c              | 117 +++++++
 drivers/iio/adc/stm32-dfsdm-adc.c             | 302 +++++++++++++++---
 drivers/iio/industrialio-backend.c            | 108 +++++--
 include/linux/iio/backend.h                   |  10 +-
 8 files changed, 679 insertions(+), 66 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/sd-modulator-backend.yaml
 create mode 100644 drivers/iio/adc/sd_adc_backend.c


base-commit: 2dfa1b7bfc07e58acb9f9eaa8c871f37189dbfee
-- 
2.25.1

[PATCH v2 1/8] iio: add read raw service to iio backend framework

[Olivier Moysan]

Add iio_backend_read_raw() service to support attributes read
from an IIO backend.

Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
Reviewed-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/industrialio-backend.c | 21 +++++++++++++++++++++
 include/linux/iio/backend.h        |  6 +++++-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index 929aff4040ed..0e2653de1956 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -357,6 +357,27 @@ int devm_iio_backend_request_buffer(struct device *dev,
 }
 EXPORT_SYMBOL_NS_GPL(devm_iio_backend_request_buffer, IIO_BACKEND);
 
+/**
+ * iio_backend_read_raw - Request a channel attribute from the IIO backend.
+ * @back:	Backend device
+ * @chan:	IIO channel reference
+ * @val:	First element of the returned value
+ * @val2:	Second element of the returned value
+ * @mask:	Specify value to retrieve
+ *
+ * This callback replicates the read_raw callback of the IIO framework, and is intended to
+ * request miscellaneous channel attributes from the backend device.
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_read_raw(struct iio_backend *back, struct iio_chan_spec const *chan, int *val,
+			 int *val2, long mask)
+{
+	return iio_backend_op_call(back, read_raw, chan, val, val2, mask);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_read_raw, IIO_BACKEND);
+
 static struct iio_backend *iio_backend_from_indio_dev_parent(const struct device *dev)
 {
 	struct iio_backend *back = ERR_PTR(-ENODEV), *iter;
diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
index 8099759d7242..24185718b20d 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -81,6 +81,7 @@ enum iio_backend_sample_trigger {
  * @extend_chan_spec: Extend an IIO channel.
  * @ext_info_set: Extended info setter.
  * @ext_info_get: Extended info getter.
+ * @read_raw: Read value from a backend device
  **/
 struct iio_backend_ops {
 	int (*enable)(struct iio_backend *back);
@@ -113,6 +114,8 @@ struct iio_backend_ops {
 			    const char *buf, size_t len);
 	int (*ext_info_get)(struct iio_backend *back, uintptr_t private,
 			    const struct iio_chan_spec *chan, char *buf);
+	int (*read_raw)(struct iio_backend *back, struct iio_chan_spec const *chan, int *val,
+			int *val2, long mask);
 };
 
 int iio_backend_chan_enable(struct iio_backend *back, unsigned int chan);
@@ -141,7 +144,8 @@ ssize_t iio_backend_ext_info_set(struct iio_dev *indio_dev, uintptr_t private,
 				 const char *buf, size_t len);
 ssize_t iio_backend_ext_info_get(struct iio_dev *indio_dev, uintptr_t private,
 				 const struct iio_chan_spec *chan, char *buf);
-
+int iio_backend_read_raw(struct iio_backend *back, struct iio_chan_spec const *chan, int *val,
+			 int *val2, long mask);
 int iio_backend_extend_chan_spec(struct iio_dev *indio_dev,
 				 struct iio_backend *back,
 				 struct iio_chan_spec *chan);
-- 
2.25.1

[PATCH v2 2/8] iio: add enable and disable services to iio backend framework

[Olivier Moysan]

Add iio_backend_disable() and iio_backend_enable() APIs to allow
IIO backend consumer to request backend disabling and enabling.

Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
Reviewed-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/industrialio-backend.c | 25 ++++++++++++++++++++++++-
 include/linux/iio/backend.h        |  2 ++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index 0e2653de1956..6be1fa9a960b 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -146,6 +146,29 @@ static void __iio_backend_disable(void *back)
 	iio_backend_void_op_call(back, disable);
 }
 
+/**
+ * iio_backend_disable - Backend disable
+ * @back: Backend device
+ */
+void iio_backend_disable(struct iio_backend *back)
+{
+	__iio_backend_disable(back);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_disable, IIO_BACKEND);
+
+/**
+ * iio_backend_enable - Backend enable
+ * @back: Backend device
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_enable(struct iio_backend *back)
+{
+	return iio_backend_op_call(back, enable);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_enable, IIO_BACKEND);
+
 /**
  * devm_iio_backend_enable - Device managed backend enable
  * @dev: Consumer device for the backend
@@ -158,7 +181,7 @@ int devm_iio_backend_enable(struct device *dev, struct iio_backend *back)
 {
 	int ret;
 
-	ret = iio_backend_op_call(back, enable);
+	ret = iio_backend_enable(back);
 	if (ret)
 		return ret;
 
diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
index 24185718b20d..23f21990b85f 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -121,6 +121,8 @@ struct iio_backend_ops {
 int iio_backend_chan_enable(struct iio_backend *back, unsigned int chan);
 int iio_backend_chan_disable(struct iio_backend *back, unsigned int chan);
 int devm_iio_backend_enable(struct device *dev, struct iio_backend *back);
+int iio_backend_enable(struct iio_backend *back);
+void iio_backend_disable(struct iio_backend *back);
 int iio_backend_data_format_set(struct iio_backend *back, unsigned int chan,
 				const struct iio_backend_data_fmt *data);
 int iio_backend_data_source_set(struct iio_backend *back, unsigned int chan,
-- 
2.25.1

[PATCH v2 3/8] iio: add child nodes support in iio backend framework

[Olivier Moysan]

Add an API to support IIO generic channels binding:
http://devicetree.org/schemas/iio/adc/adc.yaml#
This new API is needed, as generic channel DT node isn't populated as a
device.
Add devm_iio_backend_fwnode_get() to allow an IIO device backend
consumer to reference backend phandles in its child nodes.

Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
Reviewed-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/industrialio-backend.c | 62 +++++++++++++++++++++---------
 include/linux/iio/backend.h        |  2 +
 2 files changed, 45 insertions(+), 19 deletions(-)

diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index 6be1fa9a960b..8cc959ac278a 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -577,19 +577,10 @@ static int __devm_iio_backend_get(struct device *dev, struct iio_backend *back)
 	return 0;
 }
 
-/**
- * devm_iio_backend_get - Device managed backend device get
- * @dev: Consumer device for the backend
- * @name: Backend name
- *
- * Get's the backend associated with @dev.
- *
- * RETURNS:
- * A backend pointer, negative error pointer otherwise.
- */
-struct iio_backend *devm_iio_backend_get(struct device *dev, const char *name)
+static struct iio_backend *__devm_iio_backend_fwnode_get(struct device *dev, const char *name,
+							 struct fwnode_handle *fwnode)
 {
-	struct fwnode_handle *fwnode;
+	struct fwnode_handle *fwnode_back;
 	struct iio_backend *back;
 	unsigned int index;
 	int ret;
@@ -604,19 +595,19 @@ struct iio_backend *devm_iio_backend_get(struct device *dev, const char *name)
 		index = 0;
 	}
 
-	fwnode = fwnode_find_reference(dev_fwnode(dev), "io-backends", index);
-	if (IS_ERR(fwnode)) {
-		dev_err_probe(dev, PTR_ERR(fwnode),
+	fwnode_back = fwnode_find_reference(fwnode, "io-backends", index);
+	if (IS_ERR(fwnode_back)) {
+		dev_err_probe(dev, PTR_ERR(fwnode_back),
 			      "Cannot get Firmware reference\n");
-		return ERR_CAST(fwnode);
+		return ERR_CAST(fwnode_back);
 	}
 
 	guard(mutex)(&iio_back_lock);
 	list_for_each_entry(back, &iio_back_list, entry) {
-		if (!device_match_fwnode(back->dev, fwnode))
+		if (!device_match_fwnode(back->dev, fwnode_back))
 			continue;
 
-		fwnode_handle_put(fwnode);
+		fwnode_handle_put(fwnode_back);
 		ret = __devm_iio_backend_get(dev, back);
 		if (ret)
 			return ERR_PTR(ret);
@@ -624,11 +615,44 @@ struct iio_backend *devm_iio_backend_get(struct device *dev, const char *name)
 		return back;
 	}
 
-	fwnode_handle_put(fwnode);
+	fwnode_handle_put(fwnode_back);
 	return ERR_PTR(-EPROBE_DEFER);
 }
+
+/**
+ * devm_iio_backend_get - Device managed backend device get
+ * @dev: Consumer device for the backend
+ * @name: Backend name
+ *
+ * Get's the backend associated with @dev.
+ *
+ * RETURNS:
+ * A backend pointer, negative error pointer otherwise.
+ */
+struct iio_backend *devm_iio_backend_get(struct device *dev, const char *name)
+{
+	return __devm_iio_backend_fwnode_get(dev, name, dev_fwnode(dev));
+}
 EXPORT_SYMBOL_NS_GPL(devm_iio_backend_get, IIO_BACKEND);
 
+/**
+ * devm_iio_backend_fwnode_get - Device managed backend firmware node get
+ * @dev: Consumer device for the backend
+ * @name: Backend name
+ * @fwnode: Firmware node of the backend consumer
+ *
+ * Get's the backend associated with a firmware node.
+ *
+ * RETURNS:
+ * A backend pointer, negative error pointer otherwise.
+ */
+struct iio_backend *devm_iio_backend_fwnode_get(struct device *dev, const char *name,
+						struct fwnode_handle *fwnode)
+{
+	return __devm_iio_backend_fwnode_get(dev, name, fwnode);
+}
+EXPORT_SYMBOL_NS_GPL(devm_iio_backend_fwnode_get, IIO_BACKEND);
+
 /**
  * __devm_iio_backend_get_from_fwnode_lookup - Device managed fwnode backend device get
  * @dev: Consumer device for the backend
diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
index 23f21990b85f..6fa9f1c88f61 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -153,6 +153,8 @@ int iio_backend_extend_chan_spec(struct iio_dev *indio_dev,
 				 struct iio_chan_spec *chan);
 void *iio_backend_get_priv(const struct iio_backend *conv);
 struct iio_backend *devm_iio_backend_get(struct device *dev, const char *name);
+struct iio_backend *devm_iio_backend_fwnode_get(struct device *dev, const char *name,
+						struct fwnode_handle *node);
 struct iio_backend *
 __devm_iio_backend_get_from_fwnode_lookup(struct device *dev,
 					  struct fwnode_handle *fwnode);
-- 
2.25.1

[PATCH v2 4/8] dt-bindings: iio: dfsdm: move to backend framework

[Olivier Moysan]

Change the DFSDM binding to use the new IIO backend framework,
along with the adoption of IIO generic channels.
This binding change allows to add scaling support to the DFSDM.

Keep the legacy binding as deprecated for backward compatibility.

The io-backends property is supported only in generic IIO channel
binding.

- Channel description with the generic binding (Audio and Analog):

  Properties superseded by generic properties:
    st,adc-channels: becomes "reg" property in channel node
    st,adc-channel-names: becomes "label" property in channel node
  Properties moved to channel child node:
    st,adc-channel-types: becomes st,adc-channel-type
    st,adc-channel-clk-src, st,adc-alt-channel

- Analog binding:

  DFSDM filter channel is configured as an IIO backend consumer.
  Add io-backends property in channel child nodes.

  DFSDM is no more configured as a channel consumer from SD modulator.
  Use of io-channels in DFSDM node is deprecated.

- Audio binding:

  DFSDM audio DAI is configured as a channel consumer from DFSDM filter.
  No change compare to legacy.

Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
---
 .../bindings/iio/adc/st,stm32-dfsdm-adc.yaml  | 157 +++++++++++++++++-
 1 file changed, 151 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml b/Documentation/devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml
index c1b1324fa132..1802120b16b0 100644
--- a/Documentation/devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml
@@ -102,9 +102,11 @@ patternProperties:
         items:
           minimum: 0
           maximum: 7
+        deprecated: true
 
       st,adc-channel-names:
         description: List of single-ended channel names.
+        deprecated: true
 
       st,filter-order:
         description: |
@@ -118,6 +120,12 @@ patternProperties:
       "#io-channel-cells":
         const: 1
 
+      '#address-cells':
+        const: 1
+
+      '#size-cells':
+        const: 0
+
       st,adc-channel-types:
         description: |
           Single-ended channel input type.
@@ -128,6 +136,7 @@ patternProperties:
         items:
           enum: [ SPI_R, SPI_F, MANCH_R, MANCH_F ]
         $ref: /schemas/types.yaml#/definitions/non-unique-string-array
+        deprecated: true
 
       st,adc-channel-clk-src:
         description: |
@@ -139,6 +148,7 @@ patternProperties:
         items:
           enum: [ CLKIN, CLKOUT, CLKOUT_F, CLKOUT_R ]
         $ref: /schemas/types.yaml#/definitions/non-unique-string-array
+        deprecated: true
 
       st,adc-alt-channel:
         description:
@@ -147,6 +157,7 @@ patternProperties:
           If not set, channel n is connected to SPI input n.
           If set, channel n is connected to SPI input n + 1.
         type: boolean
+        deprecated: true
 
       st,filter0-sync:
         description:
@@ -165,11 +176,64 @@ patternProperties:
       - compatible
       - reg
       - interrupts
-      - st,adc-channels
-      - st,adc-channel-names
       - st,filter-order
       - "#io-channel-cells"
 
+    patternProperties:
+      "^channel@([0-9]|1[0-9])$":
+        type: object
+        $ref: adc.yaml
+        description: Represents the external channels which are connected to the DFSDM.
+
+        properties:
+          reg:
+            items:
+              minimum: 0
+              maximum: 8
+
+          label:
+            description:
+              Unique name to identify which channel this is.
+
+          st,adc-channel-type:
+            description: |
+              Single-ended channel input type.
+              - "SPI_R": SPI with data on rising edge (default)
+              - "SPI_F": SPI with data on falling edge
+              - "MANCH_R": manchester codec, rising edge = logic 0, falling edge = logic 1
+              - "MANCH_F": manchester codec, rising edge = logic 1, falling edge = logic 0
+            items:
+              enum: [ SPI_R, SPI_F, MANCH_R, MANCH_F ]
+            $ref: /schemas/types.yaml#/definitions/string
+
+          st,adc-channel-clk-src:
+            description: |
+              Conversion clock source.
+              - "CLKIN": external SPI clock (CLKIN x)
+              - "CLKOUT": internal SPI clock (CLKOUT) (default)
+              - "CLKOUT_F": internal SPI clock divided by 2 (falling edge).
+              - "CLKOUT_R": internal SPI clock divided by 2 (rising edge).
+            items:
+              enum: [ CLKIN, CLKOUT, CLKOUT_F, CLKOUT_R ]
+            $ref: /schemas/types.yaml#/definitions/string
+
+          st,adc-alt-channel:
+            description:
+              Must be defined if two sigma delta modulators are
+              connected on same SPI input.
+              If not set, channel n is connected to SPI input n.
+              If set, channel n is connected to SPI input n + 1.
+            type: boolean
+
+          io-backends:
+            description:
+              Used to pipe external sigma delta modulator or internal ADC backend to DFSDM channel.
+
+        required:
+          - reg
+
+        additionalProperties: false
+
     allOf:
       - if:
           properties:
@@ -199,9 +263,19 @@ patternProperties:
               description:
                 From common IIO binding. Used to pipe external sigma delta
                 modulator or internal ADC output to DFSDM channel.
+              deprecated: true
 
-          required:
-            - io-channels
+          if:
+            required:
+              - st,adc-channels
+          then:
+            required:
+              - io-channels
+
+          patternProperties:
+            "^channel@([0-9]|1[0-9])$":
+              required:
+                - io-backends
 
       - if:
           properties:
@@ -294,7 +368,77 @@ examples:
       #address-cells = <1>;
       #size-cells = <0>;
 
+      // Example 1: Audio use case with generic binding
       dfsdm0: filter@0 {
+        compatible = "st,stm32-dfsdm-dmic";
+        reg = <0>;
+        interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+        dmas = <&dmamux1 101 0x400 0x01>;
+        dma-names = "rx";
+        #io-channel-cells = <1>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+        st,filter-order = <5>;
+
+        channel@1 {
+          reg = <1>;
+          label = "dmic0";
+          st,adc-channel-type = "SPI_R";
+          st,adc-channel-clk-src = "CLKOUT";
+          st,adc-alt-channel;
+        };
+
+        asoc_pdm0: dfsdm-dai {
+          compatible = "st,stm32h7-dfsdm-dai";
+          #sound-dai-cells = <0>;
+          io-channels = <&dfsdm0 0>;
+        };
+      };
+
+      // Example 1: Analog use case with generic binding
+      dfsdm1: filter@1 {
+        compatible = "st,stm32-dfsdm-adc";
+        reg = <1>;
+        interrupts = <GIC_SPI 111 IRQ_TYPE_LEVEL_HIGH>;
+        dmas = <&dmamux1 102 0x400 0x01>;
+        dma-names = "rx";
+        st,filter-order = <1>;
+        #io-channel-cells = <1>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        channel@2 {
+          reg = <2>;
+          label = "in2";
+          st,adc-channel-type = "SPI_F";
+          st,adc-channel-clk-src = "CLKOUT";
+          st,adc-alt-channel;
+          io-backends = <&sd_adc2>;
+        };
+
+        channel@3 {
+          reg = <3>;
+          label = "in3";
+          st,adc-channel-type = "SPI_R";
+          st,adc-channel-clk-src = "CLKOUT";
+          io-backends = <&sd_adc3>;
+        };
+      };
+    };
+
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/stm32mp1-clks.h>
+    dfsdm_2: dfsdm@4400d000 {
+      compatible = "st,stm32mp1-dfsdm";
+      reg = <0x4400d000 0x800>;
+      clocks = <&rcc DFSDM_K>, <&rcc ADFSDM_K>;
+      clock-names = "dfsdm", "audio";
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      // Example 3: Audio use case with legacy binding
+      dfsdm0_2: filter@0 {
         compatible = "st,stm32-dfsdm-dmic";
         reg = <0>;
         interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
@@ -307,14 +451,15 @@ examples:
         st,adc-channel-clk-src = "CLKOUT";
         st,filter-order = <5>;
 
-        asoc_pdm0: dfsdm-dai {
+        asoc_pdm0_2: dfsdm-dai {
           compatible = "st,stm32h7-dfsdm-dai";
           #sound-dai-cells = <0>;
           io-channels = <&dfsdm0 0>;
         };
       };
 
-      dfsdm_pdm1: filter@1 {
+      // Example 3: Analog use case with legacy binding
+      dfsdm1_2: filter@1 {
         compatible = "st,stm32-dfsdm-adc";
         reg = <1>;
         interrupts = <GIC_SPI 111 IRQ_TYPE_LEVEL_HIGH>;
-- 
2.25.1

[PATCH v2 5/8] dt-bindings: iio: add sigma delta modulator backend

[Olivier Moysan]

Add documentation of device tree bindings to support
sigma delta modulator backend in IIO framework.

Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
---
 .../iio/adc/sd-modulator-backend.yaml         | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/sd-modulator-backend.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/sd-modulator-backend.yaml b/Documentation/devicetree/bindings/iio/adc/sd-modulator-backend.yaml
new file mode 100644
index 000000000000..3299db71f79d
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/sd-modulator-backend.yaml
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/sd-modulator-backend.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sigma delta modulator backend
+
+maintainers:
+  - Olivier Moysan <olivier.moysan@foss.st.com>
+
+properties:
+  compatible:
+    enum:
+      - ti,ads1201
+
+  '#io-backend-cells':
+    const: 0
+
+  reg:
+    maxItems: 1
+
+  vref-supply:
+    description: Phandle to the vref input analog reference voltage.
+
+required:
+  - compatible
+  - '#io-backend-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    ads1201: adc {
+      compatible = "ti,ads1201";
+      #io-backend-cells = <0>;
+    };
+
+...
-- 
2.25.1

[PATCH v2 6/8] iio: adc: stm32-dfsdm: adopt generic channels bindings

[Olivier Moysan]

Move to generic channels binding to ease new backend framework adoption
and prepare the convergence with MDF IP support on STM32MP2 SoC family.

Legacy binding:
DFSDM is an IIO channel consumer.
SD modulator is an IIO channels provider.
The channel phandles are provided in DT through io-channels property
and channel indexes through st,adc-channels property.

New binding:
DFSDM is an IIO channel provider.
The channel indexes are given by reg property in channel child node.

This new binding is intended to be used with SD modulator IIO backends.
It does not support SD modulator legacy IIO devices.
The st,adc-channels property presence is used to discriminate
between legacy and backend bindings.

The support of the DFSDM legacy channels and SD modulator IIO devices
is kept for backward compatibility.

Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
---
 drivers/iio/adc/stm32-dfsdm-adc.c | 208 ++++++++++++++++++++++++------
 1 file changed, 171 insertions(+), 37 deletions(-)

diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
index fabd654245f5..0df28c9dfa40 100644
--- a/drivers/iio/adc/stm32-dfsdm-adc.c
+++ b/drivers/iio/adc/stm32-dfsdm-adc.c
@@ -666,6 +666,64 @@ static int stm32_dfsdm_channel_parse_of(struct stm32_dfsdm *dfsdm,
 	return 0;
 }
 
+static int stm32_dfsdm_generic_channel_parse_of(struct stm32_dfsdm *dfsdm,
+						struct iio_dev *indio_dev,
+						struct iio_chan_spec *ch,
+						struct fwnode_handle *node)
+{
+	struct stm32_dfsdm_channel *df_ch;
+	const char *of_str;
+	int ret, val;
+
+	ret = fwnode_property_read_u32(node, "reg", &ch->channel);
+	if (ret < 0) {
+		dev_err(&indio_dev->dev, "Missing channel index %d\n", ret);
+		return ret;
+	}
+
+	if (ch->channel >= dfsdm->num_chs) {
+		dev_err(&indio_dev->dev, " Error bad channel number %d (max = %d)\n",
+			ch->channel, dfsdm->num_chs);
+		return -EINVAL;
+	}
+
+	ret = fwnode_property_read_string(node, "label", &ch->datasheet_name);
+	if (ret < 0) {
+		dev_err(&indio_dev->dev,
+			" Error parsing 'label' for idx %d\n", ch->channel);
+		return ret;
+	}
+
+	df_ch =  &dfsdm->ch_list[ch->channel];
+	df_ch->id = ch->channel;
+
+	ret = fwnode_property_read_string(node, "st,adc-channel-type", &of_str);
+	if (!ret) {
+		val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_type);
+		if (val < 0)
+			return val;
+	} else {
+		val = 0;
+	}
+	df_ch->type = val;
+
+	ret = fwnode_property_read_string(node, "st,adc-channel-clk-src", &of_str);
+	if (!ret) {
+		val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_src);
+		if (val < 0)
+			return val;
+	} else {
+		val = 0;
+	}
+	df_ch->src = val;
+
+	ret = fwnode_property_read_u32(node, "st,adc-alt-channel", &df_ch->alt_si);
+	if (ret != -EINVAL)
+		df_ch->alt_si = 0;
+
+	return 0;
+}
+
 static ssize_t dfsdm_adc_audio_get_spiclk(struct iio_dev *indio_dev,
 					  uintptr_t priv,
 					  const struct iio_chan_spec *chan,
@@ -1227,7 +1285,8 @@ static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
 		ret = iio_device_claim_direct_mode(indio_dev);
 		if (ret)
 			return ret;
-		ret = iio_hw_consumer_enable(adc->hwc);
+		if (adc->hwc)
+			ret = iio_hw_consumer_enable(adc->hwc);
 		if (ret < 0) {
 			dev_err(&indio_dev->dev,
 				"%s: IIO enable failed (channel %d)\n",
@@ -1236,7 +1295,8 @@ static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
 			return ret;
 		}
 		ret = stm32_dfsdm_single_conv(indio_dev, chan, val);
-		iio_hw_consumer_disable(adc->hwc);
+		if (adc->hwc)
+			iio_hw_consumer_disable(adc->hwc);
 		if (ret < 0) {
 			dev_err(&indio_dev->dev,
 				"%s: Conversion failed (channel %d)\n",
@@ -1362,15 +1422,20 @@ static int stm32_dfsdm_dma_request(struct device *dev,
 	return 0;
 }
 
-static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev,
-					 struct iio_chan_spec *ch)
+static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev, struct iio_chan_spec *ch,
+					 struct fwnode_handle *child)
 {
 	struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
 	int ret;
 
-	ret = stm32_dfsdm_channel_parse_of(adc->dfsdm, indio_dev, ch);
-	if (ret < 0)
+	if (child)
+		ret = stm32_dfsdm_generic_channel_parse_of(adc->dfsdm, indio_dev, ch, child);
+	else /* Legacy binding */
+		ret = stm32_dfsdm_channel_parse_of(adc->dfsdm, indio_dev, ch);
+	if (ret < 0) {
+		dev_err(&indio_dev->dev, "Failed to parse channel\n");
 		return ret;
+	}
 
 	ch->type = IIO_VOLTAGE;
 	ch->indexed = 1;
@@ -1385,6 +1450,7 @@ static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev,
 
 	if (adc->dev_data->type == DFSDM_AUDIO) {
 		ch->ext_info = dfsdm_adc_audio_ext_info;
+		ch->scan_index = 0;
 	} else {
 		ch->scan_type.shift = 8;
 	}
@@ -1392,8 +1458,51 @@ static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev,
 	ch->scan_type.realbits = 24;
 	ch->scan_type.storagebits = 32;
 
-	return stm32_dfsdm_chan_configure(adc->dfsdm,
-					  &adc->dfsdm->ch_list[ch->channel]);
+	return stm32_dfsdm_chan_configure(adc->dfsdm, &adc->dfsdm->ch_list[ch->channel]);
+}
+
+static int stm32_dfsdm_chan_init(struct iio_dev *indio_dev, struct iio_chan_spec *channels)
+{
+	int num_ch = indio_dev->num_channels;
+	int chan_idx = 0, ret = 0;
+
+	for (chan_idx = 0; chan_idx < num_ch; chan_idx++) {
+		channels[chan_idx].scan_index = chan_idx;
+		ret = stm32_dfsdm_adc_chan_init_one(indio_dev, &channels[chan_idx], NULL);
+		if (ret < 0) {
+			dev_err(&indio_dev->dev, "Channels init failed\n");
+			return ret;
+		}
+	}
+
+	return ret;
+}
+
+static int stm32_dfsdm_generic_chan_init(struct iio_dev *indio_dev, struct iio_chan_spec *channels)
+{
+	struct fwnode_handle *child;
+	int chan_idx = 0, ret;
+
+	device_for_each_child_node(&indio_dev->dev, child) {
+		/* Skip DAI node in DFSDM audio nodes */
+		if (fwnode_property_present(child, "compatible"))
+			continue;
+
+		channels[chan_idx].scan_index = chan_idx;
+		ret = stm32_dfsdm_adc_chan_init_one(indio_dev, &channels[chan_idx], child);
+		if (ret < 0) {
+			dev_err(&indio_dev->dev, "Channels init failed\n");
+			goto err;
+		}
+
+		chan_idx++;
+	}
+	return chan_idx;
+
+err:
+	fwnode_handle_put(child);
+
+	return ret;
 }
 
 static int stm32_dfsdm_audio_init(struct device *dev, struct iio_dev *indio_dev)
@@ -1401,15 +1510,26 @@ static int stm32_dfsdm_audio_init(struct device *dev, struct iio_dev *indio_dev)
 	struct iio_chan_spec *ch;
 	struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
 	struct stm32_dfsdm_channel *d_ch;
-	int ret;
+	bool legacy = false;
+	int num_ch, ret;
+
+	/* If st,adc-channels is defined legacy binding is used. Else assume generic binding. */
+	num_ch = of_property_count_u32_elems(indio_dev->dev.of_node, "st,adc-channels");
+	if (num_ch == 1)
+		legacy = true;
 
 	ch = devm_kzalloc(&indio_dev->dev, sizeof(*ch), GFP_KERNEL);
 	if (!ch)
 		return -ENOMEM;
 
-	ch->scan_index = 0;
+	indio_dev->num_channels = 1;
+	indio_dev->channels = ch;
+
+	if (legacy)
+		ret = stm32_dfsdm_chan_init(indio_dev, ch);
+	else
+		ret = stm32_dfsdm_generic_chan_init(indio_dev, ch);
 
-	ret = stm32_dfsdm_adc_chan_init_one(indio_dev, ch);
 	if (ret < 0) {
 		dev_err(&indio_dev->dev, "Channels init failed\n");
 		return ret;
@@ -1420,9 +1540,6 @@ static int stm32_dfsdm_audio_init(struct device *dev, struct iio_dev *indio_dev)
 	if (d_ch->src != DFSDM_CHANNEL_SPI_CLOCK_EXTERNAL)
 		adc->spi_freq = adc->dfsdm->spi_master_freq;
 
-	indio_dev->num_channels = 1;
-	indio_dev->channels = ch;
-
 	return stm32_dfsdm_dma_request(dev, indio_dev);
 }
 
@@ -1430,43 +1547,60 @@ static int stm32_dfsdm_adc_init(struct device *dev, struct iio_dev *indio_dev)
 {
 	struct iio_chan_spec *ch;
 	struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
-	int num_ch;
-	int ret, chan_idx;
+	int num_ch, ret;
+	bool legacy = false;
 
 	adc->oversamp = DFSDM_DEFAULT_OVERSAMPLING;
 	ret = stm32_dfsdm_compute_all_osrs(indio_dev, adc->oversamp);
 	if (ret < 0)
 		return ret;
 
-	num_ch = of_property_count_u32_elems(indio_dev->dev.of_node,
-					     "st,adc-channels");
-	if (num_ch < 0 || num_ch > adc->dfsdm->num_chs) {
-		dev_err(&indio_dev->dev, "Bad st,adc-channels\n");
-		return num_ch < 0 ? num_ch : -EINVAL;
+	num_ch = device_get_child_node_count(&indio_dev->dev);
+	if (!num_ch) {
+		/* No channels nodes found. Assume legacy binding */
+		num_ch = of_property_count_u32_elems(indio_dev->dev.of_node, "st,adc-channels");
+		if (num_ch < 0) {
+			dev_err(&indio_dev->dev, "Bad st,adc-channels\n");
+			return num_ch;
+		}
+
+		legacy = true;
 	}
 
-	/* Bind to SD modulator IIO device */
-	adc->hwc = devm_iio_hw_consumer_alloc(&indio_dev->dev);
-	if (IS_ERR(adc->hwc))
-		return -EPROBE_DEFER;
+	if (num_ch > adc->dfsdm->num_chs) {
+		dev_err(&indio_dev->dev, "Number of channel [%d] exceeds [%d]\n",
+			num_ch, adc->dfsdm->num_chs);
+		return -EINVAL;
+	}
+	indio_dev->num_channels = num_ch;
 
-	ch = devm_kcalloc(&indio_dev->dev, num_ch, sizeof(*ch),
-			  GFP_KERNEL);
-	if (!ch)
-		return -ENOMEM;
+	if (legacy) {
+		/* Bind to SD modulator IIO device. */
+		adc->hwc = devm_iio_hw_consumer_alloc(&indio_dev->dev);
+		if (IS_ERR(adc->hwc))
+			return -EPROBE_DEFER;
+	} else {
+		/* Generic binding. SD modulator IIO device not used. Use SD modulator backend. */
+		adc->hwc = NULL;
 
-	for (chan_idx = 0; chan_idx < num_ch; chan_idx++) {
-		ch[chan_idx].scan_index = chan_idx;
-		ret = stm32_dfsdm_adc_chan_init_one(indio_dev, &ch[chan_idx]);
-		if (ret < 0) {
-			dev_err(&indio_dev->dev, "Channels init failed\n");
-			return ret;
-		}
+		adc->backend = devm_kzalloc(&indio_dev->dev, sizeof(*adc->backend) * num_ch,
+					    GFP_KERNEL);
+		if (!adc->backend)
+			return -ENOMEM;
 	}
 
-	indio_dev->num_channels = num_ch;
+	ch = devm_kcalloc(&indio_dev->dev, num_ch, sizeof(*ch), GFP_KERNEL);
+	if (!ch)
+		return -ENOMEM;
 	indio_dev->channels = ch;
 
+	if (legacy)
+		ret = stm32_dfsdm_chan_init(indio_dev, ch);
+	else
+		ret = stm32_dfsdm_generic_chan_init(indio_dev, ch);
+	if (ret < 0)
+		return ret;
+
 	init_completion(&adc->completion);
 
 	/* Optionally request DMA */
-- 
2.25.1

[PATCH v2 7/8] iio: add sd modulator generic iio backend

[Olivier Moysan]

Add a generic driver to support sigma delta modulators.
Typically, this device is a hardware connected to an IIO device
in charge of the conversion. The device is exposed as an IIO backend
device. This backend device and the associated conversion device
can be seen as an aggregate device from IIO framework.

Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
---
 drivers/iio/adc/Kconfig          |  10 +++
 drivers/iio/adc/Makefile         |   1 +
 drivers/iio/adc/sd_adc_backend.c | 117 +++++++++++++++++++++++++++++++
 3 files changed, 128 insertions(+)
 create mode 100644 drivers/iio/adc/sd_adc_backend.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index b8184706c7d1..634dc9842fb7 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1155,6 +1155,16 @@ config SPEAR_ADC
 	  To compile this driver as a module, choose M here: the
 	  module will be called spear_adc.
 
+config SD_ADC_BACKEND
+	tristate "Generic sigma delta modulator IIO backend"
+	select IIO_BACKEND
+	help
+	  Select this option to enables sigma delta modulator. This driver can
+	  support generic sigma delta modulators, as IIO backend devices.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called sd_adc_backend.
+
 config SD_ADC_MODULATOR
 	tristate "Generic sigma delta modulator"
 	select IIO_BUFFER
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 51298c52b223..e9c4a4549fc0 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -140,3 +140,4 @@ obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
 obj-$(CONFIG_XILINX_AMS) += xilinx-ams.o
 xilinx-xadc-y := xilinx-xadc-core.o xilinx-xadc-events.o
 obj-$(CONFIG_XILINX_XADC) += xilinx-xadc.o
+obj-$(CONFIG_SD_ADC_BACKEND) += sd_adc_backend.o
diff --git a/drivers/iio/adc/sd_adc_backend.c b/drivers/iio/adc/sd_adc_backend.c
new file mode 100644
index 000000000000..b9f679c4792f
--- /dev/null
+++ b/drivers/iio/adc/sd_adc_backend.c
@@ -0,0 +1,117 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Generic sigma delta modulator IIO backend
+ *
+ * Copyright (C) 2024, STMicroelectronics - All Rights Reserved
+ */
+
+#include <linux/iio/backend.h>
+#include <linux/iio/iio.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+
+struct iio_sd_backend_priv {
+	struct regulator *vref;
+	int vref_mv;
+};
+
+static int sd_backend_enable(struct iio_backend *backend)
+{
+	struct iio_sd_backend_priv *priv = iio_backend_get_priv(backend);
+
+	if (priv->vref)
+		return regulator_enable(priv->vref);
+
+	return 0;
+};
+
+static void sd_backend_disable(struct iio_backend *backend)
+{
+	struct iio_sd_backend_priv *priv = iio_backend_get_priv(backend);
+
+	if (priv->vref)
+		regulator_disable(priv->vref);
+};
+
+static int sd_backend_read(struct iio_backend *backend, struct iio_chan_spec const *chan, int *val,
+			   int *val2, long mask)
+{
+	struct iio_sd_backend_priv *priv = iio_backend_get_priv(backend);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		*val = priv->vref_mv;
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_OFFSET:
+		*val = 0;
+		return IIO_VAL_INT;
+	}
+
+	return -EOPNOTSUPP;
+};
+
+static const struct iio_backend_ops sd_backend_ops = {
+	.enable = sd_backend_enable,
+	.disable = sd_backend_disable,
+	.read_raw = sd_backend_read,
+};
+
+static int iio_sd_backend_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct regulator *vref;
+	struct iio_sd_backend_priv *priv;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	/*
+	 * Get regulator reference if any, but don't enable regulator right now.
+	 * Rely on enable and disable callbacks to manage regulator power.
+	 */
+	vref = devm_regulator_get_optional(dev, "vref");
+	if (IS_ERR(vref)) {
+		if (PTR_ERR(vref) != -ENODEV)
+			return dev_err_probe(dev, PTR_ERR(vref), "Failed to get vref\n");
+	} else {
+		/*
+		 * Retrieve voltage right now, as regulator_get_voltage() provides it whatever
+		 * the state of the regulator.
+		 */
+		ret = regulator_get_voltage(vref);
+		if (ret < 0)
+			return ret;
+
+		priv->vref = vref;
+		priv->vref_mv = ret / 1000;
+	}
+
+	return devm_iio_backend_register(&pdev->dev, &sd_backend_ops, priv);
+};
+
+static const struct of_device_id sd_backend_of_match[] = {
+	{ .compatible = "sd-backend" },
+	{ .compatible = "ti,ads1201" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sd_backend_of_match);
+
+static struct platform_driver iio_sd_backend_adc = {
+	.driver = {
+		.name = "iio_sd_adc_backend",
+		.of_match_table = sd_backend_of_match,
+	},
+	.probe = iio_sd_backend_probe,
+};
+
+module_platform_driver(iio_sd_backend_adc);
+
+MODULE_DESCRIPTION("Basic sigma delta modulator IIO backend");
+MODULE_AUTHOR("Olivier Moysan <olivier.moysan@foss.st.com>");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(IIO_BACKEND);
-- 
2.25.1

[PATCH v2 8/8] iio: adc: stm32-dfsdm: add scaling support to dfsdm

[Olivier Moysan]

Add scaling support to STM32 DFSDM.

When used in an analog context, a DFSDM filter typically converts the data
from a sigma delta modulator. The IIO device associated to the DFSDM
filter provides these data as raw data.
The IIO device can provide scaling information (voltage and offset) to
allow conversion of raw data into physical values.

With the new binding based on IIO backend framework, the sigma delta
modulators are defined as backends providing scaling information.

The scaling is not supported with legacy binding.

Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
Acked-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/adc/Kconfig           |  1 +
 drivers/iio/adc/stm32-dfsdm-adc.c | 94 ++++++++++++++++++++++++++++++-
 2 files changed, 93 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 634dc9842fb7..352ad585c534 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1223,6 +1223,7 @@ config STM32_DFSDM_ADC
 	select IIO_BUFFER
 	select IIO_BUFFER_HW_CONSUMER
 	select IIO_TRIGGERED_BUFFER
+	select IIO_BACKEND
 	help
 	  Select this option to support ADCSigma delta modulator for
 	  STMicroelectronics STM32 digital filter for sigma delta converter.
diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
index 0df28c9dfa40..6a84ef3f32fd 100644
--- a/drivers/iio/adc/stm32-dfsdm-adc.c
+++ b/drivers/iio/adc/stm32-dfsdm-adc.c
@@ -9,6 +9,7 @@
 #include <linux/dmaengine.h>
 #include <linux/dma-mapping.h>
 #include <linux/iio/adc/stm32-dfsdm-adc.h>
+#include <linux/iio/backend.h>
 #include <linux/iio/buffer.h>
 #include <linux/iio/hw-consumer.h>
 #include <linux/iio/sysfs.h>
@@ -78,6 +79,7 @@ struct stm32_dfsdm_adc {
 	/* ADC specific */
 	unsigned int oversamp;
 	struct iio_hw_consumer *hwc;
+	struct iio_backend **backend;
 	struct completion completion;
 	u32 *buffer;
 
@@ -672,6 +674,8 @@ static int stm32_dfsdm_generic_channel_parse_of(struct stm32_dfsdm *dfsdm,
 						struct fwnode_handle *node)
 {
 	struct stm32_dfsdm_channel *df_ch;
+	struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
+	struct iio_backend *backend;
 	const char *of_str;
 	int ret, val;
 
@@ -721,6 +725,14 @@ static int stm32_dfsdm_generic_channel_parse_of(struct stm32_dfsdm *dfsdm,
 	if (ret != -EINVAL)
 		df_ch->alt_si = 0;
 
+	if (adc->dev_data->type == DFSDM_IIO) {
+		backend = devm_iio_backend_fwnode_get(&indio_dev->dev, NULL, node);
+		if (IS_ERR(backend))
+			return dev_err_probe(&indio_dev->dev, PTR_ERR(backend),
+					     "Failed to get backend\n");
+		adc->backend[ch->scan_index] = backend;
+	}
+
 	return 0;
 }
 
@@ -1056,6 +1068,7 @@ static int stm32_dfsdm_update_scan_mode(struct iio_dev *indio_dev,
 static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
 {
 	struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
+	int i = 0;
 	int ret;
 
 	/* Reset adc buffer index */
@@ -1067,6 +1080,15 @@ static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
 			return ret;
 	}
 
+	if (adc->backend) {
+		while (adc->backend[i]) {
+			ret = iio_backend_enable(adc->backend[i]);
+			if (ret < 0)
+				return ret;
+			i++;
+		}
+	}
+
 	ret = stm32_dfsdm_start_dfsdm(adc->dfsdm);
 	if (ret < 0)
 		goto err_stop_hwc;
@@ -1099,6 +1121,7 @@ static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
 static int stm32_dfsdm_predisable(struct iio_dev *indio_dev)
 {
 	struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
+	int i = 0;
 
 	stm32_dfsdm_stop_conv(indio_dev);
 
@@ -1106,6 +1129,13 @@ static int stm32_dfsdm_predisable(struct iio_dev *indio_dev)
 
 	stm32_dfsdm_stop_dfsdm(adc->dfsdm);
 
+	if (adc->backend) {
+		while (adc->backend[i]) {
+			iio_backend_disable(adc->backend[i]);
+			i++;
+		}
+	}
+
 	if (adc->hwc)
 		iio_hw_consumer_disable(adc->hwc);
 
@@ -1278,7 +1308,14 @@ static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
 				int *val2, long mask)
 {
 	struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
-	int ret;
+
+	struct stm32_dfsdm_filter *fl = &adc->dfsdm->fl_list[adc->fl_id];
+	struct stm32_dfsdm_filter_osr *flo = &fl->flo[fl->fast];
+	u32 max = flo->max << (flo->lshift - chan->scan_type.shift);
+	int ret, idx = chan->scan_index;
+
+	if (flo->lshift < chan->scan_type.shift)
+		max = flo->max >> (chan->scan_type.shift - flo->lshift);
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
@@ -1287,6 +1324,8 @@ static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
 			return ret;
 		if (adc->hwc)
 			ret = iio_hw_consumer_enable(adc->hwc);
+		if (adc->backend[idx])
+			ret = iio_backend_enable(adc->backend[idx]);
 		if (ret < 0) {
 			dev_err(&indio_dev->dev,
 				"%s: IIO enable failed (channel %d)\n",
@@ -1297,6 +1336,8 @@ static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
 		ret = stm32_dfsdm_single_conv(indio_dev, chan, val);
 		if (adc->hwc)
 			iio_hw_consumer_disable(adc->hwc);
+		if (adc->backend[idx])
+			iio_backend_disable(adc->backend[idx]);
 		if (ret < 0) {
 			dev_err(&indio_dev->dev,
 				"%s: Conversion failed (channel %d)\n",
@@ -1316,6 +1357,46 @@ static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
 		*val = adc->sample_freq;
 
 		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		/*
+		 * Scale is expressed in mV.
+		 * When fast mode is disabled, actual resolution may be lower
+		 * than 2^n, where n = realbits - 1.
+		 * This leads to underestimating the input voltage.
+		 * To compensate this deviation, the voltage reference can be
+		 * corrected with a factor = realbits resolution / actual max
+		 */
+		if (adc->backend[idx]) {
+			iio_backend_read_raw(adc->backend[idx], chan, val, val2, mask);
+
+			*val = div_u64((u64)*val * (u64)BIT(DFSDM_DATA_RES - 1), max);
+			*val2 = chan->scan_type.realbits;
+			if (chan->differential)
+				*val *= 2;
+		}
+		return IIO_VAL_FRACTIONAL_LOG2;
+
+	case IIO_CHAN_INFO_OFFSET:
+		/*
+		 * DFSDM output data are in the range [-2^n, 2^n],
+		 * with n = realbits - 1.
+		 * - Differential modulator:
+		 * Offset correspond to SD modulator offset.
+		 * - Single ended modulator:
+		 * Input is in [0V, Vref] range,
+		 * where 0V corresponds to -2^n, and Vref to 2^n.
+		 * Add 2^n to offset. (i.e. middle of input range)
+		 * offset = offset(sd) * vref / res(sd) * max / vref.
+		 */
+		if (adc->backend[idx]) {
+			iio_backend_read_raw(adc->backend[idx], chan, val, val2, mask);
+
+			*val = div_u64((u64)max * *val, BIT(*val2 - 1));
+			if (!chan->differential)
+				*val += max;
+		}
+		return IIO_VAL_INT;
 	}
 
 	return -EINVAL;
@@ -1444,7 +1525,15 @@ static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev, struct iio_c
 	 * IIO_CHAN_INFO_RAW: used to compute regular conversion
 	 * IIO_CHAN_INFO_OVERSAMPLING_RATIO: used to set oversampling
 	 */
-	ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
+	if (child) {
+		ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+					 BIT(IIO_CHAN_INFO_SCALE) |
+					 BIT(IIO_CHAN_INFO_OFFSET);
+	} else {
+		/* Legacy. Scaling not supported */
+		ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
+	}
+
 	ch->info_mask_shared_by_all = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
 					BIT(IIO_CHAN_INFO_SAMP_FREQ);
 
@@ -1811,3 +1900,4 @@ module_platform_driver(stm32_dfsdm_adc_driver);
 MODULE_DESCRIPTION("STM32 sigma delta ADC");
 MODULE_AUTHOR("Arnaud Pouliquen <arnaud.pouliquen@st.com>");
 MODULE_LICENSE("GPL v2");
+MODULE_IMPORT_NS(IIO_BACKEND);
-- 
2.25.1

Re: [PATCH v2 5/8] dt-bindings: iio: add sigma delta modulator backend

[Conor Dooley]

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

On Tue, Jun 25, 2024 at 05:07:13PM +0200, Olivier Moysan wrote:
> Add documentation of device tree bindings to support
> sigma delta modulator backend in IIO framework.
> 
> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>

I don't review bindings for a job, I can only reliably get to look at
my mail queue in the evenings, please give me a chance to reply to you
before you submit a new version.

> +$id: http://devicetree.org/schemas/iio/adc/sd-modulator-backend.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sigma delta modulator backend

Same comments about filename and title apply here as the previous
version. "TI $foo Sigma Delta Modulator" and drop the reference to back
ends or the pretence of being generic.

Thanks,
Conor.

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

Re: [PATCH v2 1/8] iio: add read raw service to iio backend framework

[Nuno Sá]

On Tue, 2024-06-25 at 17:07 +0200, Olivier Moysan wrote:
> Add iio_backend_read_raw() service to support attributes read
> from an IIO backend.
> 
> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
> ---
>  drivers/iio/industrialio-backend.c | 21 +++++++++++++++++++++
>  include/linux/iio/backend.h        |  6 +++++-
>  2 files changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-
> backend.c
> index 929aff4040ed..0e2653de1956 100644
> --- a/drivers/iio/industrialio-backend.c
> +++ b/drivers/iio/industrialio-backend.c
> @@ -357,6 +357,27 @@ int devm_iio_backend_request_buffer(struct device *dev,
>  }
>  EXPORT_SYMBOL_NS_GPL(devm_iio_backend_request_buffer, IIO_BACKEND);
>  
> +/**
> + * iio_backend_read_raw - Request a channel attribute from the IIO backend.
> + * @back:	Backend device
> + * @chan:	IIO channel reference
> + * @val:	First element of the returned value
> + * @val2:	Second element of the returned value
> + * @mask:	Specify value to retrieve
> + *
> + * This callback replicates the read_raw callback of the IIO framework, and
> is intended to
> + * request miscellaneous channel attributes from the backend device.
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_read_raw(struct iio_backend *back, struct iio_chan_spec const
> *chan, int *val,
> +			 int *val2, long mask)
> +{
> +	return iio_backend_op_call(back, read_raw, chan, val, val2, mask);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_read_raw, IIO_BACKEND);

I actually got an idea when looking at this for my existential crisis between
dedicated APIs and a catch all .read_raw() :). What we can do is just provide
the .read_raw() or write_raw() ops to backends (so we minimize the number of
ops) and then we build on top of them for providing more readable (depending on
the case; some cases it does make sense to just call iio_backend_read_raw())
APIs to frontends.

So in your case you could have in backend.h

static inline int iio_backend_read_scale(...)
{
	return iio_backend_read_raw(..., IIO_CHAN_INFO_SCALE);
}

Naturally no need for you to do this right now in your series. Just wanted to
write it down before I go into other stuff and forget about this :)

- Nuno Sá

Re: [PATCH v2 3/8] iio: add child nodes support in iio backend framework

[Nuno Sá]

On Tue, 2024-06-25 at 17:07 +0200, Olivier Moysan wrote:
> Add an API to support IIO generic channels binding:
> http://devicetree.org/schemas/iio/adc/adc.yaml#
> This new API is needed, as generic channel DT node isn't populated as a
> device.
> Add devm_iio_backend_fwnode_get() to allow an IIO device backend
> consumer to reference backend phandles in its child nodes.
> 
> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
> ---
>  drivers/iio/industrialio-backend.c | 62 +++++++++++++++++++++---------
>  include/linux/iio/backend.h        |  2 +
>  2 files changed, 45 insertions(+), 19 deletions(-)

...

> --- a/include/linux/iio/backend.h
> +++ b/include/linux/iio/backend.h
> @@ -153,6 +153,8 @@ int iio_backend_extend_chan_spec(struct iio_dev
> *indio_dev,
>  				 struct iio_chan_spec *chan);
>  void *iio_backend_get_priv(const struct iio_backend *conv);
>  struct iio_backend *devm_iio_backend_get(struct device *dev, const char
> *name);
> +struct iio_backend *devm_iio_backend_fwnode_get(struct device *dev, const
> char *name,
> +						struct fwnode_handle *node);

node -> fwnode

- Nuno Sá

Re: [PATCH v2 7/8] iio: add sd modulator generic iio backend

[Nuno Sá]

On Tue, 2024-06-25 at 17:07 +0200, Olivier Moysan wrote:
> Add a generic driver to support sigma delta modulators.
> Typically, this device is a hardware connected to an IIO device
> in charge of the conversion. The device is exposed as an IIO backend
> device. This backend device and the associated conversion device
> can be seen as an aggregate device from IIO framework.
> 
> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
> ---

Acked-by: Nuno Sa <nuno.sa@analog.com>

Re: [PATCH v2 8/8] iio: adc: stm32-dfsdm: add scaling support to dfsdm

[Nuno Sá]

Hi Olivier,

One thing that I just noticed...

On Tue, 2024-06-25 at 17:07 +0200, Olivier Moysan wrote:
> Add scaling support to STM32 DFSDM.
> 
> When used in an analog context, a DFSDM filter typically converts the data
> from a sigma delta modulator. The IIO device associated to the DFSDM
> filter provides these data as raw data.
> The IIO device can provide scaling information (voltage and offset) to
> allow conversion of raw data into physical values.
> 
> With the new binding based on IIO backend framework, the sigma delta
> modulators are defined as backends providing scaling information.
> 
> The scaling is not supported with legacy binding.
> 
> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
> Acked-by: Nuno Sa <nuno.sa@analog.com>
> ---
> 

...

> +
> +	case IIO_CHAN_INFO_SCALE:
> +		/*
> +		 * Scale is expressed in mV.
> +		 * When fast mode is disabled, actual resolution may be lower
> +		 * than 2^n, where n = realbits - 1.
> +		 * This leads to underestimating the input voltage.
> +		 * To compensate this deviation, the voltage reference can be
> +		 * corrected with a factor = realbits resolution / actual max
> +		 */
> +		if (adc->backend[idx]) {
> +			iio_backend_read_raw(adc->backend[idx], chan, val,
> val2, mask);

Eve if it does not matter for your usecase, you should still do error handling
as iio_backend_read_raw() can return an error.

> +			*val = div_u64((u64)*val * (u64)BIT(DFSDM_DATA_RES -
> 1), max);
> +			*val2 = chan->scan_type.realbits;
> +			if (chan->differential)
> +				*val *= 2;
> +		}
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +
> +	case IIO_CHAN_INFO_OFFSET:
> +		/*
> +		 * DFSDM output data are in the range [-2^n, 2^n],
> +		 * with n = realbits - 1.
> +		 * - Differential modulator:
> +		 * Offset correspond to SD modulator offset.
> +		 * - Single ended modulator:
> +		 * Input is in [0V, Vref] range,
> +		 * where 0V corresponds to -2^n, and Vref to 2^n.
> +		 * Add 2^n to offset. (i.e. middle of input range)
> +		 * offset = offset(sd) * vref / res(sd) * max / vref.
> +		 */
> +		if (adc->backend[idx]) {
> +			iio_backend_read_raw(adc->backend[idx], chan, val,
> val2, mask);

Same...

- Nuno Sá

Re: [PATCH v2 1/8] iio: add read raw service to iio backend framework

[Olivier MOYSAN]

Hi Nuno,

On 6/26/24 10:50, Nuno Sá wrote:
> On Tue, 2024-06-25 at 17:07 +0200, Olivier Moysan wrote:
>> Add iio_backend_read_raw() service to support attributes read
>> from an IIO backend.
>>
>> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
>> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
>> ---
>>   drivers/iio/industrialio-backend.c | 21 +++++++++++++++++++++
>>   include/linux/iio/backend.h        |  6 +++++-
>>   2 files changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-
>> backend.c
>> index 929aff4040ed..0e2653de1956 100644
>> --- a/drivers/iio/industrialio-backend.c
>> +++ b/drivers/iio/industrialio-backend.c
>> @@ -357,6 +357,27 @@ int devm_iio_backend_request_buffer(struct device *dev,
>>   }
>>   EXPORT_SYMBOL_NS_GPL(devm_iio_backend_request_buffer, IIO_BACKEND);
>>   
>> +/**
>> + * iio_backend_read_raw - Request a channel attribute from the IIO backend.
>> + * @back:	Backend device
>> + * @chan:	IIO channel reference
>> + * @val:	First element of the returned value
>> + * @val2:	Second element of the returned value
>> + * @mask:	Specify value to retrieve
>> + *
>> + * This callback replicates the read_raw callback of the IIO framework, and
>> is intended to
>> + * request miscellaneous channel attributes from the backend device.
>> + *
>> + * RETURNS:
>> + * 0 on success, negative error number on failure.
>> + */
>> +int iio_backend_read_raw(struct iio_backend *back, struct iio_chan_spec const
>> *chan, int *val,
>> +			 int *val2, long mask)
>> +{
>> +	return iio_backend_op_call(back, read_raw, chan, val, val2, mask);
>> +}
>> +EXPORT_SYMBOL_NS_GPL(iio_backend_read_raw, IIO_BACKEND);
> 
> I actually got an idea when looking at this for my existential crisis between
> dedicated APIs and a catch all .read_raw() :). What we can do is just provide
> the .read_raw() or write_raw() ops to backends (so we minimize the number of
> ops) and then we build on top of them for providing more readable (depending on
> the case; some cases it does make sense to just call iio_backend_read_raw())
> APIs to frontends.
> 
> So in your case you could have in backend.h
> 
> static inline int iio_backend_read_scale(...)
> {
> 	return iio_backend_read_raw(..., IIO_CHAN_INFO_SCALE);
> }
> 
> Naturally no need for you to do this right now in your series. Just wanted to
> write it down before I go into other stuff and forget about this :)
> 

Yes, this is a good compromise. Such helpers are more user-friendly for 
the consumer. As long as I have to push a v3, I might take the 
opportunity to add this.

BRs
Olivier

> - Nuno Sá
> 
> 

Re: [PATCH v2 8/8] iio: adc: stm32-dfsdm: add scaling support to dfsdm

[Olivier MOYSAN]

Hi Nuno,

On 6/26/24 10:59, Nuno Sá wrote:
> Hi Olivier,
> 
> One thing that I just noticed...
> 
> On Tue, 2024-06-25 at 17:07 +0200, Olivier Moysan wrote:
>> Add scaling support to STM32 DFSDM.
>>
>> When used in an analog context, a DFSDM filter typically converts the data
>> from a sigma delta modulator. The IIO device associated to the DFSDM
>> filter provides these data as raw data.
>> The IIO device can provide scaling information (voltage and offset) to
>> allow conversion of raw data into physical values.
>>
>> With the new binding based on IIO backend framework, the sigma delta
>> modulators are defined as backends providing scaling information.
>>
>> The scaling is not supported with legacy binding.
>>
>> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
>> Acked-by: Nuno Sa <nuno.sa@analog.com>
>> ---
>>
> 
> ...
> 
>> +
>> +	case IIO_CHAN_INFO_SCALE:
>> +		/*
>> +		 * Scale is expressed in mV.
>> +		 * When fast mode is disabled, actual resolution may be lower
>> +		 * than 2^n, where n = realbits - 1.
>> +		 * This leads to underestimating the input voltage.
>> +		 * To compensate this deviation, the voltage reference can be
>> +		 * corrected with a factor = realbits resolution / actual max
>> +		 */
>> +		if (adc->backend[idx]) {
>> +			iio_backend_read_raw(adc->backend[idx], chan, val,
>> val2, mask);
> 
> Eve if it does not matter for your usecase, you should still do error handling
> as iio_backend_read_raw() can return an error.
> 

Ack. On the same occasion, I will switch to a dedicated API.

Olivier

>> +			*val = div_u64((u64)*val * (u64)BIT(DFSDM_DATA_RES -
>> 1), max);
>> +			*val2 = chan->scan_type.realbits;
>> +			if (chan->differential)
>> +				*val *= 2;
>> +		}
>> +		return IIO_VAL_FRACTIONAL_LOG2;
>> +
>> +	case IIO_CHAN_INFO_OFFSET:
>> +		/*
>> +		 * DFSDM output data are in the range [-2^n, 2^n],
>> +		 * with n = realbits - 1.
>> +		 * - Differential modulator:
>> +		 * Offset correspond to SD modulator offset.
>> +		 * - Single ended modulator:
>> +		 * Input is in [0V, Vref] range,
>> +		 * where 0V corresponds to -2^n, and Vref to 2^n.
>> +		 * Add 2^n to offset. (i.e. middle of input range)
>> +		 * offset = offset(sd) * vref / res(sd) * max / vref.
>> +		 */
>> +		if (adc->backend[idx]) {
>> +			iio_backend_read_raw(adc->backend[idx], chan, val,
>> val2, mask);
> 
> Same...
> 
> - Nuno Sá

Re: [PATCH v2 5/8] dt-bindings: iio: add sigma delta modulator backend

[Olivier MOYSAN]

Hi Conor,

On 6/25/24 17:34, Conor Dooley wrote:
> On Tue, Jun 25, 2024 at 05:07:13PM +0200, Olivier Moysan wrote:
>> Add documentation of device tree bindings to support
>> sigma delta modulator backend in IIO framework.
>>
>> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
> 
> I don't review bindings for a job, I can only reliably get to look at
> my mail queue in the evenings, please give me a chance to reply to you
> before you submit a new version.
> 

Sorry, the short review delay.

>> +$id: http://devicetree.org/schemas/iio/adc/sd-modulator-backend.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Sigma delta modulator backend
> 
> Same comments about filename and title apply here as the previous
> version. "TI $foo Sigma Delta Modulator" and drop the reference to back
> ends or the pretence of being generic.
> 

The logic here is the same as for the former sigma delta modulator 
driver. (see discussion [1])
I mean introducing a generic and minimalist driver to support sd 
modulators, but not dedicated to a specific modulator. The ads1201 is 
chosen as a basic modulator here. But it is rather an arbitrary choice.

I agree "backend" reference is not really relevant here. I have to think 
about a way to manage the coexistence of this sigma delta modulator 
driver with its former version.

[1] https://lore.kernel.org/all/6943aaf5-b580-0fd1-7a2e-b99f7a266388@st.com/

BRs
Olivier

> Thanks,
> Conor.

Re: [PATCH v2 5/8] dt-bindings: iio: add sigma delta modulator backend

[Conor Dooley]

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

On Wed, Jun 26, 2024 at 06:40:58PM +0200, Olivier MOYSAN wrote:
> Hi Conor,
> 
> On 6/25/24 17:34, Conor Dooley wrote:
> > On Tue, Jun 25, 2024 at 05:07:13PM +0200, Olivier Moysan wrote:
> > > Add documentation of device tree bindings to support
> > > sigma delta modulator backend in IIO framework.
> > > 
> > > Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
> > 
> > I don't review bindings for a job, I can only reliably get to look at
> > my mail queue in the evenings, please give me a chance to reply to you
> > before you submit a new version.
> > 
> 
> Sorry, the short review delay.
> 
> > > +$id: http://devicetree.org/schemas/iio/adc/sd-modulator-backend.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Sigma delta modulator backend
> > 
> > Same comments about filename and title apply here as the previous
> > version. "TI $foo Sigma Delta Modulator" and drop the reference to back
> > ends or the pretence of being generic.
> > 
> 
> The logic here is the same as for the former sigma delta modulator driver.
> (see discussion [1])
> I mean introducing a generic and minimalist driver to support sd modulators,
> but not dedicated to a specific modulator. The ads1201 is chosen as a basic
> modulator here. But it is rather an arbitrary choice.
> 
> I agree "backend" reference is not really relevant here. I have to think
> about a way to manage the coexistence of this sigma delta modulator driver
> with its former version.

To be blunt, I don't care about drivers! Well I do, but not in this
particular context. You can absolutely have a driver that supports
multiple backends or sigma delta modulators, but right now we are
talking about a binding and this binding supports exactly one sigma
delta modulator - and with an explicit compatible. In that context,
presenting the binding as generic makes little sense.

> [1] https://lore.kernel.org/all/6943aaf5-b580-0fd1-7a2e-b99f7a266388@st.com/

Looking at this though, I question the binding more... The programming
model of the device is identical as a backend or otherwise, so it
shouldn't be getting a new compatible. Isn't this actually as simple as
adding #io-backend-cells to the existing binding and using that to
determine whether the device is being used as a backend or in isolation?

Thanks,
Conor.

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

Re: [PATCH v2 4/8] dt-bindings: iio: dfsdm: move to backend framework

[Rob Herring]

On Tue, Jun 25, 2024 at 05:07:12PM +0200, Olivier Moysan wrote:
> Change the DFSDM binding to use the new IIO backend framework,
> along with the adoption of IIO generic channels.
> This binding change allows to add scaling support to the DFSDM.
> 
> Keep the legacy binding as deprecated for backward compatibility.
> 
> The io-backends property is supported only in generic IIO channel
> binding.
> 
> - Channel description with the generic binding (Audio and Analog):
> 
>   Properties superseded by generic properties:
>     st,adc-channels: becomes "reg" property in channel node
>     st,adc-channel-names: becomes "label" property in channel node
>   Properties moved to channel child node:
>     st,adc-channel-types: becomes st,adc-channel-type
>     st,adc-channel-clk-src, st,adc-alt-channel
> 
> - Analog binding:
> 
>   DFSDM filter channel is configured as an IIO backend consumer.
>   Add io-backends property in channel child nodes.
> 
>   DFSDM is no more configured as a channel consumer from SD modulator.
>   Use of io-channels in DFSDM node is deprecated.
> 
> - Audio binding:
> 
>   DFSDM audio DAI is configured as a channel consumer from DFSDM filter.
>   No change compare to legacy.
> 
> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
> ---
>  .../bindings/iio/adc/st,stm32-dfsdm-adc.yaml  | 157 +++++++++++++++++-
>  1 file changed, 151 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml b/Documentation/devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml
> index c1b1324fa132..1802120b16b0 100644
> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml
> @@ -102,9 +102,11 @@ patternProperties:
>          items:
>            minimum: 0
>            maximum: 7
> +        deprecated: true
>  
>        st,adc-channel-names:
>          description: List of single-ended channel names.
> +        deprecated: true
>  
>        st,filter-order:
>          description: |
> @@ -118,6 +120,12 @@ patternProperties:
>        "#io-channel-cells":
>          const: 1
>  
> +      '#address-cells':
> +        const: 1
> +
> +      '#size-cells':
> +        const: 0
> +
>        st,adc-channel-types:
>          description: |
>            Single-ended channel input type.
> @@ -128,6 +136,7 @@ patternProperties:
>          items:
>            enum: [ SPI_R, SPI_F, MANCH_R, MANCH_F ]
>          $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> +        deprecated: true
>  
>        st,adc-channel-clk-src:
>          description: |
> @@ -139,6 +148,7 @@ patternProperties:
>          items:
>            enum: [ CLKIN, CLKOUT, CLKOUT_F, CLKOUT_R ]
>          $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> +        deprecated: true
>  
>        st,adc-alt-channel:
>          description:
> @@ -147,6 +157,7 @@ patternProperties:
>            If not set, channel n is connected to SPI input n.
>            If set, channel n is connected to SPI input n + 1.
>          type: boolean
> +        deprecated: true
>  
>        st,filter0-sync:
>          description:
> @@ -165,11 +176,64 @@ patternProperties:
>        - compatible
>        - reg
>        - interrupts
> -      - st,adc-channels
> -      - st,adc-channel-names
>        - st,filter-order
>        - "#io-channel-cells"
>  
> +    patternProperties:
> +      "^channel@([0-9]|1[0-9])$":

Unit-addresses are normally hex. And according to reg below, the max 
value is 8.

> +        type: object
> +        $ref: adc.yaml
> +        description: Represents the external channels which are connected to the DFSDM.
> +
> +        properties:
> +          reg:
> +            items:
> +              minimum: 0
> +              maximum: 8

More than 1 reg entry valid? Either way, you need maxItems. Or you can 
just drop 'items'

> +
> +          label:
> +            description:
> +              Unique name to identify which channel this is.
> +
> +          st,adc-channel-type:
> +            description: |
> +              Single-ended channel input type.
> +              - "SPI_R": SPI with data on rising edge (default)
> +              - "SPI_F": SPI with data on falling edge
> +              - "MANCH_R": manchester codec, rising edge = logic 0, falling edge = logic 1
> +              - "MANCH_F": manchester codec, rising edge = logic 1, falling edge = logic 0
> +            items:

'items' is for arrays, but...

> +              enum: [ SPI_R, SPI_F, MANCH_R, MANCH_F ]
> +            $ref: /schemas/types.yaml#/definitions/string

not an array.

> +
> +          st,adc-channel-clk-src:
> +            description: |
> +              Conversion clock source.
> +              - "CLKIN": external SPI clock (CLKIN x)
> +              - "CLKOUT": internal SPI clock (CLKOUT) (default)
> +              - "CLKOUT_F": internal SPI clock divided by 2 (falling edge).
> +              - "CLKOUT_R": internal SPI clock divided by 2 (rising edge).
> +            items:

ditto

> +              enum: [ CLKIN, CLKOUT, CLKOUT_F, CLKOUT_R ]
> +            $ref: /schemas/types.yaml#/definitions/string
> +
> +          st,adc-alt-channel:
> +            description:
> +              Must be defined if two sigma delta modulators are
> +              connected on same SPI input.
> +              If not set, channel n is connected to SPI input n.
> +              If set, channel n is connected to SPI input n + 1.
> +            type: boolean
> +
> +          io-backends:
> +            description:
> +              Used to pipe external sigma delta modulator or internal ADC backend to DFSDM channel.

How many entries (maxItems)?

> +
> +        required:
> +          - reg
> +
> +        additionalProperties: false

Put this next to the $ref for the node. And switch to 
unevaluatedProperties and drop 'label' from here.

> +
>      allOf:
>        - if:
>            properties:
> @@ -199,9 +263,19 @@ patternProperties:
>                description:
>                  From common IIO binding. Used to pipe external sigma delta
>                  modulator or internal ADC output to DFSDM channel.
> +              deprecated: true
>  
> -          required:
> -            - io-channels
> +          if:
> +            required:
> +              - st,adc-channels
> +          then:
> +            required:
> +              - io-channels
> +
> +          patternProperties:
> +            "^channel@([0-9]|1[0-9])$":
> +              required:
> +                - io-backends

Don't think this is needed here. If channel node is present, the 
io-backends should always be required, right? Then this can go under the 
node schema.

Rob

Re: [PATCH v2 1/8] iio: add read raw service to iio backend framework

[Jonathan Cameron]

On Tue, 25 Jun 2024 17:07:09 +0200
Olivier Moysan <olivier.moysan@foss.st.com> wrote:

> Add iio_backend_read_raw() service to support attributes read
> from an IIO backend.
> 
> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
Other than line wrapping moans this looks good to me.

J
> ---
>  drivers/iio/industrialio-backend.c | 21 +++++++++++++++++++++
>  include/linux/iio/backend.h        |  6 +++++-
>  2 files changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
> index 929aff4040ed..0e2653de1956 100644
> --- a/drivers/iio/industrialio-backend.c
> +++ b/drivers/iio/industrialio-backend.c
> @@ -357,6 +357,27 @@ int devm_iio_backend_request_buffer(struct device *dev,
>  }
>  EXPORT_SYMBOL_NS_GPL(devm_iio_backend_request_buffer, IIO_BACKEND);
>  
> +/**
> + * iio_backend_read_raw - Request a channel attribute from the IIO backend.
> + * @back:	Backend device
> + * @chan:	IIO channel reference
> + * @val:	First element of the returned value
> + * @val2:	Second element of the returned value
> + * @mask:	Specify value to retrieve
> + *
> + * This callback replicates the read_raw callback of the IIO framework, and is intended to
> + * request miscellaneous channel attributes from the backend device.

For IIO code, please still wrap at 80 chars unless there is a good reason to
got longer.

> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_read_raw(struct iio_backend *back, struct iio_chan_spec const *chan, int *val,
Likewise. Wrap this shorter.

> +			 int *val2, long mask)
> +{
> +	return iio_backend_op_call(back, read_raw, chan, val, val2, mask);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_read_raw, IIO_BACKEND);
> +
>  static struct iio_backend *iio_backend_from_indio_dev_parent(const struct device *dev)
>  {
>  	struct iio_backend *back = ERR_PTR(-ENODEV), *iter;
> diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
> index 8099759d7242..24185718b20d 100644
> --- a/include/linux/iio/backend.h
> +++ b/include/linux/iio/backend.h
> @@ -81,6 +81,7 @@ enum iio_backend_sample_trigger {
>   * @extend_chan_spec: Extend an IIO channel.
>   * @ext_info_set: Extended info setter.
>   * @ext_info_get: Extended info getter.
> + * @read_raw: Read value from a backend device
>   **/
>  struct iio_backend_ops {
>  	int (*enable)(struct iio_backend *back);
> @@ -113,6 +114,8 @@ struct iio_backend_ops {
>  			    const char *buf, size_t len);
>  	int (*ext_info_get)(struct iio_backend *back, uintptr_t private,
>  			    const struct iio_chan_spec *chan, char *buf);
> +	int (*read_raw)(struct iio_backend *back, struct iio_chan_spec const *chan, int *val,
And here.
> +			int *val2, long mask);
>  };
>  
>  int iio_backend_chan_enable(struct iio_backend *back, unsigned int chan);
> @@ -141,7 +144,8 @@ ssize_t iio_backend_ext_info_set(struct iio_dev *indio_dev, uintptr_t private,
>  				 const char *buf, size_t len);
>  ssize_t iio_backend_ext_info_get(struct iio_dev *indio_dev, uintptr_t private,
>  				 const struct iio_chan_spec *chan, char *buf);
> -
> +int iio_backend_read_raw(struct iio_backend *back, struct iio_chan_spec const *chan, int *val,
and here.

> +			 int *val2, long mask);
>  int iio_backend_extend_chan_spec(struct iio_dev *indio_dev,
>  				 struct iio_backend *back,
>  				 struct iio_chan_spec *chan);

Re: [PATCH v2 3/8] iio: add child nodes support in iio backend framework

[Jonathan Cameron]

On Tue, 25 Jun 2024 17:07:11 +0200
Olivier Moysan <olivier.moysan@foss.st.com> wrote:

> Add an API to support IIO generic channels binding:
> http://devicetree.org/schemas/iio/adc/adc.yaml#
> This new API is needed, as generic channel DT node isn't populated as a
> device.
> Add devm_iio_backend_fwnode_get() to allow an IIO device backend
> consumer to reference backend phandles in its child nodes.
> 
> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
> Reviewed-by: Nuno Sa <nuno.sa@analog.com>

A passing comment inline. I'm not asking for any changes in this
series (unless you want to make it more complex ;)

> ---
>  drivers/iio/industrialio-backend.c | 62 +++++++++++++++++++++---------
>  include/linux/iio/backend.h        |  2 +
>  2 files changed, 45 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
> index 6be1fa9a960b..8cc959ac278a 100644
> --- a/drivers/iio/industrialio-backend.c
> +++ b/drivers/iio/industrialio-backend.c
> @@ -577,19 +577,10 @@ static int __devm_iio_backend_get(struct device *dev, struct iio_backend *back)
>  	return 0;
>  }
>  
> -/**
> - * devm_iio_backend_get - Device managed backend device get
> - * @dev: Consumer device for the backend
> - * @name: Backend name
> - *
> - * Get's the backend associated with @dev.
> - *
> - * RETURNS:
> - * A backend pointer, negative error pointer otherwise.
> - */
> -struct iio_backend *devm_iio_backend_get(struct device *dev, const char *name)
> +static struct iio_backend *__devm_iio_backend_fwnode_get(struct device *dev, const char *name,
> +							 struct fwnode_handle *fwnode)
>  {
> -	struct fwnode_handle *fwnode;
> +	struct fwnode_handle *fwnode_back;
>  	struct iio_backend *back;
>  	unsigned int index;
>  	int ret;
> @@ -604,19 +595,19 @@ struct iio_backend *devm_iio_backend_get(struct device *dev, const char *name)
>  		index = 0;
>  	}
>  
> -	fwnode = fwnode_find_reference(dev_fwnode(dev), "io-backends", index);
> -	if (IS_ERR(fwnode)) {
> -		dev_err_probe(dev, PTR_ERR(fwnode),
> +	fwnode_back = fwnode_find_reference(fwnode, "io-backends", index);

Not really related, but this looks 'ripe' for some cleanup.h magic.

> +	if (IS_ERR(fwnode_back)) {
> +		dev_err_probe(dev, PTR_ERR(fwnode_back),
>  			      "Cannot get Firmware reference\n");
> -		return ERR_CAST(fwnode);
> +		return ERR_CAST(fwnode_back);
>  	}
>  
>  	guard(mutex)(&iio_back_lock);
>  	list_for_each_entry(back, &iio_back_list, entry) {
> -		if (!device_match_fwnode(back->dev, fwnode))
> +		if (!device_match_fwnode(back->dev, fwnode_back))
>  			continue;
>  
> -		fwnode_handle_put(fwnode);
> +		fwnode_handle_put(fwnode_back);
>  		ret = __devm_iio_backend_get(dev, back);
>  		if (ret)
>  			return ERR_PTR(ret);
> @@ -624,11 +615,44 @@ struct iio_backend *devm_iio_backend_get(struct device *dev, const char *name)
>  		return back;
>  	}
>  
> -	fwnode_handle_put(fwnode);
> +	fwnode_handle_put(fwnode_back);
>  	return ERR_PTR(-EPROBE_DEFER);
>  }

Re: [PATCH v2 6/8] iio: adc: stm32-dfsdm: adopt generic channels bindings

[Jonathan Cameron]

On Tue, 25 Jun 2024 17:07:14 +0200
Olivier Moysan <olivier.moysan@foss.st.com> wrote:

> Move to generic channels binding to ease new backend framework adoption
> and prepare the convergence with MDF IP support on STM32MP2 SoC family.
> 
> Legacy binding:
> DFSDM is an IIO channel consumer.
> SD modulator is an IIO channels provider.
> The channel phandles are provided in DT through io-channels property
> and channel indexes through st,adc-channels property.
> 
> New binding:
> DFSDM is an IIO channel provider.
> The channel indexes are given by reg property in channel child node.
> 
> This new binding is intended to be used with SD modulator IIO backends.
> It does not support SD modulator legacy IIO devices.
> The st,adc-channels property presence is used to discriminate
> between legacy and backend bindings.
> 
> The support of the DFSDM legacy channels and SD modulator IIO devices
> is kept for backward compatibility.
> 
> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>

Hi Olivier

Some minor comments inline.

thanks,

Jonathan



> @@ -1362,15 +1422,20 @@ static int stm32_dfsdm_dma_request(struct device *dev,
>  	return 0;
>  }
>  
> -static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev,
> -					 struct iio_chan_spec *ch)
> +static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev, struct iio_chan_spec *ch,
> +					 struct fwnode_handle *child)
>  {
>  	struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
>  	int ret;
>  
> -	ret = stm32_dfsdm_channel_parse_of(adc->dfsdm, indio_dev, ch);
> -	if (ret < 0)
> +	if (child)
> +		ret = stm32_dfsdm_generic_channel_parse_of(adc->dfsdm, indio_dev, ch, child);
> +	else /* Legacy binding */
> +		ret = stm32_dfsdm_channel_parse_of(adc->dfsdm, indio_dev, ch);
> +	if (ret < 0) {
> +		dev_err(&indio_dev->dev, "Failed to parse channel\n");
>  		return ret;

return dev_err_probe() assuming only called from probe() which I think is the case
but haven't actually checked.


> +	}
>  
>  	ch->type = IIO_VOLTAGE;
>  	ch->indexed = 1;
> @@ -1385,6 +1450,7 @@ static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev,
>  
>  	if (adc->dev_data->type == DFSDM_AUDIO) {
>  		ch->ext_info = dfsdm_adc_audio_ext_info;
> +		ch->scan_index = 0;
>  	} else {
>  		ch->scan_type.shift = 8;
>  	}
> @@ -1392,8 +1458,51 @@ static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev,
>  	ch->scan_type.realbits = 24;
>  	ch->scan_type.storagebits = 32;
>  
> -	return stm32_dfsdm_chan_configure(adc->dfsdm,
> -					  &adc->dfsdm->ch_list[ch->channel]);
> +	return stm32_dfsdm_chan_configure(adc->dfsdm, &adc->dfsdm->ch_list[ch->channel]);
Is there a change here? If it's just a line wrap don't do that in same patch
making real changes.

> +}
> +
> +static int stm32_dfsdm_chan_init(struct iio_dev *indio_dev, struct iio_chan_spec *channels)
> +{
> +	int num_ch = indio_dev->num_channels;
> +	int chan_idx = 0, ret = 0;

	int ret;

> +
> +	for (chan_idx = 0; chan_idx < num_ch; chan_idx++) {
> +		channels[chan_idx].scan_index = chan_idx;
> +		ret = stm32_dfsdm_adc_chan_init_one(indio_dev, &channels[chan_idx], NULL);
> +		if (ret < 0) {
> +			dev_err(&indio_dev->dev, "Channels init failed\n");
> +			return ret;
			return dev_err_probe()
(I think this is only called from probe?)

> +		}
> +	}
> +
> +	return ret;
return 0;
at least I assume it can't be positive? 
> +}
> +
> +static int stm32_dfsdm_generic_chan_init(struct iio_dev *indio_dev, struct iio_chan_spec *channels)
> +{
> +	struct fwnode_handle *child;
> +	int chan_idx = 0, ret;
> +
> +	device_for_each_child_node(&indio_dev->dev, child) {

device_for_each_child_node_scoped()  as then you can do direct returns.


> +		/* Skip DAI node in DFSDM audio nodes */
> +		if (fwnode_property_present(child, "compatible"))
> +			continue;
> +
> +		channels[chan_idx].scan_index = chan_idx;
> +		ret = stm32_dfsdm_adc_chan_init_one(indio_dev, &channels[chan_idx], child);
> +		if (ret < 0) {
> +			dev_err(&indio_dev->dev, "Channels init failed\n");
> +			goto err;
return dev_err_probe() once using scoped above.


> +		}
> +
> +		chan_idx++;
> +	}
> +	return chan_idx;
> +
> +err:
> +	fwnode_handle_put(child);
> +
> +	return ret;
>  }
>
>  
> @@ -1430,43 +1547,60 @@ static int stm32_dfsdm_adc_init(struct device *dev, struct iio_dev *indio_dev)

> -	ch = devm_kcalloc(&indio_dev->dev, num_ch, sizeof(*ch),
> -			  GFP_KERNEL);
> -	if (!ch)
> -		return -ENOMEM;
> +	if (legacy) {
> +		/* Bind to SD modulator IIO device. */
> +		adc->hwc = devm_iio_hw_consumer_alloc(&indio_dev->dev);
> +		if (IS_ERR(adc->hwc))
> +			return -EPROBE_DEFER;

Obviously in the legacy path, but worth a dev_err_probe() because
if we defer, debug info gets stashed away so it is easy to see
why a driver is continuing to defer.

> +	} else {
> +		/* Generic binding. SD modulator IIO device not used. Use SD modulator backend. */
> +		adc->hwc = NULL;

Re: [PATCH v2 5/8] dt-bindings: iio: add sigma delta modulator backend

[Olivier MOYSAN]

Hi Conor,

On 6/27/24 18:13, Conor Dooley wrote:
> On Wed, Jun 26, 2024 at 06:40:58PM +0200, Olivier MOYSAN wrote:
>> Hi Conor,
>>
>> On 6/25/24 17:34, Conor Dooley wrote:
>>> On Tue, Jun 25, 2024 at 05:07:13PM +0200, Olivier Moysan wrote:
>>>> Add documentation of device tree bindings to support
>>>> sigma delta modulator backend in IIO framework.
>>>>
>>>> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
>>>
>>> I don't review bindings for a job, I can only reliably get to look at
>>> my mail queue in the evenings, please give me a chance to reply to you
>>> before you submit a new version.
>>>
>>
>> Sorry, the short review delay.
>>
>>>> +$id: http://devicetree.org/schemas/iio/adc/sd-modulator-backend.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Sigma delta modulator backend
>>>
>>> Same comments about filename and title apply here as the previous
>>> version. "TI $foo Sigma Delta Modulator" and drop the reference to back
>>> ends or the pretence of being generic.
>>>
>>
>> The logic here is the same as for the former sigma delta modulator driver.
>> (see discussion [1])
>> I mean introducing a generic and minimalist driver to support sd modulators,
>> but not dedicated to a specific modulator. The ads1201 is chosen as a basic
>> modulator here. But it is rather an arbitrary choice.
>>
>> I agree "backend" reference is not really relevant here. I have to think
>> about a way to manage the coexistence of this sigma delta modulator driver
>> with its former version.
> 
> To be blunt, I don't care about drivers! Well I do, but not in this
> particular context. You can absolutely have a driver that supports
> multiple backends or sigma delta modulators, but right now we are
> talking about a binding and this binding supports exactly one sigma
> delta modulator - and with an explicit compatible. In that context,
> presenting the binding as generic makes little sense.
> 
>> [1] https://lore.kernel.org/all/6943aaf5-b580-0fd1-7a2e-b99f7a266388@st.com/
> 
> Looking at this though, I question the binding more... The programming
> model of the device is identical as a backend or otherwise, so it
> shouldn't be getting a new compatible. Isn't this actually as simple as
> adding #io-backend-cells to the existing binding and using that to
> determine whether the device is being used as a backend or in isolation?
>

For sure. I came to the same conclusion. My first idea was to isolate 
the deprecated binding. But I agree that the best approach is to adapt 
the existing binding. I prepared a v3 like this.

BRs
Olivier

> Thanks,
> Conor.

Re: [PATCH v2 5/8] dt-bindings: iio: add sigma delta modulator backend

[Olivier MOYSAN]

Hi Conor,

On 6/27/24 18:13, Conor Dooley wrote:
> On Wed, Jun 26, 2024 at 06:40:58PM +0200, Olivier MOYSAN wrote:
>> Hi Conor,
>>
>> On 6/25/24 17:34, Conor Dooley wrote:
>>> On Tue, Jun 25, 2024 at 05:07:13PM +0200, Olivier Moysan wrote:
>>>> Add documentation of device tree bindings to support
>>>> sigma delta modulator backend in IIO framework.
>>>>
>>>> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
>>>
>>> I don't review bindings for a job, I can only reliably get to look at
>>> my mail queue in the evenings, please give me a chance to reply to you
>>> before you submit a new version.
>>>
>>
>> Sorry, the short review delay.
>>
>>>> +$id: http://devicetree.org/schemas/iio/adc/sd-modulator-backend.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Sigma delta modulator backend
>>>
>>> Same comments about filename and title apply here as the previous
>>> version. "TI $foo Sigma Delta Modulator" and drop the reference to back
>>> ends or the pretence of being generic.
>>>
>>
>> The logic here is the same as for the former sigma delta modulator driver.
>> (see discussion [1])
>> I mean introducing a generic and minimalist driver to support sd modulators,
>> but not dedicated to a specific modulator. The ads1201 is chosen as a basic
>> modulator here. But it is rather an arbitrary choice.
>>
>> I agree "backend" reference is not really relevant here. I have to think
>> about a way to manage the coexistence of this sigma delta modulator driver
>> with its former version.
> 
> To be blunt, I don't care about drivers! Well I do, but not in this
> particular context. You can absolutely have a driver that supports
> multiple backends or sigma delta modulators, but right now we are
> talking about a binding and this binding supports exactly one sigma
> delta modulator - and with an explicit compatible. In that context,
> presenting the binding as generic makes little sense.
> 
>> [1] https://lore.kernel.org/all/6943aaf5-b580-0fd1-7a2e-b99f7a266388@st.com/
> 
> Looking at this though, I question the binding more... The programming
> model of the device is identical as a backend or otherwise, so it
> shouldn't be getting a new compatible. Isn't this actually as simple as
> adding #io-backend-cells to the existing binding and using that to
> determine whether the device is being used as a backend or in isolation?
> 

For sure. I came to the same conclusion. My first idea was to isolate 
the deprecated binding. However, I agree that the best approach is to 
adapt the existing binding. I prepared a v3 like this.

BRs
Olivier

> Thanks,
> Conor.

Re: [PATCH v2 4/8] dt-bindings: iio: dfsdm: move to backend framework

[Olivier MOYSAN]

Hi Rob,

On 6/28/24 23:35, Rob Herring wrote:
> On Tue, Jun 25, 2024 at 05:07:12PM +0200, Olivier Moysan wrote:
>> Change the DFSDM binding to use the new IIO backend framework,
>> along with the adoption of IIO generic channels.
>> This binding change allows to add scaling support to the DFSDM.
>>
>> Keep the legacy binding as deprecated for backward compatibility.
>>
>> The io-backends property is supported only in generic IIO channel
>> binding.
>>
>> - Channel description with the generic binding (Audio and Analog):
>>
>>    Properties superseded by generic properties:
>>      st,adc-channels: becomes "reg" property in channel node
>>      st,adc-channel-names: becomes "label" property in channel node
>>    Properties moved to channel child node:
>>      st,adc-channel-types: becomes st,adc-channel-type
>>      st,adc-channel-clk-src, st,adc-alt-channel
>>
>> - Analog binding:
>>
>>    DFSDM filter channel is configured as an IIO backend consumer.
>>    Add io-backends property in channel child nodes.
>>
>>    DFSDM is no more configured as a channel consumer from SD modulator.
>>    Use of io-channels in DFSDM node is deprecated.
>>
>> - Audio binding:
>>
>>    DFSDM audio DAI is configured as a channel consumer from DFSDM filter.
>>    No change compare to legacy.
>>
>> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
>> ---
>>   .../bindings/iio/adc/st,stm32-dfsdm-adc.yaml  | 157 +++++++++++++++++-
>>   1 file changed, 151 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml b/Documentation/devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml
>> index c1b1324fa132..1802120b16b0 100644
>> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml
>> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml
>> @@ -102,9 +102,11 @@ patternProperties:
>>           items:
>>             minimum: 0
>>             maximum: 7
>> +        deprecated: true
>>   
>>         st,adc-channel-names:
>>           description: List of single-ended channel names.
>> +        deprecated: true
>>   
>>         st,filter-order:
>>           description: |
>> @@ -118,6 +120,12 @@ patternProperties:
>>         "#io-channel-cells":
>>           const: 1
>>   
>> +      '#address-cells':
>> +        const: 1
>> +
>> +      '#size-cells':
>> +        const: 0
>> +
>>         st,adc-channel-types:
>>           description: |
>>             Single-ended channel input type.
>> @@ -128,6 +136,7 @@ patternProperties:
>>           items:
>>             enum: [ SPI_R, SPI_F, MANCH_R, MANCH_F ]
>>           $ref: /schemas/types.yaml#/definitions/non-unique-string-array
>> +        deprecated: true
>>   
>>         st,adc-channel-clk-src:
>>           description: |
>> @@ -139,6 +148,7 @@ patternProperties:
>>           items:
>>             enum: [ CLKIN, CLKOUT, CLKOUT_F, CLKOUT_R ]
>>           $ref: /schemas/types.yaml#/definitions/non-unique-string-array
>> +        deprecated: true
>>   
>>         st,adc-alt-channel:
>>           description:
>> @@ -147,6 +157,7 @@ patternProperties:
>>             If not set, channel n is connected to SPI input n.
>>             If set, channel n is connected to SPI input n + 1.
>>           type: boolean
>> +        deprecated: true
>>   
>>         st,filter0-sync:
>>           description:
>> @@ -165,11 +176,64 @@ patternProperties:
>>         - compatible
>>         - reg
>>         - interrupts
>> -      - st,adc-channels
>> -      - st,adc-channel-names
>>         - st,filter-order
>>         - "#io-channel-cells"
>>   
>> +    patternProperties:
>> +      "^channel@([0-9]|1[0-9])$":
> 
> Unit-addresses are normally hex. And according to reg below, the max
> value is 8.
> 

Right. The maximum number of serial interfaces is 8.
So, the pattern can be reduced to "^channel@([0-7])$":

>> +        type: object
>> +        $ref: adc.yaml
>> +        description: Represents the external channels which are connected to the DFSDM.
>> +
>> +        properties:
>> +          reg:
>> +            items:
>> +              minimum: 0
>> +              maximum: 8
> 
> More than 1 reg entry valid? Either way, you need maxItems. Or you can
> just drop 'items'
> 

Added "maxItems: 1" and dropped items.

>> +
>> +          label:
>> +            description:
>> +              Unique name to identify which channel this is.
>> +
>> +          st,adc-channel-type:
>> +            description: |
>> +              Single-ended channel input type.
>> +              - "SPI_R": SPI with data on rising edge (default)
>> +              - "SPI_F": SPI with data on falling edge
>> +              - "MANCH_R": manchester codec, rising edge = logic 0, falling edge = logic 1
>> +              - "MANCH_F": manchester codec, rising edge = logic 1, falling edge = logic 0
>> +            items:
> 
> 'items' is for arrays, but...
> 

Removed items

>> +              enum: [ SPI_R, SPI_F, MANCH_R, MANCH_F ]
>> +            $ref: /schemas/types.yaml#/definitions/string
> 
> not an array.
> 
>> +
>> +          st,adc-channel-clk-src:
>> +            description: |
>> +              Conversion clock source.
>> +              - "CLKIN": external SPI clock (CLKIN x)
>> +              - "CLKOUT": internal SPI clock (CLKOUT) (default)
>> +              - "CLKOUT_F": internal SPI clock divided by 2 (falling edge).
>> +              - "CLKOUT_R": internal SPI clock divided by 2 (rising edge).
>> +            items:
> 
> ditto
> 

Done

>> +              enum: [ CLKIN, CLKOUT, CLKOUT_F, CLKOUT_R ]
>> +            $ref: /schemas/types.yaml#/definitions/string
>> +
>> +          st,adc-alt-channel:
>> +            description:
>> +              Must be defined if two sigma delta modulators are
>> +              connected on same SPI input.
>> +              If not set, channel n is connected to SPI input n.
>> +              If set, channel n is connected to SPI input n + 1.
>> +            type: boolean
>> +
>> +          io-backends:
>> +            description:
>> +              Used to pipe external sigma delta modulator or internal ADC backend to DFSDM channel.
> 
> How many entries (maxItems)?
> 
>> +
>> +        required:
>> +          - reg
>> +
>> +        additionalProperties: false
> 
> Put this next to the $ref for the node. And switch to
> unevaluatedProperties and drop 'label' from here.
> 

Done

>> +
>>       allOf:
>>         - if:
>>             properties:
>> @@ -199,9 +263,19 @@ patternProperties:
>>                 description:
>>                   From common IIO binding. Used to pipe external sigma delta
>>                   modulator or internal ADC output to DFSDM channel.
>> +              deprecated: true
>>   
>> -          required:
>> -            - io-channels
>> +          if:
>> +            required:
>> +              - st,adc-channels
>> +          then:
>> +            required:
>> +              - io-channels
>> +
>> +          patternProperties:
>> +            "^channel@([0-9]|1[0-9])$":
>> +              required:
>> +                - io-backends
> 
> Don't think this is needed here. If channel node is present, the
> io-backends should always be required, right? Then this can go under the
> node schema.
> 

The io-backends property is required only when we use st,stm32-dfsdm-adc 
compatible. In other words, when we are in an analog use case. In this 
case the channel is a consumer of a backend (typically a sd modulator)
In an audio use case (compatible st,stm32-dfsdm-dmic) the backend is not 
required.

BRs
Olivier

> Rob
> 
>