Message ID | 20221118011714.70877-1-hal.feng@starfivetech.com |
---|---|
Headers | show |
Series | Basic device tree support for StarFive JH7110 RISC-V SoC | expand |
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.
On Fri, 18 Nov 2022 09:17:14 +0800, Hal Feng wrote: > Add CONFIG_SERIAL_8250_DW=y, which is a necessary option for > StarFive JH7110 and JH7100 SoCs to boot with serial ports. > > Applied, thanks! [8/8] RISC-V: defconfig: Enable CONFIG_SERIAL_8250_DW https://git.kernel.org/palmer/c/6925ba3d9b8c Best regards,
Hello: This series was applied to riscv/linux.git (for-next) by Palmer Dabbelt <palmer@rivosinc.com>: On Fri, 18 Nov 2022 09:17:06 +0800 you wrote: > The original patch series "Basic StarFive JH7110 RISC-V SoC support" [1] > is split into 3 patch series. They respectively add basic clock&reset, > pinctrl and device tree support for StarFive JH7110 SoC. These patch > series are independent, but the Visionfive2 board can boot up successfully > only if all these patches series applied. This one adds basic device > tree support. This patch series is pulled out from the patch 1~6 and > patch 27~30 of v1 [1]. You can simply get or review the patches at the > link [2]. > > [...] Here is the summary with links: - [v2,1/8] dt-bindings: riscv: Add StarFive JH7110 SoC and VisionFive2 board (no matching commit) - [v2,2/8] dt-bindings: timer: Add StarFive JH7110 clint (no matching commit) - [v2,3/8] dt-bindings: interrupt-controller: Add StarFive JH7110 plic (no matching commit) - [v2,4/8] dt-bindings: sifive,ccache0: Support StarFive JH7110 SoC (no matching commit) - [v2,5/8] soc: sifive: ccache: Add StarFive JH7110 support (no matching commit) - [v2,6/8] riscv: dts: starfive: Add initial StarFive JH7110 device tree (no matching commit) - [v2,7/8] riscv: dts: starfive: Add StarFive JH7110 VisionFive2 board device tree (no matching commit) - [v2,8/8] RISC-V: defconfig: Enable CONFIG_SERIAL_8250_DW https://git.kernel.org/riscv/c/6925ba3d9b8c You are awesome, thank you!
On Fri, 02 Dec 2022 11:00:17 PST (-0800), patchwork-bot+linux-riscv@kernel.org wrote: > Hello: > > This series was applied to riscv/linux.git (for-next) > by Palmer Dabbelt <palmer@rivosinc.com>: > > On Fri, 18 Nov 2022 09:17:06 +0800 you wrote: >> The original patch series "Basic StarFive JH7110 RISC-V SoC support" [1] >> is split into 3 patch series. They respectively add basic clock&reset, >> pinctrl and device tree support for StarFive JH7110 SoC. These patch >> series are independent, but the Visionfive2 board can boot up successfully >> only if all these patches series applied. This one adds basic device >> tree support. This patch series is pulled out from the patch 1~6 and >> patch 27~30 of v1 [1]. You can simply get or review the patches at the >> link [2]. >> >> [...] > > Here is the summary with links: > - [v2,1/8] dt-bindings: riscv: Add StarFive JH7110 SoC and VisionFive2 board > (no matching commit) > - [v2,2/8] dt-bindings: timer: Add StarFive JH7110 clint > (no matching commit) > - [v2,3/8] dt-bindings: interrupt-controller: Add StarFive JH7110 plic > (no matching commit) > - [v2,4/8] dt-bindings: sifive,ccache0: Support StarFive JH7110 SoC > (no matching commit) > - [v2,5/8] soc: sifive: ccache: Add StarFive JH7110 support > (no matching commit) > - [v2,6/8] riscv: dts: starfive: Add initial StarFive JH7110 device tree > (no matching commit) > - [v2,7/8] riscv: dts: starfive: Add StarFive JH7110 VisionFive2 board device tree > (no matching commit) > - [v2,8/8] RISC-V: defconfig: Enable CONFIG_SERIAL_8250_DW > https://git.kernel.org/riscv/c/6925ba3d9b8c > > You are awesome, thank you! Looks like the bot is a little confused here, it's just that last patch that's been merged.