Re: [PATCH] mkimage: allow internalization of data-position

Hi Lars,

On Thu, 28 Sept 2023 at 03:45, Lars Feyaerts lars@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@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@chromium.org

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

Regards, Simon