On Wed, Feb 21, 2024 at 03:03:57AM +0800, Yangyu Chen wrote:
> The BUILTIN_DTB kernel feature on RISC-V only works on K210 SoC only. This
> patch moved this configuration to entire riscv.
>
> Signed-off-by: Yangyu Chen <[email protected]>
A passing remark here only:
The detail in the cover letter belongs in the commit message. There's
very little value in a cover letter for a one patch "series" - it
either will duplicate information from the commit message (and is
therefore useless) or, as in this case, splits information that should
be in the commit message away from it.
Thanks,
Conor.
> ---
> arch/riscv/Kconfig | 16 ++++++++++++++-
> arch/riscv/Kconfig.socs | 32 -----------------------------
> arch/riscv/boot/dts/Makefile | 2 +-
> arch/riscv/boot/dts/canaan/Makefile | 2 --
> 4 files changed, 16 insertions(+), 36 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index b49016bb5077..23d501561e64 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -996,7 +996,21 @@ config RISCV_ISA_FALLBACK
> config BUILTIN_DTB
> bool "Built-in device tree"
> depends on OF && NONPORTABLE
> - default y if XIP_KERNEL
> + default y if XIP_KERNEL || SOC_CANAAN
> + help
> + Build a device tree into the Linux image.
> + This option should be selected if no bootloader is being used.
> + If unsure, say Y.
> +
> +
> +config BUILTIN_DTB_SOURCE
> + string "Built-in device tree source"
> + depends on BUILTIN_DTB
> + default "canaan/k210_generic" if SOC_CANAAN
> + 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..318239d9423b 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))
> \ No newline at end of file
> 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))
> --
> 2.43.0
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Wed, 2024-02-21 at 01:11 +0000, Conor Dooley wrote:
> On Wed, Feb 21, 2024 at 03:03:57AM +0800, Yangyu Chen wrote:
> > The BUILTIN_DTB kernel feature on RISC-V only works on K210 SoC
> > only. This
> > patch moved this configuration to entire riscv.
> >
> > Signed-off-by: Yangyu Chen <[email protected]>
>
> A passing remark here only:
> The detail in the cover letter belongs in the commit message. There's
> very little value in a cover letter for a one patch "series" - it
> either will duplicate information from the commit message (and is
> therefore useless) or, as in this case, splits information that
> should
> be in the commit message away from it.
>
> Thanks,
> Conor.
>
Thanks. I will move these detailed reasons to commit message. I am
waiting for other review comments before submitting patch v2.
> > ---
> > arch/riscv/Kconfig | 16 ++++++++++++++-
> > arch/riscv/Kconfig.socs | 32 -------------------------
> > ----
> > arch/riscv/boot/dts/Makefile | 2 +-
> > arch/riscv/boot/dts/canaan/Makefile | 2 --
> > 4 files changed, 16 insertions(+), 36 deletions(-)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index b49016bb5077..23d501561e64 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -996,7 +996,21 @@ config RISCV_ISA_FALLBACK
> > config BUILTIN_DTB
> > bool "Built-in device tree"
> > depends on OF && NONPORTABLE
> > - default y if XIP_KERNEL
> > + default y if XIP_KERNEL || SOC_CANAAN
> > + help
> > + Build a device tree into the Linux image.
> > + This option should be selected if no bootloader is being
> > used.
> > + If unsure, say Y.
> > +
> > +
> > +config BUILTIN_DTB_SOURCE
> > + string "Built-in device tree source"
> > + depends on BUILTIN_DTB
> > + default "canaan/k210_generic" if SOC_CANAAN
> > + 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..318239d9423b 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))
> > \ No newline at end of file
I should keep the newline at EOF, I will fix this for patch v2.
> > 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))
> > --
> > 2.43.0
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-riscv