[PATCH 00/11] mach-snapdragon: various improvements for newer boards
Supporting the newer SM8550 and SM8650 SoCs unfortunately requires a bump in complexity for us. Qualcomm changed a lot about how the memory map is handed over to the "kernel", adding many holes, not mapping certain regions, and adding regions with 0 size. The SM8650 HDK has a whopping 14 memory regions, some as small as 44k.
Supporting this properly has proven to be a bit of a headache, but I think this implementation is "pretty good".
In addition, we set a valid fallback fdt_addr_r in the U-Boot environment, allocate a buffer for fastboot, and set the loadaddr variable too.
board_fdt_blob_setup() is refactored for readability and potential future expansion (e.g. if supporting multi-dtb FIT becomes desirable).
Finally, a function is proposed to allow for mapping new memory regions at runtime, and the cmd-db driver makes use of it to map itself, since SM8650 boards don't seem to include it in their memory map.
Tested on SM8650 HDK, SDM845 OnePlus 6, SM6115 RB2, SM8250 RB5.
--- Caleb Connolly (10): mach-snapdragon: refactor board_fdt_blob_setup() mach-snapdragon: parse memory ourselves mach-snapdragon: set serial number mach-snapdragon: allocate fastboot buffer dynamically mach-snapdragon: populate fallback FDT mach-snapdragon: set loadaddr armv8: mmu: add a way to map additional regions soc: qcom: cmd-db: use strncmp() instead of memcmp() soc: qcom: cmd-db: map cmd-db region qcom_defconfig: bump CONFIG_NR_DRAM_BANKS
Neil Armstrong (1): mach-snapdragon: use 1MiB for get_page_table_size()
arch/arm/cpu/armv8/cache_v8.c | 25 +++++ arch/arm/include/asm/system.h | 10 ++ arch/arm/mach-snapdragon/board.c | 203 ++++++++++++++++++++++++++++++++++----- configs/qcom_defconfig | 1 + drivers/soc/qcom/cmd-db.c | 11 ++- 5 files changed, 221 insertions(+), 29 deletions(-) --- change-id: 20240809-b4-snapdragon-improvements-fd6d714a7fbd base-commit: a2ce853383b18a2cf920268ee341f2585a11adef
// Caleb (they/them)
If U-Boot has a DTB built in (appended to the image directly) then this was likely intentional, we should prioritise it over one provided by ABL (if there was one).
Make this behaviour explicit, and panic if no valid DTB could be found anywhere. Returning an error is not useful in this case as U-Boot would just crash later in a more confusing way.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org --- arch/arm/mach-snapdragon/board.c | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c index b439a19ec7eb..2881de28a0a3 100644 --- a/arch/arm/mach-snapdragon/board.c +++ b/arch/arm/mach-snapdragon/board.c @@ -17,8 +17,9 @@ #include <dm/uclass-internal.h> #include <dm/read.h> #include <power/regulator.h> #include <env.h> +#include <fdt_support.h> #include <init.h> #include <linux/arm-smccc.h> #include <linux/bug.h> #include <linux/psci.h> @@ -84,28 +85,37 @@ static void show_psci_version(void) PSCI_VERSION_MAJOR(res.a0), PSCI_VERSION_MINOR(res.a0)); }
+/* We support booting U-Boot with an internal DT when running as a first-stage bootloader + * or for supporting quirky devices where it's easier to leave the downstream DT in place + * to improve ABL compatibility. Otherwise, we use the DT provided by ABL. + */ void *board_fdt_blob_setup(int *err) { - phys_addr_t fdt; - /* Return DTB pointer passed by ABL */ + struct fdt_header *fdt; + bool internal_valid, external_valid; + *err = 0; - fdt = get_prev_bl_fdt_addr(); + fdt = (struct fdt_header *)get_prev_bl_fdt_addr(); + external_valid = fdt && !fdt_check_header(fdt); + internal_valid = !fdt_check_header(gd->fdt_blob);
/* - * If we bail then the board will simply not boot, instead let's - * try and use the FDT built into U-Boot if there is one... - * This avoids having a hard dependency on the previous stage bootloader + * 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) + panic("Internal FDT is invalid and no external FDT was provided! (fdt=%#llx)\n", + (phys_addr_t)fdt);
- if (IS_ENABLED(CONFIG_OF_SEPARATE) && (!fdt || fdt != ALIGN(fdt, SZ_4K) || - fdt_check_header((void *)fdt))) { - debug("%s: Using built in FDT, bootloader gave us %#llx\n", __func__, fdt); + if (internal_valid) { + debug("Using built in FDT\n"); return (void *)gd->fdt_blob; + } else { + debug("Using external FDT\n"); + return (void *)fdt; } - - return (void *)fdt; }
void reset_cpu(void) {
The generic memory parsing code in U-Boot lacks a few things that we need on Qualcomm:
1. It sets gd->ram_size and gd->ram_base to represent a single memory block. 2. setup_dest_addr() later relocates U-Boot to ram_base + ram_size, the end of that first memory block.
This results in all memory beyond U-Boot being unusable in Linux when booting with EFI.
Since the ranges in the memory node may be out of order, the only way for us to correctly determine the relocation address for U-Boot is to parse all memory regions and find the highest valid address.
We can't use fdtdec_setup_memory_banksize() since it stores the result in gd->bd which is not yet allocated.
Hence, this commit, which implements an optimised parser to read the memory blocks and store them in the .data section where they will survive relocation.
We set ram_base and ram_size to describe the entire address space of memory, with the assumption that the last memory region is big enough for U-Boot, its DTB, and heap. On all boards tested so far this seems to be a reasonable assumption.
As a nice side effect, our fdt parsing also winds up being faster since we avoid the overhead of checking address/size-cells or populating struct resource. We can safely make these optimisations since we only support ARM64, and trust the reg property to be populated correctly.
After relocation, we then populate gd->bd->bi_dram with the data we parsed earlier.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org --- arch/arm/mach-snapdragon/board.c | 93 ++++++++++++++++++++++++++++++++++------ 1 file changed, 79 insertions(+), 14 deletions(-)
diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c index 2881de28a0a3..8947cf913dff 100644 --- a/arch/arm/mach-snapdragon/board.c +++ b/arch/arm/mach-snapdragon/board.c @@ -37,11 +37,20 @@ DECLARE_GLOBAL_DATA_PTR; static struct mm_region rbx_mem_map[CONFIG_NR_DRAM_BANKS + 2] = { { 0 } };
struct mm_region *mem_map = rbx_mem_map;
+static struct { + phys_addr_t start; + phys_size_t size; +} prevbl_ddr_banks[CONFIG_NR_DRAM_BANKS] __section(".data") = { 0 }; + int dram_init(void) { - return fdtdec_setup_mem_size_base(); + /* + * gd->ram_base / ram_size have been setup already + * in qcom_parse_memory(). + */ + return 0; }
static int ddr_bank_cmp(const void *v1, const void *v2) { @@ -57,25 +66,73 @@ static int ddr_bank_cmp(const void *v1, const void *v2)
return (res1->start >> 24) - (res2->start >> 24); }
+/* This has to be done post-relocation since gd->bd isn't preserved */ +static void qcom_configure_bi_dram(void) +{ + int i; + + for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) { + gd->bd->bi_dram[i].start = prevbl_ddr_banks[i].start; + gd->bd->bi_dram[i].size = prevbl_ddr_banks[i].size; + } +} + int dram_init_banksize(void) { - int ret; - - ret = fdtdec_setup_memory_banksize(); - if (ret < 0) - return ret; - - if (CONFIG_NR_DRAM_BANKS < 2) - return 0; - - /* Sort our RAM banks -_- */ - qsort(gd->bd->bi_dram, CONFIG_NR_DRAM_BANKS, sizeof(gd->bd->bi_dram[0]), ddr_bank_cmp); + qcom_configure_bi_dram();
return 0; }
+static void qcom_parse_memory(void) +{ + ofnode node; + const fdt64_t *memory; + int memsize; + phys_addr_t ram_end = 0; + int i, j, banks; + + node = ofnode_path("/memory"); + if (!ofnode_valid(node)) { + log_err("No memory node found in device tree!\n"); + return; + } + memory = ofnode_read_prop(node, "reg", &memsize); + if (!memory) { + log_err("No memory configuration was provided by the previous bootloader!\n"); + return; + } + + banks = min(memsize / (2 * sizeof(u64)), (ulong)CONFIG_NR_DRAM_BANKS); + + if (memsize / sizeof(u64) > CONFIG_NR_DRAM_BANKS * 2) + log_err("Provided more than the max of %d memory banks\n", CONFIG_NR_DRAM_BANKS); + + if (banks > CONFIG_NR_DRAM_BANKS) + log_err("Provided more memory banks than we can handle\n"); + + for (i = 0, j = 0; i < banks * 2; i += 2, j++) { + prevbl_ddr_banks[j].start = get_unaligned_be64(&memory[i]); + prevbl_ddr_banks[j].size = get_unaligned_be64(&memory[i + 1]); + /* SM8650 boards sometimes have empty regions! */ + if (!prevbl_ddr_banks[j].size) { + j--; + continue; + } + ram_end = max(ram_end, prevbl_ddr_banks[j].start + prevbl_ddr_banks[j].size); + } + + /* Sort our RAM banks -_- */ + qsort(prevbl_ddr_banks, banks, sizeof(prevbl_ddr_banks[0]), ddr_bank_cmp); + + gd->ram_base = prevbl_ddr_banks[0].start; + gd->ram_size = ram_end - gd->ram_base; + debug("ram_base = %#011lx, ram_size = %#011llx, ram_end = %#011llx\n", + gd->ram_base, gd->ram_size, ram_end); +} + static void show_psci_version(void) { struct arm_smccc_res res;
@@ -109,13 +166,21 @@ void *board_fdt_blob_setup(int *err) (phys_addr_t)fdt);
if (internal_valid) { debug("Using built in FDT\n"); - return (void *)gd->fdt_blob; } else { debug("Using external FDT\n"); - return (void *)fdt; + /* So we can use it before returning */ + gd->fdt_blob = fdt; } + + /* + * Parse the /memory node while we're here, + * this makes it easy to do other things early. + */ + qcom_parse_memory(); + + return (void *)gd->fdt_blob; }
void reset_cpu(void) {
From: Neil Armstrong neil.armstrong@linaro.org
With 14+ entries in the memory map, we need quite a bit more space for the page tables.
Signed-off-by: Neil Armstrong neil.armstrong@linaro.org --- arch/arm/mach-snapdragon/board.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c index 8947cf913dff..0e8234a62b28 100644 --- a/arch/arm/mach-snapdragon/board.c +++ b/arch/arm/mach-snapdragon/board.c @@ -416,9 +416,9 @@ static void build_mem_map(void) }
u64 get_page_table_size(void) { - return SZ_64K; + return SZ_1M; }
static int fdt_cmp_res(const void *v1, const void *v2) {
On 09/08/2024 01:59, Caleb Connolly wrote:
From: Neil Armstrong neil.armstrong@linaro.org
With 14+ entries in the memory map, we need quite a bit more space for the page tables.
Signed-off-by: Neil Armstrong neil.armstrong@linaro.org
Forgot my SoB
arch/arm/mach-snapdragon/board.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c index 8947cf913dff..0e8234a62b28 100644 --- a/arch/arm/mach-snapdragon/board.c +++ b/arch/arm/mach-snapdragon/board.c @@ -416,9 +416,9 @@ static void build_mem_map(void) }
u64 get_page_table_size(void) {
- return SZ_64K;
return SZ_1M; }
static int fdt_cmp_res(const void *v1, const void *v2) {
In the typical case where we chainload from ABL, the serial number is available in the DT bootargs. Read it out and set the serial# environment variable so that it can be used by fastboot.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org --- arch/arm/mach-snapdragon/board.c | 62 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+)
diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c index 0e8234a62b28..36335627f4ff 100644 --- a/arch/arm/mach-snapdragon/board.c +++ b/arch/arm/mach-snapdragon/board.c @@ -243,8 +243,68 @@ int board_init(void) qcom_board_init(); return 0; }
+/** + * out_len includes the trailing null space + */ +static int get_cmdline_option(const char *cmdline, const char *key, char *out, int out_len) +{ + const char *p, *p_end; + int len; + + p = strstr(cmdline, key); + if (!p) + return -ENOENT; + + p += strlen(key); + p_end = strstr(p, " "); + if (!p_end) + return -ENOENT; + + len = p_end - p; + if (len > out_len) + len = out_len; + + strncpy(out, p, len); + out[len] = '\0'; + + return 0; +} + +/* The bootargs are populated by the previous stage bootloader */ +static const char *get_cmdline(void) +{ + ofnode node; + static const char *cmdline = NULL; + + if (cmdline) + return cmdline; + + node = ofnode_path("/chosen"); + if (!ofnode_valid(node)) + return NULL; + + cmdline = ofnode_read_string(node, "bootargs"); + + return cmdline; +} + +void qcom_set_serialno(void) +{ + const char *cmdline = get_cmdline(); + char serial[32]; + + if (!cmdline) { + log_debug("Failed to get bootargs\n"); + return; + } + + get_cmdline_option(cmdline, "androidboot.serialno=", serial, sizeof(serial)); + if (serial[0] != '\0') + env_set("serial#", serial); +} + /* Sets up the "board", and "soc" environment variables as well as constructing the devicetree * path, with a few quirks to handle non-standard dtb filenames. This is not meant to be a * comprehensive solution to automatically picking the DTB, but aims to be correct for the * majority case. For most devices it should be possible to make this algorithm work by @@ -341,8 +401,10 @@ static void configure_env(void) /* Now build the full path name */ snprintf(dt_path, sizeof(dt_path), "qcom/%s-%s.dtb", env_get("soc"), env_get("board")); env_set("fdtfile", dt_path); + + qcom_set_serialno(); }
void __weak qcom_late_init(void) {
We don't know at build time where a sensible place for this is, allocate it at runtime like the other variables.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org --- arch/arm/mach-snapdragon/board.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c index 36335627f4ff..37925c40f36f 100644 --- a/arch/arm/mach-snapdragon/board.c +++ b/arch/arm/mach-snapdragon/board.c @@ -410,8 +410,13 @@ void __weak qcom_late_init(void) { }
#define KERNEL_COMP_SIZE SZ_64M +#ifdef CONFIG_FASTBOOT_BUF_SIZE +#define FASTBOOT_BUF_SIZE CONFIG_FASTBOOT_BUF_SIZE +#else +#define FASTBOOT_BUF_SIZE 0 +#endif
#define addr_alloc(lmb, size) lmb_alloc(lmb, size, SZ_2M)
/* Stolen from arch/arm/mach-apple/board.c */ @@ -426,8 +431,10 @@ int board_late_init(void) status |= env_set_hex("kernel_addr_r", addr_alloc(&lmb, SZ_128M)); status |= env_set_hex("ramdisk_addr_r", addr_alloc(&lmb, SZ_128M)); status |= env_set_hex("kernel_comp_addr_r", addr_alloc(&lmb, KERNEL_COMP_SIZE)); status |= env_set_hex("kernel_comp_size", KERNEL_COMP_SIZE); + if (IS_ENABLED(CONFIG_FASTBOOT)) + status |= env_set_hex("fastboot_addr_r", addr_alloc(&lmb, FASTBOOT_BUF_SIZE)); status |= env_set_hex("scriptaddr", addr_alloc(&lmb, SZ_4M)); status |= env_set_hex("pxefile_addr_r", addr_alloc(&lmb, SZ_4M)); status |= env_set_hex("fdt_addr_r", addr_alloc(&lmb, SZ_2M));
Set the fdt_addr_r environment variable to a region of LMB allocated memory, and populate it by default with a copy of U-Boots FDT. This will be used for Linux if no other DT is provided.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org --- arch/arm/mach-snapdragon/board.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c index 37925c40f36f..8c0c7394698b 100644 --- a/arch/arm/mach-snapdragon/board.c +++ b/arch/arm/mach-snapdragon/board.c @@ -423,8 +423,9 @@ void __weak qcom_late_init(void) int board_late_init(void) { struct lmb lmb; u32 status = 0; + phys_addr_t fdt_addr;
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 */ @@ -435,13 +436,17 @@ int board_late_init(void) if (IS_ENABLED(CONFIG_FASTBOOT)) status |= env_set_hex("fastboot_addr_r", addr_alloc(&lmb, FASTBOOT_BUF_SIZE)); status |= env_set_hex("scriptaddr", addr_alloc(&lmb, SZ_4M)); status |= env_set_hex("pxefile_addr_r", addr_alloc(&lmb, SZ_4M)); - status |= env_set_hex("fdt_addr_r", addr_alloc(&lmb, SZ_2M)); + fdt_addr = addr_alloc(&lmb, SZ_2M); + status |= env_set_hex("fdt_addr_r", fdt_addr);
if (status) log_warning("%s: Failed to set run time variables\n", __func__);
+ /* By default copy U-Boots FDT, it will be used as a fallback */ + memcpy((void *)fdt_addr, (void *)gd->fdt_blob, gd->fdt_size); + configure_env(); qcom_late_init();
return 0;
This variable is used by default in some commands, set it to the same as kernel_addr_r.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org --- arch/arm/mach-snapdragon/board.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c index 8c0c7394698b..43bff2c5e5f8 100644 --- a/arch/arm/mach-snapdragon/board.c +++ b/arch/arm/mach-snapdragon/board.c @@ -423,29 +423,31 @@ void __weak qcom_late_init(void) int board_late_init(void) { struct lmb lmb; u32 status = 0; - phys_addr_t fdt_addr; + phys_addr_t addr;
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_128M)); + addr = addr_alloc(&lmb, SZ_128M); + status |= env_set_hex("kernel_addr_r", addr); + status |= env_set_hex("loadaddr", addr); status |= env_set_hex("ramdisk_addr_r", addr_alloc(&lmb, SZ_128M)); status |= env_set_hex("kernel_comp_addr_r", addr_alloc(&lmb, KERNEL_COMP_SIZE)); status |= env_set_hex("kernel_comp_size", KERNEL_COMP_SIZE); if (IS_ENABLED(CONFIG_FASTBOOT)) status |= env_set_hex("fastboot_addr_r", addr_alloc(&lmb, FASTBOOT_BUF_SIZE)); status |= env_set_hex("scriptaddr", addr_alloc(&lmb, SZ_4M)); status |= env_set_hex("pxefile_addr_r", addr_alloc(&lmb, SZ_4M)); - fdt_addr = addr_alloc(&lmb, SZ_2M); - status |= env_set_hex("fdt_addr_r", fdt_addr); + addr = addr_alloc(&lmb, SZ_2M); + status |= env_set_hex("fdt_addr_r", addr);
if (status) log_warning("%s: Failed to set run time variables\n", __func__);
/* By default copy U-Boots FDT, it will be used as a fallback */ - memcpy((void *)fdt_addr, (void *)gd->fdt_blob, gd->fdt_size); + memcpy((void *)addr, (void *)gd->fdt_blob, gd->fdt_size);
configure_env(); qcom_late_init();
In some cases we might want to map some memory region after enabling caches. Introduce a new helper for this.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org --- arch/arm/cpu/armv8/cache_v8.c | 25 +++++++++++++++++++++++++ arch/arm/include/asm/system.h | 10 ++++++++++ 2 files changed, 35 insertions(+)
diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c index c3f8dac648ba..631d9efa5e10 100644 --- a/arch/arm/cpu/armv8/cache_v8.c +++ b/arch/arm/cpu/armv8/cache_v8.c @@ -338,8 +338,33 @@ static void map_range(u64 virt, u64 phys, u64 size, int level, size -= next_size; } }
+void mmu_map_region(phys_addr_t addr, u64 size, bool emergency) +{ + u64 va_bits; + int level = 0; + u64 attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) | PTE_BLOCK_INNER_SHARE; + + attrs |= PTE_TYPE_BLOCK | PTE_BLOCK_AF; + + get_tcr(NULL, &va_bits); + if (va_bits < 39) + level = 1; + + if (emergency) + map_range(addr, addr, size, level, + (u64 *)gd->arch.tlb_emerg, attrs); + + /* Switch pagetables while we update the primary one */ + __asm_switch_ttbr(gd->arch.tlb_emerg); + + map_range(addr, addr, size, level, + (u64 *)gd->arch.tlb_addr, attrs); + + __asm_switch_ttbr(gd->arch.tlb_addr); +} + static void add_map(struct mm_region *map) { u64 attrs = map->attrs | PTE_TYPE_BLOCK | PTE_BLOCK_AF; u64 va_bits; diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h index 7e30cac32a09..2237d7d00668 100644 --- a/arch/arm/include/asm/system.h +++ b/arch/arm/include/asm/system.h @@ -276,8 +276,18 @@ void wait_for_wakeup(void); void protect_secure_region(void); void smp_kick_all_cpus(void);
void flush_l3_cache(void); + +/** + * mmu_map_region() - map a region of previously unmapped memory. + * Will be mapped MT_NORMAL & PTE_BLOCK_INNER_SHARE. + * + * @start: Start address of the region + * @size: Size of the region + * @emerg: Also map the region in the emergency table + */ +void mmu_map_region(phys_addr_t start, u64 size, bool emerg); void mmu_change_region_attr(phys_addr_t start, size_t size, u64 attrs);
/* * smc_call() - issue a secure monitor call
memcmp() can cause aborts on some platforms and generally seems to be the wrong approach here. Use strncmp() instead which is more correct.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org --- drivers/soc/qcom/cmd-db.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c index 08736ea936ae..c7c5230983d5 100644 --- a/drivers/soc/qcom/cmd-db.c +++ b/drivers/soc/qcom/cmd-db.c @@ -140,9 +140,9 @@ static int cmd_db_get_header(const char *id, const struct entry_header **eh, break;
ent = rsc_to_entry_header(rsc_hdr); for (j = 0; j < le16_to_cpu(rsc_hdr->cnt); j++, ent++) { - if (memcmp(ent->id, query, sizeof(ent->id)) == 0) { + if (strncmp(ent->id, query, sizeof(ent->id)) == 0) { if (eh) *eh = ent; if (rh) *rh = rsc_hdr;
On at least SM8650 this region might not be included in the memory map. Use the new mmu_map_region() helper to map it during bind().
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org --- drivers/soc/qcom/cmd-db.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c index c7c5230983d5..67be18e89f4d 100644 --- a/drivers/soc/qcom/cmd-db.c +++ b/drivers/soc/qcom/cmd-db.c @@ -5,8 +5,9 @@ */
#define pr_fmt(fmt) "cmd-db: " fmt
+#include <asm/system.h> #include <dm.h> #include <dm/ofnode.h> #include <dm/device_compat.h> #include <linux/kernel.h> @@ -181,11 +182,12 @@ u32 cmd_db_read_addr(const char *id) return ret < 0 ? 0 : le32_to_cpu(ent->addr); } EXPORT_SYMBOL_GPL(cmd_db_read_addr);
-int cmd_db_bind(struct udevice *dev) +static int cmd_db_bind(struct udevice *dev) { void __iomem *base; + fdt_size_t size; ofnode node;
if (cmd_db_header) return 0; @@ -193,14 +195,17 @@ int cmd_db_bind(struct udevice *dev) node = dev_ofnode(dev);
debug("%s(%s)\n", __func__, ofnode_get_name(node));
- base = (void __iomem *)ofnode_get_addr(node); + base = (void __iomem *)ofnode_get_addr_size(node, "reg", &size); if ((fdt_addr_t)base == FDT_ADDR_T_NONE) { log_err("%s: Failed to read base address\n", __func__); return -ENOENT; }
+ /* On SM8550/SM8650 and newer SoCs cmd-db might not be mapped */ + mmu_map_region((phys_addr_t)base, (phys_size_t)size, false); + cmd_db_header = base; if (!cmd_db_magic_matches(cmd_db_header)) { log_err("%s: Invalid Command DB Magic\n", __func__); return -EINVAL;
Some newer boards end up with a bunch of holes in the memory map due to how Qualcomm's hypervisor and ABL work. The end result is 14+ memory regions.
Bump CONFIG_NR_DRAM_BANKS to 24 so we can handle these and any future expansion easily.
Yes, this is ridiculous, but there is no other way.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org --- configs/qcom_defconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/configs/qcom_defconfig b/configs/qcom_defconfig index 8852e83a52b6..24b71ba7be29 100644 --- a/configs/qcom_defconfig +++ b/configs/qcom_defconfig @@ -2,8 +2,9 @@ CONFIG_ARM=y CONFIG_SKIP_LOWLEVEL_INIT=y CONFIG_POSITION_INDEPENDENT=y CONFIG_SYS_INIT_SP_BSS_OFFSET=1572864 CONFIG_ARCH_SNAPDRAGON=y +CONFIG_NR_DRAM_BANKS=24 CONFIG_DEFAULT_DEVICE_TREE="qcom/sdm845-db845c" CONFIG_SYS_LOAD_ADDR=0xA0000000 CONFIG_BUTTON_CMD=y CONFIG_FIT=y
On 09/08/2024 01:59, Caleb Connolly wrote:
Supporting the newer SM8550 and SM8650 SoCs unfortunately requires a bump in complexity for us. Qualcomm changed a lot about how the memory map is handed over to the "kernel", adding many holes, not mapping certain regions, and adding regions with 0 size. The SM8650 HDK has a whopping 14 memory regions, some as small as 44k.
Supporting this properly has proven to be a bit of a headache, but I think this implementation is "pretty good".
In addition, we set a valid fallback fdt_addr_r in the U-Boot environment, allocate a buffer for fastboot, and set the loadaddr variable too.
board_fdt_blob_setup() is refactored for readability and potential future expansion (e.g. if supporting multi-dtb FIT becomes desirable).
Finally, a function is proposed to allow for mapping new memory regions at runtime, and the cmd-db driver makes use of it to map itself, since SM8650 boards don't seem to include it in their memory map.
Tested on SM8650 HDK, SDM845 OnePlus 6, SM6115 RB2, SM8250 RB5.
Caleb Connolly (10): mach-snapdragon: refactor board_fdt_blob_setup() mach-snapdragon: parse memory ourselves mach-snapdragon: set serial number mach-snapdragon: allocate fastboot buffer dynamically mach-snapdragon: populate fallback FDT mach-snapdragon: set loadaddr armv8: mmu: add a way to map additional regions soc: qcom: cmd-db: use strncmp() instead of memcmp() soc: qcom: cmd-db: map cmd-db region qcom_defconfig: bump CONFIG_NR_DRAM_BANKS
Neil Armstrong (1): mach-snapdragon: use 1MiB for get_page_table_size()
arch/arm/cpu/armv8/cache_v8.c | 25 +++++ arch/arm/include/asm/system.h | 10 ++ arch/arm/mach-snapdragon/board.c | 203 ++++++++++++++++++++++++++++++++++----- configs/qcom_defconfig | 1 + drivers/soc/qcom/cmd-db.c | 11 ++- 5 files changed, 221 insertions(+), 29 deletions(-)
change-id: 20240809-b4-snapdragon-improvements-fd6d714a7fbd base-commit: a2ce853383b18a2cf920268ee341f2585a11adef
// Caleb (they/them)
Thanks for posting those :-)
Please add my: Reviewed-by: Neil Armstrong neil.armstrong@linaro.org
I'll rebase my tree and test it ASAP !
Neil
On Fri, 09 Aug 2024 01:59:23 +0200, Caleb Connolly wrote:
Supporting the newer SM8550 and SM8650 SoCs unfortunately requires a bump in complexity for us. Qualcomm changed a lot about how the memory map is handed over to the "kernel", adding many holes, not mapping certain regions, and adding regions with 0 size. The SM8650 HDK has a whopping 14 memory regions, some as small as 44k.
Supporting this properly has proven to be a bit of a headache, but I think this implementation is "pretty good".
[...]
Applied, thanks!
[01/11] mach-snapdragon: refactor board_fdt_blob_setup() https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/9e8c8769... [02/11] mach-snapdragon: parse memory ourselves https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/07f03690... [03/11] mach-snapdragon: use 1MiB for get_page_table_size() https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/4673e5c9... [04/11] mach-snapdragon: set serial number https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/c4c7bc91... [05/11] mach-snapdragon: allocate fastboot buffer dynamically https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/5f6c2202... [06/11] mach-snapdragon: populate fallback FDT https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/8ab7b82c... [07/11] mach-snapdragon: set loadaddr https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/1465eb40... [08/11] armv8: mmu: add a way to map additional regions https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/fc417bb2... [09/11] soc: qcom: cmd-db: use strncmp() instead of memcmp() https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/d83fdebf... [10/11] soc: qcom: cmd-db: map cmd-db region https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/8b890a0f... [11/11] qcom_defconfig: bump CONFIG_NR_DRAM_BANKS https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/10d5f66d...
Best regards,
participants (2)
-
Caleb Connolly -
Neil Armstrong