Re: [PATCH 9/9] ASoC: dt-bindings: meson: convert axg sound card control to schema

From: Jerome Brunet
Date: Fri Feb 03 2023 - 09:28:45 EST


On Fri 03 Feb 2023 at 09:09, Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote:

> On 02/02/2023 19:36, Jerome Brunet wrote:
>> Convert the DT binding documentation for the Amlogic axg sound card to
>> schema.
>>
>> Signed-off-by: Jerome Brunet <jbrunet@xxxxxxxxxxxx>
>> ---
>> .../bindings/sound/amlogic,axg-sound-card.txt | 124 ------------
>> .../sound/amlogic,axg-sound-card.yaml | 182 ++++++++++++++++++
>> 2 files changed, 182 insertions(+), 124 deletions(-)
>> delete mode 100644 Documentation/devicetree/bindings/sound/amlogic,axg-sound-card.txt
>> create mode 100644 Documentation/devicetree/bindings/sound/amlogic,axg-sound-card.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/sound/amlogic,axg-sound-card.txt b/Documentation/devicetree/bindings/sound/amlogic,axg-sound-card.txt
>> deleted file mode 100644
>> index 80b411296480..000000000000
>> --- a/Documentation/devicetree/bindings/sound/amlogic,axg-sound-card.txt
>> +++ /dev/null
>> @@ -1,124 +0,0 @@
>> -Amlogic AXG sound card:
>> -
>> -Required properties:
>> -
>> -- compatible: "amlogic,axg-sound-card"
>> -- model : User specified audio sound card name, one string
>> -
>> -Optional properties:
>> -
>> -- audio-aux-devs : List of phandles pointing to auxiliary devices
>> -- audio-widgets : Please refer to widgets.txt.
>> -- audio-routing : A list of the connections between audio components.
>> -
>> -Subnodes:
>> -
>> -- dai-link: Container for dai-link level properties and the CODEC
>> - sub-nodes. There should be at least one (and probably more)
>> - subnode of this type.
>> -
>> -Required dai-link properties:
>> -
>> -- sound-dai: phandle and port of the CPU DAI.
>> -
>> -Required TDM Backend dai-link properties:
>> -- dai-format : CPU/CODEC common audio format
>> -
>> -Optional TDM Backend dai-link properties:
>> -- dai-tdm-slot-rx-mask-{0,1,2,3}: Receive direction slot masks
>> -- dai-tdm-slot-tx-mask-{0,1,2,3}: Transmit direction slot masks
>> - When omitted, mask is assumed to have to no
>> - slots. A valid must have at one slot, so at
>> - least one these mask should be provided with
>> - an enabled slot.
>> -- dai-tdm-slot-num : Please refer to tdm-slot.txt.
>> - If omitted, slot number is set to accommodate the largest
>> - mask provided.
>> -- dai-tdm-slot-width : Please refer to tdm-slot.txt. default to 32 if omitted.
>> -- mclk-fs : Multiplication factor between stream rate and mclk
>> -
>> -Backend dai-link subnodes:
>> -
>> -- codec: dai-link representing backend links should have at least one subnode.
>> - One subnode for each codec of the dai-link.
>> - dai-link representing frontend links have no codec, therefore have no
>> - subnodes
>> -
>> -Required codec subnodes properties:
>> -
>> -- sound-dai: phandle and port of the CODEC DAI.
>> -
>> -Optional codec subnodes properties:
>> -
>> -- dai-tdm-slot-tx-mask : Please refer to tdm-slot.txt.
>> -- dai-tdm-slot-rx-mask : Please refer to tdm-slot.txt.
>> -
>> -Example:
>> -
>> -sound {
>> - compatible = "amlogic,axg-sound-card";
>> - model = "AXG-S420";
>> - audio-aux-devs = <&tdmin_a>, <&tdmout_c>;
>> - audio-widgets = "Line", "Lineout",
>> - "Line", "Linein",
>> - "Speaker", "Speaker1 Left",
>> - "Speaker", "Speaker1 Right";
>> - "Speaker", "Speaker2 Left",
>> - "Speaker", "Speaker2 Right";
>> - audio-routing = "TDMOUT_C IN 0", "FRDDR_A OUT 2",
>> - "SPDIFOUT IN 0", "FRDDR_A OUT 3",
>> - "TDM_C Playback", "TDMOUT_C OUT",
>> - "TDMIN_A IN 2", "TDM_C Capture",
>> - "TDMIN_A IN 5", "TDM_C Loopback",
>> - "TODDR_A IN 0", "TDMIN_A OUT",
>> - "Lineout", "Lineout AOUTL",
>> - "Lineout", "Lineout AOUTR",
>> - "Speaker1 Left", "SPK1 OUT_A",
>> - "Speaker2 Left", "SPK2 OUT_A",
>> - "Speaker1 Right", "SPK1 OUT_B",
>> - "Speaker2 Right", "SPK2 OUT_B",
>> - "Linein AINL", "Linein",
>> - "Linein AINR", "Linein";
>> -
>> - dai-link@0 {
>> - sound-dai = <&frddr_a>;
>> - };
>> -
>> - dai-link@1 {
>> - sound-dai = <&toddr_a>;
>> - };
>> -
>> - dai-link@2 {
>> - sound-dai = <&tdmif_c>;
>> - dai-format = "i2s";
>> - dai-tdm-slot-tx-mask-2 = <1 1>;
>> - dai-tdm-slot-tx-mask-3 = <1 1>;
>> - dai-tdm-slot-rx-mask-1 = <1 1>;
>> - mclk-fs = <256>;
>> -
>> - codec@0 {
>> - sound-dai = <&lineout>;
>> - };
>> -
>> - codec@1 {
>> - sound-dai = <&speaker_amp1>;
>> - };
>> -
>> - codec@2 {
>> - sound-dai = <&speaker_amp2>;
>> - };
>> -
>> - codec@3 {
>> - sound-dai = <&linein>;
>> - };
>> -
>> - };
>> -
>> - dai-link@3 {
>> - sound-dai = <&spdifout>;
>> -
>> - codec {
>> - sound-dai = <&spdif_dit>;
>> - };
>> - };
>> -};
>> diff --git a/Documentation/devicetree/bindings/sound/amlogic,axg-sound-card.yaml b/Documentation/devicetree/bindings/sound/amlogic,axg-sound-card.yaml
>> new file mode 100644
>> index 000000000000..185cd9fbeda1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/sound/amlogic,axg-sound-card.yaml
>> @@ -0,0 +1,182 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/sound/amlogic,axg-sound-card.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Amlogic AXG sound card
>> +
>> +maintainers:
>> + - Jerome Brunet <jbrunet@xxxxxxxxxxxx>
>> +
>> +properties:
>> + compatible:
>> + items:
>
> Drop
>
>> + - const: amlogic,axg-sound-card
>
> And here you can keep such code in one line?
>
>> +
>> + audio-aux-devs:
>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>> + description: list of auxiliary devices
>
> min/maxItems

Auxiliary devices are not mandatory and there is no limit on how many a
sound card may have. Any number of item is valid.

>
>> +
>> + audio-routing:
>> + $ref: /schemas/types.yaml#/definitions/non-unique-string-array
>> + description: |-
>> + A list of the connections between audio components. Each entry is a
>> + pair of strings, the first being the connection's sink, the second
>> + being the connection's source.
>> +
>> + audio-widgets:
>> + $ref: /schemas/types.yaml#/definitions/non-unique-string-array
>> + description: |-
>> + A list off component DAPM widget. Each entry is a pair of strings,
>> + the first being the widget type, the second being the widget name
>> +
>> + model:
>> + $ref: /schemas/types.yaml#/definitions/string
>> + description: User specified audio sound card name
>> +
>> +patternProperties:
>> + "^dai-link-[0-9]+$":
>> + type: object
>> + additionalProperties: false
>> + description: |-
>
> Drop |-
>
>> + dai-link child nodes:
>
> Drop, this is not a property list.
>
>> + Container for dai-link level properties and the CODEC sub-nodes.
>> + There should be at least one (and probably more) subnode of this type
>> +
>> + properties:
>> + dai-format:
>> + $ref: /schemas/types.yaml#/definitions/string
>> + enum: [ i2s, left-j, dsp_a ]
>> +
>> + dai-tdm-slot-num:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: |
>> + Number of slots in use. If omitted, slot number is set to
>> + accommodate the largest mask provided.
>> + maximum: 32
>> +
>> + dai-tdm-slot-width:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: Width in bits for each slot
>> + enum: [ 8, 16, 20, 24, 32 ]
>> + default: 32
>> +
>> + mclk-fs:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: |-
>
> Drop |-
> Drop it everywhere where not needed
>
>> + Multiplication factor between the frame rate and master clock
>> + rate
>> +
>> + sound-dai:
>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>
> Old binding was saying it is just phandle

It is just a phandle but the bindings used to have problem with phandle
that had cells.

See:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/Documentation/devicetree/bindings/sound/amlogic,gx-sound-card.yaml?h=v6.2-rc6&id=d031d99b02eaf7363c33f5b27b38086cc8104082

Was it wrong or did the situation change since then ?

>
>> + description: phandle of the CPU DAI
>> +
>> + patternProperties:
>> + "^dai-tdm-slot-(t|r)x-mask-[0-3]$":
>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>> + description: |-
>
> Drop |-
>
>> + Transmit and receive cpu slot masks of each TDM lane
>> + When omitted, mask is assumed to have to no slots. A valid
>> + interface must have at least one slot, so at least one these
>> + mask should be provided with an enabled slot.
>> +
>> + "^codec(-[0-9]+)?$":
>> + type: object
>> + additionalProperties: false
>> + description: |-
>> + dai-link representing backend links should have at least one subnode.
>> + One subnode for each codec of the dai-link. dai-link representing
>> + frontend links have no codec, therefore have no subnodes
>> +
>> + properties:
>> + sound-dai:
>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>
> phandle
>
>> + description: phandle of the codec DAI
>> +
>> + patternProperties:
>> + "^dai-tdm-slot-(t|r)x-mask$":
>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>> + description: Transmit and receive codec slot masks
>
> Are you sure codec has dai-tdm-slot-....?

Yes. Both CPU and and codecs have slots.

A classic case is a CPU with 2 codecs attached
Codec #0 plays slots 0,1
Codec #1 plays slots 2,3

CPU tx slots is <1, 1, 1, 1>
Codec #0 rx slots is <1, 1, 0, 0>
Codec #1 rx slots is <0, 0, 1, 1>

>
>> +
>> + required:
>> + - sound-dai
>> +
>> + required:
>> + - sound-dai
>> +
>> +required:
>> + - model
>> + - dai-link-0
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> + - |
>> + sound {
>> + compatible = "amlogic,axg-sound-card";
>> + model = "AXG-S420";
>> + audio-aux-devs = <&tdmin_a>, <&tdmout_c>;
>> + audio-widgets = "Line", "Lineout",
>> + "Line", "Linein",
>> + "Speaker", "Speaker1 Left",
>> + "Speaker", "Speaker1 Right",
>> + "Speaker", "Speaker2 Left",
>> + "Speaker", "Speaker2 Right";
>> + audio-routing = "TDMOUT_C IN 0", "FRDDR_A OUT 2",
>> + "SPDIFOUT IN 0", "FRDDR_A OUT 3",
>> + "TDM_C Playback", "TDMOUT_C OUT",
>> + "TDMIN_A IN 2", "TDM_C Capture",
>> + "TDMIN_A IN 5", "TDM_C Loopback",
>> + "TODDR_A IN 0", "TDMIN_A OUT",
>> + "Lineout", "Lineout AOUTL",
>> + "Lineout", "Lineout AOUTR",
>> + "Speaker1 Left", "SPK1 OUT_A",
>> + "Speaker2 Left", "SPK2 OUT_A",
>> + "Speaker1 Right", "SPK1 OUT_B",
>> + "Speaker2 Right", "SPK2 OUT_B",
>> + "Linein AINL", "Linein",
>> + "Linein AINR", "Linein";
>> +
>> + dai-link-0 {
>> + sound-dai = <&frddr_a>;
>
> Use 4 spaces for example indentation.
>
>> + };
>> +
>> + dai-link-1 {
>> + sound-dai = <&toddr_a>;
>> + };
>
> Best regards,
> Krzysztof