Hi Heinrich,
(added Simon to Cc)
On 24.07.20 18:34, Heinrich Schuchardt wrote:
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.
Thanks for looking into this. But I wonder, if it makes more sense to
change the OF_LIVE implementation so that it matches the "non-OF_LIVE"
version? We should strive to have both implementations achieving the
same results I think.
Or am I missing something?
Thanks,
Stefan
