> 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 >> >> 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
