[U-Boot] [PATCH v4 10/11] Add u-boot-pad.bin target to the Makefile

Thu Sep 20 18:01:42 CEST 2012

> -----Original Message-----
> From: Tom Rini [mailto:trini at ti.com]
> Sent: Wednesday, September 19, 2012 6:29 PM
> To: José Miguel Gonçalves
> Cc: Scott Wood; u-boot at lists.denx.de; marex at denx.de; mk7.kang at samsung.com;
> Tom Warren
> Subject: Re: [PATCH v4 10/11] Add u-boot-pad.bin target to the Makefile
> 
> * PGP Signed by an unknown key
> 
> On 09/19/12 17:38, José Miguel Gonçalves wrote:
> > Tom & Scott,
> >
> > On 09/20/2012 12:40 AM, Tom Rini wrote:
> >> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
> >>
> >> On 09/19/12 16:36, Scott Wood wrote:
> >>> On 09/19/2012 06:31:29 PM, Tom Rini wrote:
> >>>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
> >>>>
> >>>> On 09/19/12 15:39, Scott Wood wrote:
> >>>>> On 09/19/2012 06:25:26 AM, José Miguel Gonçalves wrote:
> >>>>>> Samsung's S3C24XX SoCs need this in order to generate a binary
> >>>>>> image with a padded SPL concatenated with U-Boot.
> >>>>>>
> >>>>>> Signed-off-by: José Miguel Gonçalves <jose.goncalves at inov.pt> ---
> >>>>>> Changes for v2: - None
> >>>>>>
> >>>>>> Changes for v3: - Changed new binary target name from
> >>>>>> u-boot-ubl.bin to u-boot-pad.bin
> >>>>>>
> >>>>>> Changes for v4: - None --- Makefile |   11 ++++++----- 1
> >>>>>> file changed, 6 insertions(+), 5 deletions(-)
> >>>>>>
> >>>>>> diff --git a/Makefile b/Makefile index 8738d55..86dedca
> >>>>>> 100644 --- a/Makefile +++ b/Makefile @@ -433,14 +433,15
> >>>>>> @@ $(obj)u-boot.sha1:    $(obj)u-boot.bin
> >>>>>> $(obj)u-boot.dis: $(obj)u-boot $(OBJDUMP) -d $< > $@
> >>>>>>
> >>>>>> -$(obj)u-boot.ubl:       $(obj)spl/u-boot-spl.bin
> >>>>>> $(obj)u-boot.bin +$(obj)u-boot-pad.bin:
> >>>>>> $(obj)spl/u-boot-spl.bin $(obj)u-boot.bin $(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
> >>>>>> $(obj)u-boot.bin >
> >>>>>> $(obj)u-boot-ubl.bin -        $(obj)tools/mkimage -n
> >>>>>> $(UBL_CONFIG) -T ublimage \ -        -e
> >>>>>> $(CONFIG_SYS_TEXT_BASE) -d $(obj)u-boot-ubl.bin
> >>>>>> $(obj)u-boot.ubl -        rm $(obj)u-boot-ubl.bin + cat
> >>>>>> $(obj)spl/u-boot-spl-pad.bin $(obj)u-boot.bin >
> >>>>>> $(obj)u-boot-pad.bin rm $(obj)spl/u-boot-spl-pad.bin
> >>>>> This rule lists u-boot-spl.bin as a prerequisite, but it doesn't
> >>>>> appear to use it -- it uses u-boot-spl instead.
> >>>>>
> >>>>> It seems that either spl/Makefile should produce u-boot-spl rather
> >>>>> than u-boot-spl.bin and let the toplevel Makefile deal with
> >>>>> converting it to a binary, or spl/Makefile should take care of
> >>>>> adding any needed padding for the target and the final rule should
> >>>>> just be a concatenation.
> >>>> So, if we use a weak PAD_TO value of say 0x1, we could always "pad"
> >>>> u-boot-spl.bin out and then do the concatenation here and in a few
> >>>> other targets as well.
> >>> Yes, or use a variable that includes the linker option, so that it
> >>> expands to an empty string if padding isn't required.
> >> Yes, I don't know if say IMAGE_PAD ?= "" and having boards set that
> >> to - --pad-to=... or just saying --pad-to=$(PAD_SIZE) and PAD_SIZE ?=
> >> 0x1 ends up looking cleaner once all is said and done.  First one to
> >> try both and patch... :)
> >>
> >
> > Please check the following patch. It seems to work for me...
> >
> > diff --git a/Makefile b/Makefile index 8738d55..fad8b33 100644 ---
> > a/Makefile +++ b/Makefile @@ -433,13 +433,12 @@ $(obj)u-boot.sha1:
> > $(obj)u-boot.bin $(obj)u-boot.dis:    $(obj)u-boot $(OBJDUMP) -d $<
> > > $@
> >
> > -$(obj)u-boot.ubl:       $(obj)spl/u-boot-spl.bin $(obj)u-boot.bin
> > -        $(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 $(obj)u-boot.bin > $(obj)u-boot-ubl.bin
> > +$(obj)u-boot-with-spl.bin:
> > $(obj)spl/u-boot-spl.bin $(obj)u-boot.bin +        cat $^ > $@ +
> > +$(obj)u-boot.ubl:       $(obj)u-boot-with-spl.bin
> > $(obj)tools/mkimage -n $(UBL_CONFIG) -T ublimage \ -        -e
> > $(CONFIG_SYS_TEXT_BASE) -d $(obj)u-boot-ubl.bin $(obj)u-boot.ubl -
> > rm $(obj)u-boot-ubl.bin -        rm $(obj)spl/u-boot-spl-pad.bin +
> > -e $(CONFIG_SYS_TEXT_BASE) -d $< $@
> >
> > $(obj)u-boot.ais:       $(obj)spl/u-boot-spl.bin $(obj)u-boot.img
> > $(obj)tools/mkimage -s -n /dev/null -T aisimage \ diff --git
> > a/spl/Makefile b/spl/Makefile index f96c08e..6ea9f26 100644 ---
> > a/spl/Makefile +++ b/spl/Makefile @@ -23,6 +23,8 @@ include
> > $(TOPDIR)/config.mk # We want the final binaries in this directory obj
> > := $(OBJTREE)/spl/
> >
> > +PAD_TO ?= 1 + HAVE_VENDOR_COMMON_LIB := $(shell [ -f
> > $(SRCTREE)/board/$(VENDOR)/common/Makefile ] \ && echo y || echo
> > n)
> >
> > @@ -124,7 +126,7 @@ $(obj)$(BOARD)-spl.bin: $(obj)u-boot-spl.bin endif
> >
> > $(obj)u-boot-spl.bin:    $(obj)u-boot-spl -    $(OBJCOPY)
> > $(OBJCFLAGS) -O binary $< $@ +    $(OBJCOPY) ${OBJCFLAGS}
> > --pad-to=$(PAD_TO) -O binary $< $@
> >
> > GEN_UBOOT = \ UNDEF_SYM=`$(OBJDUMP) -x $(LIBS) | \
> 
> Tom W, since I'd like you to upgrade the tegra rules after this change goes
> in (since they do a u-boot-spl.bin + pad + u-boot.bin + stuff), does this
> look good to you as well?  Thanks!

I'm not clear what you want the final binary names to be. I seem to be missing segments of this discussion - maybe not CC'd on all threads?

With Tegra, if you flash u-boot.bin, you'll get a DT-less, non-functional binary that will print a 'DT missing!' message and reset ad infinitum. Most folks won't see or care about u-boot-spl.bin, since it's hidden in the spl/ subdir. I don't see the use of having a 'u-boot-with-spl.bin' - doesn't u-boot-spl.bin say the same thing?

If you flash u-boot-dtb-tegra.bin, you'll get a fully functioning U-Boot. There's an intermediate file (u-boot-dtb.bin) that I assume is u-boot.bin+dtb - I'm not sure why it's left around - Allen could comment here.

So in my eyes, all you really need is u-boot-dtb-tegra.bin - an unwieldy name, to be sure, but it seems to satisfy your request for a Soc identifier in the name. I voted for just having u-boot.bin be the final product, since that's what most devs look for after a build, but didn't win that battle.  I will probably never use the intermediate binaries, as I'm not likely to paste different DTBs onto base u-boot binaries, etc., or if I did, I'd expect that'd be done in a Makefile, or the user that requires a different DTB appended outside of the normal one could hack the Makefile themselves.

Not to be too difficult, but could you take the Tegra binary section of Makefile and tell me exactly what it is you and/or Jose are proposing the change(s) be?

Thanks,

Tom
> 
> --
> Tom
> 
> * Unknown Key
> * 0x94391D56(L)
-- 
nvpublic