From patchwork Tue May 31 08:35:17 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Frank_Svendsb=C3=B8e?= X-Patchwork-Id: 97977 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 4DF5FB6F74 for ; Tue, 31 May 2011 18:35:31 +1000 (EST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 6D24F28126; Tue, 31 May 2011 10:35:28 +0200 (CEST) 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 TmiHN5qEbMeQ; Tue, 31 May 2011 10:35:28 +0200 (CEST) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 470F528152; Tue, 31 May 2011 10:35:26 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 96D6528152 for ; Tue, 31 May 2011 10:35:23 +0200 (CEST) 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 WUoBQWhNpxkC for ; Tue, 31 May 2011 10:35:21 +0200 (CEST) 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 mail-gw0-f44.google.com (mail-gw0-f44.google.com [74.125.83.44]) by theia.denx.de (Postfix) with ESMTPS id 1337428126 for ; Tue, 31 May 2011 10:35:18 +0200 (CEST) Received: by gwb20 with SMTP id 20so1808641gwb.3 for ; Tue, 31 May 2011 01:35:17 -0700 (PDT) MIME-Version: 1.0 Received: by 10.91.69.4 with SMTP id w4mr4724659agk.173.1306830917372; Tue, 31 May 2011 01:35:17 -0700 (PDT) Received: by 10.90.102.12 with HTTP; Tue, 31 May 2011 01:35:17 -0700 (PDT) Date: Tue, 31 May 2011 10:35:17 +0200 Message-ID: From: =?ISO-8859-1?Q?Frank_Svendsb=F8e?= To: u-boot@lists.denx.de Subject: [U-Boot] Reg. CFI flash_init and hardware write protected devices X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.9 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 We have a board that feature NOR flash and hardware write protection is handled by controlling the write enable pin. When write protection is enabled, the nWE pin is forced high by external logic. The memory controller and/or CFI logic is unaware of this, and since CFI uses write enable as part of the CFI command set, a CFI probing will fail when write protection is enabled. The rationale for controlling nWE and not WP (write protection) is that WP only protects the first sector of the device. In our case this is less than the size of the bootloader (U-boot), since that occupies two sectors. Due to this the built-in NOR write protection is rather useless. Our current solution based on controlling nWE is to hardcode flash geometry in board code when flash protection is enabled. In order to use CFI as intended when write protection is disabled, we call the generic flash_init function as defined in drivers/mtd/cfi_flash.c. When protection is enabled we hardcode the geometry info in board code. In order separate our flash_init and the generic flash_init, and be able to call both, we've introduced a new ifdef to cfi_flash.c called CONFIG_CFI_FLASH_OVERRIDE_INIT. Like this: ---- #if defined(CONFIG_ENV_IS_IN_FLASH) || defined(CONFIG_ENV_ADDR_REDUND) || (CONFIG_SYS_MONITOR_BASE >= ---- Now, in board code our redefined flash_init will be called. But if write protection is off, we call the original function, eg. __flash_init. Using the preprocessor is often considered bad design. However, the alternatives here such as adding a weak attribute to flash_init will not make us able to call the generic/original function. Therefore we consider adding an ifdef as better design than making the function weak, and it'll reduce the amount of redundant code in board code. What do you think about this? Best regards, Frank E. Svendsbøe diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index 6039e1f..772096e 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -176,6 +176,10 @@ u64 flash_read64(void *addr)__attribute__((weak, alias("__flash_read64"))); #define flash_read64 __flash_read64 #endif +#ifdef CONFIG_CFI_FLASH_OVERRIDE_INIT +#define flash_init __flash_init +#endif + /*----------------------------------------------------------------------- */