[PATCH 1/2] mach-snapdragon: pass fdt to qcom_parse_memory

[Caleb Connolly]

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 at samcday.com>

Reviewed-by: Caleb Connolly <caleb.connolly at 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;
>  }
> 

-- 
// Caleb (they/them)