Re: [RFC PATCH 1/1] riscv: dts: Allow BUILTIN_DTB for all socs

[Yangyu Chen]

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 <cyy@xxxxxxxxxxxx>
> 
> 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
> > linux-riscv@xxxxxxxxxxxxxxxxxxx
> > http://lists.infradead.org/mailman/listinfo/linux-riscv