lkml.org 
[lkml]   [2022]   [Oct]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [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 <prabhakar.csengg@gmail.com> wrote:
    > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
    >
    > 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@bp.renesas.com>

    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@bp.renesas.com>
    > +
    > +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 -- geert@linux-m68k.org

    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

    \
     
     \ /
      Last update: 2022-10-24 19:06    [W:3.422 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site