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