Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Marek Vasut]

Dear Deepak Saxena,

 commit 341764495180a712b9aaccfa0479b2ff7e44e35b
 Author: Deepak Saxena deepak_sax...@mentor.com
 Date:   Mon Dec 6 15:52:07 2010 -0800
 
  Honor /memory/reg node in DTB files
 
  This patch adds code to the bootm path to check if a valid
  /memory/reg node exists in the DTB file and if so, it
  does not override it with the values in bi_memstart and
  bi_memsize. This is particularly useful in multi-core
  environments where the memory may be partitioned across
  a large number of nodes.
 
  While the same can be accomplished on certain boards (p1022ds
  and p1_p2_rdb) by using the bootm_low and bootm_size
  environment variables, that solution is not universal and
  requires adding code ft_board_setup() for any new board
  that wants to support AMP operation. Also, given that the
  DTB is already  used to partition board devices (see commit
  dc2e673 in the Linux kernel tree), it makes sense to
  allow memory to be partitioned the same way from a user
  configuration perspective.
 
  This patch allows for the user to override the DTB file parameters
  on the p1022ds and p1_p2_rdb boards by setting the bootm_low and
  bootm_size to something other than bi_memstart and bi_memsize.
  In the long-term, those env variables should be depecrated and
  removed and system implementors should provide the memory
  partitioning information in the DTB.
 
  Signed-off-by: Deepak Saxena deepak_sax...@mentor.com
  Signed-off-by: Hollis Blanchard hollis_blanch...@mentor.com
 
 ---
 
 See http://lists.denx.de/pipermail/u-boot/2010-December/083057.html
 for initial proposal on this.
 
 diff --git a/arch/powerpc/cpu/mpc85xx/fdt.c
 b/arch/powerpc/cpu/mpc85xx/fdt.c index 4540364..6d384e3 100644
 --- a/arch/powerpc/cpu/mpc85xx/fdt.c
 +++ b/arch/powerpc/cpu/mpc85xx/fdt.c
 @@ -377,6 +377,55 @@ static void ft_fixup_qe_snum(void *blob)
   }
   #endif
 
 +/*
 + * Check to see if an valid memory/reg property exists
 + * in the fdt. If so, we do not overwrite it with what's
 + * been scanned.
 + *
 + * Valid mean all the following:
 + *
 + * - Memory node has a device-type of memory
 + * - A reg property exists which:
 + *   + has exactly as many cells as #address-cells + #size-cells
 + *   + provides a range that is within [bi_memstart, bi_memstart +
 bi_memsize]
 + */
 +static int ft_validate_memory(void *blob, bd_t *bd)
 +{
 + int nodeoffset;
 + u32 *addrcell = (u32*)fdt_getprop(blob, 0, #address-cells, NULL);
 + u32 *sizecell = (u32*)fdt_getprop(blob, 0, #size-cells, NULL);
 + u64 reg_base, reg_size;
 + void *reg, *dtype;
 + int len;
 +
 + if ((nodeoffset = fdt_path_offset(blob, /memory)) = 0)
 + {
 + dtype = fdt_getprop(blob, nodeoffset, device_type, len);
 + if (!dtype || (strcmp(dtype, memory) != 0))
 + return 0;
 +
 + reg = fdt_getprop(blob, nodeoffset, reg, len);
 + if (reg  len == ((*addrcell + *sizecell) * 4)) {
 + if (*addrcell == 2) {
 + reg_base = ((u64*)reg)[0];
 + reg_size = ((u64*)reg)[1];
 + } else {
 + reg_base = ((u32*)reg)[0];
 + reg_size = ((u32*)reg)[1];
 + }
 +
 + if ((reg_size) 
 + (reg_base = (u64)bd-bi_memstart) 
 + ((reg_size + reg_base)
 +  = ((u64)bd-bi_memstart + (u64)bd-bi_memsize)))
 + return 1;
 + }
 + }
 +
 + return 0;
 +
 +}
 +
   void ft_cpu_setup(void *blob, bd_t *bd)
   {
   int off;
 @@ -434,7 +483,8 @@ void ft_cpu_setup(void *blob, bd_t *bd)
   clock-frequency, CONFIG_SYS_CLK_FREQ, 1);
   #endif
 
 - fdt_fixup_memory(blob, (u64)bd-bi_memstart, (u64)bd-bi_memsize);
 + if (!ft_validate_memory(blob, bd))
 + fdt_fixup_memory(blob, (u64)bd-bi_memstart, (u64)bd-
bi_memsize);
 
   #ifdef CONFIG_MP
   ft_fixup_cpu(blob, (u64)bd-bi_memstart + (u64)bd-bi_memsize);
 diff --git a/board/freescale/p1022ds/p1022ds.c
 b/board/freescale/p1022ds/p1022ds.c
 index 5cdee9f..7378d88 100644
 --- a/board/freescale/p1022ds/p1022ds.c
 +++ b/board/freescale/p1022ds/p1022ds.c
 @@ -320,7 +320,8 @@ void ft_board_setup(void *blob, bd_t *bd)
   base = getenv_bootm_low();
   size = getenv_bootm_size();
 
 - fdt_fixup_memory(blob, (u64)base, (u64)size);
 + if (base != (phys_addr_t)bd-bi_memstart  size !=
 (phys_addr_t)bd-bi_memsize)
 + fdt_fixup_memory(blob, (u64)base, (u64)size);
 
   FT_FSL_PCI_SETUP;
 
 diff --git a/board/freescale/p1_p2_rdb/p1_p2_rdb.c
 b/board/freescale/p1_p2_rdb/p1_p2_rdb.c
 index fae31f2..5e4adc6 100644
 --- a/board/freescale/p1_p2_rdb/p1_p2_rdb.c
 +++ b/board/freescale/p1_p2_rdb/p1_p2_rdb.c
 @@ -220,9 +220,10 @@ 

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Wolfgang Denk]

Dear Deepak Saxena,

In message 4d026bb2.6020...@mentor.com you wrote:
 On 12/08/2010 02:34 PM, Wolfgang Denk wrote:
 
  
  I guess we can argue that the normal situation is that U-Boot will
  know how to update the DT such as needed to boot the OS. So what we
  are dealing with is a small percentage of cases where we need special
  behaviour, and where it may be acceptable if the solution is only
  semi-perfect ;-)
  
  My current thinking is to introduce something like
  
  dt_skip=memory,mac-address
  
  including eventually dt_skip=ALL.  This should cover most of the
  current use cases.
  
  If someone gets fancy he can add wildcard support.
  
  And if we need even more flexibility, we can add some dt_include
  with higher priority, so one could do for example
  
  dt_skip=ALL
  dt_include=memory
 
 I imagine this being rather ugly to implement and to keep the code clean
 and maintained. Who parses these variables? Does each and every piece of
 code in U-Boot that now touches a piece of the DT need to check for this
 variable? I could see something like this working
 if there was a central DT handler that read nodes and then called
 platform-specific over-ride function, i.e.:
 
   for_each_node_in_dt() {
   if (dt_include(node-type))
   platform_of_dt_node_process(node, boot_stage);
   }
 
   where boot_stage tells us whether we are at early init, about to
 boot an OS image, or in some other step in the process.
 
 This would provide a consistent method of handling that variable.
 Without something like this, I think and environment variable is just
 going to create confusion for users and developers.

You just described what the implementation should look like.

Thanks.

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
I know engineers. They love to change things. - Dr. McCoy
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Deepak Saxena]

On 12/08/2010 02:34 PM, Wolfgang Denk wrote:

 
 I guess we can argue that the normal situation is that U-Boot will
 know how to update the DT such as needed to boot the OS. So what we
 are dealing with is a small percentage of cases where we need special
 behaviour, and where it may be acceptable if the solution is only
 semi-perfect ;-)
 
 My current thinking is to introduce something like
 
   dt_skip=memory,mac-address
 
 including eventually dt_skip=ALL.  This should cover most of the
 current use cases.
 
 If someone gets fancy he can add wildcard support.
 
 And if we need even more flexibility, we can add some dt_include
 with higher priority, so one could do for example
 
   dt_skip=ALL
   dt_include=memory

I imagine this being rather ugly to implement and to keep the code clean
and maintained. Who parses these variables? Does each and every piece of
code in U-Boot that now touches a piece of the DT need to check for this
variable? I could see something like this working
if there was a central DT handler that read nodes and then called
platform-specific over-ride function, i.e.:

for_each_node_in_dt() {
if (dt_include(node-type))
platform_of_dt_node_process(node, boot_stage);
}

where boot_stage tells us whether we are at early init, about to
boot an OS image, or in some other step in the process.

This would provide a consistent method of handling that variable.
Without something like this, I think and environment variable is just
going to create confusion for users and developers.

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

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Wolfgang Denk]

Dear Dan,

In message 750641c9-dc97-4923-b337-05a2f1bc9...@digitaldans.com you wrote:
 
 Yes, I'm sometimes pleased  :-)

Good :-)

  My current thinking is to introduce something like  .
 
 Well, that is pretty cool.
 
  dt_skip=memory,mac-address
 
 Do we have to write a parser now, or is there something
 that currently exists to help out? :-)

We use the same format already to implement the hwconfig feature. It
just needs to be moved to common code.

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 only way you could make a happy  marriage  is  by  cuttin'  their
heads  off  as  soon  as  they say `I do', yes? You can't make happi-
ness...   - Terry Pratchett, _Witches Abroad_
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Hollis Blanchard]

On 12/07/2010 11:09 AM, Wolfgang Denk wrote:
 There are many board vendors who shipt boards with different
 configurations - with or without NAND flash; with or without other
 peripherals like CAN contollers, LCD, etc.; with different LCD sizes
 and types, in portrait or landscape orientation, etc.  Some of these
 features can be determined by probing the hardware, others (like the
 orientation of a LCD) cannot.  It is sometimes a maintenance nightmare
 to provide tens of different configurations of U-Boot for a single
 product.  Being able to cinfigure available hardware through the DT,
 and using a single common binary image of U-Boot for such systems
 would be a great benefit.
That's fine, but so far I don't see how it's related. This is 
information u-boot needs during its own initialization, right?

We need a way for our tools to specify information to the kernels' 
initialization, and still want u-boot to do all the hardware 
configuration it does today. It really doesn't matter to us if in the 
future u-boot uses device trees for that configuration: we just need a 
way to interact with the kernels.

Hollis Blanchard
Mentor Graphics, Embedded Systems Division


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

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Deepak Saxena]

On 12/07/2010 11:09 AM, Wolfgang Denk wrote:
 So far we usually had pretty static board configurations, and a static
 compile time description was all we needed.  Some developers consider
 even simple extensions like auto-sizing the available RAM as
 unnecessary luxury that just inreases the boot time and memory
 footprint.
 
 When it comes to more complicated setups we should provide means for a
 more dynamic configuration - this has been discussed before, and there
 was a general agreement that the device tree would be a usable way to
 implement such a description of the hardware.
 
 So what I'm proposing is not an opposite to what you have in mind, it
 just takes it one step further, and makes the whole system consistent
 again.
 
 Waht I don't like is the tunneling of information through U-Boot,
 while U-Boot actually needs and uses this very information as well.

I won't argue with you on this front. Having U-Boot determine some board
information from the DT and determine other board information at
run-time is not consistent and confusing.

 While we could provide a method to provide this information at build
 time to make U-Boot, this forces a static memory partitioning on the 
 system and does not mesh well with use cases where developers may
 
 This is NOT what I suggested.

Thanks for clarifying that. You had stated pass make U-Boot process
this information and I read that as in passing the information to
the build (make) process.

 In the multi-node environments, we can't simply tell U-Boot to process 
 the DTB to determine how much memory is in the system as there is one 
 DTB per AMP node. One idea that comes to mind but that I think is not
 
 Please explain: you can use the DT to tell Linux (or other OS) how
 much memory they shoulduse, but you cannot use the same mechanism to
 pass the same information to U-Boot?

I'm not against U-Boot using this information, I'm just not sure how to
do this without adding quite a bit of complexity to the code base. We
would have to have U-Boot parse the memory nodes, validate them, check
for overlapping regions, check for holes, etc. I'm not arguing that it
is not doable, but wondering if adding this complexity is worth if the
scanning of memory and passing that information to the kernel works for
the majority of use cases. What I'm trying to do is support a special
use case, so what about  wrapping support for simply passing the memory
nodes from the DT to the kernel around a CONFIG option
(CONFIG_OF_MEMORY_PASSTHROUGH?) that can be enabled by system
implementers who need this and are running on fairly controlled
environments while the larger issue of how to use the DT is hashed out?

 the right way to go due to complexity is to have the concept of
 nested DTBs that can define multiple operational machines and
 U-Boot knows how to read this and send the right sub-DTB to the
 right kernel image.
 
 This is a technical details that can and should be discussed when we
 have an agreement about the basic mechanism.
 
 I'm new to U-Boot development so would like to hear about the other use 
 cases you mentioned (pointer to email threads are OK) so I can have a 
 better understanding of the overall issues.
 
 There are many board vendors who shipt boards with different
 configurations - with or without NAND flash; with or without other
 peripherals like CAN contollers, LCD, etc.; with different LCD sizes
 and types, in portrait or landscape orientation, etc.  Some of these
 features can be determined by probing the hardware, others (like the
 orientation of a LCD) cannot.  It is sometimes a maintenance nightmare
 to provide tens of different configurations of U-Boot for a single
 product.  Being able to cinfigure available hardware through the DT,
 and using a single common binary image of U-Boot for such systems
 would be a great benefit.

I completely understand your perspective here and agree with it. DT has
made the kernel easier to maintain across multiple similar platforms so
it makes sense to leverage the same technology in U-Boot.

 Partitioning for AMP operation is about managing what and how is running
 on top of the bootloader. How much knowledge should the bootloader have
 about this? The approach of providing the memory partitioning in the DTB 
 basically removes any of this knowledge from U-Boot, while the
 
 I see many use cases where this is simply not possible, because you
 need need the help of the bootloader to determine parameters that are
 not known in advance, and that thus cannot be encoded in the DT.
 Memory size if a very typical example for such a parameter.  It may be
 OK for the use case you have currently in mind to use a fixed size,
 but this covers just a few systems and is not flexible enough for
 general use.

Again, I understand this, though I am not 100% sure there is a way to do
this in a completely generic way off the top of my head. There are use
cases where we must rely on U-Boot to scan and determine memory size and
there 

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Deepak Saxena]

On 12/07/2010 01:22 PM, Scott Wood wrote:
 On Mon, 6 Dec 2010 16:56:26 -0800
 Deepak Saxena deepak_sax...@mentor.com wrote:
 
 +/*
 + * Check to see if an valid memory/reg property exists
 + * in the fdt. If so, we do not overwrite it with what's
 + * been scanned.
 + *
 + * Valid mean all the following:
 + *
 + * - Memory node has a device-type of memory
 + * - A reg property exists which:
 + *   + has exactly as many cells as #address-cells + #size-cells
 + *   + provides a range that is within [bi_memstart, bi_memstart + 
 bi_memsize]
 + */
 
 This will get false positives -- a lot of existing device tree
 templates have something like this:

ACK. The code in the patch actually checks for this, just didn't point
it out in the comments.

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

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Scott Wood]

On Wed, 8 Dec 2010 10:59:44 -0800
Deepak Saxena deepak_sax...@mentor.com wrote:

 On 12/07/2010 01:22 PM, Scott Wood wrote:
  On Mon, 6 Dec 2010 16:56:26 -0800
  Deepak Saxena deepak_sax...@mentor.com wrote:
  
  +/*
  + * Check to see if an valid memory/reg property exists
  + * in the fdt. If so, we do not overwrite it with what's
  + * been scanned.
  + *
  + * Valid mean all the following:
  + *
  + * - Memory node has a device-type of memory
  + * - A reg property exists which:
  + *   + has exactly as many cells as #address-cells + #size-cells
  + *   + provides a range that is within [bi_memstart, bi_memstart + 
  bi_memsize]
  + */
  
  This will get false positives -- a lot of existing device tree
  templates have something like this:
 
 ACK. The code in the patch actually checks for this, just didn't point
 it out in the comments.

Ah, I looked for that and missed it.  But there are also some boards
that have socketed RAM, but for some reason have a real memory size in
the device tree (corresponding to what the board ships with,
probably).  I think this is something that should have to be selectable
by the board config (for image size reasons as well).

Also some nits:

 +static int ft_validate_memory(void *blob, bd_t *bd)
 +{
 + int nodeoffset;
 + u32 *addrcell = (u32*)fdt_getprop(blob, 0, #address-cells, NULL);
 + u32 *sizecell = (u32*)fdt_getprop(blob, 0, #size-cells, NULL);

const u32 *, and eliminate the cast.

 + u64 reg_base, reg_size;
 + void *reg, *dtype;
 + int len;
 +
 + if ((nodeoffset = fdt_path_offset(blob, /memory)) = 0)
 + {

Brace on same line as if

Don't put the assignment inside the if statement.

 + dtype = fdt_getprop(blob, nodeoffset, device_type, len);
 + if (!dtype || (strcmp(dtype, memory) != 0))
 + return 0;
 +
 + reg = fdt_getprop(blob, nodeoffset, reg, len);
 + if (reg  len == ((*addrcell + *sizecell) * 4)) {
 + if (*addrcell == 2) {
 + reg_base = ((u64*)reg)[0];
 + reg_size = ((u64*)reg)[1];
 + } else {
 + reg_base = ((u32*)reg)[0];
 + reg_size = ((u32*)reg)[1];
 + }

This only works on big-endian.

 + if ((reg_size) 
 + (reg_base = (u64)bd-bi_memstart) 
 + ((reg_size + reg_base)
 +  = ((u64)bd-bi_memstart + (u64)bd-bi_memsize)))
 + return 1;   
 + }

Probably want to complain to the user if reg is invalid and not zero/missing.

-Scott

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

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Dan Malek]

On Dec 8, 2010, at 11:11 AM, Scott Wood wrote:

 Probably want to complain to the user if reg is invalid and not  
 zero/missing.

I think you guys are making this too complicated.
There are many ways to pass stupid mistakes via
a device tree, don't get carried away trying to single
out this one for error checking where the user is likely
to really know what they are doing.  This isn't a required
specification to get correct, without anything u-boot
will provide the proper information.

Thanks.

-- Dan

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

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Scott Wood]

On Wed, 8 Dec 2010 11:22:59 -0800
Dan Malek ppc6...@digitaldans.com wrote:

 
 On Dec 8, 2010, at 11:11 AM, Scott Wood wrote:
 
  Probably want to complain to the user if reg is invalid and not  
  zero/missing.
 
 I think you guys are making this too complicated.
 There are many ways to pass stupid mistakes via
 a device tree, don't get carried away trying to single
 out this one for error checking where the user is likely
 to really know what they are doing.  This isn't a required
 specification to get correct, without anything u-boot
 will provide the proper information.

I don't think that's what this is for.  I think it's for AMP scenarios
where you want to carve up memory between guests -- in which case you
want U-Boot to not overwrite the device tree with its own idea of how
much memory is present.

This patch was trying to guess whether that's the case based on whether
the existing value looks reasonable, but I think it has to be an
explicit configuration (board config, environment variable, etc).  Once
that is done, then the test should either go away, or complain -- don't
just silently do something different if it's been told that the memory
node is supposed to be complete and correct.

As for the merits of error checking in general, obviously it's not
going to be complete.  But it can be useful to check for some common
errors, such as when a board has multiple DTSes and the user has to
pick the right one for how U-Boot has been configured.  Maybe
granting an AMP partition a memory region beyond the bounds of detected
memory is another such case.

-Scott

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

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Wolfgang Denk]

Dear Hollis Blanchard,

In message 4cffcec1.6000...@mentor.com you wrote:
 On 12/07/2010 11:09 AM, Wolfgang Denk wrote:
  There are many board vendors who shipt boards with different
  configurations - with or without NAND flash; with or without other
  peripherals like CAN contollers, LCD, etc.; with different LCD sizes
  and types, in portrait or landscape orientation, etc.  Some of these
  features can be determined by probing the hardware, others (like the
  orientation of a LCD) cannot.  It is sometimes a maintenance nightmare
  to provide tens of different configurations of U-Boot for a single
  product.  Being able to cinfigure available hardware through the DT,
  and using a single common binary image of U-Boot for such systems
  would be a great benefit.
 That's fine, but so far I don't see how it's related. This is 
 information u-boot needs during its own initialization, right?

Right.

 We need a way for our tools to specify information to the kernels' 
 initialization, and still want u-boot to do all the hardware 
 configuration it does today. It really doesn't matter to us if in the 
 future u-boot uses device trees for that configuration: we just need a 
 way to interact with the kernels.

When U-boot is supposed to do the hardware initialization, you
probably include memory initialization, right?  If so, should we then
not make sure that U-Boot and the OS we're booting use the same
information about this resource?

If, on the other hand, you really want to make U-Boot ignore any
/memory nodes in the device tree, then this should be done straight
and without additional magic.  In this case the DT should be
responsible to provide all information the OS needs, and U-Boot should
not touch this in any way.  Then just do not call fdt_fixup_memory()
at all for such configurations.

I dislike the idea that U-Boot will not touch the DT entry in one
situation, but will do so in another, without any visibility to (and
eventually without awareness by) the user.

If there is really a good reason for such magic, then this should be
clearly documented (not only in some comment in some source file),
and eventually fdt_fixup_memory() (or fdt_fixup_memory_banks() ?)
should be made weak so it can be redefined by board-specific
implementations.

In any case, any such changes should be implemented in a generic way,
i. e. not only for one specific processor family.

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
Conceptual integrity in turn dictates that the  design  must  proceed
from  one  mind,  or  from  a  very small number of agreeing resonant
minds.   - Frederick Brooks Jr., The Mythical Man Month
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Wolfgang Denk]

Dear Deepak Saxena,

In message 4cffd57c.1010...@mentor.com you wrote:

  Please explain: you can use the DT to tell Linux (or other OS) how
  much memory they shoulduse, but you cannot use the same mechanism to
  pass the same information to U-Boot?
 
 I'm not against U-Boot using this information, I'm just not sure how to
 do this without adding quite a bit of complexity to the code base. We
 would have to have U-Boot parse the memory nodes, validate them, check
 for overlapping regions, check for holes, etc. I'm not arguing that it
 is not doable, but wondering if adding this complexity is worth if the
 scanning of memory and passing that information to the kernel works for
 the majority of use cases. What I'm trying to do is support a special
 use case, so what about  wrapping support for simply passing the memory
 nodes from the DT to the kernel around a CONFIG option
 (CONFIG_OF_MEMORY_PASSTHROUGH?) that can be enabled by system
 implementers who need this and are running on fairly controlled
 environments while the larger issue of how to use the DT is hashed out?

See my previous message to Hollis.  If you really want U-Boot to keep
it's fingers off the /memory nodes in the DT, then simply do that. But
then please be consequent, add it for all architectures, and if
enabled, then without magic where U-Boot will sometimes to this and
other times do that.  And provide documentation to the end user.

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
You can observe a lot just by watchin'.  - Yogi Berra
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Hollis Blanchard]

On 12/08/2010 12:53 PM, Wolfgang Denk wrote:
 Dear Hollis Blanchard,

 In message4cffcec1.6000...@mentor.com  you wrote:
 On 12/07/2010 11:09 AM, Wolfgang Denk wrote:
 There are many board vendors who shipt boards with different
 configurations - with or without NAND flash; with or without other
 peripherals like CAN contollers, LCD, etc.; with different LCD sizes
 and types, in portrait or landscape orientation, etc.  Some of these
 features can be determined by probing the hardware, others (like the
 orientation of a LCD) cannot.  It is sometimes a maintenance nightmare
 to provide tens of different configurations of U-Boot for a single
 product.  Being able to cinfigure available hardware through the DT,
 and using a single common binary image of U-Boot for such systems
 would be a great benefit.
 That's fine, but so far I don't see how it's related. This is
 information u-boot needs during its own initialization, right?
 Right.

 We need a way for our tools to specify information to the kernels'
 initialization, and still want u-boot to do all the hardware
 configuration it does today. It really doesn't matter to us if in the
 future u-boot uses device trees for that configuration: we just need a
 way to interact with the kernels.
 When U-boot is supposed to do the hardware initialization, you
 probably include memory initialization, right?  If so, should we then
 not make sure that U-Boot and the OS we're booting use the same
 information about this resource?
Yes, I do include memory initialization. In our use case, u-boot must 
know about all memory (to configure the memory controller), but each OS 
is only made aware of a piece of it. They really do have different 
information about the resource.
 If, on the other hand, you really want to make U-Boot ignore any
 /memory nodes in the device tree, then this should be done straight
 and without additional magic.  In this case the DT should be
 responsible to provide all information the OS needs, and U-Boot should
 not touch this in any way.  Then just do not call fdt_fixup_memory()
 at all for such configurations.
I think the current way that u-boot updates the memory node is valuable 
for other use cases. In particular, it is very convenient for single-OS 
systems. Our goal is to avoid affecting those use cases.
 I dislike the idea that U-Boot will not touch the DT entry in one
 situation, but will do so in another, without any visibility to (and
 eventually without awareness by) the user.
I see it as allowing the user to optionally override auto-detected 
information. The user has to go out of their way to request this 
behavior, so I think the visibility/awareness is there.

The existing bootm_low/bootm_size facility does exactly this, but I 
think we can improve on its design.
 If there is really a good reason for such magic, then this should be
 clearly documented (not only in some comment in some source file),
 and eventually fdt_fixup_memory() (or fdt_fixup_memory_banks() ?)
 should be made weak so it can be redefined by board-specific
 implementations.

 In any case, any such changes should be implemented in a generic way,
 i. e. not only for one specific processor family.
I agree that all (device tree aware) systems should behave consistently 
in this regard. Of course, in that case, we don't need weak functions?

Documenting the behavior is a very good point.

Hollis Blanchard
Mentor Graphics, Embedded Systems Division


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

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Wolfgang Denk]

Dear Hollis,

In message 4cfff3c4.20...@mentor.com you wrote:

 I think the current way that u-boot updates the memory node is valuable 
 for other use cases. In particular, it is very convenient for single-OS 
 systems. Our goal is to avoid affecting those use cases.
  I dislike the idea that U-Boot will not touch the DT entry in one
  situation, but will do so in another, without any visibility to (and
  eventually without awareness by) the user.
 I see it as allowing the user to optionally override auto-detected 
 information. The user has to go out of their way to request this 
 behavior, so I think the visibility/awareness is there.

OK - but then the user should really be requested to select the
beviour.  This should be done in an explicit and documented way, ant
not based on some magic properties of the DT.

 The existing bootm_low/bootm_size facility does exactly this, but I 
 think we can improve on its design.

I have to admit that I also dislike the bootm_low / bootm_size stuff
as it's really confusing to users, especially as the difference
between bootm_size and CONFIG_SYS_BOOTMAPSZ (and BOOTMAPSZ, which is
sometimes used instead) is not really clear.

 I agree that all (device tree aware) systems should behave consistently 
 in this regard. Of course, in that case, we don't need weak functions?

If you want to make this switchable at runtime, then we should
probably use an environment setting.

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
Data is a lot like humans: It is  born.  Matures.  Gets  married  to
other  data, divorced. Gets old. One thing that it doesn't do is die.
It has to be killed. - Arthur Miller
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Dan Malek]

Hi Wolfgang.

On Dec 8, 2010, at 1:38 PM, Wolfgang Denk wrote:

 If you want to make this switchable at runtime, then we should
 probably use an environment setting.

I experimented with this, but could never determine the
best way to cover all behavior.  Do we have a variable that
indicates don't update DT?  If so, it means we have to
place all values in the DT, when it's really nice for U-Boot
to do some of that for us.  In fact, we want U-Boot to update
many things it discovers, just not the few we wish to actually
(sometimes) define for ourselves.

The other alternative (granted, I'm not as smart as I used
to be :-)) was to have an environment variable that specified
which node we didn't want updated by U-Boot.  For now,
that seems reasonable since there is only one, but is that
the general approach we want in the future?  It also presents
the challenge of having to update both environment and
the provided DT.

I just wanted to make these points, as I did try some alternatives
but never found anything acceptable.  By looking at key values
in the DT, at least it was confined to one place.

This feature is needed, so I propose we let it exist in the
form we have found useful and let it evolve over time as
others gain some experience.

Thanks.

-- Dan

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

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Wolfgang Denk]

Dear Dan,

In message 0ddcbda1-188f-433d-bdcc-5fdcf709a...@digitaldans.com you wrote:
 
  If you want to make this switchable at runtime, then we should
  probably use an environment setting.
 
 I experimented with this, but could never determine the
 best way to cover all behavior.  Do we have a variable that
 indicates don't update DT?  If so, it means we have to
 place all values in the DT, when it's really nice for U-Boot
 to do some of that for us.  In fact, we want U-Boot to update
 many things it discovers, just not the few we wish to actually
 (sometimes) define for ourselves.

You can please all the people some of the time and some of the people
all of the time but you can't please all the people all of the time.

 The other alternative (granted, I'm not as smart as I used
 to be :-)) was to have an environment variable that specified
 which node we didn't want updated by U-Boot.  For now,
 that seems reasonable since there is only one, but is that
 the general approach we want in the future?  It also presents
 the challenge of having to update both environment and
 the provided DT.

I guess we can argue that the normal situation is that U-Boot will
know how to update the DT such as needed to boot the OS. So what we
are dealing with is a small percentage of cases where we need special
behaviour, and where it may be acceptable if the solution is only
semi-perfect ;-)

My current thinking is to introduce something like

dt_skip=memory,mac-address

including eventually dt_skip=ALL.  This should cover most of the
current use cases.

If someone gets fancy he can add wildcard support.

And if we need even more flexibility, we can add some dt_include
with higher priority, so one could do for example

dt_skip=ALL
dt_include=memory

to have only the memory node updated.

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
If you can't beat it or corrupt it, you pretend it was your  idea  in
the first place. - Terry Pratchett, _Guards! Guards!_
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Peter Tyser]

On Wed, 2010-12-08 at 23:34 +0100, Wolfgang Denk wrote:
 Dear Dan,
 
 In message 0ddcbda1-188f-433d-bdcc-5fdcf709a...@digitaldans.com you wrote:
  
   If you want to make this switchable at runtime, then we should
   probably use an environment setting.
  
  I experimented with this, but could never determine the
  best way to cover all behavior.  Do we have a variable that
  indicates don't update DT?  If so, it means we have to
  place all values in the DT, when it's really nice for U-Boot
  to do some of that for us.  In fact, we want U-Boot to update
  many things it discovers, just not the few we wish to actually
  (sometimes) define for ourselves.
 
 You can please all the people some of the time and some of the people
 all of the time but you can't please all the people all of the time.
 
  The other alternative (granted, I'm not as smart as I used
  to be :-)) was to have an environment variable that specified
  which node we didn't want updated by U-Boot.  For now,
  that seems reasonable since there is only one, but is that
  the general approach we want in the future?  It also presents
  the challenge of having to update both environment and
  the provided DT.
 
 I guess we can argue that the normal situation is that U-Boot will
 know how to update the DT such as needed to boot the OS. So what we
 are dealing with is a small percentage of cases where we need special
 behaviour, and where it may be acceptable if the solution is only
 semi-perfect ;-)
 
 My current thinking is to introduce something like
 
   dt_skip=memory,mac-address

I haven't followed the this thread too closely, but I was curious if you
could do manual tweaks by using the 'bootm' and 'fdt' sub-commands.
This could allow more fine grained control of the boot process and let
you manually modify the DTB before booting to Linux.  Eg make the
'bootcmd' environment variable be something like:
bootm start $loadaddr; \
bootm loados; \
bootm ramdisk; \
bootm fdt; \
fdt boardsetup; \
fdt set memory reg 0 1000; \
bootm prep; \
bootm go

Some dual core Freescale board's do somewhat similar operations for AMP
operation (see doc/README.p2020rdb), although I haven't personally tried
AMP.

Best,
Peter

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

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Dan Malek]

On Dec 8, 2010, at 2:34 PM, Wolfgang Denk wrote:

 You can please all the people some of the time and some of the people
 all of the time but you can't please all the people all of the time.

Yes, I'm sometimes pleased  :-)

 My current thinking is to introduce something like  .

Well, that is pretty cool.

   dt_skip=memory,mac-address

Do we have to write a parser now, or is there something
that currently exists to help out? :-)

Thanks.

-- Dan

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

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Deepak Saxena]

On 12/06/2010 10:52 PM, Wolfgang Denk wrote:
 Dear Deepak Saxena,

 I am not sure this is a good idea.

 So far we have a pretty clear definition of responsibilities.
 U-Boot does the low level initialization, including the sizing and
 testing of the system memory.  U-Boot then passes its results to Linux
 in the (modified by U-Boot) device tree.

 Your patch inverts this situation, at least for the memory.

 I agree that there may be situations where you want an easy way to
 pass such information.  But then let's handle this right.

 If you define that the device tree is the master for information
 about the memory layout (and potentially other hardware specifics),
 then you should be consequent and pass make U-Boot process this
 information.  We've discussed before that there are a number of cases
 where it would be nice if U-Boot itself could be configured usign a
 device tree.  This appears to be another one.

 What do you think?

Hi Wolfgang,

Thanks for the response, I have a few different thoughts on the subject.

I am a big fan of having consistent and clear definitions of 
responsibilities; however, I think the model of having U-Boot
handle the creation of memory nodes in the DTB does not scale
cleanly to users configuring, deploying, and managing complicated
AMP environments.

While we could provide a method to provide this information at build
time to make U-Boot, this forces a static memory partitioning on the 
system and does not mesh well with use cases where developers may
be testing different system layout options as it would require
a rebuild and reflash every time a new configuration is to be tested.
In certain environments, a developer may not be permitted to reflash the 
bootloader on a board shared by others (such as a remote HW lab).

The bootm_low and bootm_size variables are an attempt to get around
this by overriding what U-Boot knows about the system but I think
those also don't scale well when we start dealing with large numbers
of cores (32+) with independent OS instances on them for some of the 
same reasons. I think it is far simpler to have some custom scripts to 
spit out new DTB files that uBoot is configured to load every time it 
boots than to have to change a bunch of environment variables on boot.

In the multi-node environments, we can't simply tell U-Boot to process 
the DTB to determine how much memory is in the system as there is one 
DTB per AMP node. One idea that comes to mind but that I think is not
the right way to go due to complexity is to have the concept of
nested DTBs that can define multiple operational machines and
U-Boot knows how to read this and send the right sub-DTB to the
right kernel image.

I'm new to U-Boot development so would like to hear about the other use 
cases you mentioned (pointer to email threads are OK) so I can have a 
better understanding of the overall issues.

Thinking about this at a higher level, I think the question is where
is the delineation between board bringup/configuration and run time
configuration management? Scanning memory and determining how much 
exists and programming the memory controller is a board-bring up and 
configuration task that a bootloader has traditionally done. 
Partitioning for AMP operation is about managing what and how is running
on top of the bootloader. How much knowledge should the bootloader have
about this? The approach of providing the memory partitioning in the DTB 
basically removes any of this knowledge from U-Boot, while the
other approaches (bootm, build-time configuration) make U-Boot very 
aware and tied to what might be running above and reduce flexibility
to easily change that.

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

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Wolfgang Denk]

Dear Deepak,

In message 4cfe775c.6050...@mentor.com you wrote:

 I am a big fan of having consistent and clear definitions of 
 responsibilities; however, I think the model of having U-Boot
 handle the creation of memory nodes in the DTB does not scale
 cleanly to users configuring, deploying, and managing complicated
 AMP environments.

I agree with you, but I think this is just one part (and eventually a
minor part) of the issue.  I would not protest if you say thatthe whol
(static, compile time) configuration of U-Boot does not scale well for
such requirements.

So far we usually had pretty static board configurations, and a static
compile time description was all we needed.  Some developers consider
even simple extensions like auto-sizing the available RAM as
unnecessary luxury that just inreases the boot time and memory
footprint.

When it comes to more complicated setups we should provide means for a
more dynamic configuration - this has been discussed before, and there
was a general agreement that the device tree would be a usable way to
implement such a description of the hardware.

So what I'm proposing is not an opposite to what you have in mind, it
just takes it one step further, and makes the whole system consistent
again.

Waht I don't like is the tunneling of information through U-Boot,
while U-Boot actually needs and uses this very information as well.

 While we could provide a method to provide this information at build
 time to make U-Boot, this forces a static memory partitioning on the 
 system and does not mesh well with use cases where developers may

This is NOT what I suggested.

 The bootm_low and bootm_size variables are an attempt to get around
 this by overriding what U-Boot knows about the system but I think
 those also don't scale well when we start dealing with large numbers
 of cores (32+) with independent OS instances on them for some of the 
 same reasons. I think it is far simpler to have some custom scripts to 
 spit out new DTB files that uBoot is configured to load every time it 
 boots than to have to change a bunch of environment variables on boot.

Again, there is no conflict between your statement and what I
suggested.

 In the multi-node environments, we can't simply tell U-Boot to process 
 the DTB to determine how much memory is in the system as there is one 
 DTB per AMP node. One idea that comes to mind but that I think is not

Please explain: you can use the DT to tell Linux (or other OS) how
much memory they shoulduse, but you cannot use the same mechanism to
pass the same information to U-Boot?

 the right way to go due to complexity is to have the concept of
 nested DTBs that can define multiple operational machines and
 U-Boot knows how to read this and send the right sub-DTB to the
 right kernel image.

This is a technical details that can and should be discussed when we
have an agreement about the basic mechanism.

 I'm new to U-Boot development so would like to hear about the other use 
 cases you mentioned (pointer to email threads are OK) so I can have a 
 better understanding of the overall issues.

There are many board vendors who shipt boards with different
configurations - with or without NAND flash; with or without other
peripherals like CAN contollers, LCD, etc.; with different LCD sizes
and types, in portrait or landscape orientation, etc.  Some of these
features can be determined by probing the hardware, others (like the
orientation of a LCD) cannot.  It is sometimes a maintenance nightmare
to provide tens of different configurations of U-Boot for a single
product.  Being able to cinfigure available hardware through the DT,
and using a single common binary image of U-Boot for such systems
would be a great benefit.

 Thinking about this at a higher level, I think the question is where
 is the delineation between board bringup/configuration and run time
 configuration management? Scanning memory and determining how much 
 exists and programming the memory controller is a board-bring up and 
 configuration task that a bootloader has traditionally done. 

And probaly one it still has to do, as there are good chances that you
don't know the actual memory size in advance - like on systems that
come in several configruations but use a common U-Boot image, or
systems where the user can plug in DIMMs of different size.

 Partitioning for AMP operation is about managing what and how is running
 on top of the bootloader. How much knowledge should the bootloader have
 about this? The approach of providing the memory partitioning in the DTB 
 basically removes any of this knowledge from U-Boot, while the

I see many use cases where this is simply not possible, because you
need need the help of the bootloader to determine parameters that are
not known in advance, and that thus cannot be encoded in the DT.
Memory size if a very typical example for such a parameter.  It may be
OK for the use case you have currently in mind to use a fixed size,
but this 

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Wolfgang Denk]

Dear Hollis,

In message 4cfe7fa8.2030...@mentor.com you wrote:
 On 12/06/2010 10:52 PM, Wolfgang Denk wrote:
  If you define that the device tree is the master for information
  about the memory layout (and potentially other hardware specifics),
  then you should be consequent and pass make U-Boot process this
  information.  We've discussed before that there are a number of cases
  where it would be nice if U-Boot itself could be configured usign a
  device tree.  This appears to be another one.
 I *think* what you're suggesting is basically providing u-boot with a 
 single device tree, even when it will load multiple operating systems. 
 The tree would then look something like this:

To be honest - I have not spent thoughts yet how this can be
implemented.  I would expect that there might be some common part, but
there will eventually also be per-core configurations.

  cpus
  ...
  memory
  reg = 0 2000
  soc
  ...
  partitions
  partit...@0
  memory
  reg = 0 1000
  partit...@1
  memory
  reg = 1000 1000
 
 U-boot would then be responsible for constructing multiple device trees 
 (one for each partition) itself, based on the additional information 
 found in the partitions node.
 
 Is that correct?

I did not think about that. I did not think about how to boot an OS
and how to provide a DT to these.

My concerns are still on a lower level. Memory initialization is a
very basic task of the boot loader. We are discussiong to move the
description of this resource out of U-Boot (where it was
traditionally statically coinfigured at compile time, with the
exception of auti-sizing). When doing so, we should not let U-Boot
live in a different world, or let it operate on a different set of
information. If the description of a resource is in the DT, then
U-Boot has to use this information (from the DT) for all operations
that deal with this resource.

Just passing such information to the OS, behind U-Boot's back and with
U-Boot having an independt (and probably different) view makes no
sense to me.

There are a number of features (persistent RAM, shared log buffer,
shared video buffers, ...) where U-Boot and Linux need to have exactly
the same understanding about available and usable memory.  I just want
to make sure that future extensions will allow to keep these features
instead of breaking them.


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 optimum committee has no members.
   - Norman Augustine
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Hollis Blanchard]

On 12/06/2010 10:52 PM, Wolfgang Denk wrote:
 If you define that the device tree is the master for information
 about the memory layout (and potentially other hardware specifics),
 then you should be consequent and pass make U-Boot process this
 information.  We've discussed before that there are a number of cases
 where it would be nice if U-Boot itself could be configured usign a
 device tree.  This appears to be another one.
I *think* what you're suggesting is basically providing u-boot with a 
single device tree, even when it will load multiple operating systems. 
The tree would then look something like this:

/
 cpus
 ...
 memory
 reg = 0 2000
 soc
 ...
 partitions
 partit...@0
 memory
 reg = 0 1000
 partit...@1
 memory
 reg = 1000 1000

U-boot would then be responsible for constructing multiple device trees 
(one for each partition) itself, based on the additional information 
found in the partitions node.

Is that correct?

Hollis Blanchard
Mentor Graphics, Embedded Systems Division


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

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Scott Wood]

On Mon, 6 Dec 2010 16:56:26 -0800
Deepak Saxena deepak_sax...@mentor.com wrote:

 +/*
 + * Check to see if an valid memory/reg property exists
 + * in the fdt. If so, we do not overwrite it with what's
 + * been scanned.
 + *
 + * Valid mean all the following:
 + *
 + * - Memory node has a device-type of memory
 + * - A reg property exists which:
 + *   + has exactly as many cells as #address-cells + #size-cells
 + *   + provides a range that is within [bi_memstart, bi_memstart + 
 bi_memsize]
 + */

This will get false positives -- a lot of existing device tree
templates have something like this:

memory {
device_type = memory;
reg = 0 0 0 0;// Filled by U-Boot
};

-Scott

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

[U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Deepak Saxena]

commit 341764495180a712b9aaccfa0479b2ff7e44e35b
Author: Deepak Saxena deepak_sax...@mentor.com
Date:   Mon Dec 6 15:52:07 2010 -0800

 Honor /memory/reg node in DTB files

 This patch adds code to the bootm path to check if a valid
 /memory/reg node exists in the DTB file and if so, it
 does not override it with the values in bi_memstart and
 bi_memsize. This is particularly useful in multi-core
 environments where the memory may be partitioned across
 a large number of nodes.

 While the same can be accomplished on certain boards (p1022ds
 and p1_p2_rdb) by using the bootm_low and bootm_size
 environment variables, that solution is not universal and
 requires adding code ft_board_setup() for any new board
 that wants to support AMP operation. Also, given that the
 DTB is already  used to partition board devices (see commit
 dc2e673 in the Linux kernel tree), it makes sense to
 allow memory to be partitioned the same way from a user
 configuration perspective.

 This patch allows for the user to override the DTB file parameters
 on the p1022ds and p1_p2_rdb boards by setting the bootm_low and
 bootm_size to something other than bi_memstart and bi_memsize.
 In the long-term, those env variables should be depecrated and
 removed and system implementors should provide the memory
 partitioning information in the DTB.

 Signed-off-by: Deepak Saxena deepak_sax...@mentor.com
 Signed-off-by: Hollis Blanchard hollis_blanch...@mentor.com

---

See http://lists.denx.de/pipermail/u-boot/2010-December/083057.html
for initial proposal on this.

diff --git a/arch/powerpc/cpu/mpc85xx/fdt.c b/arch/powerpc/cpu/mpc85xx/fdt.c
index 4540364..6d384e3 100644
--- a/arch/powerpc/cpu/mpc85xx/fdt.c
+++ b/arch/powerpc/cpu/mpc85xx/fdt.c
@@ -377,6 +377,55 @@ static void ft_fixup_qe_snum(void *blob)
  }
  #endif

+/*
+ * Check to see if an valid memory/reg property exists
+ * in the fdt. If so, we do not overwrite it with what's
+ * been scanned.
+ *
+ * Valid mean all the following:
+ *
+ * - Memory node has a device-type of memory
+ * - A reg property exists which:
+ *   + has exactly as many cells as #address-cells + #size-cells
+ *   + provides a range that is within [bi_memstart, bi_memstart + 
bi_memsize]
+ */
+static int ft_validate_memory(void *blob, bd_t *bd)
+{
+   int nodeoffset;
+   u32 *addrcell = (u32*)fdt_getprop(blob, 0, #address-cells, NULL);
+   u32 *sizecell = (u32*)fdt_getprop(blob, 0, #size-cells, NULL);
+   u64 reg_base, reg_size;
+   void *reg, *dtype;
+   int len;
+
+   if ((nodeoffset = fdt_path_offset(blob, /memory)) = 0)
+   {
+   dtype = fdt_getprop(blob, nodeoffset, device_type, len);
+   if (!dtype || (strcmp(dtype, memory) != 0))
+   return 0;
+
+   reg = fdt_getprop(blob, nodeoffset, reg, len);
+   if (reg  len == ((*addrcell + *sizecell) * 4)) {
+   if (*addrcell == 2) {
+   reg_base = ((u64*)reg)[0];
+   reg_size = ((u64*)reg)[1];
+   } else {
+   reg_base = ((u32*)reg)[0];
+   reg_size = ((u32*)reg)[1];
+   }
+
+   if ((reg_size) 
+   (reg_base = (u64)bd-bi_memstart) 
+   ((reg_size + reg_base)
+= ((u64)bd-bi_memstart + (u64)bd-bi_memsize)))
+   return 1;   
+   }
+   }
+
+   return 0;
+
+}
+
  void ft_cpu_setup(void *blob, bd_t *bd)
  {
int off;
@@ -434,7 +483,8 @@ void ft_cpu_setup(void *blob, bd_t *bd)
clock-frequency, CONFIG_SYS_CLK_FREQ, 1);
  #endif

-   fdt_fixup_memory(blob, (u64)bd-bi_memstart, (u64)bd-bi_memsize);
+   if (!ft_validate_memory(blob, bd))
+   fdt_fixup_memory(blob, (u64)bd-bi_memstart, 
(u64)bd-bi_memsize);

  #ifdef CONFIG_MP
ft_fixup_cpu(blob, (u64)bd-bi_memstart + (u64)bd-bi_memsize);
diff --git a/board/freescale/p1022ds/p1022ds.c 
b/board/freescale/p1022ds/p1022ds.c
index 5cdee9f..7378d88 100644
--- a/board/freescale/p1022ds/p1022ds.c
+++ b/board/freescale/p1022ds/p1022ds.c
@@ -320,7 +320,8 @@ void ft_board_setup(void *blob, bd_t *bd)
base = getenv_bootm_low();
size = getenv_bootm_size();

-   fdt_fixup_memory(blob, (u64)base, (u64)size);
+   if (base != (phys_addr_t)bd-bi_memstart  size != 
(phys_addr_t)bd-bi_memsize)
+   fdt_fixup_memory(blob, (u64)base, (u64)size);

FT_FSL_PCI_SETUP;

diff --git a/board/freescale/p1_p2_rdb/p1_p2_rdb.c 
b/board/freescale/p1_p2_rdb/p1_p2_rdb.c
index fae31f2..5e4adc6 100644
--- a/board/freescale/p1_p2_rdb/p1_p2_rdb.c
+++ b/board/freescale/p1_p2_rdb/p1_p2_rdb.c
@@ -220,9 +220,10 @@ void ft_board_setup(void *blob, bd_t 

Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

[Wolfgang Denk]

Dear Deepak Saxena,

In message 4cfd863a.7070...@mentor.com you wrote:
 commit 341764495180a712b9aaccfa0479b2ff7e44e35b
 Author: Deepak Saxena deepak_sax...@mentor.com
 Date:   Mon Dec 6 15:52:07 2010 -0800
 
  Honor /memory/reg node in DTB files
 
  This patch adds code to the bootm path to check if a valid
  /memory/reg node exists in the DTB file and if so, it
  does not override it with the values in bi_memstart and
  bi_memsize. This is particularly useful in multi-core
  environments where the memory may be partitioned across
  a large number of nodes.
 
  While the same can be accomplished on certain boards (p1022ds
  and p1_p2_rdb) by using the bootm_low and bootm_size
  environment variables, that solution is not universal and
  requires adding code ft_board_setup() for any new board
  that wants to support AMP operation. Also, given that the
  DTB is already  used to partition board devices (see commit
  dc2e673 in the Linux kernel tree), it makes sense to
  allow memory to be partitioned the same way from a user
  configuration perspective.
 
  This patch allows for the user to override the DTB file parameters
  on the p1022ds and p1_p2_rdb boards by setting the bootm_low and
  bootm_size to something other than bi_memstart and bi_memsize.
  In the long-term, those env variables should be depecrated and
  removed and system implementors should provide the memory
  partitioning information in the DTB.
 
  Signed-off-by: Deepak Saxena deepak_sax...@mentor.com
  Signed-off-by: Hollis Blanchard hollis_blanch...@mentor.com

I am not sure this is a good idea.

So far we have a pretty clear definition of responsibilities.
U-Boot does the low level initialization, including the sizing and
testing of the system memory.  U-Boot then passes its results to Linux
in the (modified by U-Boot) device tree.

Your patch inverts this situation, at least for the memory.

I agree that there may be situations where you want an easy way to
pass such information.  But then let's handle this right.

If you define that the device tree is the master for information
about the memory layout (and potentially other hardware specifics),
then you should be consequent and pass make U-Boot process this
information.  We've discussed before that there are a number of cases
where it would be nice if U-Boot itself could be configured usign a
device tree.  This appears to be another one.

What do you think?

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
When all else fails, read the instructions.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot