[U-Boot] [PATCH] fdt: fix get_next_memory_node()
The get_next_memory_node() always sets mem to -1 , which is incorrect, because then every iteration of memory bank parsing will start from the first memory bank instead of the previous one.
On systems with 1 memory bank defined in DT and CONFIG_NR_DRAM_BANKS=4 , like ie. r8a77965-salvator-x , this will result in U-Boot incorrectly reporting four identical memory banks with the same memory configuration.
Fix this by setting mem to startoffset value, which restores the behavior before the fixed patch was applied.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Jens Wiklander jens.wiklander@linaro.org Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com Fixes: 452bc121027d ("fdt: fix fdtdec_setup_memory_banksize()") --- lib/fdtdec.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/lib/fdtdec.c b/lib/fdtdec.c index a208589c48..b2ec12e9b5 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1182,10 +1182,8 @@ int fdtdec_setup_mem_size_base(void)
#if defined(CONFIG_NR_DRAM_BANKS)
-static int get_next_memory_node(const void *blob, int startoffset) +static int get_next_memory_node(const void *blob, int mem) { - int mem = -1; - do { mem = fdt_node_offset_by_prop_value(gd->fdt_blob, mem, "device_type", "memory", 7);
On 9.9.2018 16:30, Marek Vasut wrote:
The get_next_memory_node() always sets mem to -1 , which is incorrect, because then every iteration of memory bank parsing will start from the first memory bank instead of the previous one.
On systems with 1 memory bank defined in DT and CONFIG_NR_DRAM_BANKS=4 , like ie. r8a77965-salvator-x , this will result in U-Boot incorrectly reporting four identical memory banks with the same memory configuration.
Fix this by setting mem to startoffset value, which restores the behavior before the fixed patch was applied.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Jens Wiklander jens.wiklander@linaro.org Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com Fixes: 452bc121027d ("fdt: fix fdtdec_setup_memory_banksize()")
Tested-by: Michal Simek michal.simek@xilinx.com [on ZynqMP}
Before arch_number = 0x00000000 boot_params = 0x00000000 DRAM bank = 0x00000000 -> start = 0x00000000 -> size = 0x10000000 DRAM bank = 0x00000001 -> start = 0x10000000 -> size = 0x40000000 DRAM bank = 0x00000002 -> start = 0x50000000 -> size = 0x30000000 DRAM bank = 0x00000003 -> start = 0x00000000 -> size = 0x10000000
after arch_number = 0x00000000 boot_params = 0x00000000 DRAM bank = 0x00000000 -> start = 0x00000000 -> size = 0x10000000 DRAM bank = 0x00000001 -> start = 0x10000000 -> size = 0x40000000 DRAM bank = 0x00000002 -> start = 0x50000000 -> size = 0x30000000
Thanks, Michal
On 09/10/2018 01:23 PM, Michal Simek wrote:
On 9.9.2018 16:30, Marek Vasut wrote:
The get_next_memory_node() always sets mem to -1 , which is incorrect, because then every iteration of memory bank parsing will start from the first memory bank instead of the previous one.
On systems with 1 memory bank defined in DT and CONFIG_NR_DRAM_BANKS=4 , like ie. r8a77965-salvator-x , this will result in U-Boot incorrectly reporting four identical memory banks with the same memory configuration.
Fix this by setting mem to startoffset value, which restores the behavior before the fixed patch was applied.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Jens Wiklander jens.wiklander@linaro.org Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com Fixes: 452bc121027d ("fdt: fix fdtdec_setup_memory_banksize()")
Tested-by: Michal Simek michal.simek@xilinx.com [on ZynqMP}
Before arch_number = 0x00000000 boot_params = 0x00000000 DRAM bank = 0x00000000 -> start = 0x00000000 -> size = 0x10000000 DRAM bank = 0x00000001 -> start = 0x10000000 -> size = 0x40000000 DRAM bank = 0x00000002 -> start = 0x50000000 -> size = 0x30000000 DRAM bank = 0x00000003 -> start = 0x00000000 -> size = 0x10000000
after arch_number = 0x00000000 boot_params = 0x00000000 DRAM bank = 0x00000000 -> start = 0x00000000 -> size = 0x10000000 DRAM bank = 0x00000001 -> start = 0x10000000 -> size = 0x40000000 DRAM bank = 0x00000002 -> start = 0x50000000 -> size = 0x30000000
That's on a system with three banks, so it was broken before this patch?
On 10.9.2018 13:27, Marek Vasut wrote:
On 09/10/2018 01:23 PM, Michal Simek wrote:
On 9.9.2018 16:30, Marek Vasut wrote:
The get_next_memory_node() always sets mem to -1 , which is incorrect, because then every iteration of memory bank parsing will start from the first memory bank instead of the previous one.
On systems with 1 memory bank defined in DT and CONFIG_NR_DRAM_BANKS=4 , like ie. r8a77965-salvator-x , this will result in U-Boot incorrectly reporting four identical memory banks with the same memory configuration.
Fix this by setting mem to startoffset value, which restores the behavior before the fixed patch was applied.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Jens Wiklander jens.wiklander@linaro.org Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com Fixes: 452bc121027d ("fdt: fix fdtdec_setup_memory_banksize()")
Tested-by: Michal Simek michal.simek@xilinx.com [on ZynqMP}
Before arch_number = 0x00000000 boot_params = 0x00000000 DRAM bank = 0x00000000 -> start = 0x00000000 -> size = 0x10000000 DRAM bank = 0x00000001 -> start = 0x10000000 -> size = 0x40000000 DRAM bank = 0x00000002 -> start = 0x50000000 -> size = 0x30000000 DRAM bank = 0x00000003 -> start = 0x00000000 -> size = 0x10000000
after arch_number = 0x00000000 boot_params = 0x00000000 DRAM bank = 0x00000000 -> start = 0x00000000 -> size = 0x10000000 DRAM bank = 0x00000001 -> start = 0x10000000 -> size = 0x40000000 DRAM bank = 0x00000002 -> start = 0x50000000 -> size = 0x30000000
That's on a system with three banks, so it was broken before this patch?
yes that's on 3 bank system and as is visible above there are 4 banks listed and also all of them were calculated together. That's why system with 2GB of memory had surprisingly 2.3GiB.
Before with my 3 bank setup DRAM: 2.3 GiB After DRAM: 2 GiB
With one bank this is even more tricky because DRAM: 8 GiB is reported but relocation works properly.
Definitely this is regression and it should go to this release.
Thanks, Michal
On Sun, Sep 09, 2018 at 04:30:11PM +0200, Marek Vasut wrote:
The get_next_memory_node() always sets mem to -1 , which is incorrect, because then every iteration of memory bank parsing will start from the first memory bank instead of the previous one.
On systems with 1 memory bank defined in DT and CONFIG_NR_DRAM_BANKS=4 , like ie. r8a77965-salvator-x , this will result in U-Boot incorrectly reporting four identical memory banks with the same memory configuration.
Fix this by setting mem to startoffset value, which restores the behavior before the fixed patch was applied.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Jens Wiklander jens.wiklander@linaro.org Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com Fixes: 452bc121027d ("fdt: fix fdtdec_setup_memory_banksize()") Tested-by: Michal Simek michal.simek@xilinx.com [on ZynqMP}
Applied to u-boot/master, thanks!
participants (3)
-
Marek Vasut -
Michal Simek -
Tom Rini