On Fri, 17 Jun 2016 16:44:09 +0200
Jonas Gorski <jogo@xxxxxxxxxxx> wrote:
> Hi Antony,
>
> On 17.06.2016 16:01, Antony Pavlov wrote:
> > On Fri, 17 Jun 2016 14:07:39 +0200
> > Jonas Gorski <jogo@xxxxxxxxxxx> wrote:
> >
> >> Instead of rewriting the arguments, just move the appended dtb to where
> >> the decompressed kernel expects it. This eliminates the need for special
> >> casing vmlinuz.bin appended dtb files.
> >>
> >> Signed-off-by: Jonas Gorski <jogo@xxxxxxxxxxx>
> >> ---
> >> arch/mips/Kconfig | 22 ++--------------------
> >> arch/mips/boot/compressed/Makefile | 1 +
> >> arch/mips/boot/compressed/decompress.c | 21 +++++++++++++++++++++
> >> arch/mips/boot/compressed/head.S | 16 ----------------
> >> 4 files changed, 24 insertions(+), 36 deletions(-)
> >>
> >> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> >> index ac91939..0d0f71e 100644
> >> --- a/arch/mips/Kconfig
> >> +++ b/arch/mips/Kconfig
> >> @@ -2885,10 +2885,10 @@ choice
> >> the documented boot protocol using a device tree.
> >>
> >> config MIPS_RAW_APPENDED_DTB
> >> - bool "vmlinux.bin"
> >> + bool "vmlinux.bin or vmlinuz.bin"
> >> help
> >> With this option, the boot code will look for a device tree binary
> >> - DTB) appended to raw vmlinux.bin (without decompressor).
> >> + DTB) appended to raw vmlinux.bin or vmlinuz.bin.
> >> (e.g. cat vmlinux.bin <filename>.dtb > vmlinux_w_dtb).
> >>
> >> This is meant as a backward compatibility convenience for those
> >> @@ -2900,24 +2900,6 @@ choice
> >> look like a DTB header after a reboot if no actual DTB is appended
> >> to vmlinux.bin. Do not leave this option active in a production kernel
> >> if you don't intend to always append a DTB.
> >> -
> >> - config MIPS_ZBOOT_APPENDED_DTB
> >> - bool "vmlinuz.bin"
> >> - depends on SYS_SUPPORTS_ZBOOT
> >> - help
> >> - With this option, the boot code will look for a device tree binary
> >> - DTB) appended to raw vmlinuz.bin (with decompressor).
> >> - (e.g. cat vmlinuz.bin <filename>.dtb > vmlinuz_w_dtb).
> >> -
> >> - This is meant as a backward compatibility convenience for those
> >> - systems with a bootloader that can't be upgraded to accommodate
> >> - the documented boot protocol using a device tree.
> >> -
> >> - Beware that there is very little in terms of protection against
> >> - this option being confused by leftover garbage in memory that might
> >> - look like a DTB header after a reboot if no actual DTB is appended
> >> - to vmlinuz.bin. Do not leave this option active in a production kernel
> >> - if you don't intend to always append a DTB.
> >> endchoice
> >>
> >> choice
> >> diff --git a/arch/mips/boot/compressed/Makefile b/arch/mips/boot/compressed/Makefile
> >> index 90aca95..f31ec89 100644
> >> --- a/arch/mips/boot/compressed/Makefile
> >> +++ b/arch/mips/boot/compressed/Makefile
> >> @@ -29,6 +29,7 @@ KBUILD_AFLAGS := $(LINUXINCLUDE) $(KBUILD_AFLAGS) -D__ASSEMBLY__ \
> >> -DBOOT_HEAP_SIZE=$(BOOT_HEAP_SIZE) \
> >> -DKERNEL_ENTRY=$(VMLINUX_ENTRY_ADDRESS)
> >>
> >> +
> >
> > Could you please remote this extra empty line?
>
> Oops, that must have slipped through. Fixed locally.
>
> >> # decompressor objects (linked with vmlinuz)
> >> vmlinuzobjs-y := $(obj)/head.o $(obj)/decompress.o $(obj)/string.o
> >>
> >> diff --git a/arch/mips/boot/compressed/decompress.c b/arch/mips/boot/compressed/decompress.c
> >> index 080cd53..e18ab3e 100644
> >> --- a/arch/mips/boot/compressed/decompress.c
> >> +++ b/arch/mips/boot/compressed/decompress.c
> >> @@ -36,6 +36,12 @@ extern void puthex(unsigned long long val);
> >> #define puthex(val) do {} while (0)
> >> #endif
> >>
> >> +#ifdef CONFIG_MIPS_RAW_APPENDED_DTB
> >
> > Do we really need this '#ifdef' here?
>
> No we don't, I was under the assumption the __appended_dtb is
> only available when appended dtb support is enabled, for for
> the decompressor it is always defined. At least gcc doesn't
> seem to complain about it being unused in a quick test
> compile. Fixed locally.
Now the __appended_dtb variable is always available.
You can make the next step and try to transform your
+#ifdef CONFIG_MIPS_RAW_APPENDED_DTB
+ if (fdt_magic((void *)&__appended_dtb) == FDT_MAGIC) {
+ unsigned int image_size, dtb_size;
into something like this
+ if (IS_ENABLED(CONFIG_MIPS_RAW_APPENDED_DTB) &&
+ fdt_magic((void *)&__appended_dtb) == FDT_MAGIC) {
+ unsigned int image_size, dtb_size;
Using IS_ENABLED instead of #if/#ifdef the compiler can check
all the code.
--
Best regards,
Antony Pavlov
