[U-Boot] [PATCH] common/memsize.c: Simplify RAM size detection

[U-Boot] [PATCH] common/memsize.c: Simplify RAM size detection

[Eddy Petrișor]

The case of memory of size 0 is not that different from a memory of any other
size, so we remove the duplicate code and treat the small differences when it
is the case.

Series-to: u-boot
Signed-off-by: Eddy Petri?or <eddy.petrisor@gmail.com>
---
 common/memsize.c | 47 +++++++++++++++++++++--------------------------
 1 file changed, 21 insertions(+), 26 deletions(-)

diff --git a/common/memsize.c b/common/memsize.c
index 0fb9ba5..5c0d279 100644
--- a/common/memsize.c
+++ b/common/memsize.c
@@ -33,38 +33,28 @@ 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 >= 0; cnt >>= 1) {
 		addr = base + cnt;	/* pointer arith! */
 		sync();
-		save[i++] = *addr;
+		save[i] = *addr;
 		sync();
-		*addr = ~cnt;
-	}
-
-	addr = base;
-	sync();
-	save[i] = *addr;
-	sync();
-	*addr = 0;
-
-	sync();
-	if ((val = *addr) != 0) {
-		/* Restore the original data before leaving the function. */
-		sync();
-		*addr = save[i];
-		for (cnt = 1; cnt < maxsize / sizeof(long); cnt <<= 1) {
-			addr  = base + cnt;
-			sync();
-			*addr = save[--i];
+		if (cnt) {
+			i++;
+			*addr = ~cnt;
+		} else {
+			*addr = 0;
 		}
-		return (0);
 	}
 
-	for (cnt = 1; cnt < maxsize / sizeof(long); cnt <<= 1) {
+	sync();
+	cnt = 0;
+	do {
 		addr = base + cnt;	/* pointer arith! */
 		val = *addr;
-		*addr = save[--i];
-		if (val != ~cnt) {
+		*addr = save[i--];
+		sync();
+		if (((cnt == 0) && (val != 0)) ||
+		    ((cnt != 0) && (val != ~cnt))) {
 			size = cnt * sizeof(long);
 			/*
 			 * Restore the original data
@@ -74,11 +64,16 @@ long get_ram_size(long *base, long maxsize)
 			     cnt < maxsize / sizeof(long);
 			     cnt <<= 1) {
 				addr  = base + cnt;
-				*addr = save[--i];
+				*addr = save[i--];
 			}
 			return (size);
 		}
-	}
+
+		if (cnt)
+			cnt = cnt << 1;
+		else
+			cnt = 1;
+	} while (cnt < maxsize / sizeof(long));
 
 	return (maxsize);
 }
-- 
2.1.4

[U-Boot] [PATCHv2] common/memsize.c: Simplify RAM size detection

[Eddy Petrișor]

The case of memory of size 0 is not that different from a memory of any other
size, so we remove the duplicate code and treat the small differences when it
is the case.

Signed-off-by: Eddy Petri?or <eddy.petrisor@gmail.com>
---

v2: Removed patman stuff from commit message

 common/memsize.c | 47 +++++++++++++++++++++--------------------------
 1 file changed, 21 insertions(+), 26 deletions(-)

diff --git a/common/memsize.c b/common/memsize.c
index 0fb9ba5..5c0d279 100644
--- a/common/memsize.c
+++ b/common/memsize.c
@@ -33,38 +33,28 @@ 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 >= 0; cnt >>= 1) {
 		addr = base + cnt;	/* pointer arith! */
 		sync();
-		save[i++] = *addr;
+		save[i] = *addr;
 		sync();
-		*addr = ~cnt;
-	}
-
-	addr = base;
-	sync();
-	save[i] = *addr;
-	sync();
-	*addr = 0;
-
-	sync();
-	if ((val = *addr) != 0) {
-		/* Restore the original data before leaving the function. */
-		sync();
-		*addr = save[i];
-		for (cnt = 1; cnt < maxsize / sizeof(long); cnt <<= 1) {
-			addr  = base + cnt;
-			sync();
-			*addr = save[--i];
+		if (cnt) {
+			i++;
+			*addr = ~cnt;
+		} else {
+			*addr = 0;
 		}
-		return (0);
 	}
 
-	for (cnt = 1; cnt < maxsize / sizeof(long); cnt <<= 1) {
+	sync();
+	cnt = 0;
+	do {
 		addr = base + cnt;	/* pointer arith! */
 		val = *addr;
-		*addr = save[--i];
-		if (val != ~cnt) {
+		*addr = save[i--];
+		sync();
+		if (((cnt == 0) && (val != 0)) ||
+		    ((cnt != 0) && (val != ~cnt))) {
 			size = cnt * sizeof(long);
 			/*
 			 * Restore the original data
@@ -74,11 +64,16 @@ long get_ram_size(long *base, long maxsize)
 			     cnt < maxsize / sizeof(long);
 			     cnt <<= 1) {
 				addr  = base + cnt;
-				*addr = save[--i];
+				*addr = save[i--];
 			}
 			return (size);
 		}
-	}
+
+		if (cnt)
+			cnt = cnt << 1;
+		else
+			cnt = 1;
+	} while (cnt < maxsize / sizeof(long));
 
 	return (maxsize);
 }
-- 
2.1.4

[U-Boot] [PATCHv2] common/memsize.c: Simplify RAM size detection

[Albert ARIBAUD]

Hello Eddy,

On Tue,  2 Feb 2016 22:15:28 +0200, Eddy Petri?or
<eddy.petrisor@gmail.com> wrote:
> The case of memory of size 0 is not that different from a memory of any other
> size, so we remove the duplicate code and treat the small differences when it
> is the case.
> 
> Signed-off-by: Eddy Petri?or <eddy.petrisor@gmail.com>
> ---
> 
> v2: Removed patman stuff from commit message

Sorry for asking, but since you're mentioning it... If you are using
patman, then why do you alter its mails rather than just letting it
post them as-is?

Amicalement,
-- 
Albert.

[U-Boot] [PATCHv2] common/memsize.c: Simplify RAM size detection

[Eddy Petrișor]

2016-02-03 15:38 GMT+02:00 Albert ARIBAUD <albert.u.boot@aribaud.net>:
> Hello Eddy,
>
> On Tue,  2 Feb 2016 22:15:28 +0200, Eddy Petri?or
> <eddy.petrisor@gmail.com> wrote:
[..]
>> v2: Removed patman stuff from commit message
>
> Sorry for asking, but since you're mentioning it... If you are using
> patman, then why do you alter its mails rather than just letting it
> post them as-is?

Actually I tried using patman, but got all sorts of errors and wasn't
able to use it. Then I manually sent the patches with git send-email,
but forgot to remove the patman header. So in the repost I removed the
patman header since it was of no use. I hope is more clear now.

-- 
Eddy Petri?or

[U-Boot] Patman use (was: [PATCHv2] common/memsize.c: Simplify RAM size detection)

[Albert ARIBAUD]

Hello Eddy,

(re-titling)

On Wed, 3 Feb 2016 18:32:45 +0200, Eddy Petri?or
<eddy.petrisor@gmail.com> wrote:
> 2016-02-03 15:38 GMT+02:00 Albert ARIBAUD <albert.u.boot@aribaud.net>:
> > Hello Eddy,
> >
> > On Tue,  2 Feb 2016 22:15:28 +0200, Eddy Petri?or
> > <eddy.petrisor@gmail.com> wrote:
> [..]
> >> v2: Removed patman stuff from commit message
> >
> > Sorry for asking, but since you're mentioning it... If you are using
> > patman, then why do you alter its mails rather than just letting it
> > post them as-is?
> 
> Actually I tried using patman, but got all sorts of errors and wasn't
> able to use it. Then I manually sent the patches with git send-email,
> but forgot to remove the patman header. So in the repost I removed the
> patman header since it was of no use. I hope is more clear now.

If you can send with git send-email, then you should be able to, and
should, use patman. What were these errors?

> -- 
> Eddy Petri?or

Amicalement,
-- 
Albert.

[U-Boot] Fwd: Patman use (was: [PATCHv2] common/memsize.c: Simplify RAM size detection)

[Eddy Petrișor]

(Sorry Albert for the dup, I accidentally sent the reply only to you.)


---------- Forwarded message ----------
From: Eddy Petri?or <eddy.petrisor@gmail.com>
Date: 2016-02-05 9:06 GMT+02:00
Subject: Re: Patman use (was: [U-Boot] [PATCHv2] common/memsize.c:
Simplify RAM size detection)
To: Albert ARIBAUD <albert.u.boot@aribaud.net>


2016-02-03 19:48 GMT+02:00 Albert ARIBAUD <albert.u.boot@aribaud.net>:
> Hello Eddy,
>
> (re-titling)
>
>> Actually I tried using patman, but got all sorts of errors and wasn't
>> able to use it. Then I manually sent the patches with git send-email,
[..]
>
> If you can send with git send-email, then you should be able to, and
> should, use patman. What were these errors?

0 eddy at heidi ~/usr/src/u-boot $ ./tools/patman/patman -n -c1

[git:simplfy_get_ram_size]
Cleaned 1 patches
Traceback (most recent call last):
  File "./tools/patman/patman", line 158, in <module>
    options.add_maintainers)
  File "/home/eddy/usr/src/u-boot/tools/patman/series.py", line 222,
in MakeCcFile
    raise_on_error=raise_on_error)
  File "/home/eddy/usr/src/u-boot/tools/patman/gitutil.py", line 321,
in BuildEmailList
    raw += LookupEmail(item, alias, raise_on_error=raise_on_error)
  File "/home/eddy/usr/src/u-boot/tools/patman/gitutil.py", line 495,
in LookupEmail
    raise ValueError, msg
ValueError: Alias 'common' not found


--
Eddy Petri?or


-- 
Eddy Petri?or

[U-Boot] [U-Boot, PATCHv2] common/memsize.c: Simplify RAM size detection

[Tom Rini]

On Tue, Feb 02, 2016 at 10:15:28PM +0200, Eddy Petri?or wrote:

> The case of memory of size 0 is not that different from a memory of any other
> size, so we remove the duplicate code and treat the small differences when it
> is the case.
> 
> Signed-off-by: Eddy Petri?or <eddy.petrisor@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160208/43a77a44/attachment.sig>

[U-Boot] [U-Boot, PATCHv2] common/memsize.c: Simplify RAM size detection

[Eddy Petrișor]

2016-02-08 22:49 GMT+02:00 Tom Rini <trini@konsulko.com>:
> On Tue, Feb 02, 2016 at 10:15:28PM +0200, Eddy Petri?or wrote:
>
>> The case of memory of size 0 is not that different from a memory of any other
>> size, so we remove the duplicate code and treat the small differences when it
>> is the case.
>>
>> Signed-off-by: Eddy Petri?or <eddy.petrisor@gmail.com>
>
> Applied to u-boot/master, thanks!

Thank you!

-- 
Eddy Petri?or

[U-Boot] [PATCHv2] common/memsize.c: Simplify RAM size detection

[Hannes Schmelzer]

On 02.02.2016 21:15, Eddy Petri?or wrote:
> The case of memory of size 0 is not that different from a memory of any other
> size, so we remove the duplicate code and treat the small differences when it
> is the case.
>
> Signed-off-by: Eddy Petri?or <eddy.petrisor@gmail.com>
> ---
>
> v2: Removed patman stuff from commit message
>
>   common/memsize.c | 47 +++++++++++++++++++++--------------------------
>   1 file changed, 21 insertions(+), 26 deletions(-)
>
> diff --git a/common/memsize.c b/common/memsize.c
> index 0fb9ba5..5c0d279 100644
> --- a/common/memsize.c
> +++ b/common/memsize.c
> @@ -33,38 +33,28 @@ 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 >= 0; cnt >>= 1) {
>   		addr = base + cnt;	/* pointer arith! */
>   		sync();
> -		save[i++] = *addr;
> +		save[i] = *addr;
>   		sync();
> -		*addr = ~cnt;
> -	}
> -
> -	addr = base;
> -	sync();
> -	save[i] = *addr;
> -	sync();
> -	*addr = 0;
> -
> -	sync();
> -	if ((val = *addr) != 0) {
> -		/* Restore the original data before leaving the function. */
> -		sync();
> -		*addr = save[i];
> -		for (cnt = 1; cnt < maxsize / sizeof(long); cnt <<= 1) {
> -			addr  = base + cnt;
> -			sync();
> -			*addr = save[--i];
> +		if (cnt) {
> +			i++;
> +			*addr = ~cnt;
> +		} else {
> +			*addr = 0;
>   		}
> -		return (0);
>   	}
>   
> -	for (cnt = 1; cnt < maxsize / sizeof(long); cnt <<= 1) {
> +	sync();
> +	cnt = 0;
> +	do {
>   		addr = base + cnt;	/* pointer arith! */
>   		val = *addr;
> -		*addr = save[--i];
> -		if (val != ~cnt) {
> +		*addr = save[i--];
> +		sync();
> +		if (((cnt == 0) && (val != 0)) ||
> +		    ((cnt != 0) && (val != ~cnt))) {
>   			size = cnt * sizeof(long);
>   			/*
>   			 * Restore the original data
> @@ -74,11 +64,16 @@ long get_ram_size(long *base, long maxsize)
>   			     cnt < maxsize / sizeof(long);
>   			     cnt <<= 1) {
>   				addr  = base + cnt;
> -				*addr = save[--i];
> +				*addr = save[i--];
>   			}
>   			return (size);
>   		}
> -	}
> +
> +		if (cnt)
> +			cnt = cnt << 1;
> +		else
> +			cnt = 1;
> +	} while (cnt < maxsize / sizeof(long));
>   
>   	return (maxsize);
>   }
Hi Eddy,

this commit breaks my BuR AM335x boards.
The board stucks at boot:

U-Boot 2016.03-rc1-00195-g437cc77-dirty (Feb 09 2016 - 09:25:51 +0100)

initcall: 8080a384
U-Boot code: 80800000 -> 8083A6F4  BSS: -> 8088433C
initcall: 8080a174
initcall: 8080a3f0
I2C: ready
initcall: 8080a3d8
DRAM:  initcall: 80800784

has anybody else experience on am335x boards with this ?

best regards,
Hannes

[U-Boot] [PATCHv2] common/memsize.c: Simplify RAM size detection

[Eddy Petrișor]

2016-02-09 12:14 GMT+02:00 Hannes Schmelzer <hannes@schmelzer.or.at>:
>
> On 02.02.2016 21:15, Eddy Petri?or wrote:
>>
>> The case of memory of size 0 is not that different from a memory of any
>> other
>> size, so we remove the duplicate code and treat the small differences when
>> it
>> is the case.
>>
>> Signed-off-by: Eddy Petri?or <eddy.petrisor@gmail.com>
>> ---
>>
>> v2: Removed patman stuff from commit message
>>
>>   common/memsize.c | 47 +++++++++++++++++++++--------------------------
>>   1 file changed, 21 insertions(+), 26 deletions(-)
>>
>> diff --git a/common/memsize.c b/common/memsize.c
>> index 0fb9ba5..5c0d279 100644
>> --- a/common/memsize.c
>> +++ b/common/memsize.c
>> @@ -33,38 +33,28 @@ 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 >= 0; cnt >>= 1) {
>>                 addr = base + cnt;      /* pointer arith! */
>>                 sync();
>> -               save[i++] = *addr;
>> +               save[i] = *addr;
>>                 sync();
>> -               *addr = ~cnt;
>> -       }
>> -
>> -       addr = base;
>> -       sync();
>> -       save[i] = *addr;
>> -       sync();
>> -       *addr = 0;
>> -
>> -       sync();
>> -       if ((val = *addr) != 0) {
>> -               /* Restore the original data before leaving the function.
>> */
>> -               sync();
>> -               *addr = save[i];
>> -               for (cnt = 1; cnt < maxsize / sizeof(long); cnt <<= 1) {
>> -                       addr  = base + cnt;
>> -                       sync();
>> -                       *addr = save[--i];
>> +               if (cnt) {
>> +                       i++;
>> +                       *addr = ~cnt;
>> +               } else {
>> +                       *addr = 0;
>>                 }
>> -               return (0);
>>         }
>>   -     for (cnt = 1; cnt < maxsize / sizeof(long); cnt <<= 1) {
>> +       sync();
>> +       cnt = 0;
>> +       do {
>>                 addr = base + cnt;      /* pointer arith! */
>>                 val = *addr;
>> -               *addr = save[--i];
>> -               if (val != ~cnt) {
>> +               *addr = save[i--];
>> +               sync();
>> +               if (((cnt == 0) && (val != 0)) ||
>> +                   ((cnt != 0) && (val != ~cnt))) {
>>                         size = cnt * sizeof(long);
>>                         /*
>>                          * Restore the original data
>> @@ -74,11 +64,16 @@ long get_ram_size(long *base, long maxsize)
>>                              cnt < maxsize / sizeof(long);
>>                              cnt <<= 1) {
>>                                 addr  = base + cnt;
>> -                               *addr = save[--i];
>> +                               *addr = save[i--];
>>                         }
>>                         return (size);
>>                 }
>> -       }
>> +
>> +               if (cnt)
>> +                       cnt = cnt << 1;
>> +               else
>> +                       cnt = 1;
>> +       } while (cnt < maxsize / sizeof(long));
>>         return (maxsize);
>>   }
>
> Hi Eddy,
>
> this commit breaks my BuR AM335x boards.
> The board stucks at boot:
>
> U-Boot 2016.03-rc1-00195-g437cc77-dirty (Feb 09 2016 - 09:25:51 +0100)
>
> initcall: 8080a384
> U-Boot code: 80800000 -> 8083A6F4  BSS: -> 8088433C
> initcall: 8080a174
> initcall: 8080a3f0
> I2C: ready
> initcall: 8080a3d8
> DRAM:  initcall: 80800784
>
> has anybody else experience on am335x boards with this ?

Hi Hannes,

You are correct, it seems that during my clean up phase before sending
the patch I managed to send an outdated version of the patch.
The easiest fix is to add a 'break;' after the '*addr = 0;' part, but
the reported size will be incorrect.

I will work on a correct fixup.
Sorry for the inconvenience.


-- 
Eddy Petri?or

[U-Boot] Fwd: Patman use (was: [PATCHv2] common/memsize.c: Simplify RAM size detection)

[Simon Glass]

Hi Eddy,

On 5 February 2016 at 10:03, Eddy Petri?or <eddy.petrisor@gmail.com> wrote:
> (Sorry Albert for the dup, I accidentally sent the reply only to you.)
>
>
> ---------- Forwarded message ----------
> From: Eddy Petri?or <eddy.petrisor@gmail.com>
> Date: 2016-02-05 9:06 GMT+02:00
> Subject: Re: Patman use (was: [U-Boot] [PATCHv2] common/memsize.c:
> Simplify RAM size detection)
> To: Albert ARIBAUD <albert.u.boot@aribaud.net>
>
>
> 2016-02-03 19:48 GMT+02:00 Albert ARIBAUD <albert.u.boot@aribaud.net>:
>> Hello Eddy,
>>
>> (re-titling)
>>
>>> Actually I tried using patman, but got all sorts of errors and wasn't
>>> able to use it. Then I manually sent the patches with git send-email,
> [..]
>>
>> If you can send with git send-email, then you should be able to, and
>> should, use patman. What were these errors?
>
> 0 eddy at heidi ~/usr/src/u-boot $ ./tools/patman/patman -n -c1
>
> [git:simplfy_get_ram_size]
> Cleaned 1 patches
> Traceback (most recent call last):
>   File "./tools/patman/patman", line 158, in <module>
>     options.add_maintainers)
>   File "/home/eddy/usr/src/u-boot/tools/patman/series.py", line 222,
> in MakeCcFile
>     raise_on_error=raise_on_error)
>   File "/home/eddy/usr/src/u-boot/tools/patman/gitutil.py", line 321,
> in BuildEmailList
>     raw += LookupEmail(item, alias, raise_on_error=raise_on_error)
>   File "/home/eddy/usr/src/u-boot/tools/patman/gitutil.py", line 495,
> in LookupEmail
>     raise ValueError, msg
> ValueError: Alias 'common' not found

You can use -t to ignore missing tags.

Regards,
Simon