[RFC] Load U-Boot without LK on DragonBoard 410c (+ DB820c?)

[Ramon Fried]

On Sun, Jul 4, 2021 at 12:42 AM Stephan Gerhold <stephan at gerhold.net> wrote:
>
> On Thu, Jul 01, 2021 at 11:07:55AM +0200, Stephan Gerhold wrote:
> > Hi!
> >
> > at the moment the U-Boot ports for both DragonBoard 410c and 820c are
> > designed to be loaded as an Android boot image after Qualcomm's LK
> > bootloader. This is simple to set up but LK is redundant in this case,
> > since everything done by LK can be also done directly by U-Boot.
> >
> > Dropping LK entirely would have at least the following advantages:
> >   - Easier installation/board code (no need for Android boot images)
> >   - (Slightly) faster boot
> >   - Boot directly in 64-bit without a round trip to 32-bit for LK
> >
> > [...]
> >
> > 3. Remaining open questions
> > ===========================
> >
> > [...]
> >
> >   3. CONFIG_OF_EMBED: There is a big warning about this in the build log:
> >      "This option should only be used for debugging purposes. Please use
> >       CONFIG_OF_SEPARATE for boards in mainline."
> >
> >      The important part here is that we need an ELF image with both
> >      U-Boot and the DTB. CONFIG_OF_EMBED is convenient for that because
> >      we can just use the ELF image built by the linker and it already
> >      contains the DTB.
> >
> >      If CONFIG_OF_EMBED is really so bad it might be possible to build
> >      a new boot image based on "u-boot-dtb.bin" (which is U-Boot with
> >      DTB appended). I'm not sure if this is really much better though.
> >
>
> After looking some more I found "CONFIG_REMAKE_ELF" which seems to do
> exactly this (build a new ELF image based on "u-boot-dtb.bin" with
> appended DTB). So this avoids setting CONFIG_OF_EMBED and therefore the
> build warning. Sounds like the solution I was looking for. :)
>
> Unfortunately it looks like appending the DTB to U-Boot currently
> produces very strange behavior on DragonBoard 410c. It's either:
>
>   - Working fine, or
>   - Rebooting continously without serial output from U-Boot, or
>   - The following serial output:
>
>       Qualcomm-DragonBoard 410C
>       DRAM:  986 MiB
>       No serial driver found
>       resetting ...
>
> It behaves consistently given a U-Boot binary but varies when
> adding/removing random features from the U-Boot binary.
>
> After a couple of hours of debugging, I realized that the appended DTB
> becomes corrupted. Specifically, there is a "GPIO_5" written into it, e.g.
>
> 8f6905b8: edfe0dd0 9f100000 4f495047 c000355f    ........GPIO_5..
> 8f6905c8: 28000000 11000000 10000000 00000000    ...(............
> 8f6905d8: df010000 880e0000 00000000 00000000    ................
> 8f6905e8: 00000000 00000000 01000000 00000000    ................
> 8f6905f8: 03000000 04000000 00000000 02000000    ................
> 8f690608: 03000000 04000000 0f000000 02000000    ................
> 8f690618: 03000000 2d000000 1b000000 6c617551    .......-....Qual
> 8f690628: 6d6d6f63 63655420 6c6f6e68 6569676f    comm Technologie
>
> Depending on enabled features in U-Boot the "GPIO_5" corrupts different
> parts of the DTB, that's why it works somewhat sometimes.
>
> After staring at some drivers and the U-Boot linker script for a while
> I realized that the BSS section overlaps with the appended DTB before
> relocation. And indeed, mach-snapdragon/pinctrl-apq8016.c writes "GPIO_5"
> into "static char pin_name[MAX_PIN_NAME_LEN];" (= BSS) before relocation.
>
> Actually, arch/arm/lib/crt0_64.S says that BSS should not be used at all
> before relocation, because it's uninitialized and might corrupt other
> memory. I found several other commits where similar problems happened
> and it was usually fixed by moving the variables into the data section.
>
> So, I fixed the problem with the diff below and will include it together
> with the changes to drop all the LK-related code. Now everything seems
> fine. (I just wish this would have somehow been more obvious instead of
> the strange behavior...)
>
> Stephan
>
> diff --git a/arch/arm/mach-snapdragon/pinctrl-apq8016.c b/arch/arm/mach-snapdragon/pinctrl-apq8016.c
> index 1042b564c3..7940c74287 100644
> --- a/arch/arm/mach-snapdragon/pinctrl-apq8016.c
> +++ b/arch/arm/mach-snapdragon/pinctrl-apq8016.c
> @@ -10,7 +10,7 @@
>  #include <common.h>
>
>  #define MAX_PIN_NAME_LEN 32
> -static char pin_name[MAX_PIN_NAME_LEN];
> +static char pin_name[MAX_PIN_NAME_LEN] __section(".data");
>  static const char * const msm_pinctrl_pins[] = {
>         "SDC1_CLK",
>         "SDC1_CMD",
>
Hi.
If I recall correctly, the signing tool only used the ELF sections, so
the appended DTB was deleted.
That's why I kept the "embedded DTB".
In your signing tool, you probably sign the complete file without
parsing the ELF.
Thanks,
Ramon.