linux-kernel - Re: [PATCH 3/3] dt-bindings: riscv: sifive: Add SiFive Private L2 cache controller

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:   Mon, 26 Jun 2023 11:06:36 +0800
From:   Eric Lin <eric.lin@...ive.com>
To:     Conor Dooley <conor.dooley@...rochip.com>
Cc:     conor@...nel.org, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, palmer@...belt.com,
        paul.walmsley@...ive.com, aou@...s.berkeley.edu, maz@...nel.org,
        chenhuacai@...nel.org, baolu.lu@...ux.intel.com, will@...nel.org,
        kan.liang@...ux.intel.com, nnac123@...ux.ibm.com,
        pierre.gondois@....com, jgross@...e.com, chao.gao@...el.com,
        maobibo@...ngson.cn, linux-riscv@...ts.infradead.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        dslin1010@...il.com, Zong Li <zong.li@...ive.com>,
        Nick Hu <nick.hu@...ive.com>,
        Greentime Hu <greentime.hu@...ive.com>
Subject: Re: [PATCH 3/3] dt-bindings: riscv: sifive: Add SiFive Private L2
 cache controller

Hi Conor,

On Fri, Jun 16, 2023 at 6:12 PM Conor Dooley <conor.dooley@...rochip.com> wrote:
>
> Hey Eric,
>
> On Fri, Jun 16, 2023 at 02:32:10PM +0800, Eric Lin wrote:
> > This add YAML DT binding documentation for SiFive Private L2
> > cache controller
> >
> > Signed-off-by: Eric Lin <eric.lin@...ive.com>
> > Reviewed-by: Zong Li <zong.li@...ive.com>
> > Reviewed-by: Nick Hu <nick.hu@...ive.com>
>
> Firstly, bindings need to come before the driver using them.
>

OK, I'll fix it in v2.

> > ---
> >  .../bindings/riscv/sifive,pL2Cache0.yaml      | 81 +++++++++++++++++++
> >  1 file changed, 81 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/riscv/sifive,pL2Cache0.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/riscv/sifive,pL2Cache0.yaml b/Documentation/devicetree/bindings/riscv/sifive,pL2Cache0.yaml
> > new file mode 100644
> > index 000000000000..b5d8d4a39dde
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/riscv/sifive,pL2Cache0.yaml
>
> Cache bindings have moved to devicetree/bindings/cache.
>
> > @@ -0,0 +1,81 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright (C) 2023 SiFive, Inc.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/riscv/sifive,pL2Cache0.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: SiFive Private L2 Cache Controller
> > +
> > +maintainers:
> > +  - Greentime Hu  <greentime.hu@...ive.com>
> > +  - Eric Lin      <eric.lin@...ive.com>
>
> Drop the alignment here please.
>

OK, I'll fix it in v2.

> > +
> > +description:
> > +  The SiFive Private L2 Cache Controller is per hart and communicates with both the upstream
> > +  L1 caches and downstream L3 cache or memory, enabling a high-performance cache subsystem.
> > +  All the properties in ePAPR/DeviceTree specification applies for this platform.
>
> Please wrap before 80 characters.
>

OK, I'll fix it in v2.

> > +
> > +allOf:
> > +  - $ref: /schemas/cache-controller.yaml#
> > +
> > +select:
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        enum:
> > +          - sifive,pL2Cache0
> > +          - sifive,pL2Cache1
>
> Why is this select: required?
>
OK, I'll fix it in v2.

> > +  required:
> > +    - compatible
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - sifive,pL2Cache0
> > +          - sifive,pL2Cache1
>
> What is the difference between these? (and drop the caps please)

The pL2Cache1 has minor changes in hardware, but it can use the same
pl2 cache driver.
OK, I'll drop the caps in v2.

>
> Should this also not fall back to "cache"?
>
Yes,  I'll fix it in v2.

> > +
> > +  cache-block-size:
> > +    const: 64
> > +
> > +  cache-level:
> > +    const: 2
> > +
> > +  cache-sets:
> > +    const: 512
> > +
> > +  cache-size:
> > +    const: 262144
> > +
> > +  cache-unified: true
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  next-level-cache: true
> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - cache-block-size
> > +  - cache-level
> > +  - cache-sets
> > +  - cache-size
> > +  - cache-unified
> > +  - reg
> > +
> > +examples:
> > +  - |
> > +    pl2@...04000 {
>
> cache-controller@
>

OK, I'll fix it in v2.
Thanks for the review.

Best Regards,
Eric Lin.

> Cheers,
> Conor.
>
> > +        compatible = "sifive,pL2Cache0";
> > +        cache-block-size = <64>;
> > +        cache-level = <2>;
> > +        cache-sets = <512>;
> > +        cache-size = <262144>;
> > +        cache-unified;
> > +        reg = <0x10104000 0x4000>;
> > +        next-level-cache = <&L4>;
> > +    };
> > --
> > 2.40.1
> >