All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/2] fit: FDT compression
@ 2019-04-18  1:22 Julius Werner
  2019-04-18  1:22 ` [U-Boot] [PATCH 1/2] fit: Support " Julius Werner
  2019-04-18  1:22 ` [U-Boot] [PATCH 2/2] fit: Support compat string property in configuration node Julius Werner
  0 siblings, 2 replies; 9+ messages in thread
From: Julius Werner @ 2019-04-18  1:22 UTC (permalink / raw)
  To: u-boot

This patch series adds FDT compression support. The first patch adds the
compression support itself, the second adds a new feature to compatible
string matching that allows it to be useful with compressed FDTs.

Sandbox-tested with FIT images with and without compressed FDTs, with
and without overlays, in both compatible string matching and direct
config selection modes.

Julius Werner (2):
  fit: Support FDT compression
  fit: Support compat string property in configuration node

 common/image-fit.c | 142 +++++++++++++++++++++++++++------------------
 1 file changed, 85 insertions(+), 57 deletions(-)

-- 
2.20.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 1/2] fit: Support FDT compression
  2019-04-18  1:22 [U-Boot] [PATCH 0/2] fit: FDT compression Julius Werner
@ 2019-04-18  1:22 ` Julius Werner
  2019-04-18  8:29   ` Simon Goldschmidt
  2019-04-18  1:22 ` [U-Boot] [PATCH 2/2] fit: Support compat string property in configuration node Julius Werner
  1 sibling, 1 reply; 9+ messages in thread
From: Julius Werner @ 2019-04-18  1:22 UTC (permalink / raw)
  To: u-boot

This patch adds support for compressing FDT image nodes in a FIT image
(equivalent to how kernel nodes can already be compressed). This can
reduce the size of FIT images (and therefore improve boot times). FDTs
will automatically get decompressed on load.

This patch does not support extracting compatible strings from
compressed FDTs, so it's not very helpful in conjunction with
CONFIG_FIT_BEST_MATCH yet, but it can already be used in environments
that select the configuration to load explicitly.

Signed-off-by: Julius Werner <jwerner@chromium.org>
---
 common/image-fit.c | 85 +++++++++++++++++++++++++++-------------------
 1 file changed, 51 insertions(+), 34 deletions(-)

diff --git a/common/image-fit.c b/common/image-fit.c
index ac901e131c..20e02d240b 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -22,6 +22,7 @@
 DECLARE_GLOBAL_DATA_PTR;
 #endif /* !USE_HOSTCC*/
 
+#include <bootm.h>
 #include <image.h>
 #include <bootstage.h>
 #include <u-boot/crc.h>
@@ -1576,6 +1577,13 @@ int fit_conf_find_compat(const void *fit, const void *fdt)
 			      kfdt_name);
 			continue;
 		}
+
+		if (!fit_image_check_comp(fit, kfdt_noffset, IH_COMP_NONE)) {
+			debug("Can't extract compat from \"%s\" (compressed)\n",
+			      kfdt_name);
+			continue;
+		}
+
 		/*
 		 * Get a pointer to this configuration's fdt.
 		 */
@@ -1795,11 +1803,12 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
 	const char *fit_uname_config;
 	const char *fit_base_uname_config;
 	const void *fit;
-	const void *buf;
+	void *buf;
+	void *loadbuf;
 	size_t size;
 	int type_ok, os_ok;
-	ulong load, data, len;
-	uint8_t os;
+	ulong load, load_end, data, len;
+	uint8_t os, comp;
 #ifndef USE_HOSTCC
 	uint8_t os_arch;
 #endif
@@ -1895,12 +1904,6 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
 	images->os.arch = os_arch;
 #endif
 
-	if (image_type == IH_TYPE_FLATDT &&
-	    !fit_image_check_comp(fit, noffset, IH_COMP_NONE)) {
-		puts("FDT image is compressed");
-		return -EPROTONOSUPPORT;
-	}
-
 	bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL);
 	type_ok = fit_image_check_type(fit, noffset, image_type) ||
 		  fit_image_check_type(fit, noffset, IH_TYPE_FIRMWARE) ||
@@ -1931,7 +1934,8 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
 	bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL_OK);
 
 	/* get image data address and length */
-	if (fit_image_get_data_and_size(fit, noffset, &buf, &size)) {
+	if (fit_image_get_data_and_size(fit, noffset,
+					(const void **)&buf, &size)) {
 		printf("Could not find %s subimage data!\n", prop_name);
 		bootstage_error(bootstage_id + BOOTSTAGE_SUB_GET_DATA);
 		return -ENOENT;
@@ -1939,30 +1943,15 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
 
 #if !defined(USE_HOSTCC) && defined(CONFIG_FIT_IMAGE_POST_PROCESS)
 	/* perform any post-processing on the image data */
-	board_fit_image_post_process((void **)&buf, &size);
+	board_fit_image_post_process(&buf, &size);
 #endif
 
 	len = (ulong)size;
 
-	/* verify that image data is a proper FDT blob */
-	if (image_type == IH_TYPE_FLATDT && fdt_check_header(buf)) {
-		puts("Subimage data is not a FDT");
-		return -ENOEXEC;
-	}
-
 	bootstage_mark(bootstage_id + BOOTSTAGE_SUB_GET_DATA_OK);
 
-	/*
-	 * Work-around for eldk-4.2 which gives this warning if we try to
-	 * cast in the unmap_sysmem() call:
-	 * warning: initialization discards qualifiers from pointer target type
-	 */
-	{
-		void *vbuf = (void *)buf;
-
-		data = map_to_sysmem(vbuf);
-	}
-
+	data = map_to_sysmem(buf);
+	load = data;
 	if (load_op == FIT_LOAD_IGNORED) {
 		/* Don't load */
 	} else if (fit_image_get_load(fit, noffset, &load)) {
@@ -1974,8 +1963,6 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
 		}
 	} else if (load_op != FIT_LOAD_OPTIONAL_NON_ZERO || load) {
 		ulong image_start, image_end;
-		ulong load_end;
-		void *dst;
 
 		/*
 		 * move image data to the load address,
@@ -1993,14 +1980,44 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
 
 		printf("   Loading %s from 0x%08lx to 0x%08lx\n",
 		       prop_name, data, load);
+	}
 
-		dst = map_sysmem(load, len);
-		memmove(dst, buf, len);
-		data = load;
+	/*
+	 * Decompress FDTs right here. Kernel images get decompressed later in
+	 * bootm_host_load_image().
+	 */
+	comp = IH_COMP_NONE;
+	loadbuf = buf;
+	if (image_type == IH_TYPE_FLATDT &&
+	    !fit_image_get_comp(fit, noffset, &comp) &&
+	    comp != IH_COMP_NONE) {
+	    	ulong max_decomp_len = len * 20;
+		if (load == data) {
+			loadbuf = malloc(max_decomp_len);
+			load = map_to_sysmem(loadbuf);
+		} else {
+			loadbuf = map_sysmem(load, max_decomp_len);
+		}
+		if (bootm_decomp_image(comp, load, data, IH_TYPE_FLATDT,
+				loadbuf, buf, len, max_decomp_len, &load_end)) {
+			printf("Error: Cannot decompress FDT image\n");
+			return -ENOEXEC;
+		}
+		len = load_end - load;
+	} else if (load != data) {
+		loadbuf = map_sysmem(load, len);
+		memcpy(loadbuf, buf, len);
 	}
+
+	/* verify that image data is a proper FDT blob */
+	if (image_type == IH_TYPE_FLATDT && fdt_check_header(loadbuf)) {
+		puts("Subimage data is not a FDT");
+		return -ENOEXEC;
+	}
+
 	bootstage_mark(bootstage_id + BOOTSTAGE_SUB_LOAD);
 
-	*datap = data;
+	*datap = load;
 	*lenp = len;
 	if (fit_unamep)
 		*fit_unamep = (char *)fit_uname;
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 2/2] fit: Support compat string property in configuration node
  2019-04-18  1:22 [U-Boot] [PATCH 0/2] fit: FDT compression Julius Werner
  2019-04-18  1:22 ` [U-Boot] [PATCH 1/2] fit: Support " Julius Werner
@ 2019-04-18  1:22 ` Julius Werner
  1 sibling, 0 replies; 9+ messages in thread
From: Julius Werner @ 2019-04-18  1:22 UTC (permalink / raw)
  To: u-boot

This patch adds support for an optional optimization to compatible
string matching where the compatible string property from the root node
of the kernel FDT can be copied into the configuration node of the FIT
image. This is most useful when using compressed FDTs or when using FDT
overlays, where the traditional extraction of the compatible string from
the kernel FDT itself is not easily possible.

Signed-off-by: Julius Werner <jwerner@chromium.org>
---
 common/image-fit.c | 67 +++++++++++++++++++++++++++-------------------
 1 file changed, 39 insertions(+), 28 deletions(-)

diff --git a/common/image-fit.c b/common/image-fit.c
index 20e02d240b..dec55ffaec 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -1522,6 +1522,10 @@ int fit_check_format(const void *fit)
  * compatible list, "foo,bar", matches a compatible string in the root of fdt1.
  * "bim,bam" in fdt2 matches the second string which isn't as good as fdt1.
  *
+ * As an optimization, the compatible property from the FDT's root node can be
+ * copied into the configuration node in the FIT image. This is required to
+ * match configurations with compressed FDTs.
+ *
  * returns:
  *     offset to the configuration to use if one was found
  *     -1 otherwise
@@ -1554,55 +1558,62 @@ int fit_conf_find_compat(const void *fit, const void *fdt)
 	for (noffset = fdt_next_node(fit, confs_noffset, &ndepth);
 			(noffset >= 0) && (ndepth > 0);
 			noffset = fdt_next_node(fit, noffset, &ndepth)) {
-		const void *kfdt;
+		const void *fdt;
 		const char *kfdt_name;
-		int kfdt_noffset;
+		int kfdt_noffset, compat_noffset;
 		const char *cur_fdt_compat;
 		int len;
-		size_t size;
+		size_t sz;
 		int i;
 
 		if (ndepth > 1)
 			continue;
 
-		kfdt_name = fdt_getprop(fit, noffset, "fdt", &len);
-		if (!kfdt_name) {
-			debug("No fdt property found.\n");
-			continue;
-		}
-		kfdt_noffset = fdt_subnode_offset(fit, images_noffset,
-						  kfdt_name);
-		if (kfdt_noffset < 0) {
-			debug("No image node named \"%s\" found.\n",
-			      kfdt_name);
-			continue;
-		}
+		/* If there's a compat property in the config node, use that. */
+		if (fdt_getprop(fit, noffset, "compatible", NULL)) {
+			fdt = fit;		  /* search in FIT image */
+			compat_noffset = noffset; /* search under config node */
+		} else {	/* Otherwise extract it from the kernel FDT. */
+			kfdt_name = fdt_getprop(fit, noffset, "fdt", &len);
+			if (!kfdt_name) {
+				debug("No fdt property found.\n");
+				continue;
+			}
+			kfdt_noffset = fdt_subnode_offset(fit, images_noffset,
+							  kfdt_name);
+			if (kfdt_noffset < 0) {
+				debug("No image node named \"%s\" found.\n",
+				      kfdt_name);
+				continue;
+			}
 
-		if (!fit_image_check_comp(fit, kfdt_noffset, IH_COMP_NONE)) {
-			debug("Can't extract compat from \"%s\" (compressed)\n",
-			      kfdt_name);
-			continue;
-		}
+			if (!fit_image_check_comp(fit, kfdt_noffset,
+						  IH_COMP_NONE)) {
+				debug("Can't extract compat from \"%s\" "
+				      "(compressed)\n", kfdt_name);
+				continue;
+			}
 
-		/*
-		 * Get a pointer to this configuration's fdt.
-		 */
-		if (fit_image_get_data(fit, kfdt_noffset, &kfdt, &size)) {
-			debug("Failed to get fdt \"%s\".\n", kfdt_name);
-			continue;
+			/* search in this config's kernel FDT */
+			if (fit_image_get_data(fit, kfdt_noffset, &fdt, &sz)) {
+				debug("Failed to get fdt \"%s\".\n", kfdt_name);
+				continue;
+			}
+
+			compat_noffset = 0;  /* search kFDT under root node */
 		}
 
 		len = fdt_compat_len;
 		cur_fdt_compat = fdt_compat;
 		/*
 		 * Look for a match for each U-Boot compatibility string in
-		 * turn in this configuration's fdt.
+		 * turn in the compat string property.
 		 */
 		for (i = 0; len > 0 &&
 		     (!best_match_offset || best_match_pos > i); i++) {
 			int cur_len = strlen(cur_fdt_compat) + 1;
 
-			if (!fdt_node_check_compatible(kfdt, 0,
+			if (!fdt_node_check_compatible(fdt, compat_noffset,
 						       cur_fdt_compat)) {
 				best_match_offset = noffset;
 				best_match_pos = i;
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 1/2] fit: Support FDT compression
  2019-04-18  1:22 ` [U-Boot] [PATCH 1/2] fit: Support " Julius Werner
@ 2019-04-18  8:29   ` Simon Goldschmidt
  2019-04-18 19:59     ` Julius Werner
  0 siblings, 1 reply; 9+ messages in thread
From: Simon Goldschmidt @ 2019-04-18  8:29 UTC (permalink / raw)
  To: u-boot

Hi Julius,

On Thu, Apr 18, 2019 at 10:13 AM Julius Werner <jwerner@chromium.org> wrote:
>
> This patch adds support for compressing FDT image nodes in a FIT image
> (equivalent to how kernel nodes can already be compressed). This can
> reduce the size of FIT images (and therefore improve boot times). FDTs
> will automatically get decompressed on load.
>
> This patch does not support extracting compatible strings from
> compressed FDTs, so it's not very helpful in conjunction with
> CONFIG_FIT_BEST_MATCH yet, but it can already be used in environments
> that select the configuration to load explicitly.
>
> Signed-off-by: Julius Werner <jwerner@chromium.org>

I was working on a patch that allows *all* items loaded from a FIT image to
be compressed. This is especially useful on my platform where FPGA images
(quite big) are embedded in the FIT image.

I think it would make more sense to make the decompression more generic
instead of adding yet another specific case for FDT decompression.

Unfortunately, I haven't found the time to finish my patch for this, yet...

Regards,
Simon

> ---
>  common/image-fit.c | 85 +++++++++++++++++++++++++++-------------------
>  1 file changed, 51 insertions(+), 34 deletions(-)
>
> diff --git a/common/image-fit.c b/common/image-fit.c
> index ac901e131c..20e02d240b 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -22,6 +22,7 @@
>  DECLARE_GLOBAL_DATA_PTR;
>  #endif /* !USE_HOSTCC*/
>
> +#include <bootm.h>
>  #include <image.h>
>  #include <bootstage.h>
>  #include <u-boot/crc.h>
> @@ -1576,6 +1577,13 @@ int fit_conf_find_compat(const void *fit, const void *fdt)
>                               kfdt_name);
>                         continue;
>                 }
> +
> +               if (!fit_image_check_comp(fit, kfdt_noffset, IH_COMP_NONE)) {
> +                       debug("Can't extract compat from \"%s\" (compressed)\n",
> +                             kfdt_name);
> +                       continue;
> +               }
> +
>                 /*
>                  * Get a pointer to this configuration's fdt.
>                  */
> @@ -1795,11 +1803,12 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
>         const char *fit_uname_config;
>         const char *fit_base_uname_config;
>         const void *fit;
> -       const void *buf;
> +       void *buf;
> +       void *loadbuf;
>         size_t size;
>         int type_ok, os_ok;
> -       ulong load, data, len;
> -       uint8_t os;
> +       ulong load, load_end, data, len;
> +       uint8_t os, comp;
>  #ifndef USE_HOSTCC
>         uint8_t os_arch;
>  #endif
> @@ -1895,12 +1904,6 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
>         images->os.arch = os_arch;
>  #endif
>
> -       if (image_type == IH_TYPE_FLATDT &&
> -           !fit_image_check_comp(fit, noffset, IH_COMP_NONE)) {
> -               puts("FDT image is compressed");
> -               return -EPROTONOSUPPORT;
> -       }
> -
>         bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL);
>         type_ok = fit_image_check_type(fit, noffset, image_type) ||
>                   fit_image_check_type(fit, noffset, IH_TYPE_FIRMWARE) ||
> @@ -1931,7 +1934,8 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
>         bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL_OK);
>
>         /* get image data address and length */
> -       if (fit_image_get_data_and_size(fit, noffset, &buf, &size)) {
> +       if (fit_image_get_data_and_size(fit, noffset,
> +                                       (const void **)&buf, &size)) {
>                 printf("Could not find %s subimage data!\n", prop_name);
>                 bootstage_error(bootstage_id + BOOTSTAGE_SUB_GET_DATA);
>                 return -ENOENT;
> @@ -1939,30 +1943,15 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
>
>  #if !defined(USE_HOSTCC) && defined(CONFIG_FIT_IMAGE_POST_PROCESS)
>         /* perform any post-processing on the image data */
> -       board_fit_image_post_process((void **)&buf, &size);
> +       board_fit_image_post_process(&buf, &size);
>  #endif
>
>         len = (ulong)size;
>
> -       /* verify that image data is a proper FDT blob */
> -       if (image_type == IH_TYPE_FLATDT && fdt_check_header(buf)) {
> -               puts("Subimage data is not a FDT");
> -               return -ENOEXEC;
> -       }
> -
>         bootstage_mark(bootstage_id + BOOTSTAGE_SUB_GET_DATA_OK);
>
> -       /*
> -        * Work-around for eldk-4.2 which gives this warning if we try to
> -        * cast in the unmap_sysmem() call:
> -        * warning: initialization discards qualifiers from pointer target type
> -        */
> -       {
> -               void *vbuf = (void *)buf;
> -
> -               data = map_to_sysmem(vbuf);
> -       }
> -
> +       data = map_to_sysmem(buf);
> +       load = data;
>         if (load_op == FIT_LOAD_IGNORED) {
>                 /* Don't load */
>         } else if (fit_image_get_load(fit, noffset, &load)) {
> @@ -1974,8 +1963,6 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
>                 }
>         } else if (load_op != FIT_LOAD_OPTIONAL_NON_ZERO || load) {
>                 ulong image_start, image_end;
> -               ulong load_end;
> -               void *dst;
>
>                 /*
>                  * move image data to the load address,
> @@ -1993,14 +1980,44 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
>
>                 printf("   Loading %s from 0x%08lx to 0x%08lx\n",
>                        prop_name, data, load);
> +       }
>
> -               dst = map_sysmem(load, len);
> -               memmove(dst, buf, len);
> -               data = load;
> +       /*
> +        * Decompress FDTs right here. Kernel images get decompressed later in
> +        * bootm_host_load_image().
> +        */
> +       comp = IH_COMP_NONE;
> +       loadbuf = buf;
> +       if (image_type == IH_TYPE_FLATDT &&
> +           !fit_image_get_comp(fit, noffset, &comp) &&
> +           comp != IH_COMP_NONE) {
> +               ulong max_decomp_len = len * 20;
> +               if (load == data) {
> +                       loadbuf = malloc(max_decomp_len);
> +                       load = map_to_sysmem(loadbuf);
> +               } else {
> +                       loadbuf = map_sysmem(load, max_decomp_len);
> +               }
> +               if (bootm_decomp_image(comp, load, data, IH_TYPE_FLATDT,
> +                               loadbuf, buf, len, max_decomp_len, &load_end)) {
> +                       printf("Error: Cannot decompress FDT image\n");
> +                       return -ENOEXEC;
> +               }
> +               len = load_end - load;
> +       } else if (load != data) {
> +               loadbuf = map_sysmem(load, len);
> +               memcpy(loadbuf, buf, len);
>         }
> +
> +       /* verify that image data is a proper FDT blob */
> +       if (image_type == IH_TYPE_FLATDT && fdt_check_header(loadbuf)) {
> +               puts("Subimage data is not a FDT");
> +               return -ENOEXEC;
> +       }
> +
>         bootstage_mark(bootstage_id + BOOTSTAGE_SUB_LOAD);
>
> -       *datap = data;
> +       *datap = load;
>         *lenp = len;
>         if (fit_unamep)
>                 *fit_unamep = (char *)fit_uname;
> --
> 2.20.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 1/2] fit: Support FDT compression
  2019-04-18  8:29   ` Simon Goldschmidt
@ 2019-04-18 19:59     ` Julius Werner
  2019-04-18 20:06       ` Simon Goldschmidt
  0 siblings, 1 reply; 9+ messages in thread
From: Julius Werner @ 2019-04-18 19:59 UTC (permalink / raw)
  To: u-boot

Hi Simon,

Is your approach similar to what I did here (decompressing
transparently as part of fit_image_load())? I think I could easily
expand this to other image types, I just don't always know how to test
those. Really, the only thing that can't be decompressed there is the
kernel image (because that is done later), so rather than checking for
IH_TYPE_FLATDT I could check for !(IH_TYPE_KERNEL ||
IH_TYPE_KERNEL_NOLOAD)?

Alternatively, maybe we could take this as a first step and it can
later be expanded to support other types when someone has time to work
on that?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 1/2] fit: Support FDT compression
  2019-04-18 19:59     ` Julius Werner
@ 2019-04-18 20:06       ` Simon Goldschmidt
  2019-04-18 20:36         ` Julius Werner
  0 siblings, 1 reply; 9+ messages in thread
From: Simon Goldschmidt @ 2019-04-18 20:06 UTC (permalink / raw)
  To: u-boot



On 18.04.19 21:59, Julius Werner wrote:
> Hi Simon,
> 
> Is your approach similar to what I did here (decompressing
> transparently as part of fit_image_load())? I think I could easily
> expand this to other image types, I just don't always know how to test
> those. Really, the only thing that can't be decompressed there is the
> kernel image (because that is done later), so rather than checking for
> IH_TYPE_FLATDT I could check for !(IH_TYPE_KERNEL ||
> IH_TYPE_KERNEL_NOLOAD)?

My approach was to uncompress all compressed images on-the-fly in 
fit_image_load(). I also hadn't fixed the kernel issue, so maybe 
deferring uncompression of kernel would be a good enough fix.

We had discussed that back then (around october'18) and I think we got 
to the point that it would be OK to use the "compressed" flag as a hint 
for U-Boot to uncompress things (meaning that a compressed initrd that 
should be passed to kernel compressed should not have the "compressed" 
flag in the FIT).

> Alternatively, maybe we could take this as a first step and it can
> later be expanded to support other types when someone has time to work
> on that?

Or I could dig up my patches from October and we'll see how far you get 
with those?

Regards,
Simon

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 1/2] fit: Support FDT compression
  2019-04-18 20:06       ` Simon Goldschmidt
@ 2019-04-18 20:36         ` Julius Werner
  2019-04-18 20:48           ` Simon Goldschmidt
  0 siblings, 1 reply; 9+ messages in thread
From: Julius Werner @ 2019-04-18 20:36 UTC (permalink / raw)
  To: u-boot

> My approach was to uncompress all compressed images on-the-fly in
> fit_image_load().

Right, that's essentially what this patch is doing too.

> Or I could dig up my patches from October and we'll see how far you get
> with those?

I think I found your patch:
https://lists.denx.de/pipermail/u-boot/2018-October/344673.html

It seems to be doing something very close to what my patch does
anyway. My patch goes a little further by also solving the case when
no load address is given (in that case it will malloc() a buffer to
decompress into with an upper bound guess for the required size). If
there is a load size given, the two should end up doing the same
thing. Also your patch works on all image types, which as you said
there breaks it for kernels. I think the option of doing it for all
image types except kernels would be a good solution for everyone.
(Ultimately, I think it might be nicer if the kernel decompression
would also be handled there and not as a special case, but I'd rather
not tackle everything at once. This can always be iterated upon in the
future.)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 1/2] fit: Support FDT compression
  2019-04-18 20:36         ` Julius Werner
@ 2019-04-18 20:48           ` Simon Goldschmidt
  2019-04-18 21:09             ` Julius Werner
  0 siblings, 1 reply; 9+ messages in thread
From: Simon Goldschmidt @ 2019-04-18 20:48 UTC (permalink / raw)
  To: u-boot



On 18.04.19 22:36, Julius Werner wrote:
>> My approach was to uncompress all compressed images on-the-fly in
>> fit_image_load().
> 
> Right, that's essentially what this patch is doing too.

Cool. I'm sorry I haven't found the time to dig into your patch for 
details (too much day-to-day work right now).

> 
>> Or I could dig up my patches from October and we'll see how far you get
>> with those?
> 
> I think I found your patch:
> https://lists.denx.de/pipermail/u-boot/2018-October/344673.html

Exactly. I do have some further versions locally, but no real 
breakthrough I think.

> It seems to be doing something very close to what my patch does
> anyway. My patch goes a little further by also solving the case when
> no load address is given (in that case it will malloc() a buffer to
> decompress into with an upper bound guess for the required size).

Hmm, I think we might want to use the lmb functions here to allocate a 
buffer instead of relyling on malloc? The malloc pool might be large 
enough for an uncompressed devicetree, but not for an 8 MByte FPGA image...

But starting with malloc might be ok.

> If there is a load size given, the two should end up doing the same
> thing. Also your patch works on all image types, which as you said
> there breaks it for kernels. I think the option of doing it for all
> image types except kernels would be a good solution for everyone.
> (Ultimately, I think it might be nicer if the kernel decompression
> would also be handled there and not as a special case, but I'd rather
> not tackle everything at once. This can always be iterated upon in the
> future.)

The reason I discontinued that patch was that I started adding a feature 
to mkimage to add a property for the uncompressed size. This is still 
pending work I haven't sent to the ML, but I do want to continue it once 
I find the time.

So maybe we could move on with a v2 of your patch that uncompresses 
everything but the kernel? I'd like to test that with my compressed FPGA 
images then.

Regards,
Simon

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 1/2] fit: Support FDT compression
  2019-04-18 20:48           ` Simon Goldschmidt
@ 2019-04-18 21:09             ` Julius Werner
  0 siblings, 0 replies; 9+ messages in thread
From: Julius Werner @ 2019-04-18 21:09 UTC (permalink / raw)
  To: u-boot

> Hmm, I think we might want to use the lmb functions here to allocate a
> buffer instead of relyling on malloc? The malloc pool might be large
> enough for an uncompressed devicetree, but not for an 8 MByte FPGA image...
>
> But starting with malloc might be ok.

Okay, that sounds like a good possible improvement for later. For now,
you can always just specify a load address for the FPGA image so that
it doesn't need to allocate a separate buffer.

> The reason I discontinued that patch was that I started adding a feature
> to mkimage to add a property for the uncompressed size. This is still
> pending work I haven't sent to the ML, but I do want to continue it once
> I find the time.

Yeah, that would make all of this a lot cleaner.

> So maybe we could move on with a v2 of your patch that uncompresses
> everything but the kernel? I'd like to test that with my compressed FPGA
> images then.

Sounds good, v2 sent out.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-04-18 21:09 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-18  1:22 [U-Boot] [PATCH 0/2] fit: FDT compression Julius Werner
2019-04-18  1:22 ` [U-Boot] [PATCH 1/2] fit: Support " Julius Werner
2019-04-18  8:29   ` Simon Goldschmidt
2019-04-18 19:59     ` Julius Werner
2019-04-18 20:06       ` Simon Goldschmidt
2019-04-18 20:36         ` Julius Werner
2019-04-18 20:48           ` Simon Goldschmidt
2019-04-18 21:09             ` Julius Werner
2019-04-18  1:22 ` [U-Boot] [PATCH 2/2] fit: Support compat string property in configuration node Julius Werner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.