Hello Stefan,
On Wed, 15 Jul 2015 14:36:16 +0200, Stefan Agner stefan@agner.ch wrote:
As far as I see we have these two patches which fix Vybrid booting:
one which changes the boot_data_t /size/ which keeps the offset of dcd_v2_t and the image aligned.
one which changes the dcd_v2_t /size/ which compensates the unaligned size of boot_data_t and keeps the image aligned.
Seems to me that the conclusion is that the actual alignment of boot_data_t does not matter and that only the alignment of the whole imx_header_v2_t size (and, consequently, offset) matters.
How about just adding an attribute((align(8))) to imx_header_v2_t?
Hm this sounds tempting, but it does not seem to work here. I think because it only aligns the beginning of imx_header_v2_t in to 8-byte, however it does not align the size of the whole struct to 8 bytes, I guess? Hence the header size is still "unaligned".
Correct -- my fault, this aligns the address, not size, and there is no attribute that will control size alignment.
Actually I just discovered that "aligned" also changes the size (make sure using the correct syntax "__attribute__((aligned(8)))").
Does it? Then I too was under the same misconception that it did not.
However, imximage.c seems to do some calculations using sizeof(struct imx_header) and sizeof(imx_header_v2_t) which lead to this error message: ./tools/mkimage: header error
Adding __attribute__((aligned(8))) to struct imx_header seems not to alleviate the problem... Any idea?
So we really need to add manual padding.
And if we want to not introduce any artificial constaints, I suggest we define the padding as a separate field in the overall structure, i.e.:
typedef struct { flash_header_v2_t fhdr; boot_data_t boot_data; dcd_v2_t dcd_table; uint32_t padding; /* make size an 8-byte multiple */ } imx_header_v2_t;
That change would also sound good to me.
I think this is the 'least inexact' solution: it does not enforce any address or size alignment constraint that is not defined in the RM, and it does show that the constraint is not on the boot data or DCD but on the header as a whole.
Functionally, it is identical to what I did, so I'm pretty sure it works. :)
Agreed.
Ok, so let me look into the __attribute__((aligned(8))) issue. If that can be solved, I would give it my preference; if it cannot, then I will settle for manually padding the end of imx_header_v2_t.
-- Stefan
Amicalement,