From: Jiaxun Yang <[email protected]> Current code only tries to fetch the first memory node found in fdt tree and determine memory banks from multiple reg properties.
Specification allows multiple memory nodes in devicetree, rework fdtdec_setup_mem_size_base_lowest and fdtdec_setup_memory_banksize to iterate over all memory nodes. Signed-off-by: Jiaxun Yang <[email protected]> Signed-off-by: Yao Zi <[email protected]> --- Hi Simon, It's hard to add unit tests for these two functions since they operate on global states (gd), so I don't do so. Is it acceptable, or is there a better way to do this? Changed from v1 - Reword commit message to reflect the fact that dtspec allows multiple memory nodes - Replace sort_memory_banks() with qsort() - Do not use CONFIG_VAL when referencing NR_DRAM_BANKS to avoid breaking SPL build - print gd->ram_size with %pap instead of %lx, drop unnecessary cast for gd->ram_base in fdtdec_setup_size_base_lowest() - Style improvements include/asm-generic/global_data.h | 2 +- lib/fdtdec.c | 129 +++++++++++++++++------------- 2 files changed, 76 insertions(+), 55 deletions(-) diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index ad7ebb1bbc95..a4df897543a8 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -460,7 +460,7 @@ struct global_data { /** * @dram: array describing DRAM banks (start address and size for each bank) */ - struct { /* RAM configuration */ + struct dram_bank { /* RAM configuration */ phys_addr_t start; phys_size_t size; } dram[CONFIG_NR_DRAM_BANKS]; diff --git a/lib/fdtdec.c b/lib/fdtdec.c index b91e067106dd..33fd7359f2eb 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -36,6 +36,7 @@ #include <linux/lzo.h> #include <linux/ioport.h> #include <asm/global_data.h> +#include <sort.h> DECLARE_GLOBAL_DATA_PTR; @@ -1115,90 +1116,110 @@ ofnode fdtdec_get_next_memory_node(ofnode mem) return get_next_memory_node(mem); } +static int cmp_memory_bank(const void *a, const void *b) +{ + const struct dram_bank *a1 = a, *b1 = b; + + return a1->start > b1->start ? 1 : + a1->start < b1->start ? -1 : 0; +} + int fdtdec_setup_memory_banksize(void) { - int bank, ret, reg = 0; - struct resource res; + int bank = 0; ofnode mem = ofnode_null(); - mem = get_next_memory_node(mem); - if (!ofnode_valid(mem)) { - debug("%s: Missing /memory node\n", __func__); - return -EINVAL; - } + while (true) { + struct resource res; + int reg = 0; - for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) { - ret = ofnode_read_resource(mem, reg++, &res); - if (ret < 0) { - reg = 0; - mem = get_next_memory_node(mem); - if (!ofnode_valid(mem)) - break; + mem = get_next_memory_node(mem); + if (!ofnode_valid(mem)) + break; + + while (true) { + int ret = ofnode_read_resource(mem, reg, &res); - ret = ofnode_read_resource(mem, reg++, &res); if (ret < 0) break; - } - if (ret != 0) - return -EINVAL; + if (bank >= CONFIG_NR_DRAM_BANKS) + goto too_may_memory_banks; - gd->dram[bank].start = (phys_addr_t)res.start; - gd->dram[bank].size = - (phys_size_t)(res.end - res.start + 1); + gd->dram[bank].start = res.start; + gd->dram[bank].size = + (phys_size_t)(res.end - res.start + 1); - debug("%s: DRAM Bank #%d: start = %pap, size = %pap\n", - __func__, bank, - &gd->dram[bank].start, - &gd->dram[bank].size); + log_debug("%s: DRAM Bank #%d %s.%d: start = 0x%llx, size = 0x%llx\n", + __func__, bank, ofnode_get_name(mem), reg, + (unsigned long long)gd->dram[bank].start, + (unsigned long long)gd->dram[bank].size); + reg++; + bank++; + } + } + + if (!bank) { + log_warning("%s: Missing /memory node\n", __func__); + return -EINVAL; } + qsort(gd->dram, bank, sizeof(gd->dram[0]), cmp_memory_bank); + return 0; + +too_may_memory_banks: + log_warning("%s: Too many memory banks\n", __func__); + return -EINVAL; } int fdtdec_setup_mem_size_base_lowest(void) { - int bank, ret, reg = 0; - struct resource res; - unsigned long base; - phys_size_t size; + int bank = 0; ofnode mem = ofnode_null(); + __maybe_unused const char *final_name; + __maybe_unused int final_reg; - gd->ram_base = (unsigned long)~0; + gd->ram_base = ULONG_MAX; - mem = get_next_memory_node(mem); - if (!ofnode_valid(mem)) { - debug("%s: Missing /memory node\n", __func__); - return -EINVAL; - } + while (true) { + struct resource res; + phys_size_t base, size; + int reg = 0; - for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) { - ret = ofnode_read_resource(mem, reg++, &res); - if (ret < 0) { - reg = 0; - mem = get_next_memory_node(mem); - if (!ofnode_valid(mem)) - break; + mem = get_next_memory_node(mem); + if (!ofnode_valid(mem)) + break; + + while (true) { + int ret = ofnode_read_resource(mem, reg, &res); - ret = ofnode_read_resource(mem, reg++, &res); if (ret < 0) break; - } - if (ret != 0) - return -EINVAL; - - base = (unsigned long)res.start; - size = (phys_size_t)(res.end - res.start + 1); + base = res.start; + size = res.end - res.start + 1; + if (gd->ram_base > base && size) { + gd->ram_base = base; + gd->ram_size = size; + final_name = ofnode_get_name(mem); + final_reg = reg; + } - if (gd->ram_base > base && size) { - gd->ram_base = base; - gd->ram_size = size; - debug("%s: Initial DRAM base %lx size %lx\n", - __func__, base, (unsigned long)size); + reg++; + bank++; } } + if (!bank) { + log_warning("%s: Missing /memory node\n", __func__); + return -EINVAL; + } + + log_debug("%s: Initial DRAM %s.%d: base %lx size %pap\n", + __func__, final_name, final_reg, + gd->ram_base, &gd->ram_size); + return 0; } -- 2.54.0

