[PATCH v2] cfi_flash: Fix devicetree address determination

[Heinrich Schuchardt]

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.przywara at 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
>