Discussion:
[U-Boot] New item at list end for backwards compatibility
Marek Vasut
2018-12-05 14:52:20 UTC
Permalink
From: Robert Berger <***@ReliableEmbeddedSystems.com>

Signed-off-by: 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
Stefano Babic
2018-12-08 16:26:49 UTC
Permalink
Hi Marek, Robert,
Post by Marek Vasut
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.
Post by Marek Vasut
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
Post by Marek Vasut
---
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: ***@denx.de
=====================================================================
Tom Rini
2018-12-08 20:21:35 UTC
Permalink
Post by Stefano Babic
Hi Marek, Robert,
Post by Marek Vasut
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.
Post by Marek Vasut
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
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
Stefano Babic
2018-12-09 10:01:58 UTC
Permalink
Hi Tom,
Post by Tom Rini
Post by Stefano Babic
Hi Marek, Robert,
Post by Marek Vasut
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.
Post by Marek Vasut
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
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.
Post by Tom Rini
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
Post by Tom Rini
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: ***@denx.de
=====================================================================
Loading...