[v4,6/7] dt-bindings: cache: r9a07g043f-l2-cache: Add DT binding documentation for L2 cache controller
Message ID | 20221124172207.153718-7-prabhakar.mahadev-lad.rj@bp.renesas.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:f944:0:0:0:0:0 with SMTP id q4csp3523009wrr; Thu, 24 Nov 2022 09:27:47 -0800 (PST) X-Google-Smtp-Source: AA0mqf5cZy3f126wB9CznqLmd7dqmDMqZj7p10xwB1ZBWltdgFQwTa1d99s0ua1HurrFlNJQA6bC X-Received: by 2002:aa7:dbc3:0:b0:461:6b61:81ae with SMTP id v3-20020aa7dbc3000000b004616b6181aemr30842601edt.62.1669310867153; Thu, 24 Nov 2022 09:27:47 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1669310867; cv=none; d=google.com; s=arc-20160816; b=LnAODgoiK0QTuPR7DVqPsyomdfhK3cqJxLv59/OxIF2uKrr/SNswKC4Bthrn8gVER8 k9fAnuNRdlkXCILTjM1UFdJ4kr+NcxvxNADwThQM4h7Ul3rN14XjOOBnUI/M5XgQXJch f8+bkVZTUgOHeV+5yt/Sk964EY8zMTYpDHfLE2yQ/tAtrELOpSH8WqjYEm1XNmN7e815 qXua+C4fv6LmxbaeS1YTMtUPH7Yk0I0OULoYScnDGzBzi09cI+modAaxE6o8ANZEg8j/ sfwXWAb1MYNenqbjTt2vLI+gYJ6uONu+/rNU9UpWCVv3hYstIoGXn16fq6b4l5kyVekw jDAQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=6pVq/7Up2ay5U3igFGOv4evJBykOn5gce8qJ7iJVARg=; b=npz/Ve9bzdkURA2ji5GEMDUoSPOOMoVa9HtjD5HPhPcygsmL5TV4CgZmo/eJp3X+Bb rAmaDGrU8v/VJR1mqf3sXdtFBwmedMG9txPRiM3wcHQ98wap97b2LKtIDD6k9+5vZZR6 4K49oupA90273ZigaysOCtvkUrFVFsyyW9EVxkA//WAUhiPDdvZW2X7z+cokNTjVLQKP 2FNkffN1DJIFiuy6WJCRZzoldJZULUUuZfZGPJIuJvdXk2iETLaIrzD0y/M2yftSt+Ec syLKFR9mmTXudFBUKsehvtzwtFB68ROEIAkBnn7NsTNQyVDaVx7sgtCoeWdBapJmGbbj NZyA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b="VkDha0/y"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id xh13-20020a170906da8d00b007a7f207a1b9si1384406ejb.664.2022.11.24.09.27.23; Thu, 24 Nov 2022 09:27:47 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b="VkDha0/y"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229715AbiKXRXU (ORCPT <rfc822;fengqi706@gmail.com> + 99 others); Thu, 24 Nov 2022 12:23:20 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48222 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229812AbiKXRWu (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 24 Nov 2022 12:22:50 -0500 Received: from mail-wm1-x335.google.com (mail-wm1-x335.google.com [IPv6:2a00:1450:4864:20::335]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 16BA71740C; Thu, 24 Nov 2022 09:22:48 -0800 (PST) Received: by mail-wm1-x335.google.com with SMTP id v7so1775941wmn.0; Thu, 24 Nov 2022 09:22:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=6pVq/7Up2ay5U3igFGOv4evJBykOn5gce8qJ7iJVARg=; b=VkDha0/y1YroAoUc12D0j4O8KV1nd0kU2H6cs0j6bnou9/NzJ5M8TOk/BeWgCMudeH Jn2ChNGVTcQUXWao9Mlm6D+Yvuo7XSUuttSixS58iqpf1SJ0gARfYzmAkEWRGLUrW57k V72ywOMPN0+0t6X9epAS8KXtOiwKdGV1FA9RCfg2Grmawoa7bV3aitwDQP2hFYMp84hk VUtvKnten+3jTgK3kMWhI6e3kxa14k+BK4Nq4ioMdSmhamLEgeASv+o00GexKVfLIWIr LL4B11Yl4PvJnZyCUE6Hw+RlJr9aBa3xEeBkOoTyB7GVXEjubBiVUYeed7+wh7QIGJrY JjJw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=6pVq/7Up2ay5U3igFGOv4evJBykOn5gce8qJ7iJVARg=; b=QmkIUDlTzvpVUjUCJVv9RKr0LOcgQSuFASK0Xw3JpKCykWNTPtSxSgqTXGYp/+/gin ZYqEhm8KI0lB2CNxnywRpZ1qjx5w+5J4/3YySvBm9YH9t1+/HyXwK2fPYiJ88JQiI2+5 mwM78HIM7HWqh9JTCf879r4Uw68wI5IEqMsRJmvhU8bt7kN5NYsbhSuDd/3B+cKZi5J4 /3YFs6tjmOz8KUH8m1Fd+DyQjsuImDtMyGTHR20IAcAd0xQSgFvkRfPJ52uB6ltXIQG0 U4Sg0J0teUusDS4nZTTbq2EVaLM3PfjDJItFKfyZ204iA7eN5pm9V+FTFeq6AlWs6dFQ 2OZg== X-Gm-Message-State: ANoB5pkbyjoTvQx3Ye+u8bCrRwi9c5xQwMFRQaxJZMhnLTTh78OTN1gO 1aipWvNxquTJPSwNUTpR8so= X-Received: by 2002:a7b:cc85:0:b0:3bf:d1e2:1d9 with SMTP id p5-20020a7bcc85000000b003bfd1e201d9mr17967480wma.115.1669310566585; Thu, 24 Nov 2022 09:22:46 -0800 (PST) Received: from prasmi.home ([2a00:23c8:2501:c701:89ee:3f5d:1c99:35d8]) by smtp.gmail.com with ESMTPSA id v17-20020a05600c445100b003c64c186206sm2698086wmn.16.2022.11.24.09.22.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 24 Nov 2022 09:22:45 -0800 (PST) From: Prabhakar <prabhakar.csengg@gmail.com> X-Google-Original-From: Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> To: Paul Walmsley <paul.walmsley@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu>, Geert Uytterhoeven <geert+renesas@glider.be>, Magnus Damm <magnus.damm@gmail.com>, Heiko Stuebner <heiko@sntech.de>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor.dooley@microchip.com>, Guo Ren <guoren@kernel.org> Cc: Jisheng Zhang <jszhang@kernel.org>, Atish Patra <atishp@rivosinc.com>, Anup Patel <apatel@ventanamicro.com>, Andrew Jones <ajones@ventanamicro.com>, Nathan Chancellor <nathan@kernel.org>, Philipp Tomsich <philipp.tomsich@vrull.eu>, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org, linux-renesas-soc@vger.kernel.org, Prabhakar <prabhakar.csengg@gmail.com>, Biju Das <biju.das.jz@bp.renesas.com>, Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Subject: [PATCH v4 6/7] dt-bindings: cache: r9a07g043f-l2-cache: Add DT binding documentation for L2 cache controller Date: Thu, 24 Nov 2022 17:22:06 +0000 Message-Id: <20221124172207.153718-7-prabhakar.mahadev-lad.rj@bp.renesas.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20221124172207.153718-1-prabhakar.mahadev-lad.rj@bp.renesas.com> References: <20221124172207.153718-1-prabhakar.mahadev-lad.rj@bp.renesas.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1750399311858106743?= X-GMAIL-MSGID: =?utf-8?q?1750399311858106743?= |
Series |
AX45MP: Add support to non-coherent DMA
|
|
Commit Message
Lad, Prabhakar
Nov. 24, 2022, 5:22 p.m. UTC
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> --- RFC v3 -> v4 * Dropped l2 cache configuration parameters * s/larger/large * Added minItems/maxItems for andestech,pma-regions --- .../cache/andestech,ax45mp-cache.yaml | 93 +++++++++++++++++++ .../cache/andestech,ax45mp-cache.h | 38 ++++++++ 2 files changed, 131 insertions(+) create mode 100644 Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml create mode 100644 include/dt-bindings/cache/andestech,ax45mp-cache.h
Comments
On 24/11/2022 18:22, Prabhakar 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> > --- > RFC v3 -> v4 > * Dropped l2 cache configuration parameters > * s/larger/large > * Added minItems/maxItems for andestech,pma-regions > --- > .../cache/andestech,ax45mp-cache.yaml | 93 +++++++++++++++++++ > .../cache/andestech,ax45mp-cache.h | 38 ++++++++ > 2 files changed, 131 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..bf255b177d0a > --- /dev/null > +++ b/Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml > @@ -0,0 +1,93 @@ > +# 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 large 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 > + items: > + minItems: 3 > + maxItems: 3 Instead: items: items: - description: Explain - description: what is - description: here > + description: Optional array of memory regions to be set in the PMA. > + > +additionalProperties: false > + > +required: > + - compatible > + - cache-line-size > + - cache-level > + - cache-sets > + - cache-size > + - cache-unified > + - interrupts > + - reg Keep the same order as properties appear in the "properties:" > + > +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)>; > + }; > 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) What are all these? They don't look like flags, because 3 = 1 | 2... they don't look like constants, because we do not use shifts in constants. Are these some register values? I also do not see the header being used in the code, so why having a bindings header if it is not used (DTS is not usage...)? > +/* 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 */ Best regards, Krzysztof
Hi Krzysztof, Thank you for the review. On Fri, Nov 25, 2022 at 8:16 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 24/11/2022 18:22, Prabhakar 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> > > --- > > RFC v3 -> v4 > > * Dropped l2 cache configuration parameters > > * s/larger/large > > * Added minItems/maxItems for andestech,pma-regions > > --- > > .../cache/andestech,ax45mp-cache.yaml | 93 +++++++++++++++++++ > > .../cache/andestech,ax45mp-cache.h | 38 ++++++++ > > 2 files changed, 131 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..bf255b177d0a > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml > > @@ -0,0 +1,93 @@ > > +# 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 large 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 > > + items: > > + minItems: 3 > > + maxItems: 3 > > Instead: > items: > items: > - description: Explain > - description: what is > - description: here > Ok, I will do that in the next version. - description: Memory region offset to be set up in the PMA - description: Size of the PMA region - description: Flags indicating how the region should be set up in the PMA. (ETYP[1:0] | MTYP[5:2]) use the macros defined in include/dt-bindings/cache/andestech,ax45mp-cache.h. > > + description: Optional array of memory regions to be set in the PMA. > > + > > +additionalProperties: false > > + > > +required: > > + - compatible > > + - cache-line-size > > + - cache-level > > + - cache-sets > > + - cache-size > > + - cache-unified > > + - interrupts > > + - reg > > Keep the same order as properties appear in the "properties:" > Agreed, will do. > > + > > +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)>; > > + }; > > 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) > > What are all these? They don't look like flags, because 3 = 1 | 2... > they don't look like constants, because we do not use shifts in > constants. Are these some register values? I also do not see the header > being used in the code, so why having a bindings header if it is not > used (DTS is not usage...)? > These are register bit values for the MTYP[5:2] field. The DTS example in the binding doc (above) uses these macros. I haven't included the DTS/I patches with this patchset yet do think I should? Cheers, Prabhakar
, Hi Prabhakar, On Fri, Nov 25, 2022 at 11:34 AM Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > On Fri, Nov 25, 2022 at 8:16 AM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: > > On 24/11/2022 18:22, Prabhakar 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> > > > --- > > > RFC v3 -> v4 > > > * Dropped l2 cache configuration parameters > > > * s/larger/large > > > * Added minItems/maxItems for andestech,pma-regions > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml > > > +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)>; > > > + }; > > > 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) > > > > What are all these? They don't look like flags, because 3 = 1 | 2... > > they don't look like constants, because we do not use shifts in > > constants. Are these some register values? I also do not see the header > > being used in the code, so why having a bindings header if it is not > > used (DTS is not usage...)? > > > These are register bit values for the MTYP[5:2] field. The DTS example > in the binding doc (above) uses these macros. I haven't included the > DTS/I patches with this patchset yet do think I should? I think the main objection from Rob is that these look too much like raw register values to be written unchanged to registers, which is frowned upon in DT. Now, can we make this more generic? 1. Do you need AX45MP_PMACFG_ETYP_DISABLED, i.e. will it ever be specified in DTS, or is this a pure software thing? 2. Obviously you can let the driver decide if AX45MP_PMACFG_ETYP_NAPOT can be set, based on address/size? 3. If the two above are removed, the shifts can be handled in the driver instead, 4. Are there existing (more generic) definitions that can be used instead? BTW, what's the difference between non-bufferable and non-cacheable? 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
Hi Geert, On Fri, Nov 25, 2022 at 11:18 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > , Hi Prabhakar, > > On Fri, Nov 25, 2022 at 11:34 AM Lad, Prabhakar > <prabhakar.csengg@gmail.com> wrote: > > On Fri, Nov 25, 2022 at 8:16 AM Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > > > On 24/11/2022 18:22, Prabhakar 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> > > > > --- > > > > RFC v3 -> v4 > > > > * Dropped l2 cache configuration parameters > > > > * s/larger/large > > > > * Added minItems/maxItems for andestech,pma-regions > > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml > > > > > +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)>; > > > > + }; > > > > 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) > > > > > > What are all these? They don't look like flags, because 3 = 1 | 2... > > > they don't look like constants, because we do not use shifts in > > > constants. Are these some register values? I also do not see the header > > > being used in the code, so why having a bindings header if it is not > > > used (DTS is not usage...)? > > > > > These are register bit values for the MTYP[5:2] field. The DTS example > > in the binding doc (above) uses these macros. I haven't included the > > DTS/I patches with this patchset yet do think I should? > > I think the main objection from Rob is that these look too much like > raw register values to be written unchanged to registers, which is > frowned upon in DT. > > Now, can we make this more generic? > > 1. Do you need AX45MP_PMACFG_ETYP_DISABLED, i.e. will it ever be > specified in DTS, or is this a pure software thing? > 2. Obviously you can let the driver decide if AX45MP_PMACFG_ETYP_NAPOT > can be set, based on address/size? > 3. If the two above are removed, the shifts can be handled in the > driver instead, Yes we can get rid of AX45MP_PMACFG_ETYP_DISABLED and AX45MP_PMACFG_ETYP_NAPOT. If we are setting up the PMA region it always has to be a power of 2. So AX45MP_PMACFG_ETYP_NAPOT can be passed either from the driver or in the OpenSBI just OR it while setting up the PMA. > 4. Are there existing (more generic) definitions that can be used > instead? > You mean for the MTYP flags? I haven't come across any in the kernel. > BTW, what's the difference between non-bufferable and non-cacheable? > non-cacheable, from the Andes manual: Accessing to non-cacheable memory and device will bypass I-Cache, D-Cache and L2-Cache no matter the data is cached or not. The cache states are not changed by these accesses. TBH I dont have a clear answer for non-bufferable nor do we have in the Andes HW manual. I'll get the details from Andes. Cheers, Prabhakar
On 25/11/2022 11:34, Lad, Prabhakar wrote: >>> +/* 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) >> >> What are all these? They don't look like flags, because 3 = 1 | 2... >> they don't look like constants, because we do not use shifts in >> constants. Are these some register values? I also do not see the header >> being used in the code, so why having a bindings header if it is not >> used (DTS is not usage...)? >> > These are register bit values for the MTYP[5:2] field. The DTS example > in the binding doc (above) uses these macros. I haven't included the > DTS/I patches with this patchset yet do think I should? Then why storing it as bindings? Bindings headers describe the interface implemented by drivers and used by DTS, but this is not implemented by drivers. Best regards, Krzysztof
On Fri, Nov 25, 2022 at 01:12:18PM +0100, Krzysztof Kozlowski wrote: > On 25/11/2022 11:34, Lad, Prabhakar wrote: > >>> +/* 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) > >> > >> What are all these? They don't look like flags, because 3 = 1 | 2... > >> they don't look like constants, because we do not use shifts in > >> constants. Are these some register values? I also do not see the header > >> being used in the code, so why having a bindings header if it is not > >> used (DTS is not usage...)? > >> > > These are register bit values for the MTYP[5:2] field. The DTS example > > in the binding doc (above) uses these macros. I haven't included the > > DTS/I patches with this patchset yet do think I should? > > Then why storing it as bindings? Bindings headers describe the interface > implemented by drivers and used by DTS, but this is not implemented by > drivers. IIUC, some of these properties are non-discoverable attributes of the cache controller. I see two things that could be done here that are "better" than #defining bits: - add an RZ/Five specific compatible and use match data to set the attributes which is only possible if the pma-regions are set on a per SoC basis - make pma-regions into a child node, in which andestech,non-cacheable andestech,non-bufferable etc are properties of the child node Prabhakar, does that make sense or am I off with my understanding of the attributes? Thanks, Conor.
Hi Conor, On Fri, Nov 25, 2022 at 12:25 PM Conor Dooley <conor.dooley@microchip.com> wrote: > > On Fri, Nov 25, 2022 at 01:12:18PM +0100, Krzysztof Kozlowski wrote: > > On 25/11/2022 11:34, Lad, Prabhakar wrote: > > >>> +/* 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) > > >> > > >> What are all these? They don't look like flags, because 3 = 1 | 2... > > >> they don't look like constants, because we do not use shifts in > > >> constants. Are these some register values? I also do not see the header > > >> being used in the code, so why having a bindings header if it is not > > >> used (DTS is not usage...)? > > >> > > > These are register bit values for the MTYP[5:2] field. The DTS example > > > in the binding doc (above) uses these macros. I haven't included the > > > DTS/I patches with this patchset yet do think I should? > > > > Then why storing it as bindings? Bindings headers describe the interface > > implemented by drivers and used by DTS, but this is not implemented by > > drivers. > > IIUC, some of these properties are non-discoverable attributes of the > cache controller. I see two things that could be done here that are > "better" than #defining bits: > - add an RZ/Five specific compatible and use match data to set the > attributes which is only possible if the pma-regions are set on a > per SoC basis > - make pma-regions into a child node, in which andestech,non-cacheable > andestech,non-bufferable etc are properties of the child node > For now the only way to get DMA working without IOCP is to have AX45MP_PMACFG_MTYP_MEM_NON_CACHE_BUF. But for future purposes I have introduced the other available flags. So maybe for now we could just have this flag andestech,mem-non-cacheable-bufferable in the binding doc. 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-region@0x58000000 { reg = <0x58000000 0x08000000>; andestech,mem-non-cacheable-bufferable; }; andestech,pma-region@0xdeadbeef { reg = <0xdeadbeef 0x08000000>; andestech,mem-non-cacheable-bufferable; }; .... }; Did I chime in this time? Cheers, Prabhakar
On Fri, Nov 25, 2022 at 12:51:34PM +0000, Lad, Prabhakar wrote: > Hi Conor, > > On Fri, Nov 25, 2022 at 12:25 PM Conor Dooley > <conor.dooley@microchip.com> wrote: > > > > On Fri, Nov 25, 2022 at 01:12:18PM +0100, Krzysztof Kozlowski wrote: > > > On 25/11/2022 11:34, Lad, Prabhakar wrote: > > > >>> +/* 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) > > > >> > > > >> What are all these? They don't look like flags, because 3 = 1 | 2... > > > >> they don't look like constants, because we do not use shifts in > > > >> constants. Are these some register values? I also do not see the header > > > >> being used in the code, so why having a bindings header if it is not > > > >> used (DTS is not usage...)? > > > >> > > > > These are register bit values for the MTYP[5:2] field. The DTS example > > > > in the binding doc (above) uses these macros. I haven't included the > > > > DTS/I patches with this patchset yet do think I should? > > > > > > Then why storing it as bindings? Bindings headers describe the interface > > > implemented by drivers and used by DTS, but this is not implemented by > > > drivers. > > > > IIUC, some of these properties are non-discoverable attributes of the > > cache controller. I see two things that could be done here that are > > "better" than #defining bits: > > - add an RZ/Five specific compatible and use match data to set the > > attributes which is only possible if the pma-regions are set on a > > per SoC basis > > - make pma-regions into a child node, in which andestech,non-cacheable > > andestech,non-bufferable etc are properties of the child node > > > For now the only way to get DMA working without IOCP is to have > AX45MP_PMACFG_MTYP_MEM_NON_CACHE_BUF. But for future purposes I have > introduced the other available flags. > > So maybe for now we could just have this flag > andestech,mem-non-cacheable-bufferable in the binding doc. > > 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-region@0x58000000 { > reg = <0x58000000 0x08000000>; > andestech,mem-non-cacheable-bufferable; Yah, that's about what I would expect - except splitting the properties up. I think split up makes more sense from a property description point of view, rather than needing some sort of oneOf: - non-cacheable-bufferable - cacheable-non-bufferable - non-cacheable-non-bufferable > }; > andestech,pma-region@0xdeadbeef { > reg = <0xdeadbeef 0x08000000>; > andestech,mem-non-cacheable-bufferable; > }; > .... > };
On 25/11/2022 13:25, Conor Dooley wrote: > On Fri, Nov 25, 2022 at 01:12:18PM +0100, Krzysztof Kozlowski wrote: >> On 25/11/2022 11:34, Lad, Prabhakar wrote: >>>>> +/* 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) >>>> >>>> What are all these? They don't look like flags, because 3 = 1 | 2... >>>> they don't look like constants, because we do not use shifts in >>>> constants. Are these some register values? I also do not see the header >>>> being used in the code, so why having a bindings header if it is not >>>> used (DTS is not usage...)? >>>> >>> These are register bit values for the MTYP[5:2] field. The DTS example >>> in the binding doc (above) uses these macros. I haven't included the >>> DTS/I patches with this patchset yet do think I should? >> >> Then why storing it as bindings? Bindings headers describe the interface >> implemented by drivers and used by DTS, but this is not implemented by >> drivers. > > IIUC, some of these properties are non-discoverable attributes of the > cache controller. I see two things that could be done here that are > "better" than #defining bits: I did not comment about properties. I comment about constants. Why register values/offsets/addresses are in this particular case suitable for binding headers? > - add an RZ/Five specific compatible and use match data to set the > attributes which is only possible if the pma-regions are set on a > per SoC basis > - make pma-regions into a child node, in which andestech,non-cacheable > andestech,non-bufferable etc are properties of the child node > > Prabhakar, does that make sense or am I off with my understanding of the > attributes? Best regards, Krzysztof
On Fri, Nov 25, 2022 at 04:55:11PM +0100, Krzysztof Kozlowski wrote: > On 25/11/2022 13:25, Conor Dooley wrote: > > On Fri, Nov 25, 2022 at 01:12:18PM +0100, Krzysztof Kozlowski wrote: > >> On 25/11/2022 11:34, Lad, Prabhakar wrote: > >>>>> +/* 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) > >>>> > >>>> What are all these? They don't look like flags, because 3 = 1 | 2... > >>>> they don't look like constants, because we do not use shifts in > >>>> constants. Are these some register values? I also do not see the header > >>>> being used in the code, so why having a bindings header if it is not > >>>> used (DTS is not usage...)? > >>>> > >>> These are register bit values for the MTYP[5:2] field. The DTS example > >>> in the binding doc (above) uses these macros. I haven't included the > >>> DTS/I patches with this patchset yet do think I should? > >> > >> Then why storing it as bindings? Bindings headers describe the interface > >> implemented by drivers and used by DTS, but this is not implemented by > >> drivers. > > > > IIUC, some of these properties are non-discoverable attributes of the > > cache controller. I see two things that could be done here that are > > "better" than #defining bits: > > I did not comment about properties. I comment about constants. Why > register values/offsets/addresses are in this particular case suitable > for binding headers? I don't think we disagree here. I'm not in favour of the defines either here. Perhaps I confused you by accidentally not adding Prabhakar to the to field. The dt needs to convey his particular cache implementation's bufferable and/or coherent regions so I was suggesting alternatives for conveying this information, without resorting to defines. > > - add an RZ/Five specific compatible and use match data to set the > > attributes which is only possible if the pma-regions are set on a > > per SoC basis > > - make pma-regions into a child node, in which andestech,non-cacheable > > andestech,non-bufferable etc are properties of the child node
Hi Krzysztof, On Fri, Nov 25, 2022 at 12:12 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 25/11/2022 11:34, Lad, Prabhakar wrote: > >>> +/* 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) > >> > >> What are all these? They don't look like flags, because 3 = 1 | 2... > >> they don't look like constants, because we do not use shifts in > >> constants. Are these some register values? I also do not see the header > >> being used in the code, so why having a bindings header if it is not > >> used (DTS is not usage...)? > >> > > These are register bit values for the MTYP[5:2] field. The DTS example > > in the binding doc (above) uses these macros. I haven't included the > > DTS/I patches with this patchset yet do think I should? > > Then why storing it as bindings? Bindings headers describe the interface > implemented by drivers and used by DTS, but this is not implemented by > drivers. > I got your point. I'll make use of the header in the driver for the next version and fix your previously pointed comments. Cheers, Prabhakar
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..bf255b177d0a --- /dev/null +++ b/Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml @@ -0,0 +1,93 @@ +# 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 large 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 + items: + minItems: 3 + maxItems: 3 + description: Optional array of memory regions to be set in the PMA. + +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)>; + }; 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 */