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

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

---
 };

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 */
>  };
>

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.

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