[U-Boot] [PATCH v7 00/15] split tegra20 arm7 code into separate SPL

[Tom Warren]

Tegra devs,

Since I've not seen any objections (or discussion, really, beyond Simon/Allen), I'm going to apply u-boot-tegra/next to tegra/master (i.e. the SPL patches will now be de rigueur for u-boot-tegra going forward).

If you have Tegra patches that you are working on, or are in the middle of review/revision on the list (Simon's LCD and NAND), then you'll need to rework/rebase them on top of u-boot-tegra/master (w/the SPL changes) from this point on.

Thanks,

Tom

> -----Original Message-----
> From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon Glass
> Sent: Thursday, July 19, 2012 1:09 PM
> To: Tom Warren
> Cc: Allen Martin; swarren at wwwdotorg.org; thierry.reding at avionic-design.de;
> u-boot at lists.denx.de; Igor Grinberg; Konstantin Sinyuk
> Subject: Re: [PATCH v7 00/15] split tegra20 arm7 code into separate SPL
> 
> Hi Tom / Allen,
> 
> On Thu, Jul 19, 2012 at 4:37 PM, Tom Warren <TWarren at nvidia.com> wrote:
> > Allen,
> >
> >> -----Original Message-----
> >> From: Allen Martin [mailto:amartin at nvidia.com]
> >> Sent: Wednesday, July 18, 2012 5:02 PM
> >> To: Tom Warren
> >> Cc: swarren at wwwdotorg.org; sjg at chromium.org; thierry.reding at avionic-
> >> design.de; u-boot at lists.denx.de
> >> Subject: Re: [PATCH v7 00/15] split tegra20 arm7 code into separate
> >> SPL
> >>
> >> On Tue, Jul 17, 2012 at 12:32:53PM -0700, Tom Warren wrote:
> >> > Allen,
> >> >
> >> > > -----Original Message-----
> >> > > From: Allen Martin [mailto:amartin at nvidia.com]
> >> > > Sent: Monday, July 16, 2012 4:02 PM
> >> > > To: Tom Warren; swarren at wwwdotorg.org; sjg at chromium.org;
> >> > > thierry.reding at avionic-design.de
> >> > > Cc: u-boot at lists.denx.de; Allen Martin
> >> > > Subject: [PATCH v7 00/15] split tegra20 arm7 code into separate
> >> > > SPL
> >> > >
> >> > > This patch series fixes a long standing problem with the tegra20
> >> > > u-boot build.  Tegra20 contains an ARM7TDMI boot processor and a
> >> > > Cortex A9 main processor.  Prior to this patch series this was
> >> > > accomplished by #ifdefing out any armv7 code from the early boot
> >> > > sequence and creating a single binary that runs on both both the
> >> > > ARM7TDMI and A9.  This was very fragile as changes to compiler
> >> > > options or any additions or rearranging of the early boot code
> >> > > could add additional armv7 specific code causing it to fail on the
> ARM7TDMI.
> >> > >
> >> > > This patch series pulls all the armv4t code out into a separate
> >> > > SPL that does nothing more than initialize the A9 and transfer
> >> > > control to it.  The resultint SPL and armv7 u-boot are
> >> > > concatenated together into a single image.
> >> > >
> >> > > This patch series is also available from:
> >> > > git://github.com/arm000/u-boot.git
> >> > > branch: tegra-spl-v7
> >> > >
> >> >
> >> > Applied to u-boot-tegra/next AOK, tested on my Seaboard AOK, so:
> >> > Tested-by: Tom Warren <twarren at nvidia.com>
> >> >
> >> > Note that I was confused by the final binary name
> >> > (u-boot-dtb-tegra.bin),
> >> since I'm used to flashing u-boot-dtb.bin.
> >> >
> >> > We need to come to a consensus about the final binary name for
> >> > Tegra U-
> >> Boot (I'd thought we had, and that it would be u-boot-dtb.bin, since
> >> that's what most devs are used to looking for in Tegra builds).
> 
> I think so.
> 
> >> >
> >>
> >> Yeah, I'd like some stability there too.  The -dtb rule is not tegra
> >> specific, which is why I didn't want to modify or remove it.  I think
> >> we're the only one that uses it though, so maybe it's not so bad.
> 
> Not for long :-)
> 
> >>
> >> > Also, one nit: I see the 2 sign-on strings (U-Boot SPL 2012.04.xxx,
> >> > and
> >> then U-Boot 2012.04.xxx), separated by 2 lines. I think it'd look
> >> better if you had them one right after the other, i.e. eliminate the
> extra linefeeds.
> >> >
> 
> >>
> >> The extra lines come from display_banner() which is ARM generic from
> >> the main u-boot.  I assume they are there to separate the banner from
> >> any junk that was on your screen before you rebooted, so it would
> >> make sense to move them to the SPL banner instead if you have SPL
> enabled.
> >> I'll make a separate patch for that in a week after I get back from
> >> vacation.
> 
> I suspect you could remove the extra line by not printing a \n in SPL.
> The other one might be a bit tricky as I think it is in U-Boot proper as you
> say.
> 
> Also do we need the full version tag on the SPL version?
> 
> >>
> >> -Allen
> >
> > Cool, thanks. Until then:
> >
> > Tegra2 SPL patches have been applied to u-boot-tegra/next & pushed to
> Denx. I'm going to hold off putting it into tegra/master and generating a
> pull request for awhile to allow all Tegra devs to test it, comment, etc.,
> since it's a major change to the Tegra build.
> >
> > If possible, please post any new Tegra changes against tegra/next (i.e.
> using Allen's SPL file locations).
> 
> I have a few minor comments on the series now that I have made time to go
> through it in final form:
> 
> 1. In the resulting -tegra.bin image I see this:
> 
> 00108038 <_fiq>:
>   108038:	00108038 	.word	0x00108038
>   10803c:	deadbeef 	.word	0xdeadbeef
> 
> 00108040 <_TEXT_BASE>:
>   108040:	0010c000 	.word	0x0010c000
> 
> To me it seems odd that SPL shows a TEXT_BASE of 10c000 when we actually
> need to load it at 108000. Can we change that? Also due to the difference
> between arm720 and armv7 there is no 0x12345678 tag before the text base. It
> would be nice if we could have that, as it is a convenient tag to point to
> the text base.
> 
> In the 'ARM: add tegra20 support' patch:
> 
> 2. lastdec = 0 seems unnecessary since it should already be 0 at init.
> 
> 3. Can we init the JTAG earlier (before any serial output, for example)? It
> may be useful to be able to set breakpoints in SPL.
> 
> In 'tegra20: add u-boot-*-tegra.bin targets':
> 
> 4. The Makefile stuff could perhaps be split out a bit. You have:
> 
> ifeq ($(SOC),tegra20)
> ifeq ($(CONFIG_OF_SEPARATE),y)
> $(obj)u-boot-dtb-tegra.bin:	$(obj)spl/u-boot-spl.bin $(obj)u-boot.bin
> $(obj)u-boot.dtb
> 		$(OBJCOPY) ${OBJCFLAGS} --pad-to=$(CONFIG_SYS_TEXT_BASE) -O
> binary $(obj)spl/u-boot-spl $(obj)spl/u-boot-spl-pad.bin
> 		cat $(obj)spl/u-boot-spl-pad.bin $(obj)u-boot.bin $(obj)u-
> boot.dtb > $@
> 		rm $(obj)spl/u-boot-spl-pad.bin
> else
> $(obj)u-boot-nodtb-tegra.bin:	$(obj)spl/u-boot-spl.bin $(obj)u-boot.bin
> 		$(OBJCOPY) ${OBJCFLAGS} --pad-to=$(CONFIG_SYS_TEXT_BASE) -O
> binary $(obj)spl/u-boot-spl $(obj)spl/u-boot-spl-pad.bin
> 		cat $(obj)spl/u-boot-spl-pad.bin $(obj)u-boot.bin > $@
> 		rm $(obj)spl/u-boot-spl-pad.bin
> endif
> endif
> 
> Leaving aside the nodtb stuff which I think we already discussed, and
> breaking the 80col limit, maybe you could do cut it back from:
> 
> $(obj)spl/u-boot-spl-pad.bin: $(obj)spl/u-boot-spl.bin
> 		$(OBJCOPY) ${OBJCFLAGS} --pad-to=$(CONFIG_SYS_TEXT_BASE) -O
> binary \
>                          $(obj)spl/u-boot-spl $(obj)spl/u-boot-spl-pad.bin
> 
> $(obj)u-boot-dtb-tegra.bin:	$(obj)spl/u-boot-spl-pad.bin
> $(obj)u-boot.bin $(obj)u-boot.dtb
> 		cat $^ > $@
> 		rm $(obj)spl/u-boot-spl-pad.bin
> else
> $(obj)u-boot-nodtb-tegra.bin:$(obj)spl/u-boot-spl-pad.bin $(obj)u-boot.bin
> 		cat $^ > $@
> 		rm $(obj)spl/u-boot-spl-pad.bin
> 
> to something like:
> 
> %-spl-pad.bin: %-spl
> 		$(OBJCOPY) ${OBJCFLAGS} --pad-to=$(CONFIG_SYS_TEXT_BASE) -O
> binary \
>                          $< $@
> $(obj)u-boot-dtb-tegra.bin:	$(obj)spl/u-boot-spl-pad.bin
> $(obj)u-boot.bin $(obj)u-boot.dtb
> 		cat $^ > $@
> 		rm $<
> else
> $(obj)u-boot-nodtb-tegra.bin:$(obj)spl/u-boot-spl-pad.bin $(obj)u-boot.bin
> 		cat $^ > $@
> 		rm $<
> 
> If any of the above merit attention then perhaps you could do a follow-on
> patch or two?
> 
> Regards,
> Simon
> 
> >
> > Thanks,
> >
> > Tom
--
nvpublic