| Message ID | 20220829062202.3287-2-zong.li@sifive.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Rename sifive L2 cache to sifive CCACHE | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dt-meta-schema | fail | build log |
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 <greentime.hu@sifive.com> > Signed-off-by: Zong Li <zong.li@sifive.com> > --- > .../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 <paul.walmsley@sifive.com> > > 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 >
On Mon, Aug 29, 2022 at 2:45 PM <Conor.Dooley@microchip.com> 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 <greentime.hu@sifive.com> > > Signed-off-by: Zong Li <zong.li@sifive.com> > > --- > > .../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 <paul.walmsley@sifive.com> > > > > 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 > > >
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 <greentime.hu@sifive.com> > Signed-off-by: Zong Li <zong.li@sifive.com> > --- > .../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.
On Tue, Aug 30, 2022 at 2:42 AM Rob Herring <robh@kernel.org> 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 <greentime.hu@sifive.com> > > Signed-off-by: Zong Li <zong.li@sifive.com> > > --- > > .../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.
On Sun, 28 Aug 2022 23:22:00 PDT (-0700), zong.li@sifive.com 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 <greentime.hu@sifive.com> > Signed-off-by: Zong Li <zong.li@sifive.com> > --- > .../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 <paul.walmsley@sifive.com> > > 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:
On Fri, Oct 7, 2022 at 10:58 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: > > On Sun, 28 Aug 2022 23:22:00 PDT (-0700), zong.li@sifive.com 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 <greentime.hu@sifive.com> > > Signed-off-by: Zong Li <zong.li@sifive.com> > > --- > > .../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 <paul.walmsley@sifive.com> > > > > 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:
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 <paul.walmsley@sifive.com> 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: