| Message ID | 20210604120639.1447869-1-alex@ghiti.fr |
|---|---|
| State | Accepted |
| Commit | 0ddd7eaffa644baa78e247bbd220ab7195b1eed6 |
| Headers | show |
| Series | [-fixes] riscv: Fix BUILTIN_DTB for sifive and microchip soc | expand |
Hey Arnd, On Fri, Jun 4, 2021 at 3:18 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Fri, Jun 4, 2021 at 2:06 PM Alexandre Ghiti <alex@ghiti.fr> wrote: > > > > Fix BUILTIN_DTB config which resulted in a dtb that was actually not built > > into the Linux image: in the same manner as Canaan soc does, create an object > > file from the dtb file that will get linked into the Linux image. > > > > Signed-off-by: Alexandre Ghiti <alex@ghiti.fr> > > Along the same lines as the comment that Jisheng Zhang made on the fixed > address, building a dtb into the kernel itself fundamentally breaks generic > kernel images. > > I can understand using it on K210, which is extremely limited and wouldn't > run a generic kernel anyway, but for normal platforms like microchip and > sifive, it would be better to disallow CONFIG_BUILTIN_DTB in Kconfig > and require a non-broken boot loader. can't quite agree here. If we take XIP, it does make sense to have BUILTIN_DTB, since 1) this will not be a generic kernel anyway 2) we may want to skip the bootloader altogether or at least make it as thin as possible and 3) copying device tree binaries from bootloader to RAM as opposed to having it handy compiled in the kernel will be just a waste of RAM. Best regards, Vitaly > Arnd > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
Hi Arnd, Le 4/06/2021 à 15:08, Arnd Bergmann a écrit : > On Fri, Jun 4, 2021 at 2:06 PM Alexandre Ghiti <alex@ghiti.fr> wrote: >> >> Fix BUILTIN_DTB config which resulted in a dtb that was actually not built >> into the Linux image: in the same manner as Canaan soc does, create an object >> file from the dtb file that will get linked into the Linux image. >> >> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr> > > Along the same lines as the comment that Jisheng Zhang made on the fixed > address, building a dtb into the kernel itself fundamentally breaks generic > kernel images. > > I can understand using it on K210, which is extremely limited and wouldn't > run a generic kernel anyway, but for normal platforms like microchip and > sifive, it would be better to disallow CONFIG_BUILTIN_DTB in Kconfig > and require a non-broken boot loader. I kind of disagree because if I want to build a custom kernel for those platforms with a builtin dtb for some reasons (debug, development..Etc), I think I should be able to do so. > > Arnd > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv >
On Sat, Jun 5, 2021 at 8:37 AM Alex Ghiti <alex@ghiti.fr> wrote: > Le 4/06/2021 à 15:08, Arnd Bergmann a écrit : > > On Fri, Jun 4, 2021 at 2:06 PM Alexandre Ghiti <alex@ghiti.fr> wrote: > >> > >> Fix BUILTIN_DTB config which resulted in a dtb that was actually not built > >> into the Linux image: in the same manner as Canaan soc does, create an object > >> file from the dtb file that will get linked into the Linux image. > >> > >> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr> > > > > Along the same lines as the comment that Jisheng Zhang made on the fixed > > address, building a dtb into the kernel itself fundamentally breaks generic > > kernel images. > > > > I can understand using it on K210, which is extremely limited and wouldn't > > run a generic kernel anyway, but for normal platforms like microchip and > > sifive, it would be better to disallow CONFIG_BUILTIN_DTB in Kconfig > > and require a non-broken boot loader. > > I kind of disagree because if I want to build a custom kernel for those > platforms with a builtin dtb for some reasons (debug, development..Etc), > I think I should be able to do so. How is the builtin dtb better than appended dtb, or passing the dtb to the boot loader in that case? Arnd
Le 5/06/2021 à 13:00, Arnd Bergmann a écrit : > On Sat, Jun 5, 2021 at 8:37 AM Alex Ghiti <alex@ghiti.fr> wrote: >> Le 4/06/2021 à 15:08, Arnd Bergmann a écrit : >>> On Fri, Jun 4, 2021 at 2:06 PM Alexandre Ghiti <alex@ghiti.fr> wrote: >>>> >>>> Fix BUILTIN_DTB config which resulted in a dtb that was actually not built >>>> into the Linux image: in the same manner as Canaan soc does, create an object >>>> file from the dtb file that will get linked into the Linux image. >>>> >>>> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr> >>> >>> Along the same lines as the comment that Jisheng Zhang made on the fixed >>> address, building a dtb into the kernel itself fundamentally breaks generic >>> kernel images. >>> >>> I can understand using it on K210, which is extremely limited and wouldn't >>> run a generic kernel anyway, but for normal platforms like microchip and >>> sifive, it would be better to disallow CONFIG_BUILTIN_DTB in Kconfig >>> and require a non-broken boot loader. >> >> I kind of disagree because if I want to build a custom kernel for those >> platforms with a builtin dtb for some reasons (debug, development..Etc), >> I think I should be able to do so. > > How is the builtin dtb better than appended dtb, or passing the dtb to the > boot loader in that case? Ah never said it was better, just it was available so there is no reason we could not allow it :) > > Arnd > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv >
On Sun, 06 Jun 2021 00:40:34 PDT (-0700), alex@ghiti.fr wrote: > Le 5/06/2021 à 13:00, Arnd Bergmann a écrit : >> On Sat, Jun 5, 2021 at 8:37 AM Alex Ghiti <alex@ghiti.fr> wrote: >>> Le 4/06/2021 à 15:08, Arnd Bergmann a écrit : >>>> On Fri, Jun 4, 2021 at 2:06 PM Alexandre Ghiti <alex@ghiti.fr> wrote: >>>>> >>>>> Fix BUILTIN_DTB config which resulted in a dtb that was actually not built >>>>> into the Linux image: in the same manner as Canaan soc does, create an object >>>>> file from the dtb file that will get linked into the Linux image. >>>>> >>>>> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr> >>>> >>>> Along the same lines as the comment that Jisheng Zhang made on the fixed >>>> address, building a dtb into the kernel itself fundamentally breaks generic >>>> kernel images. >>>> >>>> I can understand using it on K210, which is extremely limited and wouldn't >>>> run a generic kernel anyway, but for normal platforms like microchip and >>>> sifive, it would be better to disallow CONFIG_BUILTIN_DTB in Kconfig >>>> and require a non-broken boot loader. >>> >>> I kind of disagree because if I want to build a custom kernel for those >>> platforms with a builtin dtb for some reasons (debug, development..Etc), >>> I think I should be able to do so. >> >> How is the builtin dtb better than appended dtb, or passing the dtb to the >> boot loader in that case? > > Ah never said it was better, just it was available so there is no reason > we could not allow it :) I agree: I'm not really a fan of BUILTIN_DTB (and I tried pretty hard not to have it in the first place), but whatever we have shouldn't be broken. This is on fixes.
diff --git a/arch/riscv/boot/dts/microchip/Makefile b/arch/riscv/boot/dts/microchip/Makefile index 622b12771fd3..855c1502d912 100644 --- a/arch/riscv/boot/dts/microchip/Makefile +++ b/arch/riscv/boot/dts/microchip/Makefile @@ -1,2 +1,3 @@ # SPDX-License-Identifier: GPL-2.0 dtb-$(CONFIG_SOC_MICROCHIP_POLARFIRE) += microchip-mpfs-icicle-kit.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 74c47fe9fc22..d90e4eb0ade8 100644 --- a/arch/riscv/boot/dts/sifive/Makefile +++ b/arch/riscv/boot/dts/sifive/Makefile @@ -1,3 +1,4 @@ # SPDX-License-Identifier: GPL-2.0 dtb-$(CONFIG_SOC_SIFIVE) += hifive-unleashed-a00.dtb \ hifive-unmatched-a00.dtb +obj-$(CONFIG_BUILTIN_DTB) += $(addsuffix .o, $(dtb-y))
Fix BUILTIN_DTB config which resulted in a dtb that was actually not built into the Linux image: in the same manner as Canaan soc does, create an object file from the dtb file that will get linked into the Linux image. Signed-off-by: Alexandre Ghiti <alex@ghiti.fr> --- arch/riscv/boot/dts/microchip/Makefile | 1 + arch/riscv/boot/dts/sifive/Makefile | 1 + 2 files changed, 2 insertions(+)