On 20/01/2025 20:43, Sam Day wrote:
commit fc37a73e6679 ("fdt: Swap the signature for board_fdt_blob_setup()") introduced a subtle change to the Snapdragon implementation, removing the assignment to gd->fdt_blob partway through the function.
This breaks qcom_parse_memory() which was also called during board_fdt_blob_setup().
The underlying issue here is that qcom_parse_memory is using the of_ api to traverse a devicetree, which relies on the fdt_blob in global data.
Rather than relying on this subtle behaviour, explicitly pass the FDT that should be consulted for a /memory node.
Using the OF API is typically preferable because it's easier to read, but using the lower level fdt_ methods instead here doesn't add too much complexity, I think.
Finally, a minor tweak was made to board_fdt_blob_setup to use the passed fdt blob pointer instead of gd->fdt_blob, which removes the last of the references to global data in this area.
Fixes: fc37a73e6679 (fdt: Swap the signature for board_fdt_blob_setup()) Signed-off-by: Sam Day me@samcday.com
Reviewed-by: Caleb Connolly caleb.connolly@linaro.org
Thanks so much for sending this! I'm pretty sure the whole reason we called qcom_parse_memory() from board_fdt_blob_setup() originally was to allow for the usecase you add in patch 2, so it's nice to see that fulfilled :>
arch/arm/mach-snapdragon/board.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c index f1319df4314783622bb5f4f2410d1d4fd28a1c83..2ef936aab757c7045729a2dd91944f4f9bff917e 100644 --- a/arch/arm/mach-snapdragon/board.c +++ b/arch/arm/mach-snapdragon/board.c @@ -88,20 +88,21 @@ int dram_init_banksize(void) return 0; }
-static void qcom_parse_memory(void) +static void qcom_parse_memory(const void *fdt) {
- ofnode node;
- int offset; const fdt64_t *memory; int memsize; phys_addr_t ram_end = 0; int i, j, banks;
- node = ofnode_path("/memory");
- if (!ofnode_valid(node)) {
- offset = fdt_path_offset(fdt, "/memory");
- if (offset < 0) { log_err("No memory node found in device tree!\n"); return; }
- memory = ofnode_read_prop(node, "reg", &memsize);
- memory = fdt_getprop(fdt, offset, "reg", &memsize); if (!memory) { log_err("No memory configuration was provided by the previous bootloader!\n"); return;
@@ -158,7 +159,7 @@ int board_fdt_blob_setup(void **fdtp)
fdt = (struct fdt_header *)get_prev_bl_fdt_addr(); external_valid = fdt && !fdt_check_header(fdt);
- internal_valid = !fdt_check_header(gd->fdt_blob);
internal_valid = !fdt_check_header(*fdtp);
/*
- There is no point returning an error here, U-Boot can't do anything useful in this situation.
@@ -181,7 +182,7 @@ int board_fdt_blob_setup(void **fdtp) * Parse the /memory node while we're here, * this makes it easy to do other things early. */
- qcom_parse_memory();
qcom_parse_memory(*fdtp);
return ret;
}