From: Yangyu Chen <cyy@cyyself.name>
To: Conor Dooley <conor@kernel.org>
Cc: linux-riscv@lists.infradead.org,
Paul Walmsley <paul.walmsley@sifive.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Albert Ou <aou@eecs.berkeley.edu>,
linux-kernel@vger.kernel.org,
Masahiro Yamada <masahiroy@kernel.org>,
Alexandre Ghiti <alex@ghiti.fr>, Rob Herring <robh+dt@kernel.org>,
devicetree@vger.kernel.org
Subject: Re: [RFC PATCH 1/1] riscv: dts: Allow BUILTIN_DTB for all socs
Date: Wed, 21 Feb 2024 15:01:10 +0800 [thread overview]
Message-ID: <tencent_5FFF7CF58292DA298E35B86C3196212E8D08@qq.com> (raw)
In-Reply-To: <20240221-numerator-grill-1e5603841a8d@spud>
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@cyyself.name>
>
> 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@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2024-02-21 7:06 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-20 19:01 [RFC PATCH 0/1] riscv: dts: Allow BUILTIN_DTB for all socs Yangyu Chen
2024-02-20 19:03 ` [RFC PATCH 1/1] " Yangyu Chen
2024-02-21 1:11 ` Conor Dooley
2024-02-21 7:01 ` Yangyu Chen [this message]
2024-02-21 18:40 ` Yangyu Chen
2024-02-22 21:05 ` Conor Dooley
2024-02-20 20:46 ` [RFC PATCH 0/1] " Alexandre Ghiti
2024-02-22 20:58 ` Palmer Dabbelt
2024-02-21 11:30 ` Conor Dooley
2024-02-21 14:28 ` Yangyu Chen
2024-02-22 20:59 ` Conor Dooley
2024-02-23 7:42 ` Yangyu Chen
2024-02-23 11:39 ` Conor Dooley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=tencent_5FFF7CF58292DA298E35B86C3196212E8D08@qq.com \
--to=cyy@cyyself.name \
--cc=alex@ghiti.fr \
--cc=aou@eecs.berkeley.edu \
--cc=conor@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=masahiroy@kernel.org \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).