[PATCH v2 5/5] rpi: Use the U-Boot control FDT for fdt_addr

[Simon Glass]

HI Tom,

On Mon, 9 Dec 2024 at 12:02, Tom Rini <trini at konsulko.com> wrote:
>
> On Mon, Dec 09, 2024 at 11:20:01AM -0700, Simon Glass wrote:
>
> > The fdt_addr variable is used in extlinux as a fallback devicetree if
> > none is provided by the boot command.
> >
> > The existing mechanism uses the devicetree provided to U-Boot, but in
> > its original, unrelocated position. For the rpi_4 I am using, this is
> > at 2b35ef00 which is not a convenient place in memory, if the ramdisk
> > is large.
> >
> > U-Boot already deals with this sort of problem by relocating the FDT
> > to a safe address.
> >
> > So use the control-FDT address instead.
> >
> > Remove the existing comment, which is confusing, since the FDT is not
> > actually passed unmodified to the kernel: U-Boot adds various things
> > using its FDT-fixup mechanism.
> >
> > Note that board_get_usable_ram_top() reduces the RAM top for boards with
> > less RAM. This behaviour is left unchanged as there is no other
> > mechanism for U-Boot to handle this.
> >
> > In version 2, it incorporates some changes to fdt_addr, etc. suggested
> > by Tom, as well as adding myself as a maintainer.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > Changes in v2:
> > - Drop patch to allow expanding the devicetree during relocation
> >
> >  board/raspberrypi/rpi/rpi.c | 20 ++++++++------------
> >  1 file changed, 8 insertions(+), 12 deletions(-)
> >
> > diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
> > index 9122f33d88d..8f6ab1b1b9b 100644
> > --- a/board/raspberrypi/rpi/rpi.c
> > +++ b/board/raspberrypi/rpi/rpi.c
> > @@ -3,6 +3,8 @@
> >   * (C) Copyright 2012-2016 Stephen Warren
> >   */
> >
> > +#define LOG_CATEGORY LOGC_BOARD
> > +
> >  #include <config.h>
> >  #include <dm.h>
> >  #include <env.h>
> > @@ -325,19 +327,10 @@ static void set_fdtfile(void)
> >       env_set("fdtfile", fdtfile);
> >  }
> >
> > -/*
> > - * If the firmware provided a valid FDT at boot time, let's expose it in
> > - * ${fdt_addr} so it may be passed unmodified to the kernel.
> > - */
> > +/* Allow U-Boot to use its control FDT with extlinux if one is not provided */
> >  static void set_fdt_addr(void)
> >  {
> > -     if (env_get("fdt_addr"))
> > -             return;
> > -
> > -     if (fdt_magic(fw_dtb_pointer) != FDT_MAGIC)
> > -             return;
> > -
> > -     env_set_hex("fdt_addr", fw_dtb_pointer);
> > +     env_set_hex("fdt_addr", (ulong)gd->fdt_blob);
> >  }
> >
> >  /*
> > @@ -572,7 +565,10 @@ int ft_board_setup(void *blob, struct bd_info *bd)
> >  {
> >       int node;
> >
> > -     update_fdt_from_fw(blob, (void *)fw_dtb_pointer);
> > +     if (blob == gd->fdt_blob)
> > +             log_debug("Same FDT: nothing to do\n");
> > +     else
> > +             update_fdt_from_fw(blob, (void *)gd->fdt_blob);
> >
> >       node = fdt_node_offset_by_compatible(blob, -1, "simple-framebuffer");
> >       if (node < 0)
>
> Have you tested this to make sure that assorted overlays specified in
> config.txt are still passed on correctly to the kernel? Or do we end up
> being OK here due to board_fdt_blob_setup() meaning that we set
> gd->fdt_blob to fw_dtb_pointer much earlier on, and that hook simply
> didn't exist back in 2016 when commit
> ade243a211d6 ("rpi: passthrough of the firmware provided FDT blob")
> introduced some of what you're removing here?

No I have not tested what happens with overlays, but I believe they
are handled by the pre-U-Boot firmware. U-Boot certainly doesn't look
at the config.txt file.

Regards,
Simon