2022-10-19 22:05:39

by Lad, Prabhakar

[permalink] [raw]
Subject: [RFC PATCH v3 1/2] dt-bindings: cache: r9a07g043f-l2-cache: Add DT binding documentation for L2 cache controller

From: Lad Prabhakar <[email protected]>

Add DT binding documentation for L2 cache controller found on RZ/Five SoC.

The Renesas RZ/Five microprocessor includes a RISC-V CPU Core (AX45MP
Single) from Andes. The AX45MP core has an L2 cache controller, this patch
describes the L2 cache block.

Signed-off-by: Lad Prabhakar <[email protected]>
---
.../cache/andestech,ax45mp-cache.yaml | 125 ++++++++++++++++++
.../cache/andestech,ax45mp-cache.h | 38 ++++++
2 files changed, 163 insertions(+)
create mode 100644 Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml
create mode 100644 include/dt-bindings/cache/andestech,ax45mp-cache.h

diff --git a/Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml b/Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml
new file mode 100644
index 000000000000..4c86a15bda5f
--- /dev/null
+++ b/Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml
@@ -0,0 +1,125 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright (C) 2022 Renesas Electronics Corp.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/cache/andestech,ax45mp-cache.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Andestech AX45MP L2 Cache Controller
+
+maintainers:
+ - Lad Prabhakar <[email protected]>
+
+description:
+ A level-2 cache (L2C) is used to improve the system performance by providing
+ a larger amount of cache line entries and reasonable access delays. The L2C
+ is shared between cores, and a non-inclusive non-exclusive policy is used.
+
+select:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - andestech,ax45mp-cache
+
+ required:
+ - compatible
+
+properties:
+ compatible:
+ items:
+ - const: andestech,ax45mp-cache
+ - const: cache
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ cache-line-size:
+ const: 64
+
+ cache-level:
+ const: 2
+
+ cache-sets:
+ const: 1024
+
+ cache-size:
+ enum: [131072, 262144, 524288, 1048576, 2097152]
+
+ cache-unified: true
+
+ next-level-cache: true
+
+ andestech,pma-regions:
+ $ref: /schemas/types.yaml#/definitions/uint32-matrix
+ minItems: 1
+ maxItems: 16
+ description: Optional array of memory regions to be set as non-cacheable
+ bufferable regions which will be setup in the PMA.
+
+ andestech,inst-prefetch:
+ description: Instruction prefetch depth
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [ 0, 1, 2, 3 ]
+
+ andestech,data-prefetch:
+ description: Data prefetch depth
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [ 0, 1, 2, 3 ]
+
+ andestech,tag-ram-ctl:
+ description: Tag RAM output cycle. First tuple indicates output cycle and the
+ second tuple indicates setup cycle.
+ $ref: /schemas/types.yaml#/definitions/uint8-array
+ items:
+ - minimum: 0
+ maximum: 2
+ - minimum: 0
+ maximum: 2
+
+ andestech,data-ram-ctl:
+ description: Data RAM output cycle. First tuple indicates output cycle and the
+ second tuple indicates setup cycle.
+ $ref: /schemas/types.yaml#/definitions/uint8-array
+ items:
+ - minimum: 0
+ maximum: 2
+ - minimum: 0
+ maximum: 2
+
+additionalProperties: false
+
+required:
+ - compatible
+ - cache-line-size
+ - cache-level
+ - cache-sets
+ - cache-size
+ - cache-unified
+ - interrupts
+ - reg
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/cache/andestech,ax45mp-cache.h>
+
+ cache-controller@2010000 {
+ reg = <0x13400000 0x100000>;
+ compatible = "andestech,ax45mp-cache", "cache";
+ interrupts = <508 IRQ_TYPE_LEVEL_HIGH>;
+ cache-line-size = <64>;
+ cache-level = <2>;
+ cache-sets = <1024>;
+ cache-size = <262144>;
+ cache-unified;
+ andestech,pma-regions = <0x58000000 0x08000000
+ (AX45MP_PMACFG_ETYP_NAPOT | AX45MP_PMACFG_MTYP_MEM_NON_CACHE_BUF)>;
+ andestech,inst-prefetch = <0x3>;
+ andestech,data-prefetch = <0x3>;
+ andestech,tag-ram-ctl = /bits/ 8 <0x1 0x0>;
+ andestech,data-ram-ctl = /bits/ 8 <0x1 0x0>;
+ };
diff --git a/include/dt-bindings/cache/andestech,ax45mp-cache.h b/include/dt-bindings/cache/andestech,ax45mp-cache.h
new file mode 100644
index 000000000000..aa1cad24075d
--- /dev/null
+++ b/include/dt-bindings/cache/andestech,ax45mp-cache.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * This header provides constants for Andes AX45MP PMA configuration
+ *
+ * Copyright (C) 2022 Renesas Electronics Corp.
+ */
+
+#ifndef __DT_BINDINGS_ANDESTECH_AX45MP_CACHE_H
+#define __DT_BINDINGS_ANDESTECH_AX45MP_CACHE_H
+
+/* OFF: PMA entry is disabled */
+#define AX45MP_PMACFG_ETYP_DISABLED 0
+/* Naturally aligned power of 2 region */
+#define AX45MP_PMACFG_ETYP_NAPOT 3
+
+/* Device, Non-bufferable */
+#define AX45MP_PMACFG_MTYP_DEV_NON_BUF (0 << 2)
+/* Device, bufferable */
+#define AX45MP_PMACFG_MTYP_DEV_BUF (1 << 2)
+/* Memory, Non-cacheable, Non-bufferable */
+#define AX45MP_PMACFG_MTYP_MEM_NON_CACHE_NON_BUF (2 << 2)
+/* Memory, Non-cacheable, Bufferable */
+#define AX45MP_PMACFG_MTYP_MEM_NON_CACHE_BUF (3 << 2)
+/* Memory, Write-back, No-allocate */
+#define AX45MP_PMACFG_MTYP_MEM_WB_NA (8 << 2)
+/* Memory, Write-back, Read-allocate */
+#define AX45MP_PMACFG_MTYP_MEM_WB_RA (9 << 2)
+/* Memory, Write-back, Write-allocate */
+#define AX45MP_PMACFG_MTYP_MEM_WB_WA (10 << 2)
+/* Memory, Write-back, Read and Write-allocate */
+#define AX45MP_PMACFG_MTYP_MEM_WB_R_WA (11 << 2)
+
+/* AMO instructions are supported */
+#define AX45MP_PMACFG_NAMO_AMO_SUPPORT (0 << 6)
+/* AMO instructions are not supported */
+#define AX45MP_PMACFG_NAMO_AMO_NO_SUPPORT (1 << 6)
+
+#endif /* __DT_BINDINGS_ANDESTECH_AX45MP_CACHE_H */
--
2.25.1


2022-10-21 02:43:57

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [RFC PATCH v3 1/2] dt-bindings: cache: r9a07g043f-l2-cache: Add DT binding documentation for L2 cache controller

On Wed, Oct 19, 2022 at 11:02:41PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <[email protected]>
>
> Add DT binding documentation for L2 cache controller found on RZ/Five SoC.
>
> The Renesas RZ/Five microprocessor includes a RISC-V CPU Core (AX45MP
> Single) from Andes. The AX45MP core has an L2 cache controller, this patch
> describes the L2 cache block.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> .../cache/andestech,ax45mp-cache.yaml | 125 ++++++++++++++++++
> .../cache/andestech,ax45mp-cache.h | 38 ++++++
> 2 files changed, 163 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml
> create mode 100644 include/dt-bindings/cache/andestech,ax45mp-cache.h
>
> diff --git a/Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml b/Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml
> new file mode 100644
> index 000000000000..4c86a15bda5f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml
> @@ -0,0 +1,125 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright (C) 2022 Renesas Electronics Corp.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/cache/andestech,ax45mp-cache.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Andestech AX45MP L2 Cache Controller
> +
> +maintainers:
> + - Lad Prabhakar <[email protected]>
> +
> +description:
> + A level-2 cache (L2C) is used to improve the system performance by providing
> + a larger amount of cache line entries and reasonable access delays. The L2C
> + is shared between cores, and a non-inclusive non-exclusive policy is used.
> +
> +select:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - andestech,ax45mp-cache
> +
> + required:
> + - compatible
> +
> +properties:
> + compatible:
> + items:
> + - const: andestech,ax45mp-cache
> + - const: cache
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + cache-line-size:
> + const: 64
> +
> + cache-level:
> + const: 2
> +
> + cache-sets:
> + const: 1024
> +
> + cache-size:
> + enum: [131072, 262144, 524288, 1048576, 2097152]
> +
> + cache-unified: true
> +
> + next-level-cache: true
> +
> + andestech,pma-regions:
> + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> + minItems: 1
> + maxItems: 16

What is the inner dimension of the matrix?

items:
minItems: ?
maxItems: ?

> + description: Optional array of memory regions to be set as non-cacheable
> + bufferable regions which will be setup in the PMA.
> +
> + andestech,inst-prefetch:
> + description: Instruction prefetch depth
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [ 0, 1, 2, 3 ]
> +
> + andestech,data-prefetch:
> + description: Data prefetch depth
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [ 0, 1, 2, 3 ]
> +
> + andestech,tag-ram-ctl:
> + description: Tag RAM output cycle. First tuple indicates output cycle and the
> + second tuple indicates setup cycle.
> + $ref: /schemas/types.yaml#/definitions/uint8-array
> + items:
> + - minimum: 0
> + maximum: 2
> + - minimum: 0
> + maximum: 2

maxItems: 2
items:
maximum: 2

'items' without the '-' applies to all items.

And the minimum is already 0.

> +
> + andestech,data-ram-ctl:
> + description: Data RAM output cycle. First tuple indicates output cycle and the
> + second tuple indicates setup cycle.
> + $ref: /schemas/types.yaml#/definitions/uint8-array
> + items:
> + - minimum: 0
> + maximum: 2
> + - minimum: 0
> + maximum: 2

Same here.

> +
> +additionalProperties: false
> +
> +required:
> + - compatible
> + - cache-line-size
> + - cache-level
> + - cache-sets
> + - cache-size
> + - cache-unified
> + - interrupts
> + - reg
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> + #include <dt-bindings/cache/andestech,ax45mp-cache.h>
> +
> + cache-controller@2010000 {
> + reg = <0x13400000 0x100000>;
> + compatible = "andestech,ax45mp-cache", "cache";
> + interrupts = <508 IRQ_TYPE_LEVEL_HIGH>;
> + cache-line-size = <64>;
> + cache-level = <2>;
> + cache-sets = <1024>;
> + cache-size = <262144>;
> + cache-unified;
> + andestech,pma-regions = <0x58000000 0x08000000
> + (AX45MP_PMACFG_ETYP_NAPOT | AX45MP_PMACFG_MTYP_MEM_NON_CACHE_BUF)>;
> + andestech,inst-prefetch = <0x3>;
> + andestech,data-prefetch = <0x3>;
> + andestech,tag-ram-ctl = /bits/ 8 <0x1 0x0>;
> + andestech,data-ram-ctl = /bits/ 8 <0x1 0x0>;
> + };
> diff --git a/include/dt-bindings/cache/andestech,ax45mp-cache.h b/include/dt-bindings/cache/andestech,ax45mp-cache.h
> new file mode 100644
> index 000000000000..aa1cad24075d
> --- /dev/null
> +++ b/include/dt-bindings/cache/andestech,ax45mp-cache.h
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * This header provides constants for Andes AX45MP PMA configuration
> + *
> + * Copyright (C) 2022 Renesas Electronics Corp.
> + */
> +
> +#ifndef __DT_BINDINGS_ANDESTECH_AX45MP_CACHE_H
> +#define __DT_BINDINGS_ANDESTECH_AX45MP_CACHE_H
> +
> +/* OFF: PMA entry is disabled */
> +#define AX45MP_PMACFG_ETYP_DISABLED 0
> +/* Naturally aligned power of 2 region */
> +#define AX45MP_PMACFG_ETYP_NAPOT 3
> +
> +/* Device, Non-bufferable */
> +#define AX45MP_PMACFG_MTYP_DEV_NON_BUF (0 << 2)
> +/* Device, bufferable */
> +#define AX45MP_PMACFG_MTYP_DEV_BUF (1 << 2)
> +/* Memory, Non-cacheable, Non-bufferable */
> +#define AX45MP_PMACFG_MTYP_MEM_NON_CACHE_NON_BUF (2 << 2)
> +/* Memory, Non-cacheable, Bufferable */
> +#define AX45MP_PMACFG_MTYP_MEM_NON_CACHE_BUF (3 << 2)
> +/* Memory, Write-back, No-allocate */
> +#define AX45MP_PMACFG_MTYP_MEM_WB_NA (8 << 2)
> +/* Memory, Write-back, Read-allocate */
> +#define AX45MP_PMACFG_MTYP_MEM_WB_RA (9 << 2)
> +/* Memory, Write-back, Write-allocate */
> +#define AX45MP_PMACFG_MTYP_MEM_WB_WA (10 << 2)
> +/* Memory, Write-back, Read and Write-allocate */
> +#define AX45MP_PMACFG_MTYP_MEM_WB_R_WA (11 << 2)
> +
> +/* AMO instructions are supported */
> +#define AX45MP_PMACFG_NAMO_AMO_SUPPORT (0 << 6)
> +/* AMO instructions are not supported */
> +#define AX45MP_PMACFG_NAMO_AMO_NO_SUPPORT (1 << 6)
> +
> +#endif /* __DT_BINDINGS_ANDESTECH_AX45MP_CACHE_H */
> --
> 2.25.1
>
>

2022-10-21 22:49:53

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [RFC PATCH v3 1/2] dt-bindings: cache: r9a07g043f-l2-cache: Add DT binding documentation for L2 cache controller

Hi Rob,

Thank you for the review.

On Fri, Oct 21, 2022 at 3:10 AM Rob Herring <[email protected]> wrote:
>
> On Wed, Oct 19, 2022 at 11:02:41PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <[email protected]>
> >
> > Add DT binding documentation for L2 cache controller found on RZ/Five SoC.
> >
> > The Renesas RZ/Five microprocessor includes a RISC-V CPU Core (AX45MP
> > Single) from Andes. The AX45MP core has an L2 cache controller, this patch
> > describes the L2 cache block.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > .../cache/andestech,ax45mp-cache.yaml | 125 ++++++++++++++++++
> > .../cache/andestech,ax45mp-cache.h | 38 ++++++
> > 2 files changed, 163 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml
> > create mode 100644 include/dt-bindings/cache/andestech,ax45mp-cache.h
> >
> > diff --git a/Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml b/Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml
> > new file mode 100644
> > index 000000000000..4c86a15bda5f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml
> > @@ -0,0 +1,125 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright (C) 2022 Renesas Electronics Corp.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/cache/andestech,ax45mp-cache.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Andestech AX45MP L2 Cache Controller
> > +
> > +maintainers:
> > + - Lad Prabhakar <[email protected]>
> > +
> > +description:
> > + A level-2 cache (L2C) is used to improve the system performance by providing
> > + a larger amount of cache line entries and reasonable access delays. The L2C
> > + is shared between cores, and a non-inclusive non-exclusive policy is used.
> > +
> > +select:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - andestech,ax45mp-cache
> > +
> > + required:
> > + - compatible
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - const: andestech,ax45mp-cache
> > + - const: cache
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + cache-line-size:
> > + const: 64
> > +
> > + cache-level:
> > + const: 2
> > +
> > + cache-sets:
> > + const: 1024
> > +
> > + cache-size:
> > + enum: [131072, 262144, 524288, 1048576, 2097152]
> > +
> > + cache-unified: true
> > +
> > + next-level-cache: true
> > +
> > + andestech,pma-regions:
> > + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > + minItems: 1
> > + maxItems: 16
>
> What is the inner dimension of the matrix?
>
> items:
> minItems: ?
> maxItems: ?
>
minItems = maxItems = 6

i.e. the first two entries are the address, next two is the size and
last two is the flag.
<0x0 0x58000000 0x0 0x08000000 0x0 (AX45MP_PMACFG_ETYP_NAPOT |
AX45MP_PMACFG_MTYP_MEM_NON_CACHE_BUF)>;
^^ This is
dummy and always be 0x0

Is the above OK?

> > + description: Optional array of memory regions to be set as non-cacheable
> > + bufferable regions which will be setup in the PMA.
> > +
> > + andestech,inst-prefetch:
> > + description: Instruction prefetch depth
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + enum: [ 0, 1, 2, 3 ]
> > +
> > + andestech,data-prefetch:
> > + description: Data prefetch depth
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + enum: [ 0, 1, 2, 3 ]
> > +
> > + andestech,tag-ram-ctl:
> > + description: Tag RAM output cycle. First tuple indicates output cycle and the
> > + second tuple indicates setup cycle.
> > + $ref: /schemas/types.yaml#/definitions/uint8-array
> > + items:
> > + - minimum: 0
> > + maximum: 2
> > + - minimum: 0
> > + maximum: 2
>
> maxItems: 2
> items:
> maximum: 2
>
> 'items' without the '-' applies to all items.
>
> And the minimum is already 0.
>
Thanks for the suggestion, I'll fix it in the next version.

> > +
> > + andestech,data-ram-ctl:
> > + description: Data RAM output cycle. First tuple indicates output cycle and the
> > + second tuple indicates setup cycle.
> > + $ref: /schemas/types.yaml#/definitions/uint8-array
> > + items:
> > + - minimum: 0
> > + maximum: 2
> > + - minimum: 0
> > + maximum: 2
>
> Same here.
>
Ditto.

Cheers,
Prabhakar

2022-10-24 17:14:56

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [RFC PATCH v3 1/2] dt-bindings: cache: r9a07g043f-l2-cache: Add DT binding documentation for L2 cache controller

Hi Prabhakar,

On Thu, Oct 20, 2022 at 12:02 AM Prabhakar <[email protected]> wrote:
> From: Lad Prabhakar <[email protected]>
>
> Add DT binding documentation for L2 cache controller found on RZ/Five SoC.
>
> The Renesas RZ/Five microprocessor includes a RISC-V CPU Core (AX45MP
> Single) from Andes. The AX45MP core has an L2 cache controller, this patch
> describes the L2 cache block.
>
> Signed-off-by: Lad Prabhakar <[email protected]>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml
> @@ -0,0 +1,125 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright (C) 2022 Renesas Electronics Corp.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/cache/andestech,ax45mp-cache.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Andestech AX45MP L2 Cache Controller
> +
> +maintainers:
> + - Lad Prabhakar <[email protected]>
> +
> +description:
> + A level-2 cache (L2C) is used to improve the system performance by providing
> + a larger amount of cache line entries and reasonable access delays. The L2C

large

> + is shared between cores, and a non-inclusive non-exclusive policy is used.
> +
> +select:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - andestech,ax45mp-cache
> +
> + required:
> + - compatible
> +
> +properties:
> + compatible:
> + items:
> + - const: andestech,ax45mp-cache
> + - const: cache
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + cache-line-size:
> + const: 64

This is fixed here, but the driver accepts (and uses) whatever value specified?

> +
> + cache-level:
> + const: 2
> +
> + cache-sets:
> + const: 1024
> +
> + cache-size:
> + enum: [131072, 262144, 524288, 1048576, 2097152]
> +
> + cache-unified: true
> +
> + next-level-cache: true
> +
> + andestech,pma-regions:
> + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> + minItems: 1
> + maxItems: 16
> + description: Optional array of memory regions to be set as non-cacheable
> + bufferable regions which will be setup in the PMA.
> +
> + andestech,inst-prefetch:
> + description: Instruction prefetch depth
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [ 0, 1, 2, 3 ]
> +
> + andestech,data-prefetch:
> + description: Data prefetch depth
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [ 0, 1, 2, 3 ]

According to Section 8.1.2 ("L2-Cache Prefetch"), this should be
[ 0, 2, 4, 8 ].

> + andestech,tag-ram-ctl:
> + description: Tag RAM output cycle. First tuple indicates output cycle and the
> + second tuple indicates setup cycle.

Nit: to me it sounds more logical to have the setup cycle first.
See also the order in the comment in the driver code:

/* tag RAM and data RAM setup and output cycle */

> + $ref: /schemas/types.yaml#/definitions/uint8-array
> + items:
> + - minimum: 0
> + maximum: 2
> + - minimum: 0
> + maximum: 2
> +
> + andestech,data-ram-ctl:
> + description: Data RAM output cycle. First tuple indicates output cycle and the
> + second tuple indicates setup cycle.

Likewise.

> + $ref: /schemas/types.yaml#/definitions/uint8-array
> + items:
> + - minimum: 0
> + maximum: 2
> + - minimum: 0
> + maximum: 2

Do we really need these andestech-specific properties?
If yes, how much (if any) of this do we want to be handled by the boot
loader, and how much (if any) by Linux?
If Linux is responsible, we might have to boot with L2 disabled, right?

For ARM Cortex A15/A7, we also have arm,{data,tag}-latency properties
defined, but no DTS specifies them (my patches to add them on R-Car
Gen2 were rejected). Note that this is different for e.g. older PL310.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2022-10-25 23:09:09

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [RFC PATCH v3 1/2] dt-bindings: cache: r9a07g043f-l2-cache: Add DT binding documentation for L2 cache controller

Hi Geert,

Thank you for the review.

On Mon, Oct 24, 2022 at 2:47 PM Geert Uytterhoeven <[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Thu, Oct 20, 2022 at 12:02 AM Prabhakar <[email protected]> wrote:
> > From: Lad Prabhakar <[email protected]>
> >
> > Add DT binding documentation for L2 cache controller found on RZ/Five SoC.
> >
> > The Renesas RZ/Five microprocessor includes a RISC-V CPU Core (AX45MP
> > Single) from Andes. The AX45MP core has an L2 cache controller, this patch
> > describes the L2 cache block.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
>
> Thanks for your patch!
>
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml
> > @@ -0,0 +1,125 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright (C) 2022 Renesas Electronics Corp.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/cache/andestech,ax45mp-cache.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Andestech AX45MP L2 Cache Controller
> > +
> > +maintainers:
> > + - Lad Prabhakar <[email protected]>
> > +
> > +description:
> > + A level-2 cache (L2C) is used to improve the system performance by providing
> > + a larger amount of cache line entries and reasonable access delays. The L2C
>
> large
>
OK.

> > + is shared between cores, and a non-inclusive non-exclusive policy is used.
> > +
> > +select:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - andestech,ax45mp-cache
> > +
> > + required:
> > + - compatible
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - const: andestech,ax45mp-cache
> > + - const: cache
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + cache-line-size:
> > + const: 64
>
> This is fixed here, but the driver accepts (and uses) whatever value specified?
>
Right, I'll add a check in the driver.

> > +
> > + cache-level:
> > + const: 2
> > +
> > + cache-sets:
> > + const: 1024
> > +
> > + cache-size:
> > + enum: [131072, 262144, 524288, 1048576, 2097152]
> > +
> > + cache-unified: true
> > +
> > + next-level-cache: true
> > +
> > + andestech,pma-regions:
> > + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > + minItems: 1
> > + maxItems: 16
> > + description: Optional array of memory regions to be set as non-cacheable
> > + bufferable regions which will be setup in the PMA.
> > +
> > + andestech,inst-prefetch:
> > + description: Instruction prefetch depth
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + enum: [ 0, 1, 2, 3 ]
> > +
> > + andestech,data-prefetch:
> > + description: Data prefetch depth
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + enum: [ 0, 1, 2, 3 ]
>
> According to Section 8.1.2 ("L2-Cache Prefetch"), this should be
> [ 0, 2, 4, 8 ].
>
I was directly fetching the value, but as you pointed out I'll switch
to the request count and convert the value accordingly in the driver.

> > + andestech,tag-ram-ctl:
> > + description: Tag RAM output cycle. First tuple indicates output cycle and the
> > + second tuple indicates setup cycle.
>
> Nit: to me it sounds more logical to have the setup cycle first.
> See also the order in the comment in the driver code:
>
> /* tag RAM and data RAM setup and output cycle */
>
> > + $ref: /schemas/types.yaml#/definitions/uint8-array
> > + items:
> > + - minimum: 0
> > + maximum: 2
> > + - minimum: 0
> > + maximum: 2
> > +
> > + andestech,data-ram-ctl:
> > + description: Data RAM output cycle. First tuple indicates output cycle and the
> > + second tuple indicates setup cycle.
>
> Likewise.
>
> > + $ref: /schemas/types.yaml#/definitions/uint8-array
> > + items:
> > + - minimum: 0
> > + maximum: 2
> > + - minimum: 0
> > + maximum: 2
>
> Do we really need these andestech-specific properties?
> If yes, how much (if any) of this do we want to be handled by the boot
> loader, and how much (if any) by Linux?
> If Linux is responsible, we might have to boot with L2 disabled, right?
>
> For ARM Cortex A15/A7, we also have arm,{data,tag}-latency properties
> defined, but no DTS specifies them (my patches to add them on R-Car
> Gen2 were rejected). Note that this is different for e.g. older PL310.
>
OK, I think we can get rid of the below properties completely from
here and have them configured earlier (ie in u-boot).

- andestech,inst-prefetch
- andestech,data-prefetch
- andestech,tag-ram-ctl
- andestech,data-ram-ctl

Cheers,
Prabhakar