[U-Boot] [PATCH 00/14] mkimage: Tidy up error handling

[U-Boot] [PATCH 00/14] mkimage: Tidy up error handling

[Simon Glass]

There are a few problems when mkimage is provided with invalid arguments.
In one case it crashes. When an invalid image type it is provided it lists
the valid types, but this is not implemented for compression, architecture
or OS.

This series tidies this up a little, to make mkimage more friendly.


Simon Glass (14):
  mkimage: Honour the default image type with auto-fit
  mkimage: Explain the auto-fit imagefile special case
  mkimage: Require a data file when auto-fit is used
  mkimage: Drop premature setting of params.fit_image_type
  mkimage: Drop blank line before main()
  image: Correct auto-fit architecture property name
  image: Convert the IH_... values to enums
  image: Create a table of information for each category
  image: Add a name for invalid types
  image: Add functions to obtain category information
  mkimage: Allow display of a list of any image header category
  mkimage: Use generic code for showing an 'image type' error
  mkimage: Show item lists for all categories
  tools: Allow building with debug enabled

 Kconfig           |   9 +++
 Makefile          |   3 +-
 common/image.c    |  87 ++++++++++++++++++++-
 include/image.h   | 230 ++++++++++++++++++++++++++++++++++--------------------
 tools/fit_image.c |   3 +-
 tools/mkimage.c   |  69 +++++++++-------
 6 files changed, 280 insertions(+), 121 deletions(-)

-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 01/14] mkimage: Honour the default image type with auto-fit

[Simon Glass]

The default image type is supposed to be IH_TYPE_KERNEL, as set in the
'params' variable. Honour this with auto-fit also.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/mkimage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/mkimage.c b/tools/mkimage.c
index ff3024a..53fa8bb 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -272,7 +272,7 @@ static void process_args(int argc, char **argv)
 	 * will always be IH_TYPE_FLATDT in this case).
 	 */
 	if (params.type == IH_TYPE_FLATDT) {
-		params.fit_image_type = type;
+		params.fit_image_type = type ? type : IH_TYPE_KERNEL;
 		if (!params.auto_its)
 			params.datafile = datafile;
 	} else if (type != IH_TYPE_INVALID) {
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 01/14] mkimage: Honour the default image type with auto-fit

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:07AM -0600, Simon Glass wrote:

> The default image type is supposed to be IH_TYPE_KERNEL, as set in the
> 'params' variable. Honour this with auto-fit also.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/f16fbdfb/attachment.sig>

[U-Boot] [U-Boot, 01/14] mkimage: Honour the default image type with auto-fit

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:07AM -0600, Simon Glass wrote:

> The default image type is supposed to be IH_TYPE_KERNEL, as set in the
> 'params' variable. Honour this with auto-fit also.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/49a75526/attachment.sig>

[U-Boot] [PATCH 02/14] mkimage: Explain the auto-fit imagefile special case

[Simon Glass]

There is a special case in the code when auto-fit is used. Add a comment to
make it easier to understand why this is needed.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/mkimage.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/mkimage.c b/tools/mkimage.c
index 53fa8bb..66d29ab 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -273,6 +273,7 @@ static void process_args(int argc, char **argv)
 	 */
 	if (params.type == IH_TYPE_FLATDT) {
 		params.fit_image_type = type ? type : IH_TYPE_KERNEL;
+		/* For auto_its, datafile is always 'auto' */
 		if (!params.auto_its)
 			params.datafile = datafile;
 	} else if (type != IH_TYPE_INVALID) {
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 02/14] mkimage: Explain the auto-fit imagefile special case

[Joe Hershberger]

On Thu, Jun 30, 2016 at 11:52 AM, Simon Glass <sjg@chromium.org> wrote:
> There is a special case in the code when auto-fit is used. Add a comment to
> make it easier to understand why this is needed.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [U-Boot, 02/14] mkimage: Explain the auto-fit imagefile special case

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:08AM -0600, Simon Glass wrote:

> There is a special case in the code when auto-fit is used. Add a comment to
> make it easier to understand why this is needed.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Acked-by: Joe Hershberger <joe.hershberger@ni.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/58b728ce/attachment.sig>

[U-Boot] [PATCH 03/14] mkimage: Require a data file when auto-fit is used

[Simon Glass]

When auto-fit is used, it is not valid to create a FIT without an image
file. Add a check for this to avoid a very confusing error message later
("Can't open (null): Bad address").

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/mkimage.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/mkimage.c b/tools/mkimage.c
index 66d29ab..a36c031 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -276,6 +276,8 @@ static void process_args(int argc, char **argv)
 		/* For auto_its, datafile is always 'auto' */
 		if (!params.auto_its)
 			params.datafile = datafile;
+		else if (!params.datafile)
+			usage("Missing data file for auto-FIT (use -d)");
 	} else if (type != IH_TYPE_INVALID) {
 		params.type = type;
 	}
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 03/14] mkimage: Require a data file when auto-fit is used

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:09AM -0600, Simon Glass wrote:

> When auto-fit is used, it is not valid to create a FIT without an image
> file. Add a check for this to avoid a very confusing error message later
> ("Can't open (null): Bad address").
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/bd9a31e5/attachment.sig>

[U-Boot] [U-Boot, 03/14] mkimage: Require a data file when auto-fit is used

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:09AM -0600, Simon Glass wrote:

> When auto-fit is used, it is not valid to create a FIT without an image
> file. Add a check for this to avoid a very confusing error message later
> ("Can't open (null): Bad address").
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/881f3935/attachment.sig>

[U-Boot] [PATCH 04/14] mkimage: Drop premature setting of params.fit_image_type

[Simon Glass]

There is no need to set params.fit_image_type while parsing the arguments.
It is set up later anyway.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/mkimage.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/mkimage.c b/tools/mkimage.c
index a36c031..76ae09e 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -197,7 +197,6 @@ static void process_args(int argc, char **argv)
 			 * The flattened image tree (FIT) format
 			 * requires a flattened device tree image type
 			 */
-			params.fit_image_type = params.type;
 			params.type = IH_TYPE_FLATDT;
 			params.fflag = 1;
 			break;
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 04/14] mkimage: Drop premature setting of params.fit_image_type

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:10AM -0600, Simon Glass wrote:

> There is no need to set params.fit_image_type while parsing the arguments.
> It is set up later anyway.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/08c49643/attachment.sig>

[U-Boot] [U-Boot, 04/14] mkimage: Drop premature setting of params.fit_image_type

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:10AM -0600, Simon Glass wrote:

> There is no need to set params.fit_image_type while parsing the arguments.
> It is set up later anyway.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/d4de3ba5/attachment.sig>

[U-Boot] [PATCH 05/14] mkimage: Drop blank line before main()

[Simon Glass]

This is not needed. Drop it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/mkimage.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/mkimage.c b/tools/mkimage.c
index 76ae09e..920d3be 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -285,7 +285,6 @@ static void process_args(int argc, char **argv)
 		usage("Missing output filename");
 }
 
-
 int main(int argc, char **argv)
 {
 	int ifd = -1;
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 05/14] mkimage: Drop blank line before main()

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:11AM -0600, Simon Glass wrote:

> This is not needed. Drop it.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/8b4ea716/attachment.sig>

[U-Boot] [U-Boot,05/14] mkimage: Drop blank line before main()

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:11AM -0600, Simon Glass wrote:

> This is not needed. Drop it.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/684a7931/attachment.sig>

[U-Boot] [PATCH 06/14] image: Correct auto-fit architecture property name

[Simon Glass]

The fit_write_images() function incorrectly uses the long name for the
architecture. This cannot be parsed with the FIT is read. Fix this by using
the short name instead.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/fit_image.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/fit_image.c b/tools/fit_image.c
index 58aa8e2..94229b8 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -195,7 +195,8 @@ static int fit_write_images(struct image_tool_params *params, char *fdt)
 	fdt_begin_node(fdt, str);
 	fdt_property_string(fdt, "description", params->imagename);
 	fdt_property_string(fdt, "type", typename);
-	fdt_property_string(fdt, "arch", genimg_get_arch_name(params->arch));
+	fdt_property_string(fdt, "arch",
+			    genimg_get_arch_short_name(params->arch));
 	fdt_property_string(fdt, "os", genimg_get_os_short_name(params->os));
 	fdt_property_string(fdt, "compression",
 			    genimg_get_comp_short_name(params->comp));
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 06/14] image: Correct auto-fit architecture property name

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:12AM -0600, Simon Glass wrote:

> The fit_write_images() function incorrectly uses the long name for the
> architecture. This cannot be parsed with the FIT is read. Fix this by using
> the short name instead.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/221b98ca/attachment.sig>

[U-Boot] [U-Boot, 06/14] image: Correct auto-fit architecture property name

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:12AM -0600, Simon Glass wrote:

> The fit_write_images() function incorrectly uses the long name for the
> architecture. This cannot be parsed with the FIT is read. Fix this by using
> the short name instead.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/fd55bc94/attachment.sig>

[U-Boot] [PATCH 07/14] image: Convert the IH_... values to enums

[Simon Glass]

We need to know the number of values of each category (architecture,
compression, OS and image type). To make this value easier to maintain,
convert all values to enums. The count is then automatic.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/image.h | 187 ++++++++++++++++++++++++++++++--------------------------
 1 file changed, 101 insertions(+), 86 deletions(-)

diff --git a/include/image.h b/include/image.h
index d788c26..7717b3d 100644
--- a/include/image.h
+++ b/include/image.h
@@ -126,59 +126,67 @@ struct lmb;
 /*
  * Operating System Codes
  */
-#define IH_OS_INVALID		0	/* Invalid OS	*/
-#define IH_OS_OPENBSD		1	/* OpenBSD	*/
-#define IH_OS_NETBSD		2	/* NetBSD	*/
-#define IH_OS_FREEBSD		3	/* FreeBSD	*/
-#define IH_OS_4_4BSD		4	/* 4.4BSD	*/
-#define IH_OS_LINUX		5	/* Linux	*/
-#define IH_OS_SVR4		6	/* SVR4		*/
-#define IH_OS_ESIX		7	/* Esix		*/
-#define IH_OS_SOLARIS		8	/* Solaris	*/
-#define IH_OS_IRIX		9	/* Irix		*/
-#define IH_OS_SCO		10	/* SCO		*/
-#define IH_OS_DELL		11	/* Dell		*/
-#define IH_OS_NCR		12	/* NCR		*/
-#define IH_OS_LYNXOS		13	/* LynxOS	*/
-#define IH_OS_VXWORKS		14	/* VxWorks	*/
-#define IH_OS_PSOS		15	/* pSOS		*/
-#define IH_OS_QNX		16	/* QNX		*/
-#define IH_OS_U_BOOT		17	/* Firmware	*/
-#define IH_OS_RTEMS		18	/* RTEMS	*/
-#define IH_OS_ARTOS		19	/* ARTOS	*/
-#define IH_OS_UNITY		20	/* Unity OS	*/
-#define IH_OS_INTEGRITY		21	/* INTEGRITY	*/
-#define IH_OS_OSE		22	/* OSE		*/
-#define IH_OS_PLAN9		23	/* Plan 9	*/
-#define IH_OS_OPENRTOS		24	/* OpenRTOS	*/
+enum {
+	IH_OS_INVALID		= 0,	/* Invalid OS	*/
+	IH_OS_OPENBSD,			/* OpenBSD	*/
+	IH_OS_NETBSD,			/* NetBSD	*/
+	IH_OS_FREEBSD,			/* FreeBSD	*/
+	IH_OS_4_4BSD,			/* 4.4BSD	*/
+	IH_OS_LINUX,			/* Linux	*/
+	IH_OS_SVR4,			/* SVR4		*/
+	IH_OS_ESIX,			/* Esix		*/
+	IH_OS_SOLARIS,			/* Solaris	*/
+	IH_OS_IRIX,			/* Irix		*/
+	IH_OS_SCO,			/* SCO		*/
+	IH_OS_DELL,			/* Dell		*/
+	IH_OS_NCR,			/* NCR		*/
+	IH_OS_LYNXOS,			/* LynxOS	*/
+	IH_OS_VXWORKS,			/* VxWorks	*/
+	IH_OS_PSOS,			/* pSOS		*/
+	IH_OS_QNX,			/* QNX		*/
+	IH_OS_U_BOOT,			/* Firmware	*/
+	IH_OS_RTEMS,			/* RTEMS	*/
+	IH_OS_ARTOS,			/* ARTOS	*/
+	IH_OS_UNITY,			/* Unity OS	*/
+	IH_OS_INTEGRITY,		/* INTEGRITY	*/
+	IH_OS_OSE,			/* OSE		*/
+	IH_OS_PLAN9,			/* Plan 9	*/
+	IH_OS_OPENRTOS,		/* OpenRTOS	*/
+
+	IH_OS_COUNT,
+};
 
 /*
  * CPU Architecture Codes (supported by Linux)
  */
-#define IH_ARCH_INVALID		0	/* Invalid CPU	*/
-#define IH_ARCH_ALPHA		1	/* Alpha	*/
-#define IH_ARCH_ARM		2	/* ARM		*/
-#define IH_ARCH_I386		3	/* Intel x86	*/
-#define IH_ARCH_IA64		4	/* IA64		*/
-#define IH_ARCH_MIPS		5	/* MIPS		*/
-#define IH_ARCH_MIPS64		6	/* MIPS	 64 Bit */
-#define IH_ARCH_PPC		7	/* PowerPC	*/
-#define IH_ARCH_S390		8	/* IBM S390	*/
-#define IH_ARCH_SH		9	/* SuperH	*/
-#define IH_ARCH_SPARC		10	/* Sparc	*/
-#define IH_ARCH_SPARC64		11	/* Sparc 64 Bit */
-#define IH_ARCH_M68K		12	/* M68K		*/
-#define IH_ARCH_MICROBLAZE	14	/* MicroBlaze   */
-#define IH_ARCH_NIOS2		15	/* Nios-II	*/
-#define IH_ARCH_BLACKFIN	16	/* Blackfin	*/
-#define IH_ARCH_AVR32		17	/* AVR32	*/
-#define IH_ARCH_ST200	        18	/* STMicroelectronics ST200  */
-#define IH_ARCH_SANDBOX		19	/* Sandbox architecture (test only) */
-#define IH_ARCH_NDS32	        20	/* ANDES Technology - NDS32  */
-#define IH_ARCH_OPENRISC        21	/* OpenRISC 1000  */
-#define IH_ARCH_ARM64		22	/* ARM64	*/
-#define IH_ARCH_ARC		23	/* Synopsys DesignWare ARC */
-#define IH_ARCH_X86_64		24	/* AMD x86_64, Intel and Via */
+enum {
+	IH_ARCH_INVALID		= 0,	/* Invalid CPU	*/
+	IH_ARCH_ALPHA,			/* Alpha	*/
+	IH_ARCH_ARM,			/* ARM		*/
+	IH_ARCH_I386,			/* Intel x86	*/
+	IH_ARCH_IA64,			/* IA64		*/
+	IH_ARCH_MIPS,			/* MIPS		*/
+	IH_ARCH_MIPS64,			/* MIPS	 64 Bit */
+	IH_ARCH_PPC,			/* PowerPC	*/
+	IH_ARCH_S390,			/* IBM S390	*/
+	IH_ARCH_SH,			/* SuperH	*/
+	IH_ARCH_SPARC,			/* Sparc	*/
+	IH_ARCH_SPARC64,		/* Sparc 64 Bit */
+	IH_ARCH_M68K,			/* M68K		*/
+	IH_ARCH_MICROBLAZE,		/* MicroBlaze   */
+	IH_ARCH_NIOS2,			/* Nios-II	*/
+	IH_ARCH_BLACKFIN,		/* Blackfin	*/
+	IH_ARCH_AVR32,			/* AVR32	*/
+	IH_ARCH_ST200,			/* STMicroelectronics ST200  */
+	IH_ARCH_SANDBOX,		/* Sandbox architecture (test only) */
+	IH_ARCH_NDS32,			/* ANDES Technology - NDS32  */
+	IH_ARCH_OPENRISC,		/* OpenRISC 1000  */
+	IH_ARCH_ARM64,			/* ARM64	*/
+	IH_ARCH_ARC,			/* Synopsys DesignWare ARC */
+	IH_ARCH_X86_64,			/* AMD x86_64, Intel and Via */
+
+	IH_ARCH_COUNT,
+};
 
 /*
  * Image Types
@@ -219,47 +227,54 @@ struct lmb;
  *	as command interpreter (=> Shell Scripts).
  */
 
-#define IH_TYPE_INVALID		0	/* Invalid Image		*/
-#define IH_TYPE_STANDALONE	1	/* Standalone Program		*/
-#define IH_TYPE_KERNEL		2	/* OS Kernel Image		*/
-#define IH_TYPE_RAMDISK		3	/* RAMDisk Image		*/
-#define IH_TYPE_MULTI		4	/* Multi-File Image		*/
-#define IH_TYPE_FIRMWARE	5	/* Firmware Image		*/
-#define IH_TYPE_SCRIPT		6	/* Script file			*/
-#define IH_TYPE_FILESYSTEM	7	/* Filesystem Image (any type)	*/
-#define IH_TYPE_FLATDT		8	/* Binary Flat Device Tree Blob	*/
-#define IH_TYPE_KWBIMAGE	9	/* Kirkwood Boot Image		*/
-#define IH_TYPE_IMXIMAGE	10	/* Freescale IMXBoot Image	*/
-#define IH_TYPE_UBLIMAGE	11	/* Davinci UBL Image		*/
-#define IH_TYPE_OMAPIMAGE	12	/* TI OMAP Config Header Image	*/
-#define IH_TYPE_AISIMAGE	13	/* TI Davinci AIS Image		*/
-#define IH_TYPE_KERNEL_NOLOAD	14	/* OS Kernel Image, can run from any load address */
-#define IH_TYPE_PBLIMAGE	15	/* Freescale PBL Boot Image	*/
-#define IH_TYPE_MXSIMAGE	16	/* Freescale MXSBoot Image	*/
-#define IH_TYPE_GPIMAGE		17	/* TI Keystone GPHeader Image	*/
-#define IH_TYPE_ATMELIMAGE	18	/* ATMEL ROM bootable Image	*/
-#define IH_TYPE_SOCFPGAIMAGE	19	/* Altera SOCFPGA Preloader	*/
-#define IH_TYPE_X86_SETUP	20	/* x86 setup.bin Image		*/
-#define IH_TYPE_LPC32XXIMAGE	21	/* x86 setup.bin Image		*/
-#define IH_TYPE_LOADABLE	22	/* A list of typeless images	*/
-#define IH_TYPE_RKIMAGE		23	/* Rockchip Boot Image		*/
-#define IH_TYPE_RKSD		24	/* Rockchip SD card		*/
-#define IH_TYPE_RKSPI		25	/* Rockchip SPI image		*/
-#define IH_TYPE_ZYNQIMAGE	26	/* Xilinx Zynq Boot Image */
-#define IH_TYPE_ZYNQMPIMAGE	27	/* Xilinx ZynqMP Boot Image */
-#define IH_TYPE_FPGA		28	/* FPGA Image */
-
-#define IH_TYPE_COUNT		29	/* Number of image types */
+enum {
+	IH_TYPE_INVALID		= 0,	/* Invalid Image		*/
+	IH_TYPE_STANDALONE,		/* Standalone Program		*/
+	IH_TYPE_KERNEL,			/* OS Kernel Image		*/
+	IH_TYPE_RAMDISK,		/* RAMDisk Image		*/
+	IH_TYPE_MULTI,			/* Multi-File Image		*/
+	IH_TYPE_FIRMWARE,		/* Firmware Image		*/
+	IH_TYPE_SCRIPT,			/* Script file			*/
+	IH_TYPE_FILESYSTEM,		/* Filesystem Image (any type)	*/
+	IH_TYPE_FLATDT,			/* Binary Flat Device Tree Blob	*/
+	IH_TYPE_KWBIMAGE,		/* Kirkwood Boot Image		*/
+	IH_TYPE_IMXIMAGE,		/* Freescale IMXBoot Image	*/
+	IH_TYPE_UBLIMAGE,		/* Davinci UBL Image		*/
+	IH_TYPE_OMAPIMAGE,		/* TI OMAP Config Header Image	*/
+	IH_TYPE_AISIMAGE,		/* TI Davinci AIS Image		*/
+	/* OS Kernel Image, can run from any load address */
+	IH_TYPE_KERNEL_NOLOAD,
+	IH_TYPE_PBLIMAGE,		/* Freescale PBL Boot Image	*/
+	IH_TYPE_MXSIMAGE,		/* Freescale MXSBoot Image	*/
+	IH_TYPE_GPIMAGE,		/* TI Keystone GPHeader Image	*/
+	IH_TYPE_ATMELIMAGE,		/* ATMEL ROM bootable Image	*/
+	IH_TYPE_SOCFPGAIMAGE,		/* Altera SOCFPGA Preloader	*/
+	IH_TYPE_X86_SETUP,		/* x86 setup.bin Image		*/
+	IH_TYPE_LPC32XXIMAGE,		/* x86 setup.bin Image		*/
+	IH_TYPE_LOADABLE,		/* A list of typeless images	*/
+	IH_TYPE_RKIMAGE,		/* Rockchip Boot Image		*/
+	IH_TYPE_RKSD,			/* Rockchip SD card		*/
+	IH_TYPE_RKSPI,			/* Rockchip SPI image		*/
+	IH_TYPE_ZYNQIMAGE,		/* Xilinx Zynq Boot Image */
+	IH_TYPE_ZYNQMPIMAGE,		/* Xilinx ZynqMP Boot Image */
+	IH_TYPE_FPGA,			/* FPGA Image */
+
+	IH_TYPE_COUNT,			/* Number of image types */
+};
 
 /*
  * Compression Types
  */
-#define IH_COMP_NONE		0	/*  No	 Compression Used	*/
-#define IH_COMP_GZIP		1	/* gzip	 Compression Used	*/
-#define IH_COMP_BZIP2		2	/* bzip2 Compression Used	*/
-#define IH_COMP_LZMA		3	/* lzma  Compression Used	*/
-#define IH_COMP_LZO		4	/* lzo   Compression Used	*/
-#define IH_COMP_LZ4		5	/* lz4   Compression Used	*/
+enum {
+	IH_COMP_NONE		= 0,	/*  No	 Compression Used	*/
+	IH_COMP_GZIP,			/* gzip	 Compression Used	*/
+	IH_COMP_BZIP2,			/* bzip2 Compression Used	*/
+	IH_COMP_LZMA,			/* lzma  Compression Used	*/
+	IH_COMP_LZO,			/* lzo   Compression Used	*/
+	IH_COMP_LZ4,			/* lz4   Compression Used	*/
+
+	IH_COMP_COUNT,
+};
 
 #define IH_MAGIC	0x27051956	/* Image Magic Number		*/
 #define IH_NMLEN		32	/* Image Name Length		*/
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 07/14] image: Convert the IH_... values to enums

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:13AM -0600, Simon Glass wrote:

> We need to know the number of values of each category (architecture,
> compression, OS and image type). To make this value easier to maintain,
> convert all values to enums. The count is then automatic.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/768205ee/attachment.sig>

[U-Boot] [U-Boot, 07/14] image: Convert the IH_... values to enums

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:13AM -0600, Simon Glass wrote:

> We need to know the number of values of each category (architecture,
> compression, OS and image type). To make this value easier to maintain,
> convert all values to enums. The count is then automatic.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/d51ac4d6/attachment.sig>

[U-Boot] [PATCH 08/14] image: Create a table of information for each category

[Simon Glass]

Add a table that contains the category name, the number of items in each
category and a pointer to the table of items. This will allow us to use
generic code to deal with the categories.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/image.c  | 13 +++++++++++++
 include/image.h |  9 +++++++++
 2 files changed, 22 insertions(+)

diff --git a/common/image.c b/common/image.c
index 0be09e5..4e503b3 100644
--- a/common/image.c
+++ b/common/image.c
@@ -176,6 +176,19 @@ static const table_entry_t uimage_comp[] = {
 	{	-1,		"",		"",			},
 };
 
+struct table_info {
+	const char *desc;
+	int count;
+	const table_entry_t *table;
+};
+
+static const struct table_info table_info[IH_COUNT] = {
+	{ "architecture", IH_ARCH_COUNT, uimage_arch },
+	{ "compression", IH_COMP_COUNT, uimage_comp },
+	{ "operating system", IH_OS_COUNT, uimage_os },
+	{ "image type", IH_TYPE_COUNT, uimage_type },
+};
+
 /*****************************************************************************/
 /* Legacy format routines */
 /*****************************************************************************/
diff --git a/include/image.h b/include/image.h
index 7717b3d..c5b691a 100644
--- a/include/image.h
+++ b/include/image.h
@@ -123,6 +123,15 @@ struct lmb;
 # define IMAGE_OF_SYSTEM_SETUP	0
 #endif
 
+enum ih_category {
+	IH_ARCH,
+	IH_COMP,
+	IH_OS,
+	IH_TYPE,
+
+	IH_COUNT,
+};
+
 /*
  * Operating System Codes
  */
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 08/14] image: Create a table of information for each category

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:14AM -0600, Simon Glass wrote:

> Add a table that contains the category name, the number of items in each
> category and a pointer to the table of items. This will allow us to use
> generic code to deal with the categories.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/175cc847/attachment.sig>

[U-Boot] [U-Boot, 08/14] image: Create a table of information for each category

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:14AM -0600, Simon Glass wrote:

> Add a table that contains the category name, the number of items in each
> category and a pointer to the table of items. This will allow us to use
> generic code to deal with the categories.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/e3f44ff5/attachment.sig>

[U-Boot] [PATCH 09/14] image: Add a name for invalid types

[Simon Glass]

At present the name is NULL, which prevents qsort() fromp being used. Use
the name "invalid" instead.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/image.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/common/image.c b/common/image.c
index 4e503b3..e9095f4 100644
--- a/common/image.c
+++ b/common/image.c
@@ -69,7 +69,7 @@ static const image_header_t *image_get_ramdisk(ulong rd_addr, uint8_t arch,
 #endif
 
 static const table_entry_t uimage_arch[] = {
-	{	IH_ARCH_INVALID,	NULL,		"Invalid ARCH",	},
+	{	IH_ARCH_INVALID,	"invalid",	"Invalid ARCH",	},
 	{	IH_ARCH_ALPHA,		"alpha",	"Alpha",	},
 	{	IH_ARCH_ARM,		"arm",		"ARM",		},
 	{	IH_ARCH_I386,		"x86",		"Intel x86",	},
@@ -97,7 +97,7 @@ static const table_entry_t uimage_arch[] = {
 };
 
 static const table_entry_t uimage_os[] = {
-	{	IH_OS_INVALID,	NULL,		"Invalid OS",		},
+	{	IH_OS_INVALID,	"invalid",	"Invalid OS",		},
 	{	IH_OS_LINUX,	"linux",	"Linux",		},
 #if defined(CONFIG_LYNXKDI) || defined(USE_HOSTCC)
 	{	IH_OS_LYNXOS,	"lynxos",	"LynxOS",		},
@@ -144,7 +144,7 @@ static const table_entry_t uimage_type[] = {
 	{	IH_TYPE_KERNEL_NOLOAD, "kernel_noload",  "Kernel Image (no loading done)", },
 	{	IH_TYPE_KWBIMAGE,   "kwbimage",   "Kirkwood Boot Image",},
 	{	IH_TYPE_IMXIMAGE,   "imximage",   "Freescale i.MX Boot Image",},
-	{	IH_TYPE_INVALID,    NULL,	  "Invalid Image",	},
+	{	IH_TYPE_INVALID,    "invalid",	  "Invalid Image",	},
 	{	IH_TYPE_MULTI,	    "multi",	  "Multi-File Image",	},
 	{	IH_TYPE_OMAPIMAGE,  "omapimage",  "TI OMAP SPL With GP CH",},
 	{	IH_TYPE_PBLIMAGE,   "pblimage",   "Freescale PBL Boot Image",},
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 09/14] image: Add a name for invalid types

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:15AM -0600, Simon Glass wrote:

> At present the name is NULL, which prevents qsort() fromp being used. Use
> the name "invalid" instead.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/8b88591c/attachment.sig>

[U-Boot] [U-Boot,09/14] image: Add a name for invalid types

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:15AM -0600, Simon Glass wrote:

> At present the name is NULL, which prevents qsort() fromp being used. Use
> the name "invalid" instead.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/d5677d93/attachment.sig>

[U-Boot] [PATCH 10/14] image: Add functions to obtain category information

[Simon Glass]

Add generic functions which can look up information about a category:

- the number of items in the category
- the category description
- an item long time
- an item short time

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/image.c  | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/image.h | 34 +++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+)

diff --git a/common/image.c b/common/image.c
index e9095f4..af155b2 100644
--- a/common/image.c
+++ b/common/image.c
@@ -583,6 +583,74 @@ const table_entry_t *get_table_entry(const table_entry_t *table, int id)
 	return NULL;
 }
 
+static const char *unknown_msg(enum ih_category category)
+{
+	static char msg[30];
+
+	strcpy(msg, "Unknown ");
+	strcat(msg, table_info[category].desc);
+
+	return msg;
+}
+
+/**
+ * get_cat_table_entry_name - translate entry id to long name
+ * @category: category to look up (enum ih_category)
+ * @id: entry id to be translated
+ *
+ * This will scan the translation table trying to find the entry that matches
+ * the given id.
+ *
+ * @retur long entry name if translation succeeds; error string on failure
+ */
+const char *genimg_get_cat_name(enum ih_category category, uint id)
+{
+	const table_entry_t *entry;
+
+	entry = get_table_entry(table_info[category].table, id);
+	if (!entry)
+		return unknown_msg(category);
+#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC)
+	return entry->lname;
+#else
+	return entry->lname + gd->reloc_off;
+#endif
+}
+
+/**
+ * get_cat_table_entry_short_name - translate entry id to short name
+ * @category: category to look up (enum ih_category)
+ * @id: entry id to be translated
+ *
+ * This will scan the translation table trying to find the entry that matches
+ * the given id.
+ *
+ * @retur short entry name if translation succeeds; error string on failure
+ */
+const char *genimg_get_cat_short_name(enum ih_category category, uint id)
+{
+	const table_entry_t *entry;
+
+	entry = get_table_entry(table_info[category].table, id);
+	if (!entry)
+		return unknown_msg(category);
+#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC)
+	return entry->sname;
+#else
+	return entry->sname + gd->reloc_off;
+#endif
+}
+
+int genimg_get_cat_count(enum ih_category category)
+{
+	return table_info[category].count;
+}
+
+const char *genimg_get_cat_desc(enum ih_category category)
+{
+	return table_info[category].desc;
+}
+
 /**
  * get_table_entry_name - translate entry id to long name
  * @table: pointer to a translation table for entries of a specific type
diff --git a/include/image.h b/include/image.h
index c5b691a..a116cb5 100644
--- a/include/image.h
+++ b/include/image.h
@@ -478,6 +478,40 @@ const char *genimg_get_comp_name(uint8_t comp);
  */
 const char *genimg_get_comp_short_name(uint8_t comp);
 
+/**
+ * genimg_get_cat_name() - Get the name of an item in a category
+ *
+ * @category:	Category of item
+ * @id:		Item ID
+ * @return name of item, or "Unknown ..." if unknown
+ */
+const char *genimg_get_cat_name(enum ih_category category, uint id);
+
+/**
+ * genimg_get_cat_short_name() - Get the short name of an item in a category
+ *
+ * @category:	Category of item
+ * @id:		Item ID
+ * @return short name of item, or "Unknown ..." if unknown
+ */
+const char *genimg_get_cat_short_name(enum ih_category category, uint id);
+
+/**
+ * genimg_get_cat_count() - Get the number of items in a category
+ *
+ * @category:	Category to check
+ * @return the number of items in the category (IH_xxx_COUNT)
+ */
+int genimg_get_cat_count(enum ih_category category);
+
+/**
+ * genimg_get_cat_desc() - Get the description of a category
+ *
+ * @return the description of a category, e.g. "architecture". This
+ * effectively converts the enum to a string.
+ */
+const char *genimg_get_cat_desc(enum ih_category category);
+
 int genimg_get_os_id(const char *name);
 int genimg_get_arch_id(const char *name);
 int genimg_get_type_id(const char *name);
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 10/14] image: Add functions to obtain category information

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:16AM -0600, Simon Glass wrote:

> Add generic functions which can look up information about a category:
> 
> - the number of items in the category
> - the category description
> - an item long time
> - an item short time
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/c782e7d0/attachment.sig>

[U-Boot] [U-Boot, 10/14] image: Add functions to obtain category information

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:16AM -0600, Simon Glass wrote:

> Add generic functions which can look up information about a category:
> 
> - the number of items in the category
> - the category description
> - an item long time
> - an item short time
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/298e5c0d/attachment.sig>

[U-Boot] [PATCH 11/14] mkimage: Allow display of a list of any image header category

[Simon Glass]

Add a generic function which can display a list of items in any category.
This will allow displaying of images for the -A, -C, -O and -T flags. At
present only -T is supported.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/mkimage.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/tools/mkimage.c b/tools/mkimage.c
index 920d3be..d375c2a 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -25,6 +25,49 @@ static struct image_tool_params params = {
 	.imagename2 = "",
 };
 
+static enum ih_category cur_category;
+
+static int h_compare_category_name(const void *vtype1, const void *vtype2)
+{
+	const int *type1 = vtype1;
+	const int *type2 = vtype2;
+	const char *name1 = genimg_get_cat_short_name(cur_category, *type1);
+	const char *name2 = genimg_get_cat_short_name(cur_category, *type2);
+
+	return strcmp(name1, name2);
+}
+
+int show_valid_options(enum ih_category category)
+{
+	int *order;
+	int count;
+	int item;
+	int i;
+
+	count = genimg_get_cat_count(category);
+	order = calloc(count, sizeof(*order));
+	if (!order)
+		return -ENOMEM;
+
+	/* Sort the names in order of short name for easier reading */
+	for (item = 0; item < count; item++)
+		order[item] = item;
+	cur_category = category;
+	qsort(order, count, sizeof(int), h_compare_category_name);
+
+	fprintf(stderr, "\nInvalid %s, supported are:\n",
+		genimg_get_cat_desc(category));
+	for (i = 0; i < count; i++) {
+		item = order[i];
+		fprintf(stderr, "\t%-15s  %s\n",
+			genimg_get_cat_short_name(category, item),
+			genimg_get_cat_name(category, item));
+	}
+	fprintf(stderr, "\n");
+
+	return 0;
+}
+
 static int h_compare_image_name(const void *vtype1, const void *vtype2)
 {
 	const int *type1 = vtype1;
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 11/14] mkimage: Allow display of a list of any image header category

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:17AM -0600, Simon Glass wrote:

> Add a generic function which can display a list of items in any category.
> This will allow displaying of images for the -A, -C, -O and -T flags. At
> present only -T is supported.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/34836ce2/attachment.sig>

[U-Boot] [U-Boot, 11/14] mkimage: Allow display of a list of any image header category

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:17AM -0600, Simon Glass wrote:

> Add a generic function which can display a list of items in any category.
> This will allow displaying of images for the -A, -C, -O and -T flags. At
> present only -T is supported.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/bc331981/attachment.sig>

[U-Boot] [PATCH 12/14] mkimage: Use generic code for showing an 'image type' error

[Simon Glass]

The existing error code only displays image types which are claimed by a
particular U_BOOT_IMAGE_TYPE() driver. But this does not seem correct. The
mkimage tool should support all image types, so it makes sense to allow
creation of images of any type with the tool.

When an incorrect image type is provided, use generic code to display the
error.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/mkimage.c | 45 ++-------------------------------------------
 1 file changed, 2 insertions(+), 43 deletions(-)

diff --git a/tools/mkimage.c b/tools/mkimage.c
index d375c2a..3cdbb2c 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -37,7 +37,7 @@ static int h_compare_category_name(const void *vtype1, const void *vtype2)
 	return strcmp(name1, name2);
 }
 
-int show_valid_options(enum ih_category category)
+static int show_valid_options(enum ih_category category)
 {
 	int *order;
 	int count;
@@ -68,47 +68,6 @@ int show_valid_options(enum ih_category category)
 	return 0;
 }
 
-static int h_compare_image_name(const void *vtype1, const void *vtype2)
-{
-	const int *type1 = vtype1;
-	const int *type2 = vtype2;
-	const char *name1 = genimg_get_type_short_name(*type1);
-	const char *name2 = genimg_get_type_short_name(*type2);
-
-	return strcmp(name1, name2);
-}
-
-/* Show all image types supported by mkimage */
-static void show_image_types(void)
-{
-	struct image_type_params *tparams;
-	int order[IH_TYPE_COUNT];
-	int count;
-	int type;
-	int i;
-
-	/* Sort the names in order of short name for easier reading */
-	memset(order, '\0', sizeof(order));
-	for (count = 0, type = 0; type < IH_TYPE_COUNT; type++) {
-		tparams = imagetool_get_type(type);
-		if (tparams)
-			order[count++] = type;
-	}
-	qsort(order, count, sizeof(int), h_compare_image_name);
-
-	fprintf(stderr, "\nInvalid image type. Supported image types:\n");
-	for (i = 0; i < count; i++) {
-		type = order[i];
-		tparams = imagetool_get_type(type);
-		if (tparams) {
-			fprintf(stderr, "\t%-15s  %s\n",
-				genimg_get_type_short_name(type),
-				genimg_get_type_name(type));
-		}
-	}
-	fprintf(stderr, "\n");
-}
-
 static void usage(const char *msg)
 {
 	fprintf(stderr, "Error: %s\n", msg);
@@ -286,7 +245,7 @@ static void process_args(int argc, char **argv)
 		case 'T':
 			type = genimg_get_type_id(optarg);
 			if (type < 0) {
-				show_image_types();
+				show_valid_options(IH_TYPE);
 				usage("Invalid image type");
 			}
 			break;
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 12/14] mkimage: Use generic code for showing an 'image type' error

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:18AM -0600, Simon Glass wrote:

> The existing error code only displays image types which are claimed by a
> particular U_BOOT_IMAGE_TYPE() driver. But this does not seem correct. The
> mkimage tool should support all image types, so it makes sense to allow
> creation of images of any type with the tool.
> 
> When an incorrect image type is provided, use generic code to display the
> error.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/4497133f/attachment.sig>

[U-Boot] [U-Boot, 12/14] mkimage: Use generic code for showing an 'image type' error

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:18AM -0600, Simon Glass wrote:

> The existing error code only displays image types which are claimed by a
> particular U_BOOT_IMAGE_TYPE() driver. But this does not seem correct. The
> mkimage tool should support all image types, so it makes sense to allow
> creation of images of any type with the tool.
> 
> When an incorrect image type is provided, use generic code to display the
> error.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/6acb9370/attachment.sig>

[U-Boot] [PATCH 13/14] mkimage: Show item lists for all categories

[Simon Glass]

Update the error-handling code for -A, -C and -O to show a list of valid
options when an invalid one is provided.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reported-by: Vinoth Eswaran <evinoth1206@gmail.com>
---

 tools/mkimage.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/tools/mkimage.c b/tools/mkimage.c
index 3cdbb2c..f589a41 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -152,8 +152,10 @@ static void process_args(int argc, char **argv)
 			break;
 		case 'A':
 			params.arch = genimg_get_arch_id(optarg);
-			if (params.arch < 0)
+			if (params.arch < 0) {
+				show_valid_options(IH_ARCH);
 				usage("Invalid architecture");
+			}
 			break;
 		case 'b':
 			if (add_content(IH_TYPE_FLATDT, optarg)) {
@@ -168,8 +170,10 @@ static void process_args(int argc, char **argv)
 			break;
 		case 'C':
 			params.comp = genimg_get_comp_id(optarg);
-			if (params.comp < 0)
+			if (params.comp < 0) {
+				show_valid_options(IH_COMP);
 				usage("Invalid compression type");
+			}
 			break;
 		case 'd':
 			params.datafile = optarg;
@@ -216,8 +220,10 @@ static void process_args(int argc, char **argv)
 			break;
 		case 'O':
 			params.os = genimg_get_os_id(optarg);
-			if (params.os < 0)
+			if (params.os < 0) {
+				show_valid_options(IH_OS);
 				usage("Invalid operating system");
+			}
 			break;
 		case 'p':
 			params.external_offset = strtoull(optarg, &ptr, 16);
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 13/14] mkimage: Show item lists for all categories

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:19AM -0600, Simon Glass wrote:

> Update the error-handling code for -A, -C and -O to show a list of valid
> options when an invalid one is provided.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reported-by: Vinoth Eswaran <evinoth1206@gmail.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/cb3f6d36/attachment.sig>

[U-Boot] [U-Boot, 13/14] mkimage: Show item lists for all categories

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:19AM -0600, Simon Glass wrote:

> Update the error-handling code for -A, -C and -O to show a list of valid
> options when an invalid one is provided.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reported-by: Vinoth Eswaran <evinoth1206@gmail.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/4aeacb34/attachment.sig>

[U-Boot] [PATCH 14/14] tools: Allow building with debug enabled

[Simon Glass]

Sometimes it is useful to build tools with debugging information included so
that line-number information is available when run under gdb. Add a Kconfig
option to support this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 Kconfig  | 9 +++++++++
 Makefile | 3 ++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/Kconfig b/Kconfig
index 3ceff25..c06be59 100644
--- a/Kconfig
+++ b/Kconfig
@@ -114,6 +114,15 @@ if EXPERT
 	  Warning:
 	  When disabling this, please check if malloc calls, maybe
 	  should be replaced by calloc - if one expects zeroed memory.
+
+config TOOLS_DEBUG
+	bool "Enable debug information for tools"
+	help
+	  Enable generation of debug information for tools such as mkimage.
+	  This can be used for debugging purposes. With debug information
+	  it is possible to set breakpoints on particular lines, single-step
+	  debug through the source code, etc.
+
 endif
 endmenu		# General setup
 
diff --git a/Makefile b/Makefile
index 0c47bb6..e8e2bb2 100644
--- a/Makefile
+++ b/Makefile
@@ -256,7 +256,8 @@ CONFIG_SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \
 
 HOSTCC       = cc
 HOSTCXX      = c++
-HOSTCFLAGS   = -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer
+HOSTCFLAGS   = -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer \
+		$(if $(CONFIG_TOOLS_DEBUG),-g)
 HOSTCXXFLAGS = -O2
 
 ifeq ($(HOSTOS),cygwin)
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 14/14] tools: Allow building with debug enabled

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:20AM -0600, Simon Glass wrote:

> Sometimes it is useful to build tools with debugging information included so
> that line-number information is available when run under gdb. Add a Kconfig
> option to support this.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/76a67164/attachment.sig>

[U-Boot] [U-Boot, 14/14] tools: Allow building with debug enabled

[Tom Rini]

On Thu, Jun 30, 2016 at 10:52:20AM -0600, Simon Glass wrote:

> Sometimes it is useful to build tools with debugging information included so
> that line-number information is available when run under gdb. Add a Kconfig
> option to support this.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/7a11a4f1/attachment.sig>

[U-Boot] [PATCH 00/14] mkimage: Tidy up error handling

[Stefano Babic]

Hi Simon,

On 30/06/2016 18:52, Simon Glass wrote:
> There are a few problems when mkimage is provided with invalid arguments.
> In one case it crashes. When an invalid image type it is provided it lists
> the valid types, but this is not implemented for compression, architecture
> or OS.
> 

There is another issue related to mkimage. It looks like it is broken
since a lot of time, but it appears it was not noted.

mkimage -l is broken. Testing with i.MX images (imximage), it does not
show the header, but it reports the output as it as a "gpimage".

In fact:

./tools/mkimage -l test.imx
GP Header: Size d1002040 LoadAddr 8017

It should be:

./tools/mkimage -l test.imx
Image Type:   Freescale IMX Boot Image
Image Ver:    2 (i.MX53/6/7 compatible)
Data Size:    331776 Bytes = 324.00 kB = 0.32 MB
Load Address: 177ff420
Entry Point:  17800000

The reason is due to the format of the gpimage itself. It has no magic
number, and checking for its correctness means in gph_verify_header()
just check that size and address are not NULL. That let think that the
image is a gpimage, it is not. gpimage simply uses the image and does
not let to check for further image headers that are put int the (sorted)
list with the U_BOOT_IMAGE_TYPE. Image types that are in the list
*before* gpimage are correctly recognized and printed, the following (as
imximage) not.

A quick fix (maybe just for the release ?) should be to let gpimage (but
I do not know if there is another image type that misbehaves as it does)
as the last one in the list: if all detections fail, the last one
without any possibility for detection (gpimage) runs. The following
patch works for me:

diff --git a/tools/Makefile b/tools/Makefile
index 63355aa..f72294a 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -76,8 +76,6 @@ dumpimage-mkimage-objs := aisimage.o \
 			lib/fdtdec.o \
 			fit_common.o \
 			fit_image.o \
-			gpimage.o \
-			gpimage-common.o \
 			common/image-fit.o \
 			image-host.o \
 			common/image.o \
@@ -100,6 +98,8 @@ dumpimage-mkimage-objs := aisimage.o \
 			zynqimage.o \
 			zynqmpimage.o \
 			$(LIBFDT_OBJS) \
+			gpimage.o \
+			gpimage-common.o \
 			$(RSA_OBJS-y)

 dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.o

What do you think ? If this could be a solution for release, I send a
formal patch.

Best regards,
Stefano

> This series tidies this up a little, to make mkimage more friendly.
> 
> 
> Simon Glass (14):
>   mkimage: Honour the default image type with auto-fit
>   mkimage: Explain the auto-fit imagefile special case
>   mkimage: Require a data file when auto-fit is used
>   mkimage: Drop premature setting of params.fit_image_type
>   mkimage: Drop blank line before main()
>   image: Correct auto-fit architecture property name
>   image: Convert the IH_... values to enums
>   image: Create a table of information for each category
>   image: Add a name for invalid types
>   image: Add functions to obtain category information
>   mkimage: Allow display of a list of any image header category
>   mkimage: Use generic code for showing an 'image type' error
>   mkimage: Show item lists for all categories
>   tools: Allow building with debug enabled
> 
>  Kconfig           |   9 +++
>  Makefile          |   3 +-
>  common/image.c    |  87 ++++++++++++++++++++-
>  include/image.h   | 230 ++++++++++++++++++++++++++++++++++--------------------
>  tools/fit_image.c |   3 +-
>  tools/mkimage.c   |  69 +++++++++-------
>  6 files changed, 280 insertions(+), 121 deletions(-)
> 


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH 00/14] mkimage: Tidy up error handling

[Tom Rini]

On Fri, Jul 01, 2016 at 10:44:04AM +0200, Stefano Babic wrote:
> Hi Simon,
> 
> On 30/06/2016 18:52, Simon Glass wrote:
> > There are a few problems when mkimage is provided with invalid arguments.
> > In one case it crashes. When an invalid image type it is provided it lists
> > the valid types, but this is not implemented for compression, architecture
> > or OS.
> > 
> 
> There is another issue related to mkimage. It looks like it is broken
> since a lot of time, but it appears it was not noted.
> 
> mkimage -l is broken. Testing with i.MX images (imximage), it does not
> show the header, but it reports the output as it as a "gpimage".
> 
> In fact:
> 
> ./tools/mkimage -l test.imx
> GP Header: Size d1002040 LoadAddr 8017
> 
> It should be:
> 
> ./tools/mkimage -l test.imx
> Image Type:   Freescale IMX Boot Image
> Image Ver:    2 (i.MX53/6/7 compatible)
> Data Size:    331776 Bytes = 324.00 kB = 0.32 MB
> Load Address: 177ff420
> Entry Point:  17800000
> 
> The reason is due to the format of the gpimage itself. It has no magic
> number, and checking for its correctness means in gph_verify_header()
> just check that size and address are not NULL. That let think that the
> image is a gpimage, it is not. gpimage simply uses the image and does
> not let to check for further image headers that are put int the (sorted)
> list with the U_BOOT_IMAGE_TYPE. Image types that are in the list
> *before* gpimage are correctly recognized and printed, the following (as
> imximage) not.
> 
> A quick fix (maybe just for the release ?) should be to let gpimage (but
> I do not know if there is another image type that misbehaves as it does)
> as the last one in the list: if all detections fail, the last one
> without any possibility for detection (gpimage) runs. The following
> patch works for me:
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index 63355aa..f72294a 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -76,8 +76,6 @@ dumpimage-mkimage-objs := aisimage.o \
>  			lib/fdtdec.o \
>  			fit_common.o \
>  			fit_image.o \
> -			gpimage.o \
> -			gpimage-common.o \
>  			common/image-fit.o \
>  			image-host.o \
>  			common/image.o \
> @@ -100,6 +98,8 @@ dumpimage-mkimage-objs := aisimage.o \
>  			zynqimage.o \
>  			zynqmpimage.o \
>  			$(LIBFDT_OBJS) \
> +			gpimage.o \
> +			gpimage-common.o \
>  			$(RSA_OBJS-y)
> 
>  dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.o
> 
> What do you think ? If this could be a solution for release, I send a
> formal patch.

Ug, I think we need what you're saying at least for release.  I'm
kicking off a big bisect now to see just when this last worked right.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/69a97215/attachment.sig>

[U-Boot] [PATCH 00/14] mkimage: Tidy up error handling

[Simon Glass]

Hi,

On 1 July 2016 at 08:48, Tom Rini <trini@konsulko.com> wrote:
> On Fri, Jul 01, 2016 at 10:44:04AM +0200, Stefano Babic wrote:
>> Hi Simon,
>>
>> On 30/06/2016 18:52, Simon Glass wrote:
>> > There are a few problems when mkimage is provided with invalid arguments.
>> > In one case it crashes. When an invalid image type it is provided it lists
>> > the valid types, but this is not implemented for compression, architecture
>> > or OS.
>> >
>>
>> There is another issue related to mkimage. It looks like it is broken
>> since a lot of time, but it appears it was not noted.
>>
>> mkimage -l is broken. Testing with i.MX images (imximage), it does not
>> show the header, but it reports the output as it as a "gpimage".
>>
>> In fact:
>>
>> ./tools/mkimage -l test.imx
>> GP Header: Size d1002040 LoadAddr 8017
>>
>> It should be:
>>
>> ./tools/mkimage -l test.imx
>> Image Type:   Freescale IMX Boot Image
>> Image Ver:    2 (i.MX53/6/7 compatible)
>> Data Size:    331776 Bytes = 324.00 kB = 0.32 MB
>> Load Address: 177ff420
>> Entry Point:  17800000
>>
>> The reason is due to the format of the gpimage itself. It has no magic
>> number, and checking for its correctness means in gph_verify_header()
>> just check that size and address are not NULL. That let think that the
>> image is a gpimage, it is not. gpimage simply uses the image and does
>> not let to check for further image headers that are put int the (sorted)
>> list with the U_BOOT_IMAGE_TYPE. Image types that are in the list
>> *before* gpimage are correctly recognized and printed, the following (as
>> imximage) not.
>>
>> A quick fix (maybe just for the release ?) should be to let gpimage (but
>> I do not know if there is another image type that misbehaves as it does)
>> as the last one in the list: if all detections fail, the last one
>> without any possibility for detection (gpimage) runs. The following
>> patch works for me:
>>
>> diff --git a/tools/Makefile b/tools/Makefile
>> index 63355aa..f72294a 100644
>> --- a/tools/Makefile
>> +++ b/tools/Makefile
>> @@ -76,8 +76,6 @@ dumpimage-mkimage-objs := aisimage.o \
>>                       lib/fdtdec.o \
>>                       fit_common.o \
>>                       fit_image.o \
>> -                     gpimage.o \
>> -                     gpimage-common.o \
>>                       common/image-fit.o \
>>                       image-host.o \
>>                       common/image.o \
>> @@ -100,6 +98,8 @@ dumpimage-mkimage-objs := aisimage.o \
>>                       zynqimage.o \
>>                       zynqmpimage.o \
>>                       $(LIBFDT_OBJS) \
>> +                     gpimage.o \
>> +                     gpimage-common.o \
>>                       $(RSA_OBJS-y)
>>
>>  dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.o
>>
>> What do you think ? If this could be a solution for release, I send a
>> formal patch.
>
> Ug, I think we need what you're saying at least for release.  I'm
> kicking off a big bisect now to see just when this last worked right.

That patch seems reasonable to me for now. My series is intended for
the next release.

Perhaps verify_header() should return a value meaning 'possibly'?

Regards,
Simon

[U-Boot] [PATCH 00/14] mkimage: Tidy up error handling

[Tom Rini]

On Fri, Jul 01, 2016 at 09:15:11AM -0700, Simon Glass wrote:
> Hi,
> 
> On 1 July 2016 at 08:48, Tom Rini <trini@konsulko.com> wrote:
> > On Fri, Jul 01, 2016 at 10:44:04AM +0200, Stefano Babic wrote:
> >> Hi Simon,
> >>
> >> On 30/06/2016 18:52, Simon Glass wrote:
> >> > There are a few problems when mkimage is provided with invalid arguments.
> >> > In one case it crashes. When an invalid image type it is provided it lists
> >> > the valid types, but this is not implemented for compression, architecture
> >> > or OS.
> >> >
> >>
> >> There is another issue related to mkimage. It looks like it is broken
> >> since a lot of time, but it appears it was not noted.
> >>
> >> mkimage -l is broken. Testing with i.MX images (imximage), it does not
> >> show the header, but it reports the output as it as a "gpimage".
> >>
> >> In fact:
> >>
> >> ./tools/mkimage -l test.imx
> >> GP Header: Size d1002040 LoadAddr 8017
> >>
> >> It should be:
> >>
> >> ./tools/mkimage -l test.imx
> >> Image Type:   Freescale IMX Boot Image
> >> Image Ver:    2 (i.MX53/6/7 compatible)
> >> Data Size:    331776 Bytes = 324.00 kB = 0.32 MB
> >> Load Address: 177ff420
> >> Entry Point:  17800000
> >>
> >> The reason is due to the format of the gpimage itself. It has no magic
> >> number, and checking for its correctness means in gph_verify_header()
> >> just check that size and address are not NULL. That let think that the
> >> image is a gpimage, it is not. gpimage simply uses the image and does
> >> not let to check for further image headers that are put int the (sorted)
> >> list with the U_BOOT_IMAGE_TYPE. Image types that are in the list
> >> *before* gpimage are correctly recognized and printed, the following (as
> >> imximage) not.
> >>
> >> A quick fix (maybe just for the release ?) should be to let gpimage (but
> >> I do not know if there is another image type that misbehaves as it does)
> >> as the last one in the list: if all detections fail, the last one
> >> without any possibility for detection (gpimage) runs. The following
> >> patch works for me:
> >>
> >> diff --git a/tools/Makefile b/tools/Makefile
> >> index 63355aa..f72294a 100644
> >> --- a/tools/Makefile
> >> +++ b/tools/Makefile
> >> @@ -76,8 +76,6 @@ dumpimage-mkimage-objs := aisimage.o \
> >>                       lib/fdtdec.o \
> >>                       fit_common.o \
> >>                       fit_image.o \
> >> -                     gpimage.o \
> >> -                     gpimage-common.o \
> >>                       common/image-fit.o \
> >>                       image-host.o \
> >>                       common/image.o \
> >> @@ -100,6 +98,8 @@ dumpimage-mkimage-objs := aisimage.o \
> >>                       zynqimage.o \
> >>                       zynqmpimage.o \
> >>                       $(LIBFDT_OBJS) \
> >> +                     gpimage.o \
> >> +                     gpimage-common.o \
> >>                       $(RSA_OBJS-y)
> >>
> >>  dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.o
> >>
> >> What do you think ? If this could be a solution for release, I send a
> >> formal patch.
> >
> > Ug, I think we need what you're saying at least for release.  I'm
> > kicking off a big bisect now to see just when this last worked right.
> 
> That patch seems reasonable to me for now. My series is intended for
> the next release.
> 
> Perhaps verify_header() should return a value meaning 'possibly'?

So, this was broken by:
commit 0ca6691c2ec20ff264d882854c339795579991f5
Author: Guilherme Maciel Ferreira <guilherme.maciel.ferreira@gmail.com>
Date:   Thu Jan 15 02:48:05 2015 -0200

    imagetool: move common code to imagetool module

And yes, perhaps some way to say to say "maybe" is enough.  Or maybe we
just need to allow some to say that they can't verify?  I think we're thinking
along the same lines.


-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/f5c39d49/attachment.sig>

[U-Boot] [PATCH 00/14] mkimage: Tidy up error handling

[Stefano Babic]

Hi Tom, Simon,

On 01/07/2016 18:45, Tom Rini wrote:
> On Fri, Jul 01, 2016 at 09:15:11AM -0700, Simon Glass wrote:
>> Hi,
>>
>> On 1 July 2016 at 08:48, Tom Rini <trini@konsulko.com> wrote:
>>> On Fri, Jul 01, 2016 at 10:44:04AM +0200, Stefano Babic wrote:
>>>> Hi Simon,
>>>>
>>>> On 30/06/2016 18:52, Simon Glass wrote:
>>>>> There are a few problems when mkimage is provided with invalid arguments.
>>>>> In one case it crashes. When an invalid image type it is provided it lists
>>>>> the valid types, but this is not implemented for compression, architecture
>>>>> or OS.
>>>>>
>>>>
>>>> There is another issue related to mkimage. It looks like it is broken
>>>> since a lot of time, but it appears it was not noted.
>>>>
>>>> mkimage -l is broken. Testing with i.MX images (imximage), it does not
>>>> show the header, but it reports the output as it as a "gpimage".
>>>>
>>>> In fact:
>>>>
>>>> ./tools/mkimage -l test.imx
>>>> GP Header: Size d1002040 LoadAddr 8017
>>>>
>>>> It should be:
>>>>
>>>> ./tools/mkimage -l test.imx
>>>> Image Type:   Freescale IMX Boot Image
>>>> Image Ver:    2 (i.MX53/6/7 compatible)
>>>> Data Size:    331776 Bytes = 324.00 kB = 0.32 MB
>>>> Load Address: 177ff420
>>>> Entry Point:  17800000
>>>>
>>>> The reason is due to the format of the gpimage itself. It has no magic
>>>> number, and checking for its correctness means in gph_verify_header()
>>>> just check that size and address are not NULL. That let think that the
>>>> image is a gpimage, it is not. gpimage simply uses the image and does
>>>> not let to check for further image headers that are put int the (sorted)
>>>> list with the U_BOOT_IMAGE_TYPE. Image types that are in the list
>>>> *before* gpimage are correctly recognized and printed, the following (as
>>>> imximage) not.
>>>>
>>>> A quick fix (maybe just for the release ?) should be to let gpimage (but
>>>> I do not know if there is another image type that misbehaves as it does)
>>>> as the last one in the list: if all detections fail, the last one
>>>> without any possibility for detection (gpimage) runs. The following
>>>> patch works for me:
>>>>
>>>> diff --git a/tools/Makefile b/tools/Makefile
>>>> index 63355aa..f72294a 100644
>>>> --- a/tools/Makefile
>>>> +++ b/tools/Makefile
>>>> @@ -76,8 +76,6 @@ dumpimage-mkimage-objs := aisimage.o \
>>>>                       lib/fdtdec.o \
>>>>                       fit_common.o \
>>>>                       fit_image.o \
>>>> -                     gpimage.o \
>>>> -                     gpimage-common.o \
>>>>                       common/image-fit.o \
>>>>                       image-host.o \
>>>>                       common/image.o \
>>>> @@ -100,6 +98,8 @@ dumpimage-mkimage-objs := aisimage.o \
>>>>                       zynqimage.o \
>>>>                       zynqmpimage.o \
>>>>                       $(LIBFDT_OBJS) \
>>>> +                     gpimage.o \
>>>> +                     gpimage-common.o \
>>>>                       $(RSA_OBJS-y)
>>>>
>>>>  dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.o
>>>>
>>>> What do you think ? If this could be a solution for release, I send a
>>>> formal patch.
>>>
>>> Ug, I think we need what you're saying at least for release.  I'm
>>> kicking off a big bisect now to see just when this last worked right.
>>
>> That patch seems reasonable to me for now. My series is intended for
>> the next release.
>>
>> Perhaps verify_header() should return a value meaning 'possibly'?
> 
> So, this was broken by:
> commit 0ca6691c2ec20ff264d882854c339795579991f5
> Author: Guilherme Maciel Ferreira <guilherme.maciel.ferreira@gmail.com>
> Date:   Thu Jan 15 02:48:05 2015 -0200
> 
>     imagetool: move common code to imagetool module
> 

Yes, I had bisected myself and I get this commit - but the patch is not
the readon of the breakage.

> And yes, perhaps some way to say to say "maybe" is enough.

Right, but there is a side effect. Even if a "maybe" is returned (or
let's say, a "I can't check"), the effect is that "GP Header: Size XXXX"
is still printed, because for the gpimage this *could* be a correct image.

>  Or maybe we
> just need to allow some to say that they can't verify?  I think we're thinking
> along the same lines.

Regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH 00/14] mkimage: Tidy up error handling

[Simon Glass]

Hi Stefano,

On 1 July 2016 at 11:33, Stefano Babic <sbabic@denx.de> wrote:
> Hi Tom, Simon,
>
> On 01/07/2016 18:45, Tom Rini wrote:
>> On Fri, Jul 01, 2016 at 09:15:11AM -0700, Simon Glass wrote:
>>> Hi,
>>>
>>> On 1 July 2016 at 08:48, Tom Rini <trini@konsulko.com> wrote:
>>>> On Fri, Jul 01, 2016 at 10:44:04AM +0200, Stefano Babic wrote:
>>>>> Hi Simon,
>>>>>
>>>>> On 30/06/2016 18:52, Simon Glass wrote:
>>>>>> There are a few problems when mkimage is provided with invalid arguments.
>>>>>> In one case it crashes. When an invalid image type it is provided it lists
>>>>>> the valid types, but this is not implemented for compression, architecture
>>>>>> or OS.
>>>>>>
>>>>>
>>>>> There is another issue related to mkimage. It looks like it is broken
>>>>> since a lot of time, but it appears it was not noted.
>>>>>
>>>>> mkimage -l is broken. Testing with i.MX images (imximage), it does not
>>>>> show the header, but it reports the output as it as a "gpimage".
>>>>>
>>>>> In fact:
>>>>>
>>>>> ./tools/mkimage -l test.imx
>>>>> GP Header: Size d1002040 LoadAddr 8017
>>>>>
>>>>> It should be:
>>>>>
>>>>> ./tools/mkimage -l test.imx
>>>>> Image Type:   Freescale IMX Boot Image
>>>>> Image Ver:    2 (i.MX53/6/7 compatible)
>>>>> Data Size:    331776 Bytes = 324.00 kB = 0.32 MB
>>>>> Load Address: 177ff420
>>>>> Entry Point:  17800000
>>>>>
>>>>> The reason is due to the format of the gpimage itself. It has no magic
>>>>> number, and checking for its correctness means in gph_verify_header()
>>>>> just check that size and address are not NULL. That let think that the
>>>>> image is a gpimage, it is not. gpimage simply uses the image and does
>>>>> not let to check for further image headers that are put int the (sorted)
>>>>> list with the U_BOOT_IMAGE_TYPE. Image types that are in the list
>>>>> *before* gpimage are correctly recognized and printed, the following (as
>>>>> imximage) not.
>>>>>
>>>>> A quick fix (maybe just for the release ?) should be to let gpimage (but
>>>>> I do not know if there is another image type that misbehaves as it does)
>>>>> as the last one in the list: if all detections fail, the last one
>>>>> without any possibility for detection (gpimage) runs. The following
>>>>> patch works for me:
>>>>>
>>>>> diff --git a/tools/Makefile b/tools/Makefile
>>>>> index 63355aa..f72294a 100644
>>>>> --- a/tools/Makefile
>>>>> +++ b/tools/Makefile
>>>>> @@ -76,8 +76,6 @@ dumpimage-mkimage-objs := aisimage.o \
>>>>>                       lib/fdtdec.o \
>>>>>                       fit_common.o \
>>>>>                       fit_image.o \
>>>>> -                     gpimage.o \
>>>>> -                     gpimage-common.o \
>>>>>                       common/image-fit.o \
>>>>>                       image-host.o \
>>>>>                       common/image.o \
>>>>> @@ -100,6 +98,8 @@ dumpimage-mkimage-objs := aisimage.o \
>>>>>                       zynqimage.o \
>>>>>                       zynqmpimage.o \
>>>>>                       $(LIBFDT_OBJS) \
>>>>> +                     gpimage.o \
>>>>> +                     gpimage-common.o \
>>>>>                       $(RSA_OBJS-y)
>>>>>
>>>>>  dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.o
>>>>>
>>>>> What do you think ? If this could be a solution for release, I send a
>>>>> formal patch.
>>>>
>>>> Ug, I think we need what you're saying at least for release.  I'm
>>>> kicking off a big bisect now to see just when this last worked right.
>>>
>>> That patch seems reasonable to me for now. My series is intended for
>>> the next release.
>>>
>>> Perhaps verify_header() should return a value meaning 'possibly'?
>>
>> So, this was broken by:
>> commit 0ca6691c2ec20ff264d882854c339795579991f5
>> Author: Guilherme Maciel Ferreira <guilherme.maciel.ferreira@gmail.com>
>> Date:   Thu Jan 15 02:48:05 2015 -0200
>>
>>     imagetool: move common code to imagetool module
>>
>
> Yes, I had bisected myself and I get this commit - but the patch is not
> the readon of the breakage.
>
>> And yes, perhaps some way to say to say "maybe" is enough.
>
> Right, but there is a side effect. Even if a "maybe" is returned (or
> let's say, a "I can't check"), the effect is that "GP Header: Size XXXX"
> is still printed, because for the gpimage this *could* be a correct image.

Well, verifying whether this is 'your' image should not print
anything. We probably need to clarify the API a little. The comment
for verify_header() could say that printing is not allowed.

>
>>  Or maybe we
>> just need to allow some to say that they can't verify?  I think we're thinking
>> along the same lines.

Regards,
Simon