[U-Boot] [PATCH 1/3 V2] common: Add symbol handling for generic lists into Makefile

[Marek Vasut]

Dear Wolfgang Denk,

> Dear Marek Vasut,
> 
> In message <1348589520-14395-2-git-send-email-marex at denx.de> you wrote:
> > This patch adds essential components for generation of the contents of
> > the linker section that is used by the linker-generated array. All of
> > the contents is held in a separate file, u-boot.lst, which is generated
> > at runtime just before U-Boot is linked.
> 
> ...
> 
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -510,7 +510,10 @@ else
> > 
> >  GEN_UBOOT = \
> >  
> >  		UNDEF_SYM=`$(OBJDUMP) -x $(LIBBOARD) $(LIBS) | \
> >  		sed  -n -e 's/.*\($(SYM_PREFIX)__u_boot_cmd_.*\)/-u\1/p'|sort|
uniq`;\
> > 
> > -		cd $(LNDIR) && $(LD) $(LDFLAGS) $(LDFLAGS_$(@F)) $$UNDEF_SYM 
$(__OBJS)
> > \ +		UNDEF_LST=`$(OBJDUMP) -x $(LIBBOARD) $(LIBS) | \
> > +		sed  -n -e 's/.*\($(SYM_PREFIX)__u_boot_list_.*\)/-u\1/p'|sort|
uniq`;\
> > +		cd $(LNDIR) && $(LD) $(LDFLAGS) $(LDFLAGS_$(@F)) \
> > +			$$UNDEF_SYM $$UNDEF_LST $(__OBJS) \
> 
> I dislike the exact duplication of the commands here.

These shall begone once the cmd stuff is flipped over to these generic lists.

> Would it not make sense to provide some function / script to do that?

How do you like helper.mk ? %^) But see above, I think it'll grow pointless 
soon.

> > +$(eval $(call make_u_boot_list, $(obj)u-boot.lst, $(LIBBOARD) $(LIBS)))
> > +$(obj)u-boot.lds: $(LDSCRIPT) $(obj)u-boot.lst
> > +		$(CPP) $(CPPFLAGS) $(LDPPFLAGS) -ansi -D__ASSEMBLY__ -P - <$< 
>$@
> 
> I think this needs a comment to explain what sort of magic you are
> invoking here.
> 
> ...
> 
> > --- /dev/null
> > +++ b/helper.mk
> 
> ...
> 
> > +	$(OBJDUMP) -h $(2) | \
> > +	sed -n -e 's/.*\(\.u_boot_list[^ ]\+\).*$$$$/\1/p' | \
> > +	sed 's/\.[^\.]\+$$$$//' | \
> > +	sed -n ':s /^.\+$$$$/ { p;s/^\(.*\)\.[^\.]*$$$$/\1/;b s }' | \
> > +	sed -n 'h;s/$$$$/\a/p;g;s/$$$$/@/p;g;s/$$$$/~/p;' | \
> 
> ...
> 
> Why exactly do you need 4 invocations of sed here?  Seems just like a
> waste of processes to me.
> 
> >  LDSCRIPT= $(TOPDIR)/$(CPUDIR)/u-boot-nand_spl.lds
> > 
> > +LSTSCRIPT= $(TOPDIR)/nand_spl/board/$(BOARDDIR)/u-boot.lst
> 
> BTW:  what exactly is "LST" supposed to mean?

LIST ... that's from Supaplex, there was this levels.lst file IIRC ;-)

> > +$(eval $(call make_u_boot_list, $(LSTSCRIPT), $(__OBJS)))
> > +$(nandobj)u-boot-nand_spl.lds: $(LDSCRIPT) $(LSTSCRIPT)
> > +	$(CPP) $(CPPFLAGS) $(LDPPFLAGS) -ansi -D__ASSEMBLY__ -P - <$< >$@
> 
> Comment needed.

The eval call ... ?

> 
> Best regards,
> 
> Wolfgang Denk

Best regards,
Marek Vasut