[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 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

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

[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

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