[U-Boot] [PATCH 1/2] configs: igep: Define default mtdids/mtdparts

[U-Boot] [PATCH 1/2] configs: igep: Define default mtdids/mtdparts

[Boris Brezillon]

We are trying to get rid of the weak board_mtdparts_default() function
and we need to make sure igep defconfigs have proper proper
CONFIG_MTD{IDS,PARTS}_DEFAULT before doing that.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
 configs/igep0032_defconfig | 2 ++
 configs/igep00x0_defconfig | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/configs/igep0032_defconfig b/configs/igep0032_defconfig
index 383648789c53..d2a614c98f6d 100644
--- a/configs/igep0032_defconfig
+++ b/configs/igep0032_defconfig
@@ -28,6 +28,8 @@ CONFIG_CMD_SPI=y
 CONFIG_CMD_CACHE=y
 CONFIG_CMD_EXT4_WRITE=y
 CONFIG_CMD_MTDPARTS=y
+CONFIG_MTDIDS_DEFAULT="nand0=omap2-nand,onenand0=omap2-onenand"
+CONFIG_MTDPARTS_DEFAULT="omap2-nand:512k(SPL),-(UBI);omap2-onenand:512k(SPL),-(UBI)"
 CONFIG_CMD_UBI=y
 # CONFIG_CMD_UBIFS is not set
 CONFIG_NET_RANDOM_ETHADDR=y
diff --git a/configs/igep00x0_defconfig b/configs/igep00x0_defconfig
index f2989e34e12e..5d3e109ee3c2 100644
--- a/configs/igep00x0_defconfig
+++ b/configs/igep00x0_defconfig
@@ -28,6 +28,8 @@ CONFIG_CMD_SPI=y
 CONFIG_CMD_CACHE=y
 CONFIG_CMD_EXT4_WRITE=y
 CONFIG_CMD_MTDPARTS=y
+CONFIG_MTDIDS_DEFAULT="nand0=omap2-nand,onenand0=omap2-onenand"
+CONFIG_MTDPARTS_DEFAULT="omap2-nand:512k(SPL),-(UBI);omap2-onenand:512k(SPL),-(UBI)"
 CONFIG_CMD_UBI=y
 # CONFIG_CMD_UBIFS is not set
 CONFIG_NET_RANDOM_ETHADDR=y
-- 
2.17.1

[U-Boot] [PATCH 2/2] mtd: Get rid of board_mtdparts_default()

[Boris Brezillon]

The only implementer of this function has been patched to use
CONFIG_MTD{IDS,PARTS}_DEFAULT instead. Let's get rid of this function
and the associated CONFIG_SYS_MTDPARTS_RUNTIME option.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
 board/isee/igep00x0/igep00x0.c   | 17 -----------------
 cmd/mtdparts.c                   |  6 ------
 drivers/mtd/mtd_uboot.c          | 10 ++--------
 include/configs/omap3_igep00x0.h |  2 --
 scripts/config_whitelist.txt     |  1 -
 5 files changed, 2 insertions(+), 34 deletions(-)

diff --git a/board/isee/igep00x0/igep00x0.c b/board/isee/igep00x0/igep00x0.c
index 367af82d4a16..3552be6f3902 100644
--- a/board/isee/igep00x0/igep00x0.c
+++ b/board/isee/igep00x0/igep00x0.c
@@ -239,20 +239,3 @@ int misc_init_r(void)
 
 	return 0;
 }
-
-void board_mtdparts_default(const char **mtdids, const char **mtdparts)
-{
-	struct mtd_info *mtd = get_mtd_device(NULL, 0);
-	if (mtd) {
-		static char ids[24];
-		static char parts[48];
-		const char *linux_name = "omap2-nand";
-		if (strncmp(mtd->name, "onenand0", 8) == 0)
-			linux_name = "omap2-onenand";
-		snprintf(ids, sizeof(ids), "%s=%s", mtd->name, linux_name);
-		snprintf(parts, sizeof(parts), "mtdparts=%s:%dk(SPL),-(UBI)",
-		         linux_name, 4 * mtd->erasesize >> 10);
-		*mtdids = ids;
-		*mtdparts = parts;
-	}
-}
diff --git a/cmd/mtdparts.c b/cmd/mtdparts.c
index f7ed1a077974..6b5644523898 100644
--- a/cmd/mtdparts.c
+++ b/cmd/mtdparts.c
@@ -122,9 +122,6 @@ DECLARE_GLOBAL_DATA_PTR;
 #define MTDPARTS_DEFAULT NULL
 #endif
 #endif
-#if defined(CONFIG_SYS_MTDPARTS_RUNTIME)
-extern void board_mtdparts_default(const char **mtdids, const char **mtdparts);
-#endif
 static const char *mtdids_default = MTDIDS_DEFAULT;
 static const char *mtdparts_default = MTDPARTS_DEFAULT;
 
@@ -1733,9 +1730,6 @@ int mtdparts_init(void)
 		memset(last_ids, 0, sizeof(last_ids));
 		memset(last_parts, 0, sizeof(last_parts));
 		memset(last_partition, 0, sizeof(last_partition));
-#if defined(CONFIG_SYS_MTDPARTS_RUNTIME)
-		board_mtdparts_default(&mtdids_default, &mtdparts_default);
-#endif
 		use_defaults = 1;
 		initialized = 1;
 	}
diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
index d638f700d041..ed619abac390 100644
--- a/drivers/mtd/mtd_uboot.c
+++ b/drivers/mtd/mtd_uboot.c
@@ -13,8 +13,6 @@
 
 #define MTD_NAME_MAX_LEN 20
 
-void board_mtdparts_default(const char **mtdids, const char **mtdparts);
-
 static const char *get_mtdids(void)
 {
 	__maybe_unused const char *mtdparts = NULL;
@@ -23,9 +21,7 @@ static const char *get_mtdids(void)
 	if (mtdids)
 		return mtdids;
 
-#if defined(CONFIG_SYS_MTDPARTS_RUNTIME)
-	board_mtdparts_default(&mtdids, &mtdparts);
-#elif defined(MTDIDS_DEFAULT)
+#if defined(MTDIDS_DEFAULT)
 	mtdids = MTDIDS_DEFAULT;
 #elif defined(CONFIG_MTDIDS_DEFAULT)
 	mtdids = CONFIG_MTDIDS_DEFAULT;
@@ -133,9 +129,7 @@ static const char *get_mtdparts(void)
 	if (mtdparts || !use_defaults)
 		return mtdparts;
 
-#if defined(CONFIG_SYS_MTDPARTS_RUNTIME)
-	board_mtdparts_default(&mtdids, &mtdparts);
-#elif defined(MTDPARTS_DEFAULT)
+#if defined(MTDPARTS_DEFAULT)
 	mtdparts = MTDPARTS_DEFAULT;
 #elif defined(CONFIG_MTDPARTS_DEFAULT)
 	mtdparts = CONFIG_MTDPARTS_DEFAULT;
diff --git a/include/configs/omap3_igep00x0.h b/include/configs/omap3_igep00x0.h
index b9d65697521b..280a094cdbae 100644
--- a/include/configs/omap3_igep00x0.h
+++ b/include/configs/omap3_igep00x0.h
@@ -87,8 +87,6 @@
 
 #endif
 
-#define CONFIG_SYS_MTDPARTS_RUNTIME
-
 /* OneNAND config */
 #define CONFIG_USE_ONENAND_BOARD_INIT
 #define CONFIG_SYS_ONENAND_BASE		ONENAND_MAP
diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
index b8addeaf693a..72608071c486 100644
--- a/scripts/config_whitelist.txt
+++ b/scripts/config_whitelist.txt
@@ -3511,7 +3511,6 @@ CONFIG_SYS_MRAM_SIZE
 CONFIG_SYS_MSC0_VAL
 CONFIG_SYS_MSC1_VAL
 CONFIG_SYS_MSC2_VAL
-CONFIG_SYS_MTDPARTS_RUNTIME
 CONFIG_SYS_MX5_CLK32
 CONFIG_SYS_MX5_HCLK
 CONFIG_SYS_MX6_CLK32
-- 
2.17.1

[U-Boot] [PATCH 2/2] mtd: Get rid of board_mtdparts_default()

[Enric Balletbo Serra]

+Ladis who might be also interested.
Missatge de Boris Brezillon <boris.brezillon@bootlin.com> del dia dl.,
10 de des. 2018 a les 16:38:
>
> The only implementer of this function has been patched to use
> CONFIG_MTD{IDS,PARTS}_DEFAULT instead. Let's get rid of this function
> and the associated CONFIG_SYS_MTDPARTS_RUNTIME option.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
>  board/isee/igep00x0/igep00x0.c   | 17 -----------------
>  cmd/mtdparts.c                   |  6 ------
>  drivers/mtd/mtd_uboot.c          | 10 ++--------
>  include/configs/omap3_igep00x0.h |  2 --
>  scripts/config_whitelist.txt     |  1 -
>  5 files changed, 2 insertions(+), 34 deletions(-)
>
> diff --git a/board/isee/igep00x0/igep00x0.c b/board/isee/igep00x0/igep00x0.c
> index 367af82d4a16..3552be6f3902 100644
> --- a/board/isee/igep00x0/igep00x0.c
> +++ b/board/isee/igep00x0/igep00x0.c
> @@ -239,20 +239,3 @@ int misc_init_r(void)
>
>         return 0;
>  }
> -
> -void board_mtdparts_default(const char **mtdids, const char **mtdparts)
> -{
> -       struct mtd_info *mtd = get_mtd_device(NULL, 0);
> -       if (mtd) {
> -               static char ids[24];
> -               static char parts[48];
> -               const char *linux_name = "omap2-nand";
> -               if (strncmp(mtd->name, "onenand0", 8) == 0)
> -                       linux_name = "omap2-onenand";
> -               snprintf(ids, sizeof(ids), "%s=%s", mtd->name, linux_name);
> -               snprintf(parts, sizeof(parts), "mtdparts=%s:%dk(SPL),-(UBI)",
> -                        linux_name, 4 * mtd->erasesize >> 10);
> -               *mtdids = ids;
> -               *mtdparts = parts;
> -       }
> -}
> diff --git a/cmd/mtdparts.c b/cmd/mtdparts.c
> index f7ed1a077974..6b5644523898 100644
> --- a/cmd/mtdparts.c
> +++ b/cmd/mtdparts.c
> @@ -122,9 +122,6 @@ DECLARE_GLOBAL_DATA_PTR;
>  #define MTDPARTS_DEFAULT NULL
>  #endif
>  #endif
> -#if defined(CONFIG_SYS_MTDPARTS_RUNTIME)
> -extern void board_mtdparts_default(const char **mtdids, const char **mtdparts);
> -#endif
>  static const char *mtdids_default = MTDIDS_DEFAULT;
>  static const char *mtdparts_default = MTDPARTS_DEFAULT;
>
> @@ -1733,9 +1730,6 @@ int mtdparts_init(void)
>                 memset(last_ids, 0, sizeof(last_ids));
>                 memset(last_parts, 0, sizeof(last_parts));
>                 memset(last_partition, 0, sizeof(last_partition));
> -#if defined(CONFIG_SYS_MTDPARTS_RUNTIME)
> -               board_mtdparts_default(&mtdids_default, &mtdparts_default);
> -#endif
>                 use_defaults = 1;
>                 initialized = 1;
>         }
> diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
> index d638f700d041..ed619abac390 100644
> --- a/drivers/mtd/mtd_uboot.c
> +++ b/drivers/mtd/mtd_uboot.c
> @@ -13,8 +13,6 @@
>
>  #define MTD_NAME_MAX_LEN 20
>
> -void board_mtdparts_default(const char **mtdids, const char **mtdparts);
> -
>  static const char *get_mtdids(void)
>  {
>         __maybe_unused const char *mtdparts = NULL;
> @@ -23,9 +21,7 @@ static const char *get_mtdids(void)
>         if (mtdids)
>                 return mtdids;
>
> -#if defined(CONFIG_SYS_MTDPARTS_RUNTIME)
> -       board_mtdparts_default(&mtdids, &mtdparts);
> -#elif defined(MTDIDS_DEFAULT)
> +#if defined(MTDIDS_DEFAULT)
>         mtdids = MTDIDS_DEFAULT;
>  #elif defined(CONFIG_MTDIDS_DEFAULT)
>         mtdids = CONFIG_MTDIDS_DEFAULT;
> @@ -133,9 +129,7 @@ static const char *get_mtdparts(void)
>         if (mtdparts || !use_defaults)
>                 return mtdparts;
>
> -#if defined(CONFIG_SYS_MTDPARTS_RUNTIME)
> -       board_mtdparts_default(&mtdids, &mtdparts);
> -#elif defined(MTDPARTS_DEFAULT)
> +#if defined(MTDPARTS_DEFAULT)
>         mtdparts = MTDPARTS_DEFAULT;
>  #elif defined(CONFIG_MTDPARTS_DEFAULT)
>         mtdparts = CONFIG_MTDPARTS_DEFAULT;
> diff --git a/include/configs/omap3_igep00x0.h b/include/configs/omap3_igep00x0.h
> index b9d65697521b..280a094cdbae 100644
> --- a/include/configs/omap3_igep00x0.h
> +++ b/include/configs/omap3_igep00x0.h
> @@ -87,8 +87,6 @@
>
>  #endif
>
> -#define CONFIG_SYS_MTDPARTS_RUNTIME
> -
>  /* OneNAND config */
>  #define CONFIG_USE_ONENAND_BOARD_INIT
>  #define CONFIG_SYS_ONENAND_BASE                ONENAND_MAP
> diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
> index b8addeaf693a..72608071c486 100644
> --- a/scripts/config_whitelist.txt
> +++ b/scripts/config_whitelist.txt
> @@ -3511,7 +3511,6 @@ CONFIG_SYS_MRAM_SIZE
>  CONFIG_SYS_MSC0_VAL
>  CONFIG_SYS_MSC1_VAL
>  CONFIG_SYS_MSC2_VAL
> -CONFIG_SYS_MTDPARTS_RUNTIME
>  CONFIG_SYS_MX5_CLK32
>  CONFIG_SYS_MX5_HCLK
>  CONFIG_SYS_MX6_CLK32
> --
> 2.17.1
>

[U-Boot] [PATCH 2/2] mtd: Get rid of board_mtdparts_default()

[Ladislav Michl]

Hi Boris,

On Mon, Dec 10, 2018 at 04:38:50PM +0100, Boris Brezillon wrote:
> The only implementer of this function has been patched to use
> CONFIG_MTD{IDS,PARTS}_DEFAULT instead. Let's get rid of this function
> and the associated CONFIG_SYS_MTDPARTS_RUNTIME option.

the only implementer of this fuction did so for a good reason. What is
a motivation to remove it?

The requirement is to be able to use single u-boot binary on all igep2
boards ever produced. These comes with various NAND and OneNAND chips
and  I was not able to come with single static partition configuration
to support them all. Hence runtime detection. That code could be used
on all OMAP3 boards as BootROM reads up to first four sectors searching
for SPL (MLO).

Thank you,
	ladis

> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
>  board/isee/igep00x0/igep00x0.c   | 17 -----------------
>  cmd/mtdparts.c                   |  6 ------
>  drivers/mtd/mtd_uboot.c          | 10 ++--------
>  include/configs/omap3_igep00x0.h |  2 --
>  scripts/config_whitelist.txt     |  1 -
>  5 files changed, 2 insertions(+), 34 deletions(-)
> 
> diff --git a/board/isee/igep00x0/igep00x0.c b/board/isee/igep00x0/igep00x0.c
> index 367af82d4a16..3552be6f3902 100644
> --- a/board/isee/igep00x0/igep00x0.c
> +++ b/board/isee/igep00x0/igep00x0.c
> @@ -239,20 +239,3 @@ int misc_init_r(void)
>  
>  	return 0;
>  }
> -
> -void board_mtdparts_default(const char **mtdids, const char **mtdparts)
> -{
> -	struct mtd_info *mtd = get_mtd_device(NULL, 0);
> -	if (mtd) {
> -		static char ids[24];
> -		static char parts[48];
> -		const char *linux_name = "omap2-nand";
> -		if (strncmp(mtd->name, "onenand0", 8) == 0)
> -			linux_name = "omap2-onenand";
> -		snprintf(ids, sizeof(ids), "%s=%s", mtd->name, linux_name);
> -		snprintf(parts, sizeof(parts), "mtdparts=%s:%dk(SPL),-(UBI)",
> -		         linux_name, 4 * mtd->erasesize >> 10);
> -		*mtdids = ids;
> -		*mtdparts = parts;
> -	}
> -}
> diff --git a/cmd/mtdparts.c b/cmd/mtdparts.c
> index f7ed1a077974..6b5644523898 100644
> --- a/cmd/mtdparts.c
> +++ b/cmd/mtdparts.c
> @@ -122,9 +122,6 @@ DECLARE_GLOBAL_DATA_PTR;
>  #define MTDPARTS_DEFAULT NULL
>  #endif
>  #endif
> -#if defined(CONFIG_SYS_MTDPARTS_RUNTIME)
> -extern void board_mtdparts_default(const char **mtdids, const char **mtdparts);
> -#endif
>  static const char *mtdids_default = MTDIDS_DEFAULT;
>  static const char *mtdparts_default = MTDPARTS_DEFAULT;
>  
> @@ -1733,9 +1730,6 @@ int mtdparts_init(void)
>  		memset(last_ids, 0, sizeof(last_ids));
>  		memset(last_parts, 0, sizeof(last_parts));
>  		memset(last_partition, 0, sizeof(last_partition));
> -#if defined(CONFIG_SYS_MTDPARTS_RUNTIME)
> -		board_mtdparts_default(&mtdids_default, &mtdparts_default);
> -#endif
>  		use_defaults = 1;
>  		initialized = 1;
>  	}
> diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
> index d638f700d041..ed619abac390 100644
> --- a/drivers/mtd/mtd_uboot.c
> +++ b/drivers/mtd/mtd_uboot.c
> @@ -13,8 +13,6 @@
>  
>  #define MTD_NAME_MAX_LEN 20
>  
> -void board_mtdparts_default(const char **mtdids, const char **mtdparts);
> -
>  static const char *get_mtdids(void)
>  {
>  	__maybe_unused const char *mtdparts = NULL;
> @@ -23,9 +21,7 @@ static const char *get_mtdids(void)
>  	if (mtdids)
>  		return mtdids;
>  
> -#if defined(CONFIG_SYS_MTDPARTS_RUNTIME)
> -	board_mtdparts_default(&mtdids, &mtdparts);
> -#elif defined(MTDIDS_DEFAULT)
> +#if defined(MTDIDS_DEFAULT)
>  	mtdids = MTDIDS_DEFAULT;
>  #elif defined(CONFIG_MTDIDS_DEFAULT)
>  	mtdids = CONFIG_MTDIDS_DEFAULT;
> @@ -133,9 +129,7 @@ static const char *get_mtdparts(void)
>  	if (mtdparts || !use_defaults)
>  		return mtdparts;
>  
> -#if defined(CONFIG_SYS_MTDPARTS_RUNTIME)
> -	board_mtdparts_default(&mtdids, &mtdparts);
> -#elif defined(MTDPARTS_DEFAULT)
> +#if defined(MTDPARTS_DEFAULT)
>  	mtdparts = MTDPARTS_DEFAULT;
>  #elif defined(CONFIG_MTDPARTS_DEFAULT)
>  	mtdparts = CONFIG_MTDPARTS_DEFAULT;
> diff --git a/include/configs/omap3_igep00x0.h b/include/configs/omap3_igep00x0.h
> index b9d65697521b..280a094cdbae 100644
> --- a/include/configs/omap3_igep00x0.h
> +++ b/include/configs/omap3_igep00x0.h
> @@ -87,8 +87,6 @@
>  
>  #endif
>  
> -#define CONFIG_SYS_MTDPARTS_RUNTIME
> -
>  /* OneNAND config */
>  #define CONFIG_USE_ONENAND_BOARD_INIT
>  #define CONFIG_SYS_ONENAND_BASE		ONENAND_MAP
> diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
> index b8addeaf693a..72608071c486 100644
> --- a/scripts/config_whitelist.txt
> +++ b/scripts/config_whitelist.txt
> @@ -3511,7 +3511,6 @@ CONFIG_SYS_MRAM_SIZE
>  CONFIG_SYS_MSC0_VAL
>  CONFIG_SYS_MSC1_VAL
>  CONFIG_SYS_MSC2_VAL
> -CONFIG_SYS_MTDPARTS_RUNTIME
>  CONFIG_SYS_MX5_CLK32
>  CONFIG_SYS_MX5_HCLK
>  CONFIG_SYS_MX6_CLK32
> -- 
> 2.17.1
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 2/2] mtd: Get rid of board_mtdparts_default()

[Boris Brezillon]

Hi Ladislav,

On Tue, 11 Dec 2018 23:55:26 +0100
Ladislav Michl <ladis@linux-mips.org> wrote:

> Hi Boris,
> 
> On Mon, Dec 10, 2018 at 04:38:50PM +0100, Boris Brezillon wrote:
> > The only implementer of this function has been patched to use
> > CONFIG_MTD{IDS,PARTS}_DEFAULT instead. Let's get rid of this function
> > and the associated CONFIG_SYS_MTDPARTS_RUNTIME option.  
> 
> the only implementer of this fuction did so for a good reason. What is
> a motivation to remove it?

Simplifying the code (see this discussion [1] which led me to send
this patchset).

> 
> The requirement is to be able to use single u-boot binary on all igep2
> boards ever produced. These comes with various NAND and OneNAND chips
> and  I was not able to come with single static partition configuration
> to support them all.

That's actually the question I asked Enric in [1]. Can you list all
the memory organization you have (for NAND and OneNAND chips)? I mean,
the SPL part size depends on the NAND/OneNAND erase block size, and
board vendors try to use similar flashes when they source different
parts (same page size, same block size, ...). Assuming this is the
case, you should always have the same layout for OneNAND/NAND devices,
hence my proposal to define those parts statically.

> Hence runtime detection. That code could be used
> on all OMAP3 boards as BootROM reads up to first four sectors searching
> for SPL (MLO).

Note that, for the nand side of things, you can also automate that using
a u-boot script:

nand info; setexpr splsize ${nand_erasesize} * 4; setenv mtdparts mtdparts=omap2-nand:0x${splsize}(SPL),-(UBI)

Shouldn't be hard to patch the onenand cmd to also expose writesize,
erasesize and oobsize.

Regards,

Boris

[1]https://www.mail-archive.com/u-boot at lists.denx.de/msg304933.html

[U-Boot] [PATCH 2/2] mtd: Get rid of board_mtdparts_default()

[Ladislav Michl]

Hello Boris,

On Wed, Dec 12, 2018 at 10:32:51AM +0100, Boris Brezillon wrote:
> Hi Ladislav,
> 
> On Tue, 11 Dec 2018 23:55:26 +0100
> Ladislav Michl <ladis@linux-mips.org> wrote:
> 
> > Hi Boris,
> > 
> > On Mon, Dec 10, 2018 at 04:38:50PM +0100, Boris Brezillon wrote:
> > > The only implementer of this function has been patched to use
> > > CONFIG_MTD{IDS,PARTS}_DEFAULT instead. Let's get rid of this function
> > > and the associated CONFIG_SYS_MTDPARTS_RUNTIME option.  
> > 
> > the only implementer of this fuction did so for a good reason. What is
> > a motivation to remove it?
> 
> Simplifying the code (see this discussion [1] which led me to send
> this patchset).

Thank you, makes sense.

> > The requirement is to be able to use single u-boot binary on all igep2
> > boards ever produced. These comes with various NAND and OneNAND chips
> > and  I was not able to come with single static partition configuration
> > to support them all.
> 
> That's actually the question I asked Enric in [1]. Can you list all
> the memory organization you have (for NAND and OneNAND chips)? I mean,
> the SPL part size depends on the NAND/OneNAND erase block size, and
> board vendors try to use similar flashes when they source different
> parts (same page size, same block size, ...). Assuming this is the
> case, you should always have the same layout for OneNAND/NAND devices,
> hence my proposal to define those parts statically.

First, thanks to Enric for pinging me, otherwise I would probably miss this
completely.

Now problem is that IGEPv2 comes with quite many configurations, some of
them are even customized, so static configuration is a show stopper
mainly as I do not know what devices are in field.
Another issue is how ubispl code works: It expects struct ubispl_info
filled with (among others) peb_offset of ubi partition. ubispl code counts
in terms of eraseblocks regardless of their size. So we would need to touch
this number when using static mtdparts.

> > Hence runtime detection. That code could be used
> > on all OMAP3 boards as BootROM reads up to first four sectors searching
> > for SPL (MLO).
> 
> Note that, for the nand side of things, you can also automate that using
> a u-boot script:
> 
> nand info; setexpr splsize ${nand_erasesize} * 4; setenv mtdparts mtdparts=omap2-nand:0x${splsize}(SPL),-(UBI)

That seems as a way to go!

> Shouldn't be hard to patch the onenand cmd to also expose writesize,
> erasesize and oobsize.

Side note: I never fully understand why is OneNAND using separate set of
commands.

Could you hold merging your paches until I implement above idea and test
it on a few boards? I know u-boot is now using two months merge window,
which is unfortunate, so I'll try to do it as soon as possible, but I do
not think I'll finish it till end of week.

> Regards,
> 
> Boris
> 
> [1]https://www.mail-archive.com/u-boot at lists.denx.de/msg304933.html

[U-Boot] [PATCH 2/2] mtd: Get rid of board_mtdparts_default()

[Boris Brezillon]

On Wed, 12 Dec 2018 12:37:04 +0100
Ladislav Michl <ladis@linux-mips.org> wrote:
 
> Now problem is that IGEPv2 comes with quite many configurations, some of
> them are even customized, so static configuration is a show stopper
> mainly as I do not know what devices are in field.
> Another issue is how ubispl code works: It expects struct ubispl_info
> filled with (among others) peb_offset of ubi partition. ubispl code counts
> in terms of eraseblocks regardless of their size. So we would need to touch
> this number when using static mtdparts.

Okay.

> 
> > > Hence runtime detection. That code could be used
> > > on all OMAP3 boards as BootROM reads up to first four sectors searching
> > > for SPL (MLO).  
> > 
> > Note that, for the nand side of things, you can also automate that using
> > a u-boot script:
> > 
> > nand info; setexpr splsize ${nand_erasesize} * 4; setenv mtdparts mtdparts=omap2-nand:0x${splsize}(SPL),-(UBI)  
> 
> That seems as a way to go!

Glad you like this idea.

> 
> > Shouldn't be hard to patch the onenand cmd to also expose writesize,
> > erasesize and oobsize.  
> 
> Side note: I never fully understand why is OneNAND using separate set of
> commands.

Hehe. That's exactly what Miquel tries to address with the mtd command
(one command to rule them all).

> 
> Could you hold merging your paches until I implement above idea and test
> it on a few boards? I know u-boot is now using two months merge window,
> which is unfortunate, so I'll try to do it as soon as possible, but I do
> not think I'll finish it till end of week.

No worries. This is not urgent and can definitely wait 2019.04.

Thanks,

Boris

[U-Boot] [PATCH 2/2] mtd: Get rid of board_mtdparts_default()

[Enric Balletbo Serra]

Hi Boris,

Missatge de Boris Brezillon <boris.brezillon@bootlin.com> del dia dc.,
12 de des. 2018 a les 18:36:
>
> On Wed, 12 Dec 2018 12:37:04 +0100
> Ladislav Michl <ladis@linux-mips.org> wrote:
>
> > Now problem is that IGEPv2 comes with quite many configurations, some of
> > them are even customized, so static configuration is a show stopper
> > mainly as I do not know what devices are in field.
> > Another issue is how ubispl code works: It expects struct ubispl_info
> > filled with (among others) peb_offset of ubi partition. ubispl code counts
> > in terms of eraseblocks regardless of their size. So we would need to touch
> > this number when using static mtdparts.
>
> Okay.
>
> >
> > > > Hence runtime detection. That code could be used
> > > > on all OMAP3 boards as BootROM reads up to first four sectors searching
> > > > for SPL (MLO).
> > >
> > > Note that, for the nand side of things, you can also automate that using
> > > a u-boot script:
> > >
> > > nand info; setexpr splsize ${nand_erasesize} * 4; setenv mtdparts mtdparts=omap2-nand:0x${splsize}(SPL),-(UBI)
> >
> > That seems as a way to go!
>
> Glad you like this idea.
>
> >
> > > Shouldn't be hard to patch the onenand cmd to also expose writesize,
> > > erasesize and oobsize.
> >
> > Side note: I never fully understand why is OneNAND using separate set of
> > commands.
>
> Hehe. That's exactly what Miquel tries to address with the mtd command
> (one command to rule them all).
>
> >
> > Could you hold merging your paches until I implement above idea and test
> > it on a few boards? I know u-boot is now using two months merge window,
> > which is unfortunate, so I'll try to do it as soon as possible, but I do
> > not think I'll finish it till end of week.
>
> No worries. This is not urgent and can definitely wait 2019.04.
>

Yes, will be good if we can have the Ladis work merged at the same
time. He will probably have more variety of boards than me to test
this, but from my side, and after do some tests the patch LGTM

Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Thanks,
 Enric

> Thanks,
>
> Boris

[U-Boot] [PATCH 1/2] configs: igep: Define default mtdids/mtdparts

[Enric Balletbo Serra]

+Ladis who might be also interested.
Missatge de Boris Brezillon <boris.brezillon@bootlin.com> del dia dl.,
10 de des. 2018 a les 16:38:
>
> We are trying to get rid of the weak board_mtdparts_default() function
> and we need to make sure igep defconfigs have proper proper
> CONFIG_MTD{IDS,PARTS}_DEFAULT before doing that.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
>  configs/igep0032_defconfig | 2 ++
>  configs/igep00x0_defconfig | 2 ++
>  2 files changed, 4 insertions(+)
>
> diff --git a/configs/igep0032_defconfig b/configs/igep0032_defconfig
> index 383648789c53..d2a614c98f6d 100644
> --- a/configs/igep0032_defconfig
> +++ b/configs/igep0032_defconfig
> @@ -28,6 +28,8 @@ CONFIG_CMD_SPI=y
>  CONFIG_CMD_CACHE=y
>  CONFIG_CMD_EXT4_WRITE=y
>  CONFIG_CMD_MTDPARTS=y
> +CONFIG_MTDIDS_DEFAULT="nand0=omap2-nand,onenand0=omap2-onenand"
> +CONFIG_MTDPARTS_DEFAULT="omap2-nand:512k(SPL),-(UBI);omap2-onenand:512k(SPL),-(UBI)"
>  CONFIG_CMD_UBI=y
>  # CONFIG_CMD_UBIFS is not set
>  CONFIG_NET_RANDOM_ETHADDR=y
> diff --git a/configs/igep00x0_defconfig b/configs/igep00x0_defconfig
> index f2989e34e12e..5d3e109ee3c2 100644
> --- a/configs/igep00x0_defconfig
> +++ b/configs/igep00x0_defconfig
> @@ -28,6 +28,8 @@ CONFIG_CMD_SPI=y
>  CONFIG_CMD_CACHE=y
>  CONFIG_CMD_EXT4_WRITE=y
>  CONFIG_CMD_MTDPARTS=y
> +CONFIG_MTDIDS_DEFAULT="nand0=omap2-nand,onenand0=omap2-onenand"
> +CONFIG_MTDPARTS_DEFAULT="omap2-nand:512k(SPL),-(UBI);omap2-onenand:512k(SPL),-(UBI)"
>  CONFIG_CMD_UBI=y
>  # CONFIG_CMD_UBIFS is not set
>  CONFIG_NET_RANDOM_ETHADDR=y
> --
> 2.17.1
>

[U-Boot] [PATCH 1/2] configs: igep: Define default mtdids/mtdparts

[Enric Balletbo Serra]

Hi Boris,

Missatge de Enric Balletbo Serra <eballetbo@gmail.com> del dia dl., 10
de des. 2018 a les 22:50:
>
> +Ladis who might be also interested.
> Missatge de Boris Brezillon <boris.brezillon@bootlin.com> del dia dl.,
> 10 de des. 2018 a les 16:38:
> >
> > We are trying to get rid of the weak board_mtdparts_default() function
> > and we need to make sure igep defconfigs have proper proper
> > CONFIG_MTD{IDS,PARTS}_DEFAULT before doing that.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> > ---
> >  configs/igep0032_defconfig | 2 ++
> >  configs/igep00x0_defconfig | 2 ++
> >  2 files changed, 4 insertions(+)
> >
> > diff --git a/configs/igep0032_defconfig b/configs/igep0032_defconfig
> > index 383648789c53..d2a614c98f6d 100644
> > --- a/configs/igep0032_defconfig
> > +++ b/configs/igep0032_defconfig
> > @@ -28,6 +28,8 @@ CONFIG_CMD_SPI=y
> >  CONFIG_CMD_CACHE=y
> >  CONFIG_CMD_EXT4_WRITE=y
> >  CONFIG_CMD_MTDPARTS=y
> > +CONFIG_MTDIDS_DEFAULT="nand0=omap2-nand,onenand0=omap2-onenand"
> > +CONFIG_MTDPARTS_DEFAULT="omap2-nand:512k(SPL),-(UBI);omap2-onenand:512k(SPL),-(UBI)"
> >  CONFIG_CMD_UBI=y
> >  # CONFIG_CMD_UBIFS is not set
> >  CONFIG_NET_RANDOM_ETHADDR=y
> > diff --git a/configs/igep00x0_defconfig b/configs/igep00x0_defconfig
> > index f2989e34e12e..5d3e109ee3c2 100644
> > --- a/configs/igep00x0_defconfig
> > +++ b/configs/igep00x0_defconfig
> > @@ -28,6 +28,8 @@ CONFIG_CMD_SPI=y
> >  CONFIG_CMD_CACHE=y
> >  CONFIG_CMD_EXT4_WRITE=y
> >  CONFIG_CMD_MTDPARTS=y
> > +CONFIG_MTDIDS_DEFAULT="nand0=omap2-nand,onenand0=omap2-onenand"
> > +CONFIG_MTDPARTS_DEFAULT="omap2-nand:512k(SPL),-(UBI);omap2-onenand:512k(SPL),-(UBI)"
> >  CONFIG_CMD_UBI=y
> >  # CONFIG_CMD_UBIFS is not set
> >  CONFIG_NET_RANDOM_ETHADDR=y
> > --
> > 2.17.1
> >

After do some tests, the patch lgtm

Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>