From patchwork Wed Feb 22 08:14:12 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dirk Behme X-Patchwork-Id: 142418 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from theia.denx.de (theia.denx.de [85.214.87.163]) by ozlabs.org (Postfix) with ESMTP id 06B30B6FBB for ; Wed, 22 Feb 2012 19:16:56 +1100 (EST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 7D5FD28081; Wed, 22 Feb 2012 09:16:52 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id vFYI4YjkPwd0; Wed, 22 Feb 2012 09:16:51 +0100 (CET) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 6C9B52807D; Wed, 22 Feb 2012 09:16:51 +0100 (CET) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 4FDF22807D for ; Wed, 22 Feb 2012 09:16:49 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ujjSL3HuE5QX for ; Wed, 22 Feb 2012 09:16:48 +0100 (CET) X-policyd-weight: NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5 NOT_IN_BL_NJABL=-1.5 (only DNSBL check requested) Received: from smtp2-v.fe.bosch.de (smtp2-v.fe.bosch.de [139.15.237.6]) by theia.denx.de (Postfix) with ESMTPS id D334428078 for ; Wed, 22 Feb 2012 09:16:47 +0100 (CET) Received: from vsmta12.fe.internet.bosch.com (unknown [10.4.98.30]) by imta23.fe.bosch.de (Postfix) with ESMTP id 0535F5820811; Wed, 22 Feb 2012 09:16:47 +0100 (CET) Received: from localhost (vsgw2.fe.internet.bosch.com [10.4.98.13]) by vsmta12.fe.internet.bosch.com (Postfix) with SMTP id E4E721210137; Wed, 22 Feb 2012 09:16:46 +0100 (CET) Received: from FE-HUB1000.de.bosch.com (10.4.103.107) by si-hub05.de.bosch.com (10.3.153.47) with Microsoft SMTP Server (TLS) id 8.3.213.0; Wed, 22 Feb 2012 09:16:24 +0100 Received: from [10.34.217.145] (10.34.217.145) by FE-HUB1000.de.bosch.com (10.4.103.107) with Microsoft SMTP Server id 14.1.355.2; Wed, 22 Feb 2012 09:16:23 +0100 Message-ID: <4F44A3D4.30208@de.bosch.com> Date: Wed, 22 Feb 2012 09:14:12 +0100 From: Dirk Behme Organization: Robert Bosch Car Multimedia GmbH User-Agent: Thunderbird 2.0.0.19 (Windows/20081209) MIME-Version: 1.0 To: stefano babic References: <1329814920-12295-1-git-send-email-dirk.behme@de.bosch.com> <4F43DAEF.8050907@denx.de> <4F43EE05.4090908@googlemail.com> <4F441167.9050003@denx.de> In-Reply-To: <4F441167.9050003@denx.de> X-Originating-IP: [10.34.217.145] Cc: "u-boot@lists.denx.de" Subject: Re: [U-Boot] [PATCH] imximage: header v2: Remove overwriting of flash_offset X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.11 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: u-boot-bounces@lists.denx.de Errors-To: u-boot-bounces@lists.denx.de On 21.02.2012 22:49, stefano babic wrote: > Am 21/02/2012 20:18, schrieb Dirk Behme: > >>> I think we have then a problem when there is not a BOOT_FROM statement >>> in the configuration file, that let imxhdr->flash_offset unset. >>> >>> We need to set it with the default value >> Hmm, my understanding is that there can't be any default value? > > This is desirable, but it does not correspond to the code. > >> What do >> you think would be a default value? The one for SD boot? The one for >> NAND? Or? Who will decide what the default should be? > > Whatever we choose, it is wrong. We can select SD, and one board has not > a SD interface. The same with NAND, ... > > The best one is we have not a default value. Yes, agree. >>> at the beginning of the >>> processing or to raise an error if we want to make this statement >>> mandatory in the configuration file. >> If there can't be a default value, I think to raise an error if no >> BOOT_FROM statement is there is the only option. > > Agree - BOOT_FROM must be mandatory. 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. If this is ok, I will send a v2 of the patch. Best regards Dirk [1] fhdr_v2->header.tag = IVT_HEADER_TAG; /* 0xD1 */ --- a/tools/imximage.c +++ b/tools/imximage.c @@ -253,8 +253,12 @@ static void set_imx_hdr_v2(struct imx_header *imxhdr, uint32_t dcd_len, imx_header_v2_t *hdr_v2 = &imxhdr->header.hdr_v2; flash_header_v2_t *fhdr_v2 = &hdr_v2->fhdr; - /* Set default offset */ - imxhdr->flash_offset = FLASH_OFFSET_STANDARD; + /* Exit if there is no BOOT_FROM field specifying the flash_offset */ + if(imxhdr->flash_offset == 0) { + fprintf(stderr, "Error: Header v2: No BOOT_FROM tag in %s\n", + params->imagename); + exit(EXIT_FAILURE); + } /* Set magic number */