[PATCH] Revert "ARM: Use armlinux_bootparams address for DTB"

[PATCH] Revert "ARM: Use armlinux_bootparams address for DTB"

[Sascha Hauer]

This patch placed the flattened devicetree to armlinux_bootparams.
armlinux_bootparams normally is at SDRAM_START + 0x100. The kernels
initial page tables are normally at SDRAM_START + 0x4000, so the
flattened devicetree gets overwritten once it exceeds 0x3f00 bytes
which is quite common.

Revert this patch for now once a better solution can be found

This reverts commit 0c4108f9173ece75d96f19b447ef6464bdfdf187.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Alexander Shiyan <shc_work@mail.ru>
---
 arch/arm/lib/armlinux.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/arch/arm/lib/armlinux.c b/arch/arm/lib/armlinux.c
index d962997..40a63ea 100644
--- a/arch/arm/lib/armlinux.c
+++ b/arch/arm/lib/armlinux.c
@@ -261,24 +261,16 @@ void start_linux(void *adr, int swap, unsigned long initrd_address,
 		unsigned long initrd_size, void *oftree)
 {
 	void (*kernel)(int zero, int arch, void *params) = adr;
+	void *params = NULL;
 	int architecture;
 
 	if (oftree) {
-		if (armlinux_bootparams) {
-			struct fdt_header *header = oftree;
-
-			memcpy(armlinux_bootparams, oftree,
-					fdt32_to_cpu(header->totalsize));
-		} else {
-			armlinux_bootparams = oftree;
-		}
-
-		printf("booting Linux kernel with devicetree at 0x%p\n",
-				armlinux_bootparams);
+		printf("booting Linux kernel with devicetree\n");
+		params = oftree;
 	} else {
 		setup_tags(initrd_address, initrd_size, swap);
+		params = armlinux_bootparams;
 	}
-
 	architecture = armlinux_get_architecture();
 
 	shutdown_barebox();
@@ -296,10 +288,10 @@ void start_linux(void *adr, int swap, unsigned long initrd_address,
 		"mov r2, %1\n"
 		"bx %2\n"
 		:
-		: "r" (architecture), "r" (armlinux_bootparams), "r" (kernel)
+		: "r" (architecture), "r" (params), "r" (kernel)
 		: "r0", "r1", "r2"
 	);
 #else
-	kernel(0, architecture, armlinux_bootparams);
+	kernel(0, architecture, params);
 #endif
 }
-- 
1.8.4.rc1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Revert "ARM: Use armlinux_bootparams address for DTB"

[Alexander Shiyan]

> This patch placed the flattened devicetree to armlinux_bootparams.
> armlinux_bootparams normally is at SDRAM_START + 0x100. The kernels
> initial page tables are normally at SDRAM_START + 0x4000, so the
> flattened devicetree gets overwritten once it exceeds 0x3f00 bytes
> which is quite common.
> 
> Revert this patch for now once a better solution can be found

In such a case, tree may be placed immediately after the kernel?
It can also serve as an automatic option "ARM_APPENDED_DTB".

---
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Revert "ARM: Use armlinux_bootparams address for DTB"

[Sascha Hauer]

On Wed, Aug 07, 2013 at 01:55:42PM +0400, Alexander Shiyan wrote:
> > This patch placed the flattened devicetree to armlinux_bootparams.
> > armlinux_bootparams normally is at SDRAM_START + 0x100. The kernels
> > initial page tables are normally at SDRAM_START + 0x4000, so the
> > flattened devicetree gets overwritten once it exceeds 0x3f00 bytes
> > which is quite common.
> > 
> > Revert this patch for now once a better solution can be found
> 
> In such a case, tree may be placed immediately after the kernel?
> It can also serve as an automatic option "ARM_APPENDED_DTB".

Did you actually stumbled over this problem? Normally the dtb is
allocated using malloc. The malloc region normally shouldn't overlap
with the kernels text region unless you are really tight of memory.
Was this on a clps711x?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Revert "ARM: Use armlinux_bootparams address for DTB"

[Alexander Shiyan]

> On Wed, Aug 07, 2013 at 01:55:42PM +0400, Alexander Shiyan wrote:
> > > This patch placed the flattened devicetree to armlinux_bootparams.
> > > armlinux_bootparams normally is at SDRAM_START + 0x100. The kernels
> > > initial page tables are normally at SDRAM_START + 0x4000, so the
> > > flattened devicetree gets overwritten once it exceeds 0x3f00 bytes
> > > which is quite common.
> > > 
> > > Revert this patch for now once a better solution can be found
> > 
> > In such a case, tree may be placed immediately after the kernel?
> > It can also serve as an automatic option "ARM_APPENDED_DTB".
> 
> Did you actually stumbled over this problem? Normally the dtb is
> allocated using malloc. The malloc region normally shouldn't overlap
> with the kernels text region unless you are really tight of memory.
> Was this on a clps711x?

I described it here:
http://www.spinics.net/lists/u-boot-v2/msg15612.html

---
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Revert "ARM: Use armlinux_bootparams address for DTB"

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 804 bytes --]

On Wed, Aug 07, 2013 at 01:55:42PM +0400, Alexander Shiyan wrote:
> > This patch placed the flattened devicetree to armlinux_bootparams.
> > armlinux_bootparams normally is at SDRAM_START + 0x100. The kernels
> > initial page tables are normally at SDRAM_START + 0x4000, so the
> > flattened devicetree gets overwritten once it exceeds 0x3f00 bytes
> > which is quite common.
> > 
> > Revert this patch for now once a better solution can be found
> 
> In such a case, tree may be placed immediately after the kernel?
> It can also serve as an automatic option "ARM_APPENDED_DTB".

That won't work either, since the DTB will be overwritten by the kernel
decompressor.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Revert "ARM: Use armlinux_bootparams address for DTB"

[Alexander Shiyan]

> On Wed, Aug 07, 2013 at 01:55:42PM +0400, Alexander Shiyan wrote:
> > > This patch placed the flattened devicetree to armlinux_bootparams.
> > > armlinux_bootparams normally is at SDRAM_START + 0x100. The kernels
> > > initial page tables are normally at SDRAM_START + 0x4000, so the
> > > flattened devicetree gets overwritten once it exceeds 0x3f00 bytes
> > > which is quite common.
> > > 
> > > Revert this patch for now once a better solution can be found
> > 
> > In such a case, tree may be placed immediately after the kernel?
> > It can also serve as an automatic option "ARM_APPENDED_DTB".
> 
> That won't work either, since the DTB will be overwritten by the kernel
> decompressor.

arch/arm/Kconfig:
...
config ARM_APPENDED_DTB
	bool "Use appended device tree blob to zImage (EXPERIMENTAL)"
	depends on OF && !ZBOOT_ROM
	help
	  With this option, the boot code will look for a device tree binary
	  (DTB) appended to zImage
	  (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
...

So, DTB is placed immediately after the kernel.

---
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Revert "ARM: Use armlinux_bootparams address for DTB"

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1526 bytes --]

On Wed, Aug 07, 2013 at 04:06:24PM +0400, Alexander Shiyan wrote:
> > On Wed, Aug 07, 2013 at 01:55:42PM +0400, Alexander Shiyan wrote:
> > > > This patch placed the flattened devicetree to armlinux_bootparams.
> > > > armlinux_bootparams normally is at SDRAM_START + 0x100. The kernels
> > > > initial page tables are normally at SDRAM_START + 0x4000, so the
> > > > flattened devicetree gets overwritten once it exceeds 0x3f00 bytes
> > > > which is quite common.
> > > > 
> > > > Revert this patch for now once a better solution can be found
> > > 
> > > In such a case, tree may be placed immediately after the kernel?
> > > It can also serve as an automatic option "ARM_APPENDED_DTB".
> > 
> > That won't work either, since the DTB will be overwritten by the kernel
> > decompressor.
> 
> arch/arm/Kconfig:
> ...
> config ARM_APPENDED_DTB
> 	bool "Use appended device tree blob to zImage (EXPERIMENTAL)"
> 	depends on OF && !ZBOOT_ROM
> 	help
> 	  With this option, the boot code will look for a device tree binary
> 	  (DTB) appended to zImage
> 	  (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
> ...
> 
> So, DTB is placed immediately after the kernel.

Yes, in the case where you have APPENDED_DTB, the decompressor relocates
the DTB before doing the decompression.

The trouble is when you don't have APPENDED_DTB enabled, when there is
no such relocation happening.

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Revert "ARM: Use armlinux_bootparams address for DTB"

[Alexander Shiyan]

> > On Wed, Aug 07, 2013 at 01:55:42PM +0400, Alexander Shiyan wrote:
> > > > This patch placed the flattened devicetree to armlinux_bootparams.
> > > > armlinux_bootparams normally is at SDRAM_START + 0x100. The kernels
> > > > initial page tables are normally at SDRAM_START + 0x4000, so the
> > > > flattened devicetree gets overwritten once it exceeds 0x3f00 bytes
> > > > which is quite common.
> > > > 
> > > > Revert this patch for now once a better solution can be found
> > > 
> > > In such a case, tree may be placed immediately after the kernel?
> > > It can also serve as an automatic option "ARM_APPENDED_DTB".
> > 
> > Did you actually stumbled over this problem? Normally the dtb is
> > allocated using malloc. The malloc region normally shouldn't overlap
> > with the kernels text region unless you are really tight of memory.
> > Was this on a clps711x?
> 
> I described it here:
> http://www.spinics.net/lists/u-boot-v2/msg15612.html

Seems this issue can be resolved by decrease MALLOC_SIZE.

...
    0xc0380000 - 0xc077ffff (size 0x00400000) malloc space
...

---
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Revert "ARM: Use armlinux_bootparams address for DTB"

[Sascha Hauer]

On Thu, Aug 08, 2013 at 11:17:28AM +0400, Alexander Shiyan wrote:
> > > On Wed, Aug 07, 2013 at 01:55:42PM +0400, Alexander Shiyan wrote:
> > > > > This patch placed the flattened devicetree to armlinux_bootparams.
> > > > > armlinux_bootparams normally is at SDRAM_START + 0x100. The kernels
> > > > > initial page tables are normally at SDRAM_START + 0x4000, so the
> > > > > flattened devicetree gets overwritten once it exceeds 0x3f00 bytes
> > > > > which is quite common.
> > > > > 
> > > > > Revert this patch for now once a better solution can be found
> > > > 
> > > > In such a case, tree may be placed immediately after the kernel?
> > > > It can also serve as an automatic option "ARM_APPENDED_DTB".
> > > 
> > > Did you actually stumbled over this problem? Normally the dtb is
> > > allocated using malloc. The malloc region normally shouldn't overlap
> > > with the kernels text region unless you are really tight of memory.
> > > Was this on a clps711x?
> > 
> > I described it here:
> > http://www.spinics.net/lists/u-boot-v2/msg15612.html
> 
> Seems this issue can be resolved by decrease MALLOC_SIZE.
> 
> ...
>     0xc0380000 - 0xc077ffff (size 0x00400000) malloc space
> ...

Does this mean you are fine with reverting the patch?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Revert "ARM: Use armlinux_bootparams address for DTB"

[Alexander Shiyan]

> > > > On Wed, Aug 07, 2013 at 01:55:42PM +0400, Alexander Shiyan wrote:
> > > > > > This patch placed the flattened devicetree to armlinux_bootparams.
> > > > > > armlinux_bootparams normally is at SDRAM_START + 0x100. The kernels
> > > > > > initial page tables are normally at SDRAM_START + 0x4000, so the
> > > > > > flattened devicetree gets overwritten once it exceeds 0x3f00 bytes
> > > > > > which is quite common.
> > > > > > 
> > > > > > Revert this patch for now once a better solution can be found
> > > > > 
> > > > > In such a case, tree may be placed immediately after the kernel?
> > > > > It can also serve as an automatic option "ARM_APPENDED_DTB".
> > > > 
> > > > Did you actually stumbled over this problem? Normally the dtb is
> > > > allocated using malloc. The malloc region normally shouldn't overlap
> > > > with the kernels text region unless you are really tight of memory.
> > > > Was this on a clps711x?
> > > 
> > > I described it here:
> > > http://www.spinics.net/lists/u-boot-v2/msg15612.html
> > 
> > Seems this issue can be resolved by decrease MALLOC_SIZE.
> > 
> > ...
> >     0xc0380000 - 0xc077ffff (size 0x00400000) malloc space
> > ...
> 
> Does this mean you are fine with reverting the patch?

Unfortunately I cannot test this now.
In any case if this breaks other builds, yes, we should revert this.

---
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Revert "ARM: Use armlinux_bootparams address for DTB"

[Sascha Hauer]

On Fri, Aug 09, 2013 at 11:02:26AM +0400, Alexander Shiyan wrote:
> > > > > On Wed, Aug 07, 2013 at 01:55:42PM +0400, Alexander Shiyan wrote:
> > > > > > > This patch placed the flattened devicetree to armlinux_bootparams.
> > > > > > > armlinux_bootparams normally is at SDRAM_START + 0x100. The kernels
> > > > > > > initial page tables are normally at SDRAM_START + 0x4000, so the
> > > > > > > flattened devicetree gets overwritten once it exceeds 0x3f00 bytes
> > > > > > > which is quite common.
> > > > > > > 
> > > > > > > Revert this patch for now once a better solution can be found
> > > > > > 
> > > > > > In such a case, tree may be placed immediately after the kernel?
> > > > > > It can also serve as an automatic option "ARM_APPENDED_DTB".
> > > > > 
> > > > > Did you actually stumbled over this problem? Normally the dtb is
> > > > > allocated using malloc. The malloc region normally shouldn't overlap
> > > > > with the kernels text region unless you are really tight of memory.
> > > > > Was this on a clps711x?
> > > > 
> > > > I described it here:
> > > > http://www.spinics.net/lists/u-boot-v2/msg15612.html
> > > 
> > > Seems this issue can be resolved by decrease MALLOC_SIZE.
> > > 
> > > ...
> > >     0xc0380000 - 0xc077ffff (size 0x00400000) malloc space
> > > ...
> > 
> > Does this mean you are fine with reverting the patch?
> 
> Unfortunately I cannot test this now.
> In any case if this breaks other builds, yes, we should revert this.

Ok, thanks. Just did that.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox