* [U-Boot] [PATCH 0/6] Support Andes RISC-V l2cache on AE350 platform @ 2019-05-28 9:39 Andes 2019-05-28 9:39 ` [U-Boot] [PATCH 1/6] dm: cache: add v5l2 cache controller driver Andes ` (5 more replies) 0 siblings, 6 replies; 26+ messages in thread From: Andes @ 2019-05-28 9:39 UTC (permalink / raw) To: u-boot 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 -- 2.7.4 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 1/6] dm: cache: add v5l2 cache controller driver 2019-05-28 9:39 [U-Boot] [PATCH 0/6] Support Andes RISC-V l2cache on AE350 platform Andes @ 2019-05-28 9:39 ` Andes 2019-06-04 2:48 ` Bin Meng 2019-05-28 9:39 ` [U-Boot] [PATCH 2/6] riscv: ae350: use the v5l2 driver to configure the cache Andes ` (4 subsequent siblings) 5 siblings, 1 reply; 26+ messages in thread From: Andes @ 2019-05-28 9:39 UTC (permalink / raw) To: u-boot 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, +}; -- 2.7.4 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 1/6] dm: cache: add v5l2 cache controller driver 2019-05-28 9:39 ` [U-Boot] [PATCH 1/6] dm: cache: add v5l2 cache controller driver Andes @ 2019-06-04 2:48 ` Bin Meng 2019-06-05 8:58 ` Rick Chen 0 siblings, 1 reply; 26+ messages in thread From: Bin Meng @ 2019-06-04 2:48 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 1/6] dm: cache: add v5l2 cache controller driver 2019-06-04 2:48 ` Bin Meng @ 2019-06-05 8:58 ` Rick Chen 2019-06-09 17:56 ` Auer, Lukas 0 siblings, 1 reply; 26+ messages in thread From: Rick Chen @ 2019-06-05 8:58 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 1/6] dm: cache: add v5l2 cache controller driver 2019-06-05 8:58 ` Rick Chen @ 2019-06-09 17:56 ` Auer, Lukas 2019-06-10 2:26 ` Rick Chen 0 siblings, 1 reply; 26+ messages in thread From: Auer, Lukas @ 2019-06-09 17:56 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 1/6] dm: cache: add v5l2 cache controller driver 2019-06-09 17:56 ` Auer, Lukas @ 2019-06-10 2:26 ` Rick Chen 2019-06-10 2:32 ` Bin Meng 0 siblings, 1 reply; 26+ messages in thread From: Rick Chen @ 2019-06-10 2:26 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 1/6] dm: cache: add v5l2 cache controller driver 2019-06-10 2:26 ` Rick Chen @ 2019-06-10 2:32 ` Bin Meng 2019-06-12 6:32 ` Rick Chen 0 siblings, 1 reply; 26+ messages in thread From: Bin Meng @ 2019-06-10 2:32 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 1/6] dm: cache: add v5l2 cache controller driver 2019-06-10 2:32 ` Bin Meng @ 2019-06-12 6:32 ` Rick Chen 0 siblings, 0 replies; 26+ messages in thread From: Rick Chen @ 2019-06-12 6:32 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 2/6] riscv: ae350: use the v5l2 driver to configure the cache 2019-05-28 9:39 [U-Boot] [PATCH 0/6] Support Andes RISC-V l2cache on AE350 platform Andes 2019-05-28 9:39 ` [U-Boot] [PATCH 1/6] dm: cache: add v5l2 cache controller driver Andes @ 2019-05-28 9:39 ` Andes 2019-06-04 2:48 ` Bin Meng 2019-05-28 9:39 ` [U-Boot] [PATCH 3/6] riscv: ae350: add imply v5l2 cache controller Andes ` (3 subsequent siblings) 5 siblings, 1 reply; 26+ messages in thread From: Andes @ 2019-05-28 9:39 UTC (permalink / raw) To: u-boot 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; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 2/6] riscv: ae350: use the v5l2 driver to configure the cache 2019-05-28 9:39 ` [U-Boot] [PATCH 2/6] riscv: ae350: use the v5l2 driver to configure the cache Andes @ 2019-06-04 2:48 ` Bin Meng 2019-06-05 9:02 ` Rick Chen 2019-06-05 9:04 ` Rick Chen 0 siblings, 2 replies; 26+ messages in thread From: Bin Meng @ 2019-06-04 2:48 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 2/6] riscv: ae350: use the v5l2 driver to configure the cache 2019-06-04 2:48 ` Bin Meng @ 2019-06-05 9:02 ` Rick Chen 2019-06-05 9:04 ` Rick Chen 1 sibling, 0 replies; 26+ messages in thread From: Rick Chen @ 2019-06-05 9:02 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 2/6] riscv: ae350: use the v5l2 driver to configure the cache 2019-06-04 2:48 ` Bin Meng 2019-06-05 9:02 ` Rick Chen @ 2019-06-05 9:04 ` Rick Chen 1 sibling, 0 replies; 26+ messages in thread From: Rick Chen @ 2019-06-05 9:04 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 3/6] riscv: ae350: add imply v5l2 cache controller 2019-05-28 9:39 [U-Boot] [PATCH 0/6] Support Andes RISC-V l2cache on AE350 platform Andes 2019-05-28 9:39 ` [U-Boot] [PATCH 1/6] dm: cache: add v5l2 cache controller driver Andes 2019-05-28 9:39 ` [U-Boot] [PATCH 2/6] riscv: ae350: use the v5l2 driver to configure the cache Andes @ 2019-05-28 9:39 ` Andes 2019-06-04 2:48 ` Bin Meng 2019-05-28 9:39 ` [U-Boot] [PATCH 4/6] riscv: cache: Flush L2 cache before jump to linux Andes ` (2 subsequent siblings) 5 siblings, 1 reply; 26+ messages in thread From: Andes @ 2019-05-28 9:39 UTC (permalink / raw) To: u-boot 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 -- 2.7.4 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 3/6] riscv: ae350: add imply v5l2 cache controller 2019-05-28 9:39 ` [U-Boot] [PATCH 3/6] riscv: ae350: add imply v5l2 cache controller Andes @ 2019-06-04 2:48 ` Bin Meng 2019-06-05 9:25 ` Rick Chen 0 siblings, 1 reply; 26+ messages in thread From: Bin Meng @ 2019-06-04 2:48 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 3/6] riscv: ae350: add imply v5l2 cache controller 2019-06-04 2:48 ` Bin Meng @ 2019-06-05 9:25 ` Rick Chen 0 siblings, 0 replies; 26+ messages in thread From: Rick Chen @ 2019-06-05 9:25 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 4/6] riscv: cache: Flush L2 cache before jump to linux 2019-05-28 9:39 [U-Boot] [PATCH 0/6] Support Andes RISC-V l2cache on AE350 platform Andes ` (2 preceding siblings ...) 2019-05-28 9:39 ` [U-Boot] [PATCH 3/6] riscv: ae350: add imply v5l2 cache controller Andes @ 2019-05-28 9:39 ` Andes 2019-06-04 2:48 ` Bin Meng 2019-05-28 9:39 ` [U-Boot] [PATCH 5/6] riscv: dts: move out AE350 L2 node from cpus node Andes 2019-05-28 9:39 ` [U-Boot] [PATCH 6/6] riscv: ax25: use CCTL to flush d-cache Andes 5 siblings, 1 reply; 26+ messages in thread From: Andes @ 2019-05-28 9:39 UTC (permalink / raw) To: u-boot 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; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 4/6] riscv: cache: Flush L2 cache before jump to linux 2019-05-28 9:39 ` [U-Boot] [PATCH 4/6] riscv: cache: Flush L2 cache before jump to linux Andes @ 2019-06-04 2:48 ` Bin Meng 2019-06-05 9:24 ` Rick Chen 0 siblings, 1 reply; 26+ messages in thread From: Bin Meng @ 2019-06-04 2:48 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 4/6] riscv: cache: Flush L2 cache before jump to linux 2019-06-04 2:48 ` Bin Meng @ 2019-06-05 9:24 ` Rick Chen 0 siblings, 0 replies; 26+ messages in thread From: Rick Chen @ 2019-06-05 9:24 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 5/6] riscv: dts: move out AE350 L2 node from cpus node 2019-05-28 9:39 [U-Boot] [PATCH 0/6] Support Andes RISC-V l2cache on AE350 platform Andes ` (3 preceding siblings ...) 2019-05-28 9:39 ` [U-Boot] [PATCH 4/6] riscv: cache: Flush L2 cache before jump to linux Andes @ 2019-05-28 9:39 ` Andes 2019-06-04 2:48 ` Bin Meng 2019-05-28 9:39 ` [U-Boot] [PATCH 6/6] riscv: ax25: use CCTL to flush d-cache Andes 5 siblings, 1 reply; 26+ messages in thread From: Andes @ 2019-05-28 9:39 UTC (permalink / raw) To: u-boot 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 at e0500000 { - compatible = "cache"; - cache-level = <2>; - cache-size = <0x40000>; - reg = <0x0 0xe0500000 0x0 0x40000>; - }; + L2: l2-cache at 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 at 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 at e0500000 { - compatible = "cache"; - cache-level = <2>; - cache-size = <0x40000>; - reg = <0x0 0xe0500000 0x0 0x40000>; - }; + L2: l2-cache at 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 at 0 { -- 2.7.4 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 5/6] riscv: dts: move out AE350 L2 node from cpus node 2019-05-28 9:39 ` [U-Boot] [PATCH 5/6] riscv: dts: move out AE350 L2 node from cpus node Andes @ 2019-06-04 2:48 ` Bin Meng 2019-06-05 9:33 ` Rick Chen 0 siblings, 1 reply; 26+ messages in thread From: Bin Meng @ 2019-06-04 2:48 UTC (permalink / raw) To: u-boot 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 at e0500000 { > - compatible = "cache"; > - cache-level = <2>; > - cache-size = <0x40000>; > - reg = <0x0 0xe0500000 0x0 0x40000>; > - }; > + L2: l2-cache at 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 at 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 at e0500000 { > - compatible = "cache"; > - cache-level = <2>; > - cache-size = <0x40000>; > - reg = <0x0 0xe0500000 0x0 0x40000>; > - }; > + L2: l2-cache at 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 at 0 { > -- Regards, Bin ^ permalink raw reply [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 5/6] riscv: dts: move out AE350 L2 node from cpus node 2019-06-04 2:48 ` Bin Meng @ 2019-06-05 9:33 ` Rick Chen 0 siblings, 0 replies; 26+ messages in thread From: Rick Chen @ 2019-06-05 9:33 UTC (permalink / raw) To: u-boot 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 at e0500000 { > > - compatible = "cache"; > > - cache-level = <2>; > > - cache-size = <0x40000>; > > - reg = <0x0 0xe0500000 0x0 0x40000>; > > - }; > > + L2: l2-cache at 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 at 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 at e0500000 { > > - compatible = "cache"; > > - cache-level = <2>; > > - cache-size = <0x40000>; > > - reg = <0x0 0xe0500000 0x0 0x40000>; > > - }; > > + L2: l2-cache at 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 at 0 { > > -- > > Regards, > Bin ^ permalink raw reply [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 6/6] riscv: ax25: use CCTL to flush d-cache 2019-05-28 9:39 [U-Boot] [PATCH 0/6] Support Andes RISC-V l2cache on AE350 platform Andes ` (4 preceding siblings ...) 2019-05-28 9:39 ` [U-Boot] [PATCH 5/6] riscv: dts: move out AE350 L2 node from cpus node Andes @ 2019-05-28 9:39 ` Andes 2019-06-04 2:48 ` Bin Meng 5 siblings, 1 reply; 26+ messages in thread From: Andes @ 2019-05-28 9:39 UTC (permalink / raw) To: u-boot 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" -- 2.7.4 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 6/6] riscv: ax25: use CCTL to flush d-cache 2019-05-28 9:39 ` [U-Boot] [PATCH 6/6] riscv: ax25: use CCTL to flush d-cache Andes @ 2019-06-04 2:48 ` Bin Meng 2019-06-05 9:38 ` Rick Chen 0 siblings, 1 reply; 26+ messages in thread From: Bin Meng @ 2019-06-04 2:48 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 6/6] riscv: ax25: use CCTL to flush d-cache 2019-06-04 2:48 ` Bin Meng @ 2019-06-05 9:38 ` Rick Chen 2019-06-05 9:39 ` Bin Meng 0 siblings, 1 reply; 26+ messages in thread From: Rick Chen @ 2019-06-05 9:38 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 6/6] riscv: ax25: use CCTL to flush d-cache 2019-06-05 9:38 ` Rick Chen @ 2019-06-05 9:39 ` Bin Meng 2019-06-09 17:57 ` Auer, Lukas 0 siblings, 1 reply; 26+ messages in thread From: Bin Meng @ 2019-06-05 9:39 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [U-Boot] [PATCH 6/6] riscv: ax25: use CCTL to flush d-cache 2019-06-05 9:39 ` Bin Meng @ 2019-06-09 17:57 ` Auer, Lukas 0 siblings, 0 replies; 26+ messages in thread From: Auer, Lukas @ 2019-06-09 17:57 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2019-06-12 6:32 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-28 9:39 [U-Boot] [PATCH 0/6] Support Andes RISC-V l2cache on AE350 platform Andes 2019-05-28 9:39 ` [U-Boot] [PATCH 1/6] dm: cache: add v5l2 cache controller driver Andes 2019-06-04 2:48 ` Bin Meng 2019-06-05 8:58 ` Rick Chen 2019-06-09 17:56 ` Auer, Lukas 2019-06-10 2:26 ` Rick Chen 2019-06-10 2:32 ` Bin Meng 2019-06-12 6:32 ` Rick Chen 2019-05-28 9:39 ` [U-Boot] [PATCH 2/6] riscv: ae350: use the v5l2 driver to configure the cache Andes 2019-06-04 2:48 ` Bin Meng 2019-06-05 9:02 ` Rick Chen 2019-06-05 9:04 ` Rick Chen 2019-05-28 9:39 ` [U-Boot] [PATCH 3/6] riscv: ae350: add imply v5l2 cache controller Andes 2019-06-04 2:48 ` Bin Meng 2019-06-05 9:25 ` Rick Chen 2019-05-28 9:39 ` [U-Boot] [PATCH 4/6] riscv: cache: Flush L2 cache before jump to linux Andes 2019-06-04 2:48 ` Bin Meng 2019-06-05 9:24 ` Rick Chen 2019-05-28 9:39 ` [U-Boot] [PATCH 5/6] riscv: dts: move out AE350 L2 node from cpus node Andes 2019-06-04 2:48 ` Bin Meng 2019-06-05 9:33 ` Rick Chen 2019-05-28 9:39 ` [U-Boot] [PATCH 6/6] riscv: ax25: use CCTL to flush d-cache Andes 2019-06-04 2:48 ` Bin Meng 2019-06-05 9:38 ` Rick Chen 2019-06-05 9:39 ` Bin Meng 2019-06-09 17:57 ` Auer, Lukas
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.