Re: [PATCH v2 1/2] dt-bindings: sound: Add FSL CPU DAI bindings

[Rob Herring]

On Fri, Mar 06, 2020 at 01:13:52PM +0200, Daniel Baluta wrote:
> From: Daniel Baluta <daniel.baluta@xxxxxxx>
> 
> Add dt bindings for he Generic FSL CPU DAI.
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@xxxxxxx>
> ---
>  .../devicetree/bindings/sound/fsl,dai.yaml    | 97 +++++++++++++++++++
>  1 file changed, 97 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/fsl,dai.yaml
> 
> diff --git a/Documentation/devicetree/bindings/sound/fsl,dai.yaml b/Documentation/devicetree/bindings/sound/fsl,dai.yaml
> new file mode 100644
> index 000000000000..e6426af67d30
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/fsl,dai.yaml
> @@ -0,0 +1,97 @@
> +# SPDX-License-Identifier: GPL-2.0

Dual license new bindings please:

(GPL-2.0-only OR BSD-2-Clause)

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/fsl,dai.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic CPU FSL DAI driver for resource management

Bindings are for h/w devices, not drivers.

> +
> +maintainers:
> +  - Daniel Baluta <daniel.baluta@xxxxxxx>
> +
> +description: |
> +  On platforms with a DSP we need to split the resource handling between
> +  Application Processor (AP) and DSP. On platforms where the DSP doesn't
> +  have an easy access to resources, the AP will take care of
> +  configuring them. Resources handled by this generic driver are: clocks,
> +  power domains, pinctrl.

The DT should define a DSP node with resources that are part of the 
DSP. What setup the AP has to do should be implied by the compatible 
string and possibly what resources are described.

Or maybe the audio portion of the DSP is a child node...

> +
> +properties:
> +  '#sound-dai-cells':
> +    const: 0
> +
> +  compatible:
> +    enum:
> +      - fsl,esai-dai
> +      - fsl,sai-dai

Not very specific. There's only 2 versions of the DSP and ways it is 
integrated?

> +
> +  clocks:
> +    oneOf:
> +      - items: # for ESAI
> +          - description: Core clock used to access registers.
> +          - description: ESAI baud clock for ESAI controller used to derive
> +                         HCK, SCK and FS.
> +          - description: The system clock derived from ahb clock used to derive
> +                         HCK, SCK and FS.
> +          - description: SPBA clock is required when ESAI is placed as a bus
> +                         slave of the SP Bus and when two or more bus masters
> +                         (CPU, DMA or DSP) try to access it. This property is
> +                         optional depending on SoC design.
> +      - items: # for SAI
> +          - description: Bus clock for accessing registers
> +          - description: Master clock 0 for providing bit clock and frame clock
> +          - description: Master clock 1 for providing bit clock and frame clock
> +          - description: Master clock 2 for providing bit clock and frame clock
> +          - description: Master clock 3 for providing bit clock and frame clock
> +
> +  clock-names:
> +    oneOf:
> +      - items: # for ESAI
> +          - const: core
> +          - const: extal
> +          - const: fsys
> +          - const: spba
> +      - items: # for SAI
> +          - const: bus
> +          - const: mclk0
> +          - const: mclk1
> +          - const: mclk2
> +          - const: mclk3
> +
> +  pinctrl-0:
> +    description: Should specify pin control groups used for this controller.
> +
> +  pinctrl-names:
> +    const: default

pinctrl properties are implicitly allowed an don't have to be listed 
here.

> +
> +  power-domains:
> +    $ref: '/schemas/types.yaml#/definitions/phandle-array'

Don't need a type.

> +    description:
> +      List of phandles and PM domain specifiers, as defined by bindings of the
> +      PM domain provider.

Don't need to re-define common properties.

You do need to say how many power domains (maxItems: 1?).

> +
> +  fsl,dai-index:
> +    $ref: '/schemas/types.yaml#/definitions/uint32'
> +    description: Physical DAI index, must match the index from topology file

Sorry, we don't do indexes in DT.

What's a topology file?

> +
> +required:
> +  - compatible
> +  - clocks
> +  - clock-names
> +  - fsl,dai-index
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/imx8-clock.h>
> +    esai0_port: esai-port {
> +         #sound-dai-cells = <0>;
> +        compatible = "fsl,esai-dai";
> +
> +        fsl,dai-index = <0>;
> +        pinctrl-names = "default";
> +        pinctrl-0 = <&pinctrl_esai0>;
> +
> +        clocks = <&esai0_lpcg 1>, <&esai0_lpcg 0>,  <&esai0_lpcg 1>,
> +            <&clk_dummy>;
> +        clock-names = "core", "extal", "fsys", "spba";
> +    };
> -- 
> 2.17.1
>