[PATCH RESEND 12/16] mach-snapdragon: parse incoming ARM32 ATAGS

[Casey Connolly]

On 6/4/26 00:27, Sam Day wrote:
> Heya Casey,
> 
> On Wednesday, 3 June 2026 at 11:47 PM, Casey Connolly <casey.connolly at linaro.org> wrote:
> 
>> Hi Sam,
>>
>> (CC list definitely needs some tuning on the next revision XD)
> 
> Hmm? Is it incorrect to use `b4 prep --auto-to-cc` nowadays?

Urgh yeah I think this is a U-Boot issue with MAINTAINERS, needs some 
improvement but not your fault.
> 
>>
>> On 6/1/26 10:12, Sam Day via B4 Relay wrote:
>>> From: Sam Day <me at samcday.com>
>>>
>>> Many (often fused) bootloaders on older ARMv7 qcom SoCs do not support
>>> FDTs at all, and instead pass along ATAGS in r2.
>>>
>>> Minimal support for parsing memory info out of these tags is introduced,
>>> so that we can support mainline devicetrees, which expect the bootloader
>>> to populate /memory nodes.
>>
>> I think this should go its own armv7-specific file. It would probably
>> also make sense to just have a separate implementation of
>> board_fdt_blob_setup() since it's already really quite complicated.
> 
> I was considering that as well but decided against it, since sprinkling #ifdef
> everywhere is quite ugly, and the ATAGS handling code is quite minimal. Are you
> sure it really needs to be split out?

I would have the two implementations in different files
> 
>>
>> The SMEM series will do a bit of rework, in the next spin I'll probably
>> move the caching stuff out as well, maybe I should also take
>> board_fdt_blob_setup() then the file can be arm64 specific?
> 
> I'm not sure how we can have a separate version of board_fdt_blob_setup. On
> ARMv7, sometimes you'll br receiving ATAGS in r2, sometimes it'll be FDT. It
> depends on which bootloader you're chaining from.

I see, can we impose any limitations on this in practise? If not then I 
guess this is fine. I'd like to see all this logic cleaned up a bit as 
I'm just worried about introducing bugs

At the very least I'd like it if the atags parsing was moved to a 
different file then here we can do if 
(CONFIG_IS_ENABLED(...SNAPDRAGON_ARM32)
	qcom_parse_atags()

the compiler will optimise out the function call so there's no need for 
a stub version

> 
> e.g my nokia-fame can run U-Boot directly chained from SBL3 since the bootchain
> on this device has been broken enough, in which case U-Boot gets ATAGS. My
> samsung-expressltexx chains from lk2nd, in which case U-Boot gets an FDT.
> 
> Cheers,
> -Sam
> 
>>
>>>
>>> Signed-off-by: Sam Day <me at samcday.com>
>>> ---
>>>    arch/arm/mach-snapdragon/board.c | 119 ++++++++++++++++++++++++++++++---------
>>>    1 file changed, 91 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c
>>> index 627b566d178..753983467a0 100644
>>> --- a/arch/arm/mach-snapdragon/board.c
>>> +++ b/arch/arm/mach-snapdragon/board.c
>>> @@ -11,6 +11,7 @@
>>>
>>>    #include <asm/gpio.h>
>>>    #include <asm/io.h>
>>> +#include <asm/setup.h>
>>>    #include <asm/system.h>
>>>    #include <dm/device.h>
>>>    #include <dm/pinctrl.h>
>>> @@ -175,6 +176,67 @@ static int qcom_parse_memory(const void *fdt)
>>>    	return 0;
>>>    }
>>>
>>> +static bool qcom_atags_valid(const phys_addr_t p)
>>> +{
>>> +	const struct tag *tags = (const struct tag *)p;
>>> +
>>> +	return tags && tags->hdr.tag == ATAG_CORE &&
>>> +	       tags->hdr.size >= sizeof(struct tag_header) / sizeof(u32);
>>> +}
>>> +
>>> +static int qcom_parse_atags(const struct tag *tags)
>>> +{
>>> +	phys_addr_t ram_end = 0;
>>> +	const struct tag *t;
>>> +	bool atags_end = false;
>>> +	u32 words = 0;
>>> +	int j = 0;
>>> +
>>> +	memset(prevbl_ddr_banks, 0, sizeof(prevbl_ddr_banks));
>>> +
>>> +	for (t = tags; words < SZ_16K / sizeof(u32); t = tag_next(t)) {
>>> +		if (t->hdr.tag == ATAG_NONE) {
>>> +			atags_end = true;
>>> +			break;
>>> +		}
>>> +		if (t->hdr.size < sizeof(struct tag_header) / sizeof(u32))
>>> +			return -EINVAL;
>>> +		if (t->hdr.size > SZ_16K / sizeof(u32) - words)
>>> +			return -EINVAL;
>>> +
>>> +		words += t->hdr.size;
>>> +
>>> +		if (t->hdr.tag != ATAG_MEM)
>>> +			continue;
>>> +		if (t->hdr.size < tag_size(tag_mem32))
>>> +			return -EINVAL;
>>> +		if (!t->u.mem.size)
>>> +			continue;
>>> +
>>> +		prevbl_ddr_banks[j].start = t->u.mem.start;
>>> +		prevbl_ddr_banks[j].size = t->u.mem.size;
>>> +		ram_end = max(ram_end, (phys_addr_t)t->u.mem.start + t->u.mem.size);
>>> +		j++;
>>> +
>>> +		if (j == CONFIG_NR_DRAM_BANKS)
>>> +			break;
>>> +	}
>>> +
>>> +	if (!atags_end && j < CONFIG_NR_DRAM_BANKS) {
>>> +		log_err("Provided more memory banks than we can handle\n");
>>> +		return -EINVAL;
>>> +	}
>>> +	if (!j)
>>> +		return -ENODATA;
>>> +
>>> +	qsort(prevbl_ddr_banks, j, sizeof(prevbl_ddr_banks[0]), ddr_bank_cmp);
>>> +
>>> +	gd->ram_base = prevbl_ddr_banks[0].start;
>>> +	gd->ram_size = ram_end - gd->ram_base;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>    static void show_psci_version(void)
>>>    {
>>>    #ifdef CONFIG_ARM64
>>> @@ -227,42 +289,52 @@ static void qcom_psci_fixup(void *fdt)
>>>     */
>>>    int board_fdt_blob_setup(void **fdtp)
>>>    {
>>> -	struct fdt_header *external_fdt, *internal_fdt;
>>> -	bool internal_valid, external_valid;
>>> -	int ret = -ENODATA;
>>> -
>>> -	internal_fdt = (struct fdt_header *)*fdtp;
>>> -	external_fdt = (struct fdt_header *)get_prev_bl_fdt_addr();
>>> -	external_valid = external_fdt && !fdt_check_header(external_fdt);
>>> -	internal_valid = !fdt_check_header(internal_fdt);
>>> +	int ret = -ENODATA, setup_ret = -EEXIST;
>>> +	struct fdt_header *internal_fdt = *fdtp;
>>> +	phys_addr_t prev_bl_arg = get_prev_bl_fdt_addr();
>>> +	bool internal_valid = internal_fdt && !fdt_check_header(internal_fdt);
>>> +	bool external_is_fdt = prev_bl_arg &&
>>> +			       !fdt_check_header((const void *)prev_bl_arg);
>>>
>>> -	/*
>>> -	 * There is no point returning an error here, U-Boot can't do anything useful in this situation.
>>> -	 * Bail out while we can still print a useful error message.
>>> -	 */
>>> -	if (!internal_valid && !external_valid)
>>> +	if (internal_valid) {
>>> +		debug("Using built in FDT\n");
>>> +	} else if (external_is_fdt) {
>>> +		debug("Using external FDT\n");
>>> +		*fdtp = (void *)prev_bl_arg;
>>> +		setup_ret = 0;
>>> +	} else {
>>> +		/*
>>> +		 * There is no point returning an error here, U-Boot can't do
>>> +		 * anything useful in this situation. Bail out while we can
>>> +		 * still print a useful error message.
>>> +		 */
>>>    		panic("Internal FDT is invalid and no external FDT was provided! (fdt=%p)\n",
>>> -		      external_fdt);
>>> +		      (void *)prev_bl_arg);
>>> +	}
>>>
>>>    	/* Prefer memory information from internal DT if it's present */
>>>    	if (internal_valid)
>>>    		ret = qcom_parse_memory(internal_fdt);
>>>
>>> -	if (ret < 0 && external_valid) {
>>> +	if (ret < 0 && prev_bl_arg) {
>>>    		/* No internal FDT or it lacks a proper /memory node.
>>>    		 * The previous bootloader handed us something, let's try that.
>>>    		 */
>>>    		if (internal_valid)
>>>    			debug("No memory info in internal FDT, falling back to external\n");
>>>
>>> -		ret = qcom_parse_memory(external_fdt);
>>> +		/* the prev BL arg is either an FDT or ATAGS */
>>> +		if (external_is_fdt)
>>> +			ret = qcom_parse_memory((const void *)prev_bl_arg);
>>> +		if (ret < 0 && qcom_atags_valid(prev_bl_arg))
>>> +			ret = qcom_parse_atags((const struct tag *)prev_bl_arg);
>>>    	}
>>>
>>>    	if (ret < 0)
>>>    		panic("No valid memory ranges found!\n");
>>>
>>> -	/* If we have an external FDT, it can only have come from the Android bootloader. */
>>> -	if (external_valid)
>>> +	/* If we have a prev BL arg, we assume it came from ABL */
>>> +	if (prev_bl_arg)
>>>    		qcom_boot_source = QCOM_BOOT_SOURCE_ANDROID;
>>>    	else
>>>    		qcom_boot_source = QCOM_BOOT_SOURCE_XBL;
>>> @@ -270,18 +342,9 @@ int board_fdt_blob_setup(void **fdtp)
>>>    	debug("ram_base = %#011lx, ram_size = %#011llx\n",
>>>    	      gd->ram_base, (unsigned long long)gd->ram_size);
>>>
>>> -	if (internal_valid) {
>>> -		debug("Using built in FDT\n");
>>> -		ret = -EEXIST;
>>> -	} else {
>>> -		debug("Using external FDT\n");
>>> -		*fdtp = external_fdt;
>>> -		ret = 0;
>>> -	}
>>> -
>>>    	qcom_psci_fixup(*fdtp);
>>>
>>> -	return ret;
>>> +	return setup_ret;
>>>    }
>>>
>>>    /*
>>>
>>
>>