Message ID | 20230511111950.824620-1-nuno.sa@analog.com |
---|---|
State | Accepted |
Commit | 43bacbe6ab85030a4dc424700f83108333e99536 |
Delegated to: | Stefan Roese |
Headers | show |
Series | [v4] mtd: cfi: respect reg address length | expand |
On 5/11/23 13:19, 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@analog.com> Applied to u-boot-cfi-flash/master Thanks, Stefan > --- > > v2: > * Fix compilation when CONFIG_CFI_FLASH is not set. Done by redefining > cfi_flash_bank_size() when CONFIG_CFI_FLASH is set (by returning dts > size). > > v3: > * Fix another compilation warning by explicitly casting to unsigned > long in cfi_flash_bank_size() > > v4: > * Made CI tests pass. Done by going back to the basic v1 patch and > making sure we have proper guards for CONFIG_CFI_FLASH and the proper > casts when needed. > * Dropped second patch. > > Alright, Stefan, I think now it's ready to go. All checks passed for me > in github [1]. Apparently, I needed to go back to version 1 (with proper > fixes) which adds the reg size check without changing any previous > functionality. I cannot say for sure but seems some platforms are > relying on cfi_flash_bank_size() and that cannot be overwritten to > return reg_size. Also dropped the second patch as (I think) it would > only be meaningful to have it if also converting flash_info_t:size which > would be a fairly bigger change... > > [1]: https://github.com/u-boot/u-boot/pull/281 > > drivers/mtd/cfi_flash.c | 10 +++++++++- > include/flash.h | 1 + > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c > index f378f6fb6139..8ade7949a68e 100644 > --- a/drivers/mtd/cfi_flash.c > +++ b/drivers/mtd/cfi_flash.c > @@ -2196,6 +2196,12 @@ 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); > +#ifdef CONFIG_CFI_FLASH > + if (max_size) > + max_size = min((unsigned long)info->addr_size, max_size); > + else > + max_size = info->addr_size; > +#endif > if (max_size && info->size > max_size) { > debug("[truncated from %ldMiB]", info->size >> 20); > info->size = max_size; > @@ -2492,15 +2498,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; > Viele Grüße, Stefan Roese
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index f378f6fb6139..8ade7949a68e 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -2196,6 +2196,12 @@ 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); +#ifdef CONFIG_CFI_FLASH + if (max_size) + max_size = min((unsigned long)info->addr_size, max_size); + else + max_size = info->addr_size; +#endif if (max_size && info->size > max_size) { debug("[truncated from %ldMiB]", info->size >> 20); info->size = max_size; @@ -2492,15 +2498,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;
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@analog.com> --- v2: * Fix compilation when CONFIG_CFI_FLASH is not set. Done by redefining cfi_flash_bank_size() when CONFIG_CFI_FLASH is set (by returning dts size). v3: * Fix another compilation warning by explicitly casting to unsigned long in cfi_flash_bank_size() v4: * Made CI tests pass. Done by going back to the basic v1 patch and making sure we have proper guards for CONFIG_CFI_FLASH and the proper casts when needed. * Dropped second patch. Alright, Stefan, I think now it's ready to go. All checks passed for me in github [1]. Apparently, I needed to go back to version 1 (with proper fixes) which adds the reg size check without changing any previous functionality. I cannot say for sure but seems some platforms are relying on cfi_flash_bank_size() and that cannot be overwritten to return reg_size. Also dropped the second patch as (I think) it would only be meaningful to have it if also converting flash_info_t:size which would be a fairly bigger change... [1]: https://github.com/u-boot/u-boot/pull/281 drivers/mtd/cfi_flash.c | 10 +++++++++- include/flash.h | 1 + 2 files changed, 10 insertions(+), 1 deletion(-)