[U-Boot] [PATCH] get_ram_size: memory content of base address was not restored

[U-Boot] [PATCH] get_ram_size: memory content of base address was not restored

[ratbert.chuang at gmail.com]

From: Po-Yu Chuang <ratbert@faraday-tech.com>


Signed-off-by: Po-Yu Chuang <ratbert@faraday-tech.com>
---
 common/memsize.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/common/memsize.c b/common/memsize.c
index 6c275c9..99469ab 100644
--- a/common/memsize.c
+++ b/common/memsize.c
@@ -74,6 +74,7 @@ long get_ram_size(volatile long *base, long maxsize)
                return (0);
        }

+       *addr = save[i];
        for (cnt = 1; cnt < maxsize / sizeof (long); cnt <<= 1) {
                addr = base + cnt;      /* pointer arith! */
                val = *addr;
--
1.6.0.4

[U-Boot] [PATCH] get_ram_size: memory content of base address was not restored

[Wolfgang Denk]

Dear ratbert.chuang at gmail.com,

In message <1274771093-29677-1-git-send-email-ratbert.chuang@gmail.com> you wrote:
> From: Po-Yu Chuang <ratbert@faraday-tech.com>
> 
> 
> Signed-off-by: Po-Yu Chuang <ratbert@faraday-tech.com>
> ---
>  common/memsize.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/common/memsize.c b/common/memsize.c
> index 6c275c9..99469ab 100644
> --- a/common/memsize.c
> +++ b/common/memsize.c
> @@ -74,6 +74,7 @@ long get_ram_size(volatile long *base, long maxsize)
>                 return (0);
>         }
> 
> +       *addr = save[i];
>         for (cnt = 1; cnt < maxsize / sizeof (long); cnt <<= 1) {
>                 addr = base + cnt;      /* pointer arith! */
>                 val = *addr;

I think this patch is wrong. The value at address (base + 0) is never
saved, so there is no need to restore it (nor did we store it's value
anywhere).

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The important thing about being a leader is not being right or wrong,
but being *certain*.                    - Terry Pratchett, _Truckers_

[U-Boot] [PATCH] get_ram_size: memory content of base address was not restored

[Po-Yu Chuang]

Dear Wolfgang,

2010/8/8 Wolfgang Denk <wd@denx.de>:
> Dear ratbert.chuang at gmail.com,
> I think this patch is wrong. The value at address (base + 0) is never
> saved, so there is no need to restore it (nor did we store it's value
> anywhere).

The value at base+0 is saved. Please see the following explanation.

long get_ram_size(volatile long *base, long maxsize)
{
	volatile long *addr;
	long           save[32];
	long           cnt;
	long           val;
	long           size;
	int            i = 0;

	for (cnt = (maxsize / sizeof (long)) >> 1; cnt > 0; cnt >>= 1) {
		addr = base + cnt;	/* pointer arith! */
		sync ();
		save[i++] = *addr;
		sync ();
		*addr = ~cnt;
	}

	addr = base;
	sync ();
	save[i] = *addr;	<----- value of [base+0] is saved.
	sync ();
	*addr = 0;		<----- [base+0] is modified.

	sync ();
	if ((val = *addr) != 0) {
		/* Restore the original data before leaving the function.
		 */
		sync ();
		*addr = save[i];		<----- value of [base+0] is restored.
		for (cnt = 1; cnt < maxsize / sizeof(long); cnt <<= 1) {
			addr  = base + cnt;
			sync ();
			*addr = save[--i];
		}
		return (0);
	}

+	*addr = save[i];		<----- Therefore we should also restore [base+0] here.
	for (cnt = 1; cnt < maxsize / sizeof (long); cnt <<= 1) {
		addr = base + cnt;	/* pointer arith! */
		val = *addr;
		*addr = save[--i];
		if (val != ~cnt) {
			size = cnt * sizeof (long);
			/* Restore the original data before leaving the function.
			 */
			for (cnt <<= 1; cnt < maxsize / sizeof (long); cnt <<= 1) {
				addr  = base + cnt;
				*addr = save[--i];
			}
			return (size);
		}
	}

	return (maxsize);
}

best regards,
Po-Yu Chuang