linux-kernel - Re: [PATCH 1/2] dt-bindings: riscv: sifive-l2: add a PolarFire SoC compatible

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:   Thu, 25 Aug 2022 20:03:54 +0000
From:   <Conor.Dooley@...rochip.com>
To:     <heinrich.schuchardt@...onical.com>, <Conor.Dooley@...rochip.com>
CC:     <sagar.kadam@...ive.com>, <atishp@...shpatra.org>,
        <paul.walmsley@...ive.com>, <krzysztof.kozlowski+dt@...aro.org>,
        <devicetree@...r.kernel.org>, <linux-riscv@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>, <aou@...s.berkeley.edu>,
        <Daire.McNamara@...rochip.com>, <palmer@...belt.com>,
        <robh+dt@...nel.org>
Subject: Re: [PATCH 1/2] dt-bindings: riscv: sifive-l2: add a PolarFire SoC
 compatible

On 25/08/2022 20:49, Heinrich Schuchardt wrote:
> On 8/25/22 20:56, Conor.Dooley@...rochip.com wrote:
>> On 25/08/2022 19:36, Heinrich Schuchardt wrote:
>>> On 8/25/22 20:04, Conor Dooley wrote:
>>>> From: Conor Dooley <conor.dooley@...rochip.com>

>>>> +allOf:
>>>> +  - $ref: /schemas/cache-controller.yaml#
>>>>    -then:
>>>> -  properties:
>>>> -    interrupts:
>>>> -      description: |
>>>> -        Must contain entries for DirError, DataError and DataFail signals.
>>>> -      maxItems: 3
>>>> -    cache-sets:
>>>> -      const: 1024
>>>> -
>>>> -else:
>>>> -  properties:
>>>> -    interrupts:
>>>> -      description: |
>>>> -        Must contain entries for DirError, DataError, DataFail, DirFail signals.
>>>> -      minItems: 4
>>>> -    cache-sets:
>>>> -      const: 2048
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            enum:
>>>> +              - sifive,fu740-c000-ccache
>>>> +              - microchip,mpfs-ccache
>>>> +
>>>> +    then:
>>>> +      properties:
>>>> +        interrupts:
>>>> +          description: |
>>>> +            Must contain entries for DirError, DataError, DataFail, DirFail signals.
>>>> +          minItems: 4
> 
> Above you indicated that you want strict limits for the interrupt count.
> You expect exactly 4 items here. Having 5 entries would not be correct.
> Please, add 'maxItems: 4'.

Outside of this diff, because of how the particular binding was
structured, there is:
  interrupts:
    minItems: 3
    items:
      - description: DirError interrupt
      - description: DataError interrupt
      - description: DataFail interrupt
      - description: DirFail interrupt

AFAIU, "maxItems: 4" is redundant because all possible items are listed.

> 
>>>> +
>>>> +    else:
>>>> +      properties:
>>>> +        interrupts:
>>>> +          description: |
>>>> +            Must contain entries for DirError, DataError and DataFail signals.
>>>> +          maxItems: 3
> 
> The item count should be exactly 3. Having 2 entries would not be correct.
> Please, add 'minItems: 3'.

Again, this is set by the section I pasted above - although this time
explicitly.

Hope that explains things, not the easiest binding to understand from
a diff alone. Possibly I should have passed a "-U" argument while
creating the patches to get an easier-to-follow diff.

Thanks for your (prompt) reviews,
Conor.

>>>> +
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            const: sifive,fu740-c000-ccache
>>>> +
>>>> +    then:
>>>> +      properties:
>>>> +        cache-sets:
>>>> +          const: 2048
>>>> +
>>>> +    else:
>>>> +      properties:
>>>> +        cache-sets:
>>>> +          const: 1024
>>>>      additionalProperties: false
>>>>