On 8/3/21 12:44 AM, Zong Li wrote:

This driver is currently responsible for enabling all ccache ways. Composable cache could be configure as RAM or cache, we will use it as RAM at the beginning to put the u-boot SPL there. In u-boot proper phrase, we will use the composable cache as cache, and try to enable the cache ways.

Signed-off-by: Zong Li zong.li@sifive.com

drivers/cache/Kconfig | 7 +++ drivers/cache/Makefile | 1 + drivers/cache/cache-sifive-ccache.c | 75 +++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+) create mode 100644 drivers/cache/cache-sifive-ccache.c

diff --git a/drivers/cache/Kconfig b/drivers/cache/Kconfig index 1e452ad6d9..40f41a817c 100644 --- a/drivers/cache/Kconfig +++ b/drivers/cache/Kconfig @@ -39,4 +39,11 @@ config NCORE_CACHE controller. The driver initializes cache directories and coherent agent interfaces.

+config SIFIVE_CCACHE

• bool "SiFive composable cache"
• select CACHE
• help

•  This driver is for SiFive Composable L2/L3 cache. It enables cache
  

•  ways of composable cache.
  

• endmenu

diff --git a/drivers/cache/Makefile b/drivers/cache/Makefile index fed50be3f9..ad765774e3 100644 --- a/drivers/cache/Makefile +++ b/drivers/cache/Makefile @@ -4,3 +4,4 @@ obj-$(CONFIG_SANDBOX) += sandbox_cache.o obj-$(CONFIG_L2X0_CACHE) += cache-l2x0.o obj-$(CONFIG_NCORE_CACHE) += cache-ncore.o obj-$(CONFIG_V5L2_CACHE) += cache-v5l2.o +obj-$(CONFIG_SIFIVE_CCACHE) += cache-sifive-ccache.o diff --git a/drivers/cache/cache-sifive-ccache.c b/drivers/cache/cache-sifive-ccache.c new file mode 100644 index 0000000000..76c0ab26ae --- /dev/null +++ b/drivers/cache/cache-sifive-ccache.c @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: GPL-2.0 +/*

• • Copyright (C) 2021 SiFive
• */

+#include <common.h> +#include <cache.h> +#include <dm.h> +#include <asm/io.h> +#include <dm/device.h> +#include <linux/bitfield.h>

+#define SIFIVE_CCACHE_CONFIG 0x000 +#define SIFIVE_CCACHE_CONFIG_WAYS GENMASK(15, 8)

+#define SIFIVE_CCACHE_WAY_ENABLE 0x008

+struct sifive_ccache {

• void __iomem *base;

+};

+static int sifive_ccache_enable(struct udevice *dev) +{

• struct sifive_ccache *priv = dev_get_priv(dev);
• u32 config;
• u32 ways;
• /* Enable all ways of composable cache */
• config = readl(priv->base + SIFIVE_CCACHE_CONFIG);
• ways = FIELD_GET(SIFIVE_CCACHE_CONFIG_WAYS, config);
• writel(ways - 1, priv->base + SIFIVE_CCACHE_WAY_ENABLE);
• return 0;

+}

+static int sifive_ccache_get_info(struct udevice *dev, struct cache_info *info) +{

• struct sifive_ccache *priv = dev_get_priv(dev);
• info->base = (phys_addr_t)priv->base;
• return 0;

+}

+static const struct cache_ops sifive_ccache_ops = {

• .enable = sifive_ccache_enable,
• .get_info = sifive_ccache_get_info,

+};

+static int sifive_ccache_probe(struct udevice *dev) +{

• struct sifive_ccache *priv = dev_get_priv(dev);
• priv->base = dev_read_addr_ptr(dev);
• if (!priv->base)

• return -EINVAL;
  

• return 0;

+}

+static const struct udevice_id sifive_ccache_ids[] = {

• { .compatible = "sifive,fu540-c000-ccache" },
• { .compatible = "sifive,fu740-c000-ccache" },
• {}

+};

+U_BOOT_DRIVER(sifive_ccache) = {

• .name = "sifive_ccache",
• .id = UCLASS_CACHE,
• .of_match = sifive_ccache_ids,
• .probe = sifive_ccache_probe,
• .priv_auto = sizeof(struct sifive_ccache),
• .ops = &sifive_ccache_ops,

+};

Reviewed-by: Sean Anderson seanga2@gmail.com

On 8/3/21 12:44 AM, Zong Li wrote:

Invokes the generic cache_enable interface to execute the relative implementation in SiFive ccache driver.

Signed-off-by: Zong Li zong.li@sifive.com

arch/riscv/cpu/fu540/Kconfig | 1 + arch/riscv/cpu/fu540/cache.c | 54 ++++++----------------- arch/riscv/cpu/fu740/Kconfig | 1 + arch/riscv/cpu/fu740/cache.c | 52 ++++++---------------- arch/riscv/include/asm/arch-fu540/cache.h | 2 +- arch/riscv/include/asm/arch-fu740/cache.h | 2 +- board/sifive/unleashed/unleashed.c | 10 +---- board/sifive/unmatched/unmatched.c | 9 +--- 8 files changed, 33 insertions(+), 98 deletions(-)

diff --git a/arch/riscv/cpu/fu540/Kconfig b/arch/riscv/cpu/fu540/Kconfig index 05463b2625..8608741779 100644 --- a/arch/riscv/cpu/fu540/Kconfig +++ b/arch/riscv/cpu/fu540/Kconfig @@ -19,6 +19,7 @@ config SIFIVE_FU540 imply SMP imply CLK_SIFIVE imply CLK_SIFIVE_PRCI

• imply SIFIVE_CCACHE imply SIFIVE_SERIAL imply MACB imply MII

diff --git a/arch/riscv/cpu/fu540/cache.c b/arch/riscv/cpu/fu540/cache.c index 0fc4ef6c00..bc31f664b8 100644 --- a/arch/riscv/cpu/fu540/cache.c +++ b/arch/riscv/cpu/fu540/cache.c @@ -1,55 +1,29 @@ // SPDX-License-Identifier: GPL-2.0+ /*

• • Copyright (C) 2020 SiFive, Inc

• • Copyright (C) 2020 - 2021 SiFive, Inc
  • Authors:
  • Pragnesh Patel pragnesh.patel@sifive.com

  */

  #include <common.h>

-#include <asm/global_data.h> -#include <asm/io.h> -#include <linux/bitops.h> +#include <cache.h> +#include <dm.h>

-/* Register offsets */ -#define L2_CACHE_CONFIG 0x000 -#define L2_CACHE_ENABLE 0x008

-#define MASK_NUM_WAYS GENMASK(15, 8) -#define NUM_WAYS_SHIFT 8

-DECLARE_GLOBAL_DATA_PTR;

-int cache_enable_ways(void) +int sifive_ccache_enable_ways(void) {

• const void *blob = gd->fdt_blob;
• int node;
• fdt_addr_t base;
• u32 config;
• u32 ways;
• volatile u32 *enable;
• node = fdt_node_offset_by_compatible(blob, -1,

• 			     "sifive,fu540-c000-ccache");
  

• if (node < 0)

• return node;
  

• base = fdtdec_get_addr_size_auto_parent(blob, 0, node, "reg", 0,

• 				NULL, false);
  

• if (base == FDT_ADDR_T_NONE)

• return FDT_ADDR_T_NONE;
  

• struct udevice *dev;
• int ret;

• config = readl((volatile u32 *)base + L2_CACHE_CONFIG);
• ways = (config & MASK_NUM_WAYS) >> NUM_WAYS_SHIFT;

• 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);
  
  

• enable = (volatile u32 *)(base + L2_CACHE_ENABLE);

• ret = cache_enable(dev);
• if (ret)

• return log_msg_ret("ccache enable failed", ret);
  
  

• /* memory barrier */
• mb();
• (*enable) = ways - 1;
• /* memory barrier */
• mb(); return 0; }

diff --git a/arch/riscv/cpu/fu740/Kconfig b/arch/riscv/cpu/fu740/Kconfig index 408195f149..b4cada0ea9 100644 --- a/arch/riscv/cpu/fu740/Kconfig +++ b/arch/riscv/cpu/fu740/Kconfig @@ -19,6 +19,7 @@ config SIFIVE_FU740 imply SMP imply CLK_SIFIVE imply CLK_SIFIVE_PRCI

• imply SIFIVE_CCACHE imply SIFIVE_SERIAL imply MACB imply MII

diff --git a/arch/riscv/cpu/fu740/cache.c b/arch/riscv/cpu/fu740/cache.c index 680955c9e3..e2782d76c0 100644 --- a/arch/riscv/cpu/fu740/cache.c +++ b/arch/riscv/cpu/fu740/cache.c @@ -7,49 +7,23 @@ */

#include <common.h> -#include <asm/io.h> -#include <linux/bitops.h> -#include <asm/global_data.h> +#include <cache.h> +#include <dm.h>

-/* Register offsets */ -#define L2_CACHE_CONFIG 0x000 -#define L2_CACHE_ENABLE 0x008

-#define MASK_NUM_WAYS GENMASK(15, 8) -#define NUM_WAYS_SHIFT 8

-DECLARE_GLOBAL_DATA_PTR;

-int cache_enable_ways(void) +int sifive_ccache_enable_ways(void) {

• const void *blob = gd->fdt_blob;
• int node;
• fdt_addr_t base;
• u32 config;
• u32 ways;
• volatile u32 *enable;
• node = fdt_node_offset_by_compatible(blob, -1,

• 			     "sifive,fu740-c000-ccache");
  

• if (node < 0)

• return node;
  

• base = fdtdec_get_addr_size_auto_parent(blob, 0, node, "reg", 0,

• 				NULL, false);
  

• if (base == FDT_ADDR_T_NONE)

• return FDT_ADDR_T_NONE;
  

• struct udevice *dev;
• int ret;

• config = readl((volatile u32 *)base + L2_CACHE_CONFIG);
• ways = (config & MASK_NUM_WAYS) >> NUM_WAYS_SHIFT;

• 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);
  
  

• enable = (volatile u32 *)(base + L2_CACHE_ENABLE);

• ret = cache_enable(dev);
• if (ret)

• return log_msg_ret("ccache enable failed", ret);
  
  

• /* memory barrier */
• mb();
• (*enable) = ways - 1;
• /* memory barrier */
• mb(); return 0; }

diff --git a/arch/riscv/include/asm/arch-fu540/cache.h b/arch/riscv/include/asm/arch-fu540/cache.h index 135a17c679..c252eb64d1 100644 --- a/arch/riscv/include/asm/arch-fu540/cache.h +++ b/arch/riscv/include/asm/arch-fu540/cache.h @@ -9,6 +9,6 @@ #ifndef _CACHE_SIFIVE_H #define _CACHE_SIFIVE_H

-int cache_enable_ways(void); +int sifive_ccache_enable_ways(void);

#endif /* _CACHE_SIFIVE_H */ diff --git a/arch/riscv/include/asm/arch-fu740/cache.h b/arch/riscv/include/asm/arch-fu740/cache.h index 7d4fe9942b..8c456e3658 100644 --- a/arch/riscv/include/asm/arch-fu740/cache.h +++ b/arch/riscv/include/asm/arch-fu740/cache.h @@ -9,6 +9,6 @@ #ifndef _CACHE_SIFIVE_H #define _CACHE_SIFIVE_H

-int cache_enable_ways(void); +int sifive_ccache_enable_ways(void);

#endif /* _CACHE_SIFIVE_H */ diff --git a/board/sifive/unleashed/unleashed.c b/board/sifive/unleashed/unleashed.c index 43027f0b54..12e61ec85f 100644 --- a/board/sifive/unleashed/unleashed.c +++ b/board/sifive/unleashed/unleashed.c @@ -126,14 +126,6 @@ void *board_fdt_blob_setup(void)

int board_init(void) {

• int ret;
• /* enable all cache ways */
• ret = cache_enable_ways();
• if (ret) {

• debug("%s: could not enable cache ways\n", __func__);
  

• return ret;
  

• }
• return 0;

• return sifive_ccache_enable_ways(); }

diff --git a/board/sifive/unmatched/unmatched.c b/board/sifive/unmatched/unmatched.c index 2f5629b578..d27c4d3e88 100644 --- a/board/sifive/unmatched/unmatched.c +++ b/board/sifive/unmatched/unmatched.c @@ -23,13 +23,6 @@ void *board_fdt_blob_setup(void)

int board_init(void) {

• int ret;
• /* enable all cache ways */
• ret = cache_enable_ways();
• if (ret) {

• debug("%s: could not enable cache ways\n", __func__);
  

• return ret;
  

• }
• return 0;

• return sifive_ccache_enable_ways(); }

Can you combine patches 2-4 in some way? It seems like you add some code only to immediately refactor it.

--Sean

On Tue, Aug 10, 2021 at 12:47 PM Sean Anderson seanga2@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. 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.

...

Signed-off-by: Zong Li zong.li@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

On Fri, Aug 13, 2021 at 4:20 AM Sean Anderson seanga2@gmail.com wrote:

On 8/10/21 2:57 AM, Zong Li wrote:

...

On Tue, Aug 10, 2021 at 12:47 PM Sean Anderson seanga2@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.li@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

On Tue, Aug 10, 2021 at 12:55 PM Sean Anderson seanga2@gmail.com wrote:

...

Re: [PATCH v2 5/6] riscv: lib: move platform-related libraries to sperate folder

nit: separate

Thanks for catching it. Fix it in the next version.

On 8/3/21 12:44 AM, Zong Li wrote:

...

Put the platform-related implementation into their own folder respectively. Just leave the common library in the top of lib folder.

Signed-off-by: Zong Li zong.li@sifive.com

arch/riscv/Kconfig | 7 +++++++ arch/riscv/lib/Makefile | 9 ++++----- arch/riscv/lib/andestech/Kconfig | 8 ++++++++ arch/riscv/lib/andestech/Makefile | 7 +++++++ arch/riscv/lib/{ => andestech}/andes_plic.c | 0 arch/riscv/lib/sifive/Kconfig | 8 ++++++++ arch/riscv/lib/sifive/Makefile | 9 +++++++++ arch/riscv/lib/{ => sifive}/sifive_cache.c | 0 arch/riscv/lib/{ => sifive}/sifive_clint.c | 0 9 files changed, 43 insertions(+), 5 deletions(-) create mode 100644 arch/riscv/lib/andestech/Kconfig create mode 100644 arch/riscv/lib/andestech/Makefile rename arch/riscv/lib/{ => andestech}/andes_plic.c (100%) create mode 100644 arch/riscv/lib/sifive/Kconfig create mode 100644 arch/riscv/lib/sifive/Makefile rename arch/riscv/lib/{ => sifive}/sifive_cache.c (100%) rename arch/riscv/lib/{ => sifive}/sifive_clint.c (100%)

NAK from me. I'd much rather see organization by function (e.g. clint/sbi/plic together) than by vendor. Plus, the clint/plic are not really specific to one vendor like ccache.

Yes, it makes more sense to me. In this case, there are three functionalities, so I'd like to separate clint, plic and cache at this time, does it make sense to you?

--Sean

...

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index ec651fe0a4..ed1bf2f6c8 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -72,6 +72,10 @@ source "arch/riscv/cpu/fu540/Kconfig" source "arch/riscv/cpu/fu740/Kconfig" source "arch/riscv/cpu/generic/Kconfig"

+# library-specific options below +source "arch/riscv/lib/sifive/Kconfig" +source "arch/riscv/lib/andestech/Kconfig"

• # architecture-specific options below

  choice

@@ -175,18 +179,21 @@ config SIFIVE_CLINT config SPL_SIFIVE_CLINT bool depends on SPL_RISCV_MMODE

• select SIFIVE_LIB
  help
    The SiFive CLINT block holds memory-mapped control and status registers
    associated with software and timer interrupts.
  
  

  config SIFIVE_CACHE bool

• select SIFIVE_LIB
  help
    This enables the operations to configure SiFive cache
  
  

  config ANDES_PLIC bool depends on RISCV_MMODE || SPL_RISCV_MMODE

• select ANDESTECH_LIB
  select REGMAP
  select SYSCON
  select SPL_REGMAP if SPL
  

diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile index 06020fcc2a..f58d1f9819 100644 --- a/arch/riscv/lib/Makefile +++ b/arch/riscv/lib/Makefile @@ -10,11 +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 -else +ifeq ($(CONFIG_$(SPL_)RISCV_MMODE),) obj-$(CONFIG_SBI) += sbi.o obj-$(CONFIG_SBI_IPI) += sbi_ipi.o endif @@ -42,3 +38,6 @@ extra-$(CONFIG_EFI) += $(EFI_CRT0) $(EFI_RELOC) obj-$(CONFIG_$(SPL_TPL_)USE_ARCH_MEMSET) += memset.o obj-$(CONFIG_$(SPL_TPL_)USE_ARCH_MEMMOVE) += memmove.o obj-$(CONFIG_$(SPL_TPL_)USE_ARCH_MEMCPY) += memcpy.o

+obj-$(CONFIG_SIFIVE_LIB) += sifive/ +obj-$(CONFIG_ANDESTECH_LIB) += andestech/ diff --git a/arch/riscv/lib/andestech/Kconfig b/arch/riscv/lib/andestech/Kconfig new file mode 100644 index 0000000000..75f83a8123 --- /dev/null +++ b/arch/riscv/lib/andestech/Kconfig @@ -0,0 +1,8 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright (C) 2021 SiFive, Inc

+config ANDESTECH_LIB

• bool
  

• help
  

•   This supports the specific libraries for AndesTech platforms
  

diff --git a/arch/riscv/lib/andestech/Makefile b/arch/riscv/lib/andestech/Makefile new file mode 100644 index 0000000000..49f45d0a29 --- /dev/null +++ b/arch/riscv/lib/andestech/Makefile @@ -0,0 +1,7 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright (C) 2021 SiFive, Inc

+ifeq ($(CONFIG_$(SPL_)RISCV_MMODE),y) +obj-$(CONFIG_ANDES_PLIC) += andes_plic.o +endif diff --git a/arch/riscv/lib/andes_plic.c b/arch/riscv/lib/andestech/andes_plic.c similarity index 100% rename from arch/riscv/lib/andes_plic.c rename to arch/riscv/lib/andestech/andes_plic.c diff --git a/arch/riscv/lib/sifive/Kconfig b/arch/riscv/lib/sifive/Kconfig new file mode 100644 index 0000000000..20574079e9 --- /dev/null +++ b/arch/riscv/lib/sifive/Kconfig @@ -0,0 +1,8 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright (C) 2021 SiFive, Inc

+config SIFIVE_LIB

• bool
  

• help
  

•   This supports the specific libraries for SiFive platforms
  

diff --git a/arch/riscv/lib/sifive/Makefile b/arch/riscv/lib/sifive/Makefile new file mode 100644 index 0000000000..ba120db26a --- /dev/null +++ b/arch/riscv/lib/sifive/Makefile @@ -0,0 +1,9 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright (C) 2021 SiFive, Inc

+obj-$(CONFIG_SIFIVE_CACHE) += sifive_cache.o

+ifeq ($(CONFIG_$(SPL_)RISCV_MMODE),y) +obj-$(CONFIG_$(SPL_)SIFIVE_CLINT) += sifive_clint.o +endif diff --git a/arch/riscv/lib/sifive_cache.c b/arch/riscv/lib/sifive/sifive_cache.c similarity index 100% rename from arch/riscv/lib/sifive_cache.c rename to arch/riscv/lib/sifive/sifive_cache.c diff --git a/arch/riscv/lib/sifive_clint.c b/arch/riscv/lib/sifive/sifive_clint.c similarity index 100% rename from arch/riscv/lib/sifive_clint.c rename to arch/riscv/lib/sifive/sifive_clint.c

On Fri, Aug 13, 2021 at 4:22 AM Sean Anderson seanga2@gmail.com wrote:

On 8/10/21 3:04 AM, Zong Li wrote:

...

On Tue, Aug 10, 2021 at 12:55 PM Sean Anderson seanga2@gmail.com wrote:

...

...

Re: [PATCH v2 5/6] riscv: lib: move platform-related libraries to sperate folder

nit: separate

Thanks for catching it. Fix it in the next version.

...

On 8/3/21 12:44 AM, Zong Li wrote:

...

Put the platform-related implementation into their own folder respectively. Just leave the common library in the top of lib folder.

Signed-off-by: Zong Li zong.li@sifive.com

arch/riscv/Kconfig | 7 +++++++ arch/riscv/lib/Makefile | 9 ++++----- arch/riscv/lib/andestech/Kconfig | 8 ++++++++ arch/riscv/lib/andestech/Makefile | 7 +++++++ arch/riscv/lib/{ => andestech}/andes_plic.c | 0 arch/riscv/lib/sifive/Kconfig | 8 ++++++++ arch/riscv/lib/sifive/Makefile | 9 +++++++++ arch/riscv/lib/{ => sifive}/sifive_cache.c | 0 arch/riscv/lib/{ => sifive}/sifive_clint.c | 0 9 files changed, 43 insertions(+), 5 deletions(-) create mode 100644 arch/riscv/lib/andestech/Kconfig create mode 100644 arch/riscv/lib/andestech/Makefile rename arch/riscv/lib/{ => andestech}/andes_plic.c (100%) create mode 100644 arch/riscv/lib/sifive/Kconfig create mode 100644 arch/riscv/lib/sifive/Makefile rename arch/riscv/lib/{ => sifive}/sifive_cache.c (100%) rename arch/riscv/lib/{ => sifive}/sifive_clint.c (100%)

NAK from me. I'd much rather see organization by function (e.g. clint/sbi/plic together) than by vendor. Plus, the clint/plic are not really specific to one vendor like ccache.

Yes, it makes more sense to me. In this case, there are three functionalities, so I'd like to separate clint, plic and cache at this time, does it make sense to you?

No, it does not. clint and plic should be grouped with SBI because in U-Boot they are all used for IPIs. But frankly, I don't really see the need to place them in subdirectories yet...

Okay, I got it, how about only separate cache into a subdirectory? Or you think that it might be OK to just put all of them in lib/ ?

--Sean

...

...

--Sean

...

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index ec651fe0a4..ed1bf2f6c8 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -72,6 +72,10 @@ source "arch/riscv/cpu/fu540/Kconfig" source "arch/riscv/cpu/fu740/Kconfig" source "arch/riscv/cpu/generic/Kconfig"

+# library-specific options below +source "arch/riscv/lib/sifive/Kconfig" +source "arch/riscv/lib/andestech/Kconfig"

• # architecture-specific options below

  choice

@@ -175,18 +179,21 @@ config SIFIVE_CLINT config SPL_SIFIVE_CLINT bool depends on SPL_RISCV_MMODE

• select SIFIVE_LIB
   help
     The SiFive CLINT block holds memory-mapped control and status registers
     associated with software and timer interrupts.
  
  

  config SIFIVE_CACHE bool

• select SIFIVE_LIB
   help
     This enables the operations to configure SiFive cache
  
  

  config ANDES_PLIC bool depends on RISCV_MMODE || SPL_RISCV_MMODE

• select ANDESTECH_LIB
   select REGMAP
   select SYSCON
   select SPL_REGMAP if SPL
  

diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile index 06020fcc2a..f58d1f9819 100644 --- a/arch/riscv/lib/Makefile +++ b/arch/riscv/lib/Makefile @@ -10,11 +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 -else +ifeq ($(CONFIG_$(SPL_)RISCV_MMODE),) obj-$(CONFIG_SBI) += sbi.o obj-$(CONFIG_SBI_IPI) += sbi_ipi.o endif @@ -42,3 +38,6 @@ extra-$(CONFIG_EFI) += $(EFI_CRT0) $(EFI_RELOC) obj-$(CONFIG_$(SPL_TPL_)USE_ARCH_MEMSET) += memset.o obj-$(CONFIG_$(SPL_TPL_)USE_ARCH_MEMMOVE) += memmove.o obj-$(CONFIG_$(SPL_TPL_)USE_ARCH_MEMCPY) += memcpy.o

+obj-$(CONFIG_SIFIVE_LIB) += sifive/ +obj-$(CONFIG_ANDESTECH_LIB) += andestech/ diff --git a/arch/riscv/lib/andestech/Kconfig b/arch/riscv/lib/andestech/Kconfig new file mode 100644 index 0000000000..75f83a8123 --- /dev/null +++ b/arch/riscv/lib/andestech/Kconfig @@ -0,0 +1,8 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright (C) 2021 SiFive, Inc

+config ANDESTECH_LIB

• bool
  

• help
  

•   This supports the specific libraries for AndesTech platforms
  

diff --git a/arch/riscv/lib/andestech/Makefile b/arch/riscv/lib/andestech/Makefile new file mode 100644 index 0000000000..49f45d0a29 --- /dev/null +++ b/arch/riscv/lib/andestech/Makefile @@ -0,0 +1,7 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright (C) 2021 SiFive, Inc

+ifeq ($(CONFIG_$(SPL_)RISCV_MMODE),y) +obj-$(CONFIG_ANDES_PLIC) += andes_plic.o +endif diff --git a/arch/riscv/lib/andes_plic.c b/arch/riscv/lib/andestech/andes_plic.c similarity index 100% rename from arch/riscv/lib/andes_plic.c rename to arch/riscv/lib/andestech/andes_plic.c diff --git a/arch/riscv/lib/sifive/Kconfig b/arch/riscv/lib/sifive/Kconfig new file mode 100644 index 0000000000..20574079e9 --- /dev/null +++ b/arch/riscv/lib/sifive/Kconfig @@ -0,0 +1,8 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright (C) 2021 SiFive, Inc

+config SIFIVE_LIB

• bool
  

• help
  

•   This supports the specific libraries for SiFive platforms
  

diff --git a/arch/riscv/lib/sifive/Makefile b/arch/riscv/lib/sifive/Makefile new file mode 100644 index 0000000000..ba120db26a --- /dev/null +++ b/arch/riscv/lib/sifive/Makefile @@ -0,0 +1,9 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright (C) 2021 SiFive, Inc

+obj-$(CONFIG_SIFIVE_CACHE) += sifive_cache.o

+ifeq ($(CONFIG_$(SPL_)RISCV_MMODE),y) +obj-$(CONFIG_$(SPL_)SIFIVE_CLINT) += sifive_clint.o +endif diff --git a/arch/riscv/lib/sifive_cache.c b/arch/riscv/lib/sifive/sifive_cache.c similarity index 100% rename from arch/riscv/lib/sifive_cache.c rename to arch/riscv/lib/sifive/sifive_cache.c diff --git a/arch/riscv/lib/sifive_clint.c b/arch/riscv/lib/sifive/sifive_clint.c similarity index 100% rename from arch/riscv/lib/sifive_clint.c rename to arch/riscv/lib/sifive/sifive_clint.c

On 8/3/21 12:44 AM, Zong Li wrote:

We usually use a space in function declaration, rather than a tab.

Signed-off-by: Zong Li zong.li@sifive.com

arch/riscv/include/asm/cache.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/cache.h b/arch/riscv/include/asm/cache.h index 6ebb2b4329..b700ff5021 100644 --- a/arch/riscv/include/asm/cache.h +++ b/arch/riscv/include/asm/cache.h @@ -8,7 +8,7 @@ #define _ASM_RISCV_CACHE_H

/* cache */ -void cache_flush(void); +void cache_flush(void); int cache_init(void);

/*

Reviewed-by: Sean Anderson seanga2@gmail.com