On Fri, Aug 13, 2021 at 4:20 AM Sean Anderson <sean...@gmail.com> wrote: > > On 8/10/21 2:57 AM, Zong Li wrote: > > On Tue, Aug 10, 2021 at 12:47 PM Sean Anderson <sean...@gmail.com> wrote: > >> > >> On 8/3/21 12:44 AM, Zong Li wrote: > >>> Add an interface for cache initialization. Each platform can overwrite > >>> this weak function by their own implementation, such as sifive_cache in > >>> this patch. > >> > >> Can we call this enable_caches instead of cache_init? This function is > >> called by initr_caches in board_r.c for ARM. There's even an > >> eight-year-old TODO on the subject. > >> > > > > I had considered use it, The reason I finally used cache_init here is > > that it seems to me that cache_init would be more flexible for risc-v > > platforms to do not only cache enable, but also various > > platform-specific initialization of cache, even they could decide the > > time to invoke cache_init if there is particular initialization > > sequence. > > Do you have some example in mind?
It seems to me that not all cache devices are only configured for operations related to enable/disable. It might refer to the status of the cache itself or different functionalities, then it might be a bit weird if we associate them with the "enable" term, It is a platform-specific implementation. > > > If you think that cache_init is OK to you, I would prefer to > > retain cache_init. I can still use enable_caches instead of cache_init > > if you think that it is a better way. Please let me know your thoughts > > and thanks for your review. > > I would like to reduce the proliferation of different cache enable > functions. Right now we have (i|d)cache_enable which are RISC-V-specific > and called very early during boot; cache_enable, which must be called > manually; enable_caches, which is implemented only for ARM; and your > proposed cache_init. I don't think there is need for yet another way to > accomplish the same thing. > > --Sean > > >>> > >>> Signed-off-by: Zong Li <zong...@sifive.com> > >>> --- > >>> arch/riscv/Kconfig | 5 +++++ > >>> arch/riscv/include/asm/cache.h | 1 + > >>> arch/riscv/lib/Makefile | 1 + > >>> arch/riscv/lib/cache.c | 5 +++++ > >>> arch/riscv/lib/sifive_cache.c | 27 +++++++++++++++++++++++++++ > >>> 5 files changed, 39 insertions(+) > >>> create mode 100644 arch/riscv/lib/sifive_cache.c > >>> > >>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > >>> index 4b0c3dffa6..ec651fe0a4 100644 > >>> --- a/arch/riscv/Kconfig > >>> +++ b/arch/riscv/Kconfig > >>> @@ -179,6 +179,11 @@ config SPL_SIFIVE_CLINT > >>> The SiFive CLINT block holds memory-mapped control and status > >>> registers > >>> associated with software and timer interrupts. > >>> > >>> +config SIFIVE_CACHE > >>> + bool > >>> + help > >>> + This enables the operations to configure SiFive cache > >>> + > >>> config ANDES_PLIC > >>> bool > >>> depends on RISCV_MMODE || SPL_RISCV_MMODE > >>> diff --git a/arch/riscv/include/asm/cache.h > >>> b/arch/riscv/include/asm/cache.h > >>> index ec8fe201d3..6ebb2b4329 100644 > >>> --- a/arch/riscv/include/asm/cache.h > >>> +++ b/arch/riscv/include/asm/cache.h > >>> @@ -9,6 +9,7 @@ > >>> > >>> /* cache */ > >>> void cache_flush(void); > >>> +int cache_init(void); > >>> > >>> /* > >>> * The current upper bound for RISCV L1 data cache line sizes is 32 > >>> bytes. > >>> diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > >>> index c4cc41434b..06020fcc2a 100644 > >>> --- a/arch/riscv/lib/Makefile > >>> +++ b/arch/riscv/lib/Makefile > >>> @@ -10,6 +10,7 @@ obj-$(CONFIG_CMD_BOOTM) += bootm.o > >>> obj-$(CONFIG_CMD_BOOTI) += bootm.o image.o > >>> obj-$(CONFIG_CMD_GO) += boot.o > >>> obj-y += cache.o > >>> +obj-$(CONFIG_SIFIVE_CACHE) += sifive_cache.o > >>> ifeq ($(CONFIG_$(SPL_)RISCV_MMODE),y) > >>> obj-$(CONFIG_$(SPL_)SIFIVE_CLINT) += sifive_clint.o > >>> obj-$(CONFIG_ANDES_PLIC) += andes_plic.o > >>> diff --git a/arch/riscv/lib/cache.c b/arch/riscv/lib/cache.c > >>> index b1d42bcc2b..2cd66504c6 100644 > >>> --- a/arch/riscv/lib/cache.c > >>> +++ b/arch/riscv/lib/cache.c > >>> @@ -70,3 +70,8 @@ __weak int dcache_status(void) > >>> { > >>> return 0; > >>> } > >>> + > >>> +__weak int cache_init(void) > >>> +{ > >>> + return 0; > >>> +} > >>> diff --git a/arch/riscv/lib/sifive_cache.c b/arch/riscv/lib/sifive_cache.c > >>> new file mode 100644 > >>> index 0000000000..94e84e024e > >>> --- /dev/null > >>> +++ b/arch/riscv/lib/sifive_cache.c > >>> @@ -0,0 +1,27 @@ > >>> +// SPDX-License-Identifier: GPL-2.0+ > >>> +/* > >>> + * Copyright (C) 2021 SiFive, Inc > >>> + */ > >>> + > >>> +#include <common.h> > >>> +#include <cache.h> > >>> +#include <dm.h> > >>> + > >>> +int cache_init(void) > >>> +{ > >>> + struct udevice *dev; > >>> + int ret; > >>> + > >>> + /* Enable ways of ccache */ > >>> + ret = uclass_get_device_by_driver(UCLASS_CACHE, > >>> + DM_DRIVER_GET(sifive_ccache), > >>> + &dev); > >>> + if (ret) > >>> + return log_msg_ret("Cannot enable cache ways", ret); > >>> + > >>> + ret = cache_enable(dev); > >>> + if (ret) > >>> + return log_msg_ret("ccache enable failed", ret); > >>> + > >>> + return 0; > >>> +} > >>> > >> > >> Otherwise LGTM >