2024-03-05 23:39:15

by Guo Ren

[permalink] [raw]
Subject: Re: [PATCH v5 5/7] riscv: Kconfig.socs: Split ARCH_CANAAN and SOC_CANAAN_K210

This patch cross so many subsystems, I am not sure about it. If I were
you, I would keep SOC_CANAAN and just add SOC_CANAAN_K230.

On Wed, Mar 6, 2024 at 7:04 AM Yangyu Chen <[email protected]> wrote:
>
> Since we have Canaan Kendryte K230 with MMU now. The use of SOC_CANAAN
> is no longer only referred to K210. Split them and add _K210 suffix
> to the name for old SOC_CANAAN. And allows ARCH_CANAAN to be selected
> for other Canaan SoCs.
>
> Signed-off-by: Yangyu Chen <[email protected]>
> ---
> arch/riscv/Kconfig.socs | 8 +++++---
> arch/riscv/Makefile | 2 +-
> arch/riscv/configs/nommu_k210_defconfig | 3 ++-
> arch/riscv/configs/nommu_k210_sdcard_defconfig | 3 ++-
> drivers/clk/Kconfig | 4 ++--
> drivers/pinctrl/Kconfig | 4 ++--
> drivers/reset/Kconfig | 4 ++--
> drivers/soc/Makefile | 2 +-
> drivers/soc/canaan/Kconfig | 4 ++--
> 9 files changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> index 623de5f8a208..5710aee456ac 100644
> --- a/arch/riscv/Kconfig.socs
> +++ b/arch/riscv/Kconfig.socs
> @@ -72,11 +72,13 @@ config SOC_VIRT
> This enables support for QEMU Virt Machine.
>
> config ARCH_CANAAN
> - def_bool SOC_CANAAN
> + bool "Canaan Kendryte SoC"
> + help
> + This enables support for Canaan Kendryte SoC platform hardware.
>
> -config SOC_CANAAN
> +config SOC_CANAAN_K210
> bool "Canaan Kendryte K210 SoC"
> - depends on !MMU
> + depends on !MMU && ARCH_CANAAN
> select CLINT_TIMER if RISCV_M_MODE
> select ARCH_HAS_RESET_CONTROLLER
> select PINCTRL
> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> index 252d63942f34..fa6c389c3986 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -154,7 +154,7 @@ vdso-install-y += arch/riscv/kernel/vdso/vdso.so.dbg
> vdso-install-$(CONFIG_COMPAT) += arch/riscv/kernel/compat_vdso/compat_vdso.so.dbg:../compat_vdso/compat_vdso.so
>
> ifneq ($(CONFIG_XIP_KERNEL),y)
> -ifeq ($(CONFIG_RISCV_M_MODE)$(CONFIG_ARCH_CANAAN),yy)
> +ifeq ($(CONFIG_RISCV_M_MODE)$(CONFIG_SOC_CANAAN_K210),yy)
> KBUILD_IMAGE := $(boot)/loader.bin
> else
> ifeq ($(CONFIG_EFI_ZBOOT),)
> diff --git a/arch/riscv/configs/nommu_k210_defconfig b/arch/riscv/configs/nommu_k210_defconfig
> index 7e75200543f4..2552e78074a3 100644
> --- a/arch/riscv/configs/nommu_k210_defconfig
> +++ b/arch/riscv/configs/nommu_k210_defconfig
> @@ -27,7 +27,8 @@ CONFIG_EXPERT=y
> CONFIG_SLUB=y
> CONFIG_SLUB_TINY=y
> # CONFIG_MMU is not set
> -CONFIG_SOC_CANAAN=y
> +CONFIG_ARCH_CANAAN=y
> +CONFIG_SOC_CANAAN_K210=y
> CONFIG_NONPORTABLE=y
> CONFIG_SMP=y
> CONFIG_NR_CPUS=2
> diff --git a/arch/riscv/configs/nommu_k210_sdcard_defconfig b/arch/riscv/configs/nommu_k210_sdcard_defconfig
> index 0ba353e9ca71..8f67fb830585 100644
> --- a/arch/riscv/configs/nommu_k210_sdcard_defconfig
> +++ b/arch/riscv/configs/nommu_k210_sdcard_defconfig
> @@ -19,7 +19,8 @@ CONFIG_EXPERT=y
> CONFIG_SLUB=y
> CONFIG_SLUB_TINY=y
> # CONFIG_MMU is not set
> -CONFIG_SOC_CANAAN=y
> +CONFIG_ARCH_CANAAN=y
> +CONFIG_SOC_CANAAN_K210=y
> CONFIG_NONPORTABLE=y
> CONFIG_SMP=y
> CONFIG_NR_CPUS=2
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 50af5fc7f570..7517a0dfd15c 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -451,8 +451,8 @@ config COMMON_CLK_FIXED_MMIO
>
> config COMMON_CLK_K210
> bool "Clock driver for the Canaan Kendryte K210 SoC"
> - depends on OF && RISCV && SOC_CANAAN
> - default SOC_CANAAN
> + depends on OF && RISCV && SOC_CANAAN_K210
> + default SOC_CANAAN_K210
> help
> Support for the Canaan Kendryte K210 RISC-V SoC clocks.
>
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 8163a5983166..837b3bac8aac 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -217,13 +217,13 @@ config PINCTRL_INGENIC
>
> config PINCTRL_K210
> bool "Pinctrl driver for the Canaan Kendryte K210 SoC"
> - depends on RISCV && SOC_CANAAN && OF
> + depends on RISCV && SOC_CANAAN_K210 && OF
> select GENERIC_PINMUX_FUNCTIONS
> select GENERIC_PINCONF
> select GPIOLIB
> select OF_GPIO
> select REGMAP_MMIO
> - default SOC_CANAAN
> + default SOC_CANAAN_K210
> help
> Add support for the Canaan Kendryte K210 RISC-V SOC Field
> Programmable IO Array (FPIOA) controller.
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index ccd59ddd7610..6499da7ecc3b 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -94,9 +94,9 @@ config RESET_INTEL_GW
>
> config RESET_K210
> bool "Reset controller driver for Canaan Kendryte K210 SoC"
> - depends on (SOC_CANAAN || COMPILE_TEST) && OF
> + depends on (SOC_CANAAN_K210 || COMPILE_TEST) && OF
> select MFD_SYSCON
> - default SOC_CANAAN
> + default SOC_CANAAN_K210
> help
> Support for the Canaan Kendryte K210 RISC-V SoC reset controller.
> Say Y if you want to control reset signals provided by this
> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> index ba8f5b5460e1..fb2bd31387d0 100644
> --- a/drivers/soc/Makefile
> +++ b/drivers/soc/Makefile
> @@ -7,7 +7,7 @@ obj-y += apple/
> obj-y += aspeed/
> obj-$(CONFIG_ARCH_AT91) += atmel/
> obj-y += bcm/
> -obj-$(CONFIG_SOC_CANAAN) += canaan/
> +obj-$(CONFIG_ARCH_CANAAN) += canaan/
> obj-$(CONFIG_ARCH_DOVE) += dove/
> obj-$(CONFIG_MACH_DOVE) += dove/
> obj-y += fsl/
> diff --git a/drivers/soc/canaan/Kconfig b/drivers/soc/canaan/Kconfig
> index 43ced2bf8444..3121d351fea6 100644
> --- a/drivers/soc/canaan/Kconfig
> +++ b/drivers/soc/canaan/Kconfig
> @@ -2,9 +2,9 @@
>
> config SOC_K210_SYSCTL
> bool "Canaan Kendryte K210 SoC system controller"
> - depends on RISCV && SOC_CANAAN && OF
> + depends on RISCV && SOC_CANAAN_K210 && OF
> depends on COMMON_CLK_K210
> - default SOC_CANAAN
> + default SOC_CANAAN_K210
> select PM
> select MFD_SYSCON
> help
> --
> 2.43.0
>


--
Best Regards
Guo Ren


2024-03-06 08:04:01

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v5 5/7] riscv: Kconfig.socs: Split ARCH_CANAAN and SOC_CANAAN_K210

On Wed, Mar 06, 2024 at 07:38:52AM +0800, Guo Ren wrote:

> On Wed, Mar 6, 2024 at 7:04 AM Yangyu Chen <[email protected]> wrote:
> >
> > Since we have Canaan Kendryte K230 with MMU now. The use of SOC_CANAAN
> > is no longer only referred to K210. Split them and add _K210 suffix
> > to the name for old SOC_CANAAN. And allows ARCH_CANAAN to be selected
> > for other Canaan SoCs.
> >
> > Signed-off-by: Yangyu Chen <[email protected]>
> > ---
> > arch/riscv/Kconfig.socs | 8 +++++---
> > arch/riscv/Makefile | 2 +-
> > arch/riscv/configs/nommu_k210_defconfig | 3 ++-
> > arch/riscv/configs/nommu_k210_sdcard_defconfig | 3 ++-
> > drivers/clk/Kconfig | 4 ++--
> > drivers/pinctrl/Kconfig | 4 ++--
> > drivers/reset/Kconfig | 4 ++--
> > drivers/soc/Makefile | 2 +-
> > drivers/soc/canaan/Kconfig | 4 ++--
> > 9 files changed, 19 insertions(+), 15 deletions(-)

> This patch cross so many subsystems, I am not sure about it. If I were
> you, I would keep SOC_CANAAN and just add SOC_CANAAN_K230.

Right. That is why I didn't try to rename the symbol, and just left it
as SOC_CANAAN, but if the relevant people ack it, the chances of a
significant conflict are low.


Attachments:
(No filename) (1.35 kB)
signature.asc (235.00 B)
Download all attachments

2024-03-06 08:15:00

by Yangyu Chen

[permalink] [raw]
Subject: Re: [PATCH v5 5/7] riscv: Kconfig.socs: Split ARCH_CANAAN and SOC_CANAAN_K210



> On Mar 6, 2024, at 16:01, Conor Dooley <[email protected]> wrote:
>
> On Wed, Mar 06, 2024 at 07:38:52AM +0800, Guo Ren wrote:
>
>> On Wed, Mar 6, 2024 at 7:04 AM Yangyu Chen <[email protected]> wrote:
>>>
>>> Since we have Canaan Kendryte K230 with MMU now. The use of SOC_CANAAN
>>> is no longer only referred to K210. Split them and add _K210 suffix
>>> to the name for old SOC_CANAAN. And allows ARCH_CANAAN to be selected
>>> for other Canaan SoCs.
>>>
>>> Signed-off-by: Yangyu Chen <[email protected]>
>>> ---
>>> arch/riscv/Kconfig.socs | 8 +++++---
>>> arch/riscv/Makefile | 2 +-
>>> arch/riscv/configs/nommu_k210_defconfig | 3 ++-
>>> arch/riscv/configs/nommu_k210_sdcard_defconfig | 3 ++-
>>> drivers/clk/Kconfig | 4 ++--
>>> drivers/pinctrl/Kconfig | 4 ++--
>>> drivers/reset/Kconfig | 4 ++--
>>> drivers/soc/Makefile | 2 +-
>>> drivers/soc/canaan/Kconfig | 4 ++--
>>> 9 files changed, 19 insertions(+), 15 deletions(-)
>
>> This patch cross so many subsystems, I am not sure about it. If I were
>> you, I would keep SOC_CANAAN and just add SOC_CANAAN_K230.
>
> Right. That is why I didn't try to rename the symbol, and just left it
> as SOC_CANAAN, but if the relevant people ack it, the chances of a
> significant conflict are low.
>

Maybe I should split this patch into different subsystems for better
review. I think at least drivers/soc/Makefile should changed to use
ARCH_CANAAN. Because we need some SoC drivers for K230 in the future.
And arch/riscv/Makefile should use SOC_CANAAN_K210 instead of
ARCH_CANAAN. Because we should avoid the M-Mode loader build for
other Canaan SoCs except for K210.

2024-03-20 17:40:02

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v5 5/7] riscv: Kconfig.socs: Split ARCH_CANAAN and SOC_CANAAN_K210

On Wed, Mar 06, 2024 at 04:14:33PM +0800, Yangyu Chen wrote:
>
>
> > On Mar 6, 2024, at 16:01, Conor Dooley <[email protected]> wrote:
> >
> > On Wed, Mar 06, 2024 at 07:38:52AM +0800, Guo Ren wrote:
> >
> >> On Wed, Mar 6, 2024 at 7:04 AM Yangyu Chen <[email protected]> wrote:
> >>>
> >>> Since we have Canaan Kendryte K230 with MMU now. The use of SOC_CANAAN
> >>> is no longer only referred to K210. Split them and add _K210 suffix
> >>> to the name for old SOC_CANAAN. And allows ARCH_CANAAN to be selected
> >>> for other Canaan SoCs.
> >>>
> >>> Signed-off-by: Yangyu Chen <[email protected]>
> >>> ---
> >>> arch/riscv/Kconfig.socs | 8 +++++---
> >>> arch/riscv/Makefile | 2 +-
> >>> arch/riscv/configs/nommu_k210_defconfig | 3 ++-
> >>> arch/riscv/configs/nommu_k210_sdcard_defconfig | 3 ++-
> >>> drivers/clk/Kconfig | 4 ++--
> >>> drivers/pinctrl/Kconfig | 4 ++--
> >>> drivers/reset/Kconfig | 4 ++--
> >>> drivers/soc/Makefile | 2 +-
> >>> drivers/soc/canaan/Kconfig | 4 ++--
> >>> 9 files changed, 19 insertions(+), 15 deletions(-)
> >
> >> This patch cross so many subsystems, I am not sure about it. If I were
> >> you, I would keep SOC_CANAAN and just add SOC_CANAAN_K230.
> >
> > Right. That is why I didn't try to rename the symbol, and just left it
> > as SOC_CANAAN, but if the relevant people ack it, the chances of a
> > significant conflict are low.
> >
>
> Maybe I should split this patch into different subsystems for better
> review. I think at least drivers/soc/Makefile should changed to use
> ARCH_CANAAN. Because we need some SoC drivers for K230 in the future.
> And arch/riscv/Makefile should use SOC_CANAAN_K210 instead of
> ARCH_CANAAN. Because we should avoid the M-Mode loader build for
> other Canaan SoCs except for K210.

It seems like what Damien requested is pretty much what's done here.
Can you resend this CCing the maintainers for clk pinctrl and reset?
If you leave a note under the --- line in this patch about wanting acks
to take this via riscv, I don't mind picking up this treewide patch if
the individual maintainers ack it. I don't think there's likely to be a
significant conflict caused by it going through one tree.

I got a k230 board (the canmv one) so I should be able to test this
myself before picking stuff up.

Cheers,
Conor.


Attachments:
(No filename) (2.48 kB)
signature.asc (235.00 B)
Download all attachments

2024-03-20 18:54:31

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v5 5/7] riscv: Kconfig.socs: Split ARCH_CANAAN and SOC_CANAAN_K210

On Wed, Mar 20, 2024 at 05:39:14PM +0000, Conor Dooley wrote:

> I got a k230 board (the canmv one) so I should be able to test this
> myself before picking stuff up.

I've taken a bit of a look at the "sdk" and appears to be a complete
mess to a non-chinese speaker like me.
I know you linked a copy of opensbi to use with this, but do you also
have a version of U-Boot to use with this that is not riddled with
crap and will compile with a normal toolchain?

I have chanced upon Courmisch's repo that looks significantly more
usable than whatever Canaan have so I guess I will use that:
https://code.videolan.org/Courmisch/k230-boot

:)


Attachments:
(No filename) (655.00 B)
signature.asc (235.00 B)
Download all attachments

2024-03-21 02:33:54

by Yangyu Chen

[permalink] [raw]
Subject: Re: [PATCH v5 5/7] riscv: Kconfig.socs: Split ARCH_CANAAN and SOC_CANAAN_K210



> On Mar 21, 2024, at 02:51, Conor Dooley <[email protected]> wrote:
>
> On Wed, Mar 20, 2024 at 05:39:14PM +0000, Conor Dooley wrote:
>
>> I got a k230 board (the canmv one) so I should be able to test this
>> myself before picking stuff up.
>
> I've taken a bit of a look at the "sdk" and appears to be a complete
> mess to a non-chinese speaker like me.
> I know you linked a copy of opensbi to use with this, but do you also
> have a version of U-Boot to use with this that is not riddled with
> crap and will compile with a normal toolchain?
>

I don’t have one. But I know Revy (Han Gao) built a u-boot repo
from vendor SDK which is at submodule on
https://github.com/revyos/mkimg-k230 . However, it just apply a huge
diff from vendor SDK to mainline u-boot and can be built from a
standard gcc toolchain.

> I have chanced upon Courmisch's repo that looks significantly more
> usable than whatever Canaan have so I guess I will use that:
> https://code.videolan.org/Courmisch/k230-boot
>

Thanks for this hint.

> :)