[PATCH] mkimage: allow internalization of data-position

[Simon Glass]

Hi Lars,

On Thu, 28 Sept 2023 at 03:45, Lars Feyaerts <lars at bitbiz.be> wrote:
>
> Make it possible for data that was externalized using a static external
> position (-p) to be internalized. Enables the ability to convert
> existing FIT images built with -p to be converted to a FIT image where the
> data is internal, to be converted to a FIT image where the data is
> external relative to the end of the FIT (-E) or change the initial
> static external position to a different static external position (-p).
>
> Removing the original external-data-related properties ensures that
> they're not present after conversion. Without this, they would still be
> present in the resulting FIT even if the FIT has been, for example,
> internalized.
>
> Have checkpatch.pl skip warnings for use of fdtdec_* functions in
> tooling; livetree isn't used there.
>
> Signed-off-by: Lars Feyaerts <lars at bitbiz.be>
> ---
>
>  scripts/checkpatch.pl |  4 ++--
>  tools/fit_image.c     | 26 +++++++++++++++++++++-----
>  2 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 62b764f6c38..488d73a0ed7 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2606,8 +2606,8 @@ sub u_boot_line {
>                      "Possible new uclass - make sure to add a sandbox driver, plus a test in test/dm/<name>.c\n" . $herecurr);
>         }
>
> -       # try to get people to use the livetree API
> -       if ($line =~ /^\+.*fdtdec_/) {
> +       # try to get people to use the livetree API, except when changing tooling
> +       if ($line =~ /^\+.*fdtdec_/ && $realfile !~ /^tools\//) {

Please put this in its own patch

>                 WARN("LIVETREE",
>                      "Use the livetree API (dev_read_...)\n" . $herecurr);
>         }
> diff --git a/tools/fit_image.c b/tools/fit_image.c
> index 9fe69ea0d9f..10f36e93422 100644
> --- a/tools/fit_image.c
> +++ b/tools/fit_image.c
> @@ -616,6 +616,8 @@ err:
>  static int fit_import_data(struct image_tool_params *params, const char *fname)
>  {
>         void *fdt, *old_fdt;
> +       void *data = NULL;
> +       const char *ext_data_prop = NULL;
>         int fit_size, new_size, size, data_base;
>         int fd;
>         struct stat sbuf;
> @@ -659,14 +661,28 @@ static int fit_import_data(struct image_tool_params *params, const char *fname)
>                 int buf_ptr;
>                 int len;
>
> -               buf_ptr = fdtdec_get_int(fdt, node, "data-offset", -1);
> -               len = fdtdec_get_int(fdt, node, "data-size", -1);
> -               if (buf_ptr == -1 || len == -1)
> +               /*
> +                * FIT_DATA_OFFSET_PROP and FIT_DATA_POSITION_PROP are never both present,
> +                *  but if they are, prefer FIT_DATA_OFFSET_PROP as it was there first
> +                */
> +               buf_ptr = fdtdec_get_int(fdt, node, FIT_DATA_POSITION_PROP, -1);
> +               if (buf_ptr != -1) {
> +                       ext_data_prop = FIT_DATA_POSITION_PROP;
> +                       data = old_fdt + buf_ptr;
> +               }
> +               buf_ptr = fdtdec_get_int(fdt, node, FIT_DATA_OFFSET_PROP, -1);
> +               if (buf_ptr != -1) {
> +                       ext_data_prop = FIT_DATA_OFFSET_PROP;
> +                       data = old_fdt + data_base + buf_ptr;
> +               }
> +               len = fdtdec_get_int(fdt, node, FIT_DATA_SIZE_PROP, -1);
> +               if (!data || len == -1)
>                         continue;
>                 debug("Importing data size %x\n", len);
>
> -               ret = fdt_setprop(fdt, node, "data",
> -                                 old_fdt + data_base + buf_ptr, len);
> +               ret = fdt_setprop(fdt, node, FIT_DATA_PROP, data, len);
> +               ret = fdt_delprop(fdt, node, ext_data_prop);
> +
>                 if (ret) {
>                         debug("%s: Failed to write property: %s\n", __func__,
>                               fdt_strerror(ret));
> --
> 2.34.1
>

This bit:

Reviewed-by: Simon Glass <sjg at chromium.org>

Can you add some docs about this, e.g. to mkimage.1 ?

Regards,
Simon