u-boot.lists.denx.de archive mirror
 help / color / mirror / Atom feed
From: Detlev Zundel <dzu@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [RFC][PATCH] mkimage: Add compatibility option for legacy Multi-File images
Date: Mon, 30 Aug 2010 11:29:04 +0200	[thread overview]
Message-ID: <m2occkla8v.fsf@ohwell.denx.de> (raw)
In-Reply-To: <1282164515-28846-1-git-send-email-thib@sitedethib.com> (Thibaut Girka's message of "Wed, 18 Aug 2010 22:48:35 +0200")

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

  reply	other threads:[~2010-08-30  9:29 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-18 20:48 [U-Boot] [RFC][PATCH] mkimage: Add compatibility option for legacy Multi-File images Thibaut Girka
2010-08-30  9:29 ` Detlev Zundel [this message]
2010-08-31  6:50   ` Thibaut Girka
2010-09-01 15:04     ` Detlev Zundel
2010-09-02 15:05 ` Wolfgang Denk
2010-08-29 13:05 Per Andersson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m2occkla8v.fsf@ohwell.denx.de \
    --to=dzu@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).