On 7/27/21 4:54 AM, Zong Li wrote:
This driver is currently responsible for enabling all ccache ways.

Can you expand on this a little? Perhaps describe the hardware a little. For 
example,
you could describe what a way/bank is, and that they can't be disabled by the 
hardware.


Signed-off-by: Zong Li <zong...@sifive.com>
---
  drivers/cache/Kconfig               |  7 +++
  drivers/cache/Makefile              |  1 +
  drivers/cache/cache-sifive-ccache.c | 69 +++++++++++++++++++++++++++++
  3 files changed, 77 insertions(+)
  create mode 100644 drivers/cache/cache-sifive-ccache.c

diff --git a/drivers/cache/Kconfig b/drivers/cache/Kconfig
index 1e452ad6d9..b903e3e935 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_CACHE_CCACHE

Just SIFIVE_CCACHE (or SIFIVE_CACHE) please.

+       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..92c6c5a83f 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_CACHE_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..9ea064912f
--- /dev/null
+++ b/drivers/cache/cache-sifive-ccache.c
@@ -0,0 +1,69 @@
+// 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>
+
+#define SIFIVE_CCACHE_CONFIG   0x000
+#define SIFIVE_CCACHE_ENABLE   0x008

WAY_ENABLE?

+
+#define SIFIVE_CCACHE_NUM_WAY_MASK     GENMASK(15, 8)
+#define SIFIVE_CCACHE_NUM_WAY_SHIFT    8
+
+struct sifive_ccache {
+       void __iomem *base;
+};
+
+static int sifive_ccache_enable_all_ways(struct udevice *dev)
+{
+       struct sifive_ccache *priv = dev_get_priv(dev);
+       u32 config;
+       u32 ways;
+
+       config = readl(priv->base + SIFIVE_CCACHE_CONFIG);
+       ways = (config & SIFIVE_CCACHE_NUM_WAY_MASK) >> 
SIFIVE_CCACHE_NUM_WAY_SHIFT;

ways = FIELD_GET(SIFIVE_CCACHE_NUM_WAY_MASK, config);

and perhaps this should be named SIFIVE_CCACHE_CONFIG_WAYS to better match the 
datasheet?

+
+       writel(ways - 1, priv->base + SIFIVE_CCACHE_ENABLE);
+
+       return 0;
+}
+
+static int sifive_ccache_enable(struct udevice *dev)
+{
+       return sifive_ccache_enable_all_ways(dev);

Any reason to have this in a separate function?

+}
+
+static const struct cache_ops sifive_ccache_ops = {
+       .enable = sifive_ccache_enable,

Please implement get_info as well. It should effectively just be

get_info()
{
        struct sifive_ccache *priv = dev_get_priv(dev);
        
        info->base = priv->base;
        return 0;
}

+};
+
+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 -ENODEV;

Please return -EINVAL instead [1].

--Sean

[1] 
https://u-boot.readthedocs.io/en/latest/develop/driver-model/design.html#error-codes

+
+       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,
+};


Reply via email to