[v2,5/8] soc: sifive: ccache: Add StarFive JH7110 support

  From: Emil Renner Berthing <kernel@esmil.dk>

This adds support for the StarFive JH7110 SoC which also
features this SiFive cache controller.

Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
---
 arch/riscv/Kconfig.socs            | 1 +
 drivers/soc/Makefile               | 2 +-
 drivers/soc/sifive/Kconfig         | 2 +-
 drivers/soc/sifive/sifive_ccache.c | 1 +
 4 files changed, 4 insertions(+), 2 deletions(-)
  

Hey Emil/Hal,

On Fri, Nov 18, 2022 at 09:17:11AM +0800, Hal Feng wrote:
> From: Emil Renner Berthing <kernel@esmil.dk>
> 
> This adds support for the StarFive JH7110 SoC which also
> features this SiFive cache controller.
> 
> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> ---
>  arch/riscv/Kconfig.socs            | 1 +
>  drivers/soc/Makefile               | 2 +-
>  drivers/soc/sifive/Kconfig         | 2 +-
>  drivers/soc/sifive/sifive_ccache.c | 1 +
>  4 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> index 69774bb362d6..5a40e05f8cab 100644
> --- a/arch/riscv/Kconfig.socs
> +++ b/arch/riscv/Kconfig.socs
> @@ -22,6 +22,7 @@ config SOC_STARFIVE
>  	bool "StarFive SoCs"
>  	select PINCTRL
>  	select RESET_CONTROLLER
> +	select SIFIVE_CCACHE

Please no. I am trying to get rid of these selects + I cannot figure out
why this driver is so important that you *need* to select it. Surely the
SoC is useable without it?

Is this a hang over from your vendor tree that uses the driver to do
non-coherent stuff for the jh7100?

>  	select SIFIVE_PLIC
>  	help
>  	  This enables support for StarFive SoC platform hardware.
> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> index 69ba6508cf2c..534669840858 100644
> --- a/drivers/soc/Makefile
> +++ b/drivers/soc/Makefile
> @@ -26,7 +26,7 @@ obj-y				+= qcom/
>  obj-y				+= renesas/
>  obj-y				+= rockchip/
>  obj-$(CONFIG_SOC_SAMSUNG)	+= samsung/
> -obj-$(CONFIG_SOC_SIFIVE)	+= sifive/
> +obj-y				+= sifive/

This bit is fine.

>  obj-y				+= sunxi/
>  obj-$(CONFIG_ARCH_TEGRA)	+= tegra/
>  obj-y				+= ti/
> diff --git a/drivers/soc/sifive/Kconfig b/drivers/soc/sifive/Kconfig
> index ed4c571f8771..e86870be34c9 100644
> --- a/drivers/soc/sifive/Kconfig
> +++ b/drivers/soc/sifive/Kconfig
> @@ -1,6 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>  
> -if SOC_SIFIVE
> +if SOC_SIFIVE || SOC_STARFIVE

As I suppose is this - but hardly scalable. I suppose it doesn't really
matter.

>  config SIFIVE_CCACHE
>  	bool "Sifive Composable Cache controller"
> diff --git a/drivers/soc/sifive/sifive_ccache.c b/drivers/soc/sifive/sifive_ccache.c
> index 1c171150e878..9489d1a90fbc 100644
> --- a/drivers/soc/sifive/sifive_ccache.c
> +++ b/drivers/soc/sifive/sifive_ccache.c
> @@ -107,6 +107,7 @@ static const struct of_device_id sifive_ccache_ids[] = {
>  	{ .compatible = "sifive,fu540-c000-ccache" },
>  	{ .compatible = "sifive,fu740-c000-ccache" },
>  	{ .compatible = "sifive,ccache0" },
> +	{ .compatible = "starfive,jh7110-ccache" },

Per my second reply to the previous patch, I am not sure why you do not
just have a fallback compatible in the binding/dt for the fu740 ccache
since you appear to have identical configuration?

Thanks,
Conor.
  

On Fri, 18 Nov 2022 at 02:17, Hal Feng <hal.feng@starfivetech.com> wrote:
>
> From: Emil Renner Berthing <kernel@esmil.dk>
>
> This adds support for the StarFive JH7110 SoC which also
> features this SiFive cache controller.
>
> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> ---

I'm fine with this, but it would be great if you could add the jh7100
support at the same time like the original patch did.

>  arch/riscv/Kconfig.socs            | 1 +
>  drivers/soc/Makefile               | 2 +-
>  drivers/soc/sifive/Kconfig         | 2 +-
>  drivers/soc/sifive/sifive_ccache.c | 1 +
>  4 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> index 69774bb362d6..5a40e05f8cab 100644
> --- a/arch/riscv/Kconfig.socs
> +++ b/arch/riscv/Kconfig.socs
> @@ -22,6 +22,7 @@ config SOC_STARFIVE
>         bool "StarFive SoCs"
>         select PINCTRL
>         select RESET_CONTROLLER
> +       select SIFIVE_CCACHE
>         select SIFIVE_PLIC
>         help
>           This enables support for StarFive SoC platform hardware.
> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> index 69ba6508cf2c..534669840858 100644
> --- a/drivers/soc/Makefile
> +++ b/drivers/soc/Makefile
> @@ -26,7 +26,7 @@ obj-y                         += qcom/
>  obj-y                          += renesas/
>  obj-y                          += rockchip/
>  obj-$(CONFIG_SOC_SAMSUNG)      += samsung/
> -obj-$(CONFIG_SOC_SIFIVE)       += sifive/
> +obj-y                          += sifive/
>  obj-y                          += sunxi/
>  obj-$(CONFIG_ARCH_TEGRA)       += tegra/
>  obj-y                          += ti/
> diff --git a/drivers/soc/sifive/Kconfig b/drivers/soc/sifive/Kconfig
> index ed4c571f8771..e86870be34c9 100644
> --- a/drivers/soc/sifive/Kconfig
> +++ b/drivers/soc/sifive/Kconfig
> @@ -1,6 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>
> -if SOC_SIFIVE
> +if SOC_SIFIVE || SOC_STARFIVE
>
>  config SIFIVE_CCACHE
>         bool "Sifive Composable Cache controller"
> diff --git a/drivers/soc/sifive/sifive_ccache.c b/drivers/soc/sifive/sifive_ccache.c
> index 1c171150e878..9489d1a90fbc 100644
> --- a/drivers/soc/sifive/sifive_ccache.c
> +++ b/drivers/soc/sifive/sifive_ccache.c
> @@ -107,6 +107,7 @@ static const struct of_device_id sifive_ccache_ids[] = {
>         { .compatible = "sifive,fu540-c000-ccache" },
>         { .compatible = "sifive,fu740-c000-ccache" },
>         { .compatible = "sifive,ccache0" },
> +       { .compatible = "starfive,jh7110-ccache" },
>         { /* end of table */ }
>  };
>
> --
> 2.38.1
>
  

On Fri, 18 Nov 2022 19:45:57 +0800, Conor Dooley wrote:
> Hey Emil/Hal,
> 
> On Fri, Nov 18, 2022 at 09:17:11AM +0800, Hal Feng wrote:
> > From: Emil Renner Berthing <kernel@esmil.dk>
> > 
> > This adds support for the StarFive JH7110 SoC which also
> > features this SiFive cache controller.
> > 
> > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> > Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> > ---
> >  arch/riscv/Kconfig.socs            | 1 +
> >  drivers/soc/Makefile               | 2 +-
> >  drivers/soc/sifive/Kconfig         | 2 +-
> >  drivers/soc/sifive/sifive_ccache.c | 1 +
> >  4 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> > index 69774bb362d6..5a40e05f8cab 100644
> > --- a/arch/riscv/Kconfig.socs
> > +++ b/arch/riscv/Kconfig.socs
> > @@ -22,6 +22,7 @@ config SOC_STARFIVE
> >  	bool "StarFive SoCs"
> >  	select PINCTRL
> >  	select RESET_CONTROLLER
> > +	select SIFIVE_CCACHE
> 
> Please no. I am trying to get rid of these selects + I cannot figure out
> why this driver is so important that you *need* to select it. Surely the
> SoC is useable without it> 
> Is this a hang over from your vendor tree that uses the driver to do
> non-coherent stuff for the jh7100?

I have tested that the board can successfully boot up without the cache
driver. The `select` can be removed for JH7110. @Emil, what do you think
of this?

> 
> >  	select SIFIVE_PLIC
> >  	help
> >  	  This enables support for StarFive SoC platform hardware.
> > diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> > index 69ba6508cf2c..534669840858 100644
> > --- a/drivers/soc/Makefile
> > +++ b/drivers/soc/Makefile
> > @@ -26,7 +26,7 @@ obj-y				+= qcom/
> >  obj-y				+= renesas/
> >  obj-y				+= rockchip/
> >  obj-$(CONFIG_SOC_SAMSUNG)	+= samsung/
> > -obj-$(CONFIG_SOC_SIFIVE)	+= sifive/
> > +obj-y				+= sifive/
> 
> This bit is fine.
> 
> >  obj-y				+= sunxi/
> >  obj-$(CONFIG_ARCH_TEGRA)	+= tegra/
> >  obj-y				+= ti/
> > diff --git a/drivers/soc/sifive/Kconfig b/drivers/soc/sifive/Kconfig
> > index ed4c571f8771..e86870be34c9 100644
> > --- a/drivers/soc/sifive/Kconfig
> > +++ b/drivers/soc/sifive/Kconfig
> > @@ -1,6 +1,6 @@
> >  # SPDX-License-Identifier: GPL-2.0
> >  
> > -if SOC_SIFIVE
> > +if SOC_SIFIVE || SOC_STARFIVE
> 
> As I suppose is this - but hardly scalable. I suppose it doesn't really
> matter.
> 
> >  config SIFIVE_CCACHE
> >  	bool "Sifive Composable Cache controller"
> > diff --git a/drivers/soc/sifive/sifive_ccache.c b/drivers/soc/sifive/sifive_ccache.c
> > index 1c171150e878..9489d1a90fbc 100644
> > --- a/drivers/soc/sifive/sifive_ccache.c
> > +++ b/drivers/soc/sifive/sifive_ccache.c
> > @@ -107,6 +107,7 @@ static const struct of_device_id sifive_ccache_ids[] = {
> >  	{ .compatible = "sifive,fu540-c000-ccache" },
> >  	{ .compatible = "sifive,fu740-c000-ccache" },
> >  	{ .compatible = "sifive,ccache0" },
> > +	{ .compatible = "starfive,jh7110-ccache" },
> 
> Per my second reply to the previous patch, I am not sure why you do not
> just have a fallback compatible in the binding/dt for the fu740 ccache
> since you appear to have identical configuration?

Yeah, I will use the compatible of fu740 and modify this patch.

Best regards,
Hal
  

On Sat, 19 Nov 2022 01:32:10 +0800, Emil Renner Berthing wrote:
> On Fri, 18 Nov 2022 at 02:17, Hal Feng <hal.feng@starfivetech.com> wrote:
> >
> > From: Emil Renner Berthing <kernel@esmil.dk>
> >
> > This adds support for the StarFive JH7110 SoC which also
> > features this SiFive cache controller.
> >
> > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> > Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> > ---
> 
> I'm fine with this, but it would be great if you could add the jh7100
> support at the same time like the original patch did.

I think this patch series should only add support for JH7110. Maybe
we can make a new patch series to do this.

Best regards,
Hal
  

On Tue, 22 Nov 2022 at 10:03, Hal Feng <hal.feng@starfivetech.com> wrote:
>
> On Fri, 18 Nov 2022 19:45:57 +0800, Conor Dooley wrote:
> > Hey Emil/Hal,
> >
> > On Fri, Nov 18, 2022 at 09:17:11AM +0800, Hal Feng wrote:
> > > From: Emil Renner Berthing <kernel@esmil.dk>
> > >
> > > This adds support for the StarFive JH7110 SoC which also
> > > features this SiFive cache controller.
> > >
> > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> > > Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> > > ---
> > >  arch/riscv/Kconfig.socs            | 1 +
> > >  drivers/soc/Makefile               | 2 +-
> > >  drivers/soc/sifive/Kconfig         | 2 +-
> > >  drivers/soc/sifive/sifive_ccache.c | 1 +
> > >  4 files changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> > > index 69774bb362d6..5a40e05f8cab 100644
> > > --- a/arch/riscv/Kconfig.socs
> > > +++ b/arch/riscv/Kconfig.socs
> > > @@ -22,6 +22,7 @@ config SOC_STARFIVE
> > >     bool "StarFive SoCs"
> > >     select PINCTRL
> > >     select RESET_CONTROLLER
> > > +   select SIFIVE_CCACHE
> >
> > Please no. I am trying to get rid of these selects + I cannot figure out
> > why this driver is so important that you *need* to select it. Surely the
> > SoC is useable without it>
> > Is this a hang over from your vendor tree that uses the driver to do
> > non-coherent stuff for the jh7100?
>
> I have tested that the board can successfully boot up without the cache
> driver. The `select` can be removed for JH7110. @Emil, what do you think
> of this?

Yes, for the JH7110 this is not strictly needed, just like the
Unmatched board. For the StarFive JH7100 it is though.
So if you're only adding support for the JH7110 then it's not needed.

> >
> > >     select SIFIVE_PLIC
> > >     help
> > >       This enables support for StarFive SoC platform hardware.
> > > diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> > > index 69ba6508cf2c..534669840858 100644
> > > --- a/drivers/soc/Makefile
> > > +++ b/drivers/soc/Makefile
> > > @@ -26,7 +26,7 @@ obj-y                             += qcom/
> > >  obj-y                              += renesas/
> > >  obj-y                              += rockchip/
> > >  obj-$(CONFIG_SOC_SAMSUNG)  += samsung/
> > > -obj-$(CONFIG_SOC_SIFIVE)   += sifive/
> > > +obj-y                              += sifive/
> >
> > This bit is fine.
> >
> > >  obj-y                              += sunxi/
> > >  obj-$(CONFIG_ARCH_TEGRA)   += tegra/
> > >  obj-y                              += ti/
> > > diff --git a/drivers/soc/sifive/Kconfig b/drivers/soc/sifive/Kconfig
> > > index ed4c571f8771..e86870be34c9 100644
> > > --- a/drivers/soc/sifive/Kconfig
> > > +++ b/drivers/soc/sifive/Kconfig
> > > @@ -1,6 +1,6 @@
> > >  # SPDX-License-Identifier: GPL-2.0
> > >
> > > -if SOC_SIFIVE
> > > +if SOC_SIFIVE || SOC_STARFIVE
> >
> > As I suppose is this - but hardly scalable. I suppose it doesn't really
> > matter.
> >
> > >  config SIFIVE_CCACHE
> > >     bool "Sifive Composable Cache controller"
> > > diff --git a/drivers/soc/sifive/sifive_ccache.c b/drivers/soc/sifive/sifive_ccache.c
> > > index 1c171150e878..9489d1a90fbc 100644
> > > --- a/drivers/soc/sifive/sifive_ccache.c
> > > +++ b/drivers/soc/sifive/sifive_ccache.c
> > > @@ -107,6 +107,7 @@ static const struct of_device_id sifive_ccache_ids[] = {
> > >     { .compatible = "sifive,fu540-c000-ccache" },
> > >     { .compatible = "sifive,fu740-c000-ccache" },
> > >     { .compatible = "sifive,ccache0" },
> > > +   { .compatible = "starfive,jh7110-ccache" },
> >
> > Per my second reply to the previous patch, I am not sure why you do not
> > just have a fallback compatible in the binding/dt for the fu740 ccache
> > since you appear to have identical configuration?
>
> Yeah, I will use the compatible of fu740 and modify this patch.

No, the JH7110 should not pretend to be a fu740, but if you add

compatible = "starfive,jh7110-ccache", "sifive,ccache0";

then this driver should still match "sifive,ccache0" without adding
the "starfive,jh7110-ccache" entry.

>
> Best regards,
> Hal
  

On Tue, Nov 22, 2022 at 10:54:34AM +0100, Emil Renner Berthing wrote:
> On Tue, 22 Nov 2022 at 10:03, Hal Feng <hal.feng@starfivetech.com> wrote:
> > On Fri, 18 Nov 2022 19:45:57 +0800, Conor Dooley wrote:
> > > Hey Emil/Hal,
> > > On Fri, Nov 18, 2022 at 09:17:11AM +0800, Hal Feng wrote:
> > > > From: Emil Renner Berthing <kernel@esmil.dk>

> > > > diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> > > > index 69774bb362d6..5a40e05f8cab 100644
> > > > --- a/arch/riscv/Kconfig.socs
> > > > +++ b/arch/riscv/Kconfig.socs
> > > > @@ -22,6 +22,7 @@ config SOC_STARFIVE
> > > >     bool "StarFive SoCs"
> > > >     select PINCTRL
> > > >     select RESET_CONTROLLER
> > > > +   select SIFIVE_CCACHE
> > >
> > > Please no. I am trying to get rid of these selects + I cannot figure out
> > > why this driver is so important that you *need* to select it. Surely the
> > > SoC is useable without it>
> > > Is this a hang over from your vendor tree that uses the driver to do
> > > non-coherent stuff for the jh7100?
> >
> > I have tested that the board can successfully boot up without the cache
> > driver. The `select` can be removed for JH7110. @Emil, what do you think
> > of this?
> 
> Yes, for the JH7110 this is not strictly needed, just like the
> Unmatched board. For the StarFive JH7100 it is though.
> So if you're only adding support for the JH7110 then it's not needed.

Even for the JH7100 there are other ways to do this than selects in
arch/riscv - for example
config SIFIVE_CCACHE
	default SOC_STARFIVE

But you don't need that either if you're not adding the JH7100 :)

> > > >  config SIFIVE_CCACHE
> > > >     bool "Sifive Composable Cache controller"
> > > > diff --git a/drivers/soc/sifive/sifive_ccache.c b/drivers/soc/sifive/sifive_ccache.c
> > > > index 1c171150e878..9489d1a90fbc 100644
> > > > --- a/drivers/soc/sifive/sifive_ccache.c
> > > > +++ b/drivers/soc/sifive/sifive_ccache.c
> > > > @@ -107,6 +107,7 @@ static const struct of_device_id sifive_ccache_ids[] = {
> > > >     { .compatible = "sifive,fu540-c000-ccache" },
> > > >     { .compatible = "sifive,fu740-c000-ccache" },
> > > >     { .compatible = "sifive,ccache0" },
> > > > +   { .compatible = "starfive,jh7110-ccache" },
> > >
> > > Per my second reply to the previous patch, I am not sure why you do not
> > > just have a fallback compatible in the binding/dt for the fu740 ccache
> > > since you appear to have identical configuration?
> >
> > Yeah, I will use the compatible of fu740 and modify this patch.
> 
> No, the JH7110 should not pretend to be a fu740, but if you add
> 
> compatible = "starfive,jh7110-ccache", "sifive,ccache0";
> 
> then this driver should still match "sifive,ccache0" without adding
> the "starfive,jh7110-ccache" entry.

Either works for me :) If you go for "sifive,ccache0", just make sure to
add the correct property enforcement - you can just copy the fu740 by
the looks of things (although that'd imply that it is compatible and can
fall back to it...)

Thanks,
Conor.