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

[Detlev Zundel]

Hi Bastian,

>  > Hi Bastian,
>
> Hello Detlev,
>
> sorry for the delay.

No worries ;)

>> > 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.
>>
>
> OK, i can follow your doubts here. The UBI section is available without 
> the UBIFS selection, now.

Sounds good, but I will need to actually see it, to comment on it
further ;)

>> > +       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?
>  
> In this patch UBIFS settings are necessary for the UBI image creation, 
> so i put this "Create UBI only for UBIFS" in.
>
>> 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.
>
> In a new patch i put a "Create UBI for this UBIFS" in because of the 
> dependencies mentioned above, and leave further improvements for later.

Dito.

>> > +
>> > +config UBI_CFG_VOL_NAME
>> > +       depends IMAGE_UBI
>> > +       string "vol_name"
>
> [...]
>
>> > +
>> > +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.
>> 
>
> I changed this issue to a "prefix for the ubinize tool". I would like to 
> have a build system independend version for this tool for production 
> purposes. 

Then we should use a variable UBINIZE and initialize it with ?= in the
makefile.  This way you can override it from the calling context but
still get a sensible default if you don't care.

Cheers
  Detlev

-- 
What is piracy?   Piracy is the act of stealing an artist's work without
any intention of paying for it. I'm not talking about Napster-type soft-
ware.  I'm talking about major label recording contracts.
           -- Courtney Love
--
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