Patchwork [U-Boot,v3,09/14] tegra20: add u-boot.t2 target

login
register
mail settings
Submitter Allen Martin
Date June 8, 2012, 9:16 p.m.
Message ID <1339190167-20320-10-git-send-email-amartin@nvidia.com>
Download mbox | patch
Permalink /patch/163849/
State Superseded
Headers show

Comments

Allen Martin - June 8, 2012, 9:16 p.m.
Add target for tegra20 u-boot image.  This is a concatenation of tegra
spl and normal u-boot binaries.

Signed-off-by: Allen Martin <amartin@nvidia.com>
---
 .gitignore                      |    1 +
 Makefile                        |   11 +++++++++++
 board/nvidia/seaboard/config.mk |    1 +
 3 files changed, 13 insertions(+)
 create mode 100644 board/nvidia/seaboard/config.mk
Stephen Warren - June 9, 2012, 5:21 a.m.
On 06/08/2012 03:16 PM, Allen Martin wrote:
> Add target for tegra20 u-boot image.  This is a concatenation of tegra
> spl and normal u-boot binaries.

> diff --git a/board/nvidia/seaboard/config.mk b/board/nvidia/seaboard/config.mk

> +PAD_TO=0x00208000

Can we use $(CONFIG_SYS_TEXT_BASE) or similar here to avoid duplicating
the value? Can this change be made to a common config.mk so every board
doesn't need it? If not I assume this change should edit a few more
board config files.
Simon Glass - June 9, 2012, 7:12 p.m.
Hi Allen,

On Fri, Jun 8, 2012 at 2:16 PM, Allen Martin <amartin@nvidia.com> wrote:

> Add target for tegra20 u-boot image.  This is a concatenation of tegra
> spl and normal u-boot binaries.
>
> Signed-off-by: Allen Martin <amartin@nvidia.com>
> ---
>  .gitignore                      |    1 +
>  Makefile                        |   11 +++++++++++
>  board/nvidia/seaboard/config.mk |    1 +
>  3 files changed, 13 insertions(+)
>  create mode 100644 board/nvidia/seaboard/config.mk
>
> diff --git a/.gitignore b/.gitignore
> index 0f32fd8..b9192bf 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -39,6 +39,7 @@
>  /u-boot.ais
>  /u-boot.dtb
>  /u-boot.sb
> +/u-boot.t2
>

What does t2 mean? If it is a binary file of some sort  perhaps
u-boot-t2.bin would be better?

>
>  #
>  # Generated files
> diff --git a/Makefile b/Makefile
> index b2275ed..6f4abc6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -456,6 +456,16 @@ $(obj)u-boot.sb:       $(obj)u-boot.bin
> $(obj)spl/u-boot-spl.bin
>                elftosb -zdf imx28 -c $(TOPDIR)/board/$(BOARDDIR)/u-boot.bd\
>                        -o $(obj)u-boot.sb
>
> +ifeq ($(CONFIG_OF_SEPARATE),y)
> +T2_UBOOT=$(obj)u-boot-dtb.bin
> +else
> +T2_UBOOT=$(obj)u-boot.bin
> +endif
>

What is this logic for? The dtb file is separate but that doesn't
necessarily mean that it must be immediately after the U-Boot image. We
provide other options for packaging it, like getenv(). Maybe if you want to
create this composite binary you should change its name (u-boot-dtb-t2.bin
or u-boot-t2.bin) to indicate what it contains?


> +$(obj)u-boot.t2:       $(obj)spl/u-boot-spl.bin $(T2_UBOOT)
> +               $(OBJCOPY) ${OBJCFLAGS} --pad-to=$(PAD_TO) -O binary
> $(obj)spl/u-boot-spl $(obj)spl/u-boot-spl-pad.bin
> +               cat $(obj)spl/u-boot-spl-pad.bin $(T2_UBOOT) >
> $(obj)u-boot.t2
> +               rm $(obj)spl/u-boot-spl-pad.bin
> +
>

I echo Stephen's comments. But also SPL is supposed to load U-Boot, so
shouldn't this t2 binary do that?


>  ifeq ($(CONFIG_SANDBOX),y)
>  GEN_UBOOT = \
>                cd $(LNDIR) && $(CC) $(SYMS) -T $(obj)u-boot.lds \
> @@ -775,6 +785,7 @@ clobber:    tidy
>        @rm -f $(obj)u-boot.ais
>        @rm -f $(obj)u-boot.dtb
>        @rm -f $(obj)u-boot.sb
> +       @rm -f $(obj)u-boot.t2
>        @rm -f $(obj)tools/inca-swap-bytes
>        @rm -f $(obj)arch/powerpc/cpu/mpc824x/bedbug_603e.c
>        @rm -f $(obj)arch/powerpc/cpu/mpc83xx/ddr-gen?.c
> diff --git a/board/nvidia/seaboard/config.mk b/board/nvidia/seaboard/
> config.mk
> new file mode 100644
> index 0000000..71c9f28
> --- /dev/null
> +++ b/board/nvidia/seaboard/config.mk
> @@ -0,0 +1 @@
> +PAD_TO=0x00208000
> --
> 1.7.9.5
>
> Regards,
Simon
Allen Martin - June 11, 2012, 7:09 p.m.
On Fri, Jun 08, 2012 at 10:21:15PM -0700, Stephen Warren wrote:
> On 06/08/2012 03:16 PM, Allen Martin wrote:
> > Add target for tegra20 u-boot image.  This is a concatenation of tegra
> > spl and normal u-boot binaries.
> 
> > diff --git a/board/nvidia/seaboard/config.mk b/board/nvidia/seaboard/config.mk
> 
> > +PAD_TO=0x00208000
> 
> Can we use $(CONFIG_SYS_TEXT_BASE) or similar here to avoid duplicating
> the value? Can this change be made to a common config.mk so every board
> doesn't need it? If not I assume this change should edit a few more
> board config files.

I'll take a look if CONFIG_SYS_TEXT_BASE is valid here, that would
certainly be better than trying to keep them in sync.

You're right, all boards need this.

-Allen
Allen Martin - June 11, 2012, 7:22 p.m.
On Sat, Jun 09, 2012 at 12:12:09PM -0700, Simon Glass wrote:
> Hi Allen,
> 
> On Fri, Jun 8, 2012 at 2:16 PM, Allen Martin <amartin@nvidia.com<mailto:amartin@nvidia.com>> wrote:
> Add target for tegra20 u-boot image.  This is a concatenation of tegra
> spl and normal u-boot binaries.
> 
> Signed-off-by: Allen Martin <amartin@nvidia.com<mailto:amartin@nvidia.com>>
> ---
>  .gitignore                      |    1 +
>  Makefile                        |   11 +++++++++++
>  board/nvidia/seaboard/config.mk<http://config.mk> |    1 +
>  3 files changed, 13 insertions(+)
>  create mode 100644 board/nvidia/seaboard/config.mk<http://config.mk>
> 
> diff --git a/.gitignore b/.gitignore
> index 0f32fd8..b9192bf 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -39,6 +39,7 @@
>  /u-boot.ais
>  /u-boot.dtb
>  /u-boot.sb<http://u-boot.sb>
> +/u-boot.t2
> 
> What does t2 mean? If it is a binary file of some sort  perhaps
u-boot-t2.bin would be better?

It's just means "tegra2".  I was following the convention that other
SPL builds use.  I don't have a strong opinion on the name though.

> +ifeq ($(CONFIG_OF_SEPARATE),y)
> +T2_UBOOT=$(obj)u-boot-dtb.bin
> +else
> +T2_UBOOT=$(obj)u-boot.bin
> +endif
> 
> What is this logic for? The dtb file is separate but that doesn't
> necessarily mean that it must be immediately after the U-Boot
> image. We provide other options for packaging it, like
> getenv(). Maybe if you want to create this composite binary you
> should change its name (u-boot-dtb-t2.bin or u-boot-t2.bin) to
> indicate what it contains?

This just picks up the name of the u-boot binary so it glues the SPL
to the right thing depending if devicetree is in use or not.

> 
> +$(obj)u-boot.t2:       $(obj)spl/u-boot-spl.bin $(T2_UBOOT)
> +               $(OBJCOPY) ${OBJCFLAGS} --pad-to=$(PAD_TO) -O binary $(obj)spl/u-boot-spl $(obj)spl/u-boot-spl-pad.bin
> +               cat $(obj)spl/u-boot-spl-pad.bin $(T2_UBOOT) > $(obj)u-boot.t2
> +               rm $(obj)spl/u-boot-spl-pad.bin
> +
> 
> I echo Stephen's comments. But also SPL is supposed to load U-Boot,
> so shouldn't this t2 binary do that?

The t2 binary is the SPL and u-boot concatenated together into one
binary.  The whole thing will get loaded into memory by the tegra
BootROM.  The SPL knows the address of the real u-boot at compile time
and uses that as the address for the Cortex A9 to jump to when it
comes out of reset. 

-Allen
Simon Glass - June 11, 2012, 8:21 p.m.
Hi Allen,

On Mon, Jun 11, 2012 at 12:22 PM, Allen Martin <amartin@nvidia.com> wrote:

> On Sat, Jun 09, 2012 at 12:12:09PM -0700, Simon Glass wrote:
> > Hi Allen,
> >
> > On Fri, Jun 8, 2012 at 2:16 PM, Allen Martin <amartin@nvidia.com<mailto:
> amartin@nvidia.com>> wrote:
> > Add target for tegra20 u-boot image.  This is a concatenation of tegra
> > spl and normal u-boot binaries.
> >
> > Signed-off-by: Allen Martin <amartin@nvidia.com<mailto:
> amartin@nvidia.com>>
> > ---
> >  .gitignore                      |    1 +
> >  Makefile                        |   11 +++++++++++
> >  board/nvidia/seaboard/config.mk<http://config.mk> |    1 +
> >  3 files changed, 13 insertions(+)
> >  create mode 100644 board/nvidia/seaboard/config.mk<http://config.mk>
> >
> > diff --git a/.gitignore b/.gitignore
> > index 0f32fd8..b9192bf 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -39,6 +39,7 @@
> >  /u-boot.ais
> >  /u-boot.dtb
> >  /u-boot.sb<http://u-boot.sb>
> > +/u-boot.t2
> >
> > What does t2 mean? If it is a binary file of some sort  perhaps
> u-boot-t2.bin would be better?
>
> It's just means "tegra2".  I was following the convention that other
> SPL builds use.  I don't have a strong opinion on the name though.
>

OK, still would prefer a .bin on the end, but up to you.


>
> > +ifeq ($(CONFIG_OF_SEPARATE),y)
> > +T2_UBOOT=$(obj)u-boot-dtb.bin
> > +else
> > +T2_UBOOT=$(obj)u-boot.bin
> > +endif
> >
> > What is this logic for? The dtb file is separate but that doesn't
> > necessarily mean that it must be immediately after the U-Boot
> > image. We provide other options for packaging it, like
> > getenv(). Maybe if you want to create this composite binary you
> > should change its name (u-boot-dtb-t2.bin or u-boot-t2.bin) to
> > indicate what it contains?
>
> This just picks up the name of the u-boot binary so it glues the SPL
> to the right thing depending if devicetree is in use or not.
>

OK, to avoid confusion I think you should change the name to include the
-dtb or not, rather than having a filename without the -dtb part which does
in fact include a dtb.

>
> >
> > +$(obj)u-boot.t2:       $(obj)spl/u-boot-spl.bin $(T2_UBOOT)
> > +               $(OBJCOPY) ${OBJCFLAGS} --pad-to=$(PAD_TO) -O binary
> $(obj)spl/u-boot-spl $(obj)spl/u-boot-spl-pad.bin
> > +               cat $(obj)spl/u-boot-spl-pad.bin $(T2_UBOOT) >
> $(obj)u-boot.t2
> > +               rm $(obj)spl/u-boot-spl-pad.bin
> > +
> >
> > I echo Stephen's comments. But also SPL is supposed to load U-Boot,
> > so shouldn't this t2 binary do that?
>
> The t2 binary is the SPL and u-boot concatenated together into one
> binary.  The whole thing will get loaded into memory by the tegra
> BootROM.  The SPL knows the address of the real u-boot at compile time
> and uses that as the address for the Cortex A9 to jump to when it
> comes out of reset.
>

OK I see, makes sense. I am interested in your comments as to whether we
might move to a 'true SPL' later, where U-Boot is actually loaded by SPL.


> -Allen
> --
> nvpublic
>

Regards,
Simon
Allen Martin - June 11, 2012, 11:09 p.m.
On Mon, Jun 11, 2012 at 01:21:13PM -0700, Simon Glass wrote:
> Hi Allen,
> 
> On Mon, Jun 11, 2012 at 12:22 PM, Allen Martin <amartin@nvidia.com<mailto:amartin@nvidia.com>> wrote:
> On Sat, Jun 09, 2012 at 12:12:09PM -0700, Simon Glass wrote:
> > Hi Allen,
> >
> > On Fri, Jun 8, 2012 at 2:16 PM, Allen Martin <amartin@nvidia.com<mailto:amartin@nvidia.com><mailto:amartin@nvidia.com<mailto:amartin@nvidia.com>>> wrote:
> > Add target for tegra20 u-boot image.  This is a concatenation of tegra
> > spl and normal u-boot binaries.
> >
> > Signed-off-by: Allen Martin <amartin@nvidia.com<mailto:amartin@nvidia.com><mailto:amartin@nvidia.com<mailto:amartin@nvidia.com>>>
> > ---
> >  .gitignore                      |    1 +
> >  Makefile                        |   11 +++++++++++
> >  board/nvidia/seaboard/config.mk<http://config.mk><http://config.mk> |    1 +
> >  3 files changed, 13 insertions(+)
> >  create mode 100644 board/nvidia/seaboard/config.mk<http://config.mk><http://config.mk>
> >
> > diff --git a/.gitignore b/.gitignore
> > index 0f32fd8..b9192bf 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -39,6 +39,7 @@
> >  /u-boot.ais
> >  /u-boot.dtb
> >  /u-boot.sb<http://u-boot.sb><http://u-boot.sb>
> > +/u-boot.t2
> >
> > What does t2 mean? If it is a binary file of some sort  perhaps
> u-boot-t2.bin would be better?
> 
> It's just means "tegra2".  I was following the convention that other
> SPL builds use.  I don't have a strong opinion on the name though.
> 
> OK, still would prefer a .bin on the end, but up to you.

I'm ok with that, it keeps in line with the u-boot-dtb.bin

> 
> 
> > +ifeq ($(CONFIG_OF_SEPARATE),y)
> > +T2_UBOOT=$(obj)u-boot-dtb.bin
> > +else
> > +T2_UBOOT=$(obj)u-boot.bin
> > +endif
> >
> > What is this logic for? The dtb file is separate but that doesn't
> > necessarily mean that it must be immediately after the U-Boot
> > image. We provide other options for packaging it, like
> > getenv(). Maybe if you want to create this composite binary you
> > should change its name (u-boot-dtb-t2.bin or u-boot-t2.bin) to
> > indicate what it contains?
> 
> This just picks up the name of the u-boot binary so it glues the SPL
> to the right thing depending if devicetree is in use or not.
> 
> OK, to avoid confusion I think you should change the name to include
> the -dtb or not, rather than having a filename without the -dtb part
> which does in fact include a dtb.

Ok.

> > I echo Stephen's comments. But also SPL is supposed to load U-Boot,
> > so shouldn't this t2 binary do that?
> 
> The t2 binary is the SPL and u-boot concatenated together into one
> binary.  The whole thing will get loaded into memory by the tegra
> BootROM.  The SPL knows the address of the real u-boot at compile time
> and uses that as the address for the Cortex A9 to jump to when it
> comes out of reset.
> 
> OK I see, makes sense. I am interested in your comments as to whether we might move to a 'true SPL' later, where U-Boot is actually loaded by SPL.

To keep this already complicated patch series under control I want to
just assume the SPL and regular u-boot are glued together for now, but
I'm definately thinking about the case where they are not.

In particular part of my motivation for this work is the ability to
have a version of the SPL that you can run in recovery mode when you
can't trust or don't have a BCT to initialize RAM.  In that case the
SPL would run out of IRAM and could take a BCT and u-boot from USB
DFU.  There's no reason it couldn't be extended to take those from
somewhere else the BootROM wouldn't normally be able to boot from like
SATA or network.  And it wouldn't have to be just in recovery mode
either I suppose.

-Allen
--
nvpublic
Simon Glass - June 12, 2012, 12:16 a.m.
Hi Allen,

On Mon, Jun 11, 2012 at 4:09 PM, Allen Martin <amartin@nvidia.com> wrote:

> On Mon, Jun 11, 2012 at 01:21:13PM -0700, Simon Glass wrote:
> > Hi Allen,
> >
> > On Mon, Jun 11, 2012 at 12:22 PM, Allen Martin <amartin@nvidia.com
> <mailto:amartin@nvidia.com>> wrote:
> > On Sat, Jun 09, 2012 at 12:12:09PM -0700, Simon Glass wrote:
> > > Hi Allen,
> > >
> > > On Fri, Jun 8, 2012 at 2:16 PM, Allen Martin <amartin@nvidia.com
> <mailto:amartin@nvidia.com><mailto:amartin@nvidia.com<mailto:
> amartin@nvidia.com>>> wrote:
> > > Add target for tegra20 u-boot image.  This is a concatenation of tegra
> > > spl and normal u-boot binaries.
> > >
> > > Signed-off-by: Allen Martin <amartin@nvidia.com<mailto:
> amartin@nvidia.com><mailto:amartin@nvidia.com<mailto:amartin@nvidia.com>>>
> > > ---
> > >  .gitignore                      |    1 +
> > >  Makefile                        |   11 +++++++++++
> > >  board/nvidia/seaboard/config.mk<http://config.mk><http://config.mk>
> |    1 +
> > >  3 files changed, 13 insertions(+)
> > >  create mode 100644 board/nvidia/seaboard/config.mk<http://config.mk><
> http://config.mk>
> > >
> > > diff --git a/.gitignore b/.gitignore
> > > index 0f32fd8..b9192bf 100644
> > > --- a/.gitignore
> > > +++ b/.gitignore
> > > @@ -39,6 +39,7 @@
> > >  /u-boot.ais
> > >  /u-boot.dtb
> > >  /u-boot.sb<http://u-boot.sb><http://u-boot.sb>
> > > +/u-boot.t2
> > >
> > > What does t2 mean? If it is a binary file of some sort  perhaps
> > u-boot-t2.bin would be better?
> >
> > It's just means "tegra2".  I was following the convention that other
> > SPL builds use.  I don't have a strong opinion on the name though.
> >
> > OK, still would prefer a .bin on the end, but up to you.
>
> I'm ok with that, it keeps in line with the u-boot-dtb.bin
>
> >
> >
> > > +ifeq ($(CONFIG_OF_SEPARATE),y)
> > > +T2_UBOOT=$(obj)u-boot-dtb.bin
> > > +else
> > > +T2_UBOOT=$(obj)u-boot.bin
> > > +endif
> > >
> > > What is this logic for? The dtb file is separate but that doesn't
> > > necessarily mean that it must be immediately after the U-Boot
> > > image. We provide other options for packaging it, like
> > > getenv(). Maybe if you want to create this composite binary you
> > > should change its name (u-boot-dtb-t2.bin or u-boot-t2.bin) to
> > > indicate what it contains?
> >
> > This just picks up the name of the u-boot binary so it glues the SPL
> > to the right thing depending if devicetree is in use or not.
> >
> > OK, to avoid confusion I think you should change the name to include
> > the -dtb or not, rather than having a filename without the -dtb part
> > which does in fact include a dtb.
>
> Ok.
>
> > > I echo Stephen's comments. But also SPL is supposed to load U-Boot,
> > > so shouldn't this t2 binary do that?
> >
> > The t2 binary is the SPL and u-boot concatenated together into one
> > binary.  The whole thing will get loaded into memory by the tegra
> > BootROM.  The SPL knows the address of the real u-boot at compile time
> > and uses that as the address for the Cortex A9 to jump to when it
> > comes out of reset.
> >
> > OK I see, makes sense. I am interested in your comments as to whether we
> might move to a 'true SPL' later, where U-Boot is actually loaded by SPL.
>
> To keep this already complicated patch series under control I want to
> just assume the SPL and regular u-boot are glued together for now, but
> I'm definately thinking about the case where they are not.
>

Yes, no suggestion that we do this now, I was just interested in your plans.


>
> In particular part of my motivation for this work is the ability to
> have a version of the SPL that you can run in recovery mode when you
> can't trust or don't have a BCT to initialize RAM.  In that case the
> SPL would run out of IRAM and could take a BCT and u-boot from USB
> DFU.  There's no reason it couldn't be extended to take those from
> somewhere else the BootROM wouldn't normally be able to boot from like
> SATA or network.  And it wouldn't have to be just in recovery mode
> either I suppose.
>

OK I see. Yes SPL from IRAM would be more 'traditional'.


>
> -Allen
> --
> nvpublic
>

Regards,
Simon

Patch

diff --git a/.gitignore b/.gitignore
index 0f32fd8..b9192bf 100644
--- a/.gitignore
+++ b/.gitignore
@@ -39,6 +39,7 @@ 
 /u-boot.ais
 /u-boot.dtb
 /u-boot.sb
+/u-boot.t2
 
 #
 # Generated files
diff --git a/Makefile b/Makefile
index b2275ed..6f4abc6 100644
--- a/Makefile
+++ b/Makefile
@@ -456,6 +456,16 @@  $(obj)u-boot.sb:       $(obj)u-boot.bin $(obj)spl/u-boot-spl.bin
 		elftosb -zdf imx28 -c $(TOPDIR)/board/$(BOARDDIR)/u-boot.bd \
 			-o $(obj)u-boot.sb
 
+ifeq ($(CONFIG_OF_SEPARATE),y)
+T2_UBOOT=$(obj)u-boot-dtb.bin
+else
+T2_UBOOT=$(obj)u-boot.bin
+endif
+$(obj)u-boot.t2:       $(obj)spl/u-boot-spl.bin $(T2_UBOOT)
+		$(OBJCOPY) ${OBJCFLAGS} --pad-to=$(PAD_TO) -O binary $(obj)spl/u-boot-spl $(obj)spl/u-boot-spl-pad.bin
+		cat $(obj)spl/u-boot-spl-pad.bin $(T2_UBOOT) > $(obj)u-boot.t2
+		rm $(obj)spl/u-boot-spl-pad.bin
+
 ifeq ($(CONFIG_SANDBOX),y)
 GEN_UBOOT = \
 		cd $(LNDIR) && $(CC) $(SYMS) -T $(obj)u-boot.lds \
@@ -775,6 +785,7 @@  clobber:	tidy
 	@rm -f $(obj)u-boot.ais
 	@rm -f $(obj)u-boot.dtb
 	@rm -f $(obj)u-boot.sb
+	@rm -f $(obj)u-boot.t2
 	@rm -f $(obj)tools/inca-swap-bytes
 	@rm -f $(obj)arch/powerpc/cpu/mpc824x/bedbug_603e.c
 	@rm -f $(obj)arch/powerpc/cpu/mpc83xx/ddr-gen?.c
diff --git a/board/nvidia/seaboard/config.mk b/board/nvidia/seaboard/config.mk
new file mode 100644
index 0000000..71c9f28
--- /dev/null
+++ b/board/nvidia/seaboard/config.mk
@@ -0,0 +1 @@ 
+PAD_TO=0x00208000