lkml.org 
[lkml]   [2022]   [Aug]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 2/3] soc: sifive: l2 cache: Rename SiFive L2 cache to composible cache.
On Mon, Aug 29, 2022 at 3:05 PM <Conor.Dooley@microchip.com> wrote:
>
> Hey Zong,
>
> On 29/08/2022 07:22, Zong Li wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > From: Greentime Hu <greentime.hu@sifive.com>
> >
> > Since composible cache may be L3 cache if pL2 cache exists, we should use
> > its original name composible cache to prevent confusion.
> >
> > Signed-off-by: Greentime Hu <greentime.hu@sifive.com>
> > Signed-off-by: Zong Li <zong.li@sifive.com>
> > ---
> > drivers/soc/sifive/Kconfig | 7 +-
> > drivers/soc/sifive/Makefile | 2 +-
> > drivers/soc/sifive/sifive_ccache.c | 221 +++++++++++++++++++++++++
> > drivers/soc/sifive/sifive_l2_cache.c | 237 ---------------------------
>
> Where did the 16 lines go? Could you please split renames off from any
> other changes so that it is easier to see what has changed?

As your suggestion, we should separate the difference from this patch

>
> > include/soc/sifive/sifive_ccache.h | 16 ++
> > include/soc/sifive/sifive_l2_cache.h | 16 --
> > 6 files changed, 242 insertions(+), 257 deletions(-)
> > create mode 100644 drivers/soc/sifive/sifive_ccache.c
> > delete mode 100644 drivers/soc/sifive/sifive_l2_cache.c
> > create mode 100644 include/soc/sifive/sifive_ccache.h
> > delete mode 100644 include/soc/sifive/sifive_l2_cache.h
> >
> > diff --git a/drivers/soc/sifive/Kconfig b/drivers/soc/sifive/Kconfig
> > index 58cf8c40d08d..3d65d2771f9a 100644
> > --- a/drivers/soc/sifive/Kconfig
> > +++ b/drivers/soc/sifive/Kconfig
> > @@ -2,9 +2,10 @@
> >
> > if SOC_SIFIVE
> >
> > -config SIFIVE_L2
> > - bool "Sifive L2 Cache controller"
> > +config SIFIVE_CCACHE
> > + bool "Sifive composable Cache controller"
> > + default y
>
> Changing this to default on is not a rename of the file..
> This should be in a different patch.

Okay, let me separate it to another patch, but I guess we could remove
it, and enable it by user. What is your perspective on it?

>
> > help
> > - Support for the L2 cache controller on SiFive platforms.
> > + Support for the composable cache controller on SiFive platforms.
> >
> > endif
> > diff --git a/drivers/soc/sifive/Makefile b/drivers/soc/sifive/Makefile
> > index b5caff77938f..1f5dc339bf82 100644
> > --- a/drivers/soc/sifive/Makefile
> > +++ b/drivers/soc/sifive/Makefile
> > @@ -1,3 +1,3 @@
> > # SPDX-License-Identifier: GPL-2.0
> >
> > -obj-$(CONFIG_SIFIVE_L2) += sifive_l2_cache.o
> > +obj-$(CONFIG_SIFIVE_CCACHE) += sifive_ccache.o
> > diff --git a/drivers/soc/sifive/sifive_ccache.c b/drivers/soc/sifive/sifive_ccache.c
> > new file mode 100644
> > index 000000000000..46ce33db7d30
> > --- /dev/null
> > +++ b/drivers/soc/sifive/sifive_ccache.c
> > @@ -0,0 +1,221 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * SiFive composable cache controller Driver
> > + *
> > + * Copyright (C) 2018-2019 SiFive, Inc.
> > + *
> > + */
> > +#include <linux/debugfs.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/of_address.h>
> > +#include <linux/device.h>
> > +#include <asm/cacheinfo.h>
> > +#include <soc/sifive/sifive_ccache.h>
> > +
> > +#define SIFIVE_CCACHE_DIRECCFIX_LOW 0x100
> > +#define SIFIVE_CCACHE_DIRECCFIX_HIGH 0x104
> > +#define SIFIVE_CCACHE_DIRECCFIX_COUNT 0x108
> > +
> > +#define SIFIVE_CCACHE_DATECCFIX_LOW 0x140
> > +#define SIFIVE_CCACHE_DATECCFIX_HIGH 0x144
> > +#define SIFIVE_CCACHE_DATECCFIX_COUNT 0x148
> > +
> > +#define SIFIVE_CCACHE_DATECCFAIL_LOW 0x160
> > +#define SIFIVE_CCACHE_DATECCFAIL_HIGH 0x164
> > +#define SIFIVE_CCACHE_DATECCFAIL_COUNT 0x168
> > +
> > +#define SIFIVE_CCACHE_CONFIG 0x00
> > +#define SIFIVE_CCACHE_WAYENABLE 0x08
> > +#define SIFIVE_CCACHE_ECCINJECTERR 0x40
>
> From what I can see, you've also changed these around too?
> Please generate the patch's diff so that the rename is detected & the
> diff shows only what changed in the file. The -M option is what you
> are looking for.
>
> I have a couple other comments to make about what's changed here other,
> thatn the rename but I will do so against a v2 where the diff is usable.
>

Let me sent the v2 patch, and separate the rename and diff part

> Thanks,
> Conor.
>
> > +
> > +#define SIFIVE_CCACHE_MAX_ECCINTR 3

\
 
 \ /
  Last update: 2022-08-29 10:41    [W:0.658 / U:0.136 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site