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

[Simon Glass]

Hi Tom,

On Mon, 9 Dec 2024 at 13:33, Tom Rini <trini at konsulko.com> wrote:
>
> On Mon, Dec 09, 2024 at 12:27:18PM -0700, Simon Glass wrote:
> > 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.
>
> Yes, they're handled by being applied to the device tree we're passed by
> the previous stage and stored in fw_dtb_pointer. So checking that the
> device tree has the expected overlay applied is something that needs
> doing before this can be applied.

Just so I understand this request, the checks are:

1. That adding/removing an overlay in config.txt is reflected in the
devicetree seen by U-Boot
2. That U-Boot then passes that DT to Linux?

If I add this line:

dtoverlay=disable-bt

then Linux boots with serial output (I have enable_uart=1 in there as well)

If I remove the dtoverlay, it does not have serial output. So yes,
this seems to be working fine.


For the record, the flow (within U-Boot, with this series) is:

a. fw_dtb_pointer is set in lowlevel.S
b. board_fdt_blob_setup() is called to set gd->fdt_blob to that value
c. during relocation, U-Boot copies the devicetree to the top of
memory, setting gd->fdt_blob to the new value
d. during loading, U-Boot creates a copy of the gd->fdt_blob
devicetree and sets working_fdt to it*
e. U-Boot applies fix-ups to working_fdt

* fdt_high is unset and there is no devicetree packaged with the OS

Regards,
Simon