On Mon, Dec 09, 2024 at 11:20:00AM -0700, Simon Glass wrote:

The existing values don't provide for decompressing an arm64 boot-image. Add those values and move things apart a bit so that a 50MB kernel can be accommodated.

Signed-off-by: Simon Glass sjg@chromium.org

(no changes since v1)

board/raspberrypi/rpi/rpi.env | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/board/raspberrypi/rpi/rpi.env b/board/raspberrypi/rpi/rpi.env index b79730a3e26..ee5a4810262 100644 --- a/board/raspberrypi/rpi/rpi.env +++ b/board/raspberrypi/rpi/rpi.env @@ -69,9 +69,11 @@ dfu_alt_info+=zImage fat 0 1 bootm_size=0x20000000

kernel_addr_r=0x00080000 -scriptaddr=0x02400000 -pxefile_addr_r=0x02500000 -fdt_addr_r=0x02600000 -ramdisk_addr_r=0x02700000 +kernel_comp_addr_r=0x02000000 +kernel_comp_size=0x02000000 +scriptaddr=0x05400000 +pxefile_addr_r=0x05500000 +fdt_addr_r=0x05600000 +ramdisk_addr_r=0x05700000

boot_targets=mmc usb pxe dhcp

As shown in https://patchwork.ozlabs.org/project/uboot/patch/20240723193430.3050251-1-wa... there's a large comment block to update to reflect and explain why the addresses are where they are. Please update that as well to explain the "why" of the "where" things are placed, thanks.

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@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?

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@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@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.

On Wed, Dec 11, 2024 at 10:22:24AM -0700, Simon Glass wrote:

Hi Tom,

On Mon, 9 Dec 2024 at 13:33, Tom Rini trini@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@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@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

Thanks. It seems likely that some no longer required work-arounds have built up in the pi code. But it's worth being extra cautious here given the breadth of the user base for these platforms and the need for them to really Just Work and the high cost associated with breakage.