* [U-Boot] [RFC][PATCH] mkimage: Add compatibility option for legacy Multi-File images
@ 2010-08-29 13:05 Per Andersson
0 siblings, 0 replies; 6+ messages in thread
From: Per Andersson @ 2010-08-29 13:05 UTC (permalink / raw)
To: u-boot
Hi!
Do you want to apply the patch that Thibaut Girka posted earlier in mainline
U-Boot? [0]
We need it for Debian Installer when we make images for the Neo FreeRunner. The
patch is useful for use with devices that use broken U-Boot versions.
[0] http://lists.denx.de/pipermail/u-boot/2010-August/075823.html
Best,
Per
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RFC][PATCH] mkimage: Add compatibility option for legacy Multi-File images
2010-08-18 20:48 Thibaut Girka
2010-08-30 9:29 ` Detlev Zundel
@ 2010-09-02 15:05 ` Wolfgang Denk
1 sibling, 0 replies; 6+ messages in thread
From: Wolfgang Denk @ 2010-09-02 15:05 UTC (permalink / raw)
To: u-boot
Dear Thibaut Girka,
In message <1282164515-28846-1-git-send-email-thib@sitedethib.com> you wrote:
> During a few months, offsets of files in multi-file images were miscalculated,
> this has been fixed by 02b9b22446e3d7ad6a6382be17a1ce79a7de589b,
> but unfortunatly, some devices (I'm thinking of the Neo FreeRunner) are using
> a broken version of U-Boot.
>
> This problem can easily be worked around at image creation time by adding one
> byte of junk at the end of the first (and optionnally second) file, if its
> size is a multiple of 4.
> It's not really clean, but it shouldn't cause any problem. At least, I haven't
> encountered any using this patch.
In addition to Detlev's comments a few questions:
- Is my understanding correct that there is no problem with the
current mainline code?
- You mentioned that commit 02b9b22 (i. e. v1.3.3-rc3-46-g02b9b22)
fixed the problem - can you please also state which commit
introduced the problem?
- If so, would it then not be better to update U-Boot on the devices
that are running a broken version?
As this patch is really adding a quirk for a specific version (or set
of versions) of U-Boot only, but without general use for the current
mainline code (on contrary, it's making the code more complex without
benefit to most of us), I tend to reject this patch for mainline.
If there has been one or more released versions with this problem
present, we might consider a maintenance release for these, where we add
the commit as a workaround.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RFC][PATCH] mkimage: Add compatibility option for legacy Multi-File images
2010-08-31 6:50 ` Thibaut Girka
@ 2010-09-01 15:04 ` Detlev Zundel
0 siblings, 0 replies; 6+ messages in thread
From: Detlev Zundel @ 2010-09-01 15:04 UTC (permalink / raw)
To: u-boot
Hi Thibaut,
> Le lundi 30 ao?t 2010 ? 11:29 +0200, Detlev Zundel a ?crit :
>> Hi Thibaut,
> Hi,
>
>> generally I'm not a fan to include workarounds for bugs which we do not
>> have anymore in mainline U-Boot.
>
> Hm, yeah, I can understand that...
>
>> Isn't there any other alternative for this?
>
> Well, for my use case, we have to workaround this bug.
> We can do that (adding a byte at the end of some files) outside of
> mkimage, but it's really a u-boot thing, so, it could really go in
> there.
If it goes in, can we set a date to remove it again? Or do we have to
carry it around forever? If the latter is the case then I'm really not
a big fan of it. If you can fix the problem up in the "project where it
is a problem", then the fix would better belong there.
Cheers
Detlev
--
The mathematician's patterns, like the painter's or the poet's, must be
beautiful; the ideas, like the colours or the words, must fit together in a
harmonious way. Beauty is the first test: there is no permanent place in the
world for ugly mathematics. -- G. H. Hardy
--
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RFC][PATCH] mkimage: Add compatibility option for legacy Multi-File images
2010-08-30 9:29 ` Detlev Zundel
@ 2010-08-31 6:50 ` Thibaut Girka
2010-09-01 15:04 ` Detlev Zundel
0 siblings, 1 reply; 6+ messages in thread
From: Thibaut Girka @ 2010-08-31 6:50 UTC (permalink / raw)
To: u-boot
Le lundi 30 ao?t 2010 ? 11:29 +0200, Detlev Zundel a ?crit :
> Hi Thibaut,
Hi,
> generally I'm not a fan to include workarounds for bugs which we do not
> have anymore in mainline U-Boot.
Hm, yeah, I can understand that...
> Isn't there any other alternative for this?
Well, for my use case, we have to workaround this bug.
We can do that (adding a byte at the end of some files) outside of
mkimage, but it's really a u-boot thing, so, it could really go in
there.
> If nobody objects to the genereal principle, then I have some requests
> below.
[...]
> Hm, as I read it, you add 4 bytes (not one) in case the image is already
> padded correctly to 32-bit, correct? If so, then please correct the
> comment.
Yes, you add one byte to the end of the file itself, and it'll add 4
bytes to the resulting image file.
> > It's not really clean, but it shouldn't cause any problem. At least, I haven't
> > encountered any using this patch.
[...]
> > @@ -586,6 +592,7 @@ usage ()
> > " -e ==> set entry point to 'ep' (hex)\n"
> > " -n ==> set image name to 'name'\n"
> > " -d ==> use image data from 'datafile'\n"
> > + " -p ==> force padding in multi-file images\n"
>
> This is no real padding, so please don't make it look like it is. Maybe
> use "q"(uirk) as an option character and change the description to
> indicate that this is not a "forced padding" but a "incorrect additional
> 32-bit padding to work around an old bug (see man-page)". A fix to
> "doc/mkimage.1" which now exists is also mandatory.
Yeah, indeed, it's not really a padding, I'll rephrase the command
option description and document it in mkimage.1.
Regards,
Thibaut Girka.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100831/a33e0a33/attachment.pgp
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RFC][PATCH] mkimage: Add compatibility option for legacy Multi-File images
2010-08-18 20:48 Thibaut Girka
@ 2010-08-30 9:29 ` Detlev Zundel
2010-08-31 6:50 ` Thibaut Girka
2010-09-02 15:05 ` Wolfgang Denk
1 sibling, 1 reply; 6+ messages in thread
From: Detlev Zundel @ 2010-08-30 9:29 UTC (permalink / raw)
To: u-boot
Hi Thibaut,
generally I'm not a fan to include workarounds for bugs which we do not
have anymore in mainline U-Boot. Isn't there any other alternative for
this? What do other people think?
If nobody objects to the genereal principle, then I have some requests
below.
> During a few months, offsets of files in multi-file images were miscalculated,
> this has been fixed by 02b9b22446e3d7ad6a6382be17a1ce79a7de589b,
> but unfortunatly, some devices (I'm thinking of the Neo FreeRunner) are using
> a broken version of U-Boot.
>
> This problem can easily be worked around at image creation time by adding one
> byte of junk at the end of the first (and optionnally second) file, if its
> size is a multiple of 4.
Hm, as I read it, you add 4 bytes (not one) in case the image is already
padded correctly to 32-bit, correct? If so, then please correct the
comment.
> It's not really clean, but it shouldn't cause any problem. At least, I haven't
> encountered any using this patch.
>
> Signed-off-by: Thibaut Girka <thib@sitedethib.com>
> ---
> tools/mkimage.c | 9 ++++++++-
> tools/mkimage.h | 1 +
> 2 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index f5859d7..239c1f0 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -243,6 +243,9 @@ main (int argc, char **argv)
> usage ();
> params.imagename = *++argv;
> goto NXTARG;
> + case 'p':
> + params.pflag++;
> + break;
> case 'v':
> params.vflag++;
> break;
> @@ -383,6 +386,8 @@ NXTARG: ;
> params.cmdname, file, strerror(errno));
> exit (EXIT_FAILURE);
> }
> + if (params.pflag && sep && (sbuf.st_size % 4 == 0))
> + sbuf.st_size += 1;
> size = cpu_to_uimage (sbuf.st_size);
> } else {
> size = 0;
> @@ -556,7 +561,8 @@ copy_file (int ifd, const char *datafile, int pad)
> exit (EXIT_FAILURE);
> }
>
> - if (pad && ((tail = size % 4) != 0)) {
> + tail = size % 4;
> + if (pad && (params.pflag || tail != 0)) {
>
> if (write(ifd, (char *)&zero, 4-tail) != 4-tail) {
> fprintf (stderr, "%s: Write error on %s: %s\n",
> @@ -586,6 +592,7 @@ usage ()
> " -e ==> set entry point to 'ep' (hex)\n"
> " -n ==> set image name to 'name'\n"
> " -d ==> use image data from 'datafile'\n"
> + " -p ==> force padding in multi-file images\n"
This is no real padding, so please don't make it look like it is. Maybe
use "q"(uirk) as an option character and change the description to
indicate that this is not a "forced padding" but a "incorrect additional
32-bit padding to work around an old bug (see man-page)". A fix to
"doc/mkimage.1" which now exists is also mandatory.
> " -x ==> set XIP (execute in place)\n",
> params.cmdname);
> fprintf (stderr, " %s [-D dtc_options] -f fit-image.its fit-image\n",
> diff --git a/tools/mkimage.h b/tools/mkimage.h
> index 9033a7d..6e18750 100644
> --- a/tools/mkimage.h
> +++ b/tools/mkimage.h
> @@ -58,6 +58,7 @@ struct mkimage_params {
> int eflag;
> int fflag;
> int lflag;
> + int pflag;
> int vflag;
> int xflag;
> int os;
Cheers
Detlev
--
Helena ist verh?ltnism??ig leicht zu besetzen. Eine Frau, zarteste Jugend
mit sinnlicher Reife verbindend; ?u?erst intelligent, indes von durchaus
weiblicher Denkart; phlegmatisch, aber sensibel; un?bertrefflich sch?n und
dabei von sehr pers?nlichem Charme - mehr wird da nicht verlangt.
-- Peter Hacks
--
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RFC][PATCH] mkimage: Add compatibility option for legacy Multi-File images
@ 2010-08-18 20:48 Thibaut Girka
2010-08-30 9:29 ` Detlev Zundel
2010-09-02 15:05 ` Wolfgang Denk
0 siblings, 2 replies; 6+ messages in thread
From: Thibaut Girka @ 2010-08-18 20:48 UTC (permalink / raw)
To: u-boot
During a few months, offsets of files in multi-file images were miscalculated,
this has been fixed by 02b9b22446e3d7ad6a6382be17a1ce79a7de589b,
but unfortunatly, some devices (I'm thinking of the Neo FreeRunner) are using
a broken version of U-Boot.
This problem can easily be worked around at image creation time by adding one
byte of junk at the end of the first (and optionnally second) file, if its
size is a multiple of 4.
It's not really clean, but it shouldn't cause any problem. At least, I haven't
encountered any using this patch.
Signed-off-by: Thibaut Girka <thib@sitedethib.com>
---
tools/mkimage.c | 9 ++++++++-
tools/mkimage.h | 1 +
2 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/tools/mkimage.c b/tools/mkimage.c
index f5859d7..239c1f0 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -243,6 +243,9 @@ main (int argc, char **argv)
usage ();
params.imagename = *++argv;
goto NXTARG;
+ case 'p':
+ params.pflag++;
+ break;
case 'v':
params.vflag++;
break;
@@ -383,6 +386,8 @@ NXTARG: ;
params.cmdname, file, strerror(errno));
exit (EXIT_FAILURE);
}
+ if (params.pflag && sep && (sbuf.st_size % 4 == 0))
+ sbuf.st_size += 1;
size = cpu_to_uimage (sbuf.st_size);
} else {
size = 0;
@@ -556,7 +561,8 @@ copy_file (int ifd, const char *datafile, int pad)
exit (EXIT_FAILURE);
}
- if (pad && ((tail = size % 4) != 0)) {
+ tail = size % 4;
+ if (pad && (params.pflag || tail != 0)) {
if (write(ifd, (char *)&zero, 4-tail) != 4-tail) {
fprintf (stderr, "%s: Write error on %s: %s\n",
@@ -586,6 +592,7 @@ usage ()
" -e ==> set entry point to 'ep' (hex)\n"
" -n ==> set image name to 'name'\n"
" -d ==> use image data from 'datafile'\n"
+ " -p ==> force padding in multi-file images\n"
" -x ==> set XIP (execute in place)\n",
params.cmdname);
fprintf (stderr, " %s [-D dtc_options] -f fit-image.its fit-image\n",
diff --git a/tools/mkimage.h b/tools/mkimage.h
index 9033a7d..6e18750 100644
--- a/tools/mkimage.h
+++ b/tools/mkimage.h
@@ -58,6 +58,7 @@ struct mkimage_params {
int eflag;
int fflag;
int lflag;
+ int pflag;
int vflag;
int xflag;
int os;
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-09-02 15:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-29 13:05 [U-Boot] [RFC][PATCH] mkimage: Add compatibility option for legacy Multi-File images Per Andersson
-- strict thread matches above, loose matches on Subject: below --
2010-08-18 20:48 Thibaut Girka
2010-08-30 9:29 ` Detlev Zundel
2010-08-31 6:50 ` Thibaut Girka
2010-09-01 15:04 ` Detlev Zundel
2010-09-02 15:05 ` Wolfgang Denk
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.