linux-kernel - Re: [PATCH 6/9] ASoC: dt-bindings: meson: convert axg fifo to schema

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 3 Feb 2023 18:58:12 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Jerome Brunet <jbrunet@...libre.com>,
        Mark Brown <broonie@...nel.org>, alsa-devel@...a-project.org,
        devicetree@...r.kernel.org
Cc:     linux-amlogic@...ts.infradead.org,
        Kevin Hilman <khilman@...libre.com>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 6/9] ASoC: dt-bindings: meson: convert axg fifo to schema

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