Hi Wolfgang,
On Monday 16 May 2011 05:18 PM, Wolfgang Denk wrote:
Dear Aneesh V,
In message4DD0F98A.2040302@ti.com you wrote:
@@ -141,6 +141,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_FLATDT, "flat_dt", "Flat Device Tree", }, { IH_TYPE_KWBIMAGE, "kwbimage", "Kirkwood Boot Image",}, { IH_TYPE_IMXIMAGE, "imximage", "Freescale i.MX Boot Image",},
- { IH_TYPE_OMAPIMAGE, "omapimage", "TI OMAP CH/GP Boot Image",}, { -1, "", "", },
Please keep list sorted / sort list.
Sort by the second field(kwbimage, omapimage etc), right?
First field, but the result is the same.
+struct ch_toc {
- uint32_t section_offset;
- uint32_t section_size;
- uint8_t unused[12];
- uint8_t section_name[12];
+} __attribute__ ((__packed__));
+struct ch_settings {
- uint32_t section_key;
- uint8_t valid;
- uint8_t version;
- uint16_t reserved;
- uint32_t flags;
+} __attribute__ ((__packed__));
+struct gp_header {
- uint32_t size;
- uint32_t load_addr;
+} __attribute__ ((__packed__));
Is there any good reason to have these "__attribute__ ((__packed__))" here? In general, these are only known to cause trouble and pain, and I cannot see a need here.
ROM code expects the header in a precise format. I think it will not be safe to leave it to the compiler to decide the field layout. For instance, the compiler may align the uint8_t or uint16_t to 32 bit boundary and that will break the Configuration Header.
No. Not in the structs listed above.
Why do you think it will not create any problems. For instance, what if the field "uint8_t version" in "struct ch_settings" is aligned to a 32 bit boundary by the compiler for faster access? That is not the intended layout.
best regards, Aneesh