From patchwork Wed Jun 1 14:33:14 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: 98198 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 05325B6F7D for ; Thu, 2 Jun 2011 00:33:28 +1000 (EST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 4F221281AE; Wed, 1 Jun 2011 16:33:26 +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 CliznmzrQrTY; Wed, 1 Jun 2011 16:33:26 +0200 (CEST) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id EA58F28196; Wed, 1 Jun 2011 16:33:24 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id B4BC528196 for ; Wed, 1 Jun 2011 16:33:22 +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 MaWpszrrggUJ for ; Wed, 1 Jun 2011 16:33: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-gy0-f172.google.com (mail-gy0-f172.google.com [209.85.160.172]) by theia.denx.de (Postfix) with ESMTPS id 0762028192 for ; Wed, 1 Jun 2011 16:33:18 +0200 (CEST) Received: by gyf3 with SMTP id 3so2394373gyf.3 for ; Wed, 01 Jun 2011 07:33:15 -0700 (PDT) MIME-Version: 1.0 Received: by 10.91.15.5 with SMTP id s5mr6411259agi.11.1306938795014; Wed, 01 Jun 2011 07:33:15 -0700 (PDT) Received: by 10.90.102.12 with HTTP; Wed, 1 Jun 2011 07:33:14 -0700 (PDT) In-Reply-To: <201105311637.13721.sr@denx.de> References: <201105311510.04012.sr@denx.de> <201105311637.13721.sr@denx.de> Date: Wed, 1 Jun 2011 16:33:14 +0200 Message-ID: From: =?ISO-8859-1?Q?Frank_Svendsb=F8e?= To: Stefan Roese Cc: u-boot@lists.denx.de Subject: Re: [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 Hi Stefan, On Tue, May 31, 2011 at 4:37 PM, Stefan Roese wrote: > Hi Frank, > > On Tuesday 31 May 2011 15:55:56 Frank Svendsbøe wrote: >> > Understood. But why don't you disable write-protection when you first >> > call flash_init()? And enable the write-protection after the chip is >> > correctly detected? >> >> Simply because disabling write-protection is not impossible after >> installation. Our device will be located 3000m below sea level. > > I see. > Hmm.. then you read my mind :) I meant to say "is not possible" and not "is not impossible" :) >> As I explained Mike Frysinger, the write-protection settings is not >> controlled by the PPC device running U-Boot. We can enable >> write-protection in the lab (by setting a jumper), but not write software. >> .. and here I meant to say "explained to", and "but not via software". >> Two ways/levels: 1) A hardware jumper on the factory default flash. 2) >> On the non-factory default flash, write protection is enabled/disabled >> by an FPGA and implicitly and AVR. To make it short, we cannot >> change protection scheme from U-Boot (but we can via an SPI driver I >> wrote for Linux). > .. and yet another self-correction: s/and implicitly and/and implicitly an/ .. guess I had too much coffee yesterday :) > Theoretically also possible with U-Boot. But I understand that you don't want > to do this. > True. We could write a driver for U-Boot that could access the AVR and command the FPGA to disable write protection. But in the case for the "default factory flash" where the flash write protection is enabled by a demounted jumper, and cannot be modified after installation, CFI probing wouldn't work anyway. So I think the use for such a driver would be limited. Note that for normal operation, our system is running from a non-hardware protected flash. But even in this mode, we must command the AVR to enable write access before we're permitted to write to it. This is one of the features supported by the SPI driver running in GNU/Linux. In addition to this, we have the usual CFI/MTD protection scheme where the filesystems are mounted as read-only, etc. > > >> > Why don't you think that you can't access the original function if it's >> > defined as a weak default? This should work just fine, see for example >> > ft_board_setup() in arch/powerpc/cpu/ppc4xx/fdt.c: >> > >> > void __ft_board_setup(void *blob, bd_t *bd) >> > { >> >        ... >> > } >> > void ft_board_setup(void *blob, bd_t *bd) __attribute__((weak, >> > alias("__ft_board_setup"))); >> > >> > >> > And then this weak default is overridden and still referenced in >> > board/amcc/canyonlands/canyonlands.c: >> > >> > void ft_board_setup(void *blob, bd_t *bd) >> > { >> >        __ft_board_setup(blob, bd); >> >        ... >> > >> > >> > So no need for this ifdef in the common CFI driver. Or am I missing >> > something here? >> >> Oh. I didn't knew I could access the function that was overridden by the >> weak attribute. I guess that's the alias is for right? > > Yep. > >> If both can be >> called, I'm happy to remove the ifdef. >> >> I'll test that tomorrow and provide a patch if it works. > > Good luck... > Thanks, this worked for me. Is it ok for you too? #if defined(CONFIG_ENV_IS_IN_FLASH) || defined(CONFIG_ENV_ADDR_REDUND) || (CONFIG_SYS_MONITOR_BASE >= CONFIG_SYS_FLASH_BASE) @@ -2151,7 +2152,8 @@ void flash_protect_default(void) #endif } -unsigned long flash_init (void) +unsigned long flash_init(void) __attribute__((weak, alias("__flash_init"))); +unsigned long __flash_init (void) { unsigned long size = 0; int i; diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index 6039e1f..99360af 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -176,6 +176,7 @@ u64 flash_read64(void *addr)__attribute__((weak, alias("__flash_read64"))); #define flash_read64 __flash_read64 #endif + /*----------------------------------------------------------------------- */