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@xxxxxxxxxxxxxxxx> 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@xxxxxxxx> > > > > 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.