[U-Boot] [PATCH v2] common/memsize.c: Increase save array for supporting memory size > 4GB

From: Tien Fong Chee tien.fong.chee@intel.com
In ARM 64-bits, memory size can be supported is more than 4GB, hence increasing save array is needed to cope with testing larger memory.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com ---
Changes in v2: - Change save array size to save[BITS_PER_LONG - 1] --- common/memsize.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/memsize.c b/common/memsize.c index 5670e95..13b0047 100644 --- a/common/memsize.c +++ b/common/memsize.c @@ -26,7 +26,7 @@ DECLARE_GLOBAL_DATA_PTR; long get_ram_size(long *base, long maxsize) { volatile long *addr; - long save[31]; + long save[BITS_PER_LONG - 1]; long save_base; long cnt; long val;

On 06/20/2018 09:06 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
In ARM 64-bits, memory size can be supported is more than 4GB, hence increasing save array is needed to cope with testing larger memory.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
Changes in v2:
- Change save array size to save[BITS_PER_LONG - 1]
common/memsize.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/memsize.c b/common/memsize.c index 5670e95..13b0047 100644 --- a/common/memsize.c +++ b/common/memsize.c @@ -26,7 +26,7 @@ DECLARE_GLOBAL_DATA_PTR; long get_ram_size(long *base, long maxsize) { volatile long *addr;
- long save[31];
- long save[BITS_PER_LONG - 1]; long save_base; long cnt; long val;
Does this work with LPAE systems, where bits per long == 32 and the address space is bigger ? Or with similar setups ? I mean, you can have
4 GiB of RAM on 32bit system ...

On Thu, 2018-06-21 at 06:34 +0200, Marek Vasut wrote:
On 06/20/2018 09:06 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
In ARM 64-bits, memory size can be supported is more than 4GB, hence increasing save array is needed to cope with testing larger memory.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
Changes in v2:
- Change save array size to save[BITS_PER_LONG - 1]
common/memsize.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/memsize.c b/common/memsize.c index 5670e95..13b0047 100644 --- a/common/memsize.c +++ b/common/memsize.c @@ -26,7 +26,7 @@ DECLARE_GLOBAL_DATA_PTR; long get_ram_size(long *base, long maxsize) { volatile long *addr;
- long save[31];
- long save[BITS_PER_LONG - 1];
long save_base; long cnt; long val;
Does this work with LPAE systems, where bits per long == 32 and the address space is bigger ? Or with similar setups ? I mean, you can have
4 GiB of RAM on 32bit system ...
This function is designed to work with 32bit or 64 bits system, for example the argument such as maxsize can be 32bit or 64 bit, if value larger than 4GiB is passed as maxsize argument with 32bit system, the whole thing will go wrong.

On 06/21/2018 08:31 AM, Chee, Tien Fong wrote:
On Thu, 2018-06-21 at 06:34 +0200, Marek Vasut wrote:
On 06/20/2018 09:06 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
In ARM 64-bits, memory size can be supported is more than 4GB, hence increasing save array is needed to cope with testing larger memory.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
Changes in v2:
- Change save array size to save[BITS_PER_LONG - 1]
common/memsize.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/memsize.c b/common/memsize.c index 5670e95..13b0047 100644 --- a/common/memsize.c +++ b/common/memsize.c @@ -26,7 +26,7 @@ DECLARE_GLOBAL_DATA_PTR; long get_ram_size(long *base, long maxsize) { volatile long *addr;
- long save[31];
- long save[BITS_PER_LONG - 1];
long save_base; long cnt; long val;
Does this work with LPAE systems, where bits per long == 32 and the address space is bigger ? Or with similar setups ? I mean, you can have
4 GiB of RAM on 32bit system ...
This function is designed to work with 32bit or 64 bits system, for example the argument such as maxsize can be 32bit or 64 bit, if value larger than 4GiB is passed as maxsize argument with 32bit system, the whole thing will go wrong.
And what do you do when you identify a problem in mainline ? :-)

On Sat, 2018-06-23 at 05:29 +0200, Marek Vasut wrote:
On 06/21/2018 08:31 AM, Chee, Tien Fong wrote:
On Thu, 2018-06-21 at 06:34 +0200, Marek Vasut wrote:
On 06/20/2018 09:06 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
In ARM 64-bits, memory size can be supported is more than 4GB, hence increasing save array is needed to cope with testing larger memory.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
Changes in v2:
- Change save array size to save[BITS_PER_LONG - 1]
common/memsize.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/memsize.c b/common/memsize.c index 5670e95..13b0047 100644 --- a/common/memsize.c +++ b/common/memsize.c @@ -26,7 +26,7 @@ DECLARE_GLOBAL_DATA_PTR; long get_ram_size(long *base, long maxsize) { volatile long *addr;
- long save[31];
- long save[BITS_PER_LONG - 1];
long save_base; long cnt; long val;
Does this work with LPAE systems, where bits per long == 32 and the address space is bigger ? Or with similar setups ? I mean, you can have
4 GiB of RAM on 32bit system ...
This function is designed to work with 32bit or 64 bits system, for example the argument such as maxsize can be 32bit or 64 bit, if value larger than 4GiB is passed as maxsize argument with 32bit system, the whole thing will go wrong.
And what do you do when you identify a problem in mainline ? :-)
I have no idea at this moment, how about fixing the issue for save array in 64-bit 1st?

On Wed, Jun 20, 2018 at 03:06:20PM +0800, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
In ARM 64-bits, memory size can be supported is more than 4GB, hence increasing save array is needed to cope with testing larger memory.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
Applied to u-boot/master, thanks!

On 07/11/2018 02:42 PM, Tom Rini wrote:
On Wed, Jun 20, 2018 at 03:06:20PM +0800, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
In ARM 64-bits, memory size can be supported is more than 4GB, hence increasing save array is needed to cope with testing larger memory.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
Applied to u-boot/master, thanks!
There were review comments on this patch which were not addressed, please drop.

On Wed, Jul 11, 2018 at 02:48:34PM +0200, Marek Vasut wrote:
On 07/11/2018 02:42 PM, Tom Rini wrote:
On Wed, Jun 20, 2018 at 03:06:20PM +0800, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
In ARM 64-bits, memory size can be supported is more than 4GB, hence increasing save array is needed to cope with testing larger memory.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
Applied to u-boot/master, thanks!
There were review comments on this patch which were not addressed, please drop.
I thought I had put them in my message here, sorry. I agree with Tien that we should fix the issue we have now and address the enhancement in a follow up patch.
participants (4)
-
Chee, Tien Fong
-
Marek Vasut
-
tien.fong.chee@intel.com
-
Tom Rini