Re: [U-Boot] [PATCH] powerpc/83xx: increment malloc heap size for the MPC832x MDS boards

2012-06-15 Thread Kim Phillips
On Sat, 17 Mar 2012 17:44:00 -0500
Timur Tabi ti...@freescale.com wrote:

 The malloc buffer is not large enough to hold a flash sector (0x2 bytes)
 in addition to whatever else it normally holds, so double its size.  This
 fixes a failure trying to save the environment:
 
 = save
 Saving Environment to Flash...
 Unable to save the rest of sector (122880)
 . done
 Protected 1 sectors
 
 This problem probably surfaced from some other change that significantly
 increased the normal memory usage, thereby not leaving enough room for
 the saveenv command.
 
 Signed-off-by: Timur Tabi ti...@freescale.com
 ---

applied, Thanks.

Kim

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] powerpc/83xx: increment malloc heap size for the MPC832x MDS boards

2012-03-20 Thread Scott Wood
On 03/18/2012 04:07 AM, Wolfgang Denk wrote:
 Dear Tabi Timur-B04825,
 
 In message 
 CAOZdJXUem_-mgDSqF+p4R0npZQ6qu14G1yexQCBUe2G=nr2...@mail.gmail.com you 
 wrote:

 Doubling it is kind of aggressive strategy.  You know exactly how much
 free room needs to be guaranteed, so why don't you auto-adjust the
 size?

 -#define CONFIG_SYS_MALLOC_LEN(128 * 1024)/* Res=
 erved for malloc */
 +#define CONFIG_SYS_MALLOC_LEN(256 * 1024)/* Res=
 erved for malloc */

 How about:

 #define CONFIG_SYS_MALLOC_LEN   (128 * 1024 + CONFIG_ENV_SECT_SIZE)

 That's the same thing.  CONFIG_ENV_SECT_SIZE is 128KB.  Plus, as you
 
 No, it's not the same thing.

You really want CONFIG_SYS_MALLOC_LEN to be a list of everything that
could be mallocked -- or worse, a list of just the things that people
have noticed breaking in the past (even though other things could later
be depending on that increase in size)?  You want to answer questions
about why NAND breaks on a board with a smaller NOR sector size? :-P

 Your code is static an either wasted moemory on systems with smaller sector 
 size,

Come on, you're worried about a 128K increase in memory reserved for the
use of the bootloader?  It's not wasted.  The only thing that area
of memory could otherwise be used for is loading images and such, and it
is very unlikely that this small change is going to matter there.

 or needs to be readjusted on systems with bigger sectors.
 
 Don;t just think about your current situation, but also what happens
 when you (or somebody else) copies that code for other systems.
 
 Make your code more robust.

The robust thing to do would be to not be stingy with the malloc size,
and change all boards to have at least 1 MiB for malloc (except ones
with very small amounts of RAM).  Maybe have a debug command to print
max memory usage since boot, to see if a board is getting anywhere near
the limit.

-Scott

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] powerpc/83xx: increment malloc heap size for the MPC832x MDS boards

2012-03-20 Thread Wolfgang Denk
Dear Scott Wood,

In message 4f68cf30.8000...@freescale.com you wrote:

  Make your code more robust.
 
 The robust thing to do would be to not be stingy with the malloc size,
 and change all boards to have at least 1 MiB for malloc (except ones
 with very small amounts of RAM).  Maybe have a debug command to print
 max memory usage since boot, to see if a board is getting anywhere near
 the limit.

Good idea.  Patches are welcome, as usual.

I'm not sure it the malloc code keeps any such statics - if yes, and
this can be done without any significant amount of code, this could
even be added to the default bdinfo output.

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: w...@denx.de
Reader, suppose you were an idiot. And suppose you were a  member  of
Congress. But I repeat myself.   - Mark Twain
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] powerpc/83xx: increment malloc heap size for the MPC832x MDS boards

2012-03-18 Thread Wolfgang Denk
Dear Tabi Timur-B04825,

In message CAOZdJXUem_-mgDSqF+p4R0npZQ6qu14G1yexQCBUe2G=nr2...@mail.gmail.com 
you wrote:
 
  Doubling it is kind of aggressive strategy.  You know exactly how much
  free room needs to be guaranteed, so why don't you auto-adjust the
  size?
 
  -#define CONFIG_SYS_MALLOC_LEN(128 * 1024)/* Res=
 erved for malloc */
  +#define CONFIG_SYS_MALLOC_LEN(256 * 1024)/* Res=
 erved for malloc */
 
  How about:
 
  #define CONFIG_SYS_MALLOC_LEN   (128 * 1024 + CONFIG_ENV_SECT_SIZE)
 
 That's the same thing.  CONFIG_ENV_SECT_SIZE is 128KB.  Plus, as you

No, it's not the same thing.  Your code is static an either wasted
moemory on systems with smaller sector size, or needs to be readjusted
on systems with bigger sectors.

Don;t just think about your current situation, but also what happens
when you (or somebody else) copies that code for other systems.

Make your code more robust.

 realized, this will not work if CONFIG_SYS_RAMBOOT is defined.

I consider this a bug that needs fixing anyway.

  Yes, your board config file is kind of broken and will need more
  cleanup to facilitate this, but that should be done anyway.
 
 Well, I have no desire to do that.  I'm just fixing one bug.  A major
 cleanup of the board file is not on my to-do list.

Well, fixing this bug has uncovered another shortcoming or bug
(depending on point of view) of the code, so we should fix that one,
too.

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: w...@denx.de
The goal of science is to build better mousetraps. The goal of nature
is to build better mice.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] powerpc/83xx: increment malloc heap size for the MPC832x MDS boards

2012-03-17 Thread Timur Tabi
The malloc buffer is not large enough to hold a flash sector (0x2 bytes)
in addition to whatever else it normally holds, so double its size.  This
fixes a failure trying to save the environment:

= save
Saving Environment to Flash...
Unable to save the rest of sector (122880)
. done
Protected 1 sectors

This problem probably surfaced from some other change that significantly
increased the normal memory usage, thereby not leaving enough room for
the saveenv command.

Signed-off-by: Timur Tabi ti...@freescale.com
---
 include/configs/MPC832XEMDS.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/configs/MPC832XEMDS.h b/include/configs/MPC832XEMDS.h
index 4ed5a97..6f8622c 100644
--- a/include/configs/MPC832XEMDS.h
+++ b/include/configs/MPC832XEMDS.h
@@ -181,7 +181,7 @@
 
 /* CONFIG_SYS_MONITOR_LEN must be a multiple of CONFIG_ENV_SECT_SIZE */
 #define CONFIG_SYS_MONITOR_LEN (384 * 1024)/* Reserve 384 kB for Mon */
-#define CONFIG_SYS_MALLOC_LEN  (128 * 1024)/* Reserved for malloc */
+#define CONFIG_SYS_MALLOC_LEN  (256 * 1024)/* Reserved for malloc */
 
 /*
  * Initial RAM Base Address Setup
-- 
1.7.4.4


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] powerpc/83xx: increment malloc heap size for the MPC832x MDS boards

2012-03-17 Thread Wolfgang Denk
Dear Timur Tabi,

In message 1332024240-23286-1-git-send-email-ti...@freescale.com you wrote:
 The malloc buffer is not large enough to hold a flash sector (0x2 bytes)
 in addition to whatever else it normally holds, so double its size.  This
 fixes a failure trying to save the environment:

Doubling it is kind of aggressive strategy.  You know exactly how much
free room needs to be guaranteed, so why don't you auto-adjust the
size?

 -#define CONFIG_SYS_MALLOC_LEN(128 * 1024)/* Reserved for malloc 
 */
 +#define CONFIG_SYS_MALLOC_LEN(256 * 1024)/* Reserved for malloc 
 */

How about:

#define CONFIG_SYS_MALLOC_LEN   (128 * 1024 + CONFIG_ENV_SECT_SIZE)

?

Yes, your board config file is kind of broken and will need more
cleanup to facilitate this, but that should be done anyway.

Why exactly don't you support NOR flash for RAM-booting configurations?

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: w...@denx.de
The evolution of the human race will not be accomplished in  the  ten
thousand  years  of  tame  animals,  but in the million years of wild
animals, because man is and will always be a wild animal.
  - Charles Galton Darwin
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] powerpc/83xx: increment malloc heap size for the MPC832x MDS boards

2012-03-17 Thread Tabi Timur-B04825
On Sat, Mar 17, 2012 at 6:19 PM, Wolfgang Denk w...@denx.de wrote:

 Doubling it is kind of aggressive strategy.  You know exactly how much
 free room needs to be guaranteed, so why don't you auto-adjust the
 size?

 -#define CONFIG_SYS_MALLOC_LEN        (128 * 1024)    /* Reserved for malloc 
 */
 +#define CONFIG_SYS_MALLOC_LEN        (256 * 1024)    /* Reserved for malloc 
 */

 How about:

 #define CONFIG_SYS_MALLOC_LEN   (128 * 1024 + CONFIG_ENV_SECT_SIZE)

That's the same thing.  CONFIG_ENV_SECT_SIZE is 128KB.  Plus, as you
realized, this will not work if CONFIG_SYS_RAMBOOT is defined.

 Yes, your board config file is kind of broken and will need more
 cleanup to facilitate this, but that should be done anyway.

Well, I have no desire to do that.  I'm just fixing one bug.  A major
cleanup of the board file is not on my to-do list.

 Why exactly don't you support NOR flash for RAM-booting configurations?

I have no idea.  I just noticed this one bug.  I don't support the board.

-- 
Timur Tabi
Linux kernel developer at Freescale
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot