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

[Olivier MOYSAN]

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 ]

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

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

Thanks
Olivier
>> +          minItems: 2
>> +          maxItems: 2
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/clock/stm32mp1-clks.h>
>> +    #include <dt-bindings/reset/stm32mp1-resets.h>
>> +    sai1: sai@4400a000 {
>> +      compatible = "st,stm32h7-sai";
>> +      #address-cells = <1>;
>> +      #size-cells = <1>;
>> +      ranges = <0 0x4400a000 0x400>;
>> +      reg = <0x4400a000 0x4>, <0x4400a3f0 0x10>;
>> +      interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
>> +      clocks = <&rcc SAI1>, <&rcc PLL1_Q>, <&rcc PLL2_P>;
>> +      clock-names = "pclk", "x8k", "x11k";
>> +      resets = <&rcc SAI1_R>;
>> +
>> +      sai1a: audio-controller@4400a004 {
>> +        compatible = "st,stm32-sai-sub-a";
>> +        #sound-dai-cells = <0>;
>> +        reg = <0x4 0x1c>;
>> +        clocks = <&rcc SAI1_K>;
>> +        clock-names = "sai_ck";
>> +        dmas = <&dmamux1 87 0x400 0x01>;
>> +        dma-names = "tx";
>> +      };
>> +    };
>> +
>> +...
>> -- 
>> 2.17.1
>>