[PATCH 1/4] mkimage: also honour -B even without external data

[Simon Glass]

Hi Rasmus,

On Thu, 21 Sept 2023 at 01:57, Rasmus Villemoes
<rasmus.villemoes at 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 at 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 at prevas.dk>
> >> ---
> >>  tools/fit_image.c | 40 ++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 40 insertions(+)
> >
> > Reviewed-by: Simon Glass <sjg at 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