Re: [PATCH] powerpc: make the padding for the device tree a configurable option
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
On Thu, May 20, 2010 at 1:17 AM, David Gibson 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
On Wed, May 19, 2010 at 08:46:19PM -0500, Timur Tabi wrote: > On Wed, May 19, 2010 at 8:18 PM, David Gibson > 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
On Wed, May 19, 2010 at 8:18 PM, David Gibson 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
Re: [PATCH] powerpc: make the padding for the device tree a configurable option
On Wed, May 19, 2010 at 07:03:17PM -0500, Timur Tabi wrote: > On Wed, May 19, 2010 at 5:44 PM, Benjamin Herrenschmidt > 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
In message: Timur Tabi 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
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
On Wed, May 19, 2010 at 5:44 PM, Benjamin Herrenschmidt 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
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
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
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
> ---
> 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
[PATCH] powerpc: make the padding for the device tree a configurable option
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.
Signed-off-by: Timur Tabi
---
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
--
1.6.5
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
