Skip to contentSkip to conversation

common/memsize.c: fix endless loop when saving

PATCH New Eddy Petrișor 3 participants 6 messages Feb 9, 2016 – Feb 10, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 common/memsize.c: fix endless loop when saving

When cnt reaches 0, any shift operation will keep cnt=0, so the condition cnt >= 0 doesn't make sense.

To fix this endless loop, we drop the useless condition and simply break the loop when cnt reaches 0.

---
 common/memsize.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
common/memsize.c +2 -1
index 5c0d279..a6af993 100644
@@ -33,7 +33,7 @@ long get_ram_size(long *base, long maxsize)
 	long           size;
 	int            i = 0;
 
-	for (cnt = (maxsize / sizeof(long)) >> 1; cnt >= 0; cnt >>= 1) {
+	for (cnt = (maxsize / sizeof(long)) >> 1; ; cnt >>= 1) {
 		addr = base + cnt;	/* pointer arith! */
 		sync();
 		save[i] = *addr;
@@ -43,6 +43,7 @@ long get_ram_size(long *base, long maxsize)
 			*addr = ~cnt;
 		} else {
 			*addr = 0;
+			break;
 		}
 	}
 
Signature · 1 line
1.9.2.459.g68773ac
#1 lore raw

PATCH 1/1 common/memsize.c: fix endless loop when saving

2016-02-09 19:41 GMT+02:00 Eddy Petri?or <[email protected]>:

Quoted 5 lines

When cnt reaches 0, any shift operation will keep cnt=0, so the condition cnt >= 0 doesn't make sense.

To fix this endless loop, we drop the useless condition and simply break the loop when cnt reaches 0.

This should fix the endless issue introduced by the previous patch 8e7cba048baae68ee0916a8f52b4304277328d5e

Hannes, can you confirm that with this patch the reported size is correct and the boot is normal again?
I initially thought the size might be incorret due to some buggy HW I have (it has some unstable DDR settings, which lead to incorrect reports on my side initially).

Quoted 29 lines
---
 common/memsize.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
common/memsize.c +2 -2
index 5c0d279..a6af993 100644
@@ -33,7 +33,7 @@ long get_ram_size(long *base, long maxsize)
        long           size;
        int            i = 0;
 
-       for (cnt = (maxsize / sizeof(long)) >> 1; cnt >= 0; cnt >>= 1) {
+       for (cnt = (maxsize / sizeof(long)) >> 1; ; cnt >>= 1) {
                addr = base + cnt;      /* pointer arith! */
                sync();
                save[i] = *addr;
@@ -43,6 +43,7 @@ long get_ram_size(long *base, long maxsize)
                        *addr = ~cnt;
                } else {
                        *addr = 0;
+                       break;
                }
        }
 
--

1.9.2.459.g68773ac

Signature · 1 line
Eddy Petri?or
#2 lore raw

PATCH 1/1 common/memsize.c: fix endless loop when saving

Quoted 6 lines · Eddy Petri?or

2016-02-09 19:41 GMT+02:00 Eddy Petri?or <[email protected]>:

Quoted 5 lines

When cnt reaches 0, any shift operation will keep cnt=0, so the condition cnt >= 0 doesn't make sense.

To fix this endless loop, we drop the useless condition and simply break the loop when cnt reaches 0.

Quoted 8 lines

This should fix the endless issue introduced by the previous patch 8e7cba048baae68ee0916a8f52b4304277328d5e

Hannes, can you confirm that with this patch the reported size is correct and the boot is normal again?
I initially thought the size might be incorret due to some buggy HW I have (it has some unstable DDR settings, which lead to incorrect reports on my side initially).

Hi Eddy,
the board does boot with this patch.
But the reported memsize is wrong.
1GiB instead 256MiB.

So we need some v2 to get rid.

best regards,
Hannes

#3 lore raw

PATCH 1/1 common/memsize.c: fix endless loop when saving

Pe 10 feb. 2016 8:26 a.m., "Hannes Schmelzer" <[email protected]> a scris:

Quoted 11 lines · Eddy Petri?or
Quoted 8 lines

2016-02-09 19:41 GMT+02:00 Eddy Petri?or <[email protected]>:

Hannes, can you confirm that with this patch the reported size is correct and the boot is normal again?
I initially thought the size might be incorret due to some buggy HW I have (it has some unstable DDR settings, which lead to incorrect reports on my side initially).

Quoted 6 lines

Hi Eddy,
the board does boot with this patch.
But the reported memsize is wrong.
1GiB instead 256MiB.

So we need some v2 to get rid.

Taking into account the first version was reverted, would you be willing to test a reworked patch targeting v2016.05, (when I have it ready)?

Quoted 4 lines

best regards,
Hannes

#4 lore raw

PATCH 1/1 common/memsize.c: fix endless loop when saving

Quoted 21 lines · Eddy Petri?or

Pe 10 feb. 2016 8:26 a.m., "Hannes Schmelzer" <[email protected] <mailto:[email protected]>> a scris:

Quoted 4 lines · Eddy Petri?or
Quoted 2 lines

2016-02-09 19:41 GMT+02:00 Eddy Petri?or <[email protected]

<mailto:[email protected]>>:

Quoted 11 lines
Quoted 5 lines

Hannes, can you confirm that with this patch the reported size is correct and the boot is normal again?
I initially thought the size might be incorret due to some buggy HW I have (it has some unstable DDR settings, which lead to incorrect reports on my side initially).

Hi Eddy,
the board does boot with this patch.
But the reported memsize is wrong.
1GiB instead 256MiB.

Quoted 6 lines
Quoted 1 line

So we need some v2 to get rid.

Taking into account the first version was reverted, would you be willing to test a reworked patch targeting v2016.05, (when I have it ready)?

Hi Eddy,
for me - yes.
But as Wolfgang said, the code is maybe already perfect :-) Do we really have here the need to tune this?

#5 lore raw

PATCH 1/1 common/memsize.c: fix endless loop when saving

Pe 10 feb. 2016 7:58 p.m., "Hannes Schmelzer" <[email protected]> a scris:

Quoted 4 lines · Eddy Petri?or
Quoted 1 line

Taking into account the first version was reverted, would you be willing

to test a reworked patch targeting v2016.05, (when I have it ready)?

Quoted 5 lines

Hi Eddy,
for me - yes.
But as Wolfgang said, the code is maybe already perfect :-) Do we really have here the need to tune this?

I was looking at that code due to my hw's DDR issues and spent some time trying to understand why does it have duplicated code, and I assume I am not the only one nor will be the last one.

But to be honest, I think the better approach would be to do the change in 2 steps, first the upper block, then the bottom one.

Eddy