[PATCH v3 0/5] rpi: Tidy up booting

[PATCH v3 0/5] rpi: Tidy up booting

[Simon Glass]

This series allows rpi to boot a compressed Ubuntu kernel with ~100MB
ramdisk, by expanding the available space.

It also tidies up some strange behaviour with the provided FDT, where a
separate pointer is maintained to it, even though U-Boot has copied it
and placed it in its own space. This avoids strange bugs where it
accidentally gets overwritten when loading a file into memory.

The patch to expand the devicetree was dropped, meaning that people
should be careful to unset fdt_addr in the environment.

Changes in v3:
- Add to the existing comment block
- Update the comment block with the new values, including compression

Changes in v2:
- Add new patch to make myself an rpi maintainer
- Add new patch to set bootm_size
- Add new patch to drop fdt_high and initrd_high
- Drop patch to allow expanding the devicetree during relocation

Simon Glass (5):
  rpi: Add myself to the list of maintainers
  rpi: Set bootm_size to 512MB
  rpi: Drop fdt_high and initrd_high
  rpi: Update environment to support booti and large initrd
  rpi: Use the U-Boot control FDT for fdt_addr

 MAINTAINERS                   |  1 +
 board/raspberrypi/rpi/rpi.c   | 20 ++++++++-----------
 board/raspberrypi/rpi/rpi.env | 37 +++++++++++++++++++----------------
 3 files changed, 29 insertions(+), 29 deletions(-)

-- 
2.34.1

[PATCH v3 1/5] rpi: Add myself to the list of maintainers

[Simon Glass]

Add my own name to the list, since existing maintainers are fairly busy.

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

(no changes since v2)

Changes in v2:
- Add new patch to make myself an rpi maintainer

 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8c6c0c2a4bc..4b9bb9ef3f1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -210,6 +210,7 @@ N:	aspeed
 ARM BROADCOM BCM283X / BCM27XX
 M:	Matthias Brugger <mbrugger@suse.com>
 M:	Peter Robinson <pbrobinson@gmail.com>
+M:	Simon Glass <sjg@chromium.org>
 S:	Maintained
 F:	arch/arm/dts/bcm283*
 F:	arch/arm/mach-bcm283x/
-- 
2.34.1

Re: [PATCH v3 1/5] rpi: Add myself to the list of maintainers

[Peter Robinson]

Simon,

Please don't send multiple versions of patch series back to back.

There's been 3 versions of this patch in less than one working day. You
wonder why we're busy, I can't get to one version of you patch set before
you send another 12 versions.... It's generally considered, at least in the
kernel community, you should only be sending a version of a patch set once
a week to allow all discussion to complete, and then only sending it if all
concerns are resolved.

Peter

On Mon, 9 Dec 2024 at 19:55, Simon Glass <sjg@chromium.org> wrote:

> Add my own name to the list, since existing maintainers are fairly busy.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Add new patch to make myself an rpi maintainer
>
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8c6c0c2a4bc..4b9bb9ef3f1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -210,6 +210,7 @@ N:  aspeed
>  ARM BROADCOM BCM283X / BCM27XX
>  M:     Matthias Brugger <mbrugger@suse.com>
>  M:     Peter Robinson <pbrobinson@gmail.com>
> +M:     Simon Glass <sjg@chromium.org>
>  S:     Maintained
>  F:     arch/arm/dts/bcm283*
>  F:     arch/arm/mach-bcm283x/
> --
> 2.34.1
>
>

[PATCH v3 2/5] rpi: Set bootm_size to 512MB

[Simon Glass]

Set this option so that all boot images stay within the bottom 512MB of
memory. This should allow us to drop the fdt_high and initrd_high
options.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Tom Rini <trini@konsulko.com>
Suggested-by: Tom Rini <trini@konsulko.com>
---

Changes in v3:
- Add to the existing comment block

Changes in v2:
- Add new patch to set bootm_size

 board/raspberrypi/rpi/rpi.env | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/board/raspberrypi/rpi/rpi.env b/board/raspberrypi/rpi/rpi.env
index 30228285edd..a327fccc77f 100644
--- a/board/raspberrypi/rpi/rpi.env
+++ b/board/raspberrypi/rpi/rpi.env
@@ -60,7 +60,12 @@ dfu_alt_info+=zImage fat 0 1
  * Even with the smallest possible CPU-GPU memory split of the CPU getting
  * only 64M, the remaining 25M starting at 0x02700000 should allow quite
  * large initrds before they start colliding with U-Boot.
+ *
+ * Limit bootm_size to 512MB so that all boot images stay within the bottom
+ * 512MB of memory
  */
+bootm_size=0x20000000
+
 #ifdef CONFIG_ARM64
 fdt_high=ffffffffffffffff
 initrd_high=ffffffffffffffff
-- 
2.34.1

RE: [PATCH v3 2/5] rpi: Set bootm_size to 512MB

[Jaehoon Chung]

> -----Original Message-----
> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Simon Glass
> Sent: Tuesday, December 10, 2024 4:55 AM
> To: U-Boot Mailing List <u-boot@lists.denx.de>
> Cc: Tom Rini <trini@konsulko.com>; Stephen Warren <swarren@wwwdotorg.org>; Stephen Warren
> <swarren@nvidia.com>; Matthias Brugger <mbrugger@suse.com>; Simon Glass <sjg@chromium.org>; Peter
> Robinson <pbrobinson@gmail.com>
> Subject: [PATCH v3 2/5] rpi: Set bootm_size to 512MB
>
> Set this option so that all boot images stay within the bottom 512MB of
> memory. This should allow us to drop the fdt_high and initrd_high
> options.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> Suggested-by: Tom Rini <trini@konsulko.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
Tested-by: Jaehoon Chung <jh80.chung@samsung.com>


> ---
>
> Changes in v3:
> - Add to the existing comment block
>
> Changes in v2:
> - Add new patch to set bootm_size
>
>  board/raspberrypi/rpi/rpi.env | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/board/raspberrypi/rpi/rpi.env b/board/raspberrypi/rpi/rpi.env
> index 30228285edd..a327fccc77f 100644
> --- a/board/raspberrypi/rpi/rpi.env
> +++ b/board/raspberrypi/rpi/rpi.env
> @@ -60,7 +60,12 @@ dfu_alt_info+=zImage fat 0 1
>   * Even with the smallest possible CPU-GPU memory split of the CPU getting
>   * only 64M, the remaining 25M starting at 0x02700000 should allow quite
>   * large initrds before they start colliding with U-Boot.
> + *
> + * Limit bootm_size to 512MB so that all boot images stay within the bottom
> + * 512MB of memory
>   */
> +bootm_size=0x20000000
> +
>  #ifdef CONFIG_ARM64
>  fdt_high=ffffffffffffffff
>  initrd_high=ffffffffffffffff
> --
> 2.34.1

Re: [PATCH v3 2/5] rpi: Set bootm_size to 512MB

[Peter Robinson]

On Mon, 9 Dec 2024 at 19:55, Simon Glass <sjg@chromium.org> wrote:

> Set this option so that all boot images stay within the bottom 512MB of
> memory. This should allow us to drop the fdt_high and initrd_high
> options.
>

How does this work for a device with < 256Mb of RAM? There's a number of
the original RPi series that have 256Mb of RAM and that equates to less
than that due to reservations for the vendor firmware.


> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> Suggested-by: Tom Rini <trini@konsulko.com>
> ---
>
> Changes in v3:
> - Add to the existing comment block
>
> Changes in v2:
> - Add new patch to set bootm_size
>
>  board/raspberrypi/rpi/rpi.env | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/board/raspberrypi/rpi/rpi.env b/board/raspberrypi/rpi/rpi.env
> index 30228285edd..a327fccc77f 100644
> --- a/board/raspberrypi/rpi/rpi.env
> +++ b/board/raspberrypi/rpi/rpi.env
> @@ -60,7 +60,12 @@ dfu_alt_info+=zImage fat 0 1
>   * Even with the smallest possible CPU-GPU memory split of the CPU getting
>   * only 64M, the remaining 25M starting at 0x02700000 should allow quite
>   * large initrds before they start colliding with U-Boot.
> + *
> + * Limit bootm_size to 512MB so that all boot images stay within the
> bottom
> + * 512MB of memory
>   */
> +bootm_size=0x20000000
> +
>  #ifdef CONFIG_ARM64
>  fdt_high=ffffffffffffffff
>  initrd_high=ffffffffffffffff
> --
> 2.34.1
>
>

Re: [PATCH v3 2/5] rpi: Set bootm_size to 512MB

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1093 bytes --]

On Wed, Dec 11, 2024 at 04:27:42PM +0000, Peter Robinson wrote:
> On Mon, 9 Dec 2024 at 19:55, Simon Glass <sjg@chromium.org> wrote:
> 
> > Set this option so that all boot images stay within the bottom 512MB of
> > memory. This should allow us to drop the fdt_high and initrd_high
> > options.
> >
> 
> How does this work for a device with < 256Mb of RAM? There's a number of
> the original RPi series that have 256Mb of RAM and that equates to less
> than that due to reservations for the vendor firmware.

I based this on the previous thread that said they all have 512MiB
minimum, but I guess that's just the newer ones that are 64bit too.

Top of my head? It's been a decade, but I _think_ as part of the
relocation we see that we don't have memory a base+512MiB and so go with
lower, but I wouldn't make any wager on it and would rather test (which
can also be done easy enough on another board, just set the limit to
more memory than exists, see where we relocate things to). I'll try and
try that out soon'ish, just neck deep in a few other tasks atm.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v3 2/5] rpi: Set bootm_size to 512MB

[Simon Glass]

Hi Peter,

On Wed, 11 Dec 2024 at 09:27, Peter Robinson <pbrobinson@gmail.com> wrote:
>
>
>
> On Mon, 9 Dec 2024 at 19:55, Simon Glass <sjg@chromium.org> wrote:
>>
>> Set this option so that all boot images stay within the bottom 512MB of
>> memory. This should allow us to drop the fdt_high and initrd_high
>> options.
>
>
> How does this work for a device with < 256Mb of RAM? There's a number of the original RPi series that have 256Mb of RAM and that equates to less than that due to reservations for the vendor firmware.

Looking at boot_relocate_fdt(), this is just a maximum, and lmb is
used to allocate the FDT / cmdline. So this patch shouldn't affect
that.

But the later patch to move the ramdisk up will cause problems. Based
on the comment, a 256MB (I assume you mean) device can have as little
as 64MB of RAM available to the ARM core, which is not enough to load
a ramdisk from the new start-address of about 90MB.

I found an old rpi1 which I could hook up to my lab, perhaps. Could
you point me to a distro-image that boots OK on that, ideally with
only 64MB available?

>
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> Reviewed-by: Tom Rini <trini@konsulko.com>
>> Suggested-by: Tom Rini <trini@konsulko.com>
>> ---
>>
>> Changes in v3:
>> - Add to the existing comment block
>>
>> Changes in v2:
>> - Add new patch to set bootm_size
>>
>>  board/raspberrypi/rpi/rpi.env | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/board/raspberrypi/rpi/rpi.env b/board/raspberrypi/rpi/rpi.env
>> index 30228285edd..a327fccc77f 100644
>> --- a/board/raspberrypi/rpi/rpi.env
>> +++ b/board/raspberrypi/rpi/rpi.env
>> @@ -60,7 +60,12 @@ dfu_alt_info+=zImage fat 0 1
>>   * Even with the smallest possible CPU-GPU memory split of the CPU getting
>>   * only 64M, the remaining 25M starting at 0x02700000 should allow quite
>>   * large initrds before they start colliding with U-Boot.
>> + *
>> + * Limit bootm_size to 512MB so that all boot images stay within the bottom
>> + * 512MB of memory
>>   */
>> +bootm_size=0x20000000
>> +
>>  #ifdef CONFIG_ARM64
>>  fdt_high=ffffffffffffffff
>>  initrd_high=ffffffffffffffff
>> --
>> 2.34.1
>>

Re: [PATCH v3 2/5] rpi: Set bootm_size to 512MB

[Simon Glass]

Hi Peter,

On Wed, 11 Dec 2024 at 10:26, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Peter,
>
> On Wed, 11 Dec 2024 at 09:27, Peter Robinson <pbrobinson@gmail.com> wrote:
> >
> >
> >
> > On Mon, 9 Dec 2024 at 19:55, Simon Glass <sjg@chromium.org> wrote:
> >>
> >> Set this option so that all boot images stay within the bottom 512MB of
> >> memory. This should allow us to drop the fdt_high and initrd_high
> >> options.
> >
> >
> > How does this work for a device with < 256Mb of RAM? There's a number of the original RPi series that have 256Mb of RAM and that equates to less than that due to reservations for the vendor firmware.
>
> Looking at boot_relocate_fdt(), this is just a maximum, and lmb is
> used to allocate the FDT / cmdline. So this patch shouldn't affect
> that.
>
> But the later patch to move the ramdisk up will cause problems. Based
> on the comment, a 256MB (I assume you mean) device can have as little
> as 64MB of RAM available to the ARM core, which is not enough to load
> a ramdisk from the new start-address of about 90MB.
>
> I found an old rpi1 which I could hook up to my lab, perhaps. Could
> you point me to a distro-image that boots OK on that, ideally with
> only 64MB available?

Well I set it up with Raspbian. It seems to boot OK into the kernel.
But unfortunately I only have a 512MB device.

"When I were a lad, we had hard drives of 20MB and were grateful for it!"

>
> >
> >>
> >> Signed-off-by: Simon Glass <sjg@chromium.org>
> >> Reviewed-by: Tom Rini <trini@konsulko.com>
> >> Suggested-by: Tom Rini <trini@konsulko.com>
> >> ---
> >>
> >> Changes in v3:
> >> - Add to the existing comment block
> >>
> >> Changes in v2:
> >> - Add new patch to set bootm_size
> >>
> >>  board/raspberrypi/rpi/rpi.env | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/board/raspberrypi/rpi/rpi.env b/board/raspberrypi/rpi/rpi.env
> >> index 30228285edd..a327fccc77f 100644
> >> --- a/board/raspberrypi/rpi/rpi.env
> >> +++ b/board/raspberrypi/rpi/rpi.env
> >> @@ -60,7 +60,12 @@ dfu_alt_info+=zImage fat 0 1
> >>   * Even with the smallest possible CPU-GPU memory split of the CPU getting
> >>   * only 64M, the remaining 25M starting at 0x02700000 should allow quite
> >>   * large initrds before they start colliding with U-Boot.
> >> + *
> >> + * Limit bootm_size to 512MB so that all boot images stay within the bottom
> >> + * 512MB of memory
> >>   */
> >> +bootm_size=0x20000000
> >> +
> >>  #ifdef CONFIG_ARM64
> >>  fdt_high=ffffffffffffffff
> >>  initrd_high=ffffffffffffffff
> >> --
> >> 2.34.1
> >>

Regards,
Simon

[PATCH v3 3/5] rpi: Drop fdt_high and initrd_high

[Simon Glass]

These are not needed now since there is a bootm_size setting to keep
things within the lower part of memory.

Drop them.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Tom Rini <trini@konsulko.com>
Suggested-by: Tom Rini <trini@konsulko.com>
---

(no changes since v2)

Changes in v2:
- Add new patch to drop fdt_high and initrd_high

 board/raspberrypi/rpi/rpi.env | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/board/raspberrypi/rpi/rpi.env b/board/raspberrypi/rpi/rpi.env
index a327fccc77f..9b9fad82828 100644
--- a/board/raspberrypi/rpi/rpi.env
+++ b/board/raspberrypi/rpi/rpi.env
@@ -66,13 +66,6 @@ dfu_alt_info+=zImage fat 0 1
  */
 bootm_size=0x20000000
 
-#ifdef CONFIG_ARM64
-fdt_high=ffffffffffffffff
-initrd_high=ffffffffffffffff
-#else
-fdt_high=ffffffff
-initrd_high=ffffffff
-#endif
 kernel_addr_r=0x00080000
 scriptaddr=0x02400000
 pxefile_addr_r=0x02500000
-- 
2.34.1

RE: [PATCH v3 3/5] rpi: Drop fdt_high and initrd_high

[Jaehoon Chung]

> -----Original Message-----
> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Simon Glass
> Sent: Tuesday, December 10, 2024 4:55 AM
> To: U-Boot Mailing List <u-boot@lists.denx.de>
> Cc: Tom Rini <trini@konsulko.com>; Stephen Warren <swarren@wwwdotorg.org>; Stephen Warren
> <swarren@nvidia.com>; Matthias Brugger <mbrugger@suse.com>; Simon Glass <sjg@chromium.org>; Peter
> Robinson <pbrobinson@gmail.com>
> Subject: [PATCH v3 3/5] rpi: Drop fdt_high and initrd_high
>
> These are not needed now since there is a bootm_size setting to keep
> things within the lower part of memory.
>
> Drop them.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> Suggested-by: Tom Rini <trini@konsulko.com>


Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
Tested-by: Jaehoon Chung <jh80.chung@samsung.com>

> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Add new patch to drop fdt_high and initrd_high
>
>  board/raspberrypi/rpi/rpi.env | 7 -------
>  1 file changed, 7 deletions(-)
>
> diff --git a/board/raspberrypi/rpi/rpi.env b/board/raspberrypi/rpi/rpi.env
> index a327fccc77f..9b9fad82828 100644
> --- a/board/raspberrypi/rpi/rpi.env
> +++ b/board/raspberrypi/rpi/rpi.env
> @@ -66,13 +66,6 @@ dfu_alt_info+=zImage fat 0 1
>   */
>  bootm_size=0x20000000
>
> -#ifdef CONFIG_ARM64
> -fdt_high=ffffffffffffffff
> -initrd_high=ffffffffffffffff
> -#else
> -fdt_high=ffffffff
> -initrd_high=ffffffff
> -#endif
>  kernel_addr_r=0x00080000
>  scriptaddr=0x02400000
>  pxefile_addr_r=0x02500000
> --
> 2.34.1

[PATCH v3 4/5] rpi: Update environment to support booti and large initrd

[Simon Glass]

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

Changes in v3:
- Update the comment block with the new values, including compression

 board/raspberrypi/rpi/rpi.env | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/board/raspberrypi/rpi/rpi.env b/board/raspberrypi/rpi/rpi.env
index 9b9fad82828..9ac9d6768ca 100644
--- a/board/raspberrypi/rpi/rpi.env
+++ b/board/raspberrypi/rpi/rpi.env
@@ -48,28 +48,33 @@ dfu_alt_info+=zImage fat 0 1
  *
  * scriptaddr and pxefile_addr_r can be pretty much anywhere that doesn't
  * conflict with something else. Reserving 1M for each of them at
- * 0x02400000-0x02500000 and 0x02500000-0x02600000 should be plenty.
+ * 0x05400000-0x05500000 and 0x05500000-0x05600000 should be plenty.
  *
  * On ARM, both the DTB and any possible initrd must be loaded such that they
  * fit inside the lowmem mapping in Linux. In practice, this usually means not
  * more than ~700M away from the start of the kernel image but this number can
  * be larger OR smaller depending on e.g. the 'vmalloc=xxxM' command line
  * parameter given to the kernel. So reserving memory from low to high
- * satisfies this constraint again. Reserving 1M at 0x02600000-0x02700000 for
- * the DTB leaves rest of the free RAM to the initrd starting at 0x02700000.
- * Even with the smallest possible CPU-GPU memory split of the CPU getting
- * only 64M, the remaining 25M starting at 0x02700000 should allow quite
- * large initrds before they start colliding with U-Boot.
+ * satisfies this constraint again. Reserving 1M at 0x05600000-0x05700000 for
+ * the DTB leaves rest of the free RAM to the initrd starting at 0x05700000.
+ * This means that the board must have at least 128MB of RAM available to
+ * U-Boot, more if the initrd is large.
  *
- * Limit bootm_size to 512MB so that all boot images stay within the bottom
+ * For compressed kernels, the maximum size is just under 32MB, with an area for
+ * decompression at 0x02000000 with space for 52MB, which is plenty for current
+ * kernels.
+ *
+ * limit bootm_size to 512MB so that all boot images stay within the bottom
  * 512MB of memory
  */
 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=0x03400000
+scriptaddr=0x05400000
+pxefile_addr_r=0x05500000
+fdt_addr_r=0x05600000
+ramdisk_addr_r=0x05700000
 
 boot_targets=mmc usb pxe dhcp
-- 
2.34.1

RE: [PATCH v3 4/5] rpi: Update environment to support booti and large initrd

[Jaehoon Chung]

> -----Original Message-----
> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Simon Glass
> Sent: Tuesday, December 10, 2024 4:55 AM
> To: U-Boot Mailing List <u-boot@lists.denx.de>
> Cc: Tom Rini <trini@konsulko.com>; Stephen Warren <swarren@wwwdotorg.org>; Stephen Warren
> <swarren@nvidia.com>; Matthias Brugger <mbrugger@suse.com>; Simon Glass <sjg@chromium.org>; Peter
> Robinson <pbrobinson@gmail.com>
> Subject: [PATCH v3 4/5] rpi: Update environment to support booti and large initrd
>
> 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>


Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
Tested-by: Jaehoon Chung <jh80.chung@samsung.com>

> ---
>
> Changes in v3:
> - Update the comment block with the new values, including compression
>
>  board/raspberrypi/rpi/rpi.env | 27 ++++++++++++++++-----------
>  1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/board/raspberrypi/rpi/rpi.env b/board/raspberrypi/rpi/rpi.env
> index 9b9fad82828..9ac9d6768ca 100644
> --- a/board/raspberrypi/rpi/rpi.env
> +++ b/board/raspberrypi/rpi/rpi.env
> @@ -48,28 +48,33 @@ dfu_alt_info+=zImage fat 0 1
>   *
>   * scriptaddr and pxefile_addr_r can be pretty much anywhere that doesn't
>   * conflict with something else. Reserving 1M for each of them at
> - * 0x02400000-0x02500000 and 0x02500000-0x02600000 should be plenty.
> + * 0x05400000-0x05500000 and 0x05500000-0x05600000 should be plenty.
>   *
>   * On ARM, both the DTB and any possible initrd must be loaded such that they
>   * fit inside the lowmem mapping in Linux. In practice, this usually means not
>   * more than ~700M away from the start of the kernel image but this number can
>   * be larger OR smaller depending on e.g. the 'vmalloc=xxxM' command line
>   * parameter given to the kernel. So reserving memory from low to high
> - * satisfies this constraint again. Reserving 1M at 0x02600000-0x02700000 for
> - * the DTB leaves rest of the free RAM to the initrd starting at 0x02700000.
> - * Even with the smallest possible CPU-GPU memory split of the CPU getting
> - * only 64M, the remaining 25M starting at 0x02700000 should allow quite
> - * large initrds before they start colliding with U-Boot.
> + * satisfies this constraint again. Reserving 1M at 0x05600000-0x05700000 for
> + * the DTB leaves rest of the free RAM to the initrd starting at 0x05700000.
> + * This means that the board must have at least 128MB of RAM available to
> + * U-Boot, more if the initrd is large.
>   *
> - * Limit bootm_size to 512MB so that all boot images stay within the bottom
> + * For compressed kernels, the maximum size is just under 32MB, with an area for
> + * decompression at 0x02000000 with space for 52MB, which is plenty for current
> + * kernels.
> + *
> + * limit bootm_size to 512MB so that all boot images stay within the bottom
>   * 512MB of memory
>   */
>  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=0x03400000
> +scriptaddr=0x05400000
> +pxefile_addr_r=0x05500000
> +fdt_addr_r=0x05600000
> +ramdisk_addr_r=0x05700000
>
>  boot_targets=mmc usb pxe dhcp
> --
> 2.34.1

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

[Simon Glass]

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

(no changes since v2)

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)
-- 
2.34.1

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

[Jaehoon Chung]

> -----Original Message-----
> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Simon Glass
> Sent: Tuesday, December 10, 2024 4:55 AM
> To: U-Boot Mailing List <u-boot@lists.denx.de>
> Cc: Tom Rini <trini@konsulko.com>; Stephen Warren <swarren@wwwdotorg.org>; Stephen Warren
> <swarren@nvidia.com>; Matthias Brugger <mbrugger@suse.com>; Simon Glass <sjg@chromium.org>; Francois
> Berder <fberder@outlook.fr>; Ivan T. Ivanov <iivanov@suse.de>; Patrick Rudolph
> <patrick.rudolph@9elements.com>; Peter Robinson <pbrobinson@gmail.com>; Rasmus Villemoes
> <rasmus.villemoes@prevas.dk>
> Subject: [PATCH v3 5/5] rpi: Use the U-Boot control FDT for fdt_addr
>
> 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>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
Tested-by: Jaehoon Chung <jh80.chung@samsung.com>

> ---
>
> (no changes since v2)
>
> 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)
> --
> 2.34.1

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

[Peter Robinson]

On Mon, 9 Dec 2024 at 19:55, Simon Glass <sjg@chromium.org> wrote:

> The fdt_addr variable is used in extlinux as a fallback devicetree if
> none is provided by the boot command.
>

And what about where extlinux isn't used?


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

Please re-word the above, it doesn't make sense to me, "the rpi_4 I am
using" .... what about the other millions of RPis out there?


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

Don't remove it, re-word it so it's up to date.


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

The below looks irrelevant to the patch at hand, it looks like it should be
in the cover letter.


> 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>
> ---
>
> (no changes since v2)
>
> 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)
> --
> 2.34.1
>
>

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

[Simon Glass]

Hi Peter,

On Wed, 11 Dec 2024 at 09:36, Peter Robinson <pbrobinson@gmail.com> wrote:
>
>
>
> On Mon, 9 Dec 2024 at 19:55, Simon Glass <sjg@chromium.org> wrote:
>>
>> The fdt_addr variable is used in extlinux as a fallback devicetree if
>> none is provided by the boot command.
>
>
> And what about where extlinux isn't used?
>
>>
>> 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.
>
>
> Please re-word the above, it doesn't make sense to me, "the rpi_4 I am using" .... what about the other millions of RPis out there?
>
>>
>> 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.
>
>
> Don't remove it, re-word it so it's up to date.
>
>>
>> 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.
>
>
> The below looks irrelevant to the patch at hand, it looks like it should be in the cover letter.

Do you have any comments on the code in this series?

>
>>
>> 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>
>> ---
>>
>> (no changes since v2)
>>
>> 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)
>> --
>> 2.34.1
>>

Regards,
Simon

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

[Peter Robinson]

Hi Simon.

On Mon, 16 Dec 2024 at 00:27, Simon Glass <sjg@chromium.org> wrote:

> Hi Peter,
>
> On Wed, 11 Dec 2024 at 09:36, Peter Robinson <pbrobinson@gmail.com> wrote:
> >
> >
> >
> > On Mon, 9 Dec 2024 at 19:55, Simon Glass <sjg@chromium.org> wrote:
> >>
> >> The fdt_addr variable is used in extlinux as a fallback devicetree if
> >> none is provided by the boot command.
> >
> >
> > And what about where extlinux isn't used?
> >
> >>
> >> 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.
> >
> >
> > Please re-word the above, it doesn't make sense to me, "the rpi_4 I am
> using" .... what about the other millions of RPis out there?
> >
> >>
> >> 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.
> >
> >
> > Don't remove it, re-word it so it's up to date.
> >
> >>
> >> 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.
> >
> >
> > The below looks irrelevant to the patch at hand, it looks like it should
> be in the cover letter.
>
> Do you have any comments on the code in this series?
>

I did already comment on it, please go back through your emails ;-)


> >
> >>
> >> 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>
> >> ---
> >>
> >> (no changes since v2)
> >>
> >> 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)
> >> --
> >> 2.34.1
> >>
>
> Regards,
> Simon
>

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

[Simon Glass]

Hi Peter,

On Wed, 18 Dec 2024 at 09:26, Peter Robinson <pbrobinson@gmail.com> wrote:
>
> Hi Simon.
>
> On Mon, 16 Dec 2024 at 00:27, Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Peter,
>>
>> On Wed, 11 Dec 2024 at 09:36, Peter Robinson <pbrobinson@gmail.com> wrote:
>> >
>> >
>> >
>> > On Mon, 9 Dec 2024 at 19:55, Simon Glass <sjg@chromium.org> wrote:
>> >>
>> >> The fdt_addr variable is used in extlinux as a fallback devicetree if
>> >> none is provided by the boot command.
>> >
>> >
>> > And what about where extlinux isn't used?
>> >
>> >>
>> >> 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.
>> >
>> >
>> > Please re-word the above, it doesn't make sense to me, "the rpi_4 I am using" .... what about the other millions of RPis out there?
>> >
>> >>
>> >> 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.
>> >
>> >
>> > Don't remove it, re-word it so it's up to date.
>> >
>> >>
>> >> 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.
>> >
>> >
>> > The below looks irrelevant to the patch at hand, it looks like it should be in the cover letter.
>>
>> Do you have any comments on the code in this series?
>
>
> I did already comment on it, please go back through your emails ;-)

The only thing I got was some comments about the commit message and
the comment for set_fdt_addr(). I sent a new v4 here:

https://patchwork.ozlabs.org/project/uboot/patch/20241220003447.2913443-6-sjg@chromium.org/

My question was really about whether you are happy with the code itself.