| Message ID | 20230919113705.109639-2-rasmus.villemoes@prevas.dk |
|---|---|
| State | Deferred |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | mkimage: also honour -B even without external data | expand |
Hi Rasmus, On Tue, 19 Sept 2023 at 05:37, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote: > > In some cases, using the "external data" feature is impossible or > undesirable, but one may still want (or need) the FIT image to have a > certain alignment. Also, given the current 'mkimage -h' output, > > -B => align size in hex for FIT structure and header > > it is quite unexpected for -B to be effectively ignored without -E. > > Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> > --- > tools/fit_image.c | 40 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) Reviewed-by: Simon Glass <sjg@chromium.org> Q below > > diff --git a/tools/fit_image.c b/tools/fit_image.c > index 9fe69ea0d9..2f5b25098a 100644 > --- a/tools/fit_image.c > +++ b/tools/fit_image.c > @@ -712,6 +712,42 @@ err: > return ret; > } > > +/** > + * fit_align() - Ensure FIT image has certain alignment > + * > + * This takes a normal FIT file (with embedded data) and increases its > + * size so that it is a multiple of params->bl_len. > + */ > +static int fit_align(struct image_tool_params *params, const char *fname) > +{ > + int fit_size, new_size; > + int fd; > + struct stat sbuf; > + void *fdt; > + int ret = 0; > + int align_size; > + > + align_size = params->bl_len; > + fd = mmap_fdt(params->cmdname, fname, 0, &fdt, &sbuf, false, false); > + if (fd < 0) > + return -EIO; > + > + fit_size = fdt_totalsize(fdt); > + new_size = ALIGN(fit_size, align_size); > + fdt_set_totalsize(fdt, new_size); Shouldn't this be fdt_open_into()? > + debug("Size extended from from %x to %x\n", fit_size, new_size); > + munmap(fdt, sbuf.st_size); > + > + if (ftruncate(fd, new_size)) { > + debug("%s: Failed to truncate file: %s\n", __func__, > + strerror(errno)); > + ret = -EIO; > + } > + > + close(fd); > + return ret; > +} > + > /** > * fit_handle_file - main FIT file processing function > * > @@ -817,6 +853,10 @@ static int fit_handle_file(struct image_tool_params *params) > ret = fit_extract_data(params, tmpfile); > if (ret) > goto err_system; > + } else if (params->bl_len) { > + ret = fit_align(params, tmpfile); > + if (ret) > + goto err_system; > } > > if (rename (tmpfile, params->imagefile) == -1) { > -- > 2.37.2 > Regards, Simon
On 21/09/2023 03.02, Simon Glass wrote: > Hi Rasmus, > > On Tue, 19 Sept 2023 at 05:37, Rasmus Villemoes > <rasmus.villemoes@prevas.dk> wrote: >> >> In some cases, using the "external data" feature is impossible or >> undesirable, but one may still want (or need) the FIT image to have a >> certain alignment. Also, given the current 'mkimage -h' output, >> >> -B => align size in hex for FIT structure and header >> >> it is quite unexpected for -B to be effectively ignored without -E. >> >> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> >> --- >> tools/fit_image.c | 40 ++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 40 insertions(+) > > Reviewed-by: Simon Glass <sjg@chromium.org> > > Q below > >> >> diff --git a/tools/fit_image.c b/tools/fit_image.c >> index 9fe69ea0d9..2f5b25098a 100644 >> --- a/tools/fit_image.c >> +++ b/tools/fit_image.c >> @@ -712,6 +712,42 @@ err: >> return ret; >> } >> >> +/** >> + * fit_align() - Ensure FIT image has certain alignment >> + * >> + * This takes a normal FIT file (with embedded data) and increases its >> + * size so that it is a multiple of params->bl_len. >> + */ >> +static int fit_align(struct image_tool_params *params, const char *fname) >> +{ >> + int fit_size, new_size; >> + int fd; >> + struct stat sbuf; >> + void *fdt; >> + int ret = 0; >> + int align_size; >> + >> + align_size = params->bl_len; >> + fd = mmap_fdt(params->cmdname, fname, 0, &fdt, &sbuf, false, false); >> + if (fd < 0) >> + return -EIO; >> + >> + fit_size = fdt_totalsize(fdt); >> + new_size = ALIGN(fit_size, align_size); >> + fdt_set_totalsize(fdt, new_size); > > Shouldn't this be fdt_open_into()? I honestly just copy-pasted fit_extract_data() and shaved it down to the part that does the "align the FDT part of the file". I don't really understand your question. Are you saying this doesn't work (or maybe doesn't work in some cases), or are you saying that there's a simpler way to do this? For the latter, sure, one doesn't really need to parse the whole FDT; we could just open() pread() length from FDT header, convert to cpu-endianness length = ALIGN(length) pwrite() the new length in fdt-endianness ftruncate() close() but I thought it was better to stay closer to how fit_extract_data() was done. Rasmus
Hi Rasmus, On Thu, 21 Sept 2023 at 01:57, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote: > > On 21/09/2023 03.02, Simon Glass wrote: > > Hi Rasmus, > > > > On Tue, 19 Sept 2023 at 05:37, Rasmus Villemoes > > <rasmus.villemoes@prevas.dk> wrote: > >> > >> In some cases, using the "external data" feature is impossible or > >> undesirable, but one may still want (or need) the FIT image to have a > >> certain alignment. Also, given the current 'mkimage -h' output, > >> > >> -B => align size in hex for FIT structure and header > >> > >> it is quite unexpected for -B to be effectively ignored without -E. > >> > >> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> > >> --- > >> tools/fit_image.c | 40 ++++++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 40 insertions(+) > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > Q below > > > >> > >> diff --git a/tools/fit_image.c b/tools/fit_image.c > >> index 9fe69ea0d9..2f5b25098a 100644 > >> --- a/tools/fit_image.c > >> +++ b/tools/fit_image.c > >> @@ -712,6 +712,42 @@ err: > >> return ret; > >> } > >> > >> +/** > >> + * fit_align() - Ensure FIT image has certain alignment > >> + * > >> + * This takes a normal FIT file (with embedded data) and increases its > >> + * size so that it is a multiple of params->bl_len. > >> + */ > >> +static int fit_align(struct image_tool_params *params, const char *fname) > >> +{ > >> + int fit_size, new_size; > >> + int fd; > >> + struct stat sbuf; > >> + void *fdt; > >> + int ret = 0; > >> + int align_size; > >> + > >> + align_size = params->bl_len; > >> + fd = mmap_fdt(params->cmdname, fname, 0, &fdt, &sbuf, false, false); > >> + if (fd < 0) > >> + return -EIO; > >> + > >> + fit_size = fdt_totalsize(fdt); > >> + new_size = ALIGN(fit_size, align_size); > >> + fdt_set_totalsize(fdt, new_size); > > > > Shouldn't this be fdt_open_into()? > > I honestly just copy-pasted fit_extract_data() and shaved it down to the > part that does the "align the FDT part of the file". > > I don't really understand your question. Are you saying this doesn't > work (or maybe doesn't work in some cases), or are you saying that > there's a simpler way to do this? For the latter, sure, one doesn't > really need to parse the whole FDT; we could just > > open() > pread() length from FDT header, convert to cpu-endianness > length = ALIGN(length) > pwrite() the new length in fdt-endianness > ftruncate() > close() > > but I thought it was better to stay closer to how fit_extract_data() was > done. I mean that fdt_open_into() does more than just set the size (from what I can tell). But looking further I see other code which calls fdt_set_totalsize() so perhaps it is fine. Regards, SImon
On 22/09/2023 17.26, Simon Glass wrote: >>> Shouldn't this be fdt_open_into()? >> >> I honestly just copy-pasted fit_extract_data() and shaved it down to the >> part that does the "align the FDT part of the file". >> >> I don't really understand your question. Are you saying this doesn't >> work (or maybe doesn't work in some cases), or are you saying that >> there's a simpler way to do this? For the latter, sure, one doesn't >> really need to parse the whole FDT; we could just >> >> open() >> pread() length from FDT header, convert to cpu-endianness >> length = ALIGN(length) >> pwrite() the new length in fdt-endianness >> ftruncate() >> close() >> >> but I thought it was better to stay closer to how fit_extract_data() was >> done. > > I mean that fdt_open_into() does more than just set the size (from > what I can tell). But looking further I see other code which calls > fdt_set_totalsize() so perhaps it is fine. Yes, I think it's as it should be - as a I said, this is really just a trimmed-down copy of the function which moves the data externally, and also needs to make the size of the base fdt structure aligned. Since patches 2,3,4 touch binman code, could you take all four? Thanks, Rasmus
Hi Rasmus, On Mon, 25 Sept 2023 at 02:47, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote: > > On 22/09/2023 17.26, Simon Glass wrote: > > >>> Shouldn't this be fdt_open_into()? > >> > >> I honestly just copy-pasted fit_extract_data() and shaved it down to the > >> part that does the "align the FDT part of the file". > >> > >> I don't really understand your question. Are you saying this doesn't > >> work (or maybe doesn't work in some cases), or are you saying that > >> there's a simpler way to do this? For the latter, sure, one doesn't > >> really need to parse the whole FDT; we could just > >> > >> open() > >> pread() length from FDT header, convert to cpu-endianness > >> length = ALIGN(length) > >> pwrite() the new length in fdt-endianness > >> ftruncate() > >> close() > >> > >> but I thought it was better to stay closer to how fit_extract_data() was > >> done. > > > > I mean that fdt_open_into() does more than just set the size (from > > what I can tell). But looking further I see other code which calls > > fdt_set_totalsize() so perhaps it is fine. > > Yes, I think it's as it should be - as a I said, this is really just a > trimmed-down copy of the function which moves the data externally, and > also needs to make the size of the base fdt structure aligned. > > Since patches 2,3,4 touch binman code, could you take all four? Yes, will do. I didn't pick them up in the most recent PR as I try to have things sit for a week before applying. Regards, Simon
On 25/09/2023 15.10, Simon Glass wrote: > Hi Rasmus, > > On Mon, 25 Sept 2023 at 02:47, Rasmus Villemoes > <rasmus.villemoes@prevas.dk> wrote: >> Since patches 2,3,4 touch binman code, could you take all four? > > Yes, will do. I didn't pick them up in the most recent PR as I try to > have things sit for a week before applying. Oh, absolutely, no rush :) Rasmus
On 9/19/23 07:37, Rasmus Villemoes wrote: > In some cases, using the "external data" feature is impossible or > undesirable, but one may still want (or need) the FIT image to have a > certain alignment. Also, given the current 'mkimage -h' output, > > -B => align size in hex for FIT structure and header > > it is quite unexpected for -B to be effectively ignored without -E. FWIW, this behavior is documented in doc/mkimage.1 (which should also be updated if this behavior is implemented): | The alignment, in hexadecimal, that external data will be aligned to. | This option only has an effect when -E is specified. And, for additional context, the documentation for -E is | After processing, move the image data outside the FIT and store a data | offset in the FIT. Images will be placed one after the other | immediately after the FIT, with each one aligned to a 4-byte boundary. | The existing ‘data’ property in each image will be replaced with | ‘data-offset’ and ‘data-size’ properties. A ‘data-offset’ of 0 | indicates that it starts in the first (4-byte-aligned) byte after the | FIT. Based on this documentation and my understanding of the code as-is, -B controls the alignment of the images themselves, not the size multiple of the FIT. However, from what I can tell, this patch does not actually affect the alignment of the images, but rather adjusts the size of the overall FIT to a certain alignment. I find this rather unexpected. --Sean > Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> > --- > tools/fit_image.c | 40 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > > diff --git a/tools/fit_image.c b/tools/fit_image.c > index 9fe69ea0d9..2f5b25098a 100644 > --- a/tools/fit_image.c > +++ b/tools/fit_image.c > @@ -712,6 +712,42 @@ err: > return ret; > } > > +/** > + * fit_align() - Ensure FIT image has certain alignment > + * > + * This takes a normal FIT file (with embedded data) and increases its > + * size so that it is a multiple of params->bl_len. > + */ > +static int fit_align(struct image_tool_params *params, const char *fname) > +{ > + int fit_size, new_size; > + int fd; > + struct stat sbuf; > + void *fdt; > + int ret = 0; > + int align_size; > + > + align_size = params->bl_len; > + fd = mmap_fdt(params->cmdname, fname, 0, &fdt, &sbuf, false, false); > + if (fd < 0) > + return -EIO; > + > + fit_size = fdt_totalsize(fdt); > + new_size = ALIGN(fit_size, align_size); > + fdt_set_totalsize(fdt, new_size); > + debug("Size extended from from %x to %x\n", fit_size, new_size); > + munmap(fdt, sbuf.st_size); > + > + if (ftruncate(fd, new_size)) { > + debug("%s: Failed to truncate file: %s\n", __func__, > + strerror(errno)); > + ret = -EIO; > + } > + > + close(fd); > + return ret; > +} > + > /** > * fit_handle_file - main FIT file processing function > * > @@ -817,6 +853,10 @@ static int fit_handle_file(struct image_tool_params *params) > ret = fit_extract_data(params, tmpfile); > if (ret) > goto err_system; > + } else if (params->bl_len) { > + ret = fit_align(params, tmpfile); > + if (ret) > + goto err_system; > } > > if (rename (tmpfile, params->imagefile) == -1) {
On 27/09/2023 21.02, Sean Anderson wrote: > On 9/19/23 07:37, Rasmus Villemoes wrote: >> In some cases, using the "external data" feature is impossible or >> undesirable, but one may still want (or need) the FIT image to have a >> certain alignment. Also, given the current 'mkimage -h' output, >> >> -B => align size in hex for FIT structure and header >> >> it is quite unexpected for -B to be effectively ignored without -E. > > FWIW, this behavior is documented in doc/mkimage.1 (which should also be > updated if this behavior is implemented): > > | The alignment, in hexadecimal, that external data will be aligned to. > | This option only has an effect when -E is specified. I'll send a follow-up to fix that, thanks. > And, for additional context, the documentation for -E is > > | After processing, move the image data outside the FIT and store a data > | offset in the FIT. Images will be placed one after the other > | immediately after the FIT, with each one aligned to a 4-byte boundary. > | The existing ‘data’ property in each image will be replaced with > | ‘data-offset’ and ‘data-size’ properties. A ‘data-offset’ of 0 > | indicates that it starts in the first (4-byte-aligned) byte after the > | FIT. > > Based on this documentation and my understanding of the code as-is, -B > controls the alignment of the images themselves, Yes, when -E is in effect. My patch does not affect the case where -E is present. > not the size multiple of the FIT. It is _also_ that, but mostly as a "necessary side effect" of getting the first image aligned. In order to achieve the desired alignment of the first external image, the FIT structure itself is aligned to the -B value, and each image's size similarly aligned up to that value so the next image ends on a -B multiple again. So with both -E and -B, the FIT structure itself is indeed also padded to the -B value, as the `mkimage -h` output suggests. What I want, and expected from `mkimage -h`, is to make the whole FIT have a certain size multiple, without using -E, so in that case the alignment of the FIT is the primary goal. > However, from what I can tell, this patch does not actually > affect the alignment of the images, No, because the images are (still) embedded within the FIT as ordinary values of the data= properties, and it's a basic property of the DTB format that those are always 4-byte aligned, and you can't (easily) change that [1]. Which, I suppose, is one of the reasons U-Boot invented the "external data" mechanism - so for example the .dtbs embedded in the FIT can all be guaranteed to be on an 8-byte boundary, and thus the selected one can be used in-place when passed to linux. > but rather adjusts the size of the > overall FIT to a certain alignment. I find this rather unexpected. Well, as I said, _I_ was surprised by -B having no effect based on the `mkimage -h` output, so the two sources of documentation were not in sync. The man page was/is correct wrt. the actual code. Rasmus [1] There's a nop tag one can insert, and I think I saw somebody actually suggesting to do that to align the embedded data= properties, but it's quite error-prone and inflexible, as any subsequent modification of the .dtb before that property will ruin the alignment.
On 9/28/23 03:10, Rasmus Villemoes wrote: > On 27/09/2023 21.02, Sean Anderson wrote: >> On 9/19/23 07:37, Rasmus Villemoes wrote: >>> In some cases, using the "external data" feature is impossible or >>> undesirable, but one may still want (or need) the FIT image to have a >>> certain alignment. Also, given the current 'mkimage -h' output, >>> >>> -B => align size in hex for FIT structure and header >>> >>> it is quite unexpected for -B to be effectively ignored without -E. >> >> FWIW, this behavior is documented in doc/mkimage.1 (which should also be >> updated if this behavior is implemented): >> >> | The alignment, in hexadecimal, that external data will be aligned to. >> | This option only has an effect when -E is specified. > > I'll send a follow-up to fix that, thanks. > >> And, for additional context, the documentation for -E is >> >> | After processing, move the image data outside the FIT and store a data >> | offset in the FIT. Images will be placed one after the other >> | immediately after the FIT, with each one aligned to a 4-byte boundary. >> | The existing ‘data’ property in each image will be replaced with >> | ‘data-offset’ and ‘data-size’ properties. A ‘data-offset’ of 0 >> | indicates that it starts in the first (4-byte-aligned) byte after the >> | FIT. >> >> Based on this documentation and my understanding of the code as-is, -B >> controls the alignment of the images themselves, > > Yes, when -E is in effect. My patch does not affect the case where -E is > present. Wherever possible, flags should be orthogonal. That is, they should have the same effect (or at least the same spirit) regardless of the presence of other flags. This matches their UX, as orthogonally selectable options. >> not the size multiple of the FIT. > > It is _also_ that, but mostly as a "necessary side effect" of getting > the first image aligned. In order to achieve the desired alignment of > the first external image, the FIT structure itself is aligned to the -B > value, and each image's size similarly aligned up to that value so the > next image ends on a -B multiple again. So with both -E and -B, the FIT > structure itself is indeed also padded to the -B value, as the `mkimage > -h` output suggests. The intent of -B is to align images. That the FIT itself is padded is an implementation detail. > What I want, and expected from `mkimage -h`, is to make the whole FIT > have a certain size multiple, without using -E, so in that case the > alignment of the FIT is the primary goal. Why does mkimage have to do this? Can't you just use truncate or, in a binman context, align-size? >> However, from what I can tell, this patch does not actually >> affect the alignment of the images, > > No, because the images are (still) embedded within the FIT as ordinary > values of the data= properties, and it's a basic property of the DTB > format that those are always 4-byte aligned, and you can't (easily) > change that [1]. Which, I suppose, is one of the reasons U-Boot invented > the "external data" mechanism - so for example the .dtbs embedded in the > FIT can all be guaranteed to be on an 8-byte boundary, and thus the > selected one can be used in-place when passed to linux. > >> but rather adjusts the size of the >> overall FIT to a certain alignment. I find this rather unexpected. > > Well, as I said, _I_ was surprised by -B having no effect based on the > `mkimage -h` output, so the two sources of documentation were not in > sync. The man page was/is correct wrt. the actual code. The mkimage -h putput is too terse to record the complete behavior of each option. Perhaps we should add a warning when -B is specified without -E. > Rasmus > > [1] There's a nop tag one can insert, and I think I saw somebody > actually suggesting to do that to align the embedded data= properties, > but it's quite error-prone and inflexible, as any subsequent > modification of the .dtb before that property will ruin the alignment. This would indeed be the way to go. I don't think we should worry about further modification of the fdt, as this could also ruin the alignment of external images. --Sean
Hi Rasmus, On Fri, 29 Sept 2023 at 07:16, Sean Anderson <seanga2@gmail.com> wrote: > > On 9/28/23 03:10, Rasmus Villemoes wrote: > > On 27/09/2023 21.02, Sean Anderson wrote: > >> On 9/19/23 07:37, Rasmus Villemoes wrote: > >>> In some cases, using the "external data" feature is impossible or > >>> undesirable, but one may still want (or need) the FIT image to have a > >>> certain alignment. Also, given the current 'mkimage -h' output, > >>> > >>> -B => align size in hex for FIT structure and header > >>> > >>> it is quite unexpected for -B to be effectively ignored without -E. > >> > >> FWIW, this behavior is documented in doc/mkimage.1 (which should also be > >> updated if this behavior is implemented): > >> > >> | The alignment, in hexadecimal, that external data will be aligned to. > >> | This option only has an effect when -E is specified. > > > > I'll send a follow-up to fix that, thanks. > > > >> And, for additional context, the documentation for -E is > >> > >> | After processing, move the image data outside the FIT and store a data > >> | offset in the FIT. Images will be placed one after the other > >> | immediately after the FIT, with each one aligned to a 4-byte boundary. > >> | The existing ‘data’ property in each image will be replaced with > >> | ‘data-offset’ and ‘data-size’ properties. A ‘data-offset’ of 0 > >> | indicates that it starts in the first (4-byte-aligned) byte after the > >> | FIT. > >> > >> Based on this documentation and my understanding of the code as-is, -B > >> controls the alignment of the images themselves, > > > > Yes, when -E is in effect. My patch does not affect the case where -E is > > present. > > Wherever possible, flags should be orthogonal. That is, they should have the > same effect (or at least the same spirit) regardless of the presence of other > flags. This matches their UX, as orthogonally selectable options. > > >> not the size multiple of the FIT. > > > > It is _also_ that, but mostly as a "necessary side effect" of getting > > the first image aligned. In order to achieve the desired alignment of > > the first external image, the FIT structure itself is aligned to the -B > > value, and each image's size similarly aligned up to that value so the > > next image ends on a -B multiple again. So with both -E and -B, the FIT > > structure itself is indeed also padded to the -B value, as the `mkimage > > -h` output suggests. > > The intent of -B is to align images. That the FIT itself is padded is an > implementation detail. > > > What I want, and expected from `mkimage -h`, is to make the whole FIT > > have a certain size multiple, without using -E, so in that case the > > alignment of the FIT is the primary goal. > > Why does mkimage have to do this? Can't you just use truncate or, in a > binman context, align-size? > > >> However, from what I can tell, this patch does not actually > >> affect the alignment of the images, > > > > No, because the images are (still) embedded within the FIT as ordinary > > values of the data= properties, and it's a basic property of the DTB > > format that those are always 4-byte aligned, and you can't (easily) > > change that [1]. Which, I suppose, is one of the reasons U-Boot invented > > the "external data" mechanism - so for example the .dtbs embedded in the > > FIT can all be guaranteed to be on an 8-byte boundary, and thus the > > selected one can be used in-place when passed to linux. > > > >> but rather adjusts the size of the > >> overall FIT to a certain alignment. I find this rather unexpected. > > > > Well, as I said, _I_ was surprised by -B having no effect based on the > > `mkimage -h` output, so the two sources of documentation were not in > > sync. The man page was/is correct wrt. the actual code. > > The mkimage -h putput is too terse to record the complete behavior of > each option. Perhaps we should add a warning when -B is specified without > -E. > > > Rasmus > > > > [1] There's a nop tag one can insert, and I think I saw somebody > > actually suggesting to do that to align the embedded data= properties, > > but it's quite error-prone and inflexible, as any subsequent > > modification of the .dtb before that property will ruin the alignment. > > This would indeed be the way to go. I don't think we should worry about > further modification of the fdt, as this could also ruin the alignment > of external images. Are you planning a new version of this series? Regards, Simon
On 04/11/2023 20.43, Simon Glass wrote: > Hi Rasmus, > Are you planning a new version of this series? No. AFAICT there's nothing to be done on my end. Rasmus
diff --git a/tools/fit_image.c b/tools/fit_image.c index 9fe69ea0d9..2f5b25098a 100644 --- a/tools/fit_image.c +++ b/tools/fit_image.c @@ -712,6 +712,42 @@ err: return ret; } +/** + * fit_align() - Ensure FIT image has certain alignment + * + * This takes a normal FIT file (with embedded data) and increases its + * size so that it is a multiple of params->bl_len. + */ +static int fit_align(struct image_tool_params *params, const char *fname) +{ + int fit_size, new_size; + int fd; + struct stat sbuf; + void *fdt; + int ret = 0; + int align_size; + + align_size = params->bl_len; + fd = mmap_fdt(params->cmdname, fname, 0, &fdt, &sbuf, false, false); + if (fd < 0) + return -EIO; + + fit_size = fdt_totalsize(fdt); + new_size = ALIGN(fit_size, align_size); + fdt_set_totalsize(fdt, new_size); + debug("Size extended from from %x to %x\n", fit_size, new_size); + munmap(fdt, sbuf.st_size); + + if (ftruncate(fd, new_size)) { + debug("%s: Failed to truncate file: %s\n", __func__, + strerror(errno)); + ret = -EIO; + } + + close(fd); + return ret; +} + /** * fit_handle_file - main FIT file processing function * @@ -817,6 +853,10 @@ static int fit_handle_file(struct image_tool_params *params) ret = fit_extract_data(params, tmpfile); if (ret) goto err_system; + } else if (params->bl_len) { + ret = fit_align(params, tmpfile); + if (ret) + goto err_system; } if (rename (tmpfile, params->imagefile) == -1) {
In some cases, using the "external data" feature is impossible or undesirable, but one may still want (or need) the FIT image to have a certain alignment. Also, given the current 'mkimage -h' output, -B => align size in hex for FIT structure and header it is quite unexpected for -B to be effectively ignored without -E. Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> --- tools/fit_image.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)