* [U-Boot] [PATCH] memsize: get correct memory size @ 2009-11-18 5:02 Minkyu Kang 2009-11-23 7:04 ` Minkyu Kang 0 siblings, 1 reply; 7+ messages in thread From: Minkyu Kang @ 2009-11-18 5:02 UTC (permalink / raw) To: u-boot In some case, saved address and compared address are different. (e.g: 80M) So, it can be get wrong memory size. This patch fix the such problem, and fix style problems also. Signed-off-by: Minkyu Kang <mk7.kang@samsung.com> --- common/memsize.c | 77 +++++++++++++++++++++++++++++++++--------------------- 1 files changed, 47 insertions(+), 30 deletions(-) diff --git a/common/memsize.c b/common/memsize.c index 6c275c9..57c3cd7 100644 --- a/common/memsize.c +++ b/common/memsize.c @@ -44,51 +44,68 @@ long get_ram_size(volatile long *base, long maxsize) long cnt; long val; long size; + long max = maxsize / sizeof(long); int i = 0; - for (cnt = (maxsize / sizeof (long)) >> 1; cnt > 0; cnt >>= 1) { + for (cnt = 1; cnt < max; cnt <<= 1) { addr = base + cnt; /* pointer arith! */ - sync (); - save[i++] = *addr; - sync (); + sync(); + save[++i] = *addr; + sync(); *addr = ~cnt; } + addr = base + max - 1; + sync(); + save[++i] = *addr; + sync(); + *addr = ~max; + addr = base; - sync (); - save[i] = *addr; - sync (); + sync(); + save[0] = *addr; + sync(); *addr = 0; + sync(); - 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]; - } - return (0); + val = *addr; + if (val != 0) { + i = 0; + cnt = 1; + size = 0; + goto restore; } - for (cnt = 1; cnt < maxsize / sizeof (long); cnt <<= 1) { + i = 0; + for (cnt = 1; cnt < max; cnt <<= 1) { addr = base + cnt; /* pointer arith! */ val = *addr; - *addr = save[--i]; + *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); + size = cnt * sizeof(long); + cnt <<= 1; + goto restore; } } - return (maxsize); + addr = base + max - 1; + val = *addr; + *addr = save[++i]; + if (val != ~max) + maxsize = (cnt >> 1) * sizeof(long); + + return maxsize; + +restore: + /* Restore the original data before leaving the function. */ + for (; cnt < max; cnt <<= 1) { + addr = base + cnt; + sync(); + *addr = save[++i]; + } + addr = base; + sync(); + *addr = save[0]; + + return size; } -- 1.5.4.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] memsize: get correct memory size 2009-11-18 5:02 [U-Boot] [PATCH] memsize: get correct memory size Minkyu Kang @ 2009-11-23 7:04 ` Minkyu Kang 2009-11-23 18:37 ` Wolfgang Denk 0 siblings, 1 reply; 7+ messages in thread From: Minkyu Kang @ 2009-11-23 7:04 UTC (permalink / raw) To: u-boot Dear Wolfgang, 2009/11/18 Minkyu Kang <mk7.kang@samsung.com>: > In some case, saved address and compared address are different. (e.g: 80M) > So, it can be get wrong memory size. > This patch fix the such problem, and fix style problems also. > > Signed-off-by: Minkyu Kang <mk7.kang@samsung.com> > --- > ?common/memsize.c | ? 77 +++++++++++++++++++++++++++++++++--------------------- > ?1 files changed, 47 insertions(+), 30 deletions(-) > > diff --git a/common/memsize.c b/common/memsize.c > index 6c275c9..57c3cd7 100644 > --- a/common/memsize.c > +++ b/common/memsize.c > @@ -44,51 +44,68 @@ long get_ram_size(volatile long *base, long maxsize) > ? ? ? ?long ? ? ? ? ? cnt; > ? ? ? ?long ? ? ? ? ? val; > ? ? ? ?long ? ? ? ? ? size; > + ? ? ? long ? ? ? ? ? max = maxsize / sizeof(long); > ? ? ? ?int ? ? ? ? ? ?i = 0; > > - ? ? ? for (cnt = (maxsize / sizeof (long)) >> 1; cnt > 0; cnt >>= 1) { > + ? ? ? for (cnt = 1; cnt < max; cnt <<= 1) { > ? ? ? ? ? ? ? ?addr = base + cnt; ? ? ?/* pointer arith! */ > - ? ? ? ? ? ? ? sync (); > - ? ? ? ? ? ? ? save[i++] = *addr; > - ? ? ? ? ? ? ? sync (); > + ? ? ? ? ? ? ? sync(); > + ? ? ? ? ? ? ? save[++i] = *addr; > + ? ? ? ? ? ? ? sync(); > ? ? ? ? ? ? ? ?*addr = ~cnt; > ? ? ? ?} > > + ? ? ? addr = base + max - 1; > + ? ? ? sync(); > + ? ? ? save[++i] = *addr; > + ? ? ? sync(); > + ? ? ? *addr = ~max; > + > ? ? ? ?addr = base; > - ? ? ? sync (); > - ? ? ? save[i] = *addr; > - ? ? ? sync (); > + ? ? ? sync(); > + ? ? ? save[0] = *addr; > + ? ? ? sync(); > ? ? ? ?*addr = 0; > + ? ? ? sync(); > > - ? ? ? 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]; > - ? ? ? ? ? ? ? } > - ? ? ? ? ? ? ? return (0); > + ? ? ? val = *addr; > + ? ? ? if (val != 0) { > + ? ? ? ? ? ? ? i = 0; > + ? ? ? ? ? ? ? cnt = 1; > + ? ? ? ? ? ? ? size = 0; > + ? ? ? ? ? ? ? goto restore; > ? ? ? ?} > > - ? ? ? for (cnt = 1; cnt < maxsize / sizeof (long); cnt <<= 1) { > + ? ? ? i = 0; > + ? ? ? for (cnt = 1; cnt < max; cnt <<= 1) { > ? ? ? ? ? ? ? ?addr = base + cnt; ? ? ?/* pointer arith! */ > ? ? ? ? ? ? ? ?val = *addr; > - ? ? ? ? ? ? ? *addr = save[--i]; > + ? ? ? ? ? ? ? *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); > + ? ? ? ? ? ? ? ? ? ? ? size = cnt * sizeof(long); > + ? ? ? ? ? ? ? ? ? ? ? cnt <<= 1; > + ? ? ? ? ? ? ? ? ? ? ? goto restore; > ? ? ? ? ? ? ? ?} > ? ? ? ?} > > - ? ? ? return (maxsize); > + ? ? ? addr = base + max - 1; > + ? ? ? val = *addr; > + ? ? ? *addr = save[++i]; > + ? ? ? if (val != ~max) > + ? ? ? ? ? ? ? maxsize = (cnt >> 1) * sizeof(long); > + > + ? ? ? return maxsize; > + > +restore: > + ? ? ? /* Restore the original data before leaving the function. */ > + ? ? ? for (; cnt < max; cnt <<= 1) { > + ? ? ? ? ? ? ? addr = base + cnt; > + ? ? ? ? ? ? ? sync(); > + ? ? ? ? ? ? ? *addr = save[++i]; > + ? ? ? } > + ? ? ? addr = base; > + ? ? ? sync(); > + ? ? ? *addr = save[0]; > + > + ? ? ? return size; > ?} > -- > 1.5.4.3 > _______________________________________________ > U-Boot mailing list > U-Boot at lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot > Any comments? Thanks Minkyu Kang -- from. prom. www.promsoft.net ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] memsize: get correct memory size 2009-11-23 7:04 ` Minkyu Kang @ 2009-11-23 18:37 ` Wolfgang Denk 2009-11-24 4:40 ` Minkyu Kang 0 siblings, 1 reply; 7+ messages in thread From: Wolfgang Denk @ 2009-11-23 18:37 UTC (permalink / raw) To: u-boot Dear Minkyu Kang, In message <1f3430fb0911222304pa45554epc7e46c49a9cc70f7@mail.gmail.com> you wrote: > > > In some case, saved address and compared address are different. (e.g: 80M) > > So, it can be get wrong memory size. Well, could you please explain what "some cases" are, and how exactly the code is failing for you? And especially, what "80M" has to do with it? I guess you are aware that the code silently assumes that the size of any memory bank is always a power of two, aren't you? So a size of "80M" does not make any sense to me. If you have such a memory configuration, it is most probably assembled from two separate memory banks (64 + 16 MB), which of course need to be sized independently. > > This patch fix the such problem, and fix style problems also. ... > Any comments? Frankly, I don't understand which problem you're trying to fix, nor what your fix is supposed to be, nor your changes. I don't want to imply that your patch is bad, but I don't understand it. 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 Pig: An animal (Porcus omnivorous) closely allied to the human race by the splendor and vivacity of its appetite, which, however, is in- ferior in scope, for it balks at pig. - Ambrose Bierce ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] memsize: get correct memory size 2009-11-23 18:37 ` Wolfgang Denk @ 2009-11-24 4:40 ` Minkyu Kang 2009-11-24 22:08 ` Wolfgang Denk 0 siblings, 1 reply; 7+ messages in thread From: Minkyu Kang @ 2009-11-24 4:40 UTC (permalink / raw) To: u-boot Dear Wolfgang, 2009/11/24 Wolfgang Denk <wd@denx.de>: > Dear Minkyu Kang, > > In message <1f3430fb0911222304pa45554epc7e46c49a9cc70f7@mail.gmail.com> you wrote: >> >> > In some case, saved address and compared address are different. (e.g: 80M) >> > So, it can be get wrong memory size. > > Well, could you please explain what "some cases" are, and how exactly > the code is failing for you? And especially, what "80M" has to do with > it? If memory size is 80M, cnt is decrease 0xa00000 -> 0x500000 -> 0x280000 ... 0xa -> 0x5 -> 0x2 -> 0x1 when first for loop (line 49 to 55). So, save[2] is ~0x5. And second for loop(line 77 to 91), cnt is increase 0x1 -> 0x2 -> 0x4 -> 0x8 ... 0x1000000 -> 0x2000000 ... So, will be compare ~0x5 and ~0x4. That is problem. > > I guess you are aware that the code silently assumes that the size of > any memory bank is always a power of two, aren't you? So a size of > "80M" does not make any sense to me. If you have such a memory > configuration, it is most probably assembled from two separate memory > banks (64 + 16 MB), which of course need to be sized independently. > No, we use 1Gb OneDRAM, 80M is used by AP, 32M is used by CP and 16M is shared area. As you mentioned, size of memory bank is always a power of two, but, size of "available" memory is not always a power of two. > >> > This patch fix the such problem, and fix style problems also. > ... >> Any comments? > > Frankly, I don't understand which problem you're trying to fix, nor > what your fix is supposed to be, nor your changes. My patch is fixing wrong address comparison. > > I don't want to imply that your patch is bad, but I don't understand > it. > > 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 > Pig: An animal (Porcus omnivorous) closely allied to the ?human ?race > by ?the splendor and vivacity of its appetite, which, however, is in- > ferior in scope, for it balks at pig. ? ? ? ? ? ? ? ?- Ambrose Bierce > Thanks Minkyu Kang -- from. prom. www.promsoft.net ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] memsize: get correct memory size 2009-11-24 4:40 ` Minkyu Kang @ 2009-11-24 22:08 ` Wolfgang Denk 2009-11-25 2:00 ` Minkyu Kang 0 siblings, 1 reply; 7+ messages in thread From: Wolfgang Denk @ 2009-11-24 22:08 UTC (permalink / raw) To: u-boot Dear Minkyu Kang, In message <1f3430fb0911232040k19ca1362r23664ec62ab8d158@mail.gmail.com> you wrote: > > > Well, could you please explain what "some cases" are, and how exactly > > the code is failing for you? And especially, what "80M" has to do with > > it? > > If memory size is 80M, cnt is decrease 0xa00000 -> 0x500000 -> > 0x280000 ... 0xa -> 0x5 -> 0x2 -> 0x1 when first for loop (line 49 to > 55). But size of a memory bank cannot be 80 MB. It must always be a power of 2. > > I guess you are aware that the code silently assumes that the size of > > any memory bank is always a power of two, aren't you? So a size of > > "80M" does not make any sense to me. If you have such a memory > > configuration, it is most probably assembled from two separate memory > > banks (64 + 16 MB), which of course need to be sized independently. > > No, we use 1Gb OneDRAM, > 80M is used by AP, 32M is used by CP and 16M is shared area. > As you mentioned, size of memory bank is always a power of two, > but, size of "available" memory is not always a power of two. This doe snot make any sense to me. It seems you are misunderstanding or misinterpreting the purpose of this code. The purpose is to give it the maximum possible size of memory that may be populated in one bank of memory, and test how much of this is actually there. Say you have a board where one bank of memory can be populated with 256 MB, 512 MB or 1024 MB of RAM, then you will run the test with a max size of 1024 MB and find out which of the 3 versions you have - and if there are eventually any (gross) memory errors. I have no idea what you mean by "available memory", nor why you think you could use this function to test it. > > Frankly, I don't understand which problem you're trying to fix, nor > > what your fix is supposed to be, nor your changes. > > My patch is fixing wrong address comparison. This is caused by the fact that you misuse the code under conditions was not designed for. With a 80 MB region of memory, the whole theory it's function is based on does not apply. You want to use some different memory test, then (which one I don;t know, as I still fail to understand why you would want to "auto-size" 80 MB out of 1 GB of memory. This makes absolutely no sense to me. 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 backups: always in season, never out of style. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] memsize: get correct memory size 2009-11-24 22:08 ` Wolfgang Denk @ 2009-11-25 2:00 ` Minkyu Kang 2009-11-25 16:30 ` Wolfgang Denk 0 siblings, 1 reply; 7+ messages in thread From: Minkyu Kang @ 2009-11-25 2:00 UTC (permalink / raw) To: u-boot Dear Wolfgang, 2009/11/25 Wolfgang Denk <wd@denx.de>: > Dear Minkyu Kang, > > In message <1f3430fb0911232040k19ca1362r23664ec62ab8d158@mail.gmail.com> you wrote: >> >> > Well, could you please explain what "some cases" are, and how exactly >> > the code is failing for you? And especially, what "80M" has to do with >> > it? >> >> If memory size is 80M, cnt is decrease 0xa00000 -> 0x500000 -> >> 0x280000 ... 0xa -> 0x5 -> 0x2 -> 0x1 when first for loop (line 49 to >> 55). > > But size of a memory bank cannot be 80 MB. It must always be a power > of 2. As I said, memory bank is not 80MB. > >> > I guess you are aware that the code silently assumes that the size of >> > any memory bank is always a power of two, aren't you? So a size of >> > "80M" does not make any sense to me. If you have such a memory >> > configuration, it is most probably assembled from two separate memory >> > banks (64 + 16 MB), which of course need to be sized independently. >> >> No, we use 1Gb OneDRAM, >> 80M is used by AP, 32M is used by CP and 16M is shared area. >> As you mentioned, ?size of memory bank is always a power of two, >> but, size of "available" memory is not always a power of two. > > This doe snot make any sense to me. It seems you are misunderstanding > or misinterpreting the purpose of this code. > > The purpose is to give it the maximum possible size ?of ?memory ?that > may ?be populated in one bank of memory, and test how much of this is > actually there. Say you have a board where one bank of memory can ?be > populated ?with ?256 ?MB, 512 MB or 1024 MB of RAM, then you will run > the test with a max size of 1024 MB and ?find ?out ?which ?of ?the ?3 > versions ?you ?have ?- and if there are eventually any (gross) memory > errors. So, is maxsize "maximum possible size"? or "bank size"? If you mean "maximum possible size", as I mentioned.. 1Gb OneDRAM's maximum possible size is 80MB, 32MB is used by CP (can't access by AP) and 16MB is share area (used for special purposes). If you mean "bank size" then I got 64MB ram size. I don't want to get wrong size. And my patch support you purpose also. > > I have no idea what you mean by "available memory", nor why you think > you could use this function to test it. I don't want memory test. I just want to get memory size. When I posted SMDKC100 codes, you said "please use get_ram_size function". So, I tried it another board that uses OneDRAM. But, I can't get correct memory size. If you don't mind, I will not use this function if board uses OneDRAM. > >> > Frankly, I don't understand which problem you're trying to fix, nor >> > what your fix is supposed to be, nor your changes. >> >> My patch is fixing wrong address comparison. > > This is caused by the fact that you misuse the code under ?conditions > was not designed for. With a 80 MB region of memory, the whole theory > it's function is based on does not apply. You want to use some > different memory test, then (which one I don;t know, as I still fail > to understand why you would want to "auto-size" 80 MB out of 1 GB of > memory. This makes absolutely no sense to me. What means "auto-size"? 1GB is wrong. I said 1Gb (128 MB). > > 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 > backups: always in season, never out of style. > Thanks Minkyu Kang -- from. prom. www.promsoft.net ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] memsize: get correct memory size 2009-11-25 2:00 ` Minkyu Kang @ 2009-11-25 16:30 ` Wolfgang Denk 0 siblings, 0 replies; 7+ messages in thread From: Wolfgang Denk @ 2009-11-25 16:30 UTC (permalink / raw) To: u-boot Dear Minkyu Kang, In message <1f3430fb0911241800m5c9f4837kd90af1ab8fafd303@mail.gmail.com> you wrote: > > > But size of a memory bank cannot be 80 MB. It must always be a power > > of 2. > > As I said, memory bank is not 80MB. And as I said, get_ram_size() is designed to operate on a single bank of memory only. It works only as expected when "maxsize" is a power of 2, too (also note that it is restricted to 32 bit address space, too). > > The purpose is to give it the maximum possible size of memory that > > may be populated in one bank of memory, and test how much of this is > > actually there. Say you have a board where one bank of memory can be > > populated with 256 MB, 512 MB or 1024 MB of RAM, then you will run > > the test with a max size of 1024 MB and find out which of the 3 > > versions you have - and if there are eventually any (gross) memory > > errors. > > So, is maxsize "maximum possible size"? or "bank size"? maximum possible bank size. > If you mean "maximum possible size", as I mentioned.. 1Gb OneDRAM's > maximum possible size is 80MB, > 32MB is used by CP (can't access by AP) and 16MB is share area (used > for special purposes). > If you mean "bank size" then I got 64MB ram size. > I don't want to get wrong size. Could you please try to explain what this is supposed to mean? I have no idea what "AP" or "CP" might mean, or how this is related to U-Boot or to this task. Where is U-Boot running? If U-Boot can access 80 MB of memory, this is probably _two_ banks: one bank of 64 MB and _another_ bank of 16 MB. Which means you should test both of them, using two sepoarate calls to get_ram_size(). > I just want to get memory size. > When I posted SMDKC100 codes, you said "please use get_ram_size function". > So, I tried it another board that uses OneDRAM. > But, I can't get correct memory size. > If you don't mind, I will not use this function if board uses OneDRAM. Why not? Just use it properly, please. > > This is caused by the fact that you misuse the code under conditions > > was not designed for. With a 80 MB region of memory, the whole theory > > it's function is based on does not apply. You want to use some > > different memory test, then (which one I don;t know, as I still fail > > to understand why you would want to "auto-size" 80 MB out of 1 GB of > > memory. This makes absolutely no sense to me. > > What means "auto-size"? "auto-size" means to automatically detect the real, currently available (= populated) size of a bank of RAM. > 1GB is wrong. I said 1Gb (128 MB). But this is NOT the size of a bank of memory that can be accessed by the CPU, right? 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 Der Dativ ist dem Genitiv sein Tod. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-11-25 16:30 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-11-18 5:02 [U-Boot] [PATCH] memsize: get correct memory size Minkyu Kang 2009-11-23 7:04 ` Minkyu Kang 2009-11-23 18:37 ` Wolfgang Denk 2009-11-24 4:40 ` Minkyu Kang 2009-11-24 22:08 ` Wolfgang Denk 2009-11-25 2:00 ` Minkyu Kang 2009-11-25 16:30 ` Wolfgang Denk
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.