diff mbox series

[14/21] mach-snapdragon: dynamic load addresses

Message ID 20231121-b4-qcom-common-target-v1-14-9492198e0c15@linaro.org
State New
Headers show
Series Qualcomm generic board support | expand

Commit Message

Caleb Connolly Nov. 21, 2023, 5:09 p.m. UTC
Heavily inspired by Apple board code. Use the LMB allocator to configure
load addresses at runtime, and implement a lookup table for selecting a
devicetree.

As some Qualcomm RBx boards have different RAM capacities and base
addresses, it isn't possible to hardcode these regions.

Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 arch/arm/Kconfig                                 |  1 +
 arch/arm/mach-snapdragon/board.c                 | 36 ++++++++++++++++++++++++
 board/qualcomm/dragonboard410c/dragonboard410c.c |  2 +-
 3 files changed, 38 insertions(+), 1 deletion(-)

Comments

Tom Rini Nov. 21, 2023, 7:24 p.m. UTC | #1
On Tue, Nov 21, 2023 at 05:09:37PM +0000, Caleb Connolly wrote:

> Heavily inspired by Apple board code. Use the LMB allocator to configure
> load addresses at runtime, and implement a lookup table for selecting a
> devicetree.
> 
> As some Qualcomm RBx boards have different RAM capacities and base
> addresses, it isn't possible to hardcode these regions.
> 
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
[snip]
> +#define KERNEL_COMP_SIZE	SZ_32M
> +#define SZ_96M			(SZ_64M + SZ_32M)
> +
> +#define addr_alloc(lmb, size) lmb_alloc(lmb, size, SZ_2M)
> +
> +/* Stolen from arch/arm/mach-apple/board.c */
> +int board_late_init(void)
> +{
> +	struct lmb lmb;
> +	u32 status = 0;
> +
> +	lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob);
> +
> +	/* We need to be fairly conservative here as we support boards with just 1G of TOTAL RAM */
> +	status |= env_set_hex("kernel_addr_r", addr_alloc(&lmb, SZ_96M));
> +	status |= env_set_hex("loadaddr", addr_alloc(&lmb, SZ_64M));
> +	status |= env_set_hex("fdt_addr_r", addr_alloc(&lmb, SZ_2M));
> +	status |= env_set_hex("ramdisk_addr_r", addr_alloc(&lmb, SZ_96M));
> +	status |= env_set_hex("kernel_comp_addr_r", addr_alloc(&lmb, KERNEL_COMP_SIZE));
> +	status |= env_set_hex("kernel_comp_size", KERNEL_COMP_SIZE);
> +	status |= env_set_hex("scriptaddr", addr_alloc(&lmb, SZ_4M));
> +	status |= env_set_hex("pxefile_addr_r", addr_alloc(&lmb, SZ_4M));

First, with 1G total, we should I think be fine using SZ_128M and not
needing to add SZ_96M. Second, having worked on
https://patchwork.ozlabs.org/project/uboot/patch/20231119144338.630138-2-sjg@chromium.org/
a bit I think your sizes are a bit funny. We're looking at where to
decompress the provided Image file to and so 32M is too small. My
generic v6.6 Image is 40MB. One of those "would be nice" things to see
now that I've kicked things around a little was if all of our boot$foo
decompression logic just asked the lmb to give us a new region of
max-feasible-size for a Linux kernel (or we make it a choice between 64
or 128MB max supported kernel image). And further looking at the code
and matching it up with the function docs, it's even funkier than I had
thought, sigh.

And to try and be clear, the second is an ask, but the first is a change
request, those sizes are too small and inconsistent with supporting a
large uncompressed kernel.
Caleb Connolly Nov. 21, 2023, 8:47 p.m. UTC | #2
On 21/11/2023 19:24, Tom Rini wrote:
> On Tue, Nov 21, 2023 at 05:09:37PM +0000, Caleb Connolly wrote:
> 
>> Heavily inspired by Apple board code. Use the LMB allocator to configure
>> load addresses at runtime, and implement a lookup table for selecting a
>> devicetree.
>>
>> As some Qualcomm RBx boards have different RAM capacities and base
>> addresses, it isn't possible to hardcode these regions.
>>
>> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> [snip]
>> +#define KERNEL_COMP_SIZE	SZ_32M
>> +#define SZ_96M			(SZ_64M + SZ_32M)
>> +
>> +#define addr_alloc(lmb, size) lmb_alloc(lmb, size, SZ_2M)
>> +
>> +/* Stolen from arch/arm/mach-apple/board.c */
>> +int board_late_init(void)
>> +{
>> +	struct lmb lmb;
>> +	u32 status = 0;
>> +
>> +	lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob);
>> +
>> +	/* We need to be fairly conservative here as we support boards with just 1G of TOTAL RAM */
>> +	status |= env_set_hex("kernel_addr_r", addr_alloc(&lmb, SZ_96M));
>> +	status |= env_set_hex("loadaddr", addr_alloc(&lmb, SZ_64M));
>> +	status |= env_set_hex("fdt_addr_r", addr_alloc(&lmb, SZ_2M));
>> +	status |= env_set_hex("ramdisk_addr_r", addr_alloc(&lmb, SZ_96M));
>> +	status |= env_set_hex("kernel_comp_addr_r", addr_alloc(&lmb, KERNEL_COMP_SIZE));
>> +	status |= env_set_hex("kernel_comp_size", KERNEL_COMP_SIZE);
>> +	status |= env_set_hex("scriptaddr", addr_alloc(&lmb, SZ_4M));
>> +	status |= env_set_hex("pxefile_addr_r", addr_alloc(&lmb, SZ_4M));
> 
> First, with 1G total, we should I think be fine using SZ_128M and not
> needing to add SZ_96M. Second, having worked on
> https://patchwork.ozlabs.org/project/uboot/patch/20231119144338.630138-2-sjg@chromium.org/
> a bit I think your sizes are a bit funny. We're looking at where to
> decompress the provided Image file to and so 32M is too small. My
> generic v6.6 Image is 40MB. One of those "would be nice" things to see
> now that I've kicked things around a little was if all of our boot$foo
> decompression logic just asked the lmb to give us a new region of
> max-feasible-size for a Linux kernel (or we make it a choice between 64
> or 128MB max supported kernel image). And further looking at the code
> and matching it up with the function docs, it's even funkier than I had
> thought, sigh.

To your first point, yeah it looks like I can get away with 128M
uncompressed, it definitely feels a bit safer heh

To be honest, most of my testing has been booting via bootflow/bootefi
-> systemd-boot which I assume will be using the EFI allocator to load
everything. I still have a lot of local patches to push upstream,
otherwise I'd definitely be interested in finding a cleaner / more
generic solution here.
> 
> And to try and be clear, the second is an ask, but the first is a change
> request, those sizes are too small and inconsistent with supporting a
> large uncompressed kernel.
>
diff mbox series

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 1faa70bb658d..ae44fabb1609 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1075,6 +1075,7 @@  config ARCH_SNAPDRAGON
 	select OF_SEPARATE
 	select SMEM
 	select SPMI
+	select BOARD_LATE_INIT
 	select OF_BOARD
 	select SAVE_PREV_BL_FDT_ADDR
 	imply CMD_DM
diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c
index 521390ed6eed..765a2c2a95fc 100644
--- a/arch/arm/mach-snapdragon/board.c
+++ b/arch/arm/mach-snapdragon/board.c
@@ -18,6 +18,7 @@ 
 #include <linux/bug.h>
 #include <linux/psci.h>
 #include <linux/sizes.h>
+#include <lmb.h>
 #include <malloc.h>
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -110,6 +111,41 @@  int board_init(void)
 	return 0;
 }
 
+void __weak qcom_late_init(void)
+{
+}
+
+#define KERNEL_COMP_SIZE	SZ_32M
+#define SZ_96M			(SZ_64M + SZ_32M)
+
+#define addr_alloc(lmb, size) lmb_alloc(lmb, size, SZ_2M)
+
+/* Stolen from arch/arm/mach-apple/board.c */
+int board_late_init(void)
+{
+	struct lmb lmb;
+	u32 status = 0;
+
+	lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob);
+
+	/* We need to be fairly conservative here as we support boards with just 1G of TOTAL RAM */
+	status |= env_set_hex("kernel_addr_r", addr_alloc(&lmb, SZ_96M));
+	status |= env_set_hex("loadaddr", addr_alloc(&lmb, SZ_64M));
+	status |= env_set_hex("fdt_addr_r", addr_alloc(&lmb, SZ_2M));
+	status |= env_set_hex("ramdisk_addr_r", addr_alloc(&lmb, SZ_96M));
+	status |= env_set_hex("kernel_comp_addr_r", addr_alloc(&lmb, KERNEL_COMP_SIZE));
+	status |= env_set_hex("kernel_comp_size", KERNEL_COMP_SIZE);
+	status |= env_set_hex("scriptaddr", addr_alloc(&lmb, SZ_4M));
+	status |= env_set_hex("pxefile_addr_r", addr_alloc(&lmb, SZ_4M));
+
+	if (status)
+		log_warning("%s: Failed to set run time variables\n", __func__);
+
+	qcom_late_init();
+
+	return 0;
+}
+
 static void build_mem_map(void)
 {
 	int i;
diff --git a/board/qualcomm/dragonboard410c/dragonboard410c.c b/board/qualcomm/dragonboard410c/dragonboard410c.c
index bee8034ef92f..ab652e0a7c92 100644
--- a/board/qualcomm/dragonboard410c/dragonboard410c.c
+++ b/board/qualcomm/dragonboard410c/dragonboard410c.c
@@ -113,7 +113,7 @@  int misc_init_r(void)
 	return 0;
 }
 
-int board_late_init(void)
+int qcom_late_init(void)
 {
 	char serial[16];