2024-02-28 09:00:22

by Yangyu Chen

[permalink] [raw]
Subject: [PATCH v3] riscv: dts: Move BUILTIN_DTB_SOURCE to common Kconfig

The BUILTIN_DTB_SOURCE was only configured for K210 before. Since
SOC_BUILTIN_DTB_DECLARE was removed at commit d5805af9fe9f ("riscv: Fix
builtin DTB handling") from patch [1], the kernel cannot choose one of the
dtbs from then on and always take the first one dtb to use. Then, another
commit 0ddd7eaffa64 ("riscv: Fix BUILTIN_DTB for sifive and microchip soc")
from patch [2] supports BUILTIN_DTB_SOURCE for other SoCs. However, this
feature will only work if the Kconfig we use links the dtb we expected in
the first place as mentioned in the thread [3]. Thus, a config
BUILTIN_DTB_SOURCE is needed for all SoCs to choose one dtb to use.

For some considerations, this patch also removes default y if XIP_KERNEL
for BUILTIN_DTB, as this requires setting a proper dtb to use on the
BUILTIN_DTB_SOURCE, else the kernel with XIP but does not set
BUILTIN_DTB_SOURCE or unselect BUILTIN_DTB will not boot.

Also, this patch removes the default dtb string for k210 from Kconfig to
nommu_k210_defconfig and nommu_k210_sdcard_defconfig to avoid complex
Kconfig settings for other SoCs in the future.

[1] https://lore.kernel.org/linux-riscv/[email protected]/
[2] https://lore.kernel.org/linux-riscv/[email protected]/
[3] https://lore.kernel.org/linux-riscv/CAK7LNATt_56mO2Le4v4EnPnAfd3gC8S_Sm5-GCsfa=qXy=8Lrg@mail.gmail.com/

Signed-off-by: Yangyu Chen <[email protected]>
Reviewed-by: Conor Dooley <[email protected]>
---
Changes since v2:
- some fixes on format and grammar
- v2: https://lore.kernel.org/linux-riscv/[email protected]/

Changes since v1:
- remove default y for BULTIN_DTB in any cases
- remove default DTB_SOURCE for k210 and moved to its defconfig file
- remove building dtb object file for other SoCs
- reword help message to say N if unsure for BUILTIN_DTB_SOURCE
- reword commit message
- v1: https://lore.kernel.org/linux-riscv/[email protected]/

---
arch/riscv/Kconfig | 14 +++++++-
arch/riscv/Kconfig.socs | 32 -------------------
arch/riscv/boot/dts/Makefile | 2 +-
arch/riscv/boot/dts/canaan/Makefile | 2 --
arch/riscv/boot/dts/microchip/Makefile | 1 -
arch/riscv/boot/dts/sifive/Makefile | 1 -
arch/riscv/configs/nommu_k210_defconfig | 2 ++
.../riscv/configs/nommu_k210_sdcard_defconfig | 2 ++
8 files changed, 18 insertions(+), 38 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 85c899d0133a..3d6d93d71257 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -1000,7 +1000,19 @@ config RISCV_ISA_FALLBACK
config BUILTIN_DTB
bool "Built-in device tree"
depends on OF && NONPORTABLE
- default y if XIP_KERNEL
+ help
+ Build a device tree into the Linux image.
+ This option should be selected if no bootloader is being used.
+ If unsure, say N.
+
+
+config BUILTIN_DTB_SOURCE
+ string "Built-in device tree source"
+ depends on BUILTIN_DTB
+ help
+ DTS file path (without suffix, relative to arch/riscv/boot/dts)
+ for the DTS file that will be used to produce the DTB linked into the
+ kernel.

endmenu # "Boot options"

diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
index e08e91c49abe..623de5f8a208 100644
--- a/arch/riscv/Kconfig.socs
+++ b/arch/riscv/Kconfig.socs
@@ -84,36 +84,4 @@ config SOC_CANAAN
help
This enables support for Canaan Kendryte K210 SoC platform hardware.

-if ARCH_CANAAN
-
-config ARCH_CANAAN_K210_DTB_BUILTIN
- def_bool SOC_CANAAN_K210_DTB_BUILTIN
-
-config SOC_CANAAN_K210_DTB_BUILTIN
- bool "Builtin device tree for the Canaan Kendryte K210"
- depends on ARCH_CANAAN
- default y
- select OF
- select BUILTIN_DTB
- help
- Build a device tree for the Kendryte K210 into the Linux image.
- This option should be selected if no bootloader is being used.
- If unsure, say Y.
-
-config ARCH_CANAAN_K210_DTB_SOURCE
- string
- default SOC_CANAAN_K210_DTB_SOURCE
-
-config SOC_CANAAN_K210_DTB_SOURCE
- string "Source file for the Canaan Kendryte K210 builtin DTB"
- depends on ARCH_CANAAN
- depends on ARCH_CANAAN_K210_DTB_BUILTIN
- default "k210_generic"
- help
- Base name (without suffix, relative to arch/riscv/boot/dts/canaan)
- for the DTS file that will be used to produce the DTB linked into the
- kernel.
-
-endif # ARCH_CANAAN
-
endmenu # "SoC selection"
diff --git a/arch/riscv/boot/dts/Makefile b/arch/riscv/boot/dts/Makefile
index 72030fd727af..fdae05bbf556 100644
--- a/arch/riscv/boot/dts/Makefile
+++ b/arch/riscv/boot/dts/Makefile
@@ -8,4 +8,4 @@ subdir-y += sophgo
subdir-y += starfive
subdir-y += thead

-obj-$(CONFIG_BUILTIN_DTB) := $(addsuffix /, $(subdir-y))
+obj-$(CONFIG_BUILTIN_DTB) := $(addsuffix .dtb.o, $(CONFIG_BUILTIN_DTB_SOURCE))
diff --git a/arch/riscv/boot/dts/canaan/Makefile b/arch/riscv/boot/dts/canaan/Makefile
index 520623264c87..987d1f0c41f0 100644
--- a/arch/riscv/boot/dts/canaan/Makefile
+++ b/arch/riscv/boot/dts/canaan/Makefile
@@ -5,5 +5,3 @@ dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_bit.dtb
dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_dock.dtb
dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_go.dtb
dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maixduino.dtb
-
-obj-$(CONFIG_ARCH_CANAAN_K210_DTB_BUILTIN) += $(addsuffix .dtb.o, $(CONFIG_ARCH_CANAAN_K210_DTB_SOURCE))
diff --git a/arch/riscv/boot/dts/microchip/Makefile b/arch/riscv/boot/dts/microchip/Makefile
index 45adc4926e79..e177815bf1a2 100644
--- a/arch/riscv/boot/dts/microchip/Makefile
+++ b/arch/riscv/boot/dts/microchip/Makefile
@@ -4,4 +4,3 @@ dtb-$(CONFIG_ARCH_MICROCHIP_POLARFIRE) += mpfs-m100pfsevp.dtb
dtb-$(CONFIG_ARCH_MICROCHIP_POLARFIRE) += mpfs-polarberry.dtb
dtb-$(CONFIG_ARCH_MICROCHIP_POLARFIRE) += mpfs-sev-kit.dtb
dtb-$(CONFIG_ARCH_MICROCHIP_POLARFIRE) += mpfs-tysom-m.dtb
-obj-$(CONFIG_BUILTIN_DTB) += $(addsuffix .o, $(dtb-y))
diff --git a/arch/riscv/boot/dts/sifive/Makefile b/arch/riscv/boot/dts/sifive/Makefile
index 6a5fbd4ed96a..495bf760a909 100644
--- a/arch/riscv/boot/dts/sifive/Makefile
+++ b/arch/riscv/boot/dts/sifive/Makefile
@@ -1,4 +1,3 @@
# SPDX-License-Identifier: GPL-2.0
dtb-$(CONFIG_ARCH_SIFIVE) += hifive-unleashed-a00.dtb \
hifive-unmatched-a00.dtb
-obj-$(CONFIG_BUILTIN_DTB) += $(addsuffix .o, $(dtb-y))
diff --git a/arch/riscv/configs/nommu_k210_defconfig b/arch/riscv/configs/nommu_k210_defconfig
index 146c46d0525b..7e75200543f4 100644
--- a/arch/riscv/configs/nommu_k210_defconfig
+++ b/arch/riscv/configs/nommu_k210_defconfig
@@ -33,6 +33,8 @@ CONFIG_SMP=y
CONFIG_NR_CPUS=2
CONFIG_CMDLINE="earlycon console=ttySIF0"
CONFIG_CMDLINE_FORCE=y
+CONFIG_BUILTIN_DTB=y
+CONFIG_BUILTIN_DTB_SOURCE="canaan/k210_generic"
# CONFIG_SECCOMP is not set
# CONFIG_STACKPROTECTOR is not set
# CONFIG_GCC_PLUGINS is not set
diff --git a/arch/riscv/configs/nommu_k210_sdcard_defconfig b/arch/riscv/configs/nommu_k210_sdcard_defconfig
index 95d8d1808f19..0ba353e9ca71 100644
--- a/arch/riscv/configs/nommu_k210_sdcard_defconfig
+++ b/arch/riscv/configs/nommu_k210_sdcard_defconfig
@@ -25,6 +25,8 @@ CONFIG_SMP=y
CONFIG_NR_CPUS=2
CONFIG_CMDLINE="earlycon console=ttySIF0 root=/dev/mmcblk0p1 rootwait ro"
CONFIG_CMDLINE_FORCE=y
+CONFIG_BUILTIN_DTB=y
+CONFIG_BUILTIN_DTB_SOURCE="canaan/k210_generic"
# CONFIG_SECCOMP is not set
# CONFIG_STACKPROTECTOR is not set
# CONFIG_GCC_PLUGINS is not set
--
2.43.0



2024-03-05 21:16:06

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH v3] riscv: dts: Move BUILTIN_DTB_SOURCE to common Kconfig

On Wed, 28 Feb 2024 00:52:54 PST (-0800), [email protected] wrote:
> The BUILTIN_DTB_SOURCE was only configured for K210 before. Since
> SOC_BUILTIN_DTB_DECLARE was removed at commit d5805af9fe9f ("riscv: Fix
> builtin DTB handling") from patch [1], the kernel cannot choose one of the
> dtbs from then on and always take the first one dtb to use. Then, another
> commit 0ddd7eaffa64 ("riscv: Fix BUILTIN_DTB for sifive and microchip soc")
> from patch [2] supports BUILTIN_DTB_SOURCE for other SoCs. However, this
> feature will only work if the Kconfig we use links the dtb we expected in
> the first place as mentioned in the thread [3]. Thus, a config
> BUILTIN_DTB_SOURCE is needed for all SoCs to choose one dtb to use.
>
> For some considerations, this patch also removes default y if XIP_KERNEL
> for BUILTIN_DTB, as this requires setting a proper dtb to use on the
> BUILTIN_DTB_SOURCE, else the kernel with XIP but does not set
> BUILTIN_DTB_SOURCE or unselect BUILTIN_DTB will not boot.
>
> Also, this patch removes the default dtb string for k210 from Kconfig to
> nommu_k210_defconfig and nommu_k210_sdcard_defconfig to avoid complex
> Kconfig settings for other SoCs in the future.
>
> [1] https://lore.kernel.org/linux-riscv/[email protected]/
> [2] https://lore.kernel.org/linux-riscv/[email protected]/
> [3] https://lore.kernel.org/linux-riscv/CAK7LNATt_56mO2Le4v4EnPnAfd3gC8S_Sm5-GCsfa=qXy=8Lrg@mail.gmail.com/
>
> Signed-off-by: Yangyu Chen <[email protected]>
> Reviewed-by: Conor Dooley <[email protected]>

Acked-by: Palmer Dabbelt <[email protected]>

> ---
> Changes since v2:
> - some fixes on format and grammar
> - v2: https://lore.kernel.org/linux-riscv/[email protected]/
>
> Changes since v1:
> - remove default y for BULTIN_DTB in any cases
> - remove default DTB_SOURCE for k210 and moved to its defconfig file
> - remove building dtb object file for other SoCs
> - reword help message to say N if unsure for BUILTIN_DTB_SOURCE
> - reword commit message
> - v1: https://lore.kernel.org/linux-riscv/[email protected]/
>
> ---
> arch/riscv/Kconfig | 14 +++++++-
> arch/riscv/Kconfig.socs | 32 -------------------
> arch/riscv/boot/dts/Makefile | 2 +-
> arch/riscv/boot/dts/canaan/Makefile | 2 --
> arch/riscv/boot/dts/microchip/Makefile | 1 -
> arch/riscv/boot/dts/sifive/Makefile | 1 -
> arch/riscv/configs/nommu_k210_defconfig | 2 ++
> .../riscv/configs/nommu_k210_sdcard_defconfig | 2 ++
> 8 files changed, 18 insertions(+), 38 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 85c899d0133a..3d6d93d71257 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -1000,7 +1000,19 @@ config RISCV_ISA_FALLBACK
> config BUILTIN_DTB
> bool "Built-in device tree"
> depends on OF && NONPORTABLE
> - default y if XIP_KERNEL
> + help
> + Build a device tree into the Linux image.
> + This option should be selected if no bootloader is being used.
> + If unsure, say N.
> +
> +
> +config BUILTIN_DTB_SOURCE
> + string "Built-in device tree source"
> + depends on BUILTIN_DTB
> + help
> + DTS file path (without suffix, relative to arch/riscv/boot/dts)
> + for the DTS file that will be used to produce the DTB linked into the
> + kernel.
>
> endmenu # "Boot options"
>
> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> index e08e91c49abe..623de5f8a208 100644
> --- a/arch/riscv/Kconfig.socs
> +++ b/arch/riscv/Kconfig.socs
> @@ -84,36 +84,4 @@ config SOC_CANAAN
> help
> This enables support for Canaan Kendryte K210 SoC platform hardware.
>
> -if ARCH_CANAAN
> -
> -config ARCH_CANAAN_K210_DTB_BUILTIN
> - def_bool SOC_CANAAN_K210_DTB_BUILTIN
> -
> -config SOC_CANAAN_K210_DTB_BUILTIN
> - bool "Builtin device tree for the Canaan Kendryte K210"
> - depends on ARCH_CANAAN
> - default y
> - select OF
> - select BUILTIN_DTB
> - help
> - Build a device tree for the Kendryte K210 into the Linux image.
> - This option should be selected if no bootloader is being used.
> - If unsure, say Y.
> -
> -config ARCH_CANAAN_K210_DTB_SOURCE
> - string
> - default SOC_CANAAN_K210_DTB_SOURCE
> -
> -config SOC_CANAAN_K210_DTB_SOURCE
> - string "Source file for the Canaan Kendryte K210 builtin DTB"
> - depends on ARCH_CANAAN
> - depends on ARCH_CANAAN_K210_DTB_BUILTIN
> - default "k210_generic"
> - help
> - Base name (without suffix, relative to arch/riscv/boot/dts/canaan)
> - for the DTS file that will be used to produce the DTB linked into the
> - kernel.
> -
> -endif # ARCH_CANAAN
> -
> endmenu # "SoC selection"
> diff --git a/arch/riscv/boot/dts/Makefile b/arch/riscv/boot/dts/Makefile
> index 72030fd727af..fdae05bbf556 100644
> --- a/arch/riscv/boot/dts/Makefile
> +++ b/arch/riscv/boot/dts/Makefile
> @@ -8,4 +8,4 @@ subdir-y += sophgo
> subdir-y += starfive
> subdir-y += thead
>
> -obj-$(CONFIG_BUILTIN_DTB) := $(addsuffix /, $(subdir-y))
> +obj-$(CONFIG_BUILTIN_DTB) := $(addsuffix .dtb.o, $(CONFIG_BUILTIN_DTB_SOURCE))
> diff --git a/arch/riscv/boot/dts/canaan/Makefile b/arch/riscv/boot/dts/canaan/Makefile
> index 520623264c87..987d1f0c41f0 100644
> --- a/arch/riscv/boot/dts/canaan/Makefile
> +++ b/arch/riscv/boot/dts/canaan/Makefile
> @@ -5,5 +5,3 @@ dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_bit.dtb
> dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_dock.dtb
> dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_go.dtb
> dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maixduino.dtb
> -
> -obj-$(CONFIG_ARCH_CANAAN_K210_DTB_BUILTIN) += $(addsuffix .dtb.o, $(CONFIG_ARCH_CANAAN_K210_DTB_SOURCE))
> diff --git a/arch/riscv/boot/dts/microchip/Makefile b/arch/riscv/boot/dts/microchip/Makefile
> index 45adc4926e79..e177815bf1a2 100644
> --- a/arch/riscv/boot/dts/microchip/Makefile
> +++ b/arch/riscv/boot/dts/microchip/Makefile
> @@ -4,4 +4,3 @@ dtb-$(CONFIG_ARCH_MICROCHIP_POLARFIRE) += mpfs-m100pfsevp.dtb
> dtb-$(CONFIG_ARCH_MICROCHIP_POLARFIRE) += mpfs-polarberry.dtb
> dtb-$(CONFIG_ARCH_MICROCHIP_POLARFIRE) += mpfs-sev-kit.dtb
> dtb-$(CONFIG_ARCH_MICROCHIP_POLARFIRE) += mpfs-tysom-m.dtb
> -obj-$(CONFIG_BUILTIN_DTB) += $(addsuffix .o, $(dtb-y))
> diff --git a/arch/riscv/boot/dts/sifive/Makefile b/arch/riscv/boot/dts/sifive/Makefile
> index 6a5fbd4ed96a..495bf760a909 100644
> --- a/arch/riscv/boot/dts/sifive/Makefile
> +++ b/arch/riscv/boot/dts/sifive/Makefile
> @@ -1,4 +1,3 @@
> # SPDX-License-Identifier: GPL-2.0
> dtb-$(CONFIG_ARCH_SIFIVE) += hifive-unleashed-a00.dtb \
> hifive-unmatched-a00.dtb
> -obj-$(CONFIG_BUILTIN_DTB) += $(addsuffix .o, $(dtb-y))
> diff --git a/arch/riscv/configs/nommu_k210_defconfig b/arch/riscv/configs/nommu_k210_defconfig
> index 146c46d0525b..7e75200543f4 100644
> --- a/arch/riscv/configs/nommu_k210_defconfig
> +++ b/arch/riscv/configs/nommu_k210_defconfig
> @@ -33,6 +33,8 @@ CONFIG_SMP=y
> CONFIG_NR_CPUS=2
> CONFIG_CMDLINE="earlycon console=ttySIF0"
> CONFIG_CMDLINE_FORCE=y
> +CONFIG_BUILTIN_DTB=y
> +CONFIG_BUILTIN_DTB_SOURCE="canaan/k210_generic"
> # CONFIG_SECCOMP is not set
> # CONFIG_STACKPROTECTOR is not set
> # CONFIG_GCC_PLUGINS is not set
> diff --git a/arch/riscv/configs/nommu_k210_sdcard_defconfig b/arch/riscv/configs/nommu_k210_sdcard_defconfig
> index 95d8d1808f19..0ba353e9ca71 100644
> --- a/arch/riscv/configs/nommu_k210_sdcard_defconfig
> +++ b/arch/riscv/configs/nommu_k210_sdcard_defconfig
> @@ -25,6 +25,8 @@ CONFIG_SMP=y
> CONFIG_NR_CPUS=2
> CONFIG_CMDLINE="earlycon console=ttySIF0 root=/dev/mmcblk0p1 rootwait ro"
> CONFIG_CMDLINE_FORCE=y
> +CONFIG_BUILTIN_DTB=y
> +CONFIG_BUILTIN_DTB_SOURCE="canaan/k210_generic"
> # CONFIG_SECCOMP is not set
> # CONFIG_STACKPROTECTOR is not set
> # CONFIG_GCC_PLUGINS is not set

2024-03-06 00:13:46

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3] riscv: dts: Move BUILTIN_DTB_SOURCE to common Kconfig

From: Conor Dooley <[email protected]>

On Wed, 28 Feb 2024 16:52:54 +0800, Yangyu Chen wrote:
> The BUILTIN_DTB_SOURCE was only configured for K210 before. Since
> SOC_BUILTIN_DTB_DECLARE was removed at commit d5805af9fe9f ("riscv: Fix
> builtin DTB handling") from patch [1], the kernel cannot choose one of the
> dtbs from then on and always take the first one dtb to use. Then, another
> commit 0ddd7eaffa64 ("riscv: Fix BUILTIN_DTB for sifive and microchip soc")
> from patch [2] supports BUILTIN_DTB_SOURCE for other SoCs. However, this
> feature will only work if the Kconfig we use links the dtb we expected in
> the first place as mentioned in the thread [3]. Thus, a config
> BUILTIN_DTB_SOURCE is needed for all SoCs to choose one dtb to use.
>
> [...]

Applied to riscv-dt-fixes, thanks!

[1/1] riscv: dts: Move BUILTIN_DTB_SOURCE to common Kconfig
https://git.kernel.org/conor/c/2672031b20f6

Thanks,
Conor.