Am 10.04.20 um 13:04 schrieb Stefan Roese:

From: Weijie Gao weijie.gao@mediatek.com

This patch adds support for decompressing LZMA compressed u-boot payload in legacy uImage format.

Using this patch together with u-boot-lzma.img may be useful for some platforms as they can reduce the size and load time of u-boot payload.

Signed-off-by: Weijie Gao weijie.gao@mediatek.com Signed-off-by: Stefan Roese sr@denx.de Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com

---

common/spl/spl_legacy.c | 50 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-)

Reviewed-by: Daniel Schwierzeck daniel.schwierzeck@gmail.com

nits below

diff --git a/common/spl/spl_legacy.c b/common/spl/spl_legacy.c index 7f00fc8885..41734c026f 100644 --- a/common/spl/spl_legacy.c +++ b/common/spl/spl_legacy.c @@ -4,8 +4,15 @@ */

#include <common.h> +#include <malloc.h> #include <spl.h>

+#include <lzma/LzmaTypes.h> +#include <lzma/LzmaDec.h> +#include <lzma/LzmaTools.h>

+#define LZMA_LEN (1 << 20)

int spl_parse_legacy_header(struct spl_image_info *spl_image, const struct image_header *header) { @@ -55,7 +62,10 @@ int spl_parse_legacy_header(struct spl_image_info *spl_image, int spl_load_legacy_img(struct spl_image_info *spl_image, struct spl_load_info *load, ulong header) {

• __maybe_unused SizeT lzma_len;

• __maybe_unused void *src; struct image_header hdr;

• ulong dataptr; int ret;

  /* Read header into local struct */

@@ -65,9 +75,45 @@ int spl_load_legacy_img(struct spl_image_info *spl_image, if (ret) return ret;

• dataptr = header + sizeof(hdr);
• /* Read image */

• load->read(load, header + sizeof(hdr), spl_image->size,

•    (void *)(unsigned long)spl_image->load_addr);
  

• switch (image_get_comp(&hdr)) {
• case IH_COMP_NONE:

• load->read(load, dataptr, spl_image->size,
  

• 	   (void *)(unsigned long)spl_image->load_addr);
  

• break;
  

to avoid the little increase of binary footprint due to the extra check maybe a little wrapper like this could help:

static inline int spl_image_get_comp(const struct image_header *hdr) { if (IS_ENABLED(CONFIG_SPL_LZMA) /* || IS_ENABLED(CONFIG_SPL_ANOTHER_FANCY_COMPRESSION) */) return image_get_comp(hdr);

return IH_COMP_NONE; }

switch (spl_image_get_comp(&hdr)) { ... }

then the compiler should optimise the switch/case statement away due to Dead Code Elimination.

+#if IS_ENABLED(CONFIG_SPL_LZMA)

• case IH_COMP_LZMA:

• lzma_len = LZMA_LEN;
  

• debug("LZMA: Decompressing %08lx to %08lx\n",
  

•       dataptr, spl_image->load_addr);
  

• src = malloc(spl_image->size);
  

• if (!src) {
  

• 	printf("Unable to allocate %d bytes for LZMA\n",
  

• 	       spl_image->size);
  

• 	return -ENOMEM;
  

• }
  

• load->read(load, dataptr, spl_image->size, src);
  

• ret = lzmaBuffToBuffDecompress((void *)spl_image->load_addr,
  

• 			       &lzma_len, src, spl_image->size);
  

• if (ret) {
  

• 	printf("LZMA decompression error: %d\n", ret);
  

• 	return ret;
  

• }
  

• spl_image->size = lzma_len;
  

• break;
  

+#endif

• default:

• debug("Compression method %s is not supported\n",
  

•       genimg_get_comp_short_name(image_get_comp(&hdr)));
  

• return -EINVAL;
  

• }

  return 0;

}

On 15.04.20 13:43, Daniel Schwierzeck wrote:

Am 10.04.20 um 13:04 schrieb Stefan Roese:

...

This patch adds a MIPS specific jump_to_image_no_args() implementation, which flushes the U-Boot proper image loaded from the boot device in SPL before jumping to it.

It has been noticed on MT76x8, that this cache flush is needed. Other MIPS platforms might need it as well.

Signed-off-by: Stefan Roese sr@denx.de Cc: Weijie Gao weijie.gao@mediatek.com Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com

---

arch/mips/lib/boot.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)

diff --git a/arch/mips/lib/boot.c b/arch/mips/lib/boot.c index db862f6379..bc620abd9b 100644 --- a/arch/mips/lib/boot.c +++ b/arch/mips/lib/boot.c @@ -6,6 +6,7 @@ #include <common.h> #include <command.h> #include <cpu_func.h> +#include <spl.h>

DECLARE_GLOBAL_DATA_PTR;

@@ -21,3 +22,16 @@ unsigned long do_go_exec(ulong (*entry)(int, char * const []),

return entry(argc, argv); }

+void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image) +{

• typedef void __noreturn (*image_entry_noargs_t)(void);
• image_entry_noargs_t image_entry =

• (image_entry_noargs_t)spl_image->entry_point;
  

• /* Flush cache before jumping to application */
• flush_cache((unsigned long)spl_image->load_addr, spl_image->size);
• debug("image entry point: 0x%lx\n", spl_image->entry_point);
• image_entry();

+}

the function itself looks good. But arch/mips/lib/boot.c depends on CONFIG_CMD_GO (from Makefile: obj-$(CONFIG_CMD_GO) += boot.o)

I think it's better if we establish an arch/mips/lib/spl.c for generic but MIPS specific SPL stuff and wrap it with CONFIG_SPL_BUILD.

Yes, makes sense. I'll make this change in v7 of the patchset.

Thanks, Stefan

On 15.04.20 13:46, Daniel Schwierzeck wrote:

Am 10.04.20 um 13:04 schrieb Stefan Roese:

...

Move the legacy image loading into spl_legacy.c. This makes it easier to extend the legacy image handling with new features that other SPL loaders might use (e.g. spl_spi.c etc).

No functional change intended.

Signed-off-by: Stefan Roese sr@denx.de Cc: Weijie Gao weijie.gao@mediatek.com Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com

---

RFC comment: I'm sendig these 3 patches as RFC and once we've come to an agreement on these (Acked etc), I'll integrate them into the mtmips SPL patchset from Weijie and will send v7.

Thanks, Stefan

common/spl/spl_legacy.c | 20 ++++++++++++++++++++ common/spl/spl_nor.c | 27 +++++++++++++-------------- include/spl.h | 13 +++++++++++++ 3 files changed, 46 insertions(+), 14 deletions(-)

Reviewed-by: Daniel Schwierzeck daniel.schwierzeck@gmail.com

nits below

...

diff --git a/common/spl/spl_legacy.c b/common/spl/spl_legacy.c index 772135193e..7f00fc8885 100644 --- a/common/spl/spl_legacy.c +++ b/common/spl/spl_legacy.c @@ -51,3 +51,23 @@ int spl_parse_legacy_header(struct spl_image_info *spl_image,

return 0; }

+int spl_load_legacy_img(struct spl_image_info *spl_image,

• 	struct spl_load_info *load, ulong header)
  

+{

• struct image_header hdr;
• int ret;
• /* Read header into local struct */
• load->read(load, header, sizeof(hdr), &hdr);
• ret = spl_parse_image_header(spl_image, &hdr);
• if (ret)

• return ret;
  

• /* Read image */
• load->read(load, header + sizeof(hdr), spl_image->size,

•    (void *)(unsigned long)spl_image->load_addr);
  

• return 0;

+} diff --git a/common/spl/spl_nor.c b/common/spl/spl_nor.c index b1e79b9ded..3f03ffe6a3 100644 --- a/common/spl/spl_nor.c +++ b/common/spl/spl_nor.c @@ -24,7 +24,6 @@ unsigned long __weak spl_nor_get_uboot_base(void) static int spl_nor_load_image(struct spl_image_info *spl_image, struct spl_boot_device *bootdev) {

• int ret; __maybe_unused const struct image_header *header; __maybe_unused struct spl_load_info load;

@@ -43,6 +42,8 @@ static int spl_nor_load_image(struct spl_image_info *spl_image, header = (const struct image_header *)CONFIG_SYS_OS_BASE; #ifdef CONFIG_SPL_LOAD_FIT if (image_get_magic(header) == FDT_MAGIC) {

• 	int ret;
  

• debug("Found FIT\n");
  load.bl_len = 1;
  load.read = spl_nor_load_read;
  

@@ -61,6 +62,7 @@ static int spl_nor_load_image(struct spl_image_info *spl_image, #endif if (image_get_os(header) == IH_OS_LINUX) { /* happy - was a Linux */

• 	int ret;
  
  ret = spl_parse_image_header(spl_image, header);
  if (ret)
  

@@ -93,11 +95,9 @@ static int spl_nor_load_image(struct spl_image_info *spl_image, debug("Found FIT format U-Boot\n"); load.bl_len = 1; load.read = spl_nor_load_read;

• ret = spl_load_simple_fit(spl_image, &load,
  

• 			  spl_nor_get_uboot_base(),
  

• 			  (void *)header);
  

• return ret;
  

• return spl_load_simple_fit(spl_image, &load,
  

• 			   spl_nor_get_uboot_base(),
  

• 			   (void *)header);
  

looks like an unrelated optimisation

It looks this way. But its related: This is needed, as the if statement below now results in a warning when the usage of the "ret" variable is not changed:

common/spl/spl_nor.c:27:6: warning: unused variable 'ret' [-Wunused-variable]

That's why I removed "ret" completely in this patch.

I could either generate a small patch "before" this one to remove "ret" or mention this removal in the commit text of this patch.

Thanks, Stefan

...

} #endif if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER)) { @@ -107,14 +107,13 @@ static int spl_nor_load_image(struct spl_image_info *spl_image, spl_nor_get_uboot_base()); }

• ret = spl_parse_image_header(spl_image,

• 	(const struct image_header *)spl_nor_get_uboot_base());
  

• if (ret)

• return ret;
  

• memcpy((void *)(unsigned long)spl_image->load_addr,

•       (void *)(spl_nor_get_uboot_base() + sizeof(struct image_header)),
  

•       spl_image->size);
  

• /* Legacy image handling */

• if (IS_ENABLED(CONFIG_SPL_LEGACY_IMAGE_SUPPORT)) {

• load.bl_len = 1;
  

• load.read = spl_nor_load_read;
  

• return spl_load_legacy_img(spl_image, &load,
  

• 			   spl_nor_get_uboot_base());
  

• }

  return 0; }

diff --git a/include/spl.h b/include/spl.h index 6087cd793c..c6c64b6a72 100644 --- a/include/spl.h +++ b/include/spl.h @@ -219,6 +219,19 @@ int spl_load_simple_fit(struct spl_image_info *spl_image, #define SPL_COPY_PAYLOAD_ONLY 1 #define SPL_FIT_FOUND 2

+/**

• • spl_load_legacy_img() - Loads a legacy image from a device.
• • @spl_image: Image description to set up
• • @load: Structure containing the information required to load data.
• • @header: Pointer to image header (including appended image)
• • Reads an legacy image from the device. Loads u-boot image to
• • specified load address.
• • Returns 0 on success.
• */

+int spl_load_legacy_img(struct spl_image_info *spl_image,

• 	struct spl_load_info *load, ulong header);
  

• /**
  • spl_load_imx_container() - Loads a imx container image from a device.
  • @spl_image: Image description to set up

Viele Grüße, Stefan