diff mbox

[2/2] arm: boot: store ATAG structure into DT atags field

Message ID 1431719407-18230-3-git-send-email-pali.rohar@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Pali Rohár May 15, 2015, 7:50 p.m. UTC
This patch stores existing ATAG structure into DT /atags field
and so previous patch exports it for userspace via /proc/atags.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 arch/arm/boot/compressed/atags_to_fdt.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Arnd Bergmann May 15, 2015, 8:12 p.m. UTC | #1
On Friday 15 May 2015 21:50:07 Pali Rohár wrote:
>                 }
>         }
>  
> +       /* include the terminating ATAG_NONE */
> +       atag_size = (char *)atag - (char *)atag_list + sizeof(struct tag_header);
> +       setprop(fdt, "/", "atags", atag_list, atag_size);
> +
>         if (memcount) {
>                 setprop(fdt, "/memory", "reg", mem_reg_property,
>                         4 * memcount * memsize);
> 

The property should probably have a DT binding, and be named "linux,atags".

It may also help to check if the "linux,atags" property already exists and not
create it otherwise. That way we can put it into the n900 dts file and have
it updated by the compat code, but not expose the atags on other platforms
unless they opt in.

	Arnd
Pali Rohár May 15, 2015, 8:16 p.m. UTC | #2
On Friday 15 May 2015 22:12:41 Arnd Bergmann wrote:
> On Friday 15 May 2015 21:50:07 Pali Rohár wrote:
> >                 }
> >         
> >         }
> > 
> > +       /* include the terminating ATAG_NONE */
> > +       atag_size = (char *)atag - (char *)atag_list +
> > sizeof(struct tag_header); +       setprop(fdt, "/", "atags",
> > atag_list, atag_size);
> > +
> > 
> >         if (memcount) {
> >         
> >                 setprop(fdt, "/memory", "reg", mem_reg_property,
> >                 
> >                         4 * memcount * memsize);
> 
> The property should probably have a DT binding, and be named
> "linux,atags".
> 
> It may also help to check if the "linux,atags" property already
> exists and not create it otherwise. That way we can put it into the
> n900 dts file and have it updated by the compat code, but not expose
> the atags on other platforms unless they opt in.
> 
> 	Arnd

Maybe what would help: Is there a way to tell decompressor/kernel to not 
touch atag memory and then after kernel/board-code starts it save copy 
of atags? I think it is not possible right now, but correct me if I'm 
wrong...
Arnd Bergmann May 15, 2015, 8:21 p.m. UTC | #3
On Friday 15 May 2015 22:16:24 Pali Rohár wrote:
> On Friday 15 May 2015 22:12:41 Arnd Bergmann wrote:
> > On Friday 15 May 2015 21:50:07 Pali Rohár wrote:
> > >                 }
> > >         
> > >         }
> > > 
> > > +       /* include the terminating ATAG_NONE */
> > > +       atag_size = (char *)atag - (char *)atag_list +
> > > sizeof(struct tag_header); +       setprop(fdt, "/", "atags",
> > > atag_list, atag_size);
> > > +
> > > 
> > >         if (memcount) {
> > >         
> > >                 setprop(fdt, "/memory", "reg", mem_reg_property,
> > >                 
> > >                         4 * memcount * memsize);
> > 
> > The property should probably have a DT binding, and be named
> > "linux,atags".
> > 
> > It may also help to check if the "linux,atags" property already
> > exists and not create it otherwise. That way we can put it into the
> > n900 dts file and have it updated by the compat code, but not expose
> > the atags on other platforms unless they opt in.
> > 
> >       Arnd
> 
> Maybe what would help: Is there a way to tell decompressor/kernel to not 
> touch atag memory and then after kernel/board-code starts it save copy 
> of atags? I think it is not possible right now, but correct me if I'm 
> wrong...
> 

I don't think that is possible without an incompatible change to the
boot protocol.

	Arnd
Tony Lindgren June 25, 2015, 5:12 a.m. UTC | #4
* Arnd Bergmann <arnd@arndb.de> [150515 13:23]:
> On Friday 15 May 2015 22:16:24 Pali Rohár wrote:
> > On Friday 15 May 2015 22:12:41 Arnd Bergmann wrote:
> > > On Friday 15 May 2015 21:50:07 Pali Rohár wrote:
> > > >                 }
> > > >         
> > > >         }
> > > > 
> > > > +       /* include the terminating ATAG_NONE */
> > > > +       atag_size = (char *)atag - (char *)atag_list +
> > > > sizeof(struct tag_header); +       setprop(fdt, "/", "atags",
> > > > atag_list, atag_size);
> > > > +
> > > > 
> > > >         if (memcount) {
> > > >         
> > > >                 setprop(fdt, "/memory", "reg", mem_reg_property,
> > > >                 
> > > >                         4 * memcount * memsize);
> > > 
> > > The property should probably have a DT binding, and be named
> > > "linux,atags".
> > > 
> > > It may also help to check if the "linux,atags" property already
> > > exists and not create it otherwise. That way we can put it into the
> > > n900 dts file and have it updated by the compat code, but not expose
> > > the atags on other platforms unless they opt in.

Using "linux,atags" sounds good to me. And yes checking it with
getprop before doing setprop makes sense.

> > Maybe what would help: Is there a way to tell decompressor/kernel to not 
> > touch atag memory and then after kernel/board-code starts it save copy 
> > of atags? I think it is not possible right now, but correct me if I'm 
> > wrong...
> > 
> 
> I don't think that is possible without an incompatible change to the
> boot protocol.

Agreed, let's keep the changes to minimum.

Looks like with the comments posted all the pending four patches
from Pali become quite a minimal set of three patches if we keep
the rev string as hex.

Regrds,

Tony
diff mbox

Patch

diff --git a/arch/arm/boot/compressed/atags_to_fdt.c b/arch/arm/boot/compressed/atags_to_fdt.c
index b23748e..4a4a70c 100644
--- a/arch/arm/boot/compressed/atags_to_fdt.c
+++ b/arch/arm/boot/compressed/atags_to_fdt.c
@@ -145,7 +145,7 @@  int atags_to_fdt(void *atag_list, void *fdt, int total_space)
 	 * address and size for each bank */
 	uint32_t mem_reg_property[2 * 2 * NR_BANKS];
 	int memcount = 0;
-	int ret, memsize;
+	int ret, memsize, atag_size;
 
 	/* make sure we've got an aligned pointer */
 	if ((u32)atag_list & 0x3)
@@ -219,6 +219,10 @@  int atags_to_fdt(void *atag_list, void *fdt, int total_space)
 		}
 	}
 
+	/* include the terminating ATAG_NONE */
+	atag_size = (char *)atag - (char *)atag_list + sizeof(struct tag_header);
+	setprop(fdt, "/", "atags", atag_list, atag_size);
+
 	if (memcount) {
 		setprop(fdt, "/memory", "reg", mem_reg_property,
 			4 * memcount * memsize);