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]

Message-ID: <5f00ab85-d5ac-728d-2157-e70f2a46cc90@canonical.com>
Date:   Thu, 25 Aug 2022 20:36:49 +0200
From:   Heinrich Schuchardt <heinrich.schuchardt@...onical.com>
To:     Conor Dooley <conor.dooley@...rochip.com>
Cc:     Sagar Kadam <sagar.kadam@...ive.com>,
        Atish Patra <atishp@...shpatra.org>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        devicetree@...r.kernel.org, linux-riscv@...ts.infradead.org,
        linux-kernel@...r.kernel.org, Albert Ou <aou@...s.berkeley.edu>,
        Daire McNamara <daire.mcnamara@...rochip.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Rob Herring <robh+dt@...nel.org>,
        Conor Dooley <mail@...chuod.ie>
Subject: Re: [PATCH 1/2] dt-bindings: riscv: sifive-l2: add a PolarFire SoC
 compatible

On 8/25/22 20:04, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@...rochip.com>
> 
> The l2 cache on PolarFire SoC is cross between that of the fu540 and
> the fu740. It has the extra interrupt from the fu740 but the lower
> number of cache-sets. Add a specific compatible to avoid the likes
> of:
> 
> mpfs-polarberry.dtb: cache-controller@...0000: interrupts: [[1], [3], [4], [2]] is too long

Where is such a message written? I couldn't find the string in 
next-20220825 (git grep -n 'is too long"').

Why should a different number of cache sets require an extra compatible 
string. cache-size is simply a parameter going with the existing 
compatible strings.

I would assume that you only need an extra compatible string if there is 
a functional difference that can not be expressed with the existing 
parameters.

> 
> Fixes: 34fc9cc3aebe ("riscv: dts: microchip: correct L2 cache interrupts")
> Signed-off-by: Conor Dooley <conor.dooley@...rochip.com>
> ---
>   .../bindings/riscv/sifive-l2-cache.yaml       | 79 ++++++++++++-------
>   1 file changed, 49 insertions(+), 30 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml b/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
> index 69cdab18d629..ca3b9be58058 100644
> --- a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
> +++ b/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
> @@ -17,9 +17,6 @@ description:
>     acts as directory-based coherency manager.
>     All the properties in ePAPR/DeviceTree specification applies for this platform.
>   
> -allOf:
> -  - $ref: /schemas/cache-controller.yaml#
> -
>   select:
>     properties:
>       compatible:
> @@ -33,11 +30,16 @@ select:
>   
>   properties:
>     compatible:
> -    items:
> -      - enum:
> -          - sifive,fu540-c000-ccache
> -          - sifive,fu740-c000-ccache

Why can't you simply add microchip,mpfs-ccache here?

> -      - const: cache
> +    oneOf:
> +      - items:
> +          - enum:
> +              - sifive,fu540-c000-ccache
> +              - sifive,fu740-c000-ccache
> +          - const: cache
> +      - items:
> +          - const: microchip,mpfs-ccache
> +          - const: sifive,fu540-c000-ccache

Why do we need 'sifive,fu540-c000-ccache' twice?

Best regards

Heinrich

> +          - const: cache
>   
>     cache-block-size:
>       const: 64
> @@ -72,29 +74,46 @@ properties:
>         The reference to the reserved-memory for the L2 Loosely Integrated Memory region.
>         The reserved memory node should be defined as per the bindings in reserved-memory.txt.
>   
> -if:
> -  properties:
> -    compatible:
> -      contains:
> -        const: sifive,fu540-c000-ccache
> +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
> +
> +    else:
> +      properties:
> +        interrupts:
> +          description: |
> +            Must contain entries for DirError, DataError and DataFail signals.
> +          maxItems: 3
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: sifive,fu740-c000-ccache
> +
> +    then:
> +      properties:
> +        cache-sets:
> +          const: 2048
> +
> +    else:
> +      properties:
> +        cache-sets:
> +          const: 1024
>   
>   additionalProperties: false
>