All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Anderson <seanga2@gmail.com>
To: Zong Li <zong.li@sifive.com>,
	rick@andestech.com, ycliang@andestech.com, bmeng.cn@gmail.com,
	sjg@chromium.org, green.wan@sifive.com, paul.walmsley@sifive.com,
	u-boot@lists.denx.de
Subject: Re: [PATCH 1/2] cache: add sifive composable cache driver
Date: Wed, 28 Jul 2021 00:23:55 -0400	[thread overview]
Message-ID: <b4907424-e160-14b7-9bec-a2ec7fb854dc@gmail.com> (raw)
In-Reply-To: <20210727085452.25708-1-zong.li@sifive.com>

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


  parent reply	other threads:[~2021-07-28  4:24 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-27  8:54 Zong Li
2021-07-27  8:54 ` [PATCH 2/2] board: sifive: use ccache driver instead of helper function Zong Li
2021-07-28  4:29   ` Sean Anderson
2021-07-28  7:25     ` Zong Li
2021-07-28 15:18       ` Sean Anderson
2021-07-29 12:26         ` Zong Li
2021-07-30  9:16           ` Zong Li
2021-07-28  4:23 ` Sean Anderson [this message]
2021-07-28 10:40   ` [PATCH 1/2] cache: add sifive composable cache driver Zong Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b4907424-e160-14b7-9bec-a2ec7fb854dc@gmail.com \
    --to=seanga2@gmail.com \
    --cc=bmeng.cn@gmail.com \
    --cc=green.wan@sifive.com \
    --cc=paul.walmsley@sifive.com \
    --cc=rick@andestech.com \
    --cc=sjg@chromium.org \
    --cc=u-boot@lists.denx.de \
    --cc=ycliang@andestech.com \
    --cc=zong.li@sifive.com \
    --subject='Re: [PATCH 1/2] cache: add sifive composable cache driver' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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.