Hi Andre, (added Simon)
On 18.09.20 19:45, Andre Przywara wrote:
The cfi-flash driver uses an open-coded version of the generic algorithm to decode and translate multiple frames of a "reg" property. This starts off the wrong foot by using the address-cells and size-cells properties of *this* very node, and not of the parent. This somewhat happened to work back when we were using a wrong default size of 2, but broke about a year ago with commit 0ba41ce1b781 ("libfdt: return correct value if #size-cells property is not present"). Instead of fixing the reinvented wheel, just use the generic function that does all of this properly. This fixes U-Boot on QEMU (outside of EL1), which was crashing due to decoding a wrong start address: DRAM: 1 GiB Flash: "Synchronous Abort" handler, esr 0x96000044 elr: 00000000000211dc lr : 00000000000211b0 (reloc) elr: 000000007ff5e1dc lr : 000000007ff5e1b0 x0 : 00000000000000f0 x1 : 000000007ff5e1d8 x2 : 000000007edfbc48 x3 : 0000000000000000 x4 : 0000000000000000 x5 : 00000000000000f0 x6 : 000000007edfbc2c x7 : 0000000000000000 x8 : 000000007ffd8d70 x9 : 000000000000000c x10: 0400000000000003 x11: 0000000000000055 ^^^^^^^^^^^^^^^^ Signed-off-by: Andre Przywara <andre.przyw...@arm.com> --- drivers/mtd/cfi_flash.c | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index b7289ba5394..656ff326e17 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -2468,29 +2468,18 @@ unsigned long flash_init(void) #ifdef CONFIG_CFI_FLASH /* for driver model */ static int cfi_flash_probe(struct udevice *dev) { - const fdt32_t *cell; - int addrc, sizec; - int len, idx; + fdt_addr_t addr; + fdt_size_t size; + int idx;- addrc = dev_read_addr_cells(dev);- sizec = dev_read_size_cells(dev); - - /* decode regs; there may be multiple reg tuples. */ - cell = dev_read_prop(dev, "reg", &len); - if (!cell) - return -ENOENT; - idx = 0; - len /= sizeof(fdt32_t); - while (idx < len) { - phys_addr_t addr; - - addr = dev_translate_address(dev, cell + idx); + for (idx = 0; idx < CFI_MAX_FLASH_BANKS; idx++) { + addr = devfdt_get_addr_size_index(dev, idx, &size); + if (addr == FDT_ADDR_T_NONE) + break;flash_info[cfi_flash_num_flash_banks].dev = dev;flash_info[cfi_flash_num_flash_banks].base = addr; cfi_flash_num_flash_banks++; - - idx += addrc + sizec; } gd->bd->bi_flashstart = flash_info[0].base;
This fails on my Octeon MIPS64 platform "octeon_ebb7304". I did some debugging and found that here "of_offset" is a 64 bit value (type long) which gets truncated in dev_of_offset() to 32 bit (type int). This problem only arises when of_live_active() is set. Here, "of_offset" holds a pointer AFACT and truncating it to 32 bits breaks things. I'm wondering why this did not hit me earlier on this 64bit platform. Simon, do you have a quick idea how to solve this? Thanks, Stefan
