[U-Boot] [RFC] runtime mtdparts_default; was: igep00x0: UBIize

Ladislav Michl
2016-06-04T20:48:51Z

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 <asm/arch/mux.h>
#include <asm/arch/sys_proto.h>
#include <asm/mach-types.h>
+#include <linux/mtd/mtd.h>
#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