On 24.07.20 11:14, Rick Chen wrote: > Hi Heinrich > >> From: U-Boot [mailto:u-boot-boun...@lists.denx.de] On Behalf Of Heinrich >> Schuchardt >> Sent: Tuesday, July 21, 2020 10:51 AM >> To: Stefan Roese >> Cc: Simon Glass; u-boot@lists.denx.de; Heinrich Schuchardt >> Subject: [PATCH 1/1] mtd: cfi_flash: read device tree correctly >> >> dev_read_size_cells() and dev_read_addr_cells() do not walk up the device >> tree to find the number of cells. On error they return 1 and 2 respectively. >> On qemu_arm64_defconfig this leads to the incorrect detection of address of >> the second flash bank as 0x400000000000000 instead of 0x4000000. >> >> When running >> >> qemu-system-aarch64 -machine virt -bios u-boot.bin \ >> -cpu cortex-a53 -nographic \ >> -drive if=pflash,format=raw,index=1,file=envstore.img >> >> the command 'saveenv' fails with >> >> Saving Environment to Flash... Error: start and/or end address not on >> sector boundary >> Error: start and/or end address not on sector boundary >> Failed (1) >> >> due to this incorrect address. >> >> Use function fdtdec_get_addr_size_auto_noparent() to read the array of flash >> banks from the device tree. >> >> Signed-off-by: Heinrich Schuchardt <xypron.g...@gmx.de> >> --- >> drivers/mtd/cfi_flash.c | 20 ++++++++------------ >> 1 file changed, 8 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index >> b7289ba539..dfa104bcf0 100644 >> --- a/drivers/mtd/cfi_flash.c >> +++ b/drivers/mtd/cfi_flash.c >> @@ -2469,28 +2469,24 @@ unsigned long flash_init(void) static int >> cfi_flash_probe(struct udevice *dev) { >> const fdt32_t *cell; >> - int addrc, sizec; >> - int len, idx; >> - >> - addrc = dev_read_addr_cells(dev); >> - sizec = dev_read_size_cells(dev); >> + int len; >> >> /* 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) { >> + >> + for (cfi_flash_num_flash_banks = 0; ; ++cfi_flash_num_flash_banks) { >> phys_addr_t addr; >> >> - addr = dev_translate_address(dev, cell + idx); >> + addr = fdtdec_get_addr_size_auto_noparent( >> + gd->fdt_blob, dev_of_offset(dev), "reg", >> + cfi_flash_num_flash_banks, NULL, false); >> + 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; >> >> -- >> 2.27.0 >> > > This patch remind me that I have encounter flash bank detection > problem on AE350 platform a period time ago. > And have commit a patch to work around this problem as below: >
> commit cca8b1e5b20cdab7299a5ee7139e70783f73ccdf > > riscv: dts: Add #address-cells and #size-cells in nor node > > Those are required for cfi-flash driver to get correct address > information. > Also modify size description correctly. > > With this patch, there is unnecessary to re-declaration address-cells > and size-cells in nor node indeed. > > Tested-by: Rick Chen <r...@andestech.com> > > Thanks, > Rick > Dear Stefan, dear Rick, thanks for testing on different systems. The reason for the different test results is the usage of CONFIG_OF_LIVE: CONFIG_OF_LIVE=y * octeon_ebb7304_defconfig CONFIG_OF_LIVE=n * ae350_rv32_defconfig * qemu_arm64_defconfig dev_translate_address() behaves differently depending on the usage of a live tree. I will send a revised patch that only changes the behavior only for the CONFIG_OF_LIVE=n case after testing qemu_arm64_defconfig both with and without live tree. Best regards Heinrich
