* [PATCH v2 1/2] mtd: cfi: respect reg address length
@ 2023-04-18 13:07 Nuno Sá
2023-04-18 13:07 ` [PATCH v2 2/2] mtd: cfi: change cfi_flash_bank_size() return type Nuno Sá
2023-04-24 6:54 ` [PATCH v2 1/2] mtd: cfi: respect reg address length Stefan Roese
0 siblings, 2 replies; 6+ messages in thread
From: Nuno Sá @ 2023-04-18 13:07 UTC (permalink / raw)
To: u-boot; +Cc: Stefan Roese
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).
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, &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;
--
2.40.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] mtd: cfi: change cfi_flash_bank_size() return type
2023-04-18 13:07 [PATCH v2 1/2] mtd: cfi: respect reg address length Nuno Sá
@ 2023-04-18 13:07 ` Nuno Sá
2023-04-24 6:54 ` [PATCH v2 1/2] mtd: cfi: respect reg address length Stefan Roese
1 sibling, 0 replies; 6+ messages in thread
From: Nuno Sá @ 2023-04-18 13:07 UTC (permalink / raw)
To: u-boot; +Cc: Stefan Roese
Move return type to phys_size_t instead of plain unsigned long.
Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
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(&qry, 0, sizeof(qry));
--
2.40.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] mtd: cfi: respect reg address length
2023-04-18 13:07 [PATCH v2 1/2] mtd: cfi: respect reg address length Nuno Sá
2023-04-18 13:07 ` [PATCH v2 2/2] mtd: cfi: change cfi_flash_bank_size() return type Nuno Sá
@ 2023-04-24 6:54 ` Stefan Roese
2023-04-24 7:13 ` Nuno Sá
1 sibling, 1 reply; 6+ messages in thread
From: Stefan Roese @ 2023-04-24 6:54 UTC (permalink / raw)
To: Nuno Sá, u-boot
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á <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).
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.
Thanks,
Stefan
> 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, &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
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] mtd: cfi: respect reg address length
2023-04-24 6:54 ` [PATCH v2 1/2] mtd: cfi: respect reg address length Stefan Roese
@ 2023-04-24 7:13 ` Nuno Sá
2023-04-24 7:36 ` Stefan Roese
0 siblings, 1 reply; 6+ messages in thread
From: Nuno Sá @ 2023-04-24 7:13 UTC (permalink / raw)
To: Stefan Roese, Nuno Sá, u-boot
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á <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).
>
> 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á
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] mtd: cfi: respect reg address length
2023-04-24 7:13 ` Nuno Sá
@ 2023-04-24 7:36 ` Stefan Roese
2023-04-24 11:22 ` Nuno Sá
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Roese @ 2023-04-24 7:36 UTC (permalink / raw)
To: Nuno Sá, Nuno Sá, u-boot
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á <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).
>>
>> 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.
Thanks,
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] mtd: cfi: respect reg address length
2023-04-24 7:36 ` Stefan Roese
@ 2023-04-24 11:22 ` Nuno Sá
0 siblings, 0 replies; 6+ messages in thread
From: Nuno Sá @ 2023-04-24 11:22 UTC (permalink / raw)
To: Stefan Roese, Nuno Sá, u-boot
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á <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).
> > >
> > > 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á
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-04-24 12:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-18 13:07 [PATCH v2 1/2] mtd: cfi: respect reg address length Nuno Sá
2023-04-18 13:07 ` [PATCH v2 2/2] mtd: cfi: change cfi_flash_bank_size() return type Nuno Sá
2023-04-24 6:54 ` [PATCH v2 1/2] mtd: cfi: respect reg address length Stefan Roese
2023-04-24 7:13 ` Nuno Sá
2023-04-24 7:36 ` Stefan Roese
2023-04-24 11:22 ` Nuno Sá
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.