Re: [PATCH 0/3] arm: Add Analog Devices SC5xx Machine Type
On Thu, 2024-04-11 at 20:18 -0400, Greg Malysa wrote: > I'm afraid I have to admit I don't know. I'll work with our IT team to > make sure we can run CI locally, and when v2 comes around the answer > will be yes. > AFAIR, I think you the only exception to open a PR in the github fork is to run CI. - Nuno Sá
[PATCH v4] mtd: cfi: respect reg address length
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á
---
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, );
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;
--
2.40.1
Re: [PATCH v3 1/2] mtd: cfi: respect reg address length
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á > > --- > > > > 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á
[PATCH v3 1/2] mtd: cfi: respect reg address length
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á
---
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(-)
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, );
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;
--
2.40.0
[PATCH v3 2/2] mtd: cfi: change cfi_flash_bank_size() return type
Move return type to phys_size_t instead of plain unsigned long.
Signed-off-by: Nuno Sá
---
v2:
* new patch.
v3:
* also make sure cfi_flash_bank_size() declaration (in cfi_flash.h)
gets updated to phys_size_t.
drivers/mtd/cfi_flash.c | 10 +-
include/mtd/cfi_flash.h | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index 87a3daebdabe..9c030de3afef 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -117,9 +117,9 @@ phys_addr_t cfi_flash_bank_addr(int i)
return flash_info[i].base;
}
-unsigned long cfi_flash_bank_size(int i)
+phys_size_t cfi_flash_bank_size(int i)
{
- return (unsigned long)flash_info[i].addr_size;
+ return flash_info[i].addr_size;
}
#else
__weak phys_addr_t cfi_flash_bank_addr(int i)
@@ -127,10 +127,10 @@ __weak phys_addr_t cfi_flash_bank_addr(int i)
return ((phys_addr_t [])CFG_SYS_FLASH_BANKS_LIST)[i];
}
-__weak unsigned long cfi_flash_bank_size(int i)
+__weak phys_size_t cfi_flash_bank_size(int i)
{
#ifdef CFG_SYS_FLASH_BANKS_SIZES
- return ((unsigned long [])CFG_SYS_FLASH_BANKS_SIZES)[i];
+ return ((phys_size_t [])CFG_SYS_FLASH_BANKS_SIZES)[i];
#else
return 0;
#endif
@@ -2112,7 +2112,7 @@ ulong flash_get_size(phys_addr_t base, int banknum)
int erase_region_size;
int erase_region_count;
struct cfi_qry qry;
- unsigned long max_size;
+ phys_size_t max_size;
memset(, 0, sizeof(qry));
diff --git a/include/mtd/cfi_flash.h b/include/mtd/cfi_flash.h
index 52cd1c4dbc4e..1900a4a2f6e4 100644
--- a/include/mtd/cfi_flash.h
+++ b/include/mtd/cfi_flash.h
@@ -175,7 +175,7 @@ extern int cfi_flash_num_flash_banks;
#endif
phys_addr_t cfi_flash_bank_addr(int i);
-unsigned long cfi_flash_bank_size(int i);
+phys_size_t cfi_flash_bank_size(int i);
#ifdef CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS
void flash_write8(u8 value, void *addr);
--
2.40.0
Re: [PATCH v2 1/2] mtd: cfi: respect reg address length
On Mon, 2023-04-24 at 09:36 +0200, Stefan Roese wrote:
> On 4/24/23 09:13, Nuno Sá wrote:
> > On Mon, 2023-04-24 at 08:54 +0200, Stefan Roese wrote:
> >
> > Hi Stefan,
> >
> >
> > > Hi Nuno Sá
> > >
> > > On 4/18/23 15:07, 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á
> > > > ---
> > > > 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).
> > >
> > > Unfortunately this does not work. Build fails on multiple targets.
> > > Here an example:
> > >
> > > $ make qemu_arm64_defconfig
> > > $ make -sj
> > > drivers/mtd/cfi_flash.c:120:13: error: conflicting types for
> > > 'cfi_flash_bank_size'; have 'phys_size_t(int)' {aka 'long long unsigned
> > > int(int)'}
> > > 120 | phys_size_t cfi_flash_bank_size(int i)
> > > | ^~~
> > > In file included from drivers/mtd/cfi_flash.c:36:
> > > include/mtd/cfi_flash.h:178:15: note: previous declaration of
> > > 'cfi_flash_bank_size' with type 'long unsigned int(int)'
> > > 178 | unsigned long cfi_flash_bank_size(int i);
> > > | ^~~
> > >
> > > Please run a world build next before sending out the next patchset
> > > version.
> > >
> >
> > Arghh, sorry for this!
> >
> > I did run it for micoblaze_defconfig and integratorcp_cm926ejs_defconfig.
> > Should
> > I build it with V1 or something? Or is there somewhere where I can push my
> > patchset to trigger some CI and look for these kind of things myself before
> > sending v3?
>
> I'm testing with a world build on Azure, Gitlab provides a similar
> infrastructure. Some docs should be available for this.
>
I see, I just cloned the tree from https://source.denx.de/u-boot/u-boot.git. I
will fork from github and trigger those builds.
- Nuno Sá
Re: [PATCH v2 1/2] mtd: cfi: respect reg address length
On Mon, 2023-04-24 at 08:54 +0200, Stefan Roese wrote:
Hi Stefan,
> Hi Nuno Sá
>
> On 4/18/23 15:07, 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á
> > ---
> > 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).
>
> Unfortunately this does not work. Build fails on multiple targets.
> Here an example:
>
> $ make qemu_arm64_defconfig
> $ make -sj
> drivers/mtd/cfi_flash.c:120:13: error: conflicting types for
> 'cfi_flash_bank_size'; have 'phys_size_t(int)' {aka 'long long unsigned
> int(int)'}
> 120 | phys_size_t cfi_flash_bank_size(int i)
> | ^~~
> In file included from drivers/mtd/cfi_flash.c:36:
> include/mtd/cfi_flash.h:178:15: note: previous declaration of
> 'cfi_flash_bank_size' with type 'long unsigned int(int)'
> 178 | unsigned long cfi_flash_bank_size(int i);
> | ^~~
>
> Please run a world build next before sending out the next patchset
> version.
>
Arghh, sorry for this!
I did run it for micoblaze_defconfig and integratorcp_cm926ejs_defconfig. Should
I build it with V1 or something? Or is there somewhere where I can push my
patchset to trigger some CI and look for these kind of things myself before
sending v3?
- Nuno Sá
[PATCH v2 2/2] mtd: cfi: change cfi_flash_bank_size() return type
Move return type to phys_size_t instead of plain unsigned long.
Signed-off-by: Nuno Sá
---
v2:
* new patch.
drivers/mtd/cfi_flash.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index 47a48d927201..9c030de3afef 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -117,7 +117,7 @@ phys_addr_t cfi_flash_bank_addr(int i)
return flash_info[i].base;
}
-unsigned long cfi_flash_bank_size(int i)
+phys_size_t cfi_flash_bank_size(int i)
{
return flash_info[i].addr_size;
}
@@ -127,10 +127,10 @@ __weak phys_addr_t cfi_flash_bank_addr(int i)
return ((phys_addr_t [])CFG_SYS_FLASH_BANKS_LIST)[i];
}
-__weak unsigned long cfi_flash_bank_size(int i)
+__weak phys_size_t cfi_flash_bank_size(int i)
{
#ifdef CFG_SYS_FLASH_BANKS_SIZES
- return ((unsigned long [])CFG_SYS_FLASH_BANKS_SIZES)[i];
+ return ((phys_size_t [])CFG_SYS_FLASH_BANKS_SIZES)[i];
#else
return 0;
#endif
@@ -2112,7 +2112,7 @@ ulong flash_get_size(phys_addr_t base, int banknum)
int erase_region_size;
int erase_region_count;
struct cfi_qry qry;
- unsigned long max_size;
+ phys_size_t max_size;
memset(, 0, sizeof(qry));
--
2.40.0
[PATCH v2 1/2] mtd: cfi: respect reg address length
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á
---
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).
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..47a48d927201 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 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, );
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;
--
2.40.0
Re: [PATCH] mtd: cfi: respect reg address length
Hi Stefan,
On Fri, 2023-04-14 at 08:57 +0200, Stefan Roese wrote:
> 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á
> > ---
> >
> > 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, );
> > 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/
[PATCH] mtd: cfi: respect reg address length
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á
---
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, );
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;
--
2.40.0
