Received-SPF: pass (google.com: domain of linux-kernel+bounces-86372-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <37181fc1-a485-427c-b3d2-2fcfb192e6e9@collabora.com>
Date: Thu, 29 Feb 2024 10:10:51 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 18/22] ASoC: dt-bindings: mt8195: Document audio-routing
 and dai-link subnode
Content-Language: en-US
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>, broonie@kernel.org
Cc: wenst@chromium.org, lgirdwood@gmail.com, robh@kernel.org,
 krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
 matthias.bgg@gmail.com, perex@perex.cz, tiwai@suse.com,
 trevor.wu@mediatek.com, maso.huang@mediatek.com,
 xiazhengqiao@huaqin.corp-partner.google.com, arnd@arndb.de,
 kuninori.morimoto.gx@renesas.com, shraash@google.com, amergnat@baylibre.com,
 nicolas.ferre@microchip.com, u.kleine-koenig@pengutronix.de,
 dianders@chromium.org, frank.li@vivo.com, allen-kh.cheng@mediatek.com,
 eugen.hristev@collabora.com, claudiu.beznea@tuxon.dev,
 jarkko.nikula@bitmer.com, jiaxin.yu@mediatek.com, alpernebiyasak@gmail.com,
 ckeepax@opensource.cirrus.com, zhourui@huaqin.corp-partner.google.com,
 nfraprado@collabora.com, alsa-devel@alsa-project.org,
 shane.chien@mediatek.com, linux-sound@vger.kernel.org,
 devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
 linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org,
 kernel@collabora.com
References: <20240227120939.290143-1-angelogioacchino.delregno@collabora.com>
 <20240227120939.290143-19-angelogioacchino.delregno@collabora.com>
 <32ff2f66-7a94-41ed-b77b-f78da2e57446@linaro.org>
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
In-Reply-To: <32ff2f66-7a94-41ed-b77b-f78da2e57446@linaro.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

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@collabora.com>
>> ---
> 
> 
>>   
>>     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
>