Re: [PATCH] ASoC: dt-bindings: stm32: convert sai to json-schema

[Rob Herring]

On Fri, Feb 7, 2020 at 7:42 AM Olivier MOYSAN <olivier.moysan@xxxxxx> wrote:
>
> Hi Rob,
>
> On 2/6/20 7:21 PM, Rob Herring wrote:
> > On Thu, Jan 30, 2020 at 02:50:40PM +0100, Olivier Moysan wrote:
> >> Convert the STM32 SAI bindings to DT schema format using json-schema.
> >>
> >> Signed-off-by: Olivier Moysan <olivier.moysan@xxxxxx>
> >> ---
> >>   .../bindings/sound/st,stm32-sai.txt           | 107 ----------
> >>   .../bindings/sound/st,stm32-sai.yaml          | 193 ++++++++++++++++++
> >>   2 files changed, 193 insertions(+), 107 deletions(-)
> >>   delete mode 100644 Documentation/devicetree/bindings/sound/st,stm32-sai.txt
> >>   create mode 100644 Documentation/devicetree/bindings/sound/st,stm32-sai.yaml
> >
> >> diff --git a/Documentation/devicetree/bindings/sound/st,stm32-sai.yaml b/Documentation/devicetree/bindings/sound/st,stm32-sai.yaml
> >> new file mode 100644
> >> index 000000000000..33dca007fc86
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/sound/st,stm32-sai.yaml
> >> @@ -0,0 +1,193 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/sound/st,stm32-sai.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: STMicroelectronics STM32 Serial Audio Interface (SAI)
> >> +
> >> +maintainers:
> >> +  - Olivier Moysan <olivier.moysan@xxxxxx>
> >> +
> >> +description:
> >> +  The SAI interface (Serial Audio Interface) offers a wide set of audio
> >> +  protocols as I2S standards, LSB or MSB-justified, PCM/DSP, TDM, and AC'97.
> >> +  The SAI contains two independent audio sub-blocks. Each sub-block has
> >> +  its own clock generator and I/O lines controller.
> >> +
> >> +properties:
> >> +  compatible:
> >> +    enum:
> >> +      - st,stm32f4-sai
> >> +      - st,stm32h7-sai
> >> +
> >> +  reg:
> >> +    items:
> >> +      - description: Base address and size of SAI common register set.
> >> +      - description: Base address and size of SAI identification register set.
> >> +    minItems: 1
> >> +    maxItems: 2
> >> +
> >> +  ranges:
> >> +    maxItems: 1
> >> +
> >> +  interrupts:
> >> +    maxItems: 1
> >> +
> >> +  resets:
> >> +    maxItems: 1
> >> +
> >> +  "#address-cells":
> >> +    const: 1
> >> +
> >> +  "#size-cells":
> >> +    const: 1
> >> +
> >> +  clocks:
> >> +    items:
> >> +      - description: pclk feeds the peripheral bus interface.
> >> +      - description: x8k, SAI parent clock for sampling rates multiple of 8kHz.
> >> +      - description: x11k, SAI parent clock for sampling rates multiple of 11.025kHz.
> >> +
> >> +  clock-names:
> >> +    items:
> >> +      enum: [ pclk, x8k, x11k ]
> >> +    minItems: 3
> >> +    maxItems: 3
> >> +
> >> +required:
> >> +  - compatible
> >> +  - reg
> >> +  - ranges
> >> +  - "#address-cells"
> >> +  - "#size-cells"
> >> +  - clocks
> >> +  - clock-names
> >> +
> >> +patternProperties:
> >> +  "^audio-controller@[0-9a-f]+$":
> >> +    type: object
> >> +    description:
> >> +      Two subnodes corresponding to SAI sub-block instances A et B
> >> +      can be defined. Subnode can be omitted for unsused sub-block.
> >> +
> >> +    properties:
> >> +      compatible:
> >> +        description: Compatible for SAI sub-block A or B.
> >> +        enum:
> >> +          - st,stm32-sai-sub-a
> >> +          - st,stm32-sai-sub-b
> > pattern: 'st,stm32-sai-sub-[ab]'
> I will change this in v2
> >> +
> >> +      "#sound-dai-cells":
> >> +        const: 0
> >> +
> >> +      reg:
> >> +        maxItems: 1
> >> +
> >> +      clocks:
> >> +        items:
> >> +          - description: sai_ck clock feeding the internal clock generator.
> >> +          - description: MCLK clock from a SAI set as master clock provider.
> >> +        minItems: 1
> >> +        maxItems: 2
> >> +
> >> +      clock-names:
> >> +        items:
> >> +          - const: sai_ck
> >> +          - const: MCLK
> >> +        minItems: 1
> >> +        maxItems: 2
> >> +
> >> +      dmas:
> >> +        items:
> >> +          - description: SAI sub-block is configured as a capture DAI.
> >> +          - description: SAI sub-block is configured as a playback DAI.
> >> +        minItems: 1
> >> +        maxItems: 1
> > This is defining that dmas has 2 entries, but then limits it to the 1st
> > entry only.
> dma can be either "rx" or "tx", but not both.
> Maybe, the following syntax is more appropriate:
>
>        dmas:
>          maxItems: 1
>
>        dma-names:
>          description: |
>            rx: SAI sub-block is configured as a capture DAI.
>            tx: SAI sub-block is configured as a playback DAI.
>          items:
>            - enum: [ rx, tx ]

Yes, but for a single entry you can drop 'items'.

>
> >> +
> >> +      dma-names:
> >> +        items:
> >> +          - enum: [ rx, tx ]
> >> +
> >> +      st,sync:
> >> +        description:
> >> +          Configure the SAI sub-block as slave of another SAI sub-block.
> >> +          By default SAI sub-block is in asynchronous mode.
> >> +          Must contain the phandle and index of the SAI sub-block providing
> >> +          the synchronization.
> >> +        allOf:
> >> +          - $ref: /schemas/types.yaml#definitions/phandle-array
> >> +          - maxItems: 1
> >> +
> >> +      st,iec60958:
> >> +        description:
> >> +          If set, support S/PDIF IEC6958 protocol for playback.
> >> +          IEC60958 protocol is not available for capture.
> >> +          By default, custom protocol is assumed, meaning that protocol is
> >> +          configured according to protocol defined in related DAI link node,
> >> +          such as i2s, left justified, right justified, dsp and pdm protocols.
> >> +        allOf:
> >> +          - $ref: /schemas/types.yaml#definitions/flag
> >> +
> >> +      "#clock-cells":
> >> +        description: Configure the SAI device as master clock provider.
> >> +        const: 0
> >> +
> >> +    required:
> >> +      - compatible
> >> +      - "#sound-dai-cells"
> >> +      - reg
> >> +      - clocks
> >> +      - clock-names
> >> +      - dmas
> >> +      - dma-names
> >         additionalProperties: false.
> >
> >> +
> >> +allOf:
> >> +  - if:
> >> +      properties:
> >> +        compatible:
> >> +          contains:
> >> +            const: st,stm32f4-sai
> >> +
> >> +  - then:
> >> +      properties:
> >> +        clocks:
> >> +          minItems: 2
> >> +          maxItems: 2
> >> +
> >> +        clock-names:
> >> +          items:
> >> +            enum: [ x8k, x11k ]
> > Define the order.
> >
> Do you mean, adding in clocks property  :
>            items:
>              - description: x8k, SAI parent clock for sampling rates
> multiple of 8kHz.
>              - description: x11k, SAI parent clock for sampling rates
> multiple of 11.025kHz.

That too, but for clocks you need:

clock-names:
  items:
    - const: x8k
    - const: x11k

> But, it seems to me that this is redundant with previous definition of
> clocks property.

It's not because it's clocks in different positions.

Rob