[PATCH 2/2] board: sifive: use ccache driver instead of helper function

Sean Anderson seanga2 at gmail.com
Wed Jul 28 06:29:46 CEST 2021


On 7/27/21 4:54 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 at sifive.com>
> ---
>   arch/riscv/cpu/fu540/Kconfig              |  1 +
>   arch/riscv/cpu/fu540/cache.c              | 62 ++++++++---------------
>   arch/riscv/cpu/fu740/Kconfig              |  1 +
>   arch/riscv/cpu/fu740/cache.c              | 60 +++++++---------------
>   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, 45 insertions(+), 102 deletions(-)
> 
> diff --git a/arch/riscv/cpu/fu540/Kconfig b/arch/riscv/cpu/fu540/Kconfig
> index 05463b2625..1f50b823ed 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_CACHE_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..3c754c4614 100644
> --- a/arch/riscv/cpu/fu540/cache.c
> +++ b/arch/riscv/cpu/fu540/cache.c
> @@ -1,55 +1,33 @@
>   // SPDX-License-Identifier: GPL-2.0+
>   /*
> - * Copyright (C) 2020 SiFive, Inc
> + * Copyright (C) 2020 - 2021 SiFive, Inc
>    *
>    * Authors:
>    *   Pragnesh Patel <pragnesh.patel at 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)
>   {

Is there any reason this function is duplicated? See below for further comments.

> -	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;
> -
> -	config = readl((volatile u32 *)base + L2_CACHE_CONFIG);
> -	ways = (config & MASK_NUM_WAYS) >> NUM_WAYS_SHIFT;
> -
> -	enable = (volatile u32 *)(base + L2_CACHE_ENABLE);
> +	struct udevice *dev;
> +	int ret;
> +
> +	ret = uclass_get_device_by_driver(UCLASS_CACHE,
> +					  DM_DRIVER_GET(sifive_ccache),
> +					  &dev);
> +	if (ret) {
> +		pr_debug("%s: could not enable cache ways\n", __func__);
> +		return ret;
> +	}
> +
> +	ret = cache_enable(dev);
> +	if (ret) {
> +		pr_debug("%s: ccache enable filed\n", __func__);
> +		return 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 8e54310b9c..87d8016c88 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_CACHE_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..1d6c295d98 100644
> --- a/arch/riscv/cpu/fu740/cache.c
> +++ b/arch/riscv/cpu/fu740/cache.c
> @@ -7,49 +7,27 @@
>    */
>   
>   #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;
> -
> -	config = readl((volatile u32 *)base + L2_CACHE_CONFIG);
> -	ways = (config & MASK_NUM_WAYS) >> NUM_WAYS_SHIFT;
> -
> -	enable = (volatile u32 *)(base + L2_CACHE_ENABLE);
> +	struct udevice *dev;
> +	int ret;
> +
> +	ret = uclass_get_device_by_driver(UCLASS_CACHE,
> +					  DM_DRIVER_GET(sifive_ccache),
> +					  &dev);
> +	if (ret) {
> +		pr_debug("%s: could not enable cache ways\n", __func__);
> +		return ret;

return log_msg_ret(...);

(and you don't have to add a __func__ parameter if you use that function)

> +	}
> +
> +	ret = cache_enable(dev);
> +	if (ret) {
> +		pr_debug("%s: ccache enable filed\n", __func__);

failed; ditto

> +		return ret;
> +	}
>   
> -	/* memory barrier */
> -	mb();
> -	(*enable) = ways - 1;
> -	/* memory barrier */
> -	mb();

Is this handled by the __iowmb/__iormb in writel/readl in the driver?

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

ditto, though here I would just skip the error message since you reported it above already

> -	}
> -
> -	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__);

ditto

> -		return ret;
> -	}
> -	return 0;
> +	return sifive_ccache_enable_ways();
>   }
> 

--Sean


More information about the U-Boot mailing list