Hi Geert, Thank you for the review. On Mon, Oct 24, 2022 at 2:47 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > > Hi Prabhakar, > > On Thu, Oct 20, 2022 at 12:02 AM Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > > > 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 <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > 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 <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > + > > +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
