| Message ID | 1456206959-29115-17-git-send-email-sjg@chromium.org |
|---|---|
| State | Accepted |
| Delegated to: | Tom Rini |
| Headers | show |
On Mon, Feb 22, 2016 at 10:55:48PM -0700, Simon Glass wrote: > At present FIT images are set up by providing a device tree source file > which is a file with a .its extension. We want to support automatically > creating this file based on the image supplied to mkimage. This means that > even though the final file type is always IH_TYPE_FLATDT, the image inside > may be something else. > > Signed-off-by: Simon Glass <sjg@chromium.org> Applied to u-boot/master, thanks!
Hi Simon, On 23.02.2016 06:55, Simon Glass wrote: > At present FIT images are set up by providing a device tree source file > which is a file with a .its extension. We want to support automatically > creating this file based on the image supplied to mkimage. This means that > even though the final file type is always IH_TYPE_FLATDT, the image inside > may be something else. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > Changes in v2: > - Always expect the final argument to be the image file > diff --git a/tools/mkimage.c b/tools/mkimage.c > index b8293f6..2fd1f0b 100644 > --- a/tools/mkimage.c > +++ b/tools/mkimage.c > @@ -112,10 +112,14 @@ static void usage(const char *msg) > static void process_args(int argc, char **argv) > { > char *ptr; > + int type = IH_TYPE_INVALID; > + char *datafile = NULL; > + int expecting; > int opt; > > + expecting = IH_TYPE_COUNT; /* Unknown */ > while ((opt = getopt(argc, argv, > - "a:A:cC:d:D:e:f:Fk:K:ln:O:rR:sT:vVx")) != -1) { > + "-a:A:cC:d:D:e:f:Fk:K:ln:O:rR:sT:vVx")) != -1) { I just encountered an error building the latest ToT on my OS X box with this change. Unfortunately the '-' in optstring seems to be an GNU option and is not portable. > switch (opt) { > @@ -211,14 +217,31 @@ static void process_args(int argc, char **argv) > case 'x': > params.xflag++; > break; > + case 1: > + if (expecting == type || optind == argc) { > + params.imagefile = optarg; > + expecting = IH_TYPE_INVALID; > + } > + break; > default: > usage("Invalid option"); > } > } > > - if (optind >= argc) > + /* > + * For auto-generated FIT images we need to know the image type to put > + * in the FIT, which is separate from the file's image type (which > + * will always be IH_TYPE_FLATDT in this case). > + */ > + if (params.type == IH_TYPE_FLATDT) { > + params.fit_image_type = type; > + params.datafile = datafile; > + } else if (type != IH_TYPE_INVALID) { > + params.type = type; > + } > + > + if (!params.imagefile) > usage("Missing output filename"); > - params.imagefile = argv[optind]; > } Therefore the imagefile is never set. Do you mind to switch back to the portable version with 'POSIX_CORRECT'? Andreas
Hi Andreas, On 27 April 2016 at 01:28, Andreas Bießmann <andreas.devel@googlemail.com> wrote: > Hi Simon, > > On 23.02.2016 06:55, Simon Glass wrote: >> At present FIT images are set up by providing a device tree source file >> which is a file with a .its extension. We want to support automatically >> creating this file based on the image supplied to mkimage. This means that >> even though the final file type is always IH_TYPE_FLATDT, the image inside >> may be something else. >> >> Signed-off-by: Simon Glass <sjg@chromium.org> >> --- >> >> Changes in v2: >> - Always expect the final argument to be the image file > >> diff --git a/tools/mkimage.c b/tools/mkimage.c >> index b8293f6..2fd1f0b 100644 >> --- a/tools/mkimage.c >> +++ b/tools/mkimage.c >> @@ -112,10 +112,14 @@ static void usage(const char *msg) >> static void process_args(int argc, char **argv) >> { >> char *ptr; >> + int type = IH_TYPE_INVALID; >> + char *datafile = NULL; >> + int expecting; >> int opt; >> >> + expecting = IH_TYPE_COUNT; /* Unknown */ >> while ((opt = getopt(argc, argv, >> - "a:A:cC:d:D:e:f:Fk:K:ln:O:rR:sT:vVx")) != -1) { >> + "-a:A:cC:d:D:e:f:Fk:K:ln:O:rR:sT:vVx")) != -1) { > > I just encountered an error building the latest ToT on my OS X box with > this change. Unfortunately the '-' in optstring seems to be an GNU > option and is not portable. > >> switch (opt) { > >> @@ -211,14 +217,31 @@ static void process_args(int argc, char **argv) >> case 'x': >> params.xflag++; >> break; >> + case 1: >> + if (expecting == type || optind == argc) { >> + params.imagefile = optarg; >> + expecting = IH_TYPE_INVALID; >> + } >> + break; >> default: >> usage("Invalid option"); >> } >> } >> >> - if (optind >= argc) >> + /* >> + * For auto-generated FIT images we need to know the image type to put >> + * in the FIT, which is separate from the file's image type (which >> + * will always be IH_TYPE_FLATDT in this case). >> + */ >> + if (params.type == IH_TYPE_FLATDT) { >> + params.fit_image_type = type; >> + params.datafile = datafile; >> + } else if (type != IH_TYPE_INVALID) { >> + params.type = type; >> + } >> + >> + if (!params.imagefile) >> usage("Missing output filename"); >> - params.imagefile = argv[optind]; >> } > > Therefore the imagefile is never set. Do you mind to switch back to the > portable version with 'POSIX_CORRECT'? Hmmm I didn't consider that. I see you have sent a patch (thank you) so let's continue on that thread. Regards, Simon
diff --git a/tools/imagetool.h b/tools/imagetool.h index ad2deb5..e0397f7 100644 --- a/tools/imagetool.h +++ b/tools/imagetool.h @@ -61,6 +61,7 @@ struct image_tool_params { int require_keys; /* 1 to mark signing keys as 'required' */ int file_size; /* Total size of output file */ int orig_file_size; /* Original size for file before padding */ + int fit_image_type; /* Image type to put into the FIT */ }; /* diff --git a/tools/mkimage.c b/tools/mkimage.c index b8293f6..2fd1f0b 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -112,10 +112,14 @@ static void usage(const char *msg) static void process_args(int argc, char **argv) { char *ptr; + int type = IH_TYPE_INVALID; + char *datafile = NULL; + int expecting; int opt; + expecting = IH_TYPE_COUNT; /* Unknown */ while ((opt = getopt(argc, argv, - "a:A:cC:d:D:e:f:Fk:K:ln:O:rR:sT:vVx")) != -1) { + "-a:A:cC:d:D:e:f:Fk:K:ln:O:rR:sT:vVx")) != -1) { switch (opt) { case 'a': params.addr = strtoul(optarg, &ptr, 16); @@ -162,6 +166,7 @@ static void process_args(int argc, char **argv) * The flattened image tree (FIT) format * requires a flattened device tree image type */ + params.fit_image_type = params.type; params.type = IH_TYPE_FLATDT; params.fflag = 1; break; @@ -196,11 +201,12 @@ static void process_args(int argc, char **argv) params.skipcpy = 1; break; case 'T': - params.type = genimg_get_type_id(optarg); - if (params.type < 0) { + type = genimg_get_type_id(optarg); + if (type < 0) { show_image_types(); usage("Invalid image type"); } + expecting = type; break; case 'v': params.vflag++; @@ -211,14 +217,31 @@ static void process_args(int argc, char **argv) case 'x': params.xflag++; break; + case 1: + if (expecting == type || optind == argc) { + params.imagefile = optarg; + expecting = IH_TYPE_INVALID; + } + break; default: usage("Invalid option"); } } - if (optind >= argc) + /* + * For auto-generated FIT images we need to know the image type to put + * in the FIT, which is separate from the file's image type (which + * will always be IH_TYPE_FLATDT in this case). + */ + if (params.type == IH_TYPE_FLATDT) { + params.fit_image_type = type; + params.datafile = datafile; + } else if (type != IH_TYPE_INVALID) { + params.type = type; + } + + if (!params.imagefile) usage("Missing output filename"); - params.imagefile = argv[optind]; }
At present FIT images are set up by providing a device tree source file which is a file with a .its extension. We want to support automatically creating this file based on the image supplied to mkimage. This means that even though the final file type is always IH_TYPE_FLATDT, the image inside may be something else. Signed-off-by: Simon Glass <sjg@chromium.org> --- Changes in v2: - Always expect the final argument to be the image file tools/imagetool.h | 1 + tools/mkimage.c | 33 ++++++++++++++++++++++++++++----- 2 files changed, 29 insertions(+), 5 deletions(-)