Heya Casey, On Wednesday, 3 June 2026 at 11:47 PM, Casey Connolly <[email protected]> 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? > > On 6/1/26 10:12, Sam Day via B4 Relay wrote: > > From: Sam Day <[email protected]> > > > > 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? > > 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. 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 <[email protected]> > > --- > > 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; > > } > > > > /* > > > >
