subject:"\[PATCH 1\/2\] cache\: add sifive composable cache driver"

Re: [PATCH 1/2] cache: add sifive composable cache driver

2021-07-28 Thread Zong Li

On Wed, Jul 28, 2021 at 12:23 PM Sean Anderson  wrote:
>
> 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.

Certainly, let me give more details there.

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

The idea is that the configuration name needs to be able to
distinguish multiple cache devices. For example, if there are other
two devices related to cache, we could give the following three
configurations:
 - SIFIVE_CACHE_CCACHE
 - SIFIVE_CACHE_XXX
 - SIFIVE_CACHE_YYY

SIFIVE_CCACHE is also ok to me, then we would get the following
configuration names in the future.
 - SIFIVE_CCACHE
 - SIFIVE_XXX
 - SIFIVE_YYY

>
> > + 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 00..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 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define SIFIVE_CCACHE_CONFIG 0x000
> > +#define SIFIVE_CCACHE_ENABLE 0x008
>
> WAY_ENABLE?

Changes in the next version.

>
> > +
> > +#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?
>

Yes, change it in the next version.

> > +
> > + 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?
>

sifive_ccache_enable isn't clear enough to me, we couldn't be
straightforward to know what to enable.

> > +}
> > +
> > +static const struct cache_ops sifive_ccache_ops = {
> > + .enable = sifive_ccache_enable,
>
> Please implement get_info as well. It should effectively just be
>

Add get_info in the next version. Thanks.

> 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].
>

Thanks. Modify it in the next version.

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

Re: [PATCH 1/2] cache: add sifive composable cache driver

2021-07-27 Thread Sean Anderson


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 
---
  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 00..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 
+#include 
+#include 
+#include 
+#include 
+
+#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_SHIFT8
+
+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 = _ccache_ops,
+};

[PATCH 1/2] cache: add sifive composable cache driver

2021-07-27 Thread Zong Li

This driver is currently responsible for enabling all ccache ways.

Signed-off-by: Zong Li 
---
 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
+   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 00..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 
+#include 
+#include 
+#include 
+#include 
+
+#define SIFIVE_CCACHE_CONFIG   0x000
+#define SIFIVE_CCACHE_ENABLE   0x008
+
+#define SIFIVE_CCACHE_NUM_WAY_MASK GENMASK(15, 8)
+#define SIFIVE_CCACHE_NUM_WAY_SHIFT8
+
+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;
+
+   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);
+}
+
+static const struct cache_ops sifive_ccache_ops = {
+   .enable = sifive_ccache_enable,
+};
+
+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;
+
+   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 = _ccache_ops,
+};
-- 
2.31.1

Re: [PATCH 1/2] cache: add sifive composable cache driver

Re: [PATCH 1/2] cache: add sifive composable cache driver

[PATCH 1/2] cache: add sifive composable cache driver

3 matches

Site Navigation

Mail list logo

Footer information