public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Dirk Behme <dirk.behme@de.bosch.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] imximage: header v2: Remove overwriting of	flash_offset
Date: Wed, 22 Feb 2012 10:40:46 +0100	[thread overview]
Message-ID: <4F44B81E.7070709@de.bosch.com> (raw)
In-Reply-To: <4F44B567.9010504@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

  reply	other threads:[~2012-02-22  9:40 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-21  9:02 [U-Boot] [PATCH] imximage: header v2: Remove overwriting of flash_offset Dirk Behme
2012-02-21 17:57 ` Stefano Babic
2012-02-21 19:18   ` Dirk Behme
2012-02-21 21:49     ` stefano babic
2012-02-22  8:14       ` Dirk Behme
2012-02-22  9:29         ` Stefano Babic
2012-02-22  9:40           ` Dirk Behme [this message]
2012-02-22  9:52             ` Stefano Babic

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F44B81E.7070709@de.bosch.com \
    --to=dirk.behme@de.bosch.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox