JFFS2 images: bugfixes and enhancements
cover JFFS2 images: bugfixes and enhancements
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(-)
Signature · 1 line
1.8.1.5
PATCH 1/5 image_types.bbclass: do not force --no-cleanmarkers for jffs2
--- meta/classes/image_types.bbclass | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
meta/classes/image_types.bbclass +2 -2
@@ -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}"
Signature · 1 line
1.8.1.5
PATCH 2/5 image_types.bbclass: fix endiannes for sumtool (jffs2 summary)
In reply to Andrea AdamiFor 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.
--- meta/classes/image_types.bbclass | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
meta/classes/image_types.bbclass +1 -1
@@ -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"
Signature · 1 line
1.8.1.5
PATCH 3/5 image_types.bbclass: fix EXTRA_IMAGECMD_sum.jffs2
In reply to Andrea AdamiWhen 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.
--- meta/classes/image_types.bbclass | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
meta/classes/image_types.bbclass +3 -2
@@ -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"
Signature · 1 line
1.8.1.5
Re: [PATCH 3/5] image_types.bbclass: fix EXTRA_IMAGECMD_sum.jffs2
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
Quoted 38 lines · Andrea Adami
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 <[email protected]>--- meta/classes/image_types.bbclass | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)meta/classes/image_types.bbclass +3
-3@@ -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
Re: [PATCH 3/5] image_types.bbclass: fix EXTRA_IMAGECMD_sum.jffs2
In reply to Andrea AdamiQuoted 28 lines · Andrea Adami
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 <[email protected]>--- meta/classes/image_types.bbclass | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)meta/classes/image_types.bbclass +2
-2@@ -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}"
meta/classes/image_types.bbclass inherit siteinfo · line 218
Quoted 7 lines
meta/classes/image_types.bbclass +1
-0@@ -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
Re: [PATCH 3/5] image_types.bbclass: fix EXTRA_IMAGECMD_sum.jffs2
Quoted 34 lines · Richard Purdie
Quoted 33 lines · Andrea Adami
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 <[email protected]>--- meta/classes/image_types.bbclass | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)meta/classes/image_types.bbclass +3
-2@@ -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"
Quoted 7 lines
meta/classes/image_types.bbclass inherit siteinfo · line 218
Quoted 2 lines
meta/classes/image_types.bbclass +0
-0# 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
Quoted 7 lines
We need to make this clearer and I don't think this patch does that, I'm not even sure it works...
Cheers,
Richard
Re: [PATCH 3/5] image_types.bbclass: fix EXTRA_IMAGECMD_sum.jffs2
Quoted 56 lines · Andrea Adami
Quoted 34 lines · Richard Purdie
Quoted 33 lines · Andrea Adami
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 <[email protected]>--- meta/classes/image_types.bbclass | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)meta/classes/image_types.bbclass +3
-2@@ -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"Quoted 7 lines
meta/classes/image_types.bbclass inherit siteinfo · line 218
Quoted 2 lines
meta/classes/image_types.bbclass +0
-0# 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.
Quoted 8 lines
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 customizedEXTRA_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
A message that is not in the archive (sent off-list or not yet received): CAAQYJAs91R1qH8nvTGG_rSUg_JYcZq-sZgX23tHteyqWfBF4iQ@mail.gmail.com
Re: [PATCH 3/5] image_types.bbclass: fix EXTRA_IMAGECMD_sum.jffs2
Quoted 99 lines · Andrea Adami
Quoted 66 lines · Richard Purdie
Quoted 56 lines · Andrea Adami
Quoted 34 lines · Richard Purdie
Quoted 33 lines · Andrea Adami
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 <[email protected]>--- meta/classes/image_types.bbclass | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)meta/classes/image_types.bbclass +3
-2@@ -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"Quoted 7 lines
meta/classes/image_types.bbclass inherit siteinfo · line 218
Quoted 2 lines
meta/classes/image_types.bbclass +0
-0# 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.
Quoted 8 lines
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 customizedEXTRA_IMAGECMD_jffs2.Quoted 4 lines
I understand your problem however:
a) Copy and pasting the entire
IMAGE_CMD_jffs2command into the second one is rather ugly.Yes, but using the
IMAGE_CMD_jffs2we incorporate theEXTRA_IMAGECMD_jffs2= "--pad=xyz.."Quoted 3 lines
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 with0xFFto the end of the
final eraseblock"Quoted 7 lines
You call:
sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}
and
EXTRA_IMAGECMDis set to EXTRA_IMAGECMD_sum.jffs2 due to the use of overrides in the image generation code.We cannot reuse
IMAGE_CMD_jffs2because in that caseEXTRA_IMAGECMD_jffs2is already evaluated and this contains the infamous --pad==xy.Quoted 4 lines
So whilst collie is broken, I do not think this patch makes readable code and I think we need to find something better.
Quoted 7 lines
Quoted 3 lines
The thing I'm confused on is whether there should be any --pad option to sumtool or not.
Yes, but without <size> argument
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 <[email protected]>
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.
---
meta/classes/image_types.bbclass +12 -7
@@ -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}"
Andrea Adami
Acked-byRe: [PATCH 3/5] image_types.bbclass: fix EXTRA_IMAGECMD_sum.jffs2
Quoted 171 lines · Richard Purdie
Quoted 99 lines · Andrea Adami
Quoted 66 lines · Richard Purdie
Quoted 56 lines · Andrea Adami
Quoted 34 lines · Richard Purdie
Quoted 33 lines · Andrea Adami
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 <[email protected]>--- meta/classes/image_types.bbclass | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)meta/classes/image_types.bbclass +3
-2@@ -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"Quoted 7 lines
meta/classes/image_types.bbclass inherit siteinfo · line 218
Quoted 2 lines
meta/classes/image_types.bbclass +0
-0# 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.
Quoted 8 lines
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 customizedEXTRA_IMAGECMD_jffs2.Quoted 4 lines
I understand your problem however:
a) Copy and pasting the entire
IMAGE_CMD_jffs2command into the second one is rather ugly.Yes, but using the
IMAGE_CMD_jffs2we incorporate theEXTRA_IMAGECMD_jffs2= "--pad=xyz.."Quoted 3 lines
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 with0xFFto the end of the
final eraseblock"Quoted 7 lines
You call:
sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}
and
EXTRA_IMAGECMDis set to EXTRA_IMAGECMD_sum.jffs2 due to the use of overrides in the image generation code.We cannot reuse
IMAGE_CMD_jffs2because in that caseEXTRA_IMAGECMD_jffs2is already evaluated and this contains the infamous --pad==xy.Quoted 4 lines
So whilst collie is broken, I do not think this patch makes readable code and I think we need to find something better.
Quoted 7 lines
Quoted 3 lines
The thing I'm confused on is whether there should be any --pad option to sumtool or not.
Yes, but without <size> argument
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_jffs2able to take a different value).See if you can build something on top of the patch below...
Cheers,
Richard
From
4ff883824221ce5f1b993239249b1ce1e3ab9b32Mon Sep 17 00:00:00 2001
From: Richard Purdie <[email protected]>
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_IMGAGECMDto 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 <[email protected]>---
meta/classes/image_types.bbclass +11
-5@@ -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:
meta/classes/image_types.bbclass XZ_INTEGRITY_CHECK ?= "crc32" · line 153
Quoted 9 lines
meta/classes/image_types.bbclass +1
-2@@ -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
PATCH 4/5 image_types.bbclass: add JFFS2_NOCLEANMARKERS variable
In reply to Andrea AdamiThe '--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.
--- meta/classes/image_types.bbclass | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
meta/classes/image_types.bbclass +3 -2
@@ -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"
Signature · 1 line
1.8.1.5
PATCH 5/5 image_types.bbclass: add JFFS2_PADDING variable
In reply to Andrea AdamiDefaulting to '--pad', meaning to pad until the end of the erasesector.
Allow machine.conf to override the value without redeclaring EXTRA_IMAGECMD_jffs2.
--- meta/classes/image_types.bbclass | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
meta/classes/image_types.bbclass +2 -1
@@ -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)
Signature · 1 line
1.8.1.5