[U-Boot] New item at list end for backwards compatibility

[U-Boot] New item at list end for backwards compatibility

[Marek Vasut]

From: Robert Berger <robert.berger@ReliableEmbeddedSystems.com>

Signed-off-by: Robert Berger <robert.berger@ReliableEmbeddedSystems.com>

I received this off-list from Robert, it's a bugfix to mkimage, where
the IH_TYPE_ enumeration changed recently and broke backward mkimage
backward compatibility.

Peng, can you respin the patch, test it and repost it ? Thanks

---
diff --git a/include/image.h b/include/image.h
index 031c355..866e9f1 100644
--- a/include/image.h
+++ b/include/image.h
@@ -251,7 +251,6 @@ enum {
        IH_TYPE_FLATDT,                 /* Binary Flat Device Tree Blob */
        IH_TYPE_KWBIMAGE,               /* Kirkwood Boot Image          */
        IH_TYPE_IMXIMAGE,               /* Freescale IMXBoot Image      */
-       IH_TYPE_IMX8IMAGE,              /* Freescale IMX8Boot Image     */
        IH_TYPE_UBLIMAGE,               /* Davinci UBL Image            */
        IH_TYPE_OMAPIMAGE,              /* TI OMAP Config Header Image  */
        IH_TYPE_AISIMAGE,               /* TI Davinci AIS Image         */
@@ -278,6 +277,7 @@ enum {
        IH_TYPE_PMMC,            /* TI Power Management Micro-Controller
Firmware */
        IH_TYPE_STM32IMAGE,             /* STMicroelectronics STM32 Image */
        IH_TYPE_SOCFPGAIMAGE_V1,        /* Altera SOCFPGA A10 Preloader */
+       IH_TYPE_IMX8IMAGE,              /* Freescale IMX8Boot Image     */

        IH_TYPE_COUNT,                  /* Number of image types */
 };
-- 
Best regards,
Marek Vasut

[U-Boot] New item at list end for backwards compatibility

[Stefano Babic]

Hi Marek, Robert,

On 05/12/18 15:52, Marek Vasut wrote:
> From: Robert Berger <robert.berger@ReliableEmbeddedSystems.com>
> 
> Signed-off-by: Robert Berger <robert.berger@ReliableEmbeddedSystems.com>
> 
> I received this off-list from Robert, it's a bugfix to mkimage, where
> the IH_TYPE_ enumeration changed recently and broke backward mkimage
> backward compatibility.
> 

That's true - new image type must be always appended for compatible reason.

> Peng, can you respin the patch, test it and repost it ? Thanks

I am reviewing and applying Peng's - but Peng posted a patch for i.MX8M,
patch for i.MX8 was already applied.

If nobody complains, I fix this myself by applying Peng's i.MX8M (not
MX8) patch, I mean this one:

	http://patchwork.ozlabs.org/patch/1000376/

Note: this also breaks compatibility

Regards,
Stefano

> 
> ---
> diff --git a/include/image.h b/include/image.h
> index 031c355..866e9f1 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -251,7 +251,6 @@ enum {
>         IH_TYPE_FLATDT,                 /* Binary Flat Device Tree Blob */
>         IH_TYPE_KWBIMAGE,               /* Kirkwood Boot Image          */
>         IH_TYPE_IMXIMAGE,               /* Freescale IMXBoot Image      */
> -       IH_TYPE_IMX8IMAGE,              /* Freescale IMX8Boot Image     */
>         IH_TYPE_UBLIMAGE,               /* Davinci UBL Image            */
>         IH_TYPE_OMAPIMAGE,              /* TI OMAP Config Header Image  */
>         IH_TYPE_AISIMAGE,               /* TI Davinci AIS Image         */
> @@ -278,6 +277,7 @@ enum {
>         IH_TYPE_PMMC,            /* TI Power Management Micro-Controller
> Firmware */
>         IH_TYPE_STM32IMAGE,             /* STMicroelectronics STM32 Image */
>         IH_TYPE_SOCFPGAIMAGE_V1,        /* Altera SOCFPGA A10 Preloader */
> +       IH_TYPE_IMX8IMAGE,              /* Freescale IMX8Boot Image     */
> 
>         IH_TYPE_COUNT,                  /* Number of image types */
>  };
> 

-- 
=====================================================================
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] New item at list end for backwards compatibility

[Tom Rini]

On Sat, Dec 08, 2018 at 05:26:49PM +0100, Stefano Babic wrote:
> Hi Marek, Robert,
> 
> On 05/12/18 15:52, Marek Vasut wrote:
> > From: Robert Berger <robert.berger@ReliableEmbeddedSystems.com>
> > 
> > Signed-off-by: Robert Berger <robert.berger@ReliableEmbeddedSystems.com>
> > 
> > I received this off-list from Robert, it's a bugfix to mkimage, where
> > the IH_TYPE_ enumeration changed recently and broke backward mkimage
> > backward compatibility.
> > 
> 
> That's true - new image type must be always appended for compatible reason.
> 
> > Peng, can you respin the patch, test it and repost it ? Thanks
> 
> I am reviewing and applying Peng's - but Peng posted a patch for i.MX8M,
> patch for i.MX8 was already applied.
> 
> If nobody complains, I fix this myself by applying Peng's i.MX8M (not
> MX8) patch, I mean this one:
> 
> 	http://patchwork.ozlabs.org/patch/1000376/
> 
> Note: this also breaks compatibility

I think the first problem is that the comment "Do not change values for
backward compatibility." is not clear enough because I see lots of
middle of the list insertions which in turn change all of the values
that follow.  A downside of an enum I suppose.  What we need to do is
yank the IMX8 part out to fix all of the broken images, and put the new
ones at the back, and Cc the various distro folks as they'll want to
make sure to pick this fix up.

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

[U-Boot] New item at list end for backwards compatibility

[Stefano Babic]

Hi Tom,

On 08/12/18 21:21, Tom Rini wrote:
> On Sat, Dec 08, 2018 at 05:26:49PM +0100, Stefano Babic wrote:
>> Hi Marek, Robert,
>>
>> On 05/12/18 15:52, Marek Vasut wrote:
>>> From: Robert Berger <robert.berger@ReliableEmbeddedSystems.com>
>>>
>>> Signed-off-by: Robert Berger <robert.berger@ReliableEmbeddedSystems.com>
>>>
>>> I received this off-list from Robert, it's a bugfix to mkimage, where
>>> the IH_TYPE_ enumeration changed recently and broke backward mkimage
>>> backward compatibility.
>>>
>>
>> That's true - new image type must be always appended for compatible reason.
>>
>>> Peng, can you respin the patch, test it and repost it ? Thanks
>>
>> I am reviewing and applying Peng's - but Peng posted a patch for i.MX8M,
>> patch for i.MX8 was already applied.
>>
>> If nobody complains, I fix this myself by applying Peng's i.MX8M (not
>> MX8) patch, I mean this one:
>>
>> 	http://patchwork.ozlabs.org/patch/1000376/
>>
>> Note: this also breaks compatibility
> 
> I think the first problem is that the comment "Do not change values for
> backward compatibility." is not clear enough because I see lots of
> middle of the list insertions which in turn change all of the values
> that follow.  A downside of an enum I suppose.

Yes, I remember I caused this issue myself some years ago when I added
an image for TI. The comment is weak, maybe this is one of the rare
cases where a strict list of #define with values could be better.

>  What we need to do is
> yank the IMX8 part out to fix all of the broken images, and put the new
> ones at the back,

Right

> and Cc the various distro folks as they'll want to
> make sure to pick this fix up.
> 

Best 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
=====================================================================