Skip to contentSkip to conversation

common/memsize.c: Increase save array for supporting memory size > 4GB

PATCH Superseded tien.fong.chee at intel.com 4 participants 4 messages Jun 13, 2018 – Jun 19, 2018

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 common/memsize.c: Increase save array for supporting memory size > 4GB

From: Tien Fong Chee <[email protected]>

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.

---
 common/memsize.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
common/memsize.c +1 -1
index 5670e95..b091203 100644
@@ -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];
 	long           save_base;
 	long           cnt;
 	long           val;
Signature · 1 line
1.7.7.4
#1 lore raw

PATCH 1/1 common/memsize.c: Increase save array for supporting memory size > 4GB

Quoted 15 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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.

---
 common/memsize.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
common/memsize.c +0 -0
index 5670e95..b091203 100644

common/memsize.c DECLARE_GLOBAL_DATA_PTR; · line 32

Quoted 8 lines
common/memsize.c +1 -1
@@ -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];
 	long           save_base;
 	long           cnt;
 	long           val;

Since BITS_PER_LONG is 32 or 64, shouldn't we use B_P_L - 1 here? Or are you saying there's also a case where this is wrong on 32bit today? Thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180618/600eb89a/attachment.sig>
#2 lore raw

PATCH 1/1 common/memsize.c: Increase save array for supporting memory size > 4GB

Quoted 15 lines · Tom Rini
Quoted 12 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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.

---
 common/memsize.c |    2 +-

 1 files changed, 1 insertions(+), 1 deletions(-)
Quoted 16 lines

common/memsize.c DECLARE_GLOBAL_DATA_PTR; · line 32

Quoted 13 lines
common/memsize.c +1 -1
index 5670e95..b091203 100644
@@ -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];
  	long           save_base;
  	long           cnt;
  	long           val;

Since BITS_PER_LONG is 32 or 64, shouldn't we use B_P_L - 1 here?  Or are you saying there's also a case where this is wrong on 32bit today?

As long as the array is large enough to cope with shifting implementation, then it should be fine. For 32-bit, only 31 units in array required for storing 31 shifting values, and this apply for 64- bit also as long as the implementation of first shifting value is not change(cnt = (maxsize / sizeof(long)) >> 1).
IMO, for simplifying and safety purpose(may be one day implementation change to "cnt = (maxsize / sizeof(long))", above B_P_L is still workable.

Quoted 2 lines

Thanks!

#3 lore raw

PATCH 1/1 common/memsize.c: Increase save array for supporting memory size > 4GB

Quoted 32 lines · Tien Fong
Quoted 15 lines · Tom Rini
Quoted 12 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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.

---
 common/memsize.c |    2 +-

 1 files changed, 1 insertions(+), 1 deletions(-)
Quoted 16 lines

common/memsize.c DECLARE_GLOBAL_DATA_PTR; · line 32

Quoted 13 lines
common/memsize.c +1 -1
index 5670e95..b091203 100644
@@ -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];
  	long           save_base;
  	long           cnt;
  	long           val;

Since BITS_PER_LONG is 32 or 64, shouldn't we use B_P_L - 1 here?  Or are you saying there's also a case where this is wrong on 32bit today?

Quoted 8 lines

As long as the array is large enough to cope with shifting implementation, then it should be fine. For 32-bit, only 31 units in array required for storing 31 shifting values, and this apply for 64- bit also as long as the implementation of first shifting value is not change(cnt = (maxsize / sizeof(long)) >> 1).
IMO, for simplifying and safety purpose(may be one day implementation change to "cnt = (maxsize / sizeof(long))", above B_P_L is still workable.

That's BS reasoning and just sloppy programming. I agree with Tom.

Signature · 2 lines
Best regards,
Marek Vasut