[U-Boot] [PATCH 0/6] Support Andes RISC-V l2cache on AE350 platform

From: Rick Chen rick@andestech.com
Add a v5l2 cache controller driver that is usually found on Andes RISC-V ae350 platform. It will parse and configure the cache settings (data & instruction prefetch, data & tag latency) from the device tree blob.
Also implement L2 cache flush and disable before jump to linux. The sequence will be preferred as below: L1 flush -> L1 disable -> L2 flush -> L2 disable
Rick Chen (6): dm: cache: add v5l2 cache controller driver riscv: ae350: use the v5l2 driver to configure the cache riscv: ae350: add imply v5l2 cache controller riscv: cache: Flush L2 cache before jump to linux riscv: dts: move out AE350 L2 node from cpus node riscv: ax25: use CCTL to flush d-cache
arch/riscv/cpu/ax25/cache.c | 22 ++++--- arch/riscv/cpu/ax25/cpu.c | 4 ++ arch/riscv/dts/ae350_32.dts | 17 ++++-- arch/riscv/dts/ae350_64.dts | 17 ++++-- arch/riscv/include/asm/global_data.h | 3 + arch/riscv/include/asm/v5l2cache.h | 61 +++++++++++++++++++ board/AndesTech/ax25-ae350/Kconfig | 1 + board/AndesTech/ax25-ae350/ax25-ae350.c | 15 +++++ drivers/cache/Kconfig | 9 +++ drivers/cache/Makefile | 1 + drivers/cache/cache-v5l2.c | 102 ++++++++++++++++++++++++++++++++ 11 files changed, 231 insertions(+), 21 deletions(-) create mode 100644 arch/riscv/include/asm/v5l2cache.h create mode 100644 drivers/cache/cache-v5l2.c

From: Rick Chen rick@andestech.com
Add a v5l2 cache controller driver that is usually found on Andes RISC-V ae350 platform. It will parse the cache settings from the dtb.
In this version tag and data ram control timing can be adjusted by the requirement from the dtb.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com --- arch/riscv/include/asm/global_data.h | 3 ++ arch/riscv/include/asm/v5l2cache.h | 61 +++++++++++++++++++++ drivers/cache/Kconfig | 9 ++++ drivers/cache/Makefile | 1 + drivers/cache/cache-v5l2.c | 102 +++++++++++++++++++++++++++++++++++ 5 files changed, 176 insertions(+) create mode 100644 arch/riscv/include/asm/v5l2cache.h create mode 100644 drivers/cache/cache-v5l2.c
diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h index b74bd7e..6e52d5d 100644 --- a/arch/riscv/include/asm/global_data.h +++ b/arch/riscv/include/asm/global_data.h @@ -24,6 +24,9 @@ struct arch_global_data { #ifdef CONFIG_ANDES_PLMT void __iomem *plmt; /* plmt base address */ #endif +#ifdef CONFIG_V5L2_CACHE + void __iomem *v5l2; /* v5l2 base address */ +#endif #ifdef CONFIG_SMP struct ipi_data ipi[CONFIG_NR_CPUS]; #endif diff --git a/arch/riscv/include/asm/v5l2cache.h b/arch/riscv/include/asm/v5l2cache.h new file mode 100644 index 0000000..8ed1c6c --- /dev/null +++ b/arch/riscv/include/asm/v5l2cache.h @@ -0,0 +1,61 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (C) 2019 Andes Technology Corporation + * Rick Chen, Andes Technology Corporation rick@andestech.com + */ + +#ifndef _ASM_V5_L2CACHE_H +#define _ASM_V5_L2CACHE_H + +struct l2cache { + volatile u64 configure; + volatile u64 control; + volatile u64 hpm0; + volatile u64 hpm1; + volatile u64 hpm2; + volatile u64 hpm3; + volatile u64 error_status; + volatile u64 ecc_error; + volatile u64 cctl_command0; + volatile u64 cctl_access_line0; + volatile u64 cctl_command1; + volatile u64 cctl_access_line1; + volatile u64 cctl_command2; + volatile u64 cctl_access_line2; + volatile u64 cctl_command3; + volatile u64 cctl_access_line4; + volatile u64 cctl_status; +}; + +/* Control Register */ +#define L2_ENABLE 0x1 +/* prefetch */ +#define IPREPETCH_OFF 3 +#define DPREPETCH_OFF 5 +#define IPREPETCH_MSK (3 << IPREPETCH_OFF) +#define DPREPETCH_MSK (3 << DPREPETCH_OFF) +/* tag ram */ +#define TRAMOCTL_OFF 8 +#define TRAMICTL_OFF 10 +#define TRAMOCTL_MSK (3 << TRAMOCTL_OFF) +#define TRAMICTL_MSK BIT(TRAMICTL_OFF) +/* data ram */ +#define DRAMOCTL_OFF 11 +#define DRAMICTL_OFF 13 +#define DRAMOCTL_MSK (3 << DRAMOCTL_OFF) +#define DRAMICTL_MSK BIT(DRAMICTL_OFF) + +/* CCTL Command Register */ +#define CCTL_CMD_REG(base, hart) ((ulong)(base) + 0x40 + (hart) * 0x10) +#define L2_WBINVAL_ALL 0x12 + +/* CCTL Status Register */ +#define CCTL_STATUS_MSK(hart) (0xf << ((hart) * 4)) +#define CCTL_STATUS_IDLE(hart) (0 << ((hart) * 4)) +#define CCTL_STATUS_PROCESS(hart) (1 << ((hart) * 4)) +#define CCTL_STATUS_ILLEGAL(hart) (2 << ((hart) * 4)) + +void v5l2_enable(void); +void v5l2_disable(void); + +#endif /* _ASM_V5_L2CACHE_H */ diff --git a/drivers/cache/Kconfig b/drivers/cache/Kconfig index 24def7a..665689a 100644 --- a/drivers/cache/Kconfig +++ b/drivers/cache/Kconfig @@ -22,4 +22,13 @@ config L2X0_CACHE ARMv7(32-bit) devices. The driver configures the cache settings found in the device tree.
+config V5L2_CACHE + tristate "Andes V5L2 cache driver" + select CACHE + depends on RISCV_NDS_CACHE + help + Support Andes V5L2 cache controller in AE350 platform. + It will configure tag and data ram timing control from the + device tree and enable L2 cache. + endmenu diff --git a/drivers/cache/Makefile b/drivers/cache/Makefile index 9deb961..4a6458c 100644 --- a/drivers/cache/Makefile +++ b/drivers/cache/Makefile @@ -2,3 +2,4 @@ obj-$(CONFIG_CACHE) += cache-uclass.o obj-$(CONFIG_SANDBOX) += sandbox_cache.o obj-$(CONFIG_L2X0_CACHE) += cache-l2x0.o +obj-$(CONFIG_V5L2_CACHE) += cache-v5l2.o diff --git a/drivers/cache/cache-v5l2.c b/drivers/cache/cache-v5l2.c new file mode 100644 index 0000000..7022feb --- /dev/null +++ b/drivers/cache/cache-v5l2.c @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2019 Andes Technology Corporation + * Rick Chen, Andes Technology Corporation rick@andestech.com + */ + +#include <common.h> +#include <command.h> +#include <dm.h> +#include <asm/io.h> +#include <dm/ofnode.h> +#include <asm/v5l2cache.h> + +DECLARE_GLOBAL_DATA_PTR; + +void v5l2_enable(void) +{ + struct l2cache *regs = gd->arch.v5l2; + + if (regs) + setbits_le32(®s->control, L2_ENABLE); +} + +void v5l2_disable(void) +{ + volatile struct l2cache *regs = gd->arch.v5l2; + u8 hart = gd->arch.boot_hart; + void __iomem *cctlcmd = (void __iomem *)CCTL_CMD_REG(gd->arch.v5l2, hart); + + if ((regs) && (readl(®s->control) & L2_ENABLE)) { + writel(L2_WBINVAL_ALL, cctlcmd); + + while ((readl(®s->cctl_status) & CCTL_STATUS_MSK(hart))) { + if ((readl(®s->cctl_status) & CCTL_STATUS_ILLEGAL(hart))) { + printf("L2 flush illegal! hanging..."); + hang(); + } + } + clrbits_le32(®s->control, L2_ENABLE); + } +} + +static void v5l2_of_parse_and_init(struct udevice *dev) +{ + struct l2cache *regs; + u32 ctl_val, prefetch; + u32 tram_ctl[2]; + u32 dram_ctl[2]; + + regs = (struct l2cache *)dev_read_addr(dev); + gd->arch.v5l2 = regs; + ctl_val = readl(®s->control); + + if (!(ctl_val & L2_ENABLE)) + ctl_val |= L2_ENABLE; + + /* Instruction and data fetch prefetch depth */ + if (!dev_read_u32(dev, "andes,inst-prefetch", &prefetch)) { + ctl_val &= ~(IPREPETCH_MSK); + ctl_val |= (prefetch << IPREPETCH_OFF); + } + + if (!dev_read_u32(dev, "andes,data-prefetch", &prefetch)) { + ctl_val &= ~(DPREPETCH_MSK); + ctl_val |= (prefetch << DPREPETCH_OFF); + } + + /* Set tag RAM and data RAM setup and output cycle */ + if (!dev_read_u32_array(dev, "andes,tag-ram-ctl", tram_ctl, 2)) { + ctl_val &= ~(TRAMOCTL_MSK | TRAMICTL_MSK); + ctl_val |= tram_ctl[0] << TRAMOCTL_OFF; + ctl_val |= tram_ctl[1] << TRAMICTL_OFF; + } + + if (!dev_read_u32_array(dev, "andes,data-ram-ctl", dram_ctl, 2)) { + ctl_val &= ~(DRAMOCTL_MSK | DRAMICTL_MSK); + ctl_val |= dram_ctl[0] << DRAMOCTL_OFF; + ctl_val |= dram_ctl[1] << DRAMICTL_OFF; + } + + writel(ctl_val, ®s->control); +} + +static int v5l2_probe(struct udevice *dev) +{ + v5l2_of_parse_and_init(dev); + + return 0; +} + +static const struct udevice_id v5l2_cache_ids[] = { + { .compatible = "cache" }, + {} +}; + +U_BOOT_DRIVER(v5l2_cache) = { + .name = "v5l2_cache", + .id = UCLASS_CACHE, + .of_match = v5l2_cache_ids, + .probe = v5l2_probe, + .flags = DM_FLAG_PRE_RELOC, +};

Hi Rick,
On Tue, May 28, 2019 at 5:44 PM Andes uboot@andestech.com wrote:
From: Rick Chen rick@andestech.com
Add a v5l2 cache controller driver that is usually found on Andes RISC-V ae350 platform. It will parse the cache settings from the dtb.
In this version tag and data ram control timing can be adjusted by the requirement from the dtb.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com
arch/riscv/include/asm/global_data.h | 3 ++ arch/riscv/include/asm/v5l2cache.h | 61 +++++++++++++++++++++ drivers/cache/Kconfig | 9 ++++ drivers/cache/Makefile | 1 + drivers/cache/cache-v5l2.c | 102 +++++++++++++++++++++++++++++++++++ 5 files changed, 176 insertions(+) create mode 100644 arch/riscv/include/asm/v5l2cache.h create mode 100644 drivers/cache/cache-v5l2.c
diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h index b74bd7e..6e52d5d 100644 --- a/arch/riscv/include/asm/global_data.h +++ b/arch/riscv/include/asm/global_data.h @@ -24,6 +24,9 @@ struct arch_global_data { #ifdef CONFIG_ANDES_PLMT void __iomem *plmt; /* plmt base address */ #endif +#ifdef CONFIG_V5L2_CACHE
void __iomem *v5l2; /* v5l2 base address */
+#endif
Please do not expose this to global data if it is only used inside a driver. Anything that is here is for "global" usage.
#ifdef CONFIG_SMP struct ipi_data ipi[CONFIG_NR_CPUS]; #endif diff --git a/arch/riscv/include/asm/v5l2cache.h b/arch/riscv/include/asm/v5l2cache.h new file mode 100644 index 0000000..8ed1c6c --- /dev/null +++ b/arch/riscv/include/asm/v5l2cache.h
Please create arch/riscv/include/asm/arch-ax25/v5l2cache.h. Speaking of the name, would it be more readable to name it as v5_l2cache.h? Or even add more information to v5, for me, I don't know what v5 stands for :)
@@ -0,0 +1,61 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*
- Copyright (C) 2019 Andes Technology Corporation
- Rick Chen, Andes Technology Corporation rick@andestech.com
- */
+#ifndef _ASM_V5_L2CACHE_H +#define _ASM_V5_L2CACHE_H
+struct l2cache {
volatile u64 configure;
checkpatch.pl will report warnings against volatile usage. I think we should drop these.
volatile u64 control;
volatile u64 hpm0;
volatile u64 hpm1;
volatile u64 hpm2;
volatile u64 hpm3;
volatile u64 error_status;
volatile u64 ecc_error;
volatile u64 cctl_command0;
volatile u64 cctl_access_line0;
volatile u64 cctl_command1;
volatile u64 cctl_access_line1;
volatile u64 cctl_command2;
volatile u64 cctl_access_line2;
volatile u64 cctl_command3;
volatile u64 cctl_access_line4;
volatile u64 cctl_status;
+};
+/* Control Register */ +#define L2_ENABLE 0x1 +/* prefetch */ +#define IPREPETCH_OFF 3 +#define DPREPETCH_OFF 5 +#define IPREPETCH_MSK (3 << IPREPETCH_OFF) +#define DPREPETCH_MSK (3 << DPREPETCH_OFF) +/* tag ram */ +#define TRAMOCTL_OFF 8 +#define TRAMICTL_OFF 10 +#define TRAMOCTL_MSK (3 << TRAMOCTL_OFF) +#define TRAMICTL_MSK BIT(TRAMICTL_OFF) +/* data ram */ +#define DRAMOCTL_OFF 11 +#define DRAMICTL_OFF 13 +#define DRAMOCTL_MSK (3 << DRAMOCTL_OFF) +#define DRAMICTL_MSK BIT(DRAMICTL_OFF)
+/* CCTL Command Register */ +#define CCTL_CMD_REG(base, hart) ((ulong)(base) + 0x40 + (hart) * 0x10) +#define L2_WBINVAL_ALL 0x12
+/* CCTL Status Register */ +#define CCTL_STATUS_MSK(hart) (0xf << ((hart) * 4)) +#define CCTL_STATUS_IDLE(hart) (0 << ((hart) * 4)) +#define CCTL_STATUS_PROCESS(hart) (1 << ((hart) * 4)) +#define CCTL_STATUS_ILLEGAL(hart) (2 << ((hart) * 4))
+void v5l2_enable(void); +void v5l2_disable(void);
+#endif /* _ASM_V5_L2CACHE_H */ diff --git a/drivers/cache/Kconfig b/drivers/cache/Kconfig index 24def7a..665689a 100644 --- a/drivers/cache/Kconfig +++ b/drivers/cache/Kconfig @@ -22,4 +22,13 @@ config L2X0_CACHE ARMv7(32-bit) devices. The driver configures the cache settings found in the device tree.
+config V5L2_CACHE
tristate "Andes V5L2 cache driver"
This should be bool. U-Boot does not support "tristate"
select CACHE
depends on RISCV_NDS_CACHE
help
Support Andes V5L2 cache controller in AE350 platform.
It will configure tag and data ram timing control from the
device tree and enable L2 cache.
endmenu diff --git a/drivers/cache/Makefile b/drivers/cache/Makefile index 9deb961..4a6458c 100644 --- a/drivers/cache/Makefile +++ b/drivers/cache/Makefile @@ -2,3 +2,4 @@ obj-$(CONFIG_CACHE) += cache-uclass.o obj-$(CONFIG_SANDBOX) += sandbox_cache.o obj-$(CONFIG_L2X0_CACHE) += cache-l2x0.o +obj-$(CONFIG_V5L2_CACHE) += cache-v5l2.o diff --git a/drivers/cache/cache-v5l2.c b/drivers/cache/cache-v5l2.c new file mode 100644 index 0000000..7022feb --- /dev/null +++ b/drivers/cache/cache-v5l2.c @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright (C) 2019 Andes Technology Corporation
- Rick Chen, Andes Technology Corporation rick@andestech.com
- */
+#include <common.h> +#include <command.h> +#include <dm.h> +#include <asm/io.h> +#include <dm/ofnode.h> +#include <asm/v5l2cache.h>
+DECLARE_GLOBAL_DATA_PTR;
+void v5l2_enable(void)
This should really be avoided. It looks current DM cache uclass driver is lacking of ops to do cache enable/disable. Please improve the DM cache uclass driver first.
+{
struct l2cache *regs = gd->arch.v5l2;
if (regs)
setbits_le32(®s->control, L2_ENABLE);
+}
+void v5l2_disable(void) +{
volatile struct l2cache *regs = gd->arch.v5l2;
u8 hart = gd->arch.boot_hart;
void __iomem *cctlcmd = (void __iomem *)CCTL_CMD_REG(gd->arch.v5l2, hart);
if ((regs) && (readl(®s->control) & L2_ENABLE)) {
writel(L2_WBINVAL_ALL, cctlcmd);
while ((readl(®s->cctl_status) & CCTL_STATUS_MSK(hart))) {
if ((readl(®s->cctl_status) & CCTL_STATUS_ILLEGAL(hart))) {
printf("L2 flush illegal! hanging...");
hang();
}
}
clrbits_le32(®s->control, L2_ENABLE);
}
+}
+static void v5l2_of_parse_and_init(struct udevice *dev)
The entire function below should really be created as the driver's ofdata_to_platdata() function, inside which all DT properties are parsed and saved to driver's platdata. After that, there is no need to get register base from global data.
+{
struct l2cache *regs;
u32 ctl_val, prefetch;
u32 tram_ctl[2];
u32 dram_ctl[2];
regs = (struct l2cache *)dev_read_addr(dev);
gd->arch.v5l2 = regs;
ctl_val = readl(®s->control);
if (!(ctl_val & L2_ENABLE))
ctl_val |= L2_ENABLE;
/* Instruction and data fetch prefetch depth */
if (!dev_read_u32(dev, "andes,inst-prefetch", &prefetch)) {
ctl_val &= ~(IPREPETCH_MSK);
ctl_val |= (prefetch << IPREPETCH_OFF);
}
if (!dev_read_u32(dev, "andes,data-prefetch", &prefetch)) {
ctl_val &= ~(DPREPETCH_MSK);
ctl_val |= (prefetch << DPREPETCH_OFF);
}
/* Set tag RAM and data RAM setup and output cycle */
if (!dev_read_u32_array(dev, "andes,tag-ram-ctl", tram_ctl, 2)) {
ctl_val &= ~(TRAMOCTL_MSK | TRAMICTL_MSK);
ctl_val |= tram_ctl[0] << TRAMOCTL_OFF;
ctl_val |= tram_ctl[1] << TRAMICTL_OFF;
}
if (!dev_read_u32_array(dev, "andes,data-ram-ctl", dram_ctl, 2)) {
ctl_val &= ~(DRAMOCTL_MSK | DRAMICTL_MSK);
ctl_val |= dram_ctl[0] << DRAMOCTL_OFF;
ctl_val |= dram_ctl[1] << DRAMICTL_OFF;
}
writel(ctl_val, ®s->control);
+}
+static int v5l2_probe(struct udevice *dev) +{
v5l2_of_parse_and_init(dev);
return 0;
+}
+static const struct udevice_id v5l2_cache_ids[] = {
{ .compatible = "cache" },
I suspect this compatible string is too generic. Has this been reviewed by DT community upstream?
{}
+};
+U_BOOT_DRIVER(v5l2_cache) = {
.name = "v5l2_cache",
.id = UCLASS_CACHE,
.of_match = v5l2_cache_ids,
.probe = v5l2_probe,
.flags = DM_FLAG_PRE_RELOC,
+};
Regards, Bin

Hi Bin
Bin Meng bmeng.cn@gmail.com 於 2019年6月4日 週二 上午10:48寫道:
Hi Rick,
On Tue, May 28, 2019 at 5:44 PM Andes uboot@andestech.com wrote:
From: Rick Chen rick@andestech.com
Add a v5l2 cache controller driver that is usually found on Andes RISC-V ae350 platform. It will parse the cache settings from the dtb.
In this version tag and data ram control timing can be adjusted by the requirement from the dtb.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com
arch/riscv/include/asm/global_data.h | 3 ++ arch/riscv/include/asm/v5l2cache.h | 61 +++++++++++++++++++++ drivers/cache/Kconfig | 9 ++++ drivers/cache/Makefile | 1 + drivers/cache/cache-v5l2.c | 102 +++++++++++++++++++++++++++++++++++ 5 files changed, 176 insertions(+) create mode 100644 arch/riscv/include/asm/v5l2cache.h create mode 100644 drivers/cache/cache-v5l2.c
diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h index b74bd7e..6e52d5d 100644 --- a/arch/riscv/include/asm/global_data.h +++ b/arch/riscv/include/asm/global_data.h @@ -24,6 +24,9 @@ struct arch_global_data { #ifdef CONFIG_ANDES_PLMT void __iomem *plmt; /* plmt base address */ #endif +#ifdef CONFIG_V5L2_CACHE
void __iomem *v5l2; /* v5l2 base address */
+#endif
Please do not expose this to global data if it is only used inside a driver. Anything that is here is for "global" usage.
OK. I will remove it.
#ifdef CONFIG_SMP struct ipi_data ipi[CONFIG_NR_CPUS]; #endif diff --git a/arch/riscv/include/asm/v5l2cache.h b/arch/riscv/include/asm/v5l2cache.h new file mode 100644 index 0000000..8ed1c6c --- /dev/null +++ b/arch/riscv/include/asm/v5l2cache.h
Please create arch/riscv/include/asm/arch-ax25/v5l2cache.h. Speaking of the name, would it be more readable to name it as v5_l2cache.h? Or even add more information to v5, for me, I don't know what v5 stands for :)
OK I will create arch/riscv/include/asm/arch-ax25/v5_l2cache.h
@@ -0,0 +1,61 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*
- Copyright (C) 2019 Andes Technology Corporation
- Rick Chen, Andes Technology Corporation rick@andestech.com
- */
+#ifndef _ASM_V5_L2CACHE_H +#define _ASM_V5_L2CACHE_H
+struct l2cache {
volatile u64 configure;
checkpatch.pl will report warnings against volatile usage. I think we should drop these.
I know that checkpatch.pl will report this warning. But I still need to add volatile. It help to fix some unpredictable problem. Without this some driver code flows will be optimized and go wrong somehow.
volatile u64 control;
volatile u64 hpm0;
volatile u64 hpm1;
volatile u64 hpm2;
volatile u64 hpm3;
volatile u64 error_status;
volatile u64 ecc_error;
volatile u64 cctl_command0;
volatile u64 cctl_access_line0;
volatile u64 cctl_command1;
volatile u64 cctl_access_line1;
volatile u64 cctl_command2;
volatile u64 cctl_access_line2;
volatile u64 cctl_command3;
volatile u64 cctl_access_line4;
volatile u64 cctl_status;
+};
+/* Control Register */ +#define L2_ENABLE 0x1 +/* prefetch */ +#define IPREPETCH_OFF 3 +#define DPREPETCH_OFF 5 +#define IPREPETCH_MSK (3 << IPREPETCH_OFF) +#define DPREPETCH_MSK (3 << DPREPETCH_OFF) +/* tag ram */ +#define TRAMOCTL_OFF 8 +#define TRAMICTL_OFF 10 +#define TRAMOCTL_MSK (3 << TRAMOCTL_OFF) +#define TRAMICTL_MSK BIT(TRAMICTL_OFF) +/* data ram */ +#define DRAMOCTL_OFF 11 +#define DRAMICTL_OFF 13 +#define DRAMOCTL_MSK (3 << DRAMOCTL_OFF) +#define DRAMICTL_MSK BIT(DRAMICTL_OFF)
+/* CCTL Command Register */ +#define CCTL_CMD_REG(base, hart) ((ulong)(base) + 0x40 + (hart) * 0x10) +#define L2_WBINVAL_ALL 0x12
+/* CCTL Status Register */ +#define CCTL_STATUS_MSK(hart) (0xf << ((hart) * 4)) +#define CCTL_STATUS_IDLE(hart) (0 << ((hart) * 4)) +#define CCTL_STATUS_PROCESS(hart) (1 << ((hart) * 4)) +#define CCTL_STATUS_ILLEGAL(hart) (2 << ((hart) * 4))
+void v5l2_enable(void); +void v5l2_disable(void);
+#endif /* _ASM_V5_L2CACHE_H */ diff --git a/drivers/cache/Kconfig b/drivers/cache/Kconfig index 24def7a..665689a 100644 --- a/drivers/cache/Kconfig +++ b/drivers/cache/Kconfig @@ -22,4 +22,13 @@ config L2X0_CACHE ARMv7(32-bit) devices. The driver configures the cache settings found in the device tree.
+config V5L2_CACHE
tristate "Andes V5L2 cache driver"
This should be bool. U-Boot does not support "tristate"
I will modify it as bool.
select CACHE
depends on RISCV_NDS_CACHE
help
Support Andes V5L2 cache controller in AE350 platform.
It will configure tag and data ram timing control from the
device tree and enable L2 cache.
endmenu diff --git a/drivers/cache/Makefile b/drivers/cache/Makefile index 9deb961..4a6458c 100644 --- a/drivers/cache/Makefile +++ b/drivers/cache/Makefile @@ -2,3 +2,4 @@ obj-$(CONFIG_CACHE) += cache-uclass.o obj-$(CONFIG_SANDBOX) += sandbox_cache.o obj-$(CONFIG_L2X0_CACHE) += cache-l2x0.o +obj-$(CONFIG_V5L2_CACHE) += cache-v5l2.o diff --git a/drivers/cache/cache-v5l2.c b/drivers/cache/cache-v5l2.c new file mode 100644 index 0000000..7022feb --- /dev/null +++ b/drivers/cache/cache-v5l2.c @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright (C) 2019 Andes Technology Corporation
- Rick Chen, Andes Technology Corporation rick@andestech.com
- */
+#include <common.h> +#include <command.h> +#include <dm.h> +#include <asm/io.h> +#include <dm/ofnode.h> +#include <asm/v5l2cache.h>
+DECLARE_GLOBAL_DATA_PTR;
+void v5l2_enable(void)
This should really be avoided. It looks current DM cache uclass driver is lacking of ops to do cache enable/disable. Please improve the DM cache uclass driver first.
OK I will improve the DM cache uclass driver.
+{
struct l2cache *regs = gd->arch.v5l2;
if (regs)
setbits_le32(®s->control, L2_ENABLE);
+}
+void v5l2_disable(void) +{
volatile struct l2cache *regs = gd->arch.v5l2;
u8 hart = gd->arch.boot_hart;
void __iomem *cctlcmd = (void __iomem *)CCTL_CMD_REG(gd->arch.v5l2, hart);
if ((regs) && (readl(®s->control) & L2_ENABLE)) {
writel(L2_WBINVAL_ALL, cctlcmd);
while ((readl(®s->cctl_status) & CCTL_STATUS_MSK(hart))) {
if ((readl(®s->cctl_status) & CCTL_STATUS_ILLEGAL(hart))) {
printf("L2 flush illegal! hanging...");
hang();
}
}
clrbits_le32(®s->control, L2_ENABLE);
}
+}
+static void v5l2_of_parse_and_init(struct udevice *dev)
The entire function below should really be created as the driver's ofdata_to_platdata() function, inside which all DT properties are parsed and saved to driver's platdata. After that, there is no need to get register base from global data.
I will use ofdata_to_platdata() to parse dtb information and save it into platdata instead of saving in global data.
+{
struct l2cache *regs;
u32 ctl_val, prefetch;
u32 tram_ctl[2];
u32 dram_ctl[2];
regs = (struct l2cache *)dev_read_addr(dev);
gd->arch.v5l2 = regs;
ctl_val = readl(®s->control);
if (!(ctl_val & L2_ENABLE))
ctl_val |= L2_ENABLE;
/* Instruction and data fetch prefetch depth */
if (!dev_read_u32(dev, "andes,inst-prefetch", &prefetch)) {
ctl_val &= ~(IPREPETCH_MSK);
ctl_val |= (prefetch << IPREPETCH_OFF);
}
if (!dev_read_u32(dev, "andes,data-prefetch", &prefetch)) {
ctl_val &= ~(DPREPETCH_MSK);
ctl_val |= (prefetch << DPREPETCH_OFF);
}
/* Set tag RAM and data RAM setup and output cycle */
if (!dev_read_u32_array(dev, "andes,tag-ram-ctl", tram_ctl, 2)) {
ctl_val &= ~(TRAMOCTL_MSK | TRAMICTL_MSK);
ctl_val |= tram_ctl[0] << TRAMOCTL_OFF;
ctl_val |= tram_ctl[1] << TRAMICTL_OFF;
}
if (!dev_read_u32_array(dev, "andes,data-ram-ctl", dram_ctl, 2)) {
ctl_val &= ~(DRAMOCTL_MSK | DRAMICTL_MSK);
ctl_val |= dram_ctl[0] << DRAMOCTL_OFF;
ctl_val |= dram_ctl[1] << DRAMICTL_OFF;
}
writel(ctl_val, ®s->control);
+}
+static int v5l2_probe(struct udevice *dev) +{
v5l2_of_parse_and_init(dev);
return 0;
+}
+static const struct udevice_id v5l2_cache_ids[] = {
{ .compatible = "cache" },
I suspect this compatible string is too generic. Has this been reviewed by DT community upstream?
It refer to many dts examples from arm, For example : arch/arm/dts/fsl-imx8-ca35.dtsi A35_L2: l2-cache0 { compatible = "cache"; };
Thanks Rick
{}
+};
+U_BOOT_DRIVER(v5l2_cache) = {
.name = "v5l2_cache",
.id = UCLASS_CACHE,
.of_match = v5l2_cache_ids,
.probe = v5l2_probe,
.flags = DM_FLAG_PRE_RELOC,
+};
Regards, Bin

Hi Rick,
On Wed, 2019-06-05 at 16:58 +0800, Rick Chen wrote:
Hi Bin
Bin Meng bmeng.cn@gmail.com 於 2019年6月4日 週二 上午10:48寫道:
Hi Rick,
On Tue, May 28, 2019 at 5:44 PM Andes uboot@andestech.com wrote:
From: Rick Chen rick@andestech.com
Add a v5l2 cache controller driver that is usually found on Andes RISC-V ae350 platform. It will parse the cache settings from the dtb.
In this version tag and data ram control timing can be adjusted by the requirement from the dtb.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com
arch/riscv/include/asm/global_data.h | 3 ++ arch/riscv/include/asm/v5l2cache.h | 61 +++++++++++++++++++++ drivers/cache/Kconfig | 9 ++++ drivers/cache/Makefile | 1 + drivers/cache/cache-v5l2.c | 102 +++++++++++++++++++++++++++++++++++ 5 files changed, 176 insertions(+) create mode 100644 arch/riscv/include/asm/v5l2cache.h create mode 100644 drivers/cache/cache-v5l2.c
diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h index b74bd7e..6e52d5d 100644 --- a/arch/riscv/include/asm/global_data.h +++ b/arch/riscv/include/asm/global_data.h @@ -24,6 +24,9 @@ struct arch_global_data { #ifdef CONFIG_ANDES_PLMT void __iomem *plmt; /* plmt base address */ #endif +#ifdef CONFIG_V5L2_CACHE
void __iomem *v5l2; /* v5l2 base address */
+#endif
Please do not expose this to global data if it is only used inside a driver. Anything that is here is for "global" usage.
OK. I will remove it.
#ifdef CONFIG_SMP struct ipi_data ipi[CONFIG_NR_CPUS]; #endif diff --git a/arch/riscv/include/asm/v5l2cache.h b/arch/riscv/include/asm/v5l2cache.h new file mode 100644 index 0000000..8ed1c6c --- /dev/null +++ b/arch/riscv/include/asm/v5l2cache.h
Please create arch/riscv/include/asm/arch-ax25/v5l2cache.h. Speaking of the name, would it be more readable to name it as v5_l2cache.h? Or even add more information to v5, for me, I don't know what v5 stands for :)
OK I will create arch/riscv/include/asm/arch-ax25/v5_l2cache.h
@@ -0,0 +1,61 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*
- Copyright (C) 2019 Andes Technology Corporation
- Rick Chen, Andes Technology Corporation rick@andestech.com
- */
+#ifndef _ASM_V5_L2CACHE_H +#define _ASM_V5_L2CACHE_H
+struct l2cache {
volatile u64 configure;
checkpatch.pl will report warnings against volatile usage. I think we should drop these.
I know that checkpatch.pl will report this warning. But I still need to add volatile. It help to fix some unpredictable problem. Without this some driver code flows will be optimized and go wrong somehow.
volatile u64 control;
volatile u64 hpm0;
volatile u64 hpm1;
volatile u64 hpm2;
volatile u64 hpm3;
volatile u64 error_status;
volatile u64 ecc_error;
volatile u64 cctl_command0;
volatile u64 cctl_access_line0;
volatile u64 cctl_command1;
volatile u64 cctl_access_line1;
volatile u64 cctl_command2;
volatile u64 cctl_access_line2;
volatile u64 cctl_command3;
volatile u64 cctl_access_line4;
volatile u64 cctl_status;
+};
+/* Control Register */ +#define L2_ENABLE 0x1 +/* prefetch */ +#define IPREPETCH_OFF 3 +#define DPREPETCH_OFF 5 +#define IPREPETCH_MSK (3 << IPREPETCH_OFF) +#define DPREPETCH_MSK (3 << DPREPETCH_OFF) +/* tag ram */ +#define TRAMOCTL_OFF 8 +#define TRAMICTL_OFF 10 +#define TRAMOCTL_MSK (3 << TRAMOCTL_OFF) +#define TRAMICTL_MSK BIT(TRAMICTL_OFF) +/* data ram */ +#define DRAMOCTL_OFF 11 +#define DRAMICTL_OFF 13 +#define DRAMOCTL_MSK (3 << DRAMOCTL_OFF) +#define DRAMICTL_MSK BIT(DRAMICTL_OFF)
+/* CCTL Command Register */ +#define CCTL_CMD_REG(base, hart) ((ulong)(base) + 0x40 + (hart) * 0x10) +#define L2_WBINVAL_ALL 0x12
+/* CCTL Status Register */ +#define CCTL_STATUS_MSK(hart) (0xf << ((hart) * 4)) +#define CCTL_STATUS_IDLE(hart) (0 << ((hart) * 4)) +#define CCTL_STATUS_PROCESS(hart) (1 << ((hart) * 4)) +#define CCTL_STATUS_ILLEGAL(hart) (2 << ((hart) * 4))
+void v5l2_enable(void); +void v5l2_disable(void);
+#endif /* _ASM_V5_L2CACHE_H */ diff --git a/drivers/cache/Kconfig b/drivers/cache/Kconfig index 24def7a..665689a 100644 --- a/drivers/cache/Kconfig +++ b/drivers/cache/Kconfig @@ -22,4 +22,13 @@ config L2X0_CACHE ARMv7(32-bit) devices. The driver configures the cache settings found in the device tree.
+config V5L2_CACHE
tristate "Andes V5L2 cache driver"
This should be bool. U-Boot does not support "tristate"
I will modify it as bool.
select CACHE
depends on RISCV_NDS_CACHE
help
Support Andes V5L2 cache controller in AE350 platform.
It will configure tag and data ram timing control from the
device tree and enable L2 cache.
endmenu diff --git a/drivers/cache/Makefile b/drivers/cache/Makefile index 9deb961..4a6458c 100644 --- a/drivers/cache/Makefile +++ b/drivers/cache/Makefile @@ -2,3 +2,4 @@ obj-$(CONFIG_CACHE) += cache-uclass.o obj-$(CONFIG_SANDBOX) += sandbox_cache.o obj-$(CONFIG_L2X0_CACHE) += cache-l2x0.o +obj-$(CONFIG_V5L2_CACHE) += cache-v5l2.o diff --git a/drivers/cache/cache-v5l2.c b/drivers/cache/cache-v5l2.c new file mode 100644 index 0000000..7022feb --- /dev/null +++ b/drivers/cache/cache-v5l2.c @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright (C) 2019 Andes Technology Corporation
- Rick Chen, Andes Technology Corporation rick@andestech.com
- */
+#include <common.h> +#include <command.h> +#include <dm.h> +#include <asm/io.h> +#include <dm/ofnode.h> +#include <asm/v5l2cache.h>
+DECLARE_GLOBAL_DATA_PTR;
+void v5l2_enable(void)
This should really be avoided. It looks current DM cache uclass driver is lacking of ops to do cache enable/disable. Please improve the DM cache uclass driver first.
OK I will improve the DM cache uclass driver.
+{
struct l2cache *regs = gd->arch.v5l2;
if (regs)
setbits_le32(®s->control, L2_ENABLE);
+}
+void v5l2_disable(void) +{
volatile struct l2cache *regs = gd->arch.v5l2;
u8 hart = gd->arch.boot_hart;
void __iomem *cctlcmd = (void __iomem *)CCTL_CMD_REG(gd->arch.v5l2, hart);
if ((regs) && (readl(®s->control) & L2_ENABLE)) {
writel(L2_WBINVAL_ALL, cctlcmd);
while ((readl(®s->cctl_status) & CCTL_STATUS_MSK(hart))) {
if ((readl(®s->cctl_status) & CCTL_STATUS_ILLEGAL(hart))) {
printf("L2 flush illegal! hanging...");
hang();
}
}
clrbits_le32(®s->control, L2_ENABLE);
}
+}
+static void v5l2_of_parse_and_init(struct udevice *dev)
The entire function below should really be created as the driver's ofdata_to_platdata() function, inside which all DT properties are parsed and saved to driver's platdata. After that, there is no need to get register base from global data.
I will use ofdata_to_platdata() to parse dtb information and save it into platdata instead of saving in global data.
+{
struct l2cache *regs;
u32 ctl_val, prefetch;
u32 tram_ctl[2];
u32 dram_ctl[2];
regs = (struct l2cache *)dev_read_addr(dev);
gd->arch.v5l2 = regs;
ctl_val = readl(®s->control);
if (!(ctl_val & L2_ENABLE))
ctl_val |= L2_ENABLE;
/* Instruction and data fetch prefetch depth */
if (!dev_read_u32(dev, "andes,inst-prefetch", &prefetch)) {
ctl_val &= ~(IPREPETCH_MSK);
ctl_val |= (prefetch << IPREPETCH_OFF);
}
if (!dev_read_u32(dev, "andes,data-prefetch", &prefetch)) {
ctl_val &= ~(DPREPETCH_MSK);
ctl_val |= (prefetch << DPREPETCH_OFF);
}
/* Set tag RAM and data RAM setup and output cycle */
if (!dev_read_u32_array(dev, "andes,tag-ram-ctl", tram_ctl, 2)) {
ctl_val &= ~(TRAMOCTL_MSK | TRAMICTL_MSK);
ctl_val |= tram_ctl[0] << TRAMOCTL_OFF;
ctl_val |= tram_ctl[1] << TRAMICTL_OFF;
}
if (!dev_read_u32_array(dev, "andes,data-ram-ctl", dram_ctl, 2)) {
ctl_val &= ~(DRAMOCTL_MSK | DRAMICTL_MSK);
ctl_val |= dram_ctl[0] << DRAMOCTL_OFF;
ctl_val |= dram_ctl[1] << DRAMICTL_OFF;
}
writel(ctl_val, ®s->control);
+}
+static int v5l2_probe(struct udevice *dev) +{
v5l2_of_parse_and_init(dev);
return 0;
+}
+static const struct udevice_id v5l2_cache_ids[] = {
{ .compatible = "cache" },
I suspect this compatible string is too generic. Has this been reviewed by DT community upstream?
It refer to many dts examples from arm, For example : arch/arm/dts/fsl-imx8-ca35.dtsi A35_L2: l2-cache0 { compatible = "cache"; };
None of these have a driver for the cache controller, which is why it is sufficient to just use a generic compatible string.
I agree with Bin that you should choose a more specific compatible string. This is likely to cause problems in the future otherwise. For example, if Andes develops a new L2 cache controller, it must be differentiated from this one using the compatible string. The new controller would therefore have to use a different compatible string. It is good practice to already use a more specific one now to avoid the headache later. :)
Thanks, Lukas

Hi Lukas
Hi Rick,
On Wed, 2019-06-05 at 16:58 +0800, Rick Chen wrote:
Hi Bin
Bin Meng bmeng.cn@gmail.com 於 2019年6月4日 週二 上午10:48寫道:
Hi Rick,
On Tue, May 28, 2019 at 5:44 PM Andes uboot@andestech.com wrote:
From: Rick Chen rick@andestech.com
Add a v5l2 cache controller driver that is usually found on Andes RISC-V ae350 platform. It will parse the cache settings from the dtb.
In this version tag and data ram control timing can be adjusted by the requirement from the dtb.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com
arch/riscv/include/asm/global_data.h | 3 ++ arch/riscv/include/asm/v5l2cache.h | 61 +++++++++++++++++++++ drivers/cache/Kconfig | 9 ++++ drivers/cache/Makefile | 1 + drivers/cache/cache-v5l2.c | 102 +++++++++++++++++++++++++++++++++++ 5 files changed, 176 insertions(+) create mode 100644 arch/riscv/include/asm/v5l2cache.h create mode 100644 drivers/cache/cache-v5l2.c
diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h index b74bd7e..6e52d5d 100644 --- a/arch/riscv/include/asm/global_data.h +++ b/arch/riscv/include/asm/global_data.h @@ -24,6 +24,9 @@ struct arch_global_data { #ifdef CONFIG_ANDES_PLMT void __iomem *plmt; /* plmt base address */ #endif +#ifdef CONFIG_V5L2_CACHE
void __iomem *v5l2; /* v5l2 base address */
+#endif
Please do not expose this to global data if it is only used inside a driver. Anything that is here is for "global" usage.
OK. I will remove it.
#ifdef CONFIG_SMP struct ipi_data ipi[CONFIG_NR_CPUS]; #endif diff --git a/arch/riscv/include/asm/v5l2cache.h b/arch/riscv/include/asm/v5l2cache.h new file mode 100644 index 0000000..8ed1c6c --- /dev/null +++ b/arch/riscv/include/asm/v5l2cache.h
Please create arch/riscv/include/asm/arch-ax25/v5l2cache.h. Speaking of the name, would it be more readable to name it as v5_l2cache.h? Or even add more information to v5, for me, I don't know what v5 stands for :)
OK I will create arch/riscv/include/asm/arch-ax25/v5_l2cache.h
@@ -0,0 +1,61 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*
- Copyright (C) 2019 Andes Technology Corporation
- Rick Chen, Andes Technology Corporation rick@andestech.com
- */
+#ifndef _ASM_V5_L2CACHE_H +#define _ASM_V5_L2CACHE_H
+struct l2cache {
volatile u64 configure;
checkpatch.pl will report warnings against volatile usage. I think we should drop these.
I know that checkpatch.pl will report this warning. But I still need to add volatile. It help to fix some unpredictable problem. Without this some driver code flows will be optimized and go wrong somehow.
volatile u64 control;
volatile u64 hpm0;
volatile u64 hpm1;
volatile u64 hpm2;
volatile u64 hpm3;
volatile u64 error_status;
volatile u64 ecc_error;
volatile u64 cctl_command0;
volatile u64 cctl_access_line0;
volatile u64 cctl_command1;
volatile u64 cctl_access_line1;
volatile u64 cctl_command2;
volatile u64 cctl_access_line2;
volatile u64 cctl_command3;
volatile u64 cctl_access_line4;
volatile u64 cctl_status;
+};
+/* Control Register */ +#define L2_ENABLE 0x1 +/* prefetch */ +#define IPREPETCH_OFF 3 +#define DPREPETCH_OFF 5 +#define IPREPETCH_MSK (3 << IPREPETCH_OFF) +#define DPREPETCH_MSK (3 << DPREPETCH_OFF) +/* tag ram */ +#define TRAMOCTL_OFF 8 +#define TRAMICTL_OFF 10 +#define TRAMOCTL_MSK (3 << TRAMOCTL_OFF) +#define TRAMICTL_MSK BIT(TRAMICTL_OFF) +/* data ram */ +#define DRAMOCTL_OFF 11 +#define DRAMICTL_OFF 13 +#define DRAMOCTL_MSK (3 << DRAMOCTL_OFF) +#define DRAMICTL_MSK BIT(DRAMICTL_OFF)
+/* CCTL Command Register */ +#define CCTL_CMD_REG(base, hart) ((ulong)(base) + 0x40 + (hart) * 0x10) +#define L2_WBINVAL_ALL 0x12
+/* CCTL Status Register */ +#define CCTL_STATUS_MSK(hart) (0xf << ((hart) * 4)) +#define CCTL_STATUS_IDLE(hart) (0 << ((hart) * 4)) +#define CCTL_STATUS_PROCESS(hart) (1 << ((hart) * 4)) +#define CCTL_STATUS_ILLEGAL(hart) (2 << ((hart) * 4))
+void v5l2_enable(void); +void v5l2_disable(void);
+#endif /* _ASM_V5_L2CACHE_H */ diff --git a/drivers/cache/Kconfig b/drivers/cache/Kconfig index 24def7a..665689a 100644 --- a/drivers/cache/Kconfig +++ b/drivers/cache/Kconfig @@ -22,4 +22,13 @@ config L2X0_CACHE ARMv7(32-bit) devices. The driver configures the cache settings found in the device tree.
+config V5L2_CACHE
tristate "Andes V5L2 cache driver"
This should be bool. U-Boot does not support "tristate"
I will modify it as bool.
select CACHE
depends on RISCV_NDS_CACHE
help
Support Andes V5L2 cache controller in AE350 platform.
It will configure tag and data ram timing control from the
device tree and enable L2 cache.
endmenu diff --git a/drivers/cache/Makefile b/drivers/cache/Makefile index 9deb961..4a6458c 100644 --- a/drivers/cache/Makefile +++ b/drivers/cache/Makefile @@ -2,3 +2,4 @@ obj-$(CONFIG_CACHE) += cache-uclass.o obj-$(CONFIG_SANDBOX) += sandbox_cache.o obj-$(CONFIG_L2X0_CACHE) += cache-l2x0.o +obj-$(CONFIG_V5L2_CACHE) += cache-v5l2.o diff --git a/drivers/cache/cache-v5l2.c b/drivers/cache/cache-v5l2.c new file mode 100644 index 0000000..7022feb --- /dev/null +++ b/drivers/cache/cache-v5l2.c @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright (C) 2019 Andes Technology Corporation
- Rick Chen, Andes Technology Corporation rick@andestech.com
- */
+#include <common.h> +#include <command.h> +#include <dm.h> +#include <asm/io.h> +#include <dm/ofnode.h> +#include <asm/v5l2cache.h>
+DECLARE_GLOBAL_DATA_PTR;
+void v5l2_enable(void)
This should really be avoided. It looks current DM cache uclass driver is lacking of ops to do cache enable/disable. Please improve the DM cache uclass driver first.
OK I will improve the DM cache uclass driver.
+{
struct l2cache *regs = gd->arch.v5l2;
if (regs)
setbits_le32(®s->control, L2_ENABLE);
+}
+void v5l2_disable(void) +{
volatile struct l2cache *regs = gd->arch.v5l2;
u8 hart = gd->arch.boot_hart;
void __iomem *cctlcmd = (void __iomem *)CCTL_CMD_REG(gd->arch.v5l2, hart);
if ((regs) && (readl(®s->control) & L2_ENABLE)) {
writel(L2_WBINVAL_ALL, cctlcmd);
while ((readl(®s->cctl_status) & CCTL_STATUS_MSK(hart))) {
if ((readl(®s->cctl_status) & CCTL_STATUS_ILLEGAL(hart))) {
printf("L2 flush illegal! hanging...");
hang();
}
}
clrbits_le32(®s->control, L2_ENABLE);
}
+}
+static void v5l2_of_parse_and_init(struct udevice *dev)
The entire function below should really be created as the driver's ofdata_to_platdata() function, inside which all DT properties are parsed and saved to driver's platdata. After that, there is no need to get register base from global data.
I will use ofdata_to_platdata() to parse dtb information and save it into platdata instead of saving in global data.
+{
struct l2cache *regs;
u32 ctl_val, prefetch;
u32 tram_ctl[2];
u32 dram_ctl[2];
regs = (struct l2cache *)dev_read_addr(dev);
gd->arch.v5l2 = regs;
ctl_val = readl(®s->control);
if (!(ctl_val & L2_ENABLE))
ctl_val |= L2_ENABLE;
/* Instruction and data fetch prefetch depth */
if (!dev_read_u32(dev, "andes,inst-prefetch", &prefetch)) {
ctl_val &= ~(IPREPETCH_MSK);
ctl_val |= (prefetch << IPREPETCH_OFF);
}
if (!dev_read_u32(dev, "andes,data-prefetch", &prefetch)) {
ctl_val &= ~(DPREPETCH_MSK);
ctl_val |= (prefetch << DPREPETCH_OFF);
}
/* Set tag RAM and data RAM setup and output cycle */
if (!dev_read_u32_array(dev, "andes,tag-ram-ctl", tram_ctl, 2)) {
ctl_val &= ~(TRAMOCTL_MSK | TRAMICTL_MSK);
ctl_val |= tram_ctl[0] << TRAMOCTL_OFF;
ctl_val |= tram_ctl[1] << TRAMICTL_OFF;
}
if (!dev_read_u32_array(dev, "andes,data-ram-ctl", dram_ctl, 2)) {
ctl_val &= ~(DRAMOCTL_MSK | DRAMICTL_MSK);
ctl_val |= dram_ctl[0] << DRAMOCTL_OFF;
ctl_val |= dram_ctl[1] << DRAMICTL_OFF;
}
writel(ctl_val, ®s->control);
+}
+static int v5l2_probe(struct udevice *dev) +{
v5l2_of_parse_and_init(dev);
return 0;
+}
+static const struct udevice_id v5l2_cache_ids[] = {
{ .compatible = "cache" },
I suspect this compatible string is too generic. Has this been reviewed by DT community upstream?
It refer to many dts examples from arm, For example : arch/arm/dts/fsl-imx8-ca35.dtsi A35_L2: l2-cache0 { compatible = "cache"; };
None of these have a driver for the cache controller, which is why it is sufficient to just use a generic compatible string.
I agree with Bin that you should choose a more specific compatible string. This is likely to cause problems in the future otherwise. For example, if Andes develops a new L2 cache controller, it must be differentiated from this one using the compatible string. The new controller would therefore have to use a different compatible string. It is good practice to already use a more specific one now to avoid the headache later. :)
You are right. I will modify it as compatible = "v5l2cache"
Thanks Rick
Thanks, Lukas

Hi Rick,
On Mon, Jun 10, 2019 at 10:26 AM Rick Chen rickchen36@gmail.com wrote:
Hi Lukas
Hi Rick,
On Wed, 2019-06-05 at 16:58 +0800, Rick Chen wrote:
Hi Bin
Bin Meng bmeng.cn@gmail.com 於 2019年6月4日 週二 上午10:48寫道:
Hi Rick,
On Tue, May 28, 2019 at 5:44 PM Andes uboot@andestech.com wrote:
From: Rick Chen rick@andestech.com
Add a v5l2 cache controller driver that is usually found on Andes RISC-V ae350 platform. It will parse the cache settings from the dtb.
In this version tag and data ram control timing can be adjusted by the requirement from the dtb.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com
arch/riscv/include/asm/global_data.h | 3 ++ arch/riscv/include/asm/v5l2cache.h | 61 +++++++++++++++++++++ drivers/cache/Kconfig | 9 ++++ drivers/cache/Makefile | 1 + drivers/cache/cache-v5l2.c | 102 +++++++++++++++++++++++++++++++++++ 5 files changed, 176 insertions(+) create mode 100644 arch/riscv/include/asm/v5l2cache.h create mode 100644 drivers/cache/cache-v5l2.c
diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h index b74bd7e..6e52d5d 100644 --- a/arch/riscv/include/asm/global_data.h +++ b/arch/riscv/include/asm/global_data.h @@ -24,6 +24,9 @@ struct arch_global_data { #ifdef CONFIG_ANDES_PLMT void __iomem *plmt; /* plmt base address */ #endif +#ifdef CONFIG_V5L2_CACHE
void __iomem *v5l2; /* v5l2 base address */
+#endif
Please do not expose this to global data if it is only used inside a driver. Anything that is here is for "global" usage.
OK. I will remove it.
#ifdef CONFIG_SMP struct ipi_data ipi[CONFIG_NR_CPUS]; #endif diff --git a/arch/riscv/include/asm/v5l2cache.h b/arch/riscv/include/asm/v5l2cache.h new file mode 100644 index 0000000..8ed1c6c --- /dev/null +++ b/arch/riscv/include/asm/v5l2cache.h
Please create arch/riscv/include/asm/arch-ax25/v5l2cache.h. Speaking of the name, would it be more readable to name it as v5_l2cache.h? Or even add more information to v5, for me, I don't know what v5 stands for :)
OK I will create arch/riscv/include/asm/arch-ax25/v5_l2cache.h
@@ -0,0 +1,61 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*
- Copyright (C) 2019 Andes Technology Corporation
- Rick Chen, Andes Technology Corporation rick@andestech.com
- */
+#ifndef _ASM_V5_L2CACHE_H +#define _ASM_V5_L2CACHE_H
+struct l2cache {
volatile u64 configure;
checkpatch.pl will report warnings against volatile usage. I think we should drop these.
I know that checkpatch.pl will report this warning. But I still need to add volatile. It help to fix some unpredictable problem. Without this some driver code flows will be optimized and go wrong somehow.
volatile u64 control;
volatile u64 hpm0;
volatile u64 hpm1;
volatile u64 hpm2;
volatile u64 hpm3;
volatile u64 error_status;
volatile u64 ecc_error;
volatile u64 cctl_command0;
volatile u64 cctl_access_line0;
volatile u64 cctl_command1;
volatile u64 cctl_access_line1;
volatile u64 cctl_command2;
volatile u64 cctl_access_line2;
volatile u64 cctl_command3;
volatile u64 cctl_access_line4;
volatile u64 cctl_status;
+};
+/* Control Register */ +#define L2_ENABLE 0x1 +/* prefetch */ +#define IPREPETCH_OFF 3 +#define DPREPETCH_OFF 5 +#define IPREPETCH_MSK (3 << IPREPETCH_OFF) +#define DPREPETCH_MSK (3 << DPREPETCH_OFF) +/* tag ram */ +#define TRAMOCTL_OFF 8 +#define TRAMICTL_OFF 10 +#define TRAMOCTL_MSK (3 << TRAMOCTL_OFF) +#define TRAMICTL_MSK BIT(TRAMICTL_OFF) +/* data ram */ +#define DRAMOCTL_OFF 11 +#define DRAMICTL_OFF 13 +#define DRAMOCTL_MSK (3 << DRAMOCTL_OFF) +#define DRAMICTL_MSK BIT(DRAMICTL_OFF)
+/* CCTL Command Register */ +#define CCTL_CMD_REG(base, hart) ((ulong)(base) + 0x40 + (hart) * 0x10) +#define L2_WBINVAL_ALL 0x12
+/* CCTL Status Register */ +#define CCTL_STATUS_MSK(hart) (0xf << ((hart) * 4)) +#define CCTL_STATUS_IDLE(hart) (0 << ((hart) * 4)) +#define CCTL_STATUS_PROCESS(hart) (1 << ((hart) * 4)) +#define CCTL_STATUS_ILLEGAL(hart) (2 << ((hart) * 4))
+void v5l2_enable(void); +void v5l2_disable(void);
+#endif /* _ASM_V5_L2CACHE_H */ diff --git a/drivers/cache/Kconfig b/drivers/cache/Kconfig index 24def7a..665689a 100644 --- a/drivers/cache/Kconfig +++ b/drivers/cache/Kconfig @@ -22,4 +22,13 @@ config L2X0_CACHE ARMv7(32-bit) devices. The driver configures the cache settings found in the device tree.
+config V5L2_CACHE
tristate "Andes V5L2 cache driver"
This should be bool. U-Boot does not support "tristate"
I will modify it as bool.
select CACHE
depends on RISCV_NDS_CACHE
help
Support Andes V5L2 cache controller in AE350 platform.
It will configure tag and data ram timing control from the
device tree and enable L2 cache.
endmenu diff --git a/drivers/cache/Makefile b/drivers/cache/Makefile index 9deb961..4a6458c 100644 --- a/drivers/cache/Makefile +++ b/drivers/cache/Makefile @@ -2,3 +2,4 @@ obj-$(CONFIG_CACHE) += cache-uclass.o obj-$(CONFIG_SANDBOX) += sandbox_cache.o obj-$(CONFIG_L2X0_CACHE) += cache-l2x0.o +obj-$(CONFIG_V5L2_CACHE) += cache-v5l2.o diff --git a/drivers/cache/cache-v5l2.c b/drivers/cache/cache-v5l2.c new file mode 100644 index 0000000..7022feb --- /dev/null +++ b/drivers/cache/cache-v5l2.c @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright (C) 2019 Andes Technology Corporation
- Rick Chen, Andes Technology Corporation rick@andestech.com
- */
+#include <common.h> +#include <command.h> +#include <dm.h> +#include <asm/io.h> +#include <dm/ofnode.h> +#include <asm/v5l2cache.h>
+DECLARE_GLOBAL_DATA_PTR;
+void v5l2_enable(void)
This should really be avoided. It looks current DM cache uclass driver is lacking of ops to do cache enable/disable. Please improve the DM cache uclass driver first.
OK I will improve the DM cache uclass driver.
+{
struct l2cache *regs = gd->arch.v5l2;
if (regs)
setbits_le32(®s->control, L2_ENABLE);
+}
+void v5l2_disable(void) +{
volatile struct l2cache *regs = gd->arch.v5l2;
u8 hart = gd->arch.boot_hart;
void __iomem *cctlcmd = (void __iomem *)CCTL_CMD_REG(gd->arch.v5l2, hart);
if ((regs) && (readl(®s->control) & L2_ENABLE)) {
writel(L2_WBINVAL_ALL, cctlcmd);
while ((readl(®s->cctl_status) & CCTL_STATUS_MSK(hart))) {
if ((readl(®s->cctl_status) & CCTL_STATUS_ILLEGAL(hart))) {
printf("L2 flush illegal! hanging...");
hang();
}
}
clrbits_le32(®s->control, L2_ENABLE);
}
+}
+static void v5l2_of_parse_and_init(struct udevice *dev)
The entire function below should really be created as the driver's ofdata_to_platdata() function, inside which all DT properties are parsed and saved to driver's platdata. After that, there is no need to get register base from global data.
I will use ofdata_to_platdata() to parse dtb information and save it into platdata instead of saving in global data.
+{
struct l2cache *regs;
u32 ctl_val, prefetch;
u32 tram_ctl[2];
u32 dram_ctl[2];
regs = (struct l2cache *)dev_read_addr(dev);
gd->arch.v5l2 = regs;
ctl_val = readl(®s->control);
if (!(ctl_val & L2_ENABLE))
ctl_val |= L2_ENABLE;
/* Instruction and data fetch prefetch depth */
if (!dev_read_u32(dev, "andes,inst-prefetch", &prefetch)) {
ctl_val &= ~(IPREPETCH_MSK);
ctl_val |= (prefetch << IPREPETCH_OFF);
}
if (!dev_read_u32(dev, "andes,data-prefetch", &prefetch)) {
ctl_val &= ~(DPREPETCH_MSK);
ctl_val |= (prefetch << DPREPETCH_OFF);
}
/* Set tag RAM and data RAM setup and output cycle */
if (!dev_read_u32_array(dev, "andes,tag-ram-ctl", tram_ctl, 2)) {
ctl_val &= ~(TRAMOCTL_MSK | TRAMICTL_MSK);
ctl_val |= tram_ctl[0] << TRAMOCTL_OFF;
ctl_val |= tram_ctl[1] << TRAMICTL_OFF;
}
if (!dev_read_u32_array(dev, "andes,data-ram-ctl", dram_ctl, 2)) {
ctl_val &= ~(DRAMOCTL_MSK | DRAMICTL_MSK);
ctl_val |= dram_ctl[0] << DRAMOCTL_OFF;
ctl_val |= dram_ctl[1] << DRAMICTL_OFF;
}
writel(ctl_val, ®s->control);
+}
+static int v5l2_probe(struct udevice *dev) +{
v5l2_of_parse_and_init(dev);
return 0;
+}
+static const struct udevice_id v5l2_cache_ids[] = {
{ .compatible = "cache" },
I suspect this compatible string is too generic. Has this been reviewed by DT community upstream?
It refer to many dts examples from arm, For example : arch/arm/dts/fsl-imx8-ca35.dtsi A35_L2: l2-cache0 { compatible = "cache"; };
None of these have a driver for the cache controller, which is why it is sufficient to just use a generic compatible string.
I agree with Bin that you should choose a more specific compatible string. This is likely to cause problems in the future otherwise. For example, if Andes develops a new L2 cache controller, it must be differentiated from this one using the compatible string. The new controller would therefore have to use a different compatible string. It is good practice to already use a more specific one now to avoid the headache later. :)
You are right. I will modify it as compatible = "v5l2cache"
Before you do that, could you please check whether this L2 cache driver will be supported in the Linux kernel too? If yes, I think you need go through the kernel upstream process, during which the compatible string is to be discussed and approved.
Regards, Bin

Hi Bin
Hi Rick,
On Mon, Jun 10, 2019 at 10:26 AM Rick Chen rickchen36@gmail.com wrote:
Hi Lukas
Hi Rick,
On Wed, 2019-06-05 at 16:58 +0800, Rick Chen wrote:
Hi Bin
Bin Meng bmeng.cn@gmail.com 於 2019年6月4日 週二 上午10:48寫道:
Hi Rick,
On Tue, May 28, 2019 at 5:44 PM Andes uboot@andestech.com wrote:
From: Rick Chen rick@andestech.com
Add a v5l2 cache controller driver that is usually found on Andes RISC-V ae350 platform. It will parse the cache settings from the dtb.
In this version tag and data ram control timing can be adjusted by the requirement from the dtb.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com
arch/riscv/include/asm/global_data.h | 3 ++ arch/riscv/include/asm/v5l2cache.h | 61 +++++++++++++++++++++ drivers/cache/Kconfig | 9 ++++ drivers/cache/Makefile | 1 + drivers/cache/cache-v5l2.c | 102 +++++++++++++++++++++++++++++++++++ 5 files changed, 176 insertions(+) create mode 100644 arch/riscv/include/asm/v5l2cache.h create mode 100644 drivers/cache/cache-v5l2.c
diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h index b74bd7e..6e52d5d 100644 --- a/arch/riscv/include/asm/global_data.h +++ b/arch/riscv/include/asm/global_data.h @@ -24,6 +24,9 @@ struct arch_global_data { #ifdef CONFIG_ANDES_PLMT void __iomem *plmt; /* plmt base address */ #endif +#ifdef CONFIG_V5L2_CACHE
void __iomem *v5l2; /* v5l2 base address */
+#endif
Please do not expose this to global data if it is only used inside a driver. Anything that is here is for "global" usage.
OK. I will remove it.
#ifdef CONFIG_SMP struct ipi_data ipi[CONFIG_NR_CPUS]; #endif diff --git a/arch/riscv/include/asm/v5l2cache.h b/arch/riscv/include/asm/v5l2cache.h new file mode 100644 index 0000000..8ed1c6c --- /dev/null +++ b/arch/riscv/include/asm/v5l2cache.h
Please create arch/riscv/include/asm/arch-ax25/v5l2cache.h. Speaking of the name, would it be more readable to name it as v5_l2cache.h? Or even add more information to v5, for me, I don't know what v5 stands for :)
OK I will create arch/riscv/include/asm/arch-ax25/v5_l2cache.h
@@ -0,0 +1,61 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*
- Copyright (C) 2019 Andes Technology Corporation
- Rick Chen, Andes Technology Corporation rick@andestech.com
- */
+#ifndef _ASM_V5_L2CACHE_H +#define _ASM_V5_L2CACHE_H
+struct l2cache {
volatile u64 configure;
checkpatch.pl will report warnings against volatile usage. I think we should drop these.
I know that checkpatch.pl will report this warning. But I still need to add volatile. It help to fix some unpredictable problem. Without this some driver code flows will be optimized and go wrong somehow.
volatile u64 control;
volatile u64 hpm0;
volatile u64 hpm1;
volatile u64 hpm2;
volatile u64 hpm3;
volatile u64 error_status;
volatile u64 ecc_error;
volatile u64 cctl_command0;
volatile u64 cctl_access_line0;
volatile u64 cctl_command1;
volatile u64 cctl_access_line1;
volatile u64 cctl_command2;
volatile u64 cctl_access_line2;
volatile u64 cctl_command3;
volatile u64 cctl_access_line4;
volatile u64 cctl_status;
+};
+/* Control Register */ +#define L2_ENABLE 0x1 +/* prefetch */ +#define IPREPETCH_OFF 3 +#define DPREPETCH_OFF 5 +#define IPREPETCH_MSK (3 << IPREPETCH_OFF) +#define DPREPETCH_MSK (3 << DPREPETCH_OFF) +/* tag ram */ +#define TRAMOCTL_OFF 8 +#define TRAMICTL_OFF 10 +#define TRAMOCTL_MSK (3 << TRAMOCTL_OFF) +#define TRAMICTL_MSK BIT(TRAMICTL_OFF) +/* data ram */ +#define DRAMOCTL_OFF 11 +#define DRAMICTL_OFF 13 +#define DRAMOCTL_MSK (3 << DRAMOCTL_OFF) +#define DRAMICTL_MSK BIT(DRAMICTL_OFF)
+/* CCTL Command Register */ +#define CCTL_CMD_REG(base, hart) ((ulong)(base) + 0x40 + (hart) * 0x10) +#define L2_WBINVAL_ALL 0x12
+/* CCTL Status Register */ +#define CCTL_STATUS_MSK(hart) (0xf << ((hart) * 4)) +#define CCTL_STATUS_IDLE(hart) (0 << ((hart) * 4)) +#define CCTL_STATUS_PROCESS(hart) (1 << ((hart) * 4)) +#define CCTL_STATUS_ILLEGAL(hart) (2 << ((hart) * 4))
+void v5l2_enable(void); +void v5l2_disable(void);
+#endif /* _ASM_V5_L2CACHE_H */ diff --git a/drivers/cache/Kconfig b/drivers/cache/Kconfig index 24def7a..665689a 100644 --- a/drivers/cache/Kconfig +++ b/drivers/cache/Kconfig @@ -22,4 +22,13 @@ config L2X0_CACHE ARMv7(32-bit) devices. The driver configures the cache settings found in the device tree.
+config V5L2_CACHE
tristate "Andes V5L2 cache driver"
This should be bool. U-Boot does not support "tristate"
I will modify it as bool.
select CACHE
depends on RISCV_NDS_CACHE
help
Support Andes V5L2 cache controller in AE350 platform.
It will configure tag and data ram timing control from the
device tree and enable L2 cache.
endmenu diff --git a/drivers/cache/Makefile b/drivers/cache/Makefile index 9deb961..4a6458c 100644 --- a/drivers/cache/Makefile +++ b/drivers/cache/Makefile @@ -2,3 +2,4 @@ obj-$(CONFIG_CACHE) += cache-uclass.o obj-$(CONFIG_SANDBOX) += sandbox_cache.o obj-$(CONFIG_L2X0_CACHE) += cache-l2x0.o +obj-$(CONFIG_V5L2_CACHE) += cache-v5l2.o diff --git a/drivers/cache/cache-v5l2.c b/drivers/cache/cache-v5l2.c new file mode 100644 index 0000000..7022feb --- /dev/null +++ b/drivers/cache/cache-v5l2.c @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright (C) 2019 Andes Technology Corporation
- Rick Chen, Andes Technology Corporation rick@andestech.com
- */
+#include <common.h> +#include <command.h> +#include <dm.h> +#include <asm/io.h> +#include <dm/ofnode.h> +#include <asm/v5l2cache.h>
+DECLARE_GLOBAL_DATA_PTR;
+void v5l2_enable(void)
This should really be avoided. It looks current DM cache uclass driver is lacking of ops to do cache enable/disable. Please improve the DM cache uclass driver first.
OK I will improve the DM cache uclass driver.
+{
struct l2cache *regs = gd->arch.v5l2;
if (regs)
setbits_le32(®s->control, L2_ENABLE);
+}
+void v5l2_disable(void) +{
volatile struct l2cache *regs = gd->arch.v5l2;
u8 hart = gd->arch.boot_hart;
void __iomem *cctlcmd = (void __iomem *)CCTL_CMD_REG(gd->arch.v5l2, hart);
if ((regs) && (readl(®s->control) & L2_ENABLE)) {
writel(L2_WBINVAL_ALL, cctlcmd);
while ((readl(®s->cctl_status) & CCTL_STATUS_MSK(hart))) {
if ((readl(®s->cctl_status) & CCTL_STATUS_ILLEGAL(hart))) {
printf("L2 flush illegal! hanging...");
hang();
}
}
clrbits_le32(®s->control, L2_ENABLE);
}
+}
+static void v5l2_of_parse_and_init(struct udevice *dev)
The entire function below should really be created as the driver's ofdata_to_platdata() function, inside which all DT properties are parsed and saved to driver's platdata. After that, there is no need to get register base from global data.
I will use ofdata_to_platdata() to parse dtb information and save it into platdata instead of saving in global data.
+{
struct l2cache *regs;
u32 ctl_val, prefetch;
u32 tram_ctl[2];
u32 dram_ctl[2];
regs = (struct l2cache *)dev_read_addr(dev);
gd->arch.v5l2 = regs;
ctl_val = readl(®s->control);
if (!(ctl_val & L2_ENABLE))
ctl_val |= L2_ENABLE;
/* Instruction and data fetch prefetch depth */
if (!dev_read_u32(dev, "andes,inst-prefetch", &prefetch)) {
ctl_val &= ~(IPREPETCH_MSK);
ctl_val |= (prefetch << IPREPETCH_OFF);
}
if (!dev_read_u32(dev, "andes,data-prefetch", &prefetch)) {
ctl_val &= ~(DPREPETCH_MSK);
ctl_val |= (prefetch << DPREPETCH_OFF);
}
/* Set tag RAM and data RAM setup and output cycle */
if (!dev_read_u32_array(dev, "andes,tag-ram-ctl", tram_ctl, 2)) {
ctl_val &= ~(TRAMOCTL_MSK | TRAMICTL_MSK);
ctl_val |= tram_ctl[0] << TRAMOCTL_OFF;
ctl_val |= tram_ctl[1] << TRAMICTL_OFF;
}
if (!dev_read_u32_array(dev, "andes,data-ram-ctl", dram_ctl, 2)) {
ctl_val &= ~(DRAMOCTL_MSK | DRAMICTL_MSK);
ctl_val |= dram_ctl[0] << DRAMOCTL_OFF;
ctl_val |= dram_ctl[1] << DRAMICTL_OFF;
}
writel(ctl_val, ®s->control);
+}
+static int v5l2_probe(struct udevice *dev) +{
v5l2_of_parse_and_init(dev);
return 0;
+}
+static const struct udevice_id v5l2_cache_ids[] = {
{ .compatible = "cache" },
I suspect this compatible string is too generic. Has this been reviewed by DT community upstream?
It refer to many dts examples from arm, For example : arch/arm/dts/fsl-imx8-ca35.dtsi A35_L2: l2-cache0 { compatible = "cache"; };
None of these have a driver for the cache controller, which is why it is sufficient to just use a generic compatible string.
I agree with Bin that you should choose a more specific compatible string. This is likely to cause problems in the future otherwise. For example, if Andes develops a new L2 cache controller, it must be differentiated from this one using the compatible string. The new controller would therefore have to use a different compatible string. It is good practice to already use a more specific one now to avoid the headache later. :)
You are right. I will modify it as compatible = "v5l2cache"
Before you do that, could you please check whether this L2 cache driver will be supported in the Linux kernel too? If yes, I think you need go through the kernel upstream process, during which the compatible string is to be discussed and approved.
No, we only support this L2 cache in U-Boot about upstream.
Rick
Regards, Bin

From: Rick Chen rick@andestech.com
Find the UCLASS_CACHE driver to configure the cache controller's settings.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com --- board/AndesTech/ax25-ae350/ax25-ae350.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c b/board/AndesTech/ax25-ae350/ax25-ae350.c index 3d65ce7..686ec4a 100644 --- a/board/AndesTech/ax25-ae350/ax25-ae350.c +++ b/board/AndesTech/ax25-ae350/ax25-ae350.c @@ -11,6 +11,7 @@ #include <linux/io.h> #include <faraday/ftsmc020.h> #include <fdtdec.h> +#include <dm.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -93,10 +94,24 @@ int smc_init(void) return 0; }
+int v5l2_init(void) +{ + struct udevice *dev; + int ret; + + ret = uclass_get_device(UCLASS_CACHE, 0, &dev); + + if (ret) + return ret; + + return 0; +} + #ifdef CONFIG_BOARD_EARLY_INIT_F int board_early_init_f(void) { smc_init(); + v5l2_init();
return 0; }

Hi Rick,
On Tue, May 28, 2019 at 5:44 PM Andes uboot@andestech.com wrote:
From: Rick Chen rick@andestech.com
Find the UCLASS_CACHE driver to configure the cache controller's settings.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com
board/AndesTech/ax25-ae350/ax25-ae350.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c b/board/AndesTech/ax25-ae350/ax25-ae350.c index 3d65ce7..686ec4a 100644 --- a/board/AndesTech/ax25-ae350/ax25-ae350.c +++ b/board/AndesTech/ax25-ae350/ax25-ae350.c @@ -11,6 +11,7 @@ #include <linux/io.h> #include <faraday/ftsmc020.h> #include <fdtdec.h> +#include <dm.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -93,10 +94,24 @@ int smc_init(void) return 0; }
+int v5l2_init(void) +{
struct udevice *dev;
int ret;
ret = uclass_get_device(UCLASS_CACHE, 0, &dev);
if (ret)
return ret;
return 0;
+}
#ifdef CONFIG_BOARD_EARLY_INIT_F int board_early_init_f(void) { smc_init();
v5l2_init();
Please check the return value here, or you can make v512_init() returns void.
return 0;
}
Regards, Bin

Bin Meng bmeng.cn@gmail.com 於 2019年6月4日 週二 上午10:48寫道:
Hi Rick,
On Tue, May 28, 2019 at 5:44 PM Andes uboot@andestech.com wrote:
From: Rick Chen rick@andestech.com
Find the UCLASS_CACHE driver to configure the cache controller's settings.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com
board/AndesTech/ax25-ae350/ax25-ae350.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c b/board/AndesTech/ax25-ae350/ax25-ae350.c index 3d65ce7..686ec4a 100644 --- a/board/AndesTech/ax25-ae350/ax25-ae350.c +++ b/board/AndesTech/ax25-ae350/ax25-ae350.c @@ -11,6 +11,7 @@ #include <linux/io.h> #include <faraday/ftsmc020.h> #include <fdtdec.h> +#include <dm.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -93,10 +94,24 @@ int smc_init(void) return 0; }
+int v5l2_init(void) +{
struct udevice *dev;
int ret;
ret = uclass_get_device(UCLASS_CACHE, 0, &dev);
if (ret)
return ret;
return 0;
+}
#ifdef CONFIG_BOARD_EARLY_INIT_F int board_early_init_f(void) { smc_init();
v5l2_init();
Please check the return value here, or you can make v512_init() returns void.
return 0;
}
Regards, Bin

Hi Bin
Bin Meng bmeng.cn@gmail.com 於 2019年6月4日 週二 上午10:48寫道:
Hi Rick,
On Tue, May 28, 2019 at 5:44 PM Andes uboot@andestech.com wrote:
From: Rick Chen rick@andestech.com
Find the UCLASS_CACHE driver to configure the cache controller's settings.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com
board/AndesTech/ax25-ae350/ax25-ae350.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c b/board/AndesTech/ax25-ae350/ax25-ae350.c index 3d65ce7..686ec4a 100644 --- a/board/AndesTech/ax25-ae350/ax25-ae350.c +++ b/board/AndesTech/ax25-ae350/ax25-ae350.c @@ -11,6 +11,7 @@ #include <linux/io.h> #include <faraday/ftsmc020.h> #include <fdtdec.h> +#include <dm.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -93,10 +94,24 @@ int smc_init(void) return 0; }
+int v5l2_init(void) +{
struct udevice *dev;
int ret;
ret = uclass_get_device(UCLASS_CACHE, 0, &dev);
if (ret)
return ret;
return 0;
+}
#ifdef CONFIG_BOARD_EARLY_INIT_F int board_early_init_f(void) { smc_init();
v5l2_init();
Please check the return value here, or you can make v512_init() returns void.
OK. I will check the return value here.
return 0;
}
Regards, Bin

From: Rick Chen rick@andestech.com
Select the v5l2 UCLASS_CACHE driver for AE350.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com --- board/AndesTech/ax25-ae350/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/board/AndesTech/ax25-ae350/Kconfig b/board/AndesTech/ax25-ae350/Kconfig index 5e682b6..dd299d9 100644 --- a/board/AndesTech/ax25-ae350/Kconfig +++ b/board/AndesTech/ax25-ae350/Kconfig @@ -25,5 +25,6 @@ config BOARD_SPECIFIC_OPTIONS # dummy def_bool y select RISCV_NDS imply SMP + imply V5L2_CACHE
endif

Hi Rick,
On Tue, May 28, 2019 at 5:44 PM Andes uboot@andestech.com wrote:
From: Rick Chen rick@andestech.com
Select the v5l2 UCLASS_CACHE driver for AE350.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com
board/AndesTech/ax25-ae350/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/board/AndesTech/ax25-ae350/Kconfig b/board/AndesTech/ax25-ae350/Kconfig index 5e682b6..dd299d9 100644 --- a/board/AndesTech/ax25-ae350/Kconfig +++ b/board/AndesTech/ax25-ae350/Kconfig @@ -25,5 +25,6 @@ config BOARD_SPECIFIC_OPTIONS # dummy def_bool y select RISCV_NDS imply SMP
imply V5L2_CACHE
I believe L2 cache is a CPU specific feature, hence this should be implied from arch/riscv/cpu/ax25/Kconfig
Regards, Bin

Hi Bin
Hi Rick,
On Tue, May 28, 2019 at 5:44 PM Andes uboot@andestech.com wrote:
From: Rick Chen rick@andestech.com
Select the v5l2 UCLASS_CACHE driver for AE350.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com
board/AndesTech/ax25-ae350/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/board/AndesTech/ax25-ae350/Kconfig b/board/AndesTech/ax25-ae350/Kconfig index 5e682b6..dd299d9 100644 --- a/board/AndesTech/ax25-ae350/Kconfig +++ b/board/AndesTech/ax25-ae350/Kconfig @@ -25,5 +25,6 @@ config BOARD_SPECIFIC_OPTIONS # dummy def_bool y select RISCV_NDS imply SMP
imply V5L2_CACHE
I believe L2 cache is a CPU specific feature, hence this should be implied from arch/riscv/cpu/ax25/Kconfig
OK I will move it to arch/riscv/cpu/ax25/Kconfig
Thanks Rick
Regards, Bin

From: Rick Chen rick@andestech.com
Flush and disable cache in cleanup_before_linux() which will be called before jump to linux.
The sequence will be preferred as below: L1 flush -> L1 disable -> L2 flush -> L2 disable
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com --- arch/riscv/cpu/ax25/cpu.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/riscv/cpu/ax25/cpu.c b/arch/riscv/cpu/ax25/cpu.c index 76689b2..9e7579a 100644 --- a/arch/riscv/cpu/ax25/cpu.c +++ b/arch/riscv/cpu/ax25/cpu.c @@ -7,6 +7,7 @@ /* CPU specific code */ #include <common.h> #include <asm/cache.h> +#include <asm/v5l2cache.h>
/* * cleanup_before_linux() is called just before we call linux @@ -22,6 +23,9 @@ int cleanup_before_linux(void) cache_flush(); icache_disable(); dcache_disable(); +#ifdef CONFIG_RISCV_NDS_CACHE + v5l2_disable(); +#endif
return 0; }

Hi Rick,
On Tue, May 28, 2019 at 5:44 PM Andes uboot@andestech.com wrote:
From: Rick Chen rick@andestech.com
Flush and disable cache in cleanup_before_linux() which will be called before jump to linux.
The sequence will be preferred as below: L1 flush -> L1 disable -> L2 flush -> L2 disable
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com
arch/riscv/cpu/ax25/cpu.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/riscv/cpu/ax25/cpu.c b/arch/riscv/cpu/ax25/cpu.c index 76689b2..9e7579a 100644 --- a/arch/riscv/cpu/ax25/cpu.c +++ b/arch/riscv/cpu/ax25/cpu.c @@ -7,6 +7,7 @@ /* CPU specific code */ #include <common.h> #include <asm/cache.h> +#include <asm/v5l2cache.h>
/*
- cleanup_before_linux() is called just before we call linux
@@ -22,6 +23,9 @@ int cleanup_before_linux(void) cache_flush(); icache_disable(); dcache_disable(); +#ifdef CONFIG_RISCV_NDS_CACHE
v5l2_disable();
+#endif
The direct call into a driver should be avoided. Instead, use a proper DM cache uclass driver API (see my review comments in patch [1/6])
Regards, Bin

Hi Bin
Hi Rick,
On Tue, May 28, 2019 at 5:44 PM Andes uboot@andestech.com wrote:
From: Rick Chen rick@andestech.com
Flush and disable cache in cleanup_before_linux() which will be called before jump to linux.
The sequence will be preferred as below: L1 flush -> L1 disable -> L2 flush -> L2 disable
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com
arch/riscv/cpu/ax25/cpu.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/riscv/cpu/ax25/cpu.c b/arch/riscv/cpu/ax25/cpu.c index 76689b2..9e7579a 100644 --- a/arch/riscv/cpu/ax25/cpu.c +++ b/arch/riscv/cpu/ax25/cpu.c @@ -7,6 +7,7 @@ /* CPU specific code */ #include <common.h> #include <asm/cache.h> +#include <asm/v5l2cache.h>
/*
- cleanup_before_linux() is called just before we call linux
@@ -22,6 +23,9 @@ int cleanup_before_linux(void) cache_flush(); icache_disable(); dcache_disable(); +#ifdef CONFIG_RISCV_NDS_CACHE
v5l2_disable();
+#endif
The direct call into a driver should be avoided. Instead, use a proper DM cache uclass driver API (see my review comments in patch [1/6])
OK I will use DM cache uclass driver API to disable L2 driver.
Thanks Rick
Regards, Bin

From: Rick Chen rick@andestech.com
When L2 node exists inside cpus node, uclass_get_device can not parse L2 node successfully. So move it outside from cpus node.
Also add tag-ram-ctl and data-ram-ctl attributes for v5l2 cache controller driver. This can adjust timing by requirement from dtb to improve performance.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com --- arch/riscv/dts/ae350_32.dts | 17 +++++++++++------ arch/riscv/dts/ae350_64.dts | 17 +++++++++++------ 2 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/arch/riscv/dts/ae350_32.dts b/arch/riscv/dts/ae350_32.dts index cb6ee13..83abfcb 100644 --- a/arch/riscv/dts/ae350_32.dts +++ b/arch/riscv/dts/ae350_32.dts @@ -62,13 +62,18 @@ compatible = "riscv,cpu-intc"; }; }; + };
- L2: l2-cache@e0500000 { - compatible = "cache"; - cache-level = <2>; - cache-size = <0x40000>; - reg = <0x0 0xe0500000 0x0 0x40000>; - }; + L2: l2-cache@e0500000 { + compatible = "cache"; + cache-level = <2>; + cache-size = <0x40000>; + reg = <0xe0500000 0x40000>; + andes,inst-prefetch = <3>; + andes,data-prefetch = <3>; + // The value format is <XRAMOCTL XRAMICTL> + andes,tag-ram-ctl = <0 0>; + andes,data-ram-ctl = <0 0>; };
memory@0 { diff --git a/arch/riscv/dts/ae350_64.dts b/arch/riscv/dts/ae350_64.dts index 705491a..7009bdc 100644 --- a/arch/riscv/dts/ae350_64.dts +++ b/arch/riscv/dts/ae350_64.dts @@ -62,13 +62,18 @@ compatible = "riscv,cpu-intc"; }; }; + };
- L2: l2-cache@e0500000 { - compatible = "cache"; - cache-level = <2>; - cache-size = <0x40000>; - reg = <0x0 0xe0500000 0x0 0x40000>; - }; + L2: l2-cache@e0500000 { + compatible = "cache"; + cache-level = <2>; + cache-size = <0x40000>; + reg = <0x0 0xe0500000 0x0 0x40000>; + andes,inst-prefetch = <3>; + andes,data-prefetch = <3>; + // The value format is <XRAMOCTL XRAMICTL> + andes,tag-ram-ctl = <0 0>; + andes,data-ram-ctl = <0 0>; };
memory@0 {

Hi Rick,
On Tue, May 28, 2019 at 5:44 PM Andes uboot@andestech.com wrote:
From: Rick Chen rick@andestech.com
When L2 node exists inside cpus node, uclass_get_device can not parse L2 node successfully. So move it outside from cpus node.
Also add tag-ram-ctl and data-ram-ctl attributes for v5l2 cache controller driver. This can adjust timing by requirement from dtb to improve performance.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com
arch/riscv/dts/ae350_32.dts | 17 +++++++++++------ arch/riscv/dts/ae350_64.dts | 17 +++++++++++------ 2 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/arch/riscv/dts/ae350_32.dts b/arch/riscv/dts/ae350_32.dts index cb6ee13..83abfcb 100644 --- a/arch/riscv/dts/ae350_32.dts +++ b/arch/riscv/dts/ae350_32.dts @@ -62,13 +62,18 @@ compatible = "riscv,cpu-intc"; }; };
};
L2: l2-cache@e0500000 {
compatible = "cache";
cache-level = <2>;
cache-size = <0x40000>;
reg = <0x0 0xe0500000 0x0 0x40000>;
};
L2: l2-cache@e0500000 {
compatible = "cache";
too generic compatible string (see my previous comments in patch [1/6])
cache-level = <2>;
cache-size = <0x40000>;
reg = <0xe0500000 0x40000>;
andes,inst-prefetch = <3>;
andes,data-prefetch = <3>;
// The value format is <XRAMOCTL XRAMICTL>
nits: no //, use /* */
andes,tag-ram-ctl = <0 0>;
andes,data-ram-ctl = <0 0>; }; memory@0 {
diff --git a/arch/riscv/dts/ae350_64.dts b/arch/riscv/dts/ae350_64.dts index 705491a..7009bdc 100644 --- a/arch/riscv/dts/ae350_64.dts +++ b/arch/riscv/dts/ae350_64.dts @@ -62,13 +62,18 @@ compatible = "riscv,cpu-intc"; }; };
};
L2: l2-cache@e0500000 {
compatible = "cache";
cache-level = <2>;
cache-size = <0x40000>;
reg = <0x0 0xe0500000 0x0 0x40000>;
};
L2: l2-cache@e0500000 {
compatible = "cache";
cache-level = <2>;
cache-size = <0x40000>;
reg = <0x0 0xe0500000 0x0 0x40000>;
andes,inst-prefetch = <3>;
andes,data-prefetch = <3>;
// The value format is <XRAMOCTL XRAMICTL>
nits: no //, use /* */
andes,tag-ram-ctl = <0 0>;
andes,data-ram-ctl = <0 0>; }; memory@0 {
--
Regards, Bin

Hi Bin
Bin Meng bmeng.cn@gmail.com 於 2019年6月4日 週二 上午10:48寫道:
Hi Rick,
On Tue, May 28, 2019 at 5:44 PM Andes uboot@andestech.com wrote:
From: Rick Chen rick@andestech.com
When L2 node exists inside cpus node, uclass_get_device can not parse L2 node successfully. So move it outside from cpus node.
Also add tag-ram-ctl and data-ram-ctl attributes for v5l2 cache controller driver. This can adjust timing by requirement from dtb to improve performance.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com
arch/riscv/dts/ae350_32.dts | 17 +++++++++++------ arch/riscv/dts/ae350_64.dts | 17 +++++++++++------ 2 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/arch/riscv/dts/ae350_32.dts b/arch/riscv/dts/ae350_32.dts index cb6ee13..83abfcb 100644 --- a/arch/riscv/dts/ae350_32.dts +++ b/arch/riscv/dts/ae350_32.dts @@ -62,13 +62,18 @@ compatible = "riscv,cpu-intc"; }; };
};
L2: l2-cache@e0500000 {
compatible = "cache";
cache-level = <2>;
cache-size = <0x40000>;
reg = <0x0 0xe0500000 0x0 0x40000>;
};
L2: l2-cache@e0500000 {
compatible = "cache";
too generic compatible string (see my previous comments in patch [1/6])
Same replying in patch [1/6]
cache-level = <2>;
cache-size = <0x40000>;
reg = <0xe0500000 0x40000>;
andes,inst-prefetch = <3>;
andes,data-prefetch = <3>;
// The value format is <XRAMOCTL XRAMICTL>
nits: no //, use /* */
OK I will use /* */ instead of //
andes,tag-ram-ctl = <0 0>;
andes,data-ram-ctl = <0 0>; }; memory@0 {
diff --git a/arch/riscv/dts/ae350_64.dts b/arch/riscv/dts/ae350_64.dts index 705491a..7009bdc 100644 --- a/arch/riscv/dts/ae350_64.dts +++ b/arch/riscv/dts/ae350_64.dts @@ -62,13 +62,18 @@ compatible = "riscv,cpu-intc"; }; };
};
L2: l2-cache@e0500000 {
compatible = "cache";
cache-level = <2>;
cache-size = <0x40000>;
reg = <0x0 0xe0500000 0x0 0x40000>;
};
L2: l2-cache@e0500000 {
compatible = "cache";
cache-level = <2>;
cache-size = <0x40000>;
reg = <0x0 0xe0500000 0x0 0x40000>;
andes,inst-prefetch = <3>;
andes,data-prefetch = <3>;
// The value format is <XRAMOCTL XRAMICTL>
nits: no //, use /* */
I will use /* */ instead of //
Thanks Rick
andes,tag-ram-ctl = <0 0>;
andes,data-ram-ctl = <0 0>; }; memory@0 {
--
Regards, Bin

From: Rick Chen rick@andestech.com
Use CCTL command to do d-cache write back and invalidate instead of fence.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com --- arch/riscv/cpu/ax25/cache.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/arch/riscv/cpu/ax25/cache.c b/arch/riscv/cpu/ax25/cache.c index 228fc55..d30071e 100644 --- a/arch/riscv/cpu/ax25/cache.c +++ b/arch/riscv/cpu/ax25/cache.c @@ -5,17 +5,21 @@ */
#include <common.h> +#include <asm/csr.h> + +#ifdef CONFIG_RISCV_NDS_CACHE +/* mcctlcommand */ +#define CCTL_REG_MCCTLCOMMAND_NUM 0x7cc + +/* D-cache operation */ +#define CCTL_L1D_WBINVAL_ALL 6 +#endif
void flush_dcache_all(void) { - /* - * Andes' AX25 does not have a coherence agent. U-Boot must use data - * cache flush and invalidate functions to keep data in the system - * coherent. - * The implementation of the fence instruction in the AX25 flushes the - * data cache and is used for this purpose. - */ - asm volatile ("fence" ::: "memory"); +#ifdef CONFIG_RISCV_NDS_CACHE + csr_write(CCTL_REG_MCCTLCOMMAND_NUM, CCTL_L1D_WBINVAL_ALL); +#endif }
void flush_dcache_range(unsigned long start, unsigned long end) @@ -72,8 +76,8 @@ void dcache_disable(void) { #ifndef CONFIG_SYS_DCACHE_OFF #ifdef CONFIG_RISCV_NDS_CACHE + csr_write(CCTL_REG_MCCTLCOMMAND_NUM, CCTL_L1D_WBINVAL_ALL); asm volatile ( - "fence\n\t" "csrr t1, mcache_ctl\n\t" "andi t0, t1, ~0x2\n\t" "csrw mcache_ctl, t0\n\t"

Hi Rick,
On Tue, May 28, 2019 at 5:45 PM Andes uboot@andestech.com wrote:
From: Rick Chen rick@andestech.com
Use CCTL command to do d-cache write back and invalidate instead of fence.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com
arch/riscv/cpu/ax25/cache.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/arch/riscv/cpu/ax25/cache.c b/arch/riscv/cpu/ax25/cache.c index 228fc55..d30071e 100644 --- a/arch/riscv/cpu/ax25/cache.c +++ b/arch/riscv/cpu/ax25/cache.c @@ -5,17 +5,21 @@ */
#include <common.h> +#include <asm/csr.h>
+#ifdef CONFIG_RISCV_NDS_CACHE +/* mcctlcommand */ +#define CCTL_REG_MCCTLCOMMAND_NUM 0x7cc
+/* D-cache operation */ +#define CCTL_L1D_WBINVAL_ALL 6 +#endif
void flush_dcache_all(void) {
/*
* Andes' AX25 does not have a coherence agent. U-Boot must use data
* cache flush and invalidate functions to keep data in the system
* coherent.
* The implementation of the fence instruction in the AX25 flushes the
* data cache and is used for this purpose.
*/
asm volatile ("fence" ::: "memory");
+#ifdef CONFIG_RISCV_NDS_CACHE
csr_write(CCTL_REG_MCCTLCOMMAND_NUM, CCTL_L1D_WBINVAL_ALL);
I think CCTL_REG_MCCTLCOMMAND_NUM is a vendor specific CSR. Does upstream GCC support this CSR?
+#endif }
void flush_dcache_range(unsigned long start, unsigned long end) @@ -72,8 +76,8 @@ void dcache_disable(void) { #ifndef CONFIG_SYS_DCACHE_OFF #ifdef CONFIG_RISCV_NDS_CACHE
csr_write(CCTL_REG_MCCTLCOMMAND_NUM, CCTL_L1D_WBINVAL_ALL); asm volatile (
"fence\n\t" "csrr t1, mcache_ctl\n\t" "andi t0, t1, ~0x2\n\t" "csrw mcache_ctl, t0\n\t"
--
Regards, Bin

Hi Bin
Hi Rick,
On Tue, May 28, 2019 at 5:45 PM Andes uboot@andestech.com wrote:
From: Rick Chen rick@andestech.com
Use CCTL command to do d-cache write back and invalidate instead of fence.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com
arch/riscv/cpu/ax25/cache.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/arch/riscv/cpu/ax25/cache.c b/arch/riscv/cpu/ax25/cache.c index 228fc55..d30071e 100644 --- a/arch/riscv/cpu/ax25/cache.c +++ b/arch/riscv/cpu/ax25/cache.c @@ -5,17 +5,21 @@ */
#include <common.h> +#include <asm/csr.h>
+#ifdef CONFIG_RISCV_NDS_CACHE +/* mcctlcommand */ +#define CCTL_REG_MCCTLCOMMAND_NUM 0x7cc
+/* D-cache operation */ +#define CCTL_L1D_WBINVAL_ALL 6 +#endif
void flush_dcache_all(void) {
/*
* Andes' AX25 does not have a coherence agent. U-Boot must use data
* cache flush and invalidate functions to keep data in the system
* coherent.
* The implementation of the fence instruction in the AX25 flushes the
* data cache and is used for this purpose.
*/
asm volatile ("fence" ::: "memory");
+#ifdef CONFIG_RISCV_NDS_CACHE
csr_write(CCTL_REG_MCCTLCOMMAND_NUM, CCTL_L1D_WBINVAL_ALL);
I think CCTL_REG_MCCTLCOMMAND_NUM is a vendor specific CSR. Does upstream GCC support this CSR?
Yes. It is a vendor specific CSR. Upstream GCC shall not support it. So I isolate it by CONFIG_RISCV_NDS_CACHE.
Thanks Rick
+#endif }
void flush_dcache_range(unsigned long start, unsigned long end) @@ -72,8 +76,8 @@ void dcache_disable(void) { #ifndef CONFIG_SYS_DCACHE_OFF #ifdef CONFIG_RISCV_NDS_CACHE
csr_write(CCTL_REG_MCCTLCOMMAND_NUM, CCTL_L1D_WBINVAL_ALL); asm volatile (
"fence\n\t" "csrr t1, mcache_ctl\n\t" "andi t0, t1, ~0x2\n\t" "csrw mcache_ctl, t0\n\t"
--
Regards, Bin

Hi Rick,
On Wed, Jun 5, 2019 at 5:38 PM Rick Chen rickchen36@gmail.com wrote:
Hi Bin
Hi Rick,
On Tue, May 28, 2019 at 5:45 PM Andes uboot@andestech.com wrote:
From: Rick Chen rick@andestech.com
Use CCTL command to do d-cache write back and invalidate instead of fence.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com
arch/riscv/cpu/ax25/cache.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/arch/riscv/cpu/ax25/cache.c b/arch/riscv/cpu/ax25/cache.c index 228fc55..d30071e 100644 --- a/arch/riscv/cpu/ax25/cache.c +++ b/arch/riscv/cpu/ax25/cache.c @@ -5,17 +5,21 @@ */
#include <common.h> +#include <asm/csr.h>
+#ifdef CONFIG_RISCV_NDS_CACHE +/* mcctlcommand */ +#define CCTL_REG_MCCTLCOMMAND_NUM 0x7cc
+/* D-cache operation */ +#define CCTL_L1D_WBINVAL_ALL 6 +#endif
void flush_dcache_all(void) {
/*
* Andes' AX25 does not have a coherence agent. U-Boot must use data
* cache flush and invalidate functions to keep data in the system
* coherent.
* The implementation of the fence instruction in the AX25 flushes the
* data cache and is used for this purpose.
*/
asm volatile ("fence" ::: "memory");
+#ifdef CONFIG_RISCV_NDS_CACHE
csr_write(CCTL_REG_MCCTLCOMMAND_NUM, CCTL_L1D_WBINVAL_ALL);
I think CCTL_REG_MCCTLCOMMAND_NUM is a vendor specific CSR. Does upstream GCC support this CSR?
Yes. It is a vendor specific CSR. Upstream GCC shall not support it. So I isolate it by CONFIG_RISCV_NDS_CACHE.
OK, but I suspect you will need do something for the travis build since upstream gcc is used?
Regards, Bin

Hi Rick,
On Wed, 2019-06-05 at 17:39 +0800, Bin Meng wrote:
Hi Rick,
On Wed, Jun 5, 2019 at 5:38 PM Rick Chen rickchen36@gmail.com wrote:
Hi Bin
Hi Rick,
On Tue, May 28, 2019 at 5:45 PM Andes uboot@andestech.com wrote:
From: Rick Chen rick@andestech.com
Use CCTL command to do d-cache write back and invalidate instead of fence.
Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com
arch/riscv/cpu/ax25/cache.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/arch/riscv/cpu/ax25/cache.c b/arch/riscv/cpu/ax25/cache.c index 228fc55..d30071e 100644 --- a/arch/riscv/cpu/ax25/cache.c +++ b/arch/riscv/cpu/ax25/cache.c @@ -5,17 +5,21 @@ */
#include <common.h> +#include <asm/csr.h>
+#ifdef CONFIG_RISCV_NDS_CACHE +/* mcctlcommand */ +#define CCTL_REG_MCCTLCOMMAND_NUM 0x7cc
+/* D-cache operation */ +#define CCTL_L1D_WBINVAL_ALL 6 +#endif
void flush_dcache_all(void) {
/*
* Andes' AX25 does not have a coherence agent. U-Boot must use data
* cache flush and invalidate functions to keep data in the system
* coherent.
* The implementation of the fence instruction in the AX25 flushes the
* data cache and is used for this purpose.
*/
asm volatile ("fence" ::: "memory");
+#ifdef CONFIG_RISCV_NDS_CACHE
csr_write(CCTL_REG_MCCTLCOMMAND_NUM, CCTL_L1D_WBINVAL_ALL);
I think CCTL_REG_MCCTLCOMMAND_NUM is a vendor specific CSR. Does upstream GCC support this CSR?
Yes. It is a vendor specific CSR. Upstream GCC shall not support it. So I isolate it by CONFIG_RISCV_NDS_CACHE.
OK, but I suspect you will need do something for the travis build since upstream gcc is used?
How about using the CSR address instead of the name? This way the board can be built in Travis.
Thanks, Lukas
participants (4)
-
Andes
-
Auer, Lukas
-
Bin Meng
-
Rick Chen