2022-08-29 06:30:37

by Zong Li

[permalink] [raw]
Subject: [PATCH 1/3] dt-bindings: sifive-ccache: rename SiFive L2 cache to composible cache

Since composible cache may be L3 cache if private L2 cache exists, it
should use its original name composible cache to prevent confusion.

Signed-off-by: Greentime Hu <[email protected]>
Signed-off-by: Zong Li <[email protected]>
---
.../riscv/{sifive-l2-cache.yaml => sifive-ccache.yaml} | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
rename Documentation/devicetree/bindings/riscv/{sifive-l2-cache.yaml => sifive-ccache.yaml} (92%)

diff --git a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml b/Documentation/devicetree/bindings/riscv/sifive-ccache.yaml
similarity index 92%
rename from Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
rename to Documentation/devicetree/bindings/riscv/sifive-ccache.yaml
index 69cdab18d629..1a64a5384e36 100644
--- a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
+++ b/Documentation/devicetree/bindings/riscv/sifive-ccache.yaml
@@ -12,8 +12,8 @@ maintainers:
- Paul Walmsley <[email protected]>

description:
- The SiFive Level 2 Cache Controller is used to provide access to fast copies
- of memory for masters in a Core Complex. The Level 2 Cache Controller also
+ The SiFive Composable Cache Controller is used to provide access to fast copies
+ of memory for masters in a Core Complex. The Composable Cache Controller also
acts as directory-based coherency manager.
All the properties in ePAPR/DeviceTree specification applies for this platform.

@@ -27,6 +27,7 @@ select:
enum:
- sifive,fu540-c000-ccache
- sifive,fu740-c000-ccache
+ - sifive,ccache0

required:
- compatible
@@ -37,6 +38,7 @@ properties:
- enum:
- sifive,fu540-c000-ccache
- sifive,fu740-c000-ccache
+ - sifive,ccache0
- const: cache

cache-block-size:
--
2.17.1


2022-08-29 06:56:49

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: sifive-ccache: rename SiFive L2 cache to composible cache

Hey Zong,
Couple quick comments for you:

On 29/08/2022 07:22, Zong Li wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Since composible cache may be L3 cache if private L2 cache exists, it

s/composible/composable

> should use its original name composible cache to prevent confusion.

s/composible/composable

>
> Signed-off-by: Greentime Hu <[email protected]>
> Signed-off-by: Zong Li <[email protected]>
> ---
> .../riscv/{sifive-l2-cache.yaml => sifive-ccache.yaml} | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
> rename Documentation/devicetree/bindings/riscv/{sifive-l2-cache.yaml => sifive-ccache.yaml} (92%)
>
> diff --git a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml b/Documentation/devicetree/bindings/riscv/sifive-ccache.yaml
> similarity index 92%
> rename from Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
> rename to Documentation/devicetree/bindings/riscv/sifive-ccache.yaml
> index 69cdab18d629..1a64a5384e36 100644
> --- a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
> +++ b/Documentation/devicetree/bindings/riscv/sifive-ccache.yaml

Binding filenames are supposed to match the first compatible. Since
it was not named that way in the first place, I suppose naming after
the generic compatible makes the most sense. The correct filename
would then be "sifive,ccache0.yaml"

> @@ -12,8 +12,8 @@ maintainers:
> - Paul Walmsley <[email protected]>
>
> description:
> - The SiFive Level 2 Cache Controller is used to provide access to fast copies
> - of memory for masters in a Core Complex. The Level 2 Cache Controller also
> + The SiFive Composable Cache Controller is used to provide access to fast copies
> + of memory for masters in a Core Complex. The Composable Cache Controller also
> acts as directory-based coherency manager.
> All the properties in ePAPR/DeviceTree specification applies for this platform.
>
> @@ -27,6 +27,7 @@ select:
> enum:
> - sifive,fu540-c000-ccache
> - sifive,fu740-c000-ccache
> + - sifive,ccache0

Despite what the commit message says, this is more than a rename.
Thanks,
Conor.

>
> required:
> - compatible
> @@ -37,6 +38,7 @@ properties:
> - enum:
> - sifive,fu540-c000-ccache
> - sifive,fu740-c000-ccache
> + - sifive,ccache0
> - const: cache
>
> cache-block-size:
> --
> 2.17.1
>

2022-08-29 08:07:27

by Zong Li

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: sifive-ccache: rename SiFive L2 cache to composible cache

On Mon, Aug 29, 2022 at 2:45 PM <[email protected]> wrote:
>
> Hey Zong,
> Couple quick comments for you:
>
> On 29/08/2022 07:22, Zong Li wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Since composible cache may be L3 cache if private L2 cache exists, it
>
> s/composible/composable
>
> > should use its original name composible cache to prevent confusion.
>
> s/composible/composable
>

I will fix it in each patch.

> >
> > Signed-off-by: Greentime Hu <[email protected]>
> > Signed-off-by: Zong Li <[email protected]>
> > ---
> > .../riscv/{sifive-l2-cache.yaml => sifive-ccache.yaml} | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> > rename Documentation/devicetree/bindings/riscv/{sifive-l2-cache.yaml => sifive-ccache.yaml} (92%)
> >
> > diff --git a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml b/Documentation/devicetree/bindings/riscv/sifive-ccache.yaml
> > similarity index 92%
> > rename from Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
> > rename to Documentation/devicetree/bindings/riscv/sifive-ccache.yaml
> > index 69cdab18d629..1a64a5384e36 100644
> > --- a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
> > +++ b/Documentation/devicetree/bindings/riscv/sifive-ccache.yaml
>
> Binding filenames are supposed to match the first compatible. Since
> it was not named that way in the first place, I suppose naming after
> the generic compatible makes the most sense. The correct filename
> would then be "sifive,ccache0.yaml"
>

Thanks for pointing this out. Let me change the filename.

> > @@ -12,8 +12,8 @@ maintainers:
> > - Paul Walmsley <[email protected]>
> >
> > description:
> > - The SiFive Level 2 Cache Controller is used to provide access to fast copies
> > - of memory for masters in a Core Complex. The Level 2 Cache Controller also
> > + The SiFive Composable Cache Controller is used to provide access to fast copies
> > + of memory for masters in a Core Complex. The Composable Cache Controller also
> > acts as directory-based coherency manager.
> > All the properties in ePAPR/DeviceTree specification applies for this platform.
> >
> > @@ -27,6 +27,7 @@ select:
> > enum:
> > - sifive,fu540-c000-ccache
> > - sifive,fu740-c000-ccache
> > + - sifive,ccache0
>
> Despite what the commit message says, this is more than a rename.
> Thanks,
> Conor.
>
> >
> > required:
> > - compatible
> > @@ -37,6 +38,7 @@ properties:
> > - enum:
> > - sifive,fu540-c000-ccache
> > - sifive,fu740-c000-ccache
> > + - sifive,ccache0
> > - const: cache
> >
> > cache-block-size:
> > --
> > 2.17.1
> >
>

2022-08-29 18:54:39

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: sifive-ccache: rename SiFive L2 cache to composible cache

On Mon, 29 Aug 2022 06:22:00 +0000, Zong Li wrote:
> Since composible cache may be L3 cache if private L2 cache exists, it
> should use its original name composible cache to prevent confusion.
>
> Signed-off-by: Greentime Hu <[email protected]>
> Signed-off-by: Zong Li <[email protected]>
> ---
> .../riscv/{sifive-l2-cache.yaml => sifive-ccache.yaml} | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
> rename Documentation/devicetree/bindings/riscv/{sifive-l2-cache.yaml => sifive-ccache.yaml} (92%)
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/riscv/sifive-ccache.yaml: $id: relative path/filename doesn't match actual path or filename
expected: http://devicetree.org/schemas/riscv/sifive-ccache.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

2022-08-30 03:15:53

by Zong Li

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: sifive-ccache: rename SiFive L2 cache to composible cache

On Tue, Aug 30, 2022 at 2:42 AM Rob Herring <[email protected]> wrote:
>
> On Mon, 29 Aug 2022 06:22:00 +0000, Zong Li wrote:
> > Since composible cache may be L3 cache if private L2 cache exists, it
> > should use its original name composible cache to prevent confusion.
> >
> > Signed-off-by: Greentime Hu <[email protected]>
> > Signed-off-by: Zong Li <[email protected]>
> > ---
> > .../riscv/{sifive-l2-cache.yaml => sifive-ccache.yaml} | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> > rename Documentation/devicetree/bindings/riscv/{sifive-l2-cache.yaml => sifive-ccache.yaml} (92%)
> >
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> ./Documentation/devicetree/bindings/riscv/sifive-ccache.yaml: $id: relative path/filename doesn't match actual path or filename
> expected: http://devicetree.org/schemas/riscv/sifive-ccache.yaml#
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/patch/
>
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit.
>

Thanks for the tips, I would upgrade my environment and try it again.

2022-10-07 03:10:54

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: sifive-ccache: rename SiFive L2 cache to composible cache

On Sun, 28 Aug 2022 23:22:00 PDT (-0700), [email protected] wrote:
> Since composible cache may be L3 cache if private L2 cache exists, it
> should use its original name composible cache to prevent confusion.
>
> Signed-off-by: Greentime Hu <[email protected]>
> Signed-off-by: Zong Li <[email protected]>
> ---
> .../riscv/{sifive-l2-cache.yaml => sifive-ccache.yaml} | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
> rename Documentation/devicetree/bindings/riscv/{sifive-l2-cache.yaml => sifive-ccache.yaml} (92%)
>
> diff --git a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml b/Documentation/devicetree/bindings/riscv/sifive-ccache.yaml
> similarity index 92%
> rename from Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
> rename to Documentation/devicetree/bindings/riscv/sifive-ccache.yaml
> index 69cdab18d629..1a64a5384e36 100644
> --- a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
> +++ b/Documentation/devicetree/bindings/riscv/sifive-ccache.yaml
> @@ -12,8 +12,8 @@ maintainers:
> - Paul Walmsley <[email protected]>
>
> description:
> - The SiFive Level 2 Cache Controller is used to provide access to fast copies
> - of memory for masters in a Core Complex. The Level 2 Cache Controller also
> + The SiFive Composable Cache Controller is used to provide access to fast copies
> + of memory for masters in a Core Complex. The Composable Cache Controller also
> acts as directory-based coherency manager.
> All the properties in ePAPR/DeviceTree specification applies for this platform.
>
> @@ -27,6 +27,7 @@ select:
> enum:
> - sifive,fu540-c000-ccache
> - sifive,fu740-c000-ccache
> + - sifive,ccache0

Looks like Rob's bot had comments and I don't see a v2. Sorry if I'm
missing something.

Also: I'd guess that we only had the SOC-specific mappings on purpose.
It's kind of a grey area and I'm OK either way, but I'd definately
prefer the DT folks to get a chance to review these. My guess is that
they're not looking due to the bot comments, but sorry again if I've
missed it.

> required:
> - compatible
> @@ -37,6 +38,7 @@ properties:
> - enum:
> - sifive,fu540-c000-ccache
> - sifive,fu740-c000-ccache
> + - sifive,ccache0
> - const: cache
>
> cache-block-size:

2022-10-07 04:31:49

by Zong Li

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: sifive-ccache: rename SiFive L2 cache to composible cache

On Fri, Oct 7, 2022 at 10:58 AM Palmer Dabbelt <[email protected]> wrote:
>
> On Sun, 28 Aug 2022 23:22:00 PDT (-0700), [email protected] wrote:
> > Since composible cache may be L3 cache if private L2 cache exists, it
> > should use its original name composible cache to prevent confusion.
> >
> > Signed-off-by: Greentime Hu <[email protected]>
> > Signed-off-by: Zong Li <[email protected]>
> > ---
> > .../riscv/{sifive-l2-cache.yaml => sifive-ccache.yaml} | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> > rename Documentation/devicetree/bindings/riscv/{sifive-l2-cache.yaml => sifive-ccache.yaml} (92%)
> >
> > diff --git a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml b/Documentation/devicetree/bindings/riscv/sifive-ccache.yaml
> > similarity index 92%
> > rename from Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
> > rename to Documentation/devicetree/bindings/riscv/sifive-ccache.yaml
> > index 69cdab18d629..1a64a5384e36 100644
> > --- a/Documentation/devicetree/bindings/riscv/sifive-l2-cache.yaml
> > +++ b/Documentation/devicetree/bindings/riscv/sifive-ccache.yaml
> > @@ -12,8 +12,8 @@ maintainers:
> > - Paul Walmsley <[email protected]>
> >
> > description:
> > - The SiFive Level 2 Cache Controller is used to provide access to fast copies
> > - of memory for masters in a Core Complex. The Level 2 Cache Controller also
> > + The SiFive Composable Cache Controller is used to provide access to fast copies
> > + of memory for masters in a Core Complex. The Composable Cache Controller also
> > acts as directory-based coherency manager.
> > All the properties in ePAPR/DeviceTree specification applies for this platform.
> >
> > @@ -27,6 +27,7 @@ select:
> > enum:
> > - sifive,fu540-c000-ccache
> > - sifive,fu740-c000-ccache
> > + - sifive,ccache0
>
> Looks like Rob's bot had comments and I don't see a v2. Sorry if I'm
> missing something.

Hi Palmer,
We moved this series to the following patch set:
http://lists.infradead.org/pipermail/linux-riscv/2022-October/020196.html

Sorry for the confusion. Many thanks for considering this series.

>
> Also: I'd guess that we only had the SOC-specific mappings on purpose.
> It's kind of a grey area and I'm OK either way, but I'd definately
> prefer the DT folks to get a chance to review these. My guess is that
> they're not looking due to the bot comments, but sorry again if I've
> missed it.
>
> > required:
> > - compatible
> > @@ -37,6 +38,7 @@ properties:
> > - enum:
> > - sifive,fu540-c000-ccache
> > - sifive,fu740-c000-ccache
> > + - sifive,ccache0
> > - const: cache
> >
> > cache-block-size: