All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] JFFS2 images: bugfixes and enhancements
@ 2013-10-20 22:34 Andrea Adami
  2013-10-20 22:34 ` [PATCH 1/5] image_types.bbclass: do not force --no-cleanmarkers for jffs2 Andrea Adami
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Andrea Adami @ 2013-10-20 22:34 UTC (permalink / raw)
  To: openembedded-core

The first patch is a bugfix allowing the creation of images with cleanmarkers
for devices with NOR flash.
The second and the third ones are fixes for the options passed to sumtool.

After those first patches it is possible to customize again i.e.

 EXTRA_IMAGECMD_JFFS2 = "--pad=14680064 -l --erase-block=${JFFS2_ERASEBLOCK}"

In the normal case (NAND flash) only JFFS2_ERASEBLOCK is necessary and
having to redeclare the full EXTRA_IMAGECMD_jffs2 seems counter-intuitive
and has revealed to be error-prone.

So, the fourth and fifth patches are proposed enhancements to further parametrize
for both jffs2 and sum.jffs2 images by using two new optional vars like done for
JFFS2_ERASEBLOCK:

JFFS2_ERASEBLOCK = "0x20000"
JFFS2_NOCLEANMARKERS = ""
JFFS2_PADDING = "--pad=14680064"

Andrea Adami (5):
  image_types.bbclass: do not force --no-cleanmarkers for jffs2
  image_types.bbclass: fix endiannes for sumtool (jffs2 summary)
  image_types.bbclass: fix EXTRA_IMAGECMD_sum.jffs2
  image_types.bbclass: add JFFS2_NOCLEANMARKERS variable
  image_types.bbclass: add JFFS2_PADDING variable

 meta/classes/image_types.bbclass | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

-- 
1.8.1.5



^ permalink raw reply	[flat|nested] 12+ messages in thread
* [PATCH 1/5] image_types.bbclass: do not force --no-cleanmarkers for jffs2
  2013-10-20 22:34 [PATCH 0/5] JFFS2 images: bugfixes and enhancements Andrea Adami
@ 2013-10-20 22:34 ` Andrea Adami
  2013-10-20 22:34 ` [PATCH 2/5] image_types.bbclass: fix endiannes for sumtool (jffs2 summary) Andrea Adami
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Andrea Adami @ 2013-10-20 22:34 UTC (permalink / raw)
  To: openembedded-core

Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
---
 meta/classes/image_types.bbclass | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
index 9ead059..8693a8f 100644
--- a/meta/classes/image_types.bbclass
+++ b/meta/classes/image_types.bbclass
@@ -140,9 +140,9 @@ XZ_COMPRESSION_LEVEL ?= "-e -6"
 XZ_INTEGRITY_CHECK ?= "crc32"
 XZ_THREADS ?= "-T 0"
 
-IMAGE_CMD_jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 -n ${EXTRA_IMAGECMD}"
+IMAGE_CMD_jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 ${EXTRA_IMAGECMD}"
 IMAGE_CMD_sum.jffs2 = "${IMAGE_CMD_jffs2} && sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 \
-	-o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 -n ${EXTRA_IMAGECMD}"
+	-o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
 
 IMAGE_CMD_cramfs = "mkfs.cramfs ${IMAGE_ROOTFS} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.cramfs ${EXTRA_IMAGECMD}"
 
-- 
1.8.1.5



^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH 2/5] image_types.bbclass: fix endiannes for sumtool (jffs2 summary)
  2013-10-20 22:34 [PATCH 0/5] JFFS2 images: bugfixes and enhancements Andrea Adami
  2013-10-20 22:34 ` [PATCH 1/5] image_types.bbclass: do not force --no-cleanmarkers for jffs2 Andrea Adami
@ 2013-10-20 22:34 ` Andrea Adami
  2013-10-20 22:34 ` [PATCH 3/5] image_types.bbclass: fix EXTRA_IMAGECMD_sum.jffs2 Andrea Adami
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Andrea Adami @ 2013-10-20 22:34 UTC (permalink / raw)
  To: openembedded-core

For mkfs.jffs2 endianness can be be expressed in the long or short form
with the optional size argument:

 --little-endian
 -l

Strangely the sumtool has a different syntax and does accept the forms:

 --littleendian
 -l

Prefer the short form valid for both tools.

Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
---
 meta/classes/image_types.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
index 8693a8f..b8779e0 100644
--- a/meta/classes/image_types.bbclass
+++ b/meta/classes/image_types.bbclass
@@ -209,7 +209,7 @@ IMAGE_CMD_ubifs = "mkfs.ubifs -r ${IMAGE_ROOTFS} -o ${DEPLOY_DIR_IMAGE}/${IMAGE_
 EXTRA_IMAGECMD = ""
 
 inherit siteinfo
-JFFS2_ENDIANNESS ?= "${@base_conditional('SITEINFO_ENDIANNESS', 'le', '--little-endian', '--big-endian', d)}"
+JFFS2_ENDIANNESS ?= "${@base_conditional('SITEINFO_ENDIANNESS', 'le', '-l', '-b', d)}"
 JFFS2_ERASEBLOCK ?= "0x40000"
 EXTRA_IMAGECMD_jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} --no-cleanmarkers"
 
-- 
1.8.1.5



^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH 3/5] image_types.bbclass: fix EXTRA_IMAGECMD_sum.jffs2
  2013-10-20 22:34 [PATCH 0/5] JFFS2 images: bugfixes and enhancements Andrea Adami
  2013-10-20 22:34 ` [PATCH 1/5] image_types.bbclass: do not force --no-cleanmarkers for jffs2 Andrea Adami
  2013-10-20 22:34 ` [PATCH 2/5] image_types.bbclass: fix endiannes for sumtool (jffs2 summary) Andrea Adami
@ 2013-10-20 22:34 ` Andrea Adami
  2013-10-29 10:26   ` Andrea Adami
  2013-10-29 11:01   ` Richard Purdie
  2013-10-20 22:34 ` [PATCH 4/5] image_types.bbclass: add JFFS2_NOCLEANMARKERS variable Andrea Adami
  2013-10-20 22:34 ` [PATCH 5/5] image_types.bbclass: add JFFS2_PADDING variable Andrea Adami
  4 siblings, 2 replies; 12+ messages in thread
From: Andrea Adami @ 2013-10-20 22:34 UTC (permalink / raw)
  To: openembedded-core

When overriding EXTRA_IMAGE_CMD_jffs2 = "--pad=foo ..."
we are passing a malformed option to sumtool:

sumtool: option '--pad' doesn't allow an argument

Fix this by declaring a separate variable for the purpose.

Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
---
 meta/classes/image_types.bbclass | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
index b8779e0..21391e8 100644
--- a/meta/classes/image_types.bbclass
+++ b/meta/classes/image_types.bbclass
@@ -141,8 +141,8 @@ XZ_INTEGRITY_CHECK ?= "crc32"
 XZ_THREADS ?= "-T 0"
 
 IMAGE_CMD_jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 ${EXTRA_IMAGECMD}"
-IMAGE_CMD_sum.jffs2 = "${IMAGE_CMD_jffs2} && sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 \
-	-o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
+IMAGE_CMD_sum.jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 ${EXTRA_IMAGECMD_jffs2} \
+	&& sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
 
 IMAGE_CMD_cramfs = "mkfs.cramfs ${IMAGE_ROOTFS} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.cramfs ${EXTRA_IMAGECMD}"
 
@@ -212,6 +212,7 @@ inherit siteinfo
 JFFS2_ENDIANNESS ?= "${@base_conditional('SITEINFO_ENDIANNESS', 'le', '-l', '-b', d)}"
 JFFS2_ERASEBLOCK ?= "0x40000"
 EXTRA_IMAGECMD_jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} --no-cleanmarkers"
+EXTRA_IMAGECMD_sum.jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} --no-cleanmarkers"
 
 # Change these if you want default mkfs behavior (i.e. create minimal inode number)
 EXTRA_IMAGECMD_ext2 ?= "-i 8192"
-- 
1.8.1.5



^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH 4/5] image_types.bbclass: add JFFS2_NOCLEANMARKERS variable
  2013-10-20 22:34 [PATCH 0/5] JFFS2 images: bugfixes and enhancements Andrea Adami
                   ` (2 preceding siblings ...)
  2013-10-20 22:34 ` [PATCH 3/5] image_types.bbclass: fix EXTRA_IMAGECMD_sum.jffs2 Andrea Adami
@ 2013-10-20 22:34 ` Andrea Adami
  2013-10-20 22:34 ` [PATCH 5/5] image_types.bbclass: add JFFS2_PADDING variable Andrea Adami
  4 siblings, 0 replies; 12+ messages in thread
From: Andrea Adami @ 2013-10-20 22:34 UTC (permalink / raw)
  To: openembedded-core

The '--no-clenmarkers' value is the actual default and
is valid for devices with NAND flash.

Machine configuration files can redefine it instead of
rewriting EXTRA_IMAGECMD_JFFS2.

Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
---
 meta/classes/image_types.bbclass | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
index 21391e8..372420d 100644
--- a/meta/classes/image_types.bbclass
+++ b/meta/classes/image_types.bbclass
@@ -211,8 +211,9 @@ EXTRA_IMAGECMD = ""
 inherit siteinfo
 JFFS2_ENDIANNESS ?= "${@base_conditional('SITEINFO_ENDIANNESS', 'le', '-l', '-b', d)}"
 JFFS2_ERASEBLOCK ?= "0x40000"
-EXTRA_IMAGECMD_jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} --no-cleanmarkers"
-EXTRA_IMAGECMD_sum.jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} --no-cleanmarkers"
+JFFS2_NOCLEANMARKERS ?= "--no-cleanmarkers"
+EXTRA_IMAGECMD_jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} ${JFFS2_NOCLEANMARKERS}"
+EXTRA_IMAGECMD_sum.jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} ${JFFS2_NOCLEANMARKERS}"
 
 # Change these if you want default mkfs behavior (i.e. create minimal inode number)
 EXTRA_IMAGECMD_ext2 ?= "-i 8192"
-- 
1.8.1.5



^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH 5/5] image_types.bbclass: add JFFS2_PADDING variable
  2013-10-20 22:34 [PATCH 0/5] JFFS2 images: bugfixes and enhancements Andrea Adami
                   ` (3 preceding siblings ...)
  2013-10-20 22:34 ` [PATCH 4/5] image_types.bbclass: add JFFS2_NOCLEANMARKERS variable Andrea Adami
@ 2013-10-20 22:34 ` Andrea Adami
  4 siblings, 0 replies; 12+ messages in thread
From: Andrea Adami @ 2013-10-20 22:34 UTC (permalink / raw)
  To: openembedded-core

Defaulting to '--pad', meaning to pad until the end of the erasesector.

Allow machine.conf to override the value without redeclaring
EXTRA_IMAGECMD_jffs2.

Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
---
 meta/classes/image_types.bbclass | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
index 372420d..c5bacd8 100644
--- a/meta/classes/image_types.bbclass
+++ b/meta/classes/image_types.bbclass
@@ -210,9 +210,10 @@ EXTRA_IMAGECMD = ""
 
 inherit siteinfo
 JFFS2_ENDIANNESS ?= "${@base_conditional('SITEINFO_ENDIANNESS', 'le', '-l', '-b', d)}"
+JFFS2_PADDING ?= "--pad"
 JFFS2_ERASEBLOCK ?= "0x40000"
 JFFS2_NOCLEANMARKERS ?= "--no-cleanmarkers"
-EXTRA_IMAGECMD_jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} ${JFFS2_NOCLEANMARKERS}"
+EXTRA_IMAGECMD_jffs2 ?= "${JFFS2_PADDING} ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} ${JFFS2_NOCLEANMARKERS}"
 EXTRA_IMAGECMD_sum.jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} ${JFFS2_NOCLEANMARKERS}"
 
 # Change these if you want default mkfs behavior (i.e. create minimal inode number)
-- 
1.8.1.5



^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [PATCH 3/5] image_types.bbclass: fix EXTRA_IMAGECMD_sum.jffs2
  2013-10-20 22:34 ` [PATCH 3/5] image_types.bbclass: fix EXTRA_IMAGECMD_sum.jffs2 Andrea Adami
@ 2013-10-29 10:26   ` Andrea Adami
  2013-10-29 11:01   ` Richard Purdie
  1 sibling, 0 replies; 12+ messages in thread
From: Andrea Adami @ 2013-10-29 10:26 UTC (permalink / raw)
  To: openembedded-core

BUMP

I've seen the first two bugfixes have been applied and now it is
possible to override EXTRA_IMAGECMD_jffs2.

This patch fixes the case of sum.jffs2 when a padding value is
specified because sumtool won't swallow it.

Thx

Andrea


On Mon, Oct 21, 2013 at 12:34 AM, Andrea Adami <andrea.adami@gmail.com> wrote:
> When overriding EXTRA_IMAGE_CMD_jffs2 = "--pad=foo ..."
> we are passing a malformed option to sumtool:
>
> sumtool: option '--pad' doesn't allow an argument
>
> Fix this by declaring a separate variable for the purpose.
>
> Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
> ---
>  meta/classes/image_types.bbclass | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
> index b8779e0..21391e8 100644
> --- a/meta/classes/image_types.bbclass
> +++ b/meta/classes/image_types.bbclass
> @@ -141,8 +141,8 @@ XZ_INTEGRITY_CHECK ?= "crc32"
>  XZ_THREADS ?= "-T 0"
>
>  IMAGE_CMD_jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 ${EXTRA_IMAGECMD}"
> -IMAGE_CMD_sum.jffs2 = "${IMAGE_CMD_jffs2} && sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 \
> -       -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
> +IMAGE_CMD_sum.jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 ${EXTRA_IMAGECMD_jffs2} \
> +       && sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
>
>  IMAGE_CMD_cramfs = "mkfs.cramfs ${IMAGE_ROOTFS} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.cramfs ${EXTRA_IMAGECMD}"
>
> @@ -212,6 +212,7 @@ inherit siteinfo
>  JFFS2_ENDIANNESS ?= "${@base_conditional('SITEINFO_ENDIANNESS', 'le', '-l', '-b', d)}"
>  JFFS2_ERASEBLOCK ?= "0x40000"
>  EXTRA_IMAGECMD_jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} --no-cleanmarkers"
> +EXTRA_IMAGECMD_sum.jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} --no-cleanmarkers"
>
>  # Change these if you want default mkfs behavior (i.e. create minimal inode number)
>  EXTRA_IMAGECMD_ext2 ?= "-i 8192"
> --
> 1.8.1.5
>


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 3/5] image_types.bbclass: fix EXTRA_IMAGECMD_sum.jffs2
  2013-10-20 22:34 ` [PATCH 3/5] image_types.bbclass: fix EXTRA_IMAGECMD_sum.jffs2 Andrea Adami
  2013-10-29 10:26   ` Andrea Adami
@ 2013-10-29 11:01   ` Richard Purdie
  2013-10-29 11:17     ` Andrea Adami
  1 sibling, 1 reply; 12+ messages in thread
From: Richard Purdie @ 2013-10-29 11:01 UTC (permalink / raw)
  To: Andrea Adami; +Cc: openembedded-core

On Mon, 2013-10-21 at 00:34 +0200, Andrea Adami wrote:
> When overriding EXTRA_IMAGE_CMD_jffs2 = "--pad=foo ..."
> we are passing a malformed option to sumtool:
> 
> sumtool: option '--pad' doesn't allow an argument
> 
> Fix this by declaring a separate variable for the purpose.
> 
> Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
> ---
>  meta/classes/image_types.bbclass | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
> index b8779e0..21391e8 100644
> --- a/meta/classes/image_types.bbclass
> +++ b/meta/classes/image_types.bbclass
> @@ -141,8 +141,8 @@ XZ_INTEGRITY_CHECK ?= "crc32"
>  XZ_THREADS ?= "-T 0"
>  
>  IMAGE_CMD_jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 ${EXTRA_IMAGECMD}"
> -IMAGE_CMD_sum.jffs2 = "${IMAGE_CMD_jffs2} && sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 \
> -	-o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
> +IMAGE_CMD_sum.jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 ${EXTRA_IMAGECMD_jffs2} \
> +	&& sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
>  
>  IMAGE_CMD_cramfs = "mkfs.cramfs ${IMAGE_ROOTFS} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.cramfs ${EXTRA_IMAGECMD}"
>  
> @@ -212,6 +212,7 @@ inherit siteinfo
>  JFFS2_ENDIANNESS ?= "${@base_conditional('SITEINFO_ENDIANNESS', 'le', '-l', '-b', d)}"
>  JFFS2_ERASEBLOCK ?= "0x40000"
>  EXTRA_IMAGECMD_jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} --no-cleanmarkers"
> +EXTRA_IMAGECMD_sum.jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} --no-cleanmarkers"
>  
>  # Change these if you want default mkfs behavior (i.e. create minimal inode number)
>  EXTRA_IMAGECMD_ext2 ?= "-i 8192"

This patch is very confused. You say sumtool doesn't take a --pad
option, yet "EXTRA_IMAGECMD_sum.jffs" which is presumably used with
sumtool does have the option.

We need to make this clearer and I don't think this patch does that, I'm
not even sure it works...

Cheers,

Richard



^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 3/5] image_types.bbclass: fix EXTRA_IMAGECMD_sum.jffs2
  2013-10-29 11:01   ` Richard Purdie
@ 2013-10-29 11:17     ` Andrea Adami
  2013-10-29 11:45       ` Richard Purdie
  0 siblings, 1 reply; 12+ messages in thread
From: Andrea Adami @ 2013-10-29 11:17 UTC (permalink / raw)
  To: Richard Purdie; +Cc: openembedded-core

On Tue, Oct 29, 2013 at 12:01 PM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Mon, 2013-10-21 at 00:34 +0200, Andrea Adami wrote:
>> When overriding EXTRA_IMAGE_CMD_jffs2 = "--pad=foo ..."
>> we are passing a malformed option to sumtool:
>>
>> sumtool: option '--pad' doesn't allow an argument
>>
>> Fix this by declaring a separate variable for the purpose.
>>
>> Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
>> ---
>>  meta/classes/image_types.bbclass | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
>> index b8779e0..21391e8 100644
>> --- a/meta/classes/image_types.bbclass
>> +++ b/meta/classes/image_types.bbclass
>> @@ -141,8 +141,8 @@ XZ_INTEGRITY_CHECK ?= "crc32"
>>  XZ_THREADS ?= "-T 0"
>>
>>  IMAGE_CMD_jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 ${EXTRA_IMAGECMD}"
>> -IMAGE_CMD_sum.jffs2 = "${IMAGE_CMD_jffs2} && sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 \
>> -     -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
>> +IMAGE_CMD_sum.jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 ${EXTRA_IMAGECMD_jffs2} \
>> +     && sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
>>
>>  IMAGE_CMD_cramfs = "mkfs.cramfs ${IMAGE_ROOTFS} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.cramfs ${EXTRA_IMAGECMD}"
>>
>> @@ -212,6 +212,7 @@ inherit siteinfo
>>  JFFS2_ENDIANNESS ?= "${@base_conditional('SITEINFO_ENDIANNESS', 'le', '-l', '-b', d)}"
>>  JFFS2_ERASEBLOCK ?= "0x40000"
>>  EXTRA_IMAGECMD_jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} --no-cleanmarkers"
>> +EXTRA_IMAGECMD_sum.jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} --no-cleanmarkers"
>>
>>  # Change these if you want default mkfs behavior (i.e. create minimal inode number)
>>  EXTRA_IMAGECMD_ext2 ?= "-i 8192"
>
> This patch is very confused. You say sumtool doesn't take a --pad
> option, yet "EXTRA_IMAGECMD_sum.jffs" which is presumably used with
> sumtool does have the option.
>

After the commits you've done the creation of sum.jffs2 images is
broken on collie.
The error message is described clearly in the patch:

sumtool: option '--pad' doesn't allow an argument

Why that? Because collie needs to customize the padding.
EXTRA_IMAGECMD_jffs2 = "--pad=14680064 -l -e ${JFFS2_ERASEBLOCK}"

This --pad=14680064 is then passed to IMAGE_CMD_sum.jffs2 and we get
build error.

That's why we need to redefine both IMAGE_CMD_sum.jffs2 and
EXTRA_IMAGECMD_sum.jffs2 so to pass --pad=XY only to mkfs.jffs2 and
not to the sumtool part of IMAGE_CMD_sum.jffs2.

---
By the way this sum.jffs2 imagetype is used only in meta-handheld afaik.
It would be also ok if you'd remove IMAGE_CMD_sum.jffs2 so we can
append the whole stuff in a customized EXTRA_IMAGECMD_jffs2.

Interestingly the sum.jffs2 ican not be padded to a desired size and
that could cause some issues flashing over older jffs partitions
(happened on collie flashing on top of previous jffs format).

In the case of modern devices with NAND there is a clear benefit.

Regards

Andrea

> We need to make this clearer and I don't think this patch does that, I'm
> not even sure it works...
>
> Cheers,
>
> Richard
>


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 3/5] image_types.bbclass: fix EXTRA_IMAGECMD_sum.jffs2
  2013-10-29 11:17     ` Andrea Adami
@ 2013-10-29 11:45       ` Richard Purdie
       [not found]         ` <CAAQYJAs91R1qH8nvTGG_rSUg_JYcZq-sZgX23tHteyqWfBF4iQ@mail.gmail.com>
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Purdie @ 2013-10-29 11:45 UTC (permalink / raw)
  To: Andrea Adami; +Cc: openembedded-core

On Tue, 2013-10-29 at 12:17 +0100, Andrea Adami wrote:
> On Tue, Oct 29, 2013 at 12:01 PM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > On Mon, 2013-10-21 at 00:34 +0200, Andrea Adami wrote:
> >> When overriding EXTRA_IMAGE_CMD_jffs2 = "--pad=foo ..."
> >> we are passing a malformed option to sumtool:
> >>
> >> sumtool: option '--pad' doesn't allow an argument
> >>
> >> Fix this by declaring a separate variable for the purpose.
> >>
> >> Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
> >> ---
> >>  meta/classes/image_types.bbclass | 5 +++--
> >>  1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
> >> index b8779e0..21391e8 100644
> >> --- a/meta/classes/image_types.bbclass
> >> +++ b/meta/classes/image_types.bbclass
> >> @@ -141,8 +141,8 @@ XZ_INTEGRITY_CHECK ?= "crc32"
> >>  XZ_THREADS ?= "-T 0"
> >>
> >>  IMAGE_CMD_jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 ${EXTRA_IMAGECMD}"
> >> -IMAGE_CMD_sum.jffs2 = "${IMAGE_CMD_jffs2} && sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 \
> >> -     -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
> >> +IMAGE_CMD_sum.jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 ${EXTRA_IMAGECMD_jffs2} \
> >> +     && sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
> >>
> >>  IMAGE_CMD_cramfs = "mkfs.cramfs ${IMAGE_ROOTFS} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.cramfs ${EXTRA_IMAGECMD}"
> >>
> >> @@ -212,6 +212,7 @@ inherit siteinfo
> >>  JFFS2_ENDIANNESS ?= "${@base_conditional('SITEINFO_ENDIANNESS', 'le', '-l', '-b', d)}"
> >>  JFFS2_ERASEBLOCK ?= "0x40000"
> >>  EXTRA_IMAGECMD_jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} --no-cleanmarkers"
> >> +EXTRA_IMAGECMD_sum.jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} --no-cleanmarkers"
> >>
> >>  # Change these if you want default mkfs behavior (i.e. create minimal inode number)
> >>  EXTRA_IMAGECMD_ext2 ?= "-i 8192"
> >
> > This patch is very confused. You say sumtool doesn't take a --pad
> > option, yet "EXTRA_IMAGECMD_sum.jffs" which is presumably used with
> > sumtool does have the option.
> >
> 
> After the commits you've done the creation of sum.jffs2 images is
> broken on collie.
> The error message is described clearly in the patch:
> 
> sumtool: option '--pad' doesn't allow an argument
> 
> Why that? Because collie needs to customize the padding.
> EXTRA_IMAGECMD_jffs2 = "--pad=14680064 -l -e ${JFFS2_ERASEBLOCK}"
> 
> This --pad=14680064 is then passed to IMAGE_CMD_sum.jffs2 and we get
> build error.
> 
> That's why we need to redefine both IMAGE_CMD_sum.jffs2 and
> EXTRA_IMAGECMD_sum.jffs2 so to pass --pad=XY only to mkfs.jffs2 and
> not to the sumtool part of IMAGE_CMD_sum.jffs2.
> 
> ---
> By the way this sum.jffs2 imagetype is used only in meta-handheld afaik.
> It would be also ok if you'd remove IMAGE_CMD_sum.jffs2 so we can
> append the whole stuff in a customized EXTRA_IMAGECMD_jffs2.

I understand your problem however:

a) Copy and pasting the entire IMAGE_CMD_jffs2 command into the second
one is rather ugly.
b) You still end up passing "--pad ${JFFS2_ENDIANNESS} --eraseblock=
${JFFS2_ERASEBLOCK} --no-cleanmarkers" as parameters to sumtool,
including a --pad option with no argument. Why/how?

You call:

sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}

and EXTRA_IMAGECMD is set to EXTRA_IMAGECMD_sum.jffs2 due to the use of
overrides in the image generation code.

So whilst collie is broken, I do not think this patch makes readable
code and I think we need to find something better.

The thing I'm confused on is whether there should be any --pad option to
sumtool or not.

Cheers,

Richard





^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 3/5] image_types.bbclass: fix EXTRA_IMAGECMD_sum.jffs2
       [not found]         ` <CAAQYJAs91R1qH8nvTGG_rSUg_JYcZq-sZgX23tHteyqWfBF4iQ@mail.gmail.com>
@ 2013-10-29 15:33           ` Richard Purdie
  2013-10-30 21:38             ` Andrea Adami
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Purdie @ 2013-10-29 15:33 UTC (permalink / raw)
  To: Andrea Adami; +Cc: openembedded-core

On Tue, 2013-10-29 at 13:17 +0100, Andrea Adami wrote:
> On Tue, Oct 29, 2013 at 12:45 PM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > On Tue, 2013-10-29 at 12:17 +0100, Andrea Adami wrote:
> >> On Tue, Oct 29, 2013 at 12:01 PM, Richard Purdie
> >> <richard.purdie@linuxfoundation.org> wrote:
> >> > On Mon, 2013-10-21 at 00:34 +0200, Andrea Adami wrote:
> >> >> When overriding EXTRA_IMAGE_CMD_jffs2 = "--pad=foo ..."
> >> >> we are passing a malformed option to sumtool:
> >> >>
> >> >> sumtool: option '--pad' doesn't allow an argument
> >> >>
> >> >> Fix this by declaring a separate variable for the purpose.
> >> >>
> >> >> Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
> >> >> ---
> >> >>  meta/classes/image_types.bbclass | 5 +++--
> >> >>  1 file changed, 3 insertions(+), 2 deletions(-)
> >> >>
> >> >> diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
> >> >> index b8779e0..21391e8 100644
> >> >> --- a/meta/classes/image_types.bbclass
> >> >> +++ b/meta/classes/image_types.bbclass
> >> >> @@ -141,8 +141,8 @@ XZ_INTEGRITY_CHECK ?= "crc32"
> >> >>  XZ_THREADS ?= "-T 0"
> >> >>
> >> >>  IMAGE_CMD_jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 ${EXTRA_IMAGECMD}"
> >> >> -IMAGE_CMD_sum.jffs2 = "${IMAGE_CMD_jffs2} && sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 \
> >> >> -     -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
> >> >> +IMAGE_CMD_sum.jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 ${EXTRA_IMAGECMD_jffs2} \
> >> >> +     && sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
> >> >>
> >> >>  IMAGE_CMD_cramfs = "mkfs.cramfs ${IMAGE_ROOTFS} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.cramfs ${EXTRA_IMAGECMD}"
> >> >>
> >> >> @@ -212,6 +212,7 @@ inherit siteinfo
> >> >>  JFFS2_ENDIANNESS ?= "${@base_conditional('SITEINFO_ENDIANNESS', 'le', '-l', '-b', d)}"
> >> >>  JFFS2_ERASEBLOCK ?= "0x40000"
> >> >>  EXTRA_IMAGECMD_jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} --no-cleanmarkers"
> >> >> +EXTRA_IMAGECMD_sum.jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} --no-cleanmarkers"
> >> >>
> >> >>  # Change these if you want default mkfs behavior (i.e. create minimal inode number)
> >> >>  EXTRA_IMAGECMD_ext2 ?= "-i 8192"
> >> >
> >> > This patch is very confused. You say sumtool doesn't take a --pad
> >> > option, yet "EXTRA_IMAGECMD_sum.jffs" which is presumably used with
> >> > sumtool does have the option.
> >> >
> >>
> >> After the commits you've done the creation of sum.jffs2 images is
> >> broken on collie.
> >> The error message is described clearly in the patch:
> >>
> >> sumtool: option '--pad' doesn't allow an argument
> >>
> >> Why that? Because collie needs to customize the padding.
> >> EXTRA_IMAGECMD_jffs2 = "--pad=14680064 -l -e ${JFFS2_ERASEBLOCK}"
> >>
> >> This --pad=14680064 is then passed to IMAGE_CMD_sum.jffs2 and we get
> >> build error.
> >>
> >> That's why we need to redefine both IMAGE_CMD_sum.jffs2 and
> >> EXTRA_IMAGECMD_sum.jffs2 so to pass --pad=XY only to mkfs.jffs2 and
> >> not to the sumtool part of IMAGE_CMD_sum.jffs2.
> >>
> >> ---
> >> By the way this sum.jffs2 imagetype is used only in meta-handheld afaik.
> >> It would be also ok if you'd remove IMAGE_CMD_sum.jffs2 so we can
> >> append the whole stuff in a customized EXTRA_IMAGECMD_jffs2.
> >
> > I understand your problem however:
> >
> > a) Copy and pasting the entire IMAGE_CMD_jffs2 command into the second
> > one is rather ugly.
> Yes, but using the IMAGE_CMD_jffs2 we incorporate the
> EXTRA_IMAGECMD_jffs2 = "--pad=xyz.."
> 
> > b) You still end up passing "--pad ${JFFS2_ENDIANNESS} --eraseblock=
> > ${JFFS2_ERASEBLOCK} --no-cleanmarkers" as parameters to sumtool,
> > including a --pad option with no argument. Why/how?
> 
> Because this padding is limited to the last eraseblock.
> " -p, --pad                 Pad the OUTPUT with 0xFF to the end of the
> final eraseblock"
> 
> >
> > You call:
> >
> > sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}
> >
> > and EXTRA_IMAGECMD is set to EXTRA_IMAGECMD_sum.jffs2 due to the use of
> > overrides in the image generation code.
> 
> We cannot reuse IMAGE_CMD_jffs2 because in that case
> EXTRA_IMAGECMD_jffs2 is already evaluated and this contains the
> infamous --pad==xy.
> 
> >
> > So whilst collie is broken, I do not think this patch makes readable
> > code and I think we need to find something better.
> >
> > The thing I'm confused on is whether there should be any --pad option to
> > sumtool or not.
> >
> 
> Yes, but without <size> argument
> 
> http://git.infradead.org/mtd-utils.git/blob/HEAD:/sumtool.c

Ok, we need to make this clearer in the commit message. Looking and
thinking about this further, how about we add some dependency ordering
into this code, something like the patch below. This moves the jffs2
call into its own true namespace. You should then just be able to
explicitly set EXTRA_IMGAGECMD_sum.jffs2 to the value you need (yet
still have EXTRA_IMGAGECMD_jffs2 able to take a different value).

See if you can build something on top of the patch below...

Cheers,

Richard




From 4ff883824221ce5f1b993239249b1ce1e3ab9b32 Mon Sep 17 00:00:00 2001
From: Richard Purdie <richard.purdie@linuxfoundation.org>
Date: Tue, 29 Oct 2013 15:26:54 +0000
Subject: image_types: Improve dependency handling between types (and use for sum.jffs2)

We're seeing a pattern of one image type needing to depend on another
type. A good example is jffs2 and sum.jffs2. This patch makes sum.jffs2
depend on jffs2 which will then allow a EXTRA_IMGAGECMD to be set for
sum.jffs2 individually without changing the jffs2 command. This allows the
-pad option to be configured differently.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
index b8779e0..5ce1ddb 100644
--- a/meta/classes/image_types.bbclass
+++ b/meta/classes/image_types.bbclass
@@ -7,14 +7,20 @@ def get_imagecmds(d):
     ctypes = d.getVar('COMPRESSIONTYPES', True).split()
     cimages = {}
 
+    # Image type b depends on a having been generated first
+    def addtypedepends(a, b):
+        if a in alltypes:
+            alltypes.remove(a):
+            if b not in alltypes:
+                alltypes.append(b)
+            alltypes.append(a)
+
     # The elf image depends on the cpio.gz image already having
     # been created, so we add that explicit ordering here.
+    addtypedepends("elf", "cpio.gz")
 
-    if "elf" in alltypes:
-        alltypes.remove("elf")
-        if "cpio.gz" not in alltypes:
-                alltypes.append("cpio.gz")
-        alltypes.append("elf")
+    # jffs2 sumtool'd images need jffs2
+    addtypedepends("sum.jffs2", "jffs2")
 
     # Filter out all the compressed images from alltypes
     for type in alltypes:
@@ -141,8 +147,7 @@ XZ_INTEGRITY_CHECK ?= "crc32"
 XZ_THREADS ?= "-T 0"
 
 IMAGE_CMD_jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 ${EXTRA_IMAGECMD}"
-IMAGE_CMD_sum.jffs2 = "${IMAGE_CMD_jffs2} && sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 \
-	-o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
+IMAGE_CMD_sum.jffs2 = "sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
 
 IMAGE_CMD_cramfs = "mkfs.cramfs ${IMAGE_ROOTFS} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.cramfs ${EXTRA_IMAGECMD}"
 





^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [PATCH 3/5] image_types.bbclass: fix EXTRA_IMAGECMD_sum.jffs2
  2013-10-29 15:33           ` Richard Purdie
@ 2013-10-30 21:38             ` Andrea Adami
  0 siblings, 0 replies; 12+ messages in thread
From: Andrea Adami @ 2013-10-30 21:38 UTC (permalink / raw)
  To: Richard Purdie; +Cc: openembedded-core

On Tue, Oct 29, 2013 at 4:33 PM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Tue, 2013-10-29 at 13:17 +0100, Andrea Adami wrote:
>> On Tue, Oct 29, 2013 at 12:45 PM, Richard Purdie
>> <richard.purdie@linuxfoundation.org> wrote:
>> > On Tue, 2013-10-29 at 12:17 +0100, Andrea Adami wrote:
>> >> On Tue, Oct 29, 2013 at 12:01 PM, Richard Purdie
>> >> <richard.purdie@linuxfoundation.org> wrote:
>> >> > On Mon, 2013-10-21 at 00:34 +0200, Andrea Adami wrote:
>> >> >> When overriding EXTRA_IMAGE_CMD_jffs2 = "--pad=foo ..."
>> >> >> we are passing a malformed option to sumtool:
>> >> >>
>> >> >> sumtool: option '--pad' doesn't allow an argument
>> >> >>
>> >> >> Fix this by declaring a separate variable for the purpose.
>> >> >>
>> >> >> Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
>> >> >> ---
>> >> >>  meta/classes/image_types.bbclass | 5 +++--
>> >> >>  1 file changed, 3 insertions(+), 2 deletions(-)
>> >> >>
>> >> >> diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
>> >> >> index b8779e0..21391e8 100644
>> >> >> --- a/meta/classes/image_types.bbclass
>> >> >> +++ b/meta/classes/image_types.bbclass
>> >> >> @@ -141,8 +141,8 @@ XZ_INTEGRITY_CHECK ?= "crc32"
>> >> >>  XZ_THREADS ?= "-T 0"
>> >> >>
>> >> >>  IMAGE_CMD_jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 ${EXTRA_IMAGECMD}"
>> >> >> -IMAGE_CMD_sum.jffs2 = "${IMAGE_CMD_jffs2} && sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 \
>> >> >> -     -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
>> >> >> +IMAGE_CMD_sum.jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 ${EXTRA_IMAGECMD_jffs2} \
>> >> >> +     && sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
>> >> >>
>> >> >>  IMAGE_CMD_cramfs = "mkfs.cramfs ${IMAGE_ROOTFS} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.cramfs ${EXTRA_IMAGECMD}"
>> >> >>
>> >> >> @@ -212,6 +212,7 @@ inherit siteinfo
>> >> >>  JFFS2_ENDIANNESS ?= "${@base_conditional('SITEINFO_ENDIANNESS', 'le', '-l', '-b', d)}"
>> >> >>  JFFS2_ERASEBLOCK ?= "0x40000"
>> >> >>  EXTRA_IMAGECMD_jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} --no-cleanmarkers"
>> >> >> +EXTRA_IMAGECMD_sum.jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} --no-cleanmarkers"
>> >> >>
>> >> >>  # Change these if you want default mkfs behavior (i.e. create minimal inode number)
>> >> >>  EXTRA_IMAGECMD_ext2 ?= "-i 8192"
>> >> >
>> >> > This patch is very confused. You say sumtool doesn't take a --pad
>> >> > option, yet "EXTRA_IMAGECMD_sum.jffs" which is presumably used with
>> >> > sumtool does have the option.
>> >> >
>> >>
>> >> After the commits you've done the creation of sum.jffs2 images is
>> >> broken on collie.
>> >> The error message is described clearly in the patch:
>> >>
>> >> sumtool: option '--pad' doesn't allow an argument
>> >>
>> >> Why that? Because collie needs to customize the padding.
>> >> EXTRA_IMAGECMD_jffs2 = "--pad=14680064 -l -e ${JFFS2_ERASEBLOCK}"
>> >>
>> >> This --pad=14680064 is then passed to IMAGE_CMD_sum.jffs2 and we get
>> >> build error.
>> >>
>> >> That's why we need to redefine both IMAGE_CMD_sum.jffs2 and
>> >> EXTRA_IMAGECMD_sum.jffs2 so to pass --pad=XY only to mkfs.jffs2 and
>> >> not to the sumtool part of IMAGE_CMD_sum.jffs2.
>> >>
>> >> ---
>> >> By the way this sum.jffs2 imagetype is used only in meta-handheld afaik.
>> >> It would be also ok if you'd remove IMAGE_CMD_sum.jffs2 so we can
>> >> append the whole stuff in a customized EXTRA_IMAGECMD_jffs2.
>> >
>> > I understand your problem however:
>> >
>> > a) Copy and pasting the entire IMAGE_CMD_jffs2 command into the second
>> > one is rather ugly.
>> Yes, but using the IMAGE_CMD_jffs2 we incorporate the
>> EXTRA_IMAGECMD_jffs2 = "--pad=xyz.."
>>
>> > b) You still end up passing "--pad ${JFFS2_ENDIANNESS} --eraseblock=
>> > ${JFFS2_ERASEBLOCK} --no-cleanmarkers" as parameters to sumtool,
>> > including a --pad option with no argument. Why/how?
>>
>> Because this padding is limited to the last eraseblock.
>> " -p, --pad                 Pad the OUTPUT with 0xFF to the end of the
>> final eraseblock"
>>
>> >
>> > You call:
>> >
>> > sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}
>> >
>> > and EXTRA_IMAGECMD is set to EXTRA_IMAGECMD_sum.jffs2 due to the use of
>> > overrides in the image generation code.
>>
>> We cannot reuse IMAGE_CMD_jffs2 because in that case
>> EXTRA_IMAGECMD_jffs2 is already evaluated and this contains the
>> infamous --pad==xy.
>>
>> >
>> > So whilst collie is broken, I do not think this patch makes readable
>> > code and I think we need to find something better.
>> >
>> > The thing I'm confused on is whether there should be any --pad option to
>> > sumtool or not.
>> >
>>
>> Yes, but without <size> argument
>>
>> http://git.infradead.org/mtd-utils.git/blob/HEAD:/sumtool.c
>
> Ok, we need to make this clearer in the commit message. Looking and
> thinking about this further, how about we add some dependency ordering
> into this code, something like the patch below. This moves the jffs2
> call into its own true namespace. You should then just be able to
> explicitly set EXTRA_IMGAGECMD_sum.jffs2 to the value you need (yet
> still have EXTRA_IMGAGECMD_jffs2 able to take a different value).
>
> See if you can build something on top of the patch below...
>
> Cheers,
>
> Richard
>
>
>
>
> From 4ff883824221ce5f1b993239249b1ce1e3ab9b32 Mon Sep 17 00:00:00 2001
> From: Richard Purdie <richard.purdie@linuxfoundation.org>
> Date: Tue, 29 Oct 2013 15:26:54 +0000
> Subject: image_types: Improve dependency handling between types (and use for sum.jffs2)
>
> We're seeing a pattern of one image type needing to depend on another
> type. A good example is jffs2 and sum.jffs2. This patch makes sum.jffs2
> depend on jffs2 which will then allow a EXTRA_IMGAGECMD to be set for
> sum.jffs2 individually without changing the jffs2 command. This allows the
> -pad option to be configured differently.
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
> diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
> index b8779e0..5ce1ddb 100644
> --- a/meta/classes/image_types.bbclass
> +++ b/meta/classes/image_types.bbclass
> @@ -7,14 +7,20 @@ def get_imagecmds(d):
>      ctypes = d.getVar('COMPRESSIONTYPES', True).split()
>      cimages = {}
>
> +    # Image type b depends on a having been generated first
> +    def addtypedepends(a, b):
> +        if a in alltypes:
> +            alltypes.remove(a):
> +            if b not in alltypes:
> +                alltypes.append(b)
> +            alltypes.append(a)
> +
>      # The elf image depends on the cpio.gz image already having
>      # been created, so we add that explicit ordering here.
> +    addtypedepends("elf", "cpio.gz")
>
> -    if "elf" in alltypes:
> -        alltypes.remove("elf")
> -        if "cpio.gz" not in alltypes:
> -                alltypes.append("cpio.gz")
> -        alltypes.append("elf")
> +    # jffs2 sumtool'd images need jffs2
> +    addtypedepends("sum.jffs2", "jffs2")
>
>      # Filter out all the compressed images from alltypes
>      for type in alltypes:
> @@ -141,8 +147,7 @@ XZ_INTEGRITY_CHECK ?= "crc32"
>  XZ_THREADS ?= "-T 0"
>
>  IMAGE_CMD_jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 ${EXTRA_IMAGECMD}"
> -IMAGE_CMD_sum.jffs2 = "${IMAGE_CMD_jffs2} && sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 \
> -       -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
> +IMAGE_CMD_sum.jffs2 = "sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
>
>  IMAGE_CMD_cramfs = "mkfs.cramfs ${IMAGE_ROOTFS} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.cramfs ${EXTRA_IMAGECMD}"
>
>
>
>

Richard,

your proposed solution does work as expected and I'm able to set in
the specific case (collie.conf):

EXTRA_IMAGECMD_jffs2 = "--pad=14680064 -l -e ${JFFS2_ERASEBLOCK}"
EXTRA_IMAGECMD_sum.jffs2 = "-p -l -e ${JFFS2_ERASEBLOCK}"

The run.do_rootfs shows the correct commands are used:

mkfs.jffs2 --root=/oe/oe-core/build/tmp-eglibc/work/collie-oe-linux-gnueabi/core-image-base/1.0-r0/rootfs
--faketime --output=/oe/oe-core/build/tmp-eglibc/deploy/images/collie/core-image-base-collie-20131030212928.rootfs.jffs2
--pad=14680064 -l -e 0x20000

sumtool -i /oe/oe-core/build/tmp-eglibc/deploy/images/collie/core-image-base-collie-20131030212928.rootfs.jffs2
-o /oe/oe-core/build/tmp-eglibc/deploy/images/collie/core-image-base-collie-20131030212928.rootfs.sum.jffs2
-p -l -e 0x20000

Thanks

Andrea

Acked-by: Andrea Adami <andrea.adami@gmail.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-10-30 21:38 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-20 22:34 [PATCH 0/5] JFFS2 images: bugfixes and enhancements Andrea Adami
2013-10-20 22:34 ` [PATCH 1/5] image_types.bbclass: do not force --no-cleanmarkers for jffs2 Andrea Adami
2013-10-20 22:34 ` [PATCH 2/5] image_types.bbclass: fix endiannes for sumtool (jffs2 summary) Andrea Adami
2013-10-20 22:34 ` [PATCH 3/5] image_types.bbclass: fix EXTRA_IMAGECMD_sum.jffs2 Andrea Adami
2013-10-29 10:26   ` Andrea Adami
2013-10-29 11:01   ` Richard Purdie
2013-10-29 11:17     ` Andrea Adami
2013-10-29 11:45       ` Richard Purdie
     [not found]         ` <CAAQYJAs91R1qH8nvTGG_rSUg_JYcZq-sZgX23tHteyqWfBF4iQ@mail.gmail.com>
2013-10-29 15:33           ` Richard Purdie
2013-10-30 21:38             ` Andrea Adami
2013-10-20 22:34 ` [PATCH 4/5] image_types.bbclass: add JFFS2_NOCLEANMARKERS variable Andrea Adami
2013-10-20 22:34 ` [PATCH 5/5] image_types.bbclass: add JFFS2_PADDING variable Andrea Adami

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.