[PATCH 1/1] mtd: cfi_flash: read device tree correctly

[Heinrich Schuchardt]

On 24.07.20 11:14, Rick Chen wrote:
> Hi Heinrich
>
>> From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Heinrich Schuchardt
>> Sent: Tuesday, July 21, 2020 10:51 AM
>> To: Stefan Roese
>> Cc: Simon Glass; u-boot at 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.glpk at 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 <rick at 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