From patchwork Sat Jun 4 20:48:51 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ladislav Michl X-Patchwork-Id: 630281 X-Patchwork-Delegate: scottwood@freescale.com 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 3rMY3V4MFHz9t69 for ; Sun, 5 Jun 2016 06:49:10 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 700DB4B98A; Sat, 4 Jun 2016 22:49:08 +0200 (CEST) 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 gtVEcfGIuk6R; Sat, 4 Jun 2016 22:49:08 +0200 (CEST) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id C989C4B91E; Sat, 4 Jun 2016 22:49:07 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id ABDCD4B91E for ; Sat, 4 Jun 2016 22:49:03 +0200 (CEST) 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 a8RIofV5Sdts for ; Sat, 4 Jun 2016 22:49:03 +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 cvs.linux-mips.org (eddie.linux-mips.org [148.251.95.138]) by theia.denx.de (Postfix) with ESMTP id A44804B811 for ; Sat, 4 Jun 2016 22:48:59 +0200 (CEST) Received: (from localhost user: 'ladis' uid#1021 fake: STDIN (ladis@eddie.linux-mips.org)) by eddie.linux-mips.org id S27042246AbcFDUs6wuPj8 (ORCPT ); Sat, 4 Jun 2016 22:48:58 +0200 Date: Sat, 4 Jun 2016 22:48:51 +0200 From: Ladislav Michl To: Enric Balletbo Serra Message-ID: <20160604204851.GA19032@localhost.localdomain> References: <20160117030929.GA28493@localhost.localdomain> <20160117031603.GG28493@localhost.localdomain> <56A0759F.4050509@denx.de> <56A5C30E.5070401@denx.de> <20160125155651.GA2245@localhost.localdomain> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20160125155651.GA2245@localhost.localdomain> User-Agent: Mutt/1.5.23 (2014-03-12) Cc: Scott Wood , U-Boot Mailing List , Richard Weinberger , Tom Rini Subject: [U-Boot] [RFC] runtime mtdparts_default; was: igep00x0: UBIize X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.15 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" Hi Enric and all, On Mon, Jan 25, 2016 at 04:56:51PM +0100, Ladislav Michl wrote: > Hi Enric, > > On Mon, Jan 25, 2016 at 08:26:23AM +0100, Enric Balletbo Serra wrote: > > The ROM boot on OMAP reads the first 4 blocks searching for the SPL, > > in production is a practice flash the SPL 4 times. OneNAND/NAND > > devices can have different block sizes and the OMAP ROM boot supports > > block sizes from 64KB to 512K. For IGEP boards in particular, at least > > there are boards that have block size of 128K and 256K. What I would > > meant here is set as default the mtdparts variable to reserve 2M for > > SPL, just to cover all the cases. > > > > mtdparts=mtdparts=gpmc-nand.0:2m(SPL),-(UBI)\0 > > So far there was no ack or nack to yesterday's proposal making that > dynamic; Both boot ROM and ubispl code thinks in terms of eraseblocks, > only mtd needs fixed offset. So I'd like to see this offset calculated as > 4*block_size, not some "worst case" value... So to make mtdparts_default runtime configurable, we need to fill it before relocating env, but after mtd devices get probed, see curent hack in code. Do we want some generic way of producing board specific mtdpart defaults? Patch bellow gives some clue how it could work, but implementation is not nice. Would something like that in principle acceptable (either weak function or config option)? Btw, current mtdparts init code is broken for non-relocated env as it assigns parts name pointer to tmp_parts, so later check for that pointer being null (env variable not found) does not really check for that variable. I'll send separate patch for that. Best regards, ladis PS: patch is based on the top of ubi spl series... diff --git a/board/isee/igep00x0/igep00x0.c b/board/isee/igep00x0/igep00x0.c index 759daef..b5b5cc4 100644 --- a/board/isee/igep00x0/igep00x0.c +++ b/board/isee/igep00x0/igep00x0.c @@ -18,6 +18,7 @@ #include #include #include +#include #include "igep00x0.h" DECLARE_GLOBAL_DATA_PTR; @@ -167,6 +168,23 @@ void set_fdt(void) } } +char igep00x0_mtdids[24]; +char igep00x0_mtdparts[48]; + +void mtdids_generate(void) +{ + struct mtd_info *mtd = get_mtd_device(NULL, 0); + if (mtd) { + const char *linux_name = "omap2-nand"; + snprintf(igep00x0_mtdids, sizeof(igep00x0_mtdids), + "%s=%s", + mtd->name, linux_name); + snprintf(igep00x0_mtdparts, sizeof(igep00x0_mtdparts), + "mtdparts=%s:%dk(SPL),-(UBI)", + linux_name, 4 * mtd->erasesize >> 10); + } +} + /* * Routine: misc_init_r * Description: Configure board specific parts diff --git a/cmd/mtdparts.c b/cmd/mtdparts.c index 44b2c3a..d05b8cf 100644 --- a/cmd/mtdparts.c +++ b/cmd/mtdparts.c @@ -110,15 +110,15 @@ DECLARE_GLOBAL_DATA_PTR; /* default values for mtdids and mtdparts variables */ #if defined(MTDIDS_DEFAULT) -static const char *const mtdids_default = MTDIDS_DEFAULT; +static char *mtdids_default = MTDIDS_DEFAULT; #else -static const char *const mtdids_default = NULL; +static char *mtdids_default = NULL; #endif #if defined(MTDPARTS_DEFAULT) -static const char *const mtdparts_default = MTDPARTS_DEFAULT; +static char *mtdparts_default = MTDPARTS_DEFAULT; #else -static const char *const mtdparts_default = NULL; +static char *mtdparts_default = NULL; #endif /* copies of last seen 'mtdids', 'mtdparts' and 'partition' env variables */ @@ -1524,7 +1524,7 @@ static int spread_partitions(void) */ static int parse_mtdparts(const char *const mtdparts) { - const char *p = mtdparts; + const char *p = NULL; struct mtd_device *dev; int err = 1; char tmp_parts[MTDPARTS_MAXLEN]; @@ -1541,10 +1541,13 @@ static int parse_mtdparts(const char *const mtdparts) if (gd->flags & GD_FLG_ENV_READY) { p = getenv("mtdparts"); } else { - p = tmp_parts; - getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN); + if (getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN) != -1) + p = tmp_parts; } + if (!p) + p = mtdparts; + if (strncmp(p, "mtdparts=", 9) != 0) { printf("mtdparts variable doesn't start with 'mtdparts='\n"); return err; @@ -1720,11 +1723,13 @@ int mtdparts_init(void) * before the env is relocated, then we need to use our own stack * buffer. gd->env_buf will be too small. */ - if (gd->flags & GD_FLG_ENV_READY) { + if (gd->flags & GD_FLG_ENV_READY) parts = getenv("mtdparts"); - } else { - parts = tmp_parts; - getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN); + else { + if (getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN) == -1) + parts = NULL; + else + parts = tmp_parts; } current_partition = getenv("partition"); @@ -1758,10 +1763,14 @@ int mtdparts_init(void) return 1; } - /* do no try to use defaults when mtdparts variable is not defined, - * just check the length */ - if (!parts) - printf("mtdparts variable not set, see 'help mtdparts'\n"); + /* use defaults when mtdparts variable is not defined, but do not save + * it to environment */ + if (!parts) { + if (mtdparts_default) + parts = mtdparts_default; + else + printf("mtdparts variable not set, see 'help mtdparts'\n"); + } if (parts && (strlen(parts) > MTDPARTS_MAXLEN - 1)) { printf("mtdparts too long (> %d)\n", MTDPARTS_MAXLEN); diff --git a/common/board_r.c b/common/board_r.c index d959ad3..b9b2d7f 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -491,8 +491,12 @@ static int should_load_env(void) #endif } +extern void mtdids_generate(void); + static int initr_env(void) { + mtdids_generate(); + /* initialize environment */ if (should_load_env()) env_relocate(); diff --git a/include/configs/omap3_igep00x0.h b/include/configs/omap3_igep00x0.h index c9157a8..213525d 100644 --- a/include/configs/omap3_igep00x0.h +++ b/include/configs/omap3_igep00x0.h @@ -90,10 +90,6 @@ "stdout=serial\0" \ "stderr=serial\0" -#define ENV_MTD_SETTINGS \ - "mtdids=nand0=gpmc-nand.0\0" \ - "mtdparts=mtdparts=gpmc-nand.0:512k(SPL),-(UBI)\0" - #define MEM_LAYOUT_SETTINGS \ DEFAULT_LINUX_BOOT_ENV \ "scriptaddr=0x87E00000\0" \ @@ -106,7 +102,6 @@ #define CONFIG_EXTRA_ENV_SETTINGS \ ENV_DEVICE_SETTINGS \ - ENV_MTD_SETTINGS \ MEM_LAYOUT_SETTINGS \ BOOTENV @@ -151,8 +146,12 @@ #define CONFIG_RBTREE #define CONFIG_MTD_PARTITIONS -#define MTDIDS_DEFAULT "nand0=gpmc-nand.0" -#define MTDPARTS_DEFAULT "mtdparts=gpmc-nand.0:512k(SPL),-(UBI)" +#ifndef __ASSEMBLY__ +extern char igep00x0_mtdids[]; +extern char igep00x0_mtdparts[]; +#endif +#define MTDIDS_DEFAULT igep00x0_mtdids +#define MTDPARTS_DEFAULT igep00x0_mtdparts /* OneNAND boot config */ #ifdef CONFIG_BOOT_ONENAND