From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dirk Behme Date: Wed, 22 Feb 2012 10:40:46 +0100 Subject: [U-Boot] [PATCH] imximage: header v2: Remove overwriting of flash_offset In-Reply-To: <4F44B567.9010504@denx.de> References: <1329814920-12295-1-git-send-email-dirk.behme@de.bosch.com> <4F43DAEF.8050907@denx.de> <4F43EE05.4090908@googlemail.com> <4F441167.9050003@denx.de> <4F44A3D4.30208@de.bosch.com> <4F44B567.9010504@denx.de> Message-ID: <4F44B81E.7070709@de.bosch.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 22.02.2012 10:29, Stefano Babic wrote: > On 22/02/2012 09:14, Dirk Behme wrote: >> On 21.02.2012 22:49, stefano babic wrote: >>> Am 21/02/2012 20:18, schrieb Dirk Behme: >>> > > Hi Dirk, > >> What do you think about anything like below then [1]? >> >> I looked through the imximage.c code and, well, due to the mixture to >> support the v1 and v2 header format, the execution path isn't the >> cleanest one. So, while it doesn't seem to be the cleanest way to exit >> directly in set_imx_hdr_v2, it seems to be the easiest and best place to >> add this check. Some other functions have some exit() calls, too, so it >> seems to be common practice in this code. > > It is common in all mkimage - when there is an error, it makes no sense > to go on. > > You must also fix this issue for V1 in set_imx_hdr_v1() as well, because > we do not want default value at all. Ok, the V1 topic is new. I can't touch V1 because I don't know anything about it. And I don't have any hardware to test anything V1 related. Even though the V1 code might have a similar issue, it's my understanding that it doesn't hurt there as in V1 there are no flash_offsets != FLASH_OFFSET_STANDARD. Therefore in V1 the existing code works fine (?). Same as the V2 code before Freescale introduced flash offsets which are not FLASH_OFFSET_STANDARD (== 0x400). > I suggest also you do not check > with if(imxhdr->flash_offset == 0), in case Freescale will put a SOC > without an offset in the future. But it is easy to add a value that is > not allowed. If we add something like > > FLASH_OFFSET_UNDEFINED = 0xFF > > or whatever you want that is not 32-bit aligned, we are on the safest side. I will look where the correct location might be to add this. >> If this is ok, I will send a v2 of the patch. I will try to update the V2 header with something like FLASH_OFFSET_UNDEFINED as proposed above and then send a v2 of the patch. Best regards Dirk