| Message ID | 20230426141656.1596908-1-nuno.sa@analog.com |
|---|---|
| State | Superseded |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | [v3,1/2] mtd: cfi: respect reg address length | expand |
Hi Nuno Sá, On 4/26/23 16:16, 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> > --- > > 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() > > Stefan, I did ran a world build [1] by opening a PR in github (to force CI > to run). However I had to bypass the pytest stage (it was giving me some > unrelated problems) and there are a couple of jobs failing but the > errors apparently are not related to this patchset. Hopefully things now > pass in your tests. > > [1]: https://github.com/u-boot/u-boot/pull/281 Unfortunately this breaks my usual Azure CI build in the test_py phase. And this works without any issues without those 2 patches applied. So its related to these changes. Sorry, I can't apply these patches. Please take another look at fixing these issues. Thanks, Stefan [1] https://dev.azure.com/sr0718/u-boot/_build/results?buildId=298&view=results > drivers/mtd/cfi_flash.c | 11 +++++++++-- > include/flash.h | 1 + > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c > index f378f6fb6139..87a3daebdabe 100644 > --- a/drivers/mtd/cfi_flash.c > +++ b/drivers/mtd/cfi_flash.c > @@ -116,12 +116,16 @@ phys_addr_t cfi_flash_bank_addr(int i) > { > return flash_info[i].base; > } > + > +unsigned long cfi_flash_bank_size(int i) > +{ > + return (unsigned long)flash_info[i].addr_size; > +} > #else > __weak phys_addr_t cfi_flash_bank_addr(int i) > { > return ((phys_addr_t [])CFG_SYS_FLASH_BANKS_LIST)[i]; > } > -#endif > > __weak unsigned long cfi_flash_bank_size(int i) > { > @@ -131,6 +135,7 @@ __weak unsigned long cfi_flash_bank_size(int i) > return 0; > #endif > } > +#endif > > __maybe_weak void flash_write8(u8 value, void *addr) > { > @@ -2492,15 +2497,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
Hi Stefan, On Fri, 2023-04-28 at 11:59 +0200, Stefan Roese wrote: > Hi Nuno Sá, > > On 4/26/23 16:16, 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> > > --- > > > > 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() > > > > Stefan, I did ran a world build [1] by opening a PR in github (to force CI > > to run). However I had to bypass the pytest stage (it was giving me some > > unrelated problems) and there are a couple of jobs failing but the > > errors apparently are not related to this patchset. Hopefully things now > > pass in your tests. > > > > [1]: https://github.com/u-boot/u-boot/pull/281 > > Unfortunately this breaks my usual Azure CI build in the test_py phase. > And this works without any issues without those 2 patches applied. So > its related to these changes. > Sorry for this... It seemed unrelated and the logs don't say much about why is it failing. I'll try to see what's the problem. - Nuno Sá
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index f378f6fb6139..87a3daebdabe 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -116,12 +116,16 @@ phys_addr_t cfi_flash_bank_addr(int i) { return flash_info[i].base; } + +unsigned long cfi_flash_bank_size(int i) +{ + return (unsigned long)flash_info[i].addr_size; +} #else __weak phys_addr_t cfi_flash_bank_addr(int i) { return ((phys_addr_t [])CFG_SYS_FLASH_BANKS_LIST)[i]; } -#endif __weak unsigned long cfi_flash_bank_size(int i) { @@ -131,6 +135,7 @@ __weak unsigned long cfi_flash_bank_size(int i) return 0; #endif } +#endif __maybe_weak void flash_write8(u8 value, void *addr) { @@ -2492,15 +2497,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() Stefan, I did ran a world build [1] by opening a PR in github (to force CI to run). However I had to bypass the pytest stage (it was giving me some unrelated problems) and there are a couple of jobs failing but the errors apparently are not related to this patchset. Hopefully things now pass in your tests. [1]: https://github.com/u-boot/u-boot/pull/281 drivers/mtd/cfi_flash.c | 11 +++++++++-- include/flash.h | 1 + 2 files changed, 10 insertions(+), 2 deletions(-)