Re: [PATCH] powerpc: make the padding for the device tree a configurable option

2010-05-20 Thread David Gibson
On Wed, May 19, 2010 at 08:46:19PM -0500, Timur Tabi wrote:
 On Wed, May 19, 2010 at 8:18 PM, David Gibson
 da...@gibson.dropbear.id.au wrote:
 
  Couldn't you use the configurable padding, but put the stuff to do it
  into u-boot.  i.e. repad the dtb at u-boot build time, rather than
  u-boot runtime.
 
 That's what I was trying to do.   Take a look at this thread:
 
 http://lists.denx.de/pipermail/u-boot/2010-May/071520.html

Um.. what?  As far as I can tell that thread is about runtime
expanding the space given to the dtb.  I'm talking about altering the
padding on the file before giving it to u-boot in the first place.


-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc: make the padding for the device tree a configurable option

2010-05-20 Thread Timur Tabi
On Thu, May 20, 2010 at 1:17 AM, David Gibson
da...@gibson.dropbear.id.au wrote:

 Um.. what?  As far as I can tell that thread is about runtime
 expanding the space given to the dtb.  I'm talking about altering the
 padding on the file before giving it to u-boot in the first place.

Sorry, I guess I don't understand.  How do I alter the padding before
giving it to U-Boot?

-- 
Timur Tabi
Linux kernel developer at Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc: make the padding for the device tree a configurable option

2010-05-20 Thread Scott Wood

On 05/19/2010 08:18 PM, David Gibson wrote:

On Wed, May 19, 2010 at 07:03:17PM -0500, Timur Tabi wrote:

On Wed, May 19, 2010 at 5:44 PM, Benjamin Herrenschmidt

The padding in the kernel built is intended to
make space for DT changes done by the zImage wrapper.


Well, okay.  I think it would be nice if we expanded that to handle
general usage.


Don't we have dynamic expansion in the zImage wrapper?


Maybe we could add to libfdt a way to provide a realloc() callback to it
when it hits the max size, and uboot can then move things around (or
fail).


The problem is that the code which allocates a block for the fdt is
completely distinct from the code that manipulates the fdt.  We'd need
to put in either some kind of funky callback mechanism, or insist that
every fdt exist in a block of memory allocated by some specific method
(e.g. lmb).

I'm stuck between a rock and a hard place, it seems.  No one is
willing to compromise on any of my ideas.  It's hard to convince our
BSP developers that they should be pushing more code upstream when I
get so much resistance for a such a mundane change.


Couldn't you use the configurable padding, but put the stuff to do it
into u-boot.  i.e. repad the dtb at u-boot build time, rather than
u-boot runtime.


Currently generating a dtb is not done as part of the u-boot build 
process -- and apparently some people are using the Linux makefile 
target to do it.


I think the right thing is to do whatever we need to do in u-boot to get 
dynamic expansion fully working (it already does it in some 
circumstances, just not always, or necessarily safely).  Static padding 
is too fragile.


-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc: make the padding for the device tree a configurable option

2010-05-19 Thread Benjamin Herrenschmidt
On Wed, 2010-05-19 at 14:53 -0500, Timur Tabi wrote:
 Add the DTS_PADDING Kconfig option, which allows users and board defconfig
 files to specify a padding value when compiling device trees.
 
 When a device tree source (DTS) is compiled into a binary (DTB), a hard-coded
 padding of 1024 bytes is used (i.e. dtc command-line parameter -p 1024).
 Although this has worked so far, it may not be sufficient in the future for
 some boards.  Newer versions of U-boot perform more and more fixup on the
 device tree, and eventually it will run out.
 
 So to accommodate future boards where more padding is needed, we make the
 option for the -p parameter configurable.

Can't u-boot just allocate more space ?

Ben.

 Signed-off-by: Timur Tabi ti...@freescale.com
 ---
  arch/powerpc/boot/Makefile |4 ++--
  arch/powerpc/platforms/Kconfig |   13 +
  2 files changed, 15 insertions(+), 2 deletions(-)
 
 diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
 index bb2465b..750fa6b 100644
 --- a/arch/powerpc/boot/Makefile
 +++ b/arch/powerpc/boot/Makefile
 @@ -35,7 +35,7 @@ endif
  
  BOOTCFLAGS   += -I$(obj) -I$(srctree)/$(obj)
  
 -DTS_FLAGS?= -p 1024
 +DTS_FLAGS?= -p ${CONFIG_DTS_PADDING}
  
  $(obj)/4xx.o: BOOTCFLAGS += -mcpu=405
  $(obj)/ebony.o: BOOTCFLAGS += -mcpu=405
 @@ -331,7 +331,7 @@ $(obj)/treeImage.%: vmlinux $(obj)/%.dtb $(wrapperbits)
  # Rule to build device tree blobs
  DTC = $(objtree)/scripts/dtc/dtc
  
 -$(obj)/%.dtb: $(dtstree)/%.dts
 +$(obj)/%.dtb: $(dtstree)/%.dts $(srctree)/.config
   $(DTC) -O dtb -o $(obj)/$*.dtb -b 0 $(DTS_FLAGS) $(dtstree)/$*.dts
  
  # If there isn't a platform selected then just strip the vmlinux.
 diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
 index d1663db..2b0a9c3 100644
 --- a/arch/powerpc/platforms/Kconfig
 +++ b/arch/powerpc/platforms/Kconfig
 @@ -41,6 +41,19 @@ config PPC_OF_BOOT_TRAMPOLINE
  
 In case of doubt, say Y
  
 +config DTS_PADDING
 + int Padding for the device tree binary
 + default 1024
 + help
 +   Specify the padding value to be used when compiling a DTS (device
 +   tree source) file into a DTB (device tree binary).  The padding is
 +   used to ensure enough space for any additional nodes and properties
 +   that the boot loader adds during fix-up.  If your boot loader
 +   complains about lack of space during fix-up, try increasing this
 +   value.
 +
 +   If unsure, leave this value at the default.
 +
  config UDBG_RTAS_CONSOLE
   bool RTAS based debug console
   depends on PPC_RTAS


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc: make the padding for the device tree a configurable option

2010-05-19 Thread Timur Tabi
Benjamin Herrenschmidt wrote:

 So to accommodate future boards where more padding is needed, we make the
 option for the -p parameter configurable.
 
 Can't u-boot just allocate more space ?

Yes and no.  U-Boot has functions to increase the size of an fdt, but these
functions can't be sure that the fdt will grow beyond its allocated space.
So if U-Boot calls fdt_setprop() or fdt_add_subnode(), and there isn't
enough space in the fdt, those functions will return with an error.

The problem with growing the fdt is that the function which does this
(fdt_open_into) cannot guarantee that the fdt will grow too large and
overwrite the end of whatever allocated memory it's in.

I had a long argument with Wolfgang on this (see libfdt: make
fdt_increase_size() available to everyone), and he says he'll reject any
patch that can't guarantee that fdt_open_into() won't grow too large.  He'll
also reject any patch that uses a macro constant to reserve this space, even
if I use that constant to ensure that fdt_open_into() won't do anything bad.

So in other words, U-Boot could allocate more space, but Wolfgang won't let it.

-- 
Timur Tabi
Linux kernel developer at Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc: make the padding for the device tree a configurable option

2010-05-19 Thread Benjamin Herrenschmidt
On Wed, 2010-05-19 at 16:33 -0500, Timur Tabi wrote:
 Benjamin Herrenschmidt wrote:
 
  So to accommodate future boards where more padding is needed, we make the
  option for the -p parameter configurable.
  
  Can't u-boot just allocate more space ?
 
 Yes and no.  U-Boot has functions to increase the size of an fdt, but these
 functions can't be sure that the fdt will grow beyond its allocated space.
 So if U-Boot calls fdt_setprop() or fdt_add_subnode(), and there isn't
 enough space in the fdt, those functions will return with an error.

 .../...

It's still not kernel business to have to deal with u-boot memory
allocation constraints. The padding in the kernel built is intended to
make space for DT changes done by the zImage wrapper.

Maybe we could add to libfdt a way to provide a realloc() callback to it
when it hits the max size, and uboot can then move things around (or
fail).

Ben.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc: make the padding for the device tree a configurable option

2010-05-19 Thread Timur Tabi
On Wed, May 19, 2010 at 5:44 PM, Benjamin Herrenschmidt
b...@kernel.crashing.org wrote:

 It's still not kernel business to have to deal with u-boot memory
 allocation constraints.

I agree, but it still makes sense to me to allow the padding to be configurable.

 The padding in the kernel built is intended to
 make space for DT changes done by the zImage wrapper.

Well, okay.  I think it would be nice if we expanded that to handle
general usage.

 Maybe we could add to libfdt a way to provide a realloc() callback to it
 when it hits the max size, and uboot can then move things around (or
 fail).

The problem is that the code which allocates a block for the fdt is
completely distinct from the code that manipulates the fdt.  We'd need
to put in either some kind of funky callback mechanism, or insist that
every fdt exist in a block of memory allocated by some specific method
(e.g. lmb).

I'm stuck between a rock and a hard place, it seems.  No one is
willing to compromise on any of my ideas.  It's hard to convince our
BSP developers that they should be pushing more code upstream when I
get so much resistance for a such a mundane change.

-- 
Timur Tabi
Linux kernel developer at Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc: make the padding for the device tree a configurable option

2010-05-19 Thread Benjamin Herrenschmidt
On Wed, 2010-05-19 at 19:03 -0500, Timur Tabi wrote:
 The problem is that the code which allocates a block for the fdt is
 completely distinct from the code that manipulates the fdt.  We'd need
 to put in either some kind of funky callback mechanism, or insist that
 every fdt exist in a block of memory allocated by some specific method
 (e.g. lmb).
 
 I'm stuck between a rock and a hard place, it seems.  No one is
 willing to compromise on any of my ideas.  It's hard to convince our
 BSP developers that they should be pushing more code upstream when I
 get so much resistance for a such a mundane change. 

I don't see why we couldn't add a callback to libfdt for allocation /
reallocation.

Cheers,
Ben.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc: make the padding for the device tree a configurable option

2010-05-19 Thread M. Warner Losh
In message: aanlktilefxxcmnwqksultu88r4d9w98adhlhxvuwi...@mail.gmail.com
Timur Tabi ti...@freescale.com writes:
: I'm stuck between a rock and a hard place, it seems.  No one is
: willing to compromise on any of my ideas.  It's hard to convince our
: BSP developers that they should be pushing more code upstream when I
: get so much resistance for a such a mundane change.

http://www.bikeshed.org/ seems appropriate here.

Warner
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc: make the padding for the device tree a configurable option

2010-05-19 Thread David Gibson
On Wed, May 19, 2010 at 07:03:17PM -0500, Timur Tabi wrote:
 On Wed, May 19, 2010 at 5:44 PM, Benjamin Herrenschmidt
 b...@kernel.crashing.org wrote:
 
  It's still not kernel business to have to deal with u-boot memory
  allocation constraints.
 
 I agree, but it still makes sense to me to allow the padding to be 
 configurable.
 
  The padding in the kernel built is intended to
  make space for DT changes done by the zImage wrapper.
 
 Well, okay.  I think it would be nice if we expanded that to handle
 general usage.
 
  Maybe we could add to libfdt a way to provide a realloc() callback to it
  when it hits the max size, and uboot can then move things around (or
  fail).
 
 The problem is that the code which allocates a block for the fdt is
 completely distinct from the code that manipulates the fdt.  We'd need
 to put in either some kind of funky callback mechanism, or insist that
 every fdt exist in a block of memory allocated by some specific method
 (e.g. lmb).
 
 I'm stuck between a rock and a hard place, it seems.  No one is
 willing to compromise on any of my ideas.  It's hard to convince our
 BSP developers that they should be pushing more code upstream when I
 get so much resistance for a such a mundane change.

Couldn't you use the configurable padding, but put the stuff to do it
into u-boot.  i.e. repad the dtb at u-boot build time, rather than
u-boot runtime.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc: make the padding for the device tree a configurable option

2010-05-19 Thread Timur Tabi
On Wed, May 19, 2010 at 8:18 PM, David Gibson
da...@gibson.dropbear.id.au wrote:

 Couldn't you use the configurable padding, but put the stuff to do it
 into u-boot.  i.e. repad the dtb at u-boot build time, rather than
 u-boot runtime.

That's what I was trying to do.   Take a look at this thread:

http://lists.denx.de/pipermail/u-boot/2010-May/071520.html

-- 
Timur Tabi
Linux kernel developer at Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev