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@xxxxxxxxxxx>
> > ---
> >   .../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