Hi Wolfgang and ML,
On Sat, Dec 4, 2010 at 11:47 PM, Wolfgang Denk wd@denx.de wrote:
Dear Luigi 'Comio' Mantellini,
Not only the commit message, also the remaining text is full of typos. Please run through a spell checker.
you are right. I will check better my english. sorry!
Signed-off-by: Luigi 'Comio' Mantellini luigi.mantellini@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
...
Please explain why all these new images are needed?
The u-boot.bin.* images will used as payload. u-boot-bootstrap.bin.* can be dropped. can A rule u-boot.bin.* be considered safe?
@@ -39,7 +59,7 @@ /LOG /errlog /reloc_off
+/.payload.s
.payload.s is an assembler file generated at compile time to include the payload file (using .incbin directive).
???
+examples/standalone/
+setvars
???
Sorry... these are wrongs. I will remove.
+ifeq ($(CONFIG_BOOTSTRAP),y)
...
+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?
I would like to re-factorize again the patch(es) (if you consider this feature useful...) and I would like to better understand a possible directory structure. To cerate bootstrap, I need some (1) bootstrap specific files, some (2) non specific bootstrap files (like decompression libraries) and some (3) cut-down files (that in this patch have name like *_bootstrap.c).
Which is your suggestion to organize these files?
Regarding the (1) bootstrap specific files, like main makefile, the bootstrap.c file, I will move into a single directory. I have 2 choices: /bootsrap in the rootdir or /lib/bootstrap... I prefer the second choice. Furthermore I noticed that nand_spl and onenand_ipl code are placed into the rootdir.
Regarding the (2) non specific code, following the nand_spl and onenand_ipl examples, I should create sym links to the real code sources. Honestly, I don't like this approach but if you thinks that is a good way, I will follow. I will create a ghost u-boot tree into the bootstrap directory with little-bit different makefiles, in order to check CONFIG_BOOTSTRAP_* symbols.
Regarding the (3) cut-down fles (for example strat_bootstrap.S), I will avoid to use directly the u-boot original files, because the code is similar but not equal and I will introduce a lot of ifdef/endif to adapt the behaviour. In this case, also following the nand_spl and onenand_ipl examples, I will create a ghost tree in the /bootstrap directory for each cpu (even if I will send work for only mips)
The last question is regarding the boards that should add some bootstrap specific code... where should this code be placed? into the board/BOARD/ directory or into a directory like /bootstrap/board/BOARD/?
Sorry for these stupid questions, but I would avoid to iterate a lot of time on this work... and of course I will spend time only if this can be considered useful by community (this code is from my job and my boss asked by already to do other activities...).
+__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.bz2: $(obj)u-boot.bin
- bzip2 --best -z -c $< > $@
Do we need this in the top level Makefile?
Create compressed files can be useful. I can move into a different makefile or create suffix rules like
%.bin.bz2: %.bin bzip2 --best -z -c $< > $@
@@ -373,7 +423,7 @@ $(obj)u-boot.dis: $(obj)u-boot GEN_UBOOT = \
..
- cd $(LNDIR) && $(LD) $(LDFLAGS) $$UNDEF_SYM $(__OBJS) \
- cd $(LNDIR) && $(LD) --gc-sections $(LDFLAGS) $$UNDEF_SYM $(__OBJS) \
..
Be careful!! This will break toins of boards as you did not adapt the linker scripts!
Fixed. it was from previous experiments. I removed this from my sources.
+# 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.
ok
+- 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.
I can propose code only for mips (that is the platform that use every day).
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.
I added for symmetry respect SYS_TEXT_BASE.
+#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.
C99 :) ok
+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.
The bootstrap supports just the algo used by the linked payload. I will add a coherency code to avoid multiple algo definitions
- 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.
ok SizeT is used by lzma library
- 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.
ok
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.collin@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?
This is from xz embedded module and I just copied into scripts directory. I will remove xz in the next iteration.
Thanks again for your detailed review.
best regards,
luigi
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: wd@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