LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* N900 v3.19-rc5 arm atags_to_fdt.c is broken
@ 2015-01-23 21:39 Pali Rohár
  2015-01-23 22:36 ` Pali Rohár
  0 siblings, 1 reply; 20+ messages in thread
From: Pali Rohár @ 2015-01-23 21:39 UTC (permalink / raw)
  To: linux-omap
  Cc: Linux Kernel Mailing List, Aaro Koskinen, Tony Lindgren,
	Sebastian Reichel, Ivaylo Dimitrov, Pavel Machek, Russell King,
	linux-arm-kernel, Laura Abbott, Santosh Shilimkar

[-- Attachment #1: Type: Text/Plain, Size: 534 bytes --]

Hello,

when I boot zImage with appended DT n900 in qemu fdt_open_into() 
function called from file arch/arm/boot/compressed/atags_to_fdt.c 
(in function atags_to_fdt) always returns -FDT_ERR_NOSPACE.

It means that all ATAGS (including cmdline arguments) passed by 
bootloader are ignored.

On real n900 device I see that booted DT version also ignore 
cmdline arguments from bootloader. I cannot debug decompress code 
on real device, but I think it is same problem as in qemu.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: N900 v3.19-rc5 arm atags_to_fdt.c is broken
  2015-01-23 21:39 N900 v3.19-rc5 arm atags_to_fdt.c is broken Pali Rohár
@ 2015-01-23 22:36 ` Pali Rohár
  2015-01-23 23:48   ` Pavel Machek
  2015-01-26 16:14   ` Tony Lindgren
  0 siblings, 2 replies; 20+ messages in thread
From: Pali Rohár @ 2015-01-23 22:36 UTC (permalink / raw)
  To: linux-omap
  Cc: Linux Kernel Mailing List, Aaro Koskinen, Tony Lindgren,
	Sebastian Reichel, Ivaylo Dimitrov, Pavel Machek, Russell King,
	linux-arm-kernel, Laura Abbott

[-- Attachment #1: Type: Text/Plain, Size: 1350 bytes --]

On Friday 23 January 2015 22:39:55 Pali Rohár wrote:
> Hello,
> 
> when I boot zImage with appended DT n900 in qemu
> fdt_open_into() function called from file
> arch/arm/boot/compressed/atags_to_fdt.c (in function
> atags_to_fdt) always returns -FDT_ERR_NOSPACE.
> 
> It means that all ATAGS (including cmdline arguments) passed
> by bootloader are ignored.
> 
> On real n900 device I see that booted DT version also ignore
> cmdline arguments from bootloader. I cannot debug decompress
> code on real device, but I think it is same problem as in
> qemu.

Looks like this quick patch is fixing above problem:

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 68be901..4a7d75b 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -268,7 +268,7 @@ restart:	adr	r0, LC0
 		 * area.  No GOT fixup has occurred yet, but none of the
 		 * code we're about to call uses any global variable.
 		*/
-		add	sp, sp, #0x10000
+		add	sp, sp, #0x20000
 		stmfd	sp!, {r0-r3, ip, lr}
 		mov	r0, r8
 		mov	r1, r6
@@ -289,7 +289,7 @@ restart:	adr	r0, LC0
 		bleq	atags_to_fdt
 
 		ldmfd	sp!, {r0-r3, ip, lr}
-		sub	sp, sp, #0x10000
+		sub	sp, sp, #0x20000
 #endif
 
 		mov	r8, r6			@ use the appended device tree


-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: N900 v3.19-rc5 arm atags_to_fdt.c is broken
  2015-01-23 22:36 ` Pali Rohár
@ 2015-01-23 23:48   ` Pavel Machek
  2015-01-24  9:04     ` Pali Rohár
  2015-01-26 16:14   ` Tony Lindgren
  1 sibling, 1 reply; 20+ messages in thread
From: Pavel Machek @ 2015-01-23 23:48 UTC (permalink / raw)
  To: Pali Rohár
  Cc: linux-omap, Linux Kernel Mailing List, Aaro Koskinen,
	Tony Lindgren, Sebastian Reichel, Ivaylo Dimitrov, Russell King,
	linux-arm-kernel, Laura Abbott

On Fri 2015-01-23 23:36:36, Pali Rohár wrote:
> On Friday 23 January 2015 22:39:55 Pali Rohár wrote:
> > Hello,
> > 
> > when I boot zImage with appended DT n900 in qemu
> > fdt_open_into() function called from file
> > arch/arm/boot/compressed/atags_to_fdt.c (in function
> > atags_to_fdt) always returns -FDT_ERR_NOSPACE.
> > 
> > It means that all ATAGS (including cmdline arguments) passed
> > by bootloader are ignored.
> > 
> > On real n900 device I see that booted DT version also ignore
> > cmdline arguments from bootloader. I cannot debug decompress
> > code on real device, but I think it is same problem as in
> > qemu.
> 
> Looks like this quick patch is fixing above problem:

So... something overruns stack, and bigger stack fixes it...?

							Pavel

> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index 68be901..4a7d75b 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -268,7 +268,7 @@ restart:	adr	r0, LC0
>  		 * area.  No GOT fixup has occurred yet, but none of the
>  		 * code we're about to call uses any global variable.
>  		*/
> -		add	sp, sp, #0x10000
> +		add	sp, sp, #0x20000
>  		stmfd	sp!, {r0-r3, ip, lr}
>  		mov	r0, r8
>  		mov	r1, r6
> @@ -289,7 +289,7 @@ restart:	adr	r0, LC0
>  		bleq	atags_to_fdt
>  
>  		ldmfd	sp!, {r0-r3, ip, lr}
> -		sub	sp, sp, #0x10000
> +		sub	sp, sp, #0x20000
>  #endif
>  
>  		mov	r8, r6			@ use the appended device tree
> 
> 



-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: N900 v3.19-rc5 arm atags_to_fdt.c is broken
  2015-01-23 23:48   ` Pavel Machek
@ 2015-01-24  9:04     ` Pali Rohár
  0 siblings, 0 replies; 20+ messages in thread
From: Pali Rohár @ 2015-01-24  9:04 UTC (permalink / raw)
  To: Pavel Machek
  Cc: linux-omap, Linux Kernel Mailing List, Aaro Koskinen,
	Tony Lindgren, Sebastian Reichel, Ivaylo Dimitrov, Russell King,
	linux-arm-kernel, Laura Abbott

[-- Attachment #1: Type: Text/Plain, Size: 1887 bytes --]

On Saturday 24 January 2015 00:48:55 Pavel Machek wrote:
> On Fri 2015-01-23 23:36:36, Pali Rohár wrote:
> > On Friday 23 January 2015 22:39:55 Pali Rohár wrote:
> > > Hello,
> > > 
> > > when I boot zImage with appended DT n900 in qemu
> > > fdt_open_into() function called from file
> > > arch/arm/boot/compressed/atags_to_fdt.c (in function
> > > atags_to_fdt) always returns -FDT_ERR_NOSPACE.
> > > 
> > > It means that all ATAGS (including cmdline arguments)
> > > passed by bootloader are ignored.
> > > 
> > > On real n900 device I see that booted DT version also
> > > ignore cmdline arguments from bootloader. I cannot debug
> > > decompress code on real device, but I think it is same
> > > problem as in qemu.
> > 
> > Looks like this quick patch is fixing above problem:
> So... something overruns stack, and bigger stack fixes it...?
> 
> 							Pavel
> 

Yes, it looks like that DTB file for n900 is too big and 
atags_to_fdt.c cannot append anything to it (kernel cmdline)...

> > diff --git a/arch/arm/boot/compressed/head.S
> > b/arch/arm/boot/compressed/head.S index 68be901..4a7d75b
> > 100644
> > --- a/arch/arm/boot/compressed/head.S
> > +++ b/arch/arm/boot/compressed/head.S
> > @@ -268,7 +268,7 @@ restart:	adr	r0, LC0
> > 
> >  		 * area.  No GOT fixup has occurred yet, but none of 
the
> >  		 * code we're about to call uses any global variable.
> >  		
> >  		*/
> > 
> > -		add	sp, sp, #0x10000
> > +		add	sp, sp, #0x20000
> > 
> >  		stmfd	sp!, {r0-r3, ip, lr}
> >  		mov	r0, r8
> >  		mov	r1, r6
> > 
> > @@ -289,7 +289,7 @@ restart:	adr	r0, LC0
> > 
> >  		bleq	atags_to_fdt
> >  		
> >  		ldmfd	sp!, {r0-r3, ip, lr}
> > 
> > -		sub	sp, sp, #0x10000
> > +		sub	sp, sp, #0x20000
> > 
> >  #endif
> >  
> >  		mov	r8, r6			@ use the appended device tree

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: N900 v3.19-rc5 arm atags_to_fdt.c is broken
  2015-01-23 22:36 ` Pali Rohár
  2015-01-23 23:48   ` Pavel Machek
@ 2015-01-26 16:14   ` Tony Lindgren
  2015-01-26 16:23     ` Pali Rohár
  1 sibling, 1 reply; 20+ messages in thread
From: Tony Lindgren @ 2015-01-26 16:14 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: linux-omap, Linux Kernel Mailing List, Aaro Koskinen,
	Sebastian Reichel, Ivaylo Dimitrov, Pavel Machek, Russell King,
	linux-arm-kernel, Laura Abbott, Pali Rohár

* Pali Rohár <pali.rohar@gmail.com> [150123 14:39]:
> On Friday 23 January 2015 22:39:55 Pali Rohár wrote:
> > Hello,
> > 
> > when I boot zImage with appended DT n900 in qemu
> > fdt_open_into() function called from file
> > arch/arm/boot/compressed/atags_to_fdt.c (in function
> > atags_to_fdt) always returns -FDT_ERR_NOSPACE.
> > 
> > It means that all ATAGS (including cmdline arguments) passed
> > by bootloader are ignored.
> > 
> > On real n900 device I see that booted DT version also ignore
> > cmdline arguments from bootloader. I cannot debug decompress
> > code on real device, but I think it is same problem as in
> > qemu.
> 
> Looks like this quick patch is fixing above problem:
> 
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index 68be901..4a7d75b 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -268,7 +268,7 @@ restart:	adr	r0, LC0
>  		 * area.  No GOT fixup has occurred yet, but none of the
>  		 * code we're about to call uses any global variable.
>  		*/
> -		add	sp, sp, #0x10000
> +		add	sp, sp, #0x20000
>  		stmfd	sp!, {r0-r3, ip, lr}
>  		mov	r0, r8
>  		mov	r1, r6
> @@ -289,7 +289,7 @@ restart:	adr	r0, LC0
>  		bleq	atags_to_fdt
>  
>  		ldmfd	sp!, {r0-r3, ip, lr}
> -		sub	sp, sp, #0x10000
> +		sub	sp, sp, #0x20000
>  #endif
>  
>  		mov	r8, r6			@ use the appended device tree

Nico, got any ideas about this one? This seems like a regression
somewhere..

Regards,

Tony
 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: N900 v3.19-rc5 arm atags_to_fdt.c is broken
  2015-01-26 16:14   ` Tony Lindgren
@ 2015-01-26 16:23     ` Pali Rohár
  2015-01-26 17:03       ` Tony Lindgren
  0 siblings, 1 reply; 20+ messages in thread
From: Pali Rohár @ 2015-01-26 16:23 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Nicolas Pitre, linux-omap, Linux Kernel Mailing List,
	Aaro Koskinen, Sebastian Reichel, Ivaylo Dimitrov, Pavel Machek,
	Russell King, linux-arm-kernel, Laura Abbott

[-- Attachment #1: Type: Text/Plain, Size: 1982 bytes --]

On Monday 26 January 2015 17:14:55 Tony Lindgren wrote:
> * Pali Rohár <pali.rohar@gmail.com> [150123 14:39]:
> > On Friday 23 January 2015 22:39:55 Pali Rohár wrote:
> > > Hello,
> > > 
> > > when I boot zImage with appended DT n900 in qemu
> > > fdt_open_into() function called from file
> > > arch/arm/boot/compressed/atags_to_fdt.c (in function
> > > atags_to_fdt) always returns -FDT_ERR_NOSPACE.
> > > 
> > > It means that all ATAGS (including cmdline arguments)
> > > passed by bootloader are ignored.
> > > 
> > > On real n900 device I see that booted DT version also
> > > ignore cmdline arguments from bootloader. I cannot debug
> > > decompress code on real device, but I think it is same
> > > problem as in qemu.
> > 
> > Looks like this quick patch is fixing above problem:
> > 
> > diff --git a/arch/arm/boot/compressed/head.S
> > b/arch/arm/boot/compressed/head.S index 68be901..4a7d75b
> > 100644
> > --- a/arch/arm/boot/compressed/head.S
> > +++ b/arch/arm/boot/compressed/head.S
> > @@ -268,7 +268,7 @@ restart:	adr	r0, LC0
> > 
> >  		 * area.  No GOT fixup has occurred yet, but none of 
the
> >  		 * code we're about to call uses any global variable.
> >  		
> >  		*/
> > 
> > -		add	sp, sp, #0x10000
> > +		add	sp, sp, #0x20000
> > 
> >  		stmfd	sp!, {r0-r3, ip, lr}
> >  		mov	r0, r8
> >  		mov	r1, r6
> > 
> > @@ -289,7 +289,7 @@ restart:	adr	r0, LC0
> > 
> >  		bleq	atags_to_fdt
> >  		
> >  		ldmfd	sp!, {r0-r3, ip, lr}
> > 
> > -		sub	sp, sp, #0x10000
> > +		sub	sp, sp, #0x20000
> > 
> >  #endif
> >  
> >  		mov	r8, r6			@ use the appended device tree
> 
> Nico, got any ideas about this one? This seems like a
> regression somewhere..
> 
> Regards,
> 
> Tony

$ du -b arch/arm/boot/dts/omap3-n900.dtb 
70212   arch/arm/boot/dts/omap3-n900.dtb

$ echo $((0x10000))
65536

I would say, problem is because omap3-n900 binary DT is too large

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: N900 v3.19-rc5 arm atags_to_fdt.c is broken
  2015-01-26 16:23     ` Pali Rohár
@ 2015-01-26 17:03       ` Tony Lindgren
  2015-01-26 18:57         ` Nicolas Pitre
  0 siblings, 1 reply; 20+ messages in thread
From: Tony Lindgren @ 2015-01-26 17:03 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Nicolas Pitre, linux-omap, Linux Kernel Mailing List,
	Aaro Koskinen, Sebastian Reichel, Ivaylo Dimitrov, Pavel Machek,
	Russell King, linux-arm-kernel, Laura Abbott

* Pali Rohár <pali.rohar@gmail.com> [150126 08:26]:
> On Monday 26 January 2015 17:14:55 Tony Lindgren wrote:
> > * Pali Rohár <pali.rohar@gmail.com> [150123 14:39]:
> > > On Friday 23 January 2015 22:39:55 Pali Rohár wrote:
> > > > Hello,
> > > > 
> > > > when I boot zImage with appended DT n900 in qemu
> > > > fdt_open_into() function called from file
> > > > arch/arm/boot/compressed/atags_to_fdt.c (in function
> > > > atags_to_fdt) always returns -FDT_ERR_NOSPACE.
> > > > 
> > > > It means that all ATAGS (including cmdline arguments)
> > > > passed by bootloader are ignored.
> > > > 
> > > > On real n900 device I see that booted DT version also
> > > > ignore cmdline arguments from bootloader. I cannot debug
> > > > decompress code on real device, but I think it is same
> > > > problem as in qemu.
> > > 
> > > Looks like this quick patch is fixing above problem:
> > > 
> > > diff --git a/arch/arm/boot/compressed/head.S
> > > b/arch/arm/boot/compressed/head.S index 68be901..4a7d75b
> > > 100644
> > > --- a/arch/arm/boot/compressed/head.S
> > > +++ b/arch/arm/boot/compressed/head.S
> > > @@ -268,7 +268,7 @@ restart:	adr	r0, LC0
> > > 
> > >  		 * area.  No GOT fixup has occurred yet, but none of 
> the
> > >  		 * code we're about to call uses any global variable.
> > >  		
> > >  		*/
> > > 
> > > -		add	sp, sp, #0x10000
> > > +		add	sp, sp, #0x20000
> > > 
> > >  		stmfd	sp!, {r0-r3, ip, lr}
> > >  		mov	r0, r8
> > >  		mov	r1, r6
> > > 
> > > @@ -289,7 +289,7 @@ restart:	adr	r0, LC0
> > > 
> > >  		bleq	atags_to_fdt
> > >  		
> > >  		ldmfd	sp!, {r0-r3, ip, lr}
> > > 
> > > -		sub	sp, sp, #0x10000
> > > +		sub	sp, sp, #0x20000
> > > 
> > >  #endif
> > >  
> > >  		mov	r8, r6			@ use the appended device tree
> > 
> > Nico, got any ideas about this one? This seems like a
> > regression somewhere..
> > 
> > Regards,
> > 
> > Tony
> 
> $ du -b arch/arm/boot/dts/omap3-n900.dtb 
> 70212   arch/arm/boot/dts/omap3-n900.dtb
> 
> $ echo $((0x10000))
> 65536
> 
> I would say, problem is because omap3-n900 binary DT is too large

OK if that's the case, then your patch makes sense to me. It also
seems we can have the temporary stack be larger than the initial
stack just for atags_to_fdt.

Regards,

Tony

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: N900 v3.19-rc5 arm atags_to_fdt.c is broken
  2015-01-26 17:03       ` Tony Lindgren
@ 2015-01-26 18:57         ` Nicolas Pitre
  2015-01-26 21:48           ` Pavel Machek
  2015-01-27 13:37           ` Pavel Machek
  0 siblings, 2 replies; 20+ messages in thread
From: Nicolas Pitre @ 2015-01-26 18:57 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Pali Rohár, linux-omap, Linux Kernel Mailing List,
	Aaro Koskinen, Sebastian Reichel, Ivaylo Dimitrov, Pavel Machek,
	Russell King, linux-arm-kernel, Laura Abbott

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4744 bytes --]

On Mon, 26 Jan 2015, Tony Lindgren wrote:

> * Pali Rohár <pali.rohar@gmail.com> [150126 08:26]:
> > On Monday 26 January 2015 17:14:55 Tony Lindgren wrote:
> > > * Pali Rohár <pali.rohar@gmail.com> [150123 14:39]:
> > > > On Friday 23 January 2015 22:39:55 Pali Rohár wrote:
> > > > > Hello,
> > > > > 
> > > > > when I boot zImage with appended DT n900 in qemu
> > > > > fdt_open_into() function called from file
> > > > > arch/arm/boot/compressed/atags_to_fdt.c (in function
> > > > > atags_to_fdt) always returns -FDT_ERR_NOSPACE.
> > > > > 
> > > > > It means that all ATAGS (including cmdline arguments)
> > > > > passed by bootloader are ignored.
> > > > > 
> > > > > On real n900 device I see that booted DT version also
> > > > > ignore cmdline arguments from bootloader. I cannot debug
> > > > > decompress code on real device, but I think it is same
> > > > > problem as in qemu.
> > > > 
> > > > Looks like this quick patch is fixing above problem:
> > > > 
> > > > diff --git a/arch/arm/boot/compressed/head.S
> > > > b/arch/arm/boot/compressed/head.S index 68be901..4a7d75b
> > > > 100644
> > > > --- a/arch/arm/boot/compressed/head.S
> > > > +++ b/arch/arm/boot/compressed/head.S
> > > > @@ -268,7 +268,7 @@ restart:	adr	r0, LC0
> > > > 
> > > >  		 * area.  No GOT fixup has occurred yet, but none of 
> > the
> > > >  		 * code we're about to call uses any global variable.
> > > >  		
> > > >  		*/
> > > > 
> > > > -		add	sp, sp, #0x10000
> > > > +		add	sp, sp, #0x20000
> > > > 
> > > >  		stmfd	sp!, {r0-r3, ip, lr}
> > > >  		mov	r0, r8
> > > >  		mov	r1, r6
> > > > 
> > > > @@ -289,7 +289,7 @@ restart:	adr	r0, LC0
> > > > 
> > > >  		bleq	atags_to_fdt
> > > >  		
> > > >  		ldmfd	sp!, {r0-r3, ip, lr}
> > > > 
> > > > -		sub	sp, sp, #0x10000
> > > > +		sub	sp, sp, #0x20000
> > > > 
> > > >  #endif
> > > >  
> > > >  		mov	r8, r6			@ use the appended device tree
> > > 
> > > Nico, got any ideas about this one? This seems like a
> > > regression somewhere..
> > > 
> > > Regards,
> > > 
> > > Tony
> > 
> > $ du -b arch/arm/boot/dts/omap3-n900.dtb 
> > 70212   arch/arm/boot/dts/omap3-n900.dtb
> > 
> > $ echo $((0x10000))
> > 65536
> > 
> > I would say, problem is because omap3-n900 binary DT is too large

I agree.

> OK if that's the case, then your patch makes sense to me. It also
> seems we can have the temporary stack be larger than the initial
> stack just for atags_to_fdt.

The stack size isn't the issue, but rather its location.  We need to 
position it away from the DT data.  The DT size is known and we could 
use that, plus some room for the insertion of new data coming from the
ATAG conversion.

Something like the following would be a more robust solution:

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 68be901759..89718de9dd 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -263,16 +263,37 @@ restart:	adr	r0, LC0
 		 * OK... Let's do some funky business here.
 		 * If we do have a DTB appended to zImage, and we do have
 		 * an ATAG list around, we want the later to be translated
-		 * and folded into the former here.  To be on the safe side,
-		 * let's temporarily move  the stack away into the malloc
-		 * area.  No GOT fixup has occurred yet, but none of the
-		 * code we're about to call uses any global variable.
+		 * and folded into the former here. No GOT fixup has occurred
+		 * yet, but none of the code we're about to call uses any
+		 * global variable.
 		*/
-		add	sp, sp, #0x10000
+
+		/* Get the initial DTB size */
+		ldr	r5, [r6, #4]
+#ifndef __ARMEB__
+		/* convert to little endian */
+		eor	r1, r5, r5, ror #16
+		bic	r1, r1, #0x00ff0000
+		mov	r5, r5, ror #8
+		eor	r5, r5, r1, lsr #8
+#endif
+		/* 50% DTB growth should be good enough */
+		add	r5, r5, r5, lsr #1
+		/* preserve 64-bit alignment */
+		add	r5, r5, #7
+		bic	r5, r5, #7
+		/* clamp to 32KB min and 1MB max */
+		movs	r1, r5, lsr #15
+		moveq	r5, #(1 << 15)
+		movs	r1, r5, lsr #20
+		movne	r5, #(1 << 20)
+
+		/* Temporarily relocate the stack past the DTB work space */
+		add	sp, sp, r5
 		stmfd	sp!, {r0-r3, ip, lr}
 		mov	r0, r8
 		mov	r1, r6
-		sub	r2, sp, r6
+		mov	r2, r5
 		bl	atags_to_fdt
 
 		/*
@@ -285,11 +306,11 @@ restart:	adr	r0, LC0
 		bic	r0, r0, #1
 		add	r0, r0, #0x100
 		mov	r1, r6
-		sub	r2, sp, r6
+		mov	r2, r5
 		bleq	atags_to_fdt
 
 		ldmfd	sp!, {r0-r3, ip, lr}
-		sub	sp, sp, #0x10000
+		sub	sp, sp, r5
 #endif
 
 		mov	r8, r6			@ use the appended device tree
@@ -306,7 +327,7 @@ restart:	adr	r0, LC0
 		subs	r1, r5, r1
 		addhi	r9, r9, r1
 
-		/* Get the dtb's size */
+		/* Get the current DTB size */
 		ldr	r5, [r6, #4]
 #ifndef __ARMEB__
 		/* convert r5 (dtb size) to little endian */

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: N900 v3.19-rc5 arm atags_to_fdt.c is broken
  2015-01-26 18:57         ` Nicolas Pitre
@ 2015-01-26 21:48           ` Pavel Machek
  2015-01-27  0:06             ` Nicolas Pitre
  2015-01-27 13:37           ` Pavel Machek
  1 sibling, 1 reply; 20+ messages in thread
From: Pavel Machek @ 2015-01-26 21:48 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Tony Lindgren, Pali Rohár, linux-omap,
	Linux Kernel Mailing List, Aaro Koskinen, Sebastian Reichel,
	Ivaylo Dimitrov, Russell King, linux-arm-kernel, Laura Abbott

Hi!

> > > $ du -b arch/arm/boot/dts/omap3-n900.dtb 
> > > 70212   arch/arm/boot/dts/omap3-n900.dtb
> > > 
> > > $ echo $((0x10000))
> > > 65536
> > > 
> > > I would say, problem is because omap3-n900 binary DT is too large
> 
> I agree.
> 
> > OK if that's the case, then your patch makes sense to me. It also
> > seems we can have the temporary stack be larger than the initial
> > stack just for atags_to_fdt.
> 
> The stack size isn't the issue, but rather its location.  We need to 
> position it away from the DT data.  The DT size is known and we could 
> use that, plus some room for the insertion of new data coming from the
> ATAG conversion.
> 
> Something like the following would be a more robust solution:
> 
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index 68be901759..89718de9dd 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -263,16 +263,37 @@ restart:	adr	r0, LC0
>  		 * OK... Let's do some funky business here.
>  		 * If we do have a DTB appended to zImage, and we do have
>  		 * an ATAG list around, we want the later to be translated
> -		 * and folded into the former here.  To be on the safe side,
> -		 * let's temporarily move  the stack away into the malloc
> -		 * area.  No GOT fixup has occurred yet, but none of the
> -		 * code we're about to call uses any global variable.
> +		 * and folded into the former here. No GOT fixup has occurred
> +		 * yet, but none of the code we're about to call uses any
> +		 * global variable.
>  		*/
> -		add	sp, sp, #0x10000
> +
> +		/* Get the initial DTB size */
> +		ldr	r5, [r6, #4]
> +#ifndef __ARMEB__
> +		/* convert to little endian */
> +		eor	r1, r5, r5, ror #16
> +		bic	r1, r1, #0x00ff0000
> +		mov	r5, r5, ror #8
> +		eor	r5, r5, r1, lsr #8
> +#endif
> +		/* 50% DTB growth should be good enough */
> +		add	r5, r5, r5, lsr #1
> +		/* preserve 64-bit alignment */
> +		add	r5, r5, #7
> +		bic	r5, r5, #7
> +		/* clamp to 32KB min and 1MB max */
> +		movs	r1, r5, lsr #15
> +		moveq	r5, #(1 << 15)
> +		movs	r1, r5, lsr #20
> +		movne	r5, #(1 << 20)

Dunno. Would it be easier/simpler to just use 1MB, always? Do we
support machines with <16MB RAM?

Best regards,
								

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: N900 v3.19-rc5 arm atags_to_fdt.c is broken
  2015-01-26 21:48           ` Pavel Machek
@ 2015-01-27  0:06             ` Nicolas Pitre
  0 siblings, 0 replies; 20+ messages in thread
From: Nicolas Pitre @ 2015-01-27  0:06 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Tony Lindgren, Pali Rohár, linux-omap,
	Linux Kernel Mailing List, Aaro Koskinen, Sebastian Reichel,
	Ivaylo Dimitrov, Russell King, linux-arm-kernel, Laura Abbott

On Mon, 26 Jan 2015, Pavel Machek wrote:

> Hi!
> 
> > > > $ du -b arch/arm/boot/dts/omap3-n900.dtb 
> > > > 70212   arch/arm/boot/dts/omap3-n900.dtb
> > > > 
> > > > $ echo $((0x10000))
> > > > 65536
> > > > 
> > > > I would say, problem is because omap3-n900 binary DT is too large
> > 
> > I agree.
> > 
> > > OK if that's the case, then your patch makes sense to me. It also
> > > seems we can have the temporary stack be larger than the initial
> > > stack just for atags_to_fdt.
> > 
> > The stack size isn't the issue, but rather its location.  We need to 
> > position it away from the DT data.  The DT size is known and we could 
> > use that, plus some room for the insertion of new data coming from the
> > ATAG conversion.
> > 
> > Something like the following would be a more robust solution:
> > 
> > diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> > index 68be901759..89718de9dd 100644
> > --- a/arch/arm/boot/compressed/head.S
> > +++ b/arch/arm/boot/compressed/head.S
> > @@ -263,16 +263,37 @@ restart:	adr	r0, LC0
> >  		 * OK... Let's do some funky business here.
> >  		 * If we do have a DTB appended to zImage, and we do have
> >  		 * an ATAG list around, we want the later to be translated
> > -		 * and folded into the former here.  To be on the safe side,
> > -		 * let's temporarily move  the stack away into the malloc
> > -		 * area.  No GOT fixup has occurred yet, but none of the
> > -		 * code we're about to call uses any global variable.
> > +		 * and folded into the former here. No GOT fixup has occurred
> > +		 * yet, but none of the code we're about to call uses any
> > +		 * global variable.
> >  		*/
> > -		add	sp, sp, #0x10000
> > +
> > +		/* Get the initial DTB size */
> > +		ldr	r5, [r6, #4]
> > +#ifndef __ARMEB__
> > +		/* convert to little endian */
> > +		eor	r1, r5, r5, ror #16
> > +		bic	r1, r1, #0x00ff0000
> > +		mov	r5, r5, ror #8
> > +		eor	r5, r5, r1, lsr #8
> > +#endif
> > +		/* 50% DTB growth should be good enough */
> > +		add	r5, r5, r5, lsr #1
> > +		/* preserve 64-bit alignment */
> > +		add	r5, r5, #7
> > +		bic	r5, r5, #7
> > +		/* clamp to 32KB min and 1MB max */
> > +		movs	r1, r5, lsr #15
> > +		moveq	r5, #(1 << 15)
> > +		movs	r1, r5, lsr #20
> > +		movne	r5, #(1 << 20)
> 
> Dunno. Would it be easier/simpler to just use 1MB, always? Do we
> support machines with <16MB RAM?

If people are used to put other things relatively close to the kernel 
image like, say, some initrd image, then I'd prefer to be more 
conservative and avoid spreading out too much.


Nicolas

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: N900 v3.19-rc5 arm atags_to_fdt.c is broken
  2015-01-26 18:57         ` Nicolas Pitre
  2015-01-26 21:48           ` Pavel Machek
@ 2015-01-27 13:37           ` Pavel Machek
  2015-01-27 15:16             ` Nicolas Pitre
  1 sibling, 1 reply; 20+ messages in thread
From: Pavel Machek @ 2015-01-27 13:37 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Tony Lindgren, Pali Rohár, linux-omap,
	Linux Kernel Mailing List, Aaro Koskinen, Sebastian Reichel,
	Ivaylo Dimitrov, Russell King, linux-arm-kernel, Laura Abbott


> > > I would say, problem is because omap3-n900 binary DT is too large
> 
> I agree.
> 
> > OK if that's the case, then your patch makes sense to me. It also
> > seems we can have the temporary stack be larger than the initial
> > stack just for atags_to_fdt.
> 
> The stack size isn't the issue, but rather its location.  We need to 
> position it away from the DT data.  The DT size is known and we could 
> use that, plus some room for the insertion of new data coming from the
> ATAG conversion.
> 
> Something like the following would be a more robust solution:

Tested-by: Pavel Machek <pavel@ucw.cz>

(Note, that in 3.19 dts for n900 got too big, so we are actually
triggering old bugs. That means that this is a regression fix, and
should go in ASAP).



> 
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index 68be901759..89718de9dd 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -263,16 +263,37 @@ restart:	adr	r0, LC0
>  		 * OK... Let's do some funky business here.
>  		 * If we do have a DTB appended to zImage, and we do have
>  		 * an ATAG list around, we want the later to be translated
> -		 * and folded into the former here.  To be on the safe side,
> -		 * let's temporarily move  the stack away into the malloc
> -		 * area.  No GOT fixup has occurred yet, but none of the
> -		 * code we're about to call uses any global variable.
> +		 * and folded into the former here. No GOT fixup has occurred
> +		 * yet, but none of the code we're about to call uses any
> +		 * global variable.
>  		*/
> -		add	sp, sp, #0x10000
> +
> +		/* Get the initial DTB size */
> +		ldr	r5, [r6, #4]
> +#ifndef __ARMEB__
> +		/* convert to little endian */
> +		eor	r1, r5, r5, ror #16
> +		bic	r1, r1, #0x00ff0000
> +		mov	r5, r5, ror #8
> +		eor	r5, r5, r1, lsr #8
> +#endif
> +		/* 50% DTB growth should be good enough */
> +		add	r5, r5, r5, lsr #1
> +		/* preserve 64-bit alignment */
> +		add	r5, r5, #7
> +		bic	r5, r5, #7
> +		/* clamp to 32KB min and 1MB max */
> +		movs	r1, r5, lsr #15
> +		moveq	r5, #(1 << 15)
> +		movs	r1, r5, lsr #20
> +		movne	r5, #(1 << 20)
> +
> +		/* Temporarily relocate the stack past the DTB work space */
> +		add	sp, sp, r5
>  		stmfd	sp!, {r0-r3, ip, lr}
>  		mov	r0, r8
>  		mov	r1, r6
> -		sub	r2, sp, r6
> +		mov	r2, r5
>  		bl	atags_to_fdt
>  
>  		/*
> @@ -285,11 +306,11 @@ restart:	adr	r0, LC0
>  		bic	r0, r0, #1
>  		add	r0, r0, #0x100
>  		mov	r1, r6
> -		sub	r2, sp, r6
> +		mov	r2, r5
>  		bleq	atags_to_fdt
>  
>  		ldmfd	sp!, {r0-r3, ip, lr}
> -		sub	sp, sp, #0x10000
> +		sub	sp, sp, r5
>  #endif
>  
>  		mov	r8, r6			@ use the appended device tree
> @@ -306,7 +327,7 @@ restart:	adr	r0, LC0
>  		subs	r1, r5, r1
>  		addhi	r9, r9, r1
>  
> -		/* Get the dtb's size */
> +		/* Get the current DTB size */
>  		ldr	r5, [r6, #4]
>  #ifndef __ARMEB__
>  		/* convert r5 (dtb size) to little endian */



-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: N900 v3.19-rc5 arm atags_to_fdt.c is broken
  2015-01-27 13:37           ` Pavel Machek
@ 2015-01-27 15:16             ` Nicolas Pitre
  2015-01-27 15:26               ` Pali Rohár
                                 ` (3 more replies)
  0 siblings, 4 replies; 20+ messages in thread
From: Nicolas Pitre @ 2015-01-27 15:16 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Tony Lindgren, Pali Rohár, linux-omap,
	Linux Kernel Mailing List, Aaro Koskinen, Sebastian Reichel,
	Ivaylo Dimitrov, Russell King, linux-arm-kernel, Laura Abbott

On Tue, 27 Jan 2015, Pavel Machek wrote:

> 
> > > > I would say, problem is because omap3-n900 binary DT is too large
> > 
> > I agree.
> > 
> > > OK if that's the case, then your patch makes sense to me. It also
> > > seems we can have the temporary stack be larger than the initial
> > > stack just for atags_to_fdt.
> > 
> > The stack size isn't the issue, but rather its location.  We need to 
> > position it away from the DT data.  The DT size is known and we could 
> > use that, plus some room for the insertion of new data coming from the
> > ATAG conversion.
> > 
> > Something like the following would be a more robust solution:
> 
> Tested-by: Pavel Machek <pavel@ucw.cz>
> 
> (Note, that in 3.19 dts for n900 got too big, so we are actually
> triggering old bugs. That means that this is a regression fix, and
> should go in ASAP).

It is queued here:
http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8294/1


Nicolas

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: N900 v3.19-rc5 arm atags_to_fdt.c is broken
  2015-01-27 15:16             ` Nicolas Pitre
@ 2015-01-27 15:26               ` Pali Rohár
  2015-01-27 15:26               ` Tony Lindgren
                                 ` (2 subsequent siblings)
  3 siblings, 0 replies; 20+ messages in thread
From: Pali Rohár @ 2015-01-27 15:26 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Pavel Machek, Tony Lindgren, linux-omap,
	Linux Kernel Mailing List, Aaro Koskinen, Sebastian Reichel,
	Ivaylo Dimitrov, Russell King, linux-arm-kernel, Laura Abbott

[-- Attachment #1: Type: Text/Plain, Size: 1311 bytes --]

On Tuesday 27 January 2015 16:16:24 Nicolas Pitre wrote:
> On Tue, 27 Jan 2015, Pavel Machek wrote:
> > > > > I would say, problem is because omap3-n900 binary DT
> > > > > is too large
> > > 
> > > I agree.
> > > 
> > > > OK if that's the case, then your patch makes sense to
> > > > me. It also seems we can have the temporary stack be
> > > > larger than the initial stack just for atags_to_fdt.
> > > 
> > > The stack size isn't the issue, but rather its location. 
> > > We need to position it away from the DT data.  The DT
> > > size is known and we could use that, plus some room for
> > > the insertion of new data coming from the ATAG
> > > conversion.
> > 
> > > Something like the following would be a more robust 
solution:
> > Tested-by: Pavel Machek <pavel@ucw.cz>
> > 
> > (Note, that in 3.19 dts for n900 got too big, so we are
> > actually triggering old bugs. That means that this is a
> > regression fix, and should go in ASAP).
> 
> It is queued here:
> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id
> =8294/1
> 
> 
> Nicolas

I tested your patch and fdt_open_into() does not return error 
FDT_ERR_NOSPACE anymore. It is working. So you can add my

Tested-by: Pali Rohár <pali.rohar@gmail.com>

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: N900 v3.19-rc5 arm atags_to_fdt.c is broken
  2015-01-27 15:16             ` Nicolas Pitre
  2015-01-27 15:26               ` Pali Rohár
@ 2015-01-27 15:26               ` Tony Lindgren
  2015-01-27 17:53               ` Russell King - ARM Linux
  2015-02-02 13:39               ` Pavel Machek
  3 siblings, 0 replies; 20+ messages in thread
From: Tony Lindgren @ 2015-01-27 15:26 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Pavel Machek, Pali Rohár, linux-omap,
	Linux Kernel Mailing List, Aaro Koskinen, Sebastian Reichel,
	Ivaylo Dimitrov, Russell King, linux-arm-kernel, Laura Abbott

* Nicolas Pitre <nicolas.pitre@linaro.org> [150127 07:19]:
> On Tue, 27 Jan 2015, Pavel Machek wrote:
> 
> > 
> > > > > I would say, problem is because omap3-n900 binary DT is too large
> > > 
> > > I agree.
> > > 
> > > > OK if that's the case, then your patch makes sense to me. It also
> > > > seems we can have the temporary stack be larger than the initial
> > > > stack just for atags_to_fdt.
> > > 
> > > The stack size isn't the issue, but rather its location.  We need to 
> > > position it away from the DT data.  The DT size is known and we could 
> > > use that, plus some room for the insertion of new data coming from the
> > > ATAG conversion.
> > > 
> > > Something like the following would be a more robust solution:
> > 
> > Tested-by: Pavel Machek <pavel@ucw.cz>
> > 
> > (Note, that in 3.19 dts for n900 got too big, so we are actually
> > triggering old bugs. That means that this is a regression fix, and
> > should go in ASAP).
> 
> It is queued here:
> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8294/1

Thanks for fixing that one!

Regards,

Tony

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: N900 v3.19-rc5 arm atags_to_fdt.c is broken
  2015-01-27 15:16             ` Nicolas Pitre
  2015-01-27 15:26               ` Pali Rohár
  2015-01-27 15:26               ` Tony Lindgren
@ 2015-01-27 17:53               ` Russell King - ARM Linux
  2015-01-27 18:50                 ` Nicolas Pitre
  2015-02-02 13:39               ` Pavel Machek
  3 siblings, 1 reply; 20+ messages in thread
From: Russell King - ARM Linux @ 2015-01-27 17:53 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Pavel Machek, Tony Lindgren, Pali Rohár, linux-omap,
	Linux Kernel Mailing List, Aaro Koskinen, Sebastian Reichel,
	Ivaylo Dimitrov, linux-arm-kernel, Laura Abbott

On Tue, Jan 27, 2015 at 10:16:24AM -0500, Nicolas Pitre wrote:
> On Tue, 27 Jan 2015, Pavel Machek wrote:
> > (Note, that in 3.19 dts for n900 got too big, so we are actually
> > triggering old bugs. That means that this is a regression fix, and
> > should go in ASAP).
> 
> It is queued here:
> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8294/1

Irrespective of that patch, I commented on another thread (about adding
the serial number to DT):

| How about something a little more radical.
| 
| Rather than trying to squeeze various ATAGs into DT, why don't we add a
| standard ATAG to contain the DT and pass that through into the kernel.
| This is IMHO how we _should_ have done the ATAG compatibility from the
| start.
| 
| That means we could get rid of most of the libfdt in the decompressor,
| and instead resolve the differences in the kernel.

That would allow us to get rid of all the FDT compatibility code in the
decompressor, and when we encounter these special ATAGs, we can deal with
them in the kernel instead of having to mess around with the DT.  I'd
also assume (although I haven't checked) that it would be much easier to
add additional DT properties in the kernel environment, rather than in
the rather restrictive decompressor environment.

We'd need to be careful about how we deal with the atags vs DT.  I'd
suggest (as above) that we have a tag which points at the DT in
physical memory, and its size, and pre-scan the atag list for that.
When we find it, we kick off the DT based stuff as we would normally
do, and follow it with a parse of the atags.  This would cause the
atags to override whatever is in the DT, which afaics is the behaviour
that we actually want.

We could augment that with code to update DT too so that we can export
just the DT based information to kexec'd kernels.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: N900 v3.19-rc5 arm atags_to_fdt.c is broken
  2015-01-27 17:53               ` Russell King - ARM Linux
@ 2015-01-27 18:50                 ` Nicolas Pitre
  2015-01-27 23:09                   ` Aaro Koskinen
  2015-01-27 23:17                   ` Pavel Machek
  0 siblings, 2 replies; 20+ messages in thread
From: Nicolas Pitre @ 2015-01-27 18:50 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Pavel Machek, Tony Lindgren, Pali Rohár, linux-omap,
	Linux Kernel Mailing List, Aaro Koskinen, Sebastian Reichel,
	Ivaylo Dimitrov, linux-arm-kernel, Laura Abbott

On Tue, 27 Jan 2015, Russell King - ARM Linux wrote:

> On Tue, Jan 27, 2015 at 10:16:24AM -0500, Nicolas Pitre wrote:
> > On Tue, 27 Jan 2015, Pavel Machek wrote:
> > > (Note, that in 3.19 dts for n900 got too big, so we are actually
> > > triggering old bugs. That means that this is a regression fix, and
> > > should go in ASAP).
> > 
> > It is queued here:
> > http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8294/1
> 
> Irrespective of that patch, I commented on another thread (about adding
> the serial number to DT):
> 
> | How about something a little more radical.
> | 
> | Rather than trying to squeeze various ATAGs into DT, why don't we add a
> | standard ATAG to contain the DT and pass that through into the kernel.
> | This is IMHO how we _should_ have done the ATAG compatibility from the
> | start.
> | 
> | That means we could get rid of most of the libfdt in the decompressor,
> | and instead resolve the differences in the kernel.
> 
> That would allow us to get rid of all the FDT compatibility code in the
> decompressor, and when we encounter these special ATAGs, we can deal with
> them in the kernel instead of having to mess around with the DT.  I'd
> also assume (although I haven't checked) that it would be much easier to
> add additional DT properties in the kernel environment, rather than in
> the rather restrictive decompressor environment.

Well...

I was initially against any ATAG-to-DT compatibility at all.  IMHO the 
"right" solution is to update the bootloader.

However people complained that updating the bootloader wasn't possible. 
I therefore suggested they go with a shim layer I termed the "impedance 
matcher" acting like an intermediate boot stage converting their (often 
unofficial and out-of-tree) ATAGs into proper DT nodes.  Some people did 
exactly that and it works wonderfully for them. It also works 
wonderfully for kernel maintenance as the platform specific tweaks are 
kept out of the kernel tree.

Still, that wasn't good enough for some cases as the move to DT 
regressed some platforms that just used to work before.  Concatenating a 
DTB to zImage was tolerable but installing an additional shim layer was 
too much.  Hence the ATAG-to-DT compat code in the decompressor.  This 
was meant to smooth things around the transition to DT, etc.  After all, 
those devices with non-replaceable bootloaders where shim layers are not 
possible should get out of commission eventually?

> We'd need to be careful about how we deal with the atags vs DT.  I'd
> suggest (as above) that we have a tag which points at the DT in
> physical memory, and its size, and pre-scan the atag list for that.
> When we find it, we kick off the DT based stuff as we would normally
> do, and follow it with a parse of the atags.  This would cause the
> atags to override whatever is in the DT, which afaics is the behaviour
> that we actually want.

Maybe.  However that would be yet another scheme for supporting legacy 
platforms with locked bootloaders.  I really think we have enough of 
them already and I'd much prefer if we started _removing_ them not 
creating additional ones.  The ATAG-to-DT code in the bootloader is 
still rather small and it works quite well, and it allows for 
configuring out ATAG support out of the kernel entirely.  It should be 
sufficient for all the official (e.g. upstream) ATAGs already.  If 
that's not good enough then people should consider the impedance matcher 
approach.

> We could augment that with code to update DT too so that we can export
> just the DT based information to kexec'd kernels.

That should already be the case, no?


Nicolas

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: N900 v3.19-rc5 arm atags_to_fdt.c is broken
  2015-01-27 18:50                 ` Nicolas Pitre
@ 2015-01-27 23:09                   ` Aaro Koskinen
  2015-01-27 23:17                   ` Pavel Machek
  1 sibling, 0 replies; 20+ messages in thread
From: Aaro Koskinen @ 2015-01-27 23:09 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Russell King - ARM Linux, Pavel Machek, Tony Lindgren,
	Pali Rohár, linux-omap, Linux Kernel Mailing List,
	Sebastian Reichel, Ivaylo Dimitrov, linux-arm-kernel,
	Laura Abbott

On Tue, Jan 27, 2015 at 01:50:22PM -0500, Nicolas Pitre wrote:
> Hence the ATAG-to-DT compat code in the decompressor.  This 
> was meant to smooth things around the transition to DT, etc.  After all, 
> those devices with non-replaceable bootloaders where shim layers are not 
> possible should get out of commission eventually?

Probably after 5 years there are still people using and hacking mainline
Linux with N900 etc., and newer OMAPs (since there no "useful" devices)
are long forgotten...

A.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: N900 v3.19-rc5 arm atags_to_fdt.c is broken
  2015-01-27 18:50                 ` Nicolas Pitre
  2015-01-27 23:09                   ` Aaro Koskinen
@ 2015-01-27 23:17                   ` Pavel Machek
  1 sibling, 0 replies; 20+ messages in thread
From: Pavel Machek @ 2015-01-27 23:17 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Russell King - ARM Linux, Tony Lindgren, Pali Rohár,
	linux-omap, Linux Kernel Mailing List, Aaro Koskinen,
	Sebastian Reichel, Ivaylo Dimitrov, linux-arm-kernel,
	Laura Abbott

On Tue 2015-01-27 13:50:22, Nicolas Pitre wrote:
> On Tue, 27 Jan 2015, Russell King - ARM Linux wrote:
> 
> > On Tue, Jan 27, 2015 at 10:16:24AM -0500, Nicolas Pitre wrote:
> > > On Tue, 27 Jan 2015, Pavel Machek wrote:
> > > > (Note, that in 3.19 dts for n900 got too big, so we are actually
> > > > triggering old bugs. That means that this is a regression fix, and
> > > > should go in ASAP).
> > > 
> > > It is queued here:
> > > http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8294/1
> > 
> > Irrespective of that patch, I commented on another thread (about adding
> > the serial number to DT):
> > 
> > | How about something a little more radical.
> > | 
> > | Rather than trying to squeeze various ATAGs into DT, why don't we add a
> > | standard ATAG to contain the DT and pass that through into the kernel.
> > | This is IMHO how we _should_ have done the ATAG compatibility from the
> > | start.
> > | 
> > | That means we could get rid of most of the libfdt in the decompressor,
> > | and instead resolve the differences in the kernel.
> > 
> > That would allow us to get rid of all the FDT compatibility code in the
> > decompressor, and when we encounter these special ATAGs, we can deal with
> > them in the kernel instead of having to mess around with the DT.  I'd
> > also assume (although I haven't checked) that it would be much easier to
> > add additional DT properties in the kernel environment, rather than in
> > the rather restrictive decompressor environment.
> 
> Well...
> 
> I was initially against any ATAG-to-DT compatibility at all.  IMHO the 
> "right" solution is to update the bootloader.
> 
> However people complained that updating the bootloader wasn't possible. 
> I therefore suggested they go with a shim layer I termed the "impedance 
> matcher" acting like an intermediate boot stage converting their (often 
> unofficial and out-of-tree) ATAGs into proper DT nodes.  Some people did 
> exactly that and it works wonderfully for them. It also works 
> wonderfully for kernel maintenance as the platform specific tweaks are 
> kept out of the kernel tree.
> 
> Still, that wasn't good enough for some cases as the move to DT 
> regressed some platforms that just used to work before.  Concatenating a 
> DTB to zImage was tolerable but installing an additional shim layer was 
> too much.  Hence the ATAG-to-DT compat code in the decompressor.  This 
> was meant to smooth things around the transition to DT, etc.  After all, 
> those devices with non-replaceable bootloaders where shim layers are not 
> possible should get out of commission eventually?

N900 is still phone that is best-supported by the mainline,
AFAICT. Yes, it is rather sad, but it means that no, it is probably
not going out of commission anytime soon.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: N900 v3.19-rc5 arm atags_to_fdt.c is broken
  2015-01-27 15:16             ` Nicolas Pitre
                                 ` (2 preceding siblings ...)
  2015-01-27 17:53               ` Russell King - ARM Linux
@ 2015-02-02 13:39               ` Pavel Machek
  2015-02-02 13:56                 ` Nicolas Pitre
  3 siblings, 1 reply; 20+ messages in thread
From: Pavel Machek @ 2015-02-02 13:39 UTC (permalink / raw)
  To: Nicolas Pitre, Linus Torvalds
  Cc: Tony Lindgren, Pali Rohár, linux-omap,
	Linux Kernel Mailing List, Aaro Koskinen, Sebastian Reichel,
	Ivaylo Dimitrov, Russell King, linux-arm-kernel, Laura Abbott,
	Russell King

On Tue 2015-01-27 10:16:24, Nicolas Pitre wrote:
> On Tue, 27 Jan 2015, Pavel Machek wrote:
> 
> > 
> > > > > I would say, problem is because omap3-n900 binary DT is too large
> > > 
> > > I agree.
> > > 
> > > > OK if that's the case, then your patch makes sense to me. It also
> > > > seems we can have the temporary stack be larger than the initial
> > > > stack just for atags_to_fdt.
> > > 
> > > The stack size isn't the issue, but rather its location.  We need to 
> > > position it away from the DT data.  The DT size is known and we could 
> > > use that, plus some room for the insertion of new data coming from the
> > > ATAG conversion.
> > > 
> > > Something like the following would be a more robust solution:
> > 
> > Tested-by: Pavel Machek <pavel@ucw.cz>
> > 
> > (Note, that in 3.19 dts for n900 got too big, so we are actually
> > triggering old bugs. That means that this is a regression fix, and
> > should go in ASAP).
> 
> It is queued here:
> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8294/1

Hmm, but it should be on kernel.org, not in private arm trees, as it
fixes a regression. Russell, you are the ARM maintainer, can you push
it to Linus?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: N900 v3.19-rc5 arm atags_to_fdt.c is broken
  2015-02-02 13:39               ` Pavel Machek
@ 2015-02-02 13:56                 ` Nicolas Pitre
  0 siblings, 0 replies; 20+ messages in thread
From: Nicolas Pitre @ 2015-02-02 13:56 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Linus Torvalds, Tony Lindgren, Pali Rohár, linux-omap,
	Linux Kernel Mailing List, Aaro Koskinen, Sebastian Reichel,
	Ivaylo Dimitrov, Russell King, linux-arm-kernel, Laura Abbott,
	Russell King

On Mon, 2 Feb 2015, Pavel Machek wrote:

> On Tue 2015-01-27 10:16:24, Nicolas Pitre wrote:
> > On Tue, 27 Jan 2015, Pavel Machek wrote:
> > 
> > > 
> > > > > > I would say, problem is because omap3-n900 binary DT is too large
> > > > 
> > > > I agree.
> > > > 
> > > > > OK if that's the case, then your patch makes sense to me. It also
> > > > > seems we can have the temporary stack be larger than the initial
> > > > > stack just for atags_to_fdt.
> > > > 
> > > > The stack size isn't the issue, but rather its location.  We need to 
> > > > position it away from the DT data.  The DT size is known and we could 
> > > > use that, plus some room for the insertion of new data coming from the
> > > > ATAG conversion.
> > > > 
> > > > Something like the following would be a more robust solution:
> > > 
> > > Tested-by: Pavel Machek <pavel@ucw.cz>
> > > 
> > > (Note, that in 3.19 dts for n900 got too big, so we are actually
> > > triggering old bugs. That means that this is a regression fix, and
> > > should go in ASAP).
> > 
> > It is queued here:
> > http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8294/1
> 
> Hmm, but it should be on kernel.org, not in private arm trees, as it
> fixes a regression. Russell, you are the ARM maintainer, can you push
> it to Linus?

The patch is included in RMK's "fixes" branch already, along with other 
fixes.  I suppose it'll make its way to Linus before v3.19 final.


Nicolas

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2015-02-02 13:56 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-23 21:39 N900 v3.19-rc5 arm atags_to_fdt.c is broken Pali Rohár
2015-01-23 22:36 ` Pali Rohár
2015-01-23 23:48   ` Pavel Machek
2015-01-24  9:04     ` Pali Rohár
2015-01-26 16:14   ` Tony Lindgren
2015-01-26 16:23     ` Pali Rohár
2015-01-26 17:03       ` Tony Lindgren
2015-01-26 18:57         ` Nicolas Pitre
2015-01-26 21:48           ` Pavel Machek
2015-01-27  0:06             ` Nicolas Pitre
2015-01-27 13:37           ` Pavel Machek
2015-01-27 15:16             ` Nicolas Pitre
2015-01-27 15:26               ` Pali Rohár
2015-01-27 15:26               ` Tony Lindgren
2015-01-27 17:53               ` Russell King - ARM Linux
2015-01-27 18:50                 ` Nicolas Pitre
2015-01-27 23:09                   ` Aaro Koskinen
2015-01-27 23:17                   ` Pavel Machek
2015-02-02 13:39               ` Pavel Machek
2015-02-02 13:56                 ` Nicolas Pitre

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).