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

[Eric Lin]

Hi Krzysztof,

On Mon, Jun 26, 2023 at 2:19 PM Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> wrote:
>
> On 26/06/2023 05:26, Eric Lin wrote:
> > Hi Krzysztof,
> >
> > On Fri, Jun 16, 2023 at 6:45 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@xxxxxxxxxx> wrote:
> >>
> >> On 16/06/2023 08:32, Eric Lin wrote:
> >>> This add YAML DT binding documentation for SiFive Private L2
> >>> cache controller
> >>>
> >>> Signed-off-by: Eric Lin <eric.lin@xxxxxxxxxx>
> >>> Reviewed-by: Zong Li <zong.li@xxxxxxxxxx>
> >>> Reviewed-by: Nick Hu <nick.hu@xxxxxxxxxx>
> >>> ---
> >>>  .../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
> >>> @@ -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@xxxxxxxxxx>
> >>> +  - Eric Lin      <eric.lin@xxxxxxxxxx>
> >>> +
> >>> +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.
> >>
> >> Drop the last sentence. Why specification would not apply?
> >>
> > OK, I'll drop it in v2.
> >
> >>> +
> >>> +allOf:
> >>> +  - $ref: /schemas/cache-controller.yaml#
> >>> +
> >>> +select:
> >>> +  properties:
> >>> +    compatible:
> >>> +      contains:
> >>> +        enum:
> >>> +          - sifive,pL2Cache0
> >>> +          - sifive,pL2Cache
> >>> +
> >>> +  required:
> >>> +    - compatible
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    items:
> >>
> >>
> >> You have only one item, so no need for items... unless you just missed
> >> proper fallback.
> >
> > OK, I'll fix it in v2.
> >
> >>
> >>> +      - enum:
> >>> +          - sifive,pL2Cache0
> >>> +          - sifive,pL2Cache1
> >>
> >> What is "0" and "1" here? What do these compatibles represent? Why they
> >> do not have any SoC related part?
> >
> > The pL2Cache1 has minor changes in hardware, but it can use the same
> > pl2 cache driver.
>
> Then why aren't they compatible?
>

The pL2Cache1 has removed some unused bits in the register compared to
pl2Cache0.
>From the hardware perspective, they are not compatible but they can
share the same pl2 cache driver in software.
Thus, we would like to keep both. It would be great if you can provide
some suggestions. Thanks.

Best Regards,
Eric Lin.

> > May I ask, what do you mean about the SoC-related part? Thanks.
>
> This is part of a SoC, right? We expect SoC blocks to have compatible
> based on the SoC.
>
>
>
> Best regards,
> Krzysztof
>