[PATCH] mtd: cfi: respect reg address length

[Stefan Roese]

Hi Nuno,

On 3/27/23 15:22, Nuno Sá wrote:
> flash_get_size() will get the flash size from the device itself and go
> through all erase regions to read protection status. However, the device
> mappable region (eg: devicetree reg property) might be lower than the
> device full size which means that the above cycle will result in a data
> bus exception. This change fixes it by reading the 'addr_size' during
> probe() and also use that as one possible upper limit.
> 
> Signed-off-by: Nuno Sá <nuno.sa at analog.com>
> ---
> 
> To give a bit more of background for this patch, I caught this issue
> when running u-boot on microblaze which uses xilinx axi_linear_flash IP.
> On ADI designs using that IP, the flash size was set to 32MiB resulting
> in the mentioned data bus exception as we obviously access past the
> IP size.
> 
> That said, there was not real reason for the flash truncation so we'll
> be fixing our designs so the IP will contemplate the complete flash
> size.
> 
> For the above reason, I was not really sure to mark the patch as RFC or
> not... Anyways, it still feels like a valid "protection" to have rather
> then just aborting (even if it does not really make sense to ever
> truncate the flash in devicetree).
> 
>   drivers/mtd/cfi_flash.c | 10 ++++++++--
>   include/flash.h         |  1 +
>   2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
> index f378f6fb6139..432d0d5c9ecb 100644
> --- a/drivers/mtd/cfi_flash.c
> +++ b/drivers/mtd/cfi_flash.c
> @@ -2196,7 +2196,11 @@ ulong flash_get_size(phys_addr_t base, int banknum)
>   		/* multiply the size by the number of chips */
>   		info->size *= size_ratio;
>   		max_size = cfi_flash_bank_size(banknum);
> -		if (max_size && info->size > max_size) {
> +		if (max_size)
> +			max_size = min(info->addr_size, max_size);
> +		else
> +			max_size = info->addr_size;
> +		if (info->size > max_size) {
>   			debug("[truncated from %ldMiB]", info->size >> 20);
>   			info->size = max_size;
>   		}
> @@ -2492,15 +2496,17 @@ unsigned long flash_init(void)
>   static int cfi_flash_probe(struct udevice *dev)
>   {
>   	fdt_addr_t addr;
> +	fdt_size_t size;
>   	int idx;
>   
>   	for (idx = 0; idx < CFI_MAX_FLASH_BANKS; idx++) {
> -		addr = dev_read_addr_index(dev, idx);
> +		addr = dev_read_addr_size_index(dev, idx, &size);
>   		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;
> +		flash_info[cfi_flash_num_flash_banks].addr_size = size;
>   		cfi_flash_num_flash_banks++;
>   	}
>   	gd->bd->bi_flashstart = flash_info[0].base;
> diff --git a/include/flash.h b/include/flash.h
> index 95992fa689bb..3710a2731b76 100644
> --- a/include/flash.h
> +++ b/include/flash.h
> @@ -46,6 +46,7 @@ typedef struct {
>   #ifdef CONFIG_CFI_FLASH			/* DM-specific parts */
>   	struct udevice *dev;
>   	phys_addr_t base;
> +	phys_size_t addr_size;
>   #endif
>   } flash_info_t;
>   

Running a CI world build leads to many errors. Here an example:

$ make integratorcp_cm926ejs_defconfig
$ make -sj
In file included from include/linux/bitops.h:22,
                  from include/log.h:15,
                  from include/linux/printk.h:4,
                  from include/common.h:20,
                  from drivers/mtd/cfi_flash.c:19:
drivers/mtd/cfi_flash.c: In function 'flash_get_size':
drivers/mtd/cfi_flash.c:2200:44: error: 'flash_info_t' has no member 
named 'addr_size'
  2200 |                         max_size = min(info->addr_size, max_size);
       |                                            ^~
include/linux/kernel.h:182:16: note: in definition of macro 'min'
   182 |         typeof(x) _min1 = (x);                  \
       |                ^
drivers/mtd/cfi_flash.c:2200:44: error: 'flash_info_t' has no member 
named 'addr_size'
  2200 |                         max_size = min(info->addr_size, max_size);
       |                                            ^~
include/linux/kernel.h:182:28: note: in definition of macro 'min'
   182 |         typeof(x) _min1 = (x);                  \
       |                            ^
include/linux/kernel.h:184:24: warning: comparison of distinct pointer 
types lacks a cast
   184 |         (void) (&_min1 == &_min2);              \
       |                        ^~
drivers/mtd/cfi_flash.c:2200:36: note: in expansion of macro 'min'
  2200 |                         max_size = min(info->addr_size, max_size);
       |                                    ^~~
drivers/mtd/cfi_flash.c:2202:40: error: 'flash_info_t' has no member 
named 'addr_size'
  2202 |                         max_size = info->addr_size;
       |                                        ^~
make[2]: *** [scripts/Makefile.build:257: drivers/mtd/cfi_flash.o] Error 1
make[1]: *** [scripts/Makefile.build:397: drivers/mtd] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1850: drivers] Error 2
make: *** Waiting for unfinished jobs....

Could you please take a look and make sure that v2 successfully runs
a world build?

Thanks,
Stefan