Hi Wolfgang,
On Monday 16 May 2011 12:36 AM, Wolfgang Denk wrote:
> Dear Aneesh V,
>
> In message<1305472900-4004-2-git-send-email-ane...@ti.com> you wrote:
>> From: John Rigby<john.ri...@linaro.org>
>>
>> Signed-off-by: John Rigby<john.ri...@linaro.org>
>> ---
>> common/image.c | 1 +
>> include/image.h | 1 +
>> tools/Makefile | 2 +
>> tools/mkimage.c | 2 +
>> tools/mkimage.h | 1 +
>> tools/omapimage.c | 229
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>> tools/omapimage.h | 50 ++++++++++++
>> 7 files changed, 286 insertions(+), 0 deletions(-)
>> create mode 100644 tools/omapimage.c
>> create mode 100644 tools/omapimage.h
>>
>> diff --git a/common/image.c b/common/image.c
>> index e542a57..7f6fe1c 100644
>> --- a/common/image.c
>> +++ b/common/image.c
>> @@ -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?
>
>> diff --git a/tools/Makefile b/tools/Makefile
>> index 623f908..a1c4ed7 100644
>> --- a/tools/Makefile
>> +++ b/tools/Makefile
>> @@ -84,6 +84,7 @@ OBJ_FILES-$(CONFIG_CMD_LOADS) += img2srec.o
>> OBJ_FILES-$(CONFIG_INCA_IP) += inca-swap-bytes.o
>> NOPED_OBJ_FILES-y += kwbimage.o
>> NOPED_OBJ_FILES-y += imximage.o
>> +NOPED_OBJ_FILES-y += omapimage.o
>> NOPED_OBJ_FILES-y += mkimage.o
>> OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o
>> NOPED_OBJ_FILES-y += os_support.o
>> @@ -180,6 +181,7 @@ $(obj)mkimage$(SFX): $(obj)crc32.o \
>> $(obj)fit_image.o \
>> $(obj)image.o \
>> $(obj)imximage.o \
>> + $(obj)omapimage.o \
>> $(obj)kwbimage.o \
>> $(obj)md5.o \
>> $(obj)mkimage.o \
>
> Please keep lists sorted.
Ok.
>
>> --- /dev/null
>> +++ b/tools/omapimage.c
>> @@ -0,0 +1,229 @@
...
>> +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.
Just curious, what are the issues caused by "__packed__"?
>
> Best regards,
>
> Wolfgang Denk
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
