[U-Boot,1/2] Pass through start.elf bootargs on Raspberry Pi

On Raspberry Pi, the primary bootloader start.elf uses the options in
config.txt, as well as options hidden in the firmware itself, to tell
the Linux kernel e.g. framebuffer sizes, memory regions, MAC addresses
and more.

Normally, u-boot would not be able to pass through these options to
the Linux kernel. This patch adds pass-through support via an
additional env variable "bootargs_orig" which is loaded with the
original cmdline obtained from ATAG or the FDT.

In addition, this allows the user to configure DT overlays the "Pi
way" by enabling them in config.txt, as this patch passes through the
full FDT if it has been passed one by u-boot.

Note that, as stated in the comment block, for FDT passthrough to
work, the u-boot.img must be run through a perl script by the
Raspberry Pi Foundation, which appends a trailer to the image telling
start.elf to supply a FDT instead of an ATAG struct.
---
 board/raspberrypi/rpi/rpi.c | 66 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

+               }
+       }
+
        return 0;
 }

--
2.6.2

On 11/29/2015 11:40 AM, Marco Schuster wrote:
> On Raspberry Pi, the primary bootloader start.elf uses the options in
> config.txt, as well as options hidden in the firmware itself, to tell
> the Linux kernel e.g. framebuffer sizes, memory regions, MAC addresses
> and more.

Was this patch sent using "git send-email"? I assume not, since it is
corrupted due to wrapping.

I suspect ./scripts/checkpatch.pl also wasn't run, since it would
complain about over-long lines, and multiple other formatting errors.

> Normally, u-boot would not be able to pass through these options to
> the Linux kernel. This patch adds pass-through support via an
> additional env variable "bootargs_orig" which is loaded with the
> original cmdline obtained from ATAG or the FDT.
> 
> In addition, this allows the user to configure DT overlays the "Pi
> way" by enabling them in config.txt, as this patch passes through the
> full FDT if it has been passed one by u-boot.
> 
> Note that, as stated in the comment block, for FDT passthrough to
> work, the u-boot.img must be run through a perl script by the
> Raspberry Pi Foundation, which appends a trailer to the image telling
> start.elf to supply a FDT instead of an ATAG struct.

This feature sounds fine. However, it should be optional. It likely only
makes sense when booting a downstream Pi Foundation kernel, and not when
booting an upstream/mainline kernel with matching DT using the
standardized DT bindings. As such, I think this feature should be
optional (e.g. perhaps en-/dis-abled by an environment variable at
run-time) so that the user gets to choose whether to use the feature.

> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
> index 6451d1d..d8d0fbb 100644
> --- a/board/raspberrypi/rpi/rpi.c
> +++ b/board/raspberrypi/rpi/rpi.c
> @@ -259,6 +259,72 @@ int misc_init_r(void)
>  #ifdef CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG
>         set_board_info();
>  #endif
> +
> +       /*
> +        * The RPi start.elf passes important system configuration
> +        * like memory sizes and LED GPIO assignments to the kernel
> +        * via kernel command line.
> +        *
> +        * Therefore we have to parse the passed ATAG or FDT to get the

s/have to/may/

> +        * commandline and pass it through to script-land via the env
> +        * variable "bootargs_orig" so it can be passed on.
> +        *
> +        * By default, start.elf assumes a non-DT capable kernel and
> +        * passes the commandline via ATAG; this requires u-boot to
> +        * load the FDT from /boot and pass it on. This works if no
> +        * overlays have been passed through, but once overlays are in
> +        * the mix, stuff gets complicated to do in u-boot.

Ideally, DT overlays would be either implemented in U-Boot, or deferred
until the kernel boots and user-space can load overlays. I suspect the
latter is more inline with the usage model expected by
upstream/mainline, although I haven't checked to see.

> +        *
> +        * To force start.elf to pass a processed, ready-to-go DT,
> +        * you have to use the mkknlimg tool on the u-boot image:
> +        * ./mkknlimg --dtok u-boot.bin /boot/u-boot.bin
> +        *
> +        * mkknlimg can be obtained from
> https://github.com/raspberrypi/tools/blob/master/mkimage/knlinfo
> +        *
> +        * User scripts can check for successful bootargs retrieval
> +        * in the env variable pi_bootmode, which is either fdt, atag
> +        * or unknown in case of an error.
> +        *
> +        * The location 0x100 is hard-coded in start.elf, and it is
> +        * the same as in the default bootscripts, so you only have
> +        * to omit the fatload command loading the raw FDT to get
> +        * going.

"omit the fatload command" from where?

Personally I've been using extlinux.conf on my Pi for a while, so there
is no "fatload" anywhere that I'm aware of.

> +        */
> +
> +       void* ptr = (char*) 0x100;
> +       struct tag_header* atag_ptr = ptr;
> +       setenv("pi_bootmode","unknown");
> +
> +       if(atag_ptr->tag != ATAG_CORE) {
> +               if(atag_ptr->size == be32_to_cpu(FDT_MAGIC)) {
> +                       set_working_fdt_addr((ulong) ptr);

Doesn't that set the address of the DT that U-Boot uses to configure
itself, not the DT that U-Boot will pass to the kernel? The two are
certainly separate concepts. This code should set $fdt_addr/$fdt_addr_r
if the DT is detected, so that scripts can affect whether this DT is used.

> +                       int nodeoffset = fdt_path_offset(working_fdt,
> "/chosen");
> +                       int len;
> +                       const void* nodep = fdt_getprop(working_fdt,
> nodeoffset, "bootargs", &len);
> +                       if(len==0) {
> +                               printf("WARNING: Could not determine
> bootargs from FDT!\n");
> +                       } else {
> +                               printf("Set bootargs_orig from FDT\n");

debug() seems more appropriate than printf(), except for errors.

> +                               setenv("bootargs_orig", (char*) nodep);
> +                               setenv("pi_bootmode","fdt");
> +                       }
> +               } else {
> +                       printf("Warning: start.elf did not pass ATAG
> or FDT parameters\n");
> +               }
> +       } else {
> +               while(atag_ptr->tag != ATAG_NONE) {
> +                       printf("at %p, have %x (len
> %x)\n",atag_ptr,atag_ptr->tag, atag_ptr->size);
> +                       if(atag_ptr->tag == ATAG_CMDLINE) {
> +                               char* old_cmdline = ptr + 8;
> +                               printf("Set bootargs_orig from ATAG\n");
> +                               setenv("bootargs_orig", old_cmdline);
> +                               setenv("pi_bootmode", "atag");
> +                       }
> +                       ptr = ptr + (atag_ptr->size * 4);
> +                       atag_ptr=ptr;
> +               }
> +       }
> +
>         return 0;
>  }

On Sun, Nov 29, 2015 at 07:40:12PM +0100, Marco Schuster wrote:

> On Raspberry Pi, the primary bootloader start.elf uses the options in
> config.txt, as well as options hidden in the firmware itself, to tell
> the Linux kernel e.g. framebuffer sizes, memory regions, MAC addresses
> and more.

While Stephen has made some important and useful comments, I want to
poke at one other thing:

>> +        * To force start.elf to pass a processed, ready-to-go DT,
> +        * you have to use the mkknlimg tool on the u-boot image:
> +        * ./mkknlimg --dtok u-boot.bin /boot/u-boot.bin
> +        *
> +        * mkknlimg can be obtained from
> https://github.com/raspberrypi/tools/blob/master/mkimage/knlinfo

I think there is value in being able to support the Pi-official flow.
What I'd like to see (along with making sure that we don't break support
of using mainline kernels) is that knlinfo be updated with a more
complete license statement (it doesn't say what version of the GPL it's
licensed under) and then dropped into U-Boot so that we can generate the
correct binary for this case out of the box.  That's a much cleaner
experience for the users.