Re: [PATCH v3 2/2] ASoC: dt-bindings: renesas,rsnd.yaml: add R-Car Gen4 support

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 14/02/2023 18:52, Mark Brown wrote:
> On Mon, Feb 13, 2023 at 09:58:15AM +0100, Krzysztof Kozlowski wrote:
>> On 13/02/2023 03:13, Kuninori Morimoto wrote:
> 
>>>    clock-names:
>>>      description: List of necessary clock names.
>>> -    minItems: 1
>>> -    maxItems: 31
> 
>> Not much of an improvement here. We asked to keep the constraints here.
>> I gave you the reference how it should look like. Why did you decide to
>> do it differently than what I linked?
> 
> Krzysztof, please take more time to explain what issues you're
> seeing, what you want people to do and why.  I'm having a hard
> time identifying what the issue is here - AFAICT when you talk
> about the example you linked you're referring to:
> 
>   https://elixir.bootlin.com/linux/v5.19-rc6/source/Documentation/devicetree/bindings/clock/samsung,exynos7-clock.yaml#L57
> 
> which as far as I can tell looks like what Morimoto-san is trying
> to accomplish here.  I can't tell what the issue you're raising
> is, let alone if it's something important or just a stylistic
> nit.

If you leave the top-level definition without any constraints and you
forget to include all your compatibles in allOf:if:then, then you end up
without constraints. Consider:

-----
properties:
  compatible:
    enum:
      - foo
      - bar

clock-names:
  description: anything

if:
  prop:
    compat:
      enum:
        - foo
then:
  prop:
    clock-names:
      - ahb
      - sclk
-----

What clocks are valid for bar?

For simple cases this might be obvious, for more complex this is easy to
miss. So the recommended syntax is with constraints at the top.

BTW, if there is reason not to use it - sure, bring the reason, we talk
and maybe skip it, I don't mind. But there was no reason - just part was
skipped/missing.


Best regards,
Krzysztof