Re: [PATCH 18/22] ASoC: dt-bindings: mt8195: Document audio-routing and dai-link subnode

[AngeloGioacchino Del Regno]

Il 29/02/24 09:25, Krzysztof Kozlowski ha scritto:
> On 27/02/2024 13:09, AngeloGioacchino Del Regno wrote:
> > Document the dai-link subnodes and the audio-routing property, allowing
> > to describe machine specific audio hardware and links in device tree.
> >
> > While at it, also deprecate the old properties which were previously
> > used with driver hardcoded configuration.
> >
> > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>
> > ---
>
>
> >   
> >     mediatek,adsp:
> >       $ref: /schemas/types.yaml#/definitions/phandle
> > @@ -45,12 +56,75 @@ properties:
> >         A list of the desired dai-links in the sound card. Each entry is a
> >         name defined in the machine driver.
> >   
> > +patternProperties:
> > +  ".*-dai-link$":
> > +    type: object
> > +    description:
> > +      Container for dai-link level properties and CODEC sub-nodes.
> > +
> > +    properties:
> > +      link-name:
> > +        description: Indicates dai-link name and PCM stream name
> > +        items:
>
> That's not a list, but just enum.

Ok! Will change for v2.

> > +          enum:
> > +            - DPTX_BE
> > +            - ETDM1_IN_BE
> > +            - ETDM2_IN_BE
> > +            - ETDM1_OUT_BE
> > +            - ETDM2_OUT_BE
> > +            - ETDM3_OUT_BE
> > +            - PCM1_BE
> > +
> > +      codec:
> > +        description: Holds subnode which indicates codec dai.
> > +        type: object
> > +        additionalProperties: false
> > +        properties:
> > +          sound-dai:
> > +            minItems: 1
> > +            maxItems: 2
> > +        required:
> > +          - sound-dai
> > +
> > +      dai-format:
> > +        description: audio format
> > +        items:
>
> Ditto
>
> > +          enum:
> > +            - i2s
> > +            - right_j
> > +            - left_j
> > +            - dsp_a
> > +            - dsp_b
> > +
> > +      mediatek,clk-provider:
> > +        $ref: /schemas/types.yaml#/definitions/string
> > +        description: Indicates dai-link clock master.
> > +        items:
>
> Ditto
>
> > +          enum:
> > +            - cpu
> > +            - codec
> > +
> > +    additionalProperties: false
>
> This goes either to the top of the section (after type:object) for
> readability or after required: block below.

I think I actually saw this in some other binding, that's why it's there... or
maybe I am confusing something else - whatever.

I'll move it, anyway - thanks

> > +
> > +    required:
> > +      - link-name
> > +
> >   additionalProperties: false
>
> >   
> >   required:
> >     - compatible
> >     - mediatek,platform
> >   
> > +# Disallow legacy properties if dai-link-xxx nodes are specified
> > +if:
> > +  not:
>
> I don't think this works. To test if node is present or node, you would
> need to use required.
> https://elixir.bootlin.com/linux/v6.4-rc7/source/Documentation/devicetree/bindings/net/qcom,ipa.yaml#L174
>
> Are you sure this if:then: works as expected?

Honestly, coming up with something that worked took me almost a full hour.
I was going a bit crazy over this one :-)

btw - Yes, it does work.

If you wanna give it a shot too, patch 21 and 22 are introducing the audio-routing
and (x)-dai-link nodes in (respectively) mt8195-cherry.dtsi and mt8186-corsola.dtsi
devicetrees - and those do validate just fine both with and without those patches.

I also tested this by adding the forbidden dptx-codec/hdmi-codec properties along
with the dai-link nodes in the example of this yaml file, and I've verified that
adding those makes the validation fail as expected.

Exactly, this is what I want and also what I've tested with dtbs/dt_binding_check:

fmt: (test nodes/properties) <- (result)

x-dai-link { ... }; mediatek,dptx-codec = <...>; <- FAIL (OK)
x-dai-link { ... }; mediatek,hdmi-codec = <...>; <- FAIL (OK)
x-dai-link { ... }; mediatek,dptx-codec = ..; mediatek,hdmi-codec .. <- FAIL (OK)

x-dai-link { ... }; (none of those two) <- PASS (OK)

(NO x-dai-link) mediatek,dptx-codec = <...> <- PASS (OK)
(NO x-dai-link) mediatek,hdmi-codec = <...> <- PASS (OK)
(NO x-dai-link) mediatek,dptx-codec = <...> mediatek,hdmi-codec = <...> <- PASS(OK)

..and of course, also tested the easy part
dai-link-x { ... }; <- FAIL (OK)    :-)

Cheers,
Angelo

> > +    patternProperties:
> > +      ".*-dai-link$": false
> > +then:
> > +  properties:
> > +    mediatek,dptx-codec: false
> > +    mediatek,hdmi-codec: false
> > +
> >   examples:
> >     - |
> >   
>
>
> Best regards,
> Krzysztof