Message-ID: <d72f2c93-934c-67fe-0afd-96bc52b12a9f@linaro.org>
Date:   Fri, 3 Feb 2023 18:58:12 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH 6/9] ASoC: dt-bindings: meson: convert axg fifo to schema
Content-Language: en-US
To:     Jerome Brunet <jbrunet@baylibre.com>,
        Mark Brown <broonie@kernel.org>, alsa-devel@alsa-project.org,
        devicetree@vger.kernel.org
Cc:     linux-amlogic@lists.infradead.org,
        Kevin Hilman <khilman@baylibre.com>,
        Neil Armstrong <neil.armstrong@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        linux-kernel@vger.kernel.org
References: <20230202183653.486216-1-jbrunet@baylibre.com>
 <20230202183653.486216-7-jbrunet@baylibre.com>
 <6e6ed493-4748-46ca-7a26-fe9cf6e2377a@linaro.org>
 <1jmt5un9u4.fsf@starbuckisacylon.baylibre.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <1jmt5un9u4.fsf@starbuckisacylon.baylibre.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 03/02/2023 14:27, Jerome Brunet wrote:
> 

>>> +  resets:
>>> +    items:
>>> +      - description: Memory ARB line
>>> +      - description: Dedicated device reset line
>>
>> This won't work without minItems and you should see errors on your DTS
>> or in dt_binding_check
>>
> 
> The example provided here worked but there is indeed a warning with the
> axg-frddr variant.
> 
> I'm adding a 2nd example so it does not happen again.

If the difference is only in one property, no need. If the difference is
in more properties - then could be. We do not keep examples for every
trivial change, because the assumption is that submitter tests DTS as well.

> 
>>> +
>>> +  reset-names: true
>>
>> minItems
>> maxItems
> 
> Adding this causes troubles with the reset-names definitions in the 'if'
> clause. If I put min: 1, max: 2 and min: 2 in the 'then' clause I get:
> 
>> Documentation/devicetree/bindings/sound/amlogic,axg-fifo.yaml: allOf:1:then:properties:reset-names: 'oneOf' conditional failed, one must be fixed:
>>        [{'const': 'arb'}, {'const': 'rst'}] is too long
>>        [{'const': 'arb'}, {'const': 'rst'}] is too short
>>        False schema does not allow 2
>>        1 was expected
>>        hint: "minItems" is only needed if less than the "items" list length
>>        from schema $id: http://devicetree.org/meta-schemas/items.yaml#

Probably because rest of binding does not match. One way is like this:

https://elixir.bootlin.com/linux/v5.19-rc6/source/Documentation/devicetree/bindings/clock/samsung,exynos7-clock.yaml#L57

> 
> The older devices just have the 'arb' reset.
> Newer devices have a 2nd reset line (called rst here)
> 
> If I just restrict the min and max, it would be valid for the older
> devices to have 'rst' only - but it is not valid.

How? Why would you define for old devices "rst" as one name if this is
not correct?

> 
> With just 'true', it works as expected (throw errors if an incorrect
> name or number of names is passed). Min and Max comes from the items list.

Because the rest is not in recommended way. Once you implement it in
recommended way, there will be no such...

> 
> Any suggestions ?

Implement rest of comments.


Best regards,
Krzysztof