Re: [PATCH v2 2/2] soc: sifive: ccache: Add StarFive JH7100 support

[Conor Dooley]

On Tue, Oct 31, 2023 at 03:14:44PM +0100, Emil Renner Berthing wrote:
> From: Emil Renner Berthing <kernel@xxxxxxxx>
> 
> This adds support for the StarFive JH7100 SoC which also features this
> SiFive cache controller.
> 
> The JH7100 has non-coherent DMAs but predate the standard RISC-V Zicbom
> exension, so instead we need to use this cache controller for
> non-standard cache management operations.
> 
> Unfortunately the interrupt for uncorrected data is broken on the JH7100
> and fires continuously, so add a quirk to not register a handler for it.
> 
> Signed-off-by: Emil Renner Berthing <kernel@xxxxxxxx>
> ---
>  drivers/soc/sifive/sifive_ccache.c | 62 +++++++++++++++++++++++++++++-
>  1 file changed, 60 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/sifive/sifive_ccache.c b/drivers/soc/sifive/sifive_ccache.c
> index 3684f5b40a80..0da3d1bd0866 100644
> --- a/drivers/soc/sifive/sifive_ccache.c
> +++ b/drivers/soc/sifive/sifive_ccache.c
> @@ -8,13 +8,16 @@
>  
>  #define pr_fmt(fmt) "CCACHE: " fmt
>  
> +#include <linux/align.h>
>  #include <linux/debugfs.h>
>  #include <linux/interrupt.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_address.h>
>  #include <linux/device.h>
>  #include <linux/bitfield.h>
> +#include <asm/cacheflush.h>
>  #include <asm/cacheinfo.h>
> +#include <asm/dma-noncoherent.h>
>  #include <soc/sifive/sifive_ccache.h>
>  
>  #define SIFIVE_CCACHE_DIRECCFIX_LOW 0x100
> @@ -39,10 +42,14 @@
>  #define SIFIVE_CCACHE_CONFIG_SETS_MASK GENMASK_ULL(23, 16)
>  #define SIFIVE_CCACHE_CONFIG_BLKS_MASK GENMASK_ULL(31, 24)
>  
> +#define SIFIVE_CCACHE_FLUSH64 0x200
> +#define SIFIVE_CCACHE_FLUSH32 0x240
> +
>  #define SIFIVE_CCACHE_WAYENABLE 0x08
>  #define SIFIVE_CCACHE_ECCINJECTERR 0x40
>  
>  #define SIFIVE_CCACHE_MAX_ECCINTR 4
> +#define SIFIVE_CCACHE_LINE_SIZE 64
>  
>  static void __iomem *ccache_base;
>  static int g_irq[SIFIVE_CCACHE_MAX_ECCINTR];
> @@ -56,6 +63,11 @@ enum {
>  	DIR_UNCORR,
>  };
>  
> +enum {
> +	QUIRK_NONSTANDARD_CACHE_OPS	= BIT(0),
> +	QUIRK_BROKEN_DATA_UNCORR	= BIT(1),
> +};
> +
>  #ifdef CONFIG_DEBUG_FS
>  static struct dentry *sifive_test;
>  
> @@ -106,6 +118,8 @@ static void ccache_config_read(void)
>  static const struct of_device_id sifive_ccache_ids[] = {
>  	{ .compatible = "sifive,fu540-c000-ccache" },
>  	{ .compatible = "sifive,fu740-c000-ccache" },
> +	{ .compatible = "starfive,jh7100-ccache",
> +	  .data = (void *)(QUIRK_NONSTANDARD_CACHE_OPS | QUIRK_BROKEN_DATA_UNCORR) },
>  	{ .compatible = "sifive,ccache0" },
>  	{ /* end of table */ }
>  };
> @@ -124,6 +138,34 @@ int unregister_sifive_ccache_error_notifier(struct notifier_block *nb)
>  }
>  EXPORT_SYMBOL_GPL(unregister_sifive_ccache_error_notifier);
>  
> +#ifdef CONFIG_RISCV_NONSTANDARD_CACHE_OPS
> +static void ccache_flush_range(phys_addr_t start, size_t len)
> +{
> +	phys_addr_t end = start + len;
> +	phys_addr_t line;
> +
> +	if (!len)
> +		return;
> +
> +	mb();

Apparently memory barriers are supposed to be commented as to why they
are required.
I'm not sure if I care about that particular checkpatch complaint here.

> +	for (line = ALIGN_DOWN(start, SIFIVE_CCACHE_LINE_SIZE); line < end;
> +			line += SIFIVE_CCACHE_LINE_SIZE) {
> +#ifdef CONFIG_32BIT
> +		writel(line >> 4, ccache_base + SIFIVE_CCACHE_FLUSH32);
> +#else
> +		writeq(line, ccache_base + SIFIVE_CCACHE_FLUSH64);
> +#endif
> +		mb();
> +	}
> +}
> +
> +static const struct riscv_nonstd_cache_ops ccache_mgmt_ops __initdata = {

And apparently this should be __initconst rather than __initdata. I can
squash that in.

Cheers,
Conor.

Attachment: signature.asc
Description: PGP signature