[U-Boot] [PATCH v2 01/22] mkimage: Add OMAP boot image support

[Wolfgang Denk]

Dear Aneesh V,

In message <4DD0F98A.2040302 at 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.


> Just curious, what are the issues caused by "__packed__"?

For example, 32 bit data may be accessed in 4 8-bit operations which
may be fatal when accessing device registers.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
God may be subtle, but He isn't plain mean.         - Albert Einstein