On 08.10.20 09:08, Stefan Roese wrote: > On 24.09.20 01:22, 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 (-arm64), which was crashing due to decoding >> a wrong flash base 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> > > Applied to u-boot-cfi-flash/master > > Thanks, > Stefan > >> --- >> Changes v1 .. v2: >> - Use live tree compatible function >> - Drop unneeded size variable >> >> drivers/mtd/cfi_flash.c | 24 ++++++------------------ >> 1 file changed, 6 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c >> index b7289ba5394..9e3a652f445 100644 >> --- a/drivers/mtd/cfi_flash.c >> +++ b/drivers/mtd/cfi_flash.c >> @@ -2468,29 +2468,17 @@ 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; >> + 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 = dev_read_addr_index(dev, idx);
Why don't you additionally read the size here to populate flash_info[].size? fdt_size_t size; addr = dev_read_addr_size_index(dev, idx, &size); Best regards Heinrich >> + 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; >> > > > Viele Grüße, > Stefan >