Dear Luigi 'Comio' Mantellini,
In message <1291469358-25023-5-git-send-email-luigi.mantell...@idf-hit.com> you
wrote:
> See README file for details regarding ho bootstrap code works.
Please provide at least a basic explanation of that this commit is
doing in the commitm essage.
Not only the commit message, also the remaining text is full of
typos. Please run through a spell checker.
> Signed-off-by: Luigi 'Comio' Mantellini <luigi.mantell...@idf-hit.com>
> ---
> .gitignore | 25 ++++++-
> Makefile | 174
> +++++++++++++++++++++++++++++++++++++++++++-
Please consider if this can be handled separately, without adding tons
of new code to the top level Makefile.
> diff --git a/.gitignore b/.gitignore
> index e71f6ac..8db8f0f 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -23,6 +23,12 @@
> /u-boot.hex
> /u-boot.map
> /u-boot.bin
> +/u-boot.bin.bz2
> +/u-boot.bin.gz
> +/u-boot.bin.lzma
> +/u-boot.bin.lzo
> +/u-boot.bin.xz
> +/u-boot.dis
> /u-boot.srec
> /u-boot.ldr
> /u-boot.ldr.hex
> @@ -30,6 +36,20 @@
> /u-boot.lds
> /u-boot-onenand.bin
> /u-boot-flexonenand.bin
> +/u-boot-bootstrap
> +/u-boot-bootstrap.hex
> +/u-boot-bootstrap.map
> +/u-boot-bootstrap.bin
> +/u-boot-bootstrap.bin.bz2
> +/u-boot-bootstrap.bin.gz
> +/u-boot-bootstrap.bin.lzma
> +/u-boot-bootstrap.bin.lzo
> +/u-boot-bootstrap.dis
> +/u-boot-bootstrap.srec
> +/u-boot-bootstrap.ldr
> +/u-boot-bootstrap.ldr.hex
> +/u-boot-bootstrap.ldr.srec
> +/u-boot-bootstrap.lds
Please explain why all these new images are needed?
> @@ -39,7 +59,7 @@
> /LOG
> /errlog
> /reloc_off
> -
> +/.payload.s
???
> /onenand_ipl/onenand-ipl*
> /onenand_ipl/board/*/onenand*
> /onenand_ipl/board/*/*.S
> +examples/standalone/
> +
> +setvars
???
> +ifeq ($(CONFIG_BOOTSTRAP),y)
> +BOOTSTRAP_OBJS = $(CPUDIR)/start_bootstrap.o
> +
> +BOOTSTRAP_OBJS := $(addprefix $(obj),$(BOOTSTRAP_OBJS))
> +endif
> +
> +
> LIBS = lib/libgeneric.o
> LIBS += lib/lzma/liblzma.o
> LIBS += lib/lzo/liblzo.o
> @@ -270,6 +277,24 @@ LIBS := $(addprefix $(obj),$(sort $(LIBS)))
> LIBBOARD = board/$(BOARDDIR)/lib$(BOARD).o
> LIBBOARD := $(addprefix $(obj),$(LIBBOARD))
>
> +ifeq ($(CONFIG_BOOTSTRAP),y)
> +BOOTSTRAP_LIBS = lib/libgeneric_bootstrap.o
> +BOOTSTRAP_LIBS += arch/$(ARCH)/cpu/lib$(ARCH)_bootstrap.o
> +BOOTSTRAP_LIBS += arch/$(ARCH)/lib/lib$(ARCH)_bootstrap.o
> +BOOTSTRAP_LIBS += common/libcommon_bootstrap.o
> +BOOTSTRAP_LIBS += drivers/serial/libserial.o
> +
> +BOOTSTRAP_LIBS-$(CONFIG_BOOTSTRAP_LZMA) += lib/lzma/liblzma.o
> +BOOTSTRAP_LIBS-$(CONFIG_BOOTSTRAP_LZO) += lib/lzo/liblzo.o
> +BOOTSTRAP_LIBS-$(CONFIG_BOOTSTRAP_XZ) += lib/xz/libxz.o
> +BOOTSTRAP_LIBS += $(BOOTSTRAP_LIBS-y)
> +
> +.PHONY : $(BOOTSTRAP_LIBS)
> +
> +BOOTSTRAP_LIBBOARD = board/$(BOARDDIR)/lib$(BOARD)_bootstrap.o
> +BOOTSTRAP_LIBBOARD := $(addprefix $(obj),$(BOOTSTRAP_LIBBOARD))
> +endif
Can this not go to a separate, new directory?
> +__BOOTSTRAP_LIBS := $(subst $(obj),,$(BOOTSTRAP_LIBS)) $(subst
> $(obj),,$(BOOTSTRAP_LIBBOARD))
Lines too long... Please fix globally.
> +$(obj)u-boot.bin.gz: $(obj)u-boot.bin
> + gzip -c $< > $@
> +
> +$(obj)u-boot.bin.lzma: $(obj)u-boot.bin
> + lzma -e -z -c $< > $@
> +
> +$(obj)u-boot.bin.xz: $(obj)u-boot.bin
> + tools/xz_wrap.sh misc < $< > $@
> +
> +$(obj)u-boot.bin.lzo: $(obj)u-boot.bin
> + lzop -9 -c $< > $@
> +
> +$(obj)u-boot.bin.bz2: $(obj)u-boot.bin
> + bzip2 --best -z -c $< > $@
> +
Do we need this in the top level Makefile?
> @@ -373,7 +423,7 @@ $(obj)u-boot.dis: $(obj)u-boot
> 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) $$UNDEF_SYM $(__OBJS) \
> + cd $(LNDIR) && $(LD) --gc-sections $(LDFLAGS) $$UNDEF_SYM
> $(__OBJS) \
> --start-group $(__LIBS) --end-group $(PLATFORM_LIBS) \
> -Map u-boot.map -o u-boot
Be careful!! This will break toins of boards as you did not adapt the
linker scripts!
> +# Bootstrap targets
> +
> +ifeq ($(CONFIG_BOOTSTRAP),y)
> +$(obj)u-boot-bootstrap.hex: $(obj)u-boot-bootstrap
> + $(OBJCOPY) ${OBJCFLAGS} -O ihex $< $@
> +
> +$(obj)u-boot-bootstrap.srec: $(obj)u-boot-bootstrap
> + $(OBJCOPY) -O srec $< $@
> +
> +$(obj)u-boot-bootstrap.bin: $(obj)u-boot-bootstrap
> + $(OBJCOPY) ${OBJCFLAGS} -O binary $< $@
> + $(BOARD_SIZE_CHECK)
> +
> +$(obj)u-boot-bootstrap.bin.gz: $(obj)u-boot-bootstrap.bin
> + gzip -c $< > $@
> +
> +$(obj)u-boot-bootstrap.bin.lzma: $(obj)u-boot-bootstrap.bin
> + lzma -e -z -c $< > $@
> +
> +$(obj)u-boot.bin-bootstrap.lzo: $(obj)u-boot-bootstrap.bin
> + lzop -9 -c $< > $@
> +
> +$(obj)u-boot.bin-bootstrap.bz2: $(obj)u-boot-bootstrap.bin
> + bzip2 --best -z -c $< > $@
> +
> +$(obj)u-boot-bootstrap.ldr: $(obj)u-boot-bootstrap
> + $(CREATE_LDR_ENV)
> + $(LDR) -T $(CONFIG_BFIN_CPU) -c $@ $< $(LDR_FLAGS)
> + $(BOARD_SIZE_CHECK)
> +
> +$(obj)u-boot-bootstrap.ldr.hex: $(obj)u-boot-bootstrap.ldr
> + $(OBJCOPY) ${OBJCFLAGS} -O ihex $< $@ -I binary
> +
> +$(obj)u-boot-bootstrap.ldr.srec: $(obj)u-boot-bootstrap.ldr
> + $(OBJCOPY) ${OBJCFLAGS} -O srec $< $@ -I binary
> +
> +$(obj)u-boot-bootstrap.img: $(obj)u-boot-bootstrap.bin
> + $(obj)tools/mkimage -A $(ARCH) -T firmware -C none \
> + -a $(CONFIG_BOOTSTRAP_BASE) -e 0 \
> + -n $(shell sed -n -e 's/.*U_BOOT_VERSION//p' $(VERSION_FILE) | \
> + sed -e 's/"[ ]*$$/ for $(BOARD) board"/') \
> + -d $< $@
> +
> +$(obj)u-boot-bootstrap.imx: $(obj)u-boot-bootstrap.bin
> + $(obj)tools/mkimage -n $(IMX_CONFIG) -T imximage \
> + -e $(CONFIG_BOOTSTRAP_BASE) -d $< $@
> +
> +$(obj)u-boot-bootstrap.kwb: $(obj)u-boot-bootstrap.bin
> + $(obj)tools/mkimage -n $(CONFIG_SYS_KWD_CONFIG) -T kwbimage \
> + -a $(CONFIG_SYS_TEXT_BASE) -e $(CONFIG_SYS_TEXT_BASE) -d $< $@
> +
> +$(obj)u-boot-bootstrap.sha1: $(obj)u-boot-bootstrap.bin
> + $(obj)tools/ubsha1 $(obj)u-boot-bootstrap.bin
> +
> +$(obj)u-boot-bootstrap.dis: $(obj)u-boot-bootstrap
> + $(OBJDUMP) -d $< > $@
> +
> +PAYLOAD_FILE_BASE=$(obj)u-boot.bin
> +ifeq ($(CONFIG_BOOTSTRAP_GZIP),y)
> +PAYLOAD_FILE_EXT:=.gz
> +endif
> +ifeq ($(CONFIG_BOOTSTRAP_LZMA),y)
> +PAYLOAD_FILE_EXT:=.lzma
> +endif
> +ifeq ($(CONFIG_BOOTSTRAP_LZO),y)
> +PAYLOAD_FILE_EXT:=.lzo
> +endif
> +ifeq ($(CONFIG_BOOTSTRAP_BZIP2),y)
> +PAYLOAD_FILE_EXT:=.bz2
> +endif
> +ifeq ($(CONFIG_BOOTSTRAP_XZ),y)
> +PAYLOAD_FILE_EXT:=.xz
> +endif
> +
> +PAYLOAD_FILE := $(PAYLOAD_FILE_BASE)$(PAYLOAD_FILE_EXT)
> +
> +$(obj).payload.s: $(PAYLOAD_FILE)
> + echo ".globl payload_start" > $@
> + echo ".globl payload_end" >> $@
> + echo ".globl payload_size" >> $@
> + echo ".globl payload_uncsize" >> $@
> + echo .section .payload,\"a\",@progbits >> $@
> + echo "payload_size:" >> $@
> + echo -n ".word " >> $@
> + wc -c $(PAYLOAD_FILE) | cut -f1 -d' ' >> $@
> + echo "payload_uncsize:" >> $@
> + echo -n ".word " >> $@
> + wc -c $(obj)u-boot.bin | cut -f1 -d' ' >> $@
> + echo "payload_start:" >> $@
> + echo .incbin \"$(PAYLOAD_FILE)\" >> $@
> + echo "payload_end:" >> $@
> +
> +GEN_UBOOT_BOOTSTRAP = \
> + UNDEF_SYM=`$(OBJDUMP) -x $(BOOTSTRAP_LIBBOARD)
> $(BOOTSTRAP_LIBS) | \
> + sed -n -e
> 's/.*\($(SYM_PREFIX)__u_boot_cmd_.*\)/-u\1/p'|sort|uniq`;\
> + cd $(LNDIR) && $(LD) --gc-sections $(BOOTSTRAP_LDFLAGS)
> $$UNDEF_SYM $(obj).payload.o $(__BOOTSTRAP_OBJS) \
> + --start-group $(__BOOTSTRAP_LIBS) --end-group
> $(BOOTSTRAP_PLATFORM_LIBS) \
> + -Map u-boot-bootstrap.map -o u-boot-bootstrap
> +$(obj)u-boot-bootstrap: depend $(SUBDIRS) $(BOOTSTRAP_OBJS)
> $(BOOTSTRAP_LIBBOARD) $(BOOTSTRAP_LIBS) $(BOOTSTRAP_LDSCRIPT)
> $(obj)u-boot-bootstrap.lds $(obj).payload.o
> + $(GEN_UBOOT_BOOTSTRAP)
> +ifeq ($(CONFIG_KALLSYMS),y)
> + smap=`$(call SYSTEM_MAP,u-boot-bootstrap) | \
> + awk '$$2 ~ /[tTwW]/ {printf $$1 $$3 "\\\\000"}'` ; \
> + $(CC) $(CFLAGS) -DSYSTEM_MAP="\"$${smap}\"" \
> + -c common/system_map.c -o $(obj)common/system_map.o
> + $(GEN_UBOOT_BOOTSTRAP) $(obj)common/system_map.o
> +endif
> +
> +$(BOOTSTRAP_LIBBOARD): depend $(BOOTSTRAP_LIBS)
> + $(MAKE) -C $(dir $(subst $(obj),,$@)) $(notdir $@)
> +endif
NAK!! Please find a way to implement this without adding all that
code to the top level Makefile.
> +- Compressed U-Boot support:
> + CONFIG_BOOTSTRAP_BASE
> +
> + This option enables the Boostrap infrastructure.
> + The bootstrap code consists of a small binary that is able
> + to decompress an attached payload (a full U-Boot image),
> + allowing to have a small but also full featured U-Boot
> + bootloader.
That means you have to make adjustments to the init code of all
architectures / boards. I do not see this in this patch, nor are
there any preceeding patches that prepare the ground. In other words,
this will not work at best, or more likely break zillions of boards.
On which architectures / boards has this been tested?
> + BOOTSTRAP_LDSCRIPT
> +
> + LD script for bootstrap code. If not defined, a board specific
> script will be
> + used.
This is the wrong way around. Only if a board specific script is
needed there should be need for such a definition; in all other cases
a default script should be used.
> +ifneq ($(CONFIG_BOOTSTRAP_TEXT_BASE),)
> +CPPFLAGS += -DCONFIG_BOOTSTRAP_TEXT_BASE=$(CONFIG_BOOTSTRAP_TEXT_BASE)
> +endif
This should never be necessary.
> +#ifdef CONFIG_BOOTSTRAP_BZIP2
> +#define _CONFIG_BOOTSTRAP_RELOCATE
> +#define _CONFIG_BOOTSTRAP_MALLOC
> +// #define _CONFIG_BOOTSTRAP_FAKEMALLOC
> +#endif
C++ comments are not allowed. Please fix globally.
> +static const char *algo =
> +#if defined(CONFIG_BOOTSTRAP_GZIP)
> + "gzip";
> +#elif defined(CONFIG_BOOTSTRAP_LZMA)
> + "lzma";
> +#elif defined(CONFIG_BOOTSTRAP_LZO)
> + "lzo";
> +#elif defined(CONFIG_BOOTSTRAP_BZIP2)
> + "bzip2";
> +#elif defined(CONFIG_BOOTSTRAP_XZ)
> + "xz";
> +#else
> + "flat";
> +#endif
Previous descriptions sounded as if all these formats were supported
in parallel. Not it seems only one is at a time, and there is a search
order. This must at least be documented. It would be better if an
error was raised if two options are selected.
> + printf("Uncompressing payload (%s)...", algo);
> +#if defined(CONFIG_BOOTSTRAP_GZIP)
> + ret = gunzip(dst, unc_size, src, &size);
> +#elif defined(CONFIG_BOOTSTRAP_LZMA)
> + SizeT outsize = unc_size;
What's "SizeT" ?? We don;t allow such names in U-Boot.
> + if (ret) {
> + printf("failed with error %d.\n", ret);
> + bootstrap_hang();
> + } else {
> + puts("done.\n");
> + }
> + return ret;
Indentation by TAB, please!
> \ No newline at end of file
And fix this, too.
> diff --git a/tools/xz_wrap.sh b/tools/xz_wrap.sh
> new file mode 100755
> index 0000000..dbe5e55
> --- /dev/null
> +++ b/tools/xz_wrap.sh
> @@ -0,0 +1,45 @@
> +#!/bin/sh
> +#
> +# This is a wrapper for xz to use appropriate compression options depending
> +# on what is being compressed. The only argument to this script should be
> +# "kernel" or "misc" to indicate what is being compressed.
What do we use?
> +# Author: Lasse Collin <lasse.col...@tukaani.org>
> +#
> +# This file has been put into the public domain.
> +# You can do whatever you want with this file.
> +#
> +
> +# Defaults: No BCJ filter and no extra LZMA2 options.
> +BCJ=
> +LZMA2OPTS=
> +
> +# Big dictionary is OK for the kernel image, but it's not OK
> +# for other things.
> +#
> +# BCJ filter is used only for the kernel, at least for now.
> +# It could be useful for non-trivial initramfs too, but it
> +# depends on the exact content of the initramfs image.
> +case $1 in
> + kernel)
> + DICT=16MiB
> + case $ARCH in
> + x86|x86_64) BCJ=--x86 ;;
> + powerpc) BCJ=--powerpc ;;
> + ia64) BCJ=--ia64; LZMA2OPTS=pb=4 ;;
> + arm) BCJ=--arm ;;
> + sparc) BCJ=--sparc ;;
> + esac
Error handling for other architectures?
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
ACHTUNG!!!
Das machine is nicht fur gefingerpoken und mittengrabben. Ist easy
schnappen der springenwerk, blowenfusen und corkenpoppen mit spitzen-
sparken. Ist nicht fur gewerken by das dummkopfen. Das rubbernecken
sightseeren keepen hands in das pockets. Relaxen und vatch das
blinkenlights!!!
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
