Re: [PATCH v3 5/7] riscv: Kconfig.socs: Allow SOC_CANAAN with MMU for K230
From: Yangyu Chen
Date: Wed Mar 13 2024 - 14:01:13 EST
On 2024/3/8 05:03, Yangyu Chen wrote:
On Mar 6, 2024, at 07:58, Damien Le Moal <dlemoal@xxxxxxxxxx> wrote:
On 3/6/24 02:20, Conor Dooley wrote:
On Tue, Mar 05, 2024 at 03:47:15PM +0800, Yangyu Chen wrote:
On 2024/3/5 07:46, Damien Le Moal wrote:
On 3/5/24 06:05, Yangyu Chen wrote:
Since K230 was released, SOC_CANAAN is no longer only referred to the K210.
Remove it depends on !MMU will allow building dts for K230 and remove the
K210 string from the help message.
Signed-off-by: Yangyu Chen <cyy@xxxxxxxxxxxx>
Reviewed-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
---
arch/riscv/Kconfig.socs | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
index 623de5f8a208..b4e9b7f75510 100644
--- a/arch/riscv/Kconfig.socs
+++ b/arch/riscv/Kconfig.socs
@@ -75,13 +75,12 @@ config ARCH_CANAAN
def_bool SOC_CANAAN
config SOC_CANAAN
- bool "Canaan Kendryte K210 SoC"
- depends on !MMU
This seems wrong to me. The k210 support does require no-mmu. So why remove
this ?
It just allows SOC_CANAAN to be selected when MMU=y. With this patch,
nommu_k210_defconfig still works.
I think the concern here is that this would allow people to build a
kernel for the k120 with the MMU enabled, not that the existing nommu
build will be affected.
Yes, this is my concern. Apologies for the lack of clarity.
Hi,
Thanks for the review comments. After thinking about it for a while,
I think we don't need to change it as we have changed the help
message which deleted the "K210". And the dts on k210.dtsi shows
mmu-type is riscv.none, I think if someone noticed this would know
why it fails to boot on the S-Mode MMU Kernel on K210. The only
special thing for ARCH_CANAAN is that a loader.bin will be built
when M-Mode is on arch/riscv/Makefile. However, Canaan has no other
M-Mode chips except for K210. So I think we don't need to change
it.
Another reason is that SOC_CANAAN for K210 is somehow hard to change.
If we continue using SOC_CANAAN for K210 but not for other Canaan
SoCs such as K230, it will cause some confusion to users. If we
rename SOC_CANAAN to SOC_CANAAN_K210, it will change many drivers
in many subsystems like my patch v5 [1]. So I don't think we need
to fix it.
If we don't change it, A concern for this is that some drivers for
K210 will be built when SOC_CANAAN=y and if we add this to defconfig,
all riscv builds will also build some K210 drivers even on MMU. But
I think this will not be a problem just need some memory/storage
for a slightly bigger kernel. Also, we will enable some new configs
in defconfig when a new soc gets supported, it's normal for K210
SoC drivers.
Thus, I think we don't need to change it. If you have some other
opinions, please let me know.
[1] https://lore.kernel.org/linux-riscv/tencent_6F35FEF31908DE6AEB385AE30AC658863C0A@xxxxxx/
Thanks,
Yangyu Chen
Hi Damien,
I'm waiting for your decision before sending the next patch revision.
Please reply to this when you are free.
Thanks,
Yangyu Chen
Maybe you could squash in something like the following?
diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
index b4e9b7f75510..75d55059163f 100644
--- a/arch/riscv/Kconfig.socs
+++ b/arch/riscv/Kconfig.socs
@@ -72,15 +72,19 @@ config SOC_VIRT
This enables support for QEMU Virt Machine.
config ARCH_CANAAN
- def_bool SOC_CANAAN
+ bool "Canaan Kendryte SoCs"
+ help
+ This enables support for Canaan Kendryte SoC platform hardware.
config SOC_CANAAN
- bool "Canaan Kendryte SoC"
+ bool "Canaan Kendryte K210 SoC"
+ depends on !MMU
+ depends on ARCH_CANAAN
select CLINT_TIMER if RISCV_M_MODE
select ARCH_HAS_RESET_CONTROLLER
select PINCTRL
select COMMON_CLK
help
- This enables support for Canaan Kendryte SoC platform hardware.
+ This enables support for Canaan Kendryte K210 SoC platform hardware.
endmenu # "SoC selection"
(Which reminds me, I really need to go and finish sorting out the ARCH_
stuff)
--
Damien Le Moal
Western Digital Research