[U-Boot] [PATCH] mkimage: Fix 'Unknown OMAP image type - 5'

[U-Boot] [PATCH] mkimage: Fix 'Unknown OMAP image type - 5'

[Dirk Behme]

From: Dirk Behme <dirk.behme@googlemail.com>

Using mkimage with e.g.

tools/mkimage -A arm -T firmware -O u-boot -d u-boot.bin foo.img

gives a warning

"Unknown OMAP image type - 5"

while it seems that the image itself is created successfully.

This does come from the patch "mkimage: Add OMAP boot image support".

Reordering the init_xx_image_type() sequence does make this
message go away.

Signed-off-by: Dirk Behme <dirk.behme@googlemail.com>
CC: John Rigby <john.rigby@linaro.org>
CC: Aneesh V <aneesh@ti.com>
CC: Sandeep Paulraj <s-paulraj@ti.com>

---

This is reproducable with the recent mainline mkimage where the
patch "mkimage: Add OMAP boot image support" is applied:

http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commit;h=3decb14abe76d244ba98fd158ef95f89e7e37d70

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

Index: u-boot.git/tools/mkimage.c
===================================================================
--- u-boot.git.orig/tools/mkimage.c
+++ u-boot.git/tools/mkimage.c
@@ -156,12 +156,12 @@ main (int argc, char **argv)
 	init_imx_image_type ();
 	/* Init FIT image generation/list support */
 	init_fit_image_type ();
-	/* Init TI OMAP Boot image generation/list support */
-	init_omap_image_type();
 	/* Init Default image generation/list support */
 	init_default_image_type ();
 	/* Init Davinci UBL support */
 	init_ubl_image_type();
+	/* Init TI OMAP Boot image generation/list support */
+	init_omap_image_type();
 
 	params.cmdname = *argv;
 	params.addr = params.ep = 0;

[U-Boot] [PATCH] mkimage: Fix 'Unknown OMAP image type - 5'

[Aneesh V]

Hi Dirk,

On Saturday 06 August 2011 12:12 AM, Dirk Behme wrote:
> From: Dirk Behme<dirk.behme@googlemail.com>
>
> Using mkimage with e.g.
>
> tools/mkimage -A arm -T firmware -O u-boot -d u-boot.bin foo.img
>
> gives a warning
>
> "Unknown OMAP image type - 5"
>
> while it seems that the image itself is created successfully.
>
> This does come from the patch "mkimage: Add OMAP boot image support".
>
> Reordering the init_xx_image_type() sequence does make this
> message go away.

This solves the problem for me too.

Tested-by: Aneesh V <aneesh@ti.com>

best regards,
Aneesh

[U-Boot] [PATCH] mkimage: Fix 'Unknown OMAP image type - 5'

[Albert ARIBAUD]

Hi Dirk,

Le 05/08/2011 20:42, Dirk Behme a ?crit :
> From: Dirk Behme<dirk.behme@googlemail.com>
>
> Using mkimage with e.g.
>
> tools/mkimage -A arm -T firmware -O u-boot -d u-boot.bin foo.img
>
> gives a warning
>
> "Unknown OMAP image type - 5"
>
> while it seems that the image itself is created successfully.
>
> This does come from the patch "mkimage: Add OMAP boot image support".
>
> Reordering the init_xx_image_type() sequence does make this
> message go away.
>
> Signed-off-by: Dirk Behme<dirk.behme@googlemail.com>
> CC: John Rigby<john.rigby@linaro.org>
> CC: Aneesh V<aneesh@ti.com>
> CC: Sandeep Paulraj<s-paulraj@ti.com>
>
> ---
>
> This is reproducable with the recent mainline mkimage where the
> patch "mkimage: Add OMAP boot image support" is applied:
>
> http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commit;h=3decb14abe76d244ba98fd158ef95f89e7e37d70
>
>   tools/mkimage.c |    4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> Index: u-boot.git/tools/mkimage.c
> ===================================================================
> --- u-boot.git.orig/tools/mkimage.c
> +++ u-boot.git/tools/mkimage.c
> @@ -156,12 +156,12 @@ main (int argc, char **argv)
>   	init_imx_image_type ();
>   	/* Init FIT image generation/list support */
>   	init_fit_image_type ();
> -	/* Init TI OMAP Boot image generation/list support */
> -	init_omap_image_type();
>   	/* Init Default image generation/list support */
>   	init_default_image_type ();
>   	/* Init Davinci UBL support */
>   	init_ubl_image_type();
> +	/* Init TI OMAP Boot image generation/list support */
> +	init_omap_image_type();
>
>   	params.cmdname = *argv;
>   	params.addr = params.ep = 0;
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

Any idea why reordering fixes the issue? Seems to me like init functions 
are not / should not be dependent on order, so the "fix" seems fragile 
to me, at least as long as we cannot add a good explanation.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH] mkimage: Fix 'Unknown OMAP image type - 5'

[Dirk Behme]

On 08.08.2011 21:05, Albert ARIBAUD wrote:
> Hi Dirk,
>
> Le 05/08/2011 20:42, Dirk Behme a ?crit :
>> From: Dirk Behme<dirk.behme@googlemail.com>
>>
>> Using mkimage with e.g.
>>
>> tools/mkimage -A arm -T firmware -O u-boot -d u-boot.bin foo.img
>>
>> gives a warning
>>
>> "Unknown OMAP image type - 5"
>>
>> while it seems that the image itself is created successfully.
>>
>> This does come from the patch "mkimage: Add OMAP boot image support".
>>
>> Reordering the init_xx_image_type() sequence does make this
>> message go away.
>>
>> Signed-off-by: Dirk Behme<dirk.behme@googlemail.com>
>> CC: John Rigby<john.rigby@linaro.org>
>> CC: Aneesh V<aneesh@ti.com>
>> CC: Sandeep Paulraj<s-paulraj@ti.com>
>>
>> ---
>>
>> This is reproducable with the recent mainline mkimage where the
>> patch "mkimage: Add OMAP boot image support" is applied:
>>
>> http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commit;h=3decb14abe76d244ba98fd158ef95f89e7e37d70
>>
>>
>> tools/mkimage.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> Index: u-boot.git/tools/mkimage.c
>> ===================================================================
>> --- u-boot.git.orig/tools/mkimage.c
>> +++ u-boot.git/tools/mkimage.c
>> @@ -156,12 +156,12 @@ main (int argc, char **argv)
>> init_imx_image_type ();
>> /* Init FIT image generation/list support */
>> init_fit_image_type ();
>> - /* Init TI OMAP Boot image generation/list support */
>> - init_omap_image_type();
>> /* Init Default image generation/list support */
>> init_default_image_type ();
>> /* Init Davinci UBL support */
>> init_ubl_image_type();
>> + /* Init TI OMAP Boot image generation/list support */
>> + init_omap_image_type();
>>
>> params.cmdname = *argv;
>> params.addr = params.ep = 0;
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>
> Any idea why reordering fixes the issue?

No, unfortunately not. I looked at the other patches touching this 
section of mkimage.c and it looked like everybody is adding new init 
functions below the existing ones. And this helped ;)

I hoped that John or Aneesh would comment on this...

> Seems to me like init
> functions are not / should not be dependent on order, so the "fix"
> seems fragile to me, at least as long as we cannot add a good
> explanation.

Yes, I agree.

John? Aneesh? Any idea?

Best regards

Dirk

[U-Boot] [PATCH] mkimage: Fix 'Unknown OMAP image type - 5'

[John Rigby]

>
>> Seems to me like init
>> functions are not / should not be dependent on order, so the "fix"
>> seems fragile to me, at least as long as we cannot add a good
>> explanation.
>
> Yes, I agree.
>
> John? Aneesh? Any idea?
>
The method check_image_type in image_type_params is supposed to just
return success or failure.  However, for omap it also calls fprintf:

static int omapimage_check_image_types(uint8_t type)
{
	if (type == IH_TYPE_OMAPIMAGE)
		return EXIT_SUCCESS;
	else {
		fprintf(stderr, "Unknown OMAP image type - %x", type);
		return EXIT_FAILURE;
	}
}

I just looked at all the other image checkers and no others have this
so the fix is to simply remove the fprintf.  I obviously did not
understand this when I wrote the original.

John