[ELDK] [PATCH] RFSB: add support for ubi image creation for ubifs images

[Detlev Zundel]

Hi Bastian,

> An ubifs image can be placed in an ubi image. This image can
> be flashed with the ubiformat utility.

Thanks for the patch - I like what you do, but I have a few comments.

> Signed-off-by: Bastian Ruppert <Bastian.Ruppert at Sewerin.de>
> ---
>  Config.in |   43 +++++++++++++++++++++++++++++++++++++++++++
>  Makefile  |   26 ++++++++++++++++++++++++--
>  2 files changed, 67 insertions(+), 2 deletions(-)
>
> diff --git a/Config.in b/Config.in
> index 03dd3ff..3cea227 100644
> --- a/Config.in
> +++ b/Config.in
> @@ -204,6 +204,49 @@ config UBIFS_MAX_LEB_COUNT
>  	 there is no default value.  More information under:
>  	 http://www.linux-mtd.infradead.org/faq/ubifs.html#L_max_leb_cnt
>  
> +menu "UBI Image"
> +     depends IMAGE_UBIFS
> +
> +config IMAGE_UBI
> +       depends IMAGE_UBIFS

Maybe this is not generic enough - I can think of situations wher we
want to put another filesystem image (i.e. squashfs) also into a ubi
image.  If we can do this, then this dependency is not correct.

> +       bool "Create UBI image"
> +       default n
> +       help
> +         Create a UBI image containing the UBIFS image. This image can
> +	 be flashed to a mtd device with the ubiformat tool.

Maybe we should try to have a "build an UBI image with the given
contents" and allow the contents to be defined.  What do you think?  But
if we cannot ponder on the complexity of this, then I'd rather add your
change as is and leave the improvements for later.

> +
> +config UBI_CFG_VOL_NAME
> +       depends IMAGE_UBI
> +       string "vol_name"
> +       default "rootfs"
> +       help
> +         This will be used for vol_name in the cfg file
> +
> +config UBI_ERASE_BLOCK_SIZE
> +       depends IMAGE_UBI
> +       int "ubi Eraseblock size in KiB"
> +       default 0
> +       help
> +         You must set this value because it is device dependent.  Set
> +	 the Eraseblock size (peb-size) as reported by the mtdinfo utility.
> +
> +config UBI_SUB_PAGE_SIZE
> +       depends IMAGE_UBI
> +       int "Sub page size"
> +       default 0
> +       help
> +         You must set this value because it is device dependent.  Set
> +	 the Sub page size as reported by the mtdinfo utility.
> +
> +config UBI_UBINIZE_CMD
> +       depends IMAGE_UBI
> +       string "ubinize command"
> +       default "ubinize"
> +       help
> +         Set the ubinize tool to use.

Is this needed?  Will there ever be another tool that we want to use?
If not, then please remove this option and continue to use a makefile
variable to reference the tool defined at the top.

> +
> +endmenu
> +
>  config IMAGE_CRAMFS
>         bool "Create CRamFS image"
>         default n
> diff --git a/Makefile b/Makefile
> index 7c40960..0b8fd4a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -38,6 +38,8 @@ CRAMFS_FILE	  := $(BUILD)/images/image.cramfs
>  EXT2_FILE	  := $(BUILD)/images/image.ext2
>  JFFS2_FILE	  := $(BUILD)/images/image.jffs2
>  UBIFS_FILE	  := $(BUILD)/images/image.ubifs
> +UBI_FILE	  := $(BUILD)/images/image.ubi
> +UBI_CFG_FILE	  := $(BUILD)/images/ubinize_rootfs.cfg

Hm, this file does not belong into the "images" directory.  Maybe we
should add a "AUTOGENDIR := $(BUILD)/generated" directory for such
files?

>  URAMDISK_FILE	  := $(BUILD)/images/uRamdisk
>  SQFS_DEV_TAB	  := $(BUILD)/images/squashfs_devices.tab
>  SQUASHFS_FILE	  := $(BUILD)/images/image.squashfs
> @@ -97,7 +99,7 @@ ifeq ($(strip $$($(2))),y)
>  endif
>  endef
>  image_targets =
> -IMAGE_CONFIGS = EXT2 URAMDISK JFFS2 CRAMFS SQUASHFS UBIFS
> +IMAGE_CONFIGS = EXT2 URAMDISK JFFS2 CRAMFS SQUASHFS UBIFS UBI
>  $(foreach img,$(IMAGE_CONFIGS),$(eval $(call ADD_CONDITIONAL,image_targets,IMAGE_$(img),$$($(img)_FILE))))
>  $(info Current target list is $(image_targets))
>  
> @@ -295,7 +297,7 @@ pkgs_clean:	$(TARGETS_CLEAN)
>  ############################################################
>  # Image targets below
>  ############################################################
> -.PHONY:	image_cramfs image_ext2 image_jffs2 image_ubifs image_uramdisk
> +.PHONY:	image_cramfs image_ext2 image_jffs2 image_ubifs image_ubi image_uramdisk
>  
>  image_ext2:	$(EXT2_FILE)
>  
> @@ -328,6 +330,26 @@ $(UBIFS_FILE):	$(STAMP_ROOTFS)
>  		--max-leb-cnt=$(UBIFS_MAX_LEB_COUNT) -o $(UBIFS_FILE); \
>  	fi
>  
> +image_ubi:	$(UBI_FILE) $(UBIFS_FILE)
> +
> +$(UBI_FILE):	$(STAMP_ROOTFS)
> +	@echo Generating UBI image
> +	@if [[ $(UBI_SUB_PAGE_SIZE) == 0 || $(UBI_ERASE_BLOCK_SIZE) == 0 ]];then \
> +		echo "ERROR: Not all UBI parameters set.  Fix your configuration and try again" >&2 ; \
> +	else \
> +	echo "[ubifs]" > $(UBI_CFG_FILE);	\
> +	echo "mode=ubi" >> $(UBI_CFG_FILE);	\
> +	echo "image="$(UBIFS_FILE) >> $(UBI_CFG_FILE);	\
> +	echo "vol_id=0" >> $(UBI_CFG_FILE);	\
> +	echo "vol_size="$(IMAGE_SIZE)"KiB" >> $(UBI_CFG_FILE); \
> +	echo "vol_type=dynamic" >> $(UBI_CFG_FILE);\
> +	echo "vol_name="$(UBI_CFG_VOL_NAME) >> $(UBI_CFG_FILE);\
> +	echo "vol_flags=autoresize" >> $(UBI_CFG_FILE);\
> +	$(UBI_UBINIZE_CMD) --output=$(UBI_FILE) --min-io-size=$(UBIFS_MIN_IO_SIZE) \
> +		--peb-size=$(UBI_ERASE_BLOCK_SIZE)KiB --sub-page-size=$(UBI_SUB_PAGE_SIZE) \
> +		$(UBI_CFG_FILE);\
> +	fi
> +
>  image_uramdisk:	$(URAMDISK_FILE)
>  
>  $(URAMDISK_FILE):	$(EXT2_FILE)

Cheers
  Detlev

-- 
Cyberwar is certainly not a myth. But you haven't seen it yet, despite
the attacks on Estonia. Cyberwar is warfare in cyberspace. And warfare
involves massive death and destruction. When you see it, you'll know it.
                           -- Bruce Schneier, Nov. 2007
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de