[U-Boot] [PATCH 0/6] miscellaneous ubispl and ubi improvements

[U-Boot] [PATCH 0/6] miscellaneous ubispl and ubi improvements

[Markus Klotzbuecher]

From: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>

Hello Heiko and Kyungmin

This series contains a couple of UBI and UBI SPL improvements, notably
a ubispl extension to allow loading volumes by name. The motivation is
to use the UBI atomic volume rename functionality to allow double copy
software updates of U-Boot on UBI. To do that we configured the SPL to
always load the same volume name (e.g. "u-boot"), and the software
updater always installs into the secondary volume "u-boot_r". After a
successful upgrade, these two volume names are switched.

This extension is protected by #ifdefs as it will somewhat slow down
loading of volumes by id. This is because the code needs to disable
the optimization of ignoring all volume ids which are not
to-be-loaded, since these can only be resolved after attaching.

We have tested both with and without fastmap enabled and both paths
seems to work reliably.

I also moved the ubispl config to Kconfig. There seem to be two boards
that use this (Maintainers cc'ed on patch). I have not updated their
defconfigs (not being sure if I should) but would be pleased to do so.

Hamish Guthrie (2):
  env: ubi: support configurable VID offset
  ubispl: add support for loading volumes by name

Markus Klotzbuecher (4):
  env: ubi: KConfig: add CONFIG_ENV_UBI_VOLUME_REDUND
  ubi: fix UBI_SILENCE_MSG
  ubispl: migrate configuration to Kconfig
  ubispl: introduce separate CONFIG_UBI_SPL_SILENCE_MSG

 common/spl/Kconfig           |  98 ++++++++++++++++
 common/spl/spl_ubi.c         |   7 ++
 drivers/mtd/ubi/Kconfig      |   2 +-
 drivers/mtd/ubi/io.c         |   2 +
 drivers/mtd/ubispl/ubispl.c  | 215 ++++++++++++++++++++++++++++++++++-
 drivers/mtd/ubispl/ubispl.h  |   9 +-
 env/Kconfig                  |  12 ++
 env/ubi.c                    |  17 ++-
 include/ubispl.h             |   6 +
 scripts/config_whitelist.txt |  13 ---
 10 files changed, 360 insertions(+), 21 deletions(-)

-- 
2.20.1

[U-Boot] [PATCH 1/6] env: ubi: KConfig: add CONFIG_ENV_UBI_VOLUME_REDUND

[Markus Klotzbuecher]

From: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>

Signed-off-by: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
Cc: Heiko Schocher <hs@denx.de>
Cc: Kyungmin Park <kmpark@infradead.org>
---
 env/Kconfig                  | 6 ++++++
 scripts/config_whitelist.txt | 1 -
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/env/Kconfig b/env/Kconfig
index 78300660c7..44c47220c2 100644
--- a/env/Kconfig
+++ b/env/Kconfig
@@ -513,6 +513,12 @@ config ENV_UBI_VOLUME
 	help
 	  Name of the volume that you want to store the environment in.
 
+config ENV_UBI_VOLUME_REDUND
+	string "UBI redundant volume name"
+	depends on ENV_IS_IN_UBI
+	help
+	  Name of the redundant volume that you want to store the environment in.
+
 endif
 
 config USE_DEFAULT_ENV_FILE
diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
index fa98efc24c..5d76c781d3 100644
--- a/scripts/config_whitelist.txt
+++ b/scripts/config_whitelist.txt
@@ -504,7 +504,6 @@ CONFIG_ENV_SROM_BANK
 CONFIG_ENV_TOTAL_SIZE
 CONFIG_ENV_UBIFS_OPTION
 CONFIG_ENV_UBI_MTD
-CONFIG_ENV_UBI_VOLUME_REDUND
 CONFIG_ENV_VERSION
 CONFIG_EP9302
 CONFIG_EP9307
-- 
2.20.1

[U-Boot] [PATCH 2/6] env: ubi: support configurable VID offset

[Markus Klotzbuecher]

From: Hamish Guthrie <hamish.guthrie@kistler.com>

Signed-off-by: Hamish Guthrie <hamish.guthrie@kistler.com>
Signed-off-by: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
Cc: Heiko Schocher <hs@denx.de>
Cc: Kyungmin Park <kmpark@infradead.org>
---
 env/Kconfig |  6 ++++++
 env/ubi.c   | 17 +++++++++++++----
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/env/Kconfig b/env/Kconfig
index 44c47220c2..595b658c26 100644
--- a/env/Kconfig
+++ b/env/Kconfig
@@ -519,6 +519,12 @@ config ENV_UBI_VOLUME_REDUND
 	help
 	  Name of the redundant volume that you want to store the environment in.
 
+config ENV_UBI_VID_OFFSET
+	int "ubi environment VID offset"
+	depends on ENV_IS_IN_UBI
+	help
+	  UBI VID offset for environment.
+
 endif
 
 config USE_DEFAULT_ENV_FILE
diff --git a/env/ubi.c b/env/ubi.c
index 1dfdf0a8c8..67379f2557 100644
--- a/env/ubi.c
+++ b/env/ubi.c
@@ -15,6 +15,15 @@
 #include <ubi_uboot.h>
 #undef crc32
 
+#define _QUOTE(x) #x
+#define QUOTE(x) _QUOTE(x)
+
+#ifdef CONFIG_ENV_UBI_VID_OFFSET
+ #define UBI_VID_OFFSET QUOTE(CONFIG_ENV_UBI_VID_OFFSET)
+#else
+ #define UBI_VID_OFFSET NULL
+#endif
+
 DECLARE_GLOBAL_DATA_PTR;
 
 #ifdef CONFIG_CMD_SAVEENV
@@ -28,7 +37,7 @@ static int env_ubi_save(void)
 	if (ret)
 		return ret;
 
-	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
+	if (ubi_part(CONFIG_ENV_UBI_PART, UBI_VID_OFFSET)) {
 		printf("\n** Cannot find mtd partition \"%s\"\n",
 		       CONFIG_ENV_UBI_PART);
 		return 1;
@@ -70,7 +79,7 @@ static int env_ubi_save(void)
 	if (ret)
 		return ret;
 
-	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
+	if (ubi_part(CONFIG_ENV_UBI_PART, UBI_VID_OFFSET)) {
 		printf("\n** Cannot find mtd partition \"%s\"\n",
 		       CONFIG_ENV_UBI_PART);
 		return 1;
@@ -111,7 +120,7 @@ static int env_ubi_load(void)
 	tmp_env1 = (env_t *)env1_buf;
 	tmp_env2 = (env_t *)env2_buf;
 
-	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
+	if (ubi_part(CONFIG_ENV_UBI_PART, UBI_VID_OFFSET)) {
 		printf("\n** Cannot find mtd partition \"%s\"\n",
 		       CONFIG_ENV_UBI_PART);
 		set_default_env(NULL, 0);
@@ -148,7 +157,7 @@ static int env_ubi_load(void)
 	 */
 	memset(buf, 0x0, CONFIG_ENV_SIZE);
 
-	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
+	if (ubi_part(CONFIG_ENV_UBI_PART, UBI_VID_OFFSET)) {
 		printf("\n** Cannot find mtd partition \"%s\"\n",
 		       CONFIG_ENV_UBI_PART);
 		set_default_env(NULL, 0);
-- 
2.20.1

[U-Boot] [PATCH 3/6] ubi: fix UBI_SILENCE_MSG

[Markus Klotzbuecher]

From: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>

 - drop CONFIG_ prefix from kconfig entry
 - fix small compilation issue with CONFIG_UBI_SILENCE_MSG

Signed-off-by: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
Cc: Heiko Schocher <hs@denx.de>
Cc: Kyungmin Park <kmpark@infradead.org>
---
 drivers/mtd/ubi/Kconfig | 2 +-
 drivers/mtd/ubi/io.c    | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig
index cf84783356..e22dda8564 100644
--- a/drivers/mtd/ubi/Kconfig
+++ b/drivers/mtd/ubi/Kconfig
@@ -1,6 +1,6 @@
 menu "UBI support"
 
-config CONFIG_UBI_SILENCE_MSG
+config UBI_SILENCE_MSG
 	bool "UBI silence verbose messages"
 	default ENV_IS_IN_UBI
 	help
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index 8ef7823b37..688fb509d2 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -1351,6 +1351,7 @@ static int self_check_write(struct ubi_device *ubi, const void *buf, int pnum,
 
 		ubi_err(ubi, "self-check failed for PEB %d:%d, len %d",
 			pnum, offset, len);
+#if !defined(CONFIG_UBI_SILENCE_MSG)
 		ubi_msg(ubi, "data differ at position %d", i);
 		ubi_msg(ubi, "hex dump of the original buffer from %d to %d",
 			i, i + dump_len);
@@ -1360,6 +1361,7 @@ static int self_check_write(struct ubi_device *ubi, const void *buf, int pnum,
 			i, i + dump_len);
 		print_hex_dump("", DUMP_PREFIX_OFFSET, 32, 1,
 			       buf1 + i, dump_len, 1);
+#endif
 		dump_stack();
 		err = -EINVAL;
 		goto out_free;
-- 
2.20.1

[U-Boot] [PATCH 4/6] ubispl: migrate configuration to Kconfig

[Markus Klotzbuecher]

From: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>

Signed-off-by: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
Cc: Heiko Schocher <hs@denx.de>
Cc: Kyungmin Park <kmpark@infradead.org>
Cc: Javier Martinez Canillas <javier@dowhile0.org>
Cc: Enric Balletbo i Serra <eballetbo@gmail.com>
---
 common/spl/Kconfig           | 79 ++++++++++++++++++++++++++++++++++++
 scripts/config_whitelist.txt | 12 ------
 2 files changed, 79 insertions(+), 12 deletions(-)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 206c24076d..71bedea638 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -549,6 +549,85 @@ config SPL_NAND_SUPPORT
 	  This enables the drivers in drivers/mtd/nand/raw as part of an SPL
 	  build.
 
+config SPL_UBI
+	bool "Support UBI"
+	help
+	  Enable support for loading payloads from UBI. See
+	  README.ubispl for more info.
+
+if SPL_UBI
+config SPL_UBI_MAX_VOL_LEBS
+	int "Maximum number of LEBs per volume"
+	depends on SPL_UBI
+	help
+	  The maximum number of logical eraseblocks which a static volume
+	  to load can contain. Used for sizing the scan data structure.
+
+config SPL_UBI_MAX_PEB_SIZE
+	int "Maximum PEB size"
+	depends on SPL_UBI
+	help
+	  The maximum physical erase block size.
+
+config SPL_UBI_MAX_PEBS
+	int "Maximum number of PEBs"
+	depends on SPL_UBI
+	help
+	  The maximum physical erase block size. If not overridden by
+	  board code, this value will be used as the actual number of PEBs.
+
+config SPL_UBI_PEB_OFFSET
+	int "Offset to first UBI PEB"
+	depends on SPL_UBI
+	help
+	  The offset in number of PEBs from the start of flash to the first
+	  PEB part of the UBI image.
+
+config SPL_UBI_VID_OFFSET
+	int "Offset to VID header"
+	depends on SPL_UBI
+
+config SPL_UBI_LEB_START
+	int "Offset to LEB in PEB"
+	depends on SPL_UBI
+	help
+	  The offset in bytes to the LEB within a PEB.
+
+config SPL_UBI_INFO_ADDR
+	hex "Address to place UBI scan info"
+	depends on SPL_UBI
+	help
+	  Address for ubispl to place the scan info. Read README.ubispl to
+	  determine the required size
+
+config SPL_UBI_VOL_IDS
+	int "Maximum volume id"
+	depends on SPL_UBI
+	help
+	  The maximum volume id which can be loaded. Used for sizing the
+	  scan data structure.
+
+config SPL_UBI_LOAD_MONITOR_ID
+	int "id of U-Boot volume"
+	depends on SPL_UBI
+	help
+	  The UBI volume id from which to load U-Boot
+
+config SPL_UBI_LOAD_KERNEL_ID
+	int "id of kernel volume"
+	depends on SPL_OS_BOOT && SPL_UBI
+	help
+	  The UBI volume id from which to load the kernel
+
+config SPL_UBI_LOAD_ARGS_ID
+	int "id of kernel args volume"
+	depends on SPL_OS_BOOT && SPL_UBI
+	help
+	  The UBI volume id from which to load the device tree
+
+
+endif   # if SPL_UBI
+
 config SPL_NET_SUPPORT
 	bool "Support networking"
 	help
diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
index 5d76c781d3..d72e7c4ff4 100644
--- a/scripts/config_whitelist.txt
+++ b/scripts/config_whitelist.txt
@@ -1886,18 +1886,6 @@ CONFIG_SPL_STACK_SIZE
 CONFIG_SPL_START_S_PATH
 CONFIG_SPL_TARGET
 CONFIG_SPL_TEXT_BASE
-CONFIG_SPL_UBI
-CONFIG_SPL_UBI_INFO_ADDR
-CONFIG_SPL_UBI_LEB_START
-CONFIG_SPL_UBI_LOAD_ARGS_ID
-CONFIG_SPL_UBI_LOAD_KERNEL_ID
-CONFIG_SPL_UBI_LOAD_MONITOR_ID
-CONFIG_SPL_UBI_MAX_PEBS
-CONFIG_SPL_UBI_MAX_PEB_SIZE
-CONFIG_SPL_UBI_MAX_VOL_LEBS
-CONFIG_SPL_UBI_PEB_OFFSET
-CONFIG_SPL_UBI_VID_OFFSET
-CONFIG_SPL_UBI_VOL_IDS
 CONFIG_SPL_UBOOT_KEY_HASH
 CONFIG_SRAM_BASE
 CONFIG_SRAM_SIZE
-- 
2.20.1

[U-Boot] [PATCH 5/6] ubispl: add support for loading volumes by name

[Markus Klotzbuecher]

From: Hamish Guthrie <hamish.guthrie@kistler.com>

The motivation is to use the UBI atomic volume rename functionality to
allow double copy software updates on UBI. To that end the SPL is
configured to always load the same volume name (e.g. "u-boot"),
whereas a software updater always installs into the secondary volume
"u-boot_r". After successful installation, these two volume names are
switched.

This extension is protected by #ifdefs as it will somewhat slow down
loading of volumes by id. This is because the code needs to disable
the optimization of ignoring all volume ids which are not
to-be-loaded, since these can only be resolved after attaching.

Signed-off-by: Hamish Guthrie <hamish.guthrie@kistler.com>
Signed-off-by: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
Cc: Heiko Schocher <hs@denx.de>
Cc: Kyungmin Park <kmpark@infradead.org>
---
 common/spl/Kconfig          |  13 +++
 common/spl/spl_ubi.c        |   7 ++
 drivers/mtd/ubispl/ubispl.c | 215 +++++++++++++++++++++++++++++++++++-
 drivers/mtd/ubispl/ubispl.h |   7 ++
 include/ubispl.h            |   6 +
 5 files changed, 246 insertions(+), 2 deletions(-)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 71bedea638..0e91bd309b 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -556,6 +556,13 @@ config SPL_UBI
 	  README.ubispl for more info.
 
 if SPL_UBI
+config SPL_UBI_LOAD_BY_VOLNAME
+	bool "Support loading volumes by name"
+	help
+	  This enables support for loading UBI volumes by name. When this
+	  is set, CONFIG_SPL_UBI_LOAD_MONITOR_VOLNAME can be used to
+	  configure the volume name from which to load U-Boot.
+
 config SPL_UBI_MAX_VOL_LEBS
 	int "Maximum number of LEBs per volume"
 	depends on SPL_UBI
@@ -613,6 +620,12 @@ config SPL_UBI_LOAD_MONITOR_ID
 	help
 	  The UBI volume id from which to load U-Boot
 
+config SPL_UBI_LOAD_MONITOR_VOLNAME
+	string "volume name of U-Boot volume"
+	depends on SPL_UBI_LOAD_BY_VOLNAME
+	help
+	  The UBI volume name from which to load U-Boot
+
 config SPL_UBI_LOAD_KERNEL_ID
 	int "id of kernel volume"
 	depends on SPL_OS_BOOT && SPL_UBI
diff --git a/common/spl/spl_ubi.c b/common/spl/spl_ubi.c
index 67e5fadd7c..0cb5080882 100644
--- a/common/spl/spl_ubi.c
+++ b/common/spl/spl_ubi.c
@@ -62,7 +62,14 @@ int spl_ubi_load_image(struct spl_image_info *spl_image,
 	}
 #endif
 	header = spl_get_load_buffer(-sizeof(*header), sizeof(header));
+#ifdef CONFIG_SPL_UBI_LOAD_BY_VOLNAME
+	volumes[0].vol_id = -1;
+	strncpy(volumes[0].name,
+		CONFIG_SPL_UBI_LOAD_MONITOR_VOLNAME,
+		UBI_VOL_NAME_MAX + 1);
+#else
 	volumes[0].vol_id = CONFIG_SPL_UBI_LOAD_MONITOR_ID;
+#endif
 	volumes[0].load_addr = (void *)header;
 
 	ret = ubispl_load_volumes(&info, volumes, 1);
diff --git a/drivers/mtd/ubispl/ubispl.c b/drivers/mtd/ubispl/ubispl.c
index eeb1cbefb7..3f3b9b4367 100644
--- a/drivers/mtd/ubispl/ubispl.c
+++ b/drivers/mtd/ubispl/ubispl.c
@@ -45,6 +45,187 @@ static int ubi_io_is_bad(struct ubi_scan_info *ubi, int peb)
 	return peb >= ubi->peb_count || peb < 0;
 }
 
+#ifdef CONFIG_SPL_UBI_LOAD_BY_VOLNAME
+
+/**
+ * ubi_dump_vtbl_record - dump a &struct ubi_vtbl_record object.
+ * @r: the object to dump
+ * @idx: volume table index
+ */
+void ubi_dump_vtbl_record(const struct ubi_vtbl_record *r, int idx)
+{
+	int name_len = be16_to_cpu(r->name_len);
+
+	ubi_dbg("Volume table record %d dump: size: %d",
+		idx, sizeof(struct ubi_vtbl_record));
+	ubi_dbg("\treserved_pebs   %d", be32_to_cpu(r->reserved_pebs));
+	ubi_dbg("\talignment       %d", be32_to_cpu(r->alignment));
+	ubi_dbg("\tdata_pad        %d", be32_to_cpu(r->data_pad));
+	ubi_dbg("\tvol_type        %d", (int)r->vol_type);
+	ubi_dbg("\tupd_marker      %d", (int)r->upd_marker);
+	ubi_dbg("\tname_len        %d", name_len);
+
+	if (r->name[0] == '\0') {
+		ubi_dbg("\tname            NULL");
+		return;
+	}
+
+	if (name_len <= UBI_VOL_NAME_MAX &&
+	    strnlen(&r->name[0], name_len + 1) == name_len) {
+		ubi_dbg("\tname            %s", &r->name[0]);
+	} else {
+		ubi_dbg("\t1st 5 characters of name: %c%c%c%c%c",
+			r->name[0], r->name[1], r->name[2], r->name[3],
+			r->name[4]);
+	}
+	ubi_dbg("\tcrc             %#08x", be32_to_cpu(r->crc));
+}
+
+/* Empty volume table record */
+static struct ubi_vtbl_record empty_vtbl_record;
+
+/**
+ * vtbl_check - check if volume table is not corrupted and sensible.
+ * @ubi: UBI device description object
+ * @vtbl: volume table
+ *
+ * This function returns zero if @vtbl is all right, %1 if CRC is incorrect,
+ * and %-EINVAL if it contains inconsistent data.
+ */
+static int vtbl_check(struct ubi_scan_info *ubi,
+		      struct ubi_vtbl_record *vtbl)
+{
+	int i, n, reserved_pebs, alignment, data_pad, vol_type, name_len;
+	int upd_marker, err;
+	uint32_t crc;
+	const char *name;
+
+	for (i = 0; i < UBI_SPL_VOL_IDS; i++) {
+		reserved_pebs = be32_to_cpu(vtbl[i].reserved_pebs);
+		alignment = be32_to_cpu(vtbl[i].alignment);
+		data_pad = be32_to_cpu(vtbl[i].data_pad);
+		upd_marker = vtbl[i].upd_marker;
+		vol_type = vtbl[i].vol_type;
+		name_len = be16_to_cpu(vtbl[i].name_len);
+		name = &vtbl[i].name[0];
+
+		crc = crc32(UBI_CRC32_INIT, &vtbl[i], UBI_VTBL_RECORD_SIZE_CRC);
+		if (be32_to_cpu(vtbl[i].crc) != crc) {
+			ubi_err("bad CRC@record %u: %#08x, not %#08x",
+				i, crc, be32_to_cpu(vtbl[i].crc));
+			ubi_dump_vtbl_record(&vtbl[i], i);
+			return 1;
+		}
+
+		if (reserved_pebs == 0) {
+			if (memcmp(&vtbl[i], &empty_vtbl_record,
+				   UBI_VTBL_RECORD_SIZE)) {
+				err = 2;
+				goto bad;
+			}
+			continue;
+		}
+
+		if (reserved_pebs < 0 || alignment < 0 || data_pad < 0 ||
+		    name_len < 0) {
+			err = 3;
+			goto bad;
+		}
+
+		if (alignment > ubi->leb_size || alignment == 0) {
+			err = 4;
+			goto bad;
+		}
+
+		n = alignment & (CONFIG_SPL_UBI_VID_OFFSET - 1);
+		if (alignment != 1 && n) {
+			err = 5;
+			goto bad;
+		}
+
+		n = ubi->leb_size % alignment;
+		if (data_pad != n) {
+			ubi_err("bad data_pad, has to be %d", n);
+			err = 6;
+			goto bad;
+		}
+
+		if (vol_type != UBI_VID_DYNAMIC && vol_type != UBI_VID_STATIC) {
+			err = 7;
+			goto bad;
+		}
+
+		if (upd_marker != 0 && upd_marker != 1) {
+			err = 8;
+			goto bad;
+		}
+
+		if (name_len > UBI_VOL_NAME_MAX) {
+			err = 10;
+			goto bad;
+		}
+
+		if (name[0] == '\0') {
+			err = 11;
+			goto bad;
+		}
+
+		if (name_len != strnlen(name, name_len + 1)) {
+			err = 12;
+			goto bad;
+		}
+
+		ubi_dump_vtbl_record(&vtbl[i], i);
+	}
+
+	/* Checks that all names are unique */
+	for (i = 0; i < UBI_SPL_VOL_IDS - 1; i++) {
+		for (n = i + 1; n < UBI_SPL_VOL_IDS; n++) {
+			int len1 = be16_to_cpu(vtbl[i].name_len);
+			int len2 = be16_to_cpu(vtbl[n].name_len);
+
+			if (len1 > 0 && len1 == len2 &&
+			    !strncmp(vtbl[i].name, vtbl[n].name, len1)) {
+				ubi_err("volumes %d and %d have the same name \"%s\"",
+					i, n, vtbl[i].name);
+				ubi_dump_vtbl_record(&vtbl[i], i);
+				ubi_dump_vtbl_record(&vtbl[n], n);
+				return -EINVAL;
+			}
+		}
+	}
+
+	return 0;
+
+bad:
+	ubi_err("volume table check failed: record %d, error %d", i, err);
+	ubi_dump_vtbl_record(&vtbl[i], i);
+	return -EINVAL;
+}
+
+static int ubi_read_volume_table(struct ubi_scan_info *ubi, u32 pnum)
+{
+	int err = -EINVAL;
+
+	empty_vtbl_record.crc = cpu_to_be32(0xf116c36b);
+
+	err = ubi_io_read(ubi, &ubi->vtbl, pnum, ubi->leb_start,
+			  sizeof(struct ubi_vtbl_record) * UBI_SPL_VOL_IDS);
+	if (err && err != UBI_IO_BITFLIPS) {
+		ubi_err("unable to read volume table");
+		goto out;
+	}
+
+	if (!vtbl_check(ubi, ubi->vtbl)) {
+		ubi->vtbl_valid = 1;
+		err = 0;
+	}
+out:
+	return err;
+}
+
+#endif /* CONFIG_SPL_UBI_LOAD_BY_VOLNAME */
+
 static int ubi_io_read_vid_hdr(struct ubi_scan_info *ubi, int pnum,
 			       struct ubi_vid_hdr *vh, int unused)
 {
@@ -210,14 +391,23 @@ static int ubi_scan_vid_hdr(struct ubi_scan_info *ubi, struct ubi_vid_hdr *vh,
 	if (vol_id == UBI_FM_SB_VOLUME_ID)
 		return ubi->fm_enabled ? UBI_FASTMAP_ANCHOR : 0;
 
+#ifdef CONFIG_SPL_UBI_LOAD_BY_VOLNAME
+	/* If this is a UBI volume table, read it and return */
+	if (vol_id == UBI_LAYOUT_VOLUME_ID && !ubi->vtbl_valid) {
+		res = ubi_read_volume_table(ubi, pnum);
+		return res;
+	}
+#endif
+
 	/* We only care about static volumes with an id < UBI_SPL_VOL_IDS */
 	if (vol_id >= UBI_SPL_VOL_IDS || vh->vol_type != UBI_VID_STATIC)
 		return 0;
 
+#ifndef CONFIG_SPL_UBI_LOAD_BY_VOLNAME
 	/* We are only interested in the volumes to load */
 	if (!test_bit(vol_id, ubi->toload))
 		return 0;
-
+#endif
 	lnum = be32_to_cpu(vh->lnum);
 	return ubi_add_peb_to_vol(ubi, vh, vol_id, pnum, lnum);
 }
@@ -232,13 +422,14 @@ static int assign_aeb_to_av(struct ubi_scan_info *ubi, u32 pnum, u32 lnum,
 
 	ubi->fastmap_pebs++;
 
+#ifndef CONFIG_SPL_UBI_LOAD_BY_VOLNAME
 	if (vol_id >= UBI_SPL_VOL_IDS || vol_type != UBI_STATIC_VOLUME)
 		return 0;
 
 	/* We are only interested in the volumes to load */
 	if (!test_bit(vol_id, ubi->toload))
 		return 0;
-
+#endif
 	vh = ubi->blockinfo + pnum;
 
 	return ubi_scan_vid_hdr(ubi, vh, pnum);
@@ -892,6 +1083,10 @@ retry:
 	ubi->peb_count = info->peb_count;
 	ubi->peb_offset = info->peb_offset;
 
+#ifdef CONFIG_SPL_UBI_LOAD_BY_VOLNAME
+	ubi->vtbl_valid = 0;
+#endif
+
 	fsize = info->peb_size * info->peb_count;
 	ubi->fsize_mb = fsize >> 20;
 
@@ -910,7 +1105,23 @@ retry:
 	for (i = 0; i < nrvols; i++) {
 		struct ubispl_load *lv = lvols + i;
 
+#ifdef CONFIG_SPL_UBI_LOAD_BY_VOLNAME
+		if (lv->vol_id == -1) {
+			for (int j = 0; j < UBI_SPL_VOL_IDS; j++) {
+				int len = be16_to_cpu(ubi->vtbl[j].name_len);
+
+				if (strncmp(lv->name,
+					    ubi->vtbl[j].name,
+					    len) == 0) {
+					lv->vol_id = j;
+					break;
+				}
+			}
+		}
+		ubi_msg("Loading VolName %s (VolId #%d)", lv->name, lv->vol_id);
+#else
 		ubi_msg("Loading VolId #%d", lv->vol_id);
+#endif
 		res = ipl_load(ubi, lv->vol_id, lv->load_addr);
 		if (res < 0) {
 			if (fastmap) {
diff --git a/drivers/mtd/ubispl/ubispl.h b/drivers/mtd/ubispl/ubispl.h
index 9e40b46eac..bcc376c6d7 100644
--- a/drivers/mtd/ubispl/ubispl.h
+++ b/drivers/mtd/ubispl/ubispl.h
@@ -77,6 +77,8 @@ struct ubi_vol_info {
  * @blockinfo:		The vid headers of the scanned blocks
  * @volinfo:		The volume information of the interesting (toload)
  *			volumes
+ * @vtbl_corrupted:	Flag to indicate status of volume table
+ * @vtbl:		Volume table
  *
  * @fm_buf:		The large fastmap attach buffer
  */
@@ -112,6 +114,11 @@ struct ubi_scan_info {
 	struct ubi_vol_info		volinfo[UBI_SPL_VOL_IDS];
 	struct ubi_vid_hdr		blockinfo[CONFIG_SPL_UBI_MAX_PEBS];
 
+#ifdef CONFIG_SPL_UBI_LOAD_BY_VOLNAME
+	/* Volume table */
+	int                             vtbl_valid;
+	struct ubi_vtbl_record          vtbl[UBI_SPL_VOL_IDS];
+#endif
 	/* The large buffer for the fastmap */
 	uint8_t				fm_buf[UBI_FM_BUF_SIZE];
 };
diff --git a/include/ubispl.h b/include/ubispl.h
index 1e5da94eb1..ecfe0c93c1 100644
--- a/include/ubispl.h
+++ b/include/ubispl.h
@@ -5,6 +5,8 @@
 #ifndef __UBOOT_UBISPL_H
 #define __UBOOT_UBISPL_H
 
+#define UBI_VOL_NAME_MAX	127
+
 /*
  * The following CONFIG options are relevant for UBISPL
  *
@@ -74,6 +76,10 @@ struct ubispl_info {
  */
 struct ubispl_load {
 	int		vol_id;
+#ifdef CONFIG_SPL_UBI_LOAD_BY_VOLNAME
+	u32		name_len;
+	char		name[UBI_VOL_NAME_MAX + 1];
+#endif
 	void		*load_addr;
 };
 
-- 
2.20.1

[U-Boot] [PATCH 6/6] ubispl: introduce separate CONFIG_UBI_SPL_SILENCE_MSG

[Markus Klotzbuecher]

From: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>

This allows to silence ubi and ubispl individually.

Signed-off-by: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
Cc: Heiko Schocher <hs@denx.de>
Cc: Kyungmin Park <kmpark@infradead.org>
---
 common/spl/Kconfig          | 6 ++++++
 drivers/mtd/ubispl/ubispl.h | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 0e91bd309b..37dce69e81 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -638,6 +638,12 @@ config SPL_UBI_LOAD_ARGS_ID
 	help
 	  The UBI volume id from which to load the device tree
 
+config UBI_SPL_SILENCE_MSG
+	bool "silence UBI SPL messages"
+	default n
+	help
+	  Disable messages from UBI SPL. This leaves warnings
+	  and errors enabled.
 
 endif   # if SPL_UBI
 
diff --git a/drivers/mtd/ubispl/ubispl.h b/drivers/mtd/ubispl/ubispl.h
index bcc376c6d7..b7cb7fc941 100644
--- a/drivers/mtd/ubispl/ubispl.h
+++ b/drivers/mtd/ubispl/ubispl.h
@@ -129,7 +129,7 @@ struct ubi_scan_info {
 #define ubi_dbg(fmt, ...)
 #endif
 
-#ifdef CONFIG_UBI_SILENCE_MSG
+#ifdef CONFIG_UBI_SPL_SILENCE_MSG
 #define ubi_msg(fmt, ...)
 #else
 #define ubi_msg(fmt, ...) printf("UBI: " fmt "\n", ##__VA_ARGS__)
-- 
2.20.1

[U-Boot] [PATCH 0/6] miscellaneous ubispl and ubi improvements

[Heiko Schocher]

Hello Markus,

Am 15.04.2019 um 17:32 schrieb Markus Klotzbuecher:
> From: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
> 
> Hello Heiko and Kyungmin
> 
> This series contains a couple of UBI and UBI SPL improvements, notably
> a ubispl extension to allow loading volumes by name. The motivation is
> to use the UBI atomic volume rename functionality to allow double copy
> software updates of U-Boot on UBI. To do that we configured the SPL to
> always load the same volume name (e.g. "u-boot"), and the software
> updater always installs into the secondary volume "u-boot_r". After a
> successful upgrade, these two volume names are switched.

Ok ... Hmm... if you use the bootcounter in SPL, you may have a chance
to unbreak he board, if "u-boot" does not boot?

How do you detect a successfull upgrade ?

> This extension is protected by #ifdefs as it will somewhat slow down
> loading of volumes by id. This is because the code needs to disable
> the optimization of ignoring all volume ids which are not
> to-be-loaded, since these can only be resolved after attaching.
> 
> We have tested both with and without fastmap enabled and both paths
> seems to work reliably.

Fine, thanks!

> I also moved the ubispl config to Kconfig. There seem to be two boards

Thanks!

> that use this (Maintainers cc'ed on patch). I have not updated their
> defconfigs (not being sure if I should) but would be pleased to do so.

I prefer that you do so, but we will see, if the board maintainers
give some feedback.

Give me some time to look through your patches.

bye,
Heiko

> Hamish Guthrie (2):
>    env: ubi: support configurable VID offset
>    ubispl: add support for loading volumes by name
> 
> Markus Klotzbuecher (4):
>    env: ubi: KConfig: add CONFIG_ENV_UBI_VOLUME_REDUND
>    ubi: fix UBI_SILENCE_MSG
>    ubispl: migrate configuration to Kconfig
>    ubispl: introduce separate CONFIG_UBI_SPL_SILENCE_MSG
> 
>   common/spl/Kconfig           |  98 ++++++++++++++++
>   common/spl/spl_ubi.c         |   7 ++
>   drivers/mtd/ubi/Kconfig      |   2 +-
>   drivers/mtd/ubi/io.c         |   2 +
>   drivers/mtd/ubispl/ubispl.c  | 215 ++++++++++++++++++++++++++++++++++-
>   drivers/mtd/ubispl/ubispl.h  |   9 +-
>   env/Kconfig                  |  12 ++
>   env/ubi.c                    |  17 ++-
>   include/ubispl.h             |   6 +
>   scripts/config_whitelist.txt |  13 ---
>   10 files changed, 360 insertions(+), 21 deletions(-)
> 

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[U-Boot] [PATCH 0/6] miscellaneous ubispl and ubi improvements

[Heiko Schocher]

Hello Markus,

may I am to dummy, but I cannot find your patches in patchwork :-(

any ideas?

bye,
Heiko

Am 16.04.2019 um 05:52 schrieb Heiko Schocher:
> Hello Markus,
> 
> Am 15.04.2019 um 17:32 schrieb Markus Klotzbuecher:
>> From: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
>>
>> Hello Heiko and Kyungmin
>>
>> This series contains a couple of UBI and UBI SPL improvements, notably
>> a ubispl extension to allow loading volumes by name. The motivation is
>> to use the UBI atomic volume rename functionality to allow double copy
>> software updates of U-Boot on UBI. To do that we configured the SPL to
>> always load the same volume name (e.g. "u-boot"), and the software
>> updater always installs into the secondary volume "u-boot_r". After a
>> successful upgrade, these two volume names are switched.
> 
> Ok ... Hmm... if you use the bootcounter in SPL, you may have a chance
> to unbreak he board, if "u-boot" does not boot?
> 
> How do you detect a successfull upgrade ?
> 
>> This extension is protected by #ifdefs as it will somewhat slow down
>> loading of volumes by id. This is because the code needs to disable
>> the optimization of ignoring all volume ids which are not
>> to-be-loaded, since these can only be resolved after attaching.
>>
>> We have tested both with and without fastmap enabled and both paths
>> seems to work reliably.
> 
> Fine, thanks!
> 
>> I also moved the ubispl config to Kconfig. There seem to be two boards
> 
> Thanks!
> 
>> that use this (Maintainers cc'ed on patch). I have not updated their
>> defconfigs (not being sure if I should) but would be pleased to do so.
> 
> I prefer that you do so, but we will see, if the board maintainers
> give some feedback.
> 
> Give me some time to look through your patches.
> 
> bye,
> Heiko
> 
>> Hamish Guthrie (2):
>>    env: ubi: support configurable VID offset
>>    ubispl: add support for loading volumes by name
>>
>> Markus Klotzbuecher (4):
>>    env: ubi: KConfig: add CONFIG_ENV_UBI_VOLUME_REDUND
>>    ubi: fix UBI_SILENCE_MSG
>>    ubispl: migrate configuration to Kconfig
>>    ubispl: introduce separate CONFIG_UBI_SPL_SILENCE_MSG
>>
>>   common/spl/Kconfig           |  98 ++++++++++++++++
>>   common/spl/spl_ubi.c         |   7 ++
>>   drivers/mtd/ubi/Kconfig      |   2 +-
>>   drivers/mtd/ubi/io.c         |   2 +
>>   drivers/mtd/ubispl/ubispl.c  | 215 ++++++++++++++++++++++++++++++++++-
>>   drivers/mtd/ubispl/ubispl.h  |   9 +-
>>   env/Kconfig                  |  12 ++
>>   env/ubi.c                    |  17 ++-
>>   include/ubispl.h             |   6 +
>>   scripts/config_whitelist.txt |  13 ---
>>   10 files changed, 360 insertions(+), 21 deletions(-)
>>
> 

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[U-Boot] [PATCH 0/6] miscellaneous ubispl and ubi improvements

[Markus Klotzbuecher]

Hello Heiko

On Tue, Apr 16, 2019 at 06:03:31AM +0200, Heiko Schocher wrote:

>may I am to dummy, but I cannot find your patches in patchwork :-(
>
>any ideas?

As this was a first post from this address, I think they are still in
the list moderation queue...

Best regards
Markus

-- 
Markus Klotzbuecher
Freelancer Embedded, Distributed & Real-time
Am See 28, 78465 Konstanz, Germany

[U-Boot] [PATCH 0/6] miscellaneous ubispl and ubi improvements

[Markus Klotzbuecher]

Hello Heiko

On Tue, Apr 16, 2019 at 05:52:15AM +0200, Heiko Schocher wrote:
>
>Am 15.04.2019 um 17:32 schrieb Markus Klotzbuecher:
>> From: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
>> 
>> Hello Heiko and Kyungmin
>> 
>> This series contains a couple of UBI and UBI SPL improvements, notably
>> a ubispl extension to allow loading volumes by name. The motivation is
>> to use the UBI atomic volume rename functionality to allow double copy
>> software updates of U-Boot on UBI. To do that we configured the SPL to
>> always load the same volume name (e.g. "u-boot"), and the software
>> updater always installs into the secondary volume "u-boot_r". After a
>> successful upgrade, these two volume names are switched.
>
>Ok ... Hmm... if you use the bootcounter in SPL, you may have a chance
>to unbreak he board, if "u-boot" does not boot?
>
>How do you detect a successfull upgrade ?

At this point, if writing to the to-be-updated volume succeeds, we
consider it a successful upgrade. If not, the rename is not carried
out and the old volume will remain the active one.

To further improve reliability, this could be combined with an SPL
bootcounter, however to really be an improvement, that should probably
use an environment on UBI too (or anything more reliable than raw
NAND).

>> This extension is protected by #ifdefs as it will somewhat slow down
>> loading of volumes by id. This is because the code needs to disable
>> the optimization of ignoring all volume ids which are not
>> to-be-loaded, since these can only be resolved after attaching.
>> 
>> We have tested both with and without fastmap enabled and both paths
>> seems to work reliably.
>
>Fine, thanks!
>
>> I also moved the ubispl config to Kconfig. There seem to be two boards
>
>Thanks!
>
>> that use this (Maintainers cc'ed on patch). I have not updated their
>> defconfigs (not being sure if I should) but would be pleased to do so.
>
>I prefer that you do so, but we will see, if the board maintainers
>give some feedback.
>
>Give me some time to look through your patches.

Sure, thank you!

Best regards
Markus

-- 
Markus Klotzbuecher
Freelancer Embedded, Distributed & Real-time
Am See 28, 78465 Konstanz, Germany

[U-Boot] [PATCH 1/6] env: ubi: KConfig: add CONFIG_ENV_UBI_VOLUME_REDUND

[Heiko Schocher]

Hello Markus,

Am 15.04.2019 um 17:32 schrieb Markus Klotzbuecher:
> From: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>

please add a commit message.

> Signed-off-by: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Kyungmin Park <kmpark@infradead.org>
> ---
>   env/Kconfig                  | 6 ++++++
>   scripts/config_whitelist.txt | 1 -
>   2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/env/Kconfig b/env/Kconfig
> index 78300660c7..44c47220c2 100644
> --- a/env/Kconfig
> +++ b/env/Kconfig
> @@ -513,6 +513,12 @@ config ENV_UBI_VOLUME
>   	help
>   	  Name of the volume that you want to store the environment in.
>   
> +config ENV_UBI_VOLUME_REDUND
> +	string "UBI redundant volume name"
> +	depends on ENV_IS_IN_UBI
> +	help
> +	  Name of the redundant volume that you want to store the environment in.
> +
>   endif
>   
>   config USE_DEFAULT_ENV_FILE
> diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
> index fa98efc24c..5d76c781d3 100644
> --- a/scripts/config_whitelist.txt
> +++ b/scripts/config_whitelist.txt
> @@ -504,7 +504,6 @@ CONFIG_ENV_SROM_BANK
>   CONFIG_ENV_TOTAL_SIZE
>   CONFIG_ENV_UBIFS_OPTION
>   CONFIG_ENV_UBI_MTD
> -CONFIG_ENV_UBI_VOLUME_REDUND
>   CONFIG_ENV_VERSION
>   CONFIG_EP9302
>   CONFIG_EP9307
> 

Please move from the config files:

./include/configs/omap3_igep00x0.h
./include/configs/gardena-smart-gateway-at91sam.h
./include/configs/am335x_igep003x.h

also the symbols to the defconfig files, thanks.

BTW: you can use the tool tools/moveconfig.py in u-boot source
for this purpose.

Beside of this, you can add my:

Reviewed-by: Heiko Schocher <hs@denx.de>

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[U-Boot] [PATCH 2/6] env: ubi: support configurable VID offset

[Heiko Schocher]

Hello Markus,

Am 15.04.2019 um 17:32 schrieb Markus Klotzbuecher:
> From: Hamish Guthrie <hamish.guthrie@kistler.com>

Please add a commit message.

> Signed-off-by: Hamish Guthrie <hamish.guthrie@kistler.com>
> Signed-off-by: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Kyungmin Park <kmpark@infradead.org>
> ---
>   env/Kconfig |  6 ++++++
>   env/ubi.c   | 17 +++++++++++++----
>   2 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/env/Kconfig b/env/Kconfig
> index 44c47220c2..595b658c26 100644
> --- a/env/Kconfig
> +++ b/env/Kconfig
> @@ -519,6 +519,12 @@ config ENV_UBI_VOLUME_REDUND
>   	help
>   	  Name of the redundant volume that you want to store the environment in.
>   
> +config ENV_UBI_VID_OFFSET
> +	int "ubi environment VID offset"
> +	depends on ENV_IS_IN_UBI
> +	help
> +	  UBI VID offset for environment.
> +
>   endif
>   
>   config USE_DEFAULT_ENV_FILE
> diff --git a/env/ubi.c b/env/ubi.c
> index 1dfdf0a8c8..67379f2557 100644
> --- a/env/ubi.c
> +++ b/env/ubi.c
> @@ -15,6 +15,15 @@
>   #include <ubi_uboot.h>
>   #undef crc32
>   
> +#define _QUOTE(x) #x
> +#define QUOTE(x) _QUOTE(x)
> +
> +#ifdef CONFIG_ENV_UBI_VID_OFFSET
> + #define UBI_VID_OFFSET QUOTE(CONFIG_ENV_UBI_VID_OFFSET)
> +#else
> + #define UBI_VID_OFFSET NULL
> +#endif
> +
>   DECLARE_GLOBAL_DATA_PTR;
>   
>   #ifdef CONFIG_CMD_SAVEENV
> @@ -28,7 +37,7 @@ static int env_ubi_save(void)
>   	if (ret)
>   		return ret;
>   
> -	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
> +	if (ubi_part(CONFIG_ENV_UBI_PART, UBI_VID_OFFSET)) {
>   		printf("\n** Cannot find mtd partition \"%s\"\n",
>   		       CONFIG_ENV_UBI_PART);
>   		return 1;
> @@ -70,7 +79,7 @@ static int env_ubi_save(void)
>   	if (ret)
>   		return ret;
>   
> -	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
> +	if (ubi_part(CONFIG_ENV_UBI_PART, UBI_VID_OFFSET)) {
>   		printf("\n** Cannot find mtd partition \"%s\"\n",
>   		       CONFIG_ENV_UBI_PART);
>   		return 1;
> @@ -111,7 +120,7 @@ static int env_ubi_load(void)
>   	tmp_env1 = (env_t *)env1_buf;
>   	tmp_env2 = (env_t *)env2_buf;
>   
> -	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
> +	if (ubi_part(CONFIG_ENV_UBI_PART, UBI_VID_OFFSET)) {
>   		printf("\n** Cannot find mtd partition \"%s\"\n",
>   		       CONFIG_ENV_UBI_PART);
>   		set_default_env(NULL, 0);
> @@ -148,7 +157,7 @@ static int env_ubi_load(void)
>   	 */
>   	memset(buf, 0x0, CONFIG_ENV_SIZE);
>   
> -	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
> +	if (ubi_part(CONFIG_ENV_UBI_PART, UBI_VID_OFFSET)) {
>   		printf("\n** Cannot find mtd partition \"%s\"\n",
>   		       CONFIG_ENV_UBI_PART);
>   		set_default_env(NULL, 0);
> 

Reviewed-by: Heiko Schocher <hs@denx.de>

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[U-Boot] [PATCH 3/6] ubi: fix UBI_SILENCE_MSG

[Heiko Schocher]

Hello Markus,

Am 15.04.2019 um 17:32 schrieb Markus Klotzbuecher:
> From: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
> 
>   - drop CONFIG_ prefix from kconfig entry
>   - fix small compilation issue with CONFIG_UBI_SILENCE_MSG
> 
> Signed-off-by: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Kyungmin Park <kmpark@infradead.org>
> ---
>   drivers/mtd/ubi/Kconfig | 2 +-
>   drivers/mtd/ubi/io.c    | 2 ++
>   2 files changed, 3 insertions(+), 1 deletion(-)

good catch!

Reviewed-by: Heiko Schocher <hs@denx.de>

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[U-Boot] [PATCH 4/6] ubispl: migrate configuration to Kconfig

[Heiko Schocher]

Hello Markus,

Am 15.04.2019 um 17:32 schrieb Markus Klotzbuecher:
> From: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
> 
> Signed-off-by: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Kyungmin Park <kmpark@infradead.org>
> Cc: Javier Martinez Canillas <javier@dowhile0.org>
> Cc: Enric Balletbo i Serra <eballetbo@gmail.com>
> ---
>   common/spl/Kconfig           | 79 ++++++++++++++++++++++++++++++++++++
>   scripts/config_whitelist.txt | 12 ------
>   2 files changed, 79 insertions(+), 12 deletions(-)

Please move from the config files:

./include/configs/omap3_igep00x0.h
./include/configs/am335x_igep003x.h

also the symbols to the defconfig files, thanks.

bye,
Heiko

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[U-Boot] [PATCH 5/6] ubispl: add support for loading volumes by name

[Heiko Schocher]

Hello Markus,

Am 15.04.2019 um 17:32 schrieb Markus Klotzbuecher:
> From: Hamish Guthrie <hamish.guthrie@kistler.com>
> 
> The motivation is to use the UBI atomic volume rename functionality to
> allow double copy software updates on UBI. To that end the SPL is
> configured to always load the same volume name (e.g. "u-boot"),
> whereas a software updater always installs into the secondary volume
> "u-boot_r". After successful installation, these two volume names are
> switched.
> 
> This extension is protected by #ifdefs as it will somewhat slow down
> loading of volumes by id. This is because the code needs to disable
> the optimization of ignoring all volume ids which are not
> to-be-loaded, since these can only be resolved after attaching.
> 
> Signed-off-by: Hamish Guthrie <hamish.guthrie@kistler.com>
> Signed-off-by: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Kyungmin Park <kmpark@infradead.org>
> ---
>   common/spl/Kconfig          |  13 +++
>   common/spl/spl_ubi.c        |   7 ++
>   drivers/mtd/ubispl/ubispl.c | 215 +++++++++++++++++++++++++++++++++++-
>   drivers/mtd/ubispl/ubispl.h |   7 ++
>   include/ubispl.h            |   6 +
>   5 files changed, 246 insertions(+), 2 deletions(-)
> 
> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> index 71bedea638..0e91bd309b 100644
> --- a/common/spl/Kconfig
> +++ b/common/spl/Kconfig
> @@ -556,6 +556,13 @@ config SPL_UBI
>   	  README.ubispl for more info.
>   
>   if SPL_UBI
> +config SPL_UBI_LOAD_BY_VOLNAME
> +	bool "Support loading volumes by name"
> +	help
> +	  This enables support for loading UBI volumes by name. When this
> +	  is set, CONFIG_SPL_UBI_LOAD_MONITOR_VOLNAME can be used to
> +	  configure the volume name from which to load U-Boot.
> +
>   config SPL_UBI_MAX_VOL_LEBS
>   	int "Maximum number of LEBs per volume"
>   	depends on SPL_UBI
> @@ -613,6 +620,12 @@ config SPL_UBI_LOAD_MONITOR_ID
>   	help
>   	  The UBI volume id from which to load U-Boot
>   
> +config SPL_UBI_LOAD_MONITOR_VOLNAME
> +	string "volume name of U-Boot volume"
> +	depends on SPL_UBI_LOAD_BY_VOLNAME
> +	help
> +	  The UBI volume name from which to load U-Boot
> +
>   config SPL_UBI_LOAD_KERNEL_ID
>   	int "id of kernel volume"
>   	depends on SPL_OS_BOOT && SPL_UBI
> diff --git a/common/spl/spl_ubi.c b/common/spl/spl_ubi.c
> index 67e5fadd7c..0cb5080882 100644
> --- a/common/spl/spl_ubi.c
> +++ b/common/spl/spl_ubi.c
> @@ -62,7 +62,14 @@ int spl_ubi_load_image(struct spl_image_info *spl_image,
>   	}
>   #endif
>   	header = spl_get_load_buffer(-sizeof(*header), sizeof(header));
> +#ifdef CONFIG_SPL_UBI_LOAD_BY_VOLNAME
> +	volumes[0].vol_id = -1;
> +	strncpy(volumes[0].name,
> +		CONFIG_SPL_UBI_LOAD_MONITOR_VOLNAME,
> +		UBI_VOL_NAME_MAX + 1);
> +#else
>   	volumes[0].vol_id = CONFIG_SPL_UBI_LOAD_MONITOR_ID;
> +#endif
>   	volumes[0].load_addr = (void *)header;
>   
>   	ret = ubispl_load_volumes(&info, volumes, 1);
> diff --git a/drivers/mtd/ubispl/ubispl.c b/drivers/mtd/ubispl/ubispl.c
> index eeb1cbefb7..3f3b9b4367 100644
> --- a/drivers/mtd/ubispl/ubispl.c
> +++ b/drivers/mtd/ubispl/ubispl.c
> @@ -45,6 +45,187 @@ static int ubi_io_is_bad(struct ubi_scan_info *ubi, int peb)
>   	return peb >= ubi->peb_count || peb < 0;
>   }
>   
> +#ifdef CONFIG_SPL_UBI_LOAD_BY_VOLNAME
> +
> +/**
> + * ubi_dump_vtbl_record - dump a &struct ubi_vtbl_record object.
> + * @r: the object to dump
> + * @idx: volume table index
> + */
> +void ubi_dump_vtbl_record(const struct ubi_vtbl_record *r, int idx)
> +{
> +	int name_len = be16_to_cpu(r->name_len);
> +
> +	ubi_dbg("Volume table record %d dump: size: %d",
> +		idx, sizeof(struct ubi_vtbl_record));
> +	ubi_dbg("\treserved_pebs   %d", be32_to_cpu(r->reserved_pebs));
> +	ubi_dbg("\talignment       %d", be32_to_cpu(r->alignment));
> +	ubi_dbg("\tdata_pad        %d", be32_to_cpu(r->data_pad));
> +	ubi_dbg("\tvol_type        %d", (int)r->vol_type);
> +	ubi_dbg("\tupd_marker      %d", (int)r->upd_marker);
> +	ubi_dbg("\tname_len        %d", name_len);
> +
> +	if (r->name[0] == '\0') {
> +		ubi_dbg("\tname            NULL");
> +		return;
> +	}
> +
> +	if (name_len <= UBI_VOL_NAME_MAX &&
> +	    strnlen(&r->name[0], name_len + 1) == name_len) {
> +		ubi_dbg("\tname            %s", &r->name[0]);
> +	} else {
> +		ubi_dbg("\t1st 5 characters of name: %c%c%c%c%c",
> +			r->name[0], r->name[1], r->name[2], r->name[3],
> +			r->name[4]);
> +	}
> +	ubi_dbg("\tcrc             %#08x", be32_to_cpu(r->crc));
> +}
> +
> +/* Empty volume table record */
> +static struct ubi_vtbl_record empty_vtbl_record;
> +
> +/**
> + * vtbl_check - check if volume table is not corrupted and sensible.
> + * @ubi: UBI device description object
> + * @vtbl: volume table
> + *
> + * This function returns zero if @vtbl is all right, %1 if CRC is incorrect,
> + * and %-EINVAL if it contains inconsistent data.
> + */
> +static int vtbl_check(struct ubi_scan_info *ubi,
> +		      struct ubi_vtbl_record *vtbl)
> +{
> +	int i, n, reserved_pebs, alignment, data_pad, vol_type, name_len;
> +	int upd_marker, err;
> +	uint32_t crc;

please use u32 instead uint32_t, thanks. Hmm... is this code from linux
kernel?

If so, can you please add in the commit message from which kernel commit
id you copied this function?

Also last sync with linux kernel was:

* 0195a7bb36 - ubi,ubifs: sync with linux v4.2 (vor 3 Jahren, und 6 Monaten) <Heiko Schocher>

very long time ... so it is interesting, from what kernel version you
have copied from, thanks!

(Also a sync from UBI/UBIFS with current linux kernel sources would be nice,
  but may not trivial)

> +	const char *name;
> +
> +	for (i = 0; i < UBI_SPL_VOL_IDS; i++) {
> +		reserved_pebs = be32_to_cpu(vtbl[i].reserved_pebs);
> +		alignment = be32_to_cpu(vtbl[i].alignment);
> +		data_pad = be32_to_cpu(vtbl[i].data_pad);
> +		upd_marker = vtbl[i].upd_marker;
> +		vol_type = vtbl[i].vol_type;
> +		name_len = be16_to_cpu(vtbl[i].name_len);
> +		name = &vtbl[i].name[0];
> +
> +		crc = crc32(UBI_CRC32_INIT, &vtbl[i], UBI_VTBL_RECORD_SIZE_CRC);
> +		if (be32_to_cpu(vtbl[i].crc) != crc) {
> +			ubi_err("bad CRC at record %u: %#08x, not %#08x",
> +				i, crc, be32_to_cpu(vtbl[i].crc));
> +			ubi_dump_vtbl_record(&vtbl[i], i);
> +			return 1;
> +		}
> +
> +		if (reserved_pebs == 0) {
> +			if (memcmp(&vtbl[i], &empty_vtbl_record,
> +				   UBI_VTBL_RECORD_SIZE)) {
> +				err = 2;
> +				goto bad;
> +			}
> +			continue;
> +		}
> +
> +		if (reserved_pebs < 0 || alignment < 0 || data_pad < 0 ||
> +		    name_len < 0) {
> +			err = 3;
> +			goto bad;
> +		}
> +
> +		if (alignment > ubi->leb_size || alignment == 0) {
> +			err = 4;
> +			goto bad;
> +		}
> +
> +		n = alignment & (CONFIG_SPL_UBI_VID_OFFSET - 1);
> +		if (alignment != 1 && n) {
> +			err = 5;
> +			goto bad;
> +		}
> +
> +		n = ubi->leb_size % alignment;
> +		if (data_pad != n) {
> +			ubi_err("bad data_pad, has to be %d", n);
> +			err = 6;
> +			goto bad;
> +		}
> +
> +		if (vol_type != UBI_VID_DYNAMIC && vol_type != UBI_VID_STATIC) {
> +			err = 7;
> +			goto bad;
> +		}
> +
> +		if (upd_marker != 0 && upd_marker != 1) {
> +			err = 8;
> +			goto bad;
> +		}
> +
> +		if (name_len > UBI_VOL_NAME_MAX) {
> +			err = 10;
> +			goto bad;
> +		}
> +
> +		if (name[0] == '\0') {
> +			err = 11;
> +			goto bad;
> +		}
> +
> +		if (name_len != strnlen(name, name_len + 1)) {
> +			err = 12;
> +			goto bad;
> +		}
> +
> +		ubi_dump_vtbl_record(&vtbl[i], i);
> +	}
> +
> +	/* Checks that all names are unique */
> +	for (i = 0; i < UBI_SPL_VOL_IDS - 1; i++) {
> +		for (n = i + 1; n < UBI_SPL_VOL_IDS; n++) {
> +			int len1 = be16_to_cpu(vtbl[i].name_len);
> +			int len2 = be16_to_cpu(vtbl[n].name_len);
> +
> +			if (len1 > 0 && len1 == len2 &&
> +			    !strncmp(vtbl[i].name, vtbl[n].name, len1)) {
> +				ubi_err("volumes %d and %d have the same name \"%s\"",
> +					i, n, vtbl[i].name);
> +				ubi_dump_vtbl_record(&vtbl[i], i);
> +				ubi_dump_vtbl_record(&vtbl[n], n);
> +				return -EINVAL;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +
> +bad:
> +	ubi_err("volume table check failed: record %d, error %d", i, err);
> +	ubi_dump_vtbl_record(&vtbl[i], i);
> +	return -EINVAL;
> +}
> +
> +static int ubi_read_volume_table(struct ubi_scan_info *ubi, u32 pnum)
> +{
> +	int err = -EINVAL;
> +
> +	empty_vtbl_record.crc = cpu_to_be32(0xf116c36b);
> +
> +	err = ubi_io_read(ubi, &ubi->vtbl, pnum, ubi->leb_start,
> +			  sizeof(struct ubi_vtbl_record) * UBI_SPL_VOL_IDS);
> +	if (err && err != UBI_IO_BITFLIPS) {
> +		ubi_err("unable to read volume table");
> +		goto out;
> +	}
> +
> +	if (!vtbl_check(ubi, ubi->vtbl)) {
> +		ubi->vtbl_valid = 1;
> +		err = 0;
> +	}
> +out:
> +	return err;
> +}
> +
> +#endif /* CONFIG_SPL_UBI_LOAD_BY_VOLNAME */
> +
>   static int ubi_io_read_vid_hdr(struct ubi_scan_info *ubi, int pnum,
>   			       struct ubi_vid_hdr *vh, int unused)
>   {
> @@ -210,14 +391,23 @@ static int ubi_scan_vid_hdr(struct ubi_scan_info *ubi, struct ubi_vid_hdr *vh,
>   	if (vol_id == UBI_FM_SB_VOLUME_ID)
>   		return ubi->fm_enabled ? UBI_FASTMAP_ANCHOR : 0;
>   
> +#ifdef CONFIG_SPL_UBI_LOAD_BY_VOLNAME
> +	/* If this is a UBI volume table, read it and return */
> +	if (vol_id == UBI_LAYOUT_VOLUME_ID && !ubi->vtbl_valid) {
> +		res = ubi_read_volume_table(ubi, pnum);
> +		return res;
> +	}
> +#endif
> +
>   	/* We only care about static volumes with an id < UBI_SPL_VOL_IDS */
>   	if (vol_id >= UBI_SPL_VOL_IDS || vh->vol_type != UBI_VID_STATIC)
>   		return 0;
>   
> +#ifndef CONFIG_SPL_UBI_LOAD_BY_VOLNAME
>   	/* We are only interested in the volumes to load */
>   	if (!test_bit(vol_id, ubi->toload))
>   		return 0;
> -
> +#endif
>   	lnum = be32_to_cpu(vh->lnum);
>   	return ubi_add_peb_to_vol(ubi, vh, vol_id, pnum, lnum);
>   }
> @@ -232,13 +422,14 @@ static int assign_aeb_to_av(struct ubi_scan_info *ubi, u32 pnum, u32 lnum,
>   
>   	ubi->fastmap_pebs++;
>   
> +#ifndef CONFIG_SPL_UBI_LOAD_BY_VOLNAME
>   	if (vol_id >= UBI_SPL_VOL_IDS || vol_type != UBI_STATIC_VOLUME)
>   		return 0;
>   
>   	/* We are only interested in the volumes to load */
>   	if (!test_bit(vol_id, ubi->toload))
>   		return 0;
> -
> +#endif
>   	vh = ubi->blockinfo + pnum;
>   
>   	return ubi_scan_vid_hdr(ubi, vh, pnum);
> @@ -892,6 +1083,10 @@ retry:
>   	ubi->peb_count = info->peb_count;
>   	ubi->peb_offset = info->peb_offset;
>   
> +#ifdef CONFIG_SPL_UBI_LOAD_BY_VOLNAME
> +	ubi->vtbl_valid = 0;
> +#endif
> +
>   	fsize = info->peb_size * info->peb_count;
>   	ubi->fsize_mb = fsize >> 20;
>   
> @@ -910,7 +1105,23 @@ retry:
>   	for (i = 0; i < nrvols; i++) {
>   		struct ubispl_load *lv = lvols + i;
>   
> +#ifdef CONFIG_SPL_UBI_LOAD_BY_VOLNAME
> +		if (lv->vol_id == -1) {
> +			for (int j = 0; j < UBI_SPL_VOL_IDS; j++) {
> +				int len = be16_to_cpu(ubi->vtbl[j].name_len);
> +
> +				if (strncmp(lv->name,
> +					    ubi->vtbl[j].name,
> +					    len) == 0) {
> +					lv->vol_id = j;
> +					break;
> +				}
> +			}
> +		}
> +		ubi_msg("Loading VolName %s (VolId #%d)", lv->name, lv->vol_id);
> +#else
>   		ubi_msg("Loading VolId #%d", lv->vol_id);
> +#endif
>   		res = ipl_load(ubi, lv->vol_id, lv->load_addr);
>   		if (res < 0) {
>   			if (fastmap) {
> diff --git a/drivers/mtd/ubispl/ubispl.h b/drivers/mtd/ubispl/ubispl.h
> index 9e40b46eac..bcc376c6d7 100644
> --- a/drivers/mtd/ubispl/ubispl.h
> +++ b/drivers/mtd/ubispl/ubispl.h
> @@ -77,6 +77,8 @@ struct ubi_vol_info {
>    * @blockinfo:		The vid headers of the scanned blocks
>    * @volinfo:		The volume information of the interesting (toload)
>    *			volumes
> + * @vtbl_corrupted:	Flag to indicate status of volume table
> + * @vtbl:		Volume table
>    *
>    * @fm_buf:		The large fastmap attach buffer
>    */
> @@ -112,6 +114,11 @@ struct ubi_scan_info {
>   	struct ubi_vol_info		volinfo[UBI_SPL_VOL_IDS];
>   	struct ubi_vid_hdr		blockinfo[CONFIG_SPL_UBI_MAX_PEBS];
>   
> +#ifdef CONFIG_SPL_UBI_LOAD_BY_VOLNAME
> +	/* Volume table */
> +	int                             vtbl_valid;
> +	struct ubi_vtbl_record          vtbl[UBI_SPL_VOL_IDS];
> +#endif
>   	/* The large buffer for the fastmap */
>   	uint8_t				fm_buf[UBI_FM_BUF_SIZE];
>   };
> diff --git a/include/ubispl.h b/include/ubispl.h
> index 1e5da94eb1..ecfe0c93c1 100644
> --- a/include/ubispl.h
> +++ b/include/ubispl.h
> @@ -5,6 +5,8 @@
>   #ifndef __UBOOT_UBISPL_H
>   #define __UBOOT_UBISPL_H
>   
> +#define UBI_VOL_NAME_MAX	127
> +
>   /*
>    * The following CONFIG options are relevant for UBISPL
>    *
> @@ -74,6 +76,10 @@ struct ubispl_info {
>    */
>   struct ubispl_load {
>   	int		vol_id;
> +#ifdef CONFIG_SPL_UBI_LOAD_BY_VOLNAME
> +	u32		name_len;
> +	char		name[UBI_VOL_NAME_MAX + 1];
> +#endif
>   	void		*load_addr;
>   };
>   
> 

Reviewed-by: Heiko Schocher <hs@denx.de>

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[U-Boot] [PATCH 6/6] ubispl: introduce separate CONFIG_UBI_SPL_SILENCE_MSG

[Heiko Schocher]

Hello Markus,

Am 15.04.2019 um 17:32 schrieb Markus Klotzbuecher:
> From: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
> 
> This allows to silence ubi and ubispl individually.
> 
> Signed-off-by: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Kyungmin Park <kmpark@infradead.org>
> ---
>   common/spl/Kconfig          | 6 ++++++
>   drivers/mtd/ubispl/ubispl.h | 2 +-
>   2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> index 0e91bd309b..37dce69e81 100644
> --- a/common/spl/Kconfig
> +++ b/common/spl/Kconfig
> @@ -638,6 +638,12 @@ config SPL_UBI_LOAD_ARGS_ID
>   	help
>   	  The UBI volume id from which to load the device tree
>   
> +config UBI_SPL_SILENCE_MSG
> +	bool "silence UBI SPL messages"
> +	default n
> +	help
> +	  Disable messages from UBI SPL. This leaves warnings
> +	  and errors enabled.
>   
>   endif   # if SPL_UBI
>   
> diff --git a/drivers/mtd/ubispl/ubispl.h b/drivers/mtd/ubispl/ubispl.h
> index bcc376c6d7..b7cb7fc941 100644
> --- a/drivers/mtd/ubispl/ubispl.h
> +++ b/drivers/mtd/ubispl/ubispl.h
> @@ -129,7 +129,7 @@ struct ubi_scan_info {
>   #define ubi_dbg(fmt, ...)
>   #endif
>   
> -#ifdef CONFIG_UBI_SILENCE_MSG
> +#ifdef CONFIG_UBI_SPL_SILENCE_MSG
>   #define ubi_msg(fmt, ...)
>   #else
>   #define ubi_msg(fmt, ...) printf("UBI: " fmt "\n", ##__VA_ARGS__)
> 

Reviewed-by: Heiko Schocher <hs@denx.de>

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[U-Boot] [PATCH 1/6] env: ubi: KConfig: add CONFIG_ENV_UBI_VOLUME_REDUND

[Markus Klotzbuecher]

Hello Heiko

On Tue, Apr 30, 2019 at 06:54:01AM +0200, Heiko Schocher wrote:

>Am 15.04.2019 um 17:32 schrieb Markus Klotzbuecher:
>> From: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
>
>please add a commit message.
>
>> Signed-off-by: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
>> Cc: Heiko Schocher <hs@denx.de>
>> Cc: Kyungmin Park <kmpark@infradead.org>
>> ---
>>   env/Kconfig                  | 6 ++++++
>>   scripts/config_whitelist.txt | 1 -
>>   2 files changed, 6 insertions(+), 1 deletion(-)
>> 
>> diff --git a/env/Kconfig b/env/Kconfig
>> index 78300660c7..44c47220c2 100644
>> --- a/env/Kconfig
>> +++ b/env/Kconfig
>> @@ -513,6 +513,12 @@ config ENV_UBI_VOLUME
>>   	help
>>   	  Name of the volume that you want to store the environment in.
>> +config ENV_UBI_VOLUME_REDUND
>> +	string "UBI redundant volume name"
>> +	depends on ENV_IS_IN_UBI
>> +	help
>> +	  Name of the redundant volume that you want to store the environment in.
>> +
>>   endif
>>   config USE_DEFAULT_ENV_FILE
>> diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
>> index fa98efc24c..5d76c781d3 100644
>> --- a/scripts/config_whitelist.txt
>> +++ b/scripts/config_whitelist.txt
>> @@ -504,7 +504,6 @@ CONFIG_ENV_SROM_BANK
>>   CONFIG_ENV_TOTAL_SIZE
>>   CONFIG_ENV_UBIFS_OPTION
>>   CONFIG_ENV_UBI_MTD
>> -CONFIG_ENV_UBI_VOLUME_REDUND
>>   CONFIG_ENV_VERSION
>>   CONFIG_EP9302
>>   CONFIG_EP9307
>> 
>
>Please move from the config files:
>
>./include/configs/omap3_igep00x0.h
>./include/configs/gardena-smart-gateway-at91sam.h
>./include/configs/am335x_igep003x.h
>
>also the symbols to the defconfig files, thanks.
>
>BTW: you can use the tool tools/moveconfig.py in u-boot source
>for this purpose.
>
>Beside of this, you can add my:
>
>Reviewed-by: Heiko Schocher <hs@denx.de>

Thank you for your feedback! I will go through it and submit a v2
within a few days.

Best regards
Markus

-- 
Markus Klotzbuecher
Freelancer Embedded, Distributed & Real-time Systems
Am See 28, 78465 Konstanz, Germany

[U-Boot] [PATCH 1/6] env: ubi: KConfig: add CONFIG_ENV_UBI_VOLUME_REDUND

[Markus Klotzbuecher]

Hello Heiko

On Tue, Apr 30, 2019 at 06:54:01AM +0200, Heiko Schocher wrote:

>Am 15.04.2019 um 17:32 schrieb Markus Klotzbuecher:
>> From: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
>
>please add a commit message.
>
>> Signed-off-by: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
>> Cc: Heiko Schocher <hs@denx.de>
>> Cc: Kyungmin Park <kmpark@infradead.org>
>> ---
>>   env/Kconfig                  | 6 ++++++
>>   scripts/config_whitelist.txt | 1 -
>>   2 files changed, 6 insertions(+), 1 deletion(-)
>> 
>> diff --git a/env/Kconfig b/env/Kconfig
>> index 78300660c7..44c47220c2 100644
>> --- a/env/Kconfig
>> +++ b/env/Kconfig
>> @@ -513,6 +513,12 @@ config ENV_UBI_VOLUME
>>   	help
>>   	  Name of the volume that you want to store the environment in.
>> +config ENV_UBI_VOLUME_REDUND
>> +	string "UBI redundant volume name"
>> +	depends on ENV_IS_IN_UBI
>> +	help
>> +	  Name of the redundant volume that you want to store the environment in.
>> +
>>   endif
>>   config USE_DEFAULT_ENV_FILE
>> diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
>> index fa98efc24c..5d76c781d3 100644
>> --- a/scripts/config_whitelist.txt
>> +++ b/scripts/config_whitelist.txt
>> @@ -504,7 +504,6 @@ CONFIG_ENV_SROM_BANK
>>   CONFIG_ENV_TOTAL_SIZE
>>   CONFIG_ENV_UBIFS_OPTION
>>   CONFIG_ENV_UBI_MTD
>> -CONFIG_ENV_UBI_VOLUME_REDUND
>>   CONFIG_ENV_VERSION
>>   CONFIG_EP9302
>>   CONFIG_EP9307
>> 
>
>Please move from the config files:
>
>./include/configs/omap3_igep00x0.h
>./include/configs/gardena-smart-gateway-at91sam.h
>./include/configs/am335x_igep003x.h
>
>also the symbols to the defconfig files, thanks.

I have a question: to convert these, I need to make available the
additional ENV_ configs to OMAP2PLUS and AT91:

diff --git a/env/Kconfig b/env/Kconfig
index 44c47220c2..1250656d74 100644
--- a/env/Kconfig
+++ b/env/Kconfig
@@ -470,7 +470,7 @@ config ENV_EXT4_FILE
 	  It's a string of the EXT4 file name. This file use to store the
 	  environment (explicit path to the file)
 
-if ARCH_ROCKCHIP || ARCH_SUNXI || ARCH_ZYNQ || ARCH_ZYNQMP || ARCH_VERSAL || ARC
+if ARCH_ROCKCHIP || ARCH_SUNXI || ARCH_ZYNQ || ARCH_ZYNQMP || ARCH_VERSAL || ARC || ARCH_OMAP2PLUS || ARCH_AT91

However, this "if" region contains a few other, non UBI settings such
as ENV_SIZE, which would become visible to a large number of OMAP2PLUS
and AT91 boards, which still define this in the headers.

I'm a bit hesitant to touch all of these. What is the suggested way to
solve this?

Thank you,
Markus

-- 
Markus Klotzbuecher
Freelancer Embedded, Distributed & Real-time Systems
Am See 28, 78465 Konstanz, Germany

[U-Boot] [PATCH 1/6] env: ubi: KConfig: add CONFIG_ENV_UBI_VOLUME_REDUND

[Heiko Schocher]

Hello Markus,

Am 09.05.2019 um 10:59 schrieb Markus Klotzbuecher:
> Hello Heiko
> 
> On Tue, Apr 30, 2019 at 06:54:01AM +0200, Heiko Schocher wrote:
> 
>> Am 15.04.2019 um 17:32 schrieb Markus Klotzbuecher:
>>> From: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
>>
>> please add a commit message.
>>
>>> Signed-off-by: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
>>> Cc: Heiko Schocher <hs@denx.de>
>>> Cc: Kyungmin Park <kmpark@infradead.org>
>>> ---
>>>    env/Kconfig                  | 6 ++++++
>>>    scripts/config_whitelist.txt | 1 -
>>>    2 files changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/env/Kconfig b/env/Kconfig
>>> index 78300660c7..44c47220c2 100644
>>> --- a/env/Kconfig
>>> +++ b/env/Kconfig
>>> @@ -513,6 +513,12 @@ config ENV_UBI_VOLUME
>>>    	help
>>>    	  Name of the volume that you want to store the environment in.
>>> +config ENV_UBI_VOLUME_REDUND
>>> +	string "UBI redundant volume name"
>>> +	depends on ENV_IS_IN_UBI
>>> +	help
>>> +	  Name of the redundant volume that you want to store the environment in.
>>> +
>>>    endif
>>>    config USE_DEFAULT_ENV_FILE
>>> diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
>>> index fa98efc24c..5d76c781d3 100644
>>> --- a/scripts/config_whitelist.txt
>>> +++ b/scripts/config_whitelist.txt
>>> @@ -504,7 +504,6 @@ CONFIG_ENV_SROM_BANK
>>>    CONFIG_ENV_TOTAL_SIZE
>>>    CONFIG_ENV_UBIFS_OPTION
>>>    CONFIG_ENV_UBI_MTD
>>> -CONFIG_ENV_UBI_VOLUME_REDUND
>>>    CONFIG_ENV_VERSION
>>>    CONFIG_EP9302
>>>    CONFIG_EP9307
>>>
>>
>> Please move from the config files:
>>
>> ./include/configs/omap3_igep00x0.h
>> ./include/configs/gardena-smart-gateway-at91sam.h
>> ./include/configs/am335x_igep003x.h
>>
>> also the symbols to the defconfig files, thanks.
> 
> I have a question: to convert these, I need to make available the
> additional ENV_ configs to OMAP2PLUS and AT91:
> 
> diff --git a/env/Kconfig b/env/Kconfig
> index 44c47220c2..1250656d74 100644
> --- a/env/Kconfig
> +++ b/env/Kconfig
> @@ -470,7 +470,7 @@ config ENV_EXT4_FILE
>   	  It's a string of the EXT4 file name. This file use to store the
>   	  environment (explicit path to the file)
>   
> -if ARCH_ROCKCHIP || ARCH_SUNXI || ARCH_ZYNQ || ARCH_ZYNQMP || ARCH_VERSAL || ARC
> +if ARCH_ROCKCHIP || ARCH_SUNXI || ARCH_ZYNQ || ARCH_ZYNQMP || ARCH_VERSAL || ARC || ARCH_OMAP2PLUS || ARCH_AT91
> 
> However, this "if" region contains a few other, non UBI settings such
> as ENV_SIZE, which would become visible to a large number of OMAP2PLUS
> and AT91 boards, which still define this in the headers.

Huch?

If so, than they are not converted (yet) ... :-(

> I'm a bit hesitant to touch all of these. What is the suggested way to
> solve this?

I think, they should be converted too ...

Sorry for the additional work ... I can understand your hesitantion
to do such a conversion...

Hmm... I used some year(s) ago tbot for checking, if a config change
did not introduced diffs in created binaries for all boards [1] ...

In principal I did:

- build all boards with "SOURCE_DATE_EPOCH=0"
   and created a md5sum from each binary
- apply patch(es)
- build again, create md5sums and check if mdsum is the same

Unfortunately not converted this testcase to the new tbot ...

But may it is possible to convert this into a script ?

bye,
Heiko
[1] https://github.com/hsdenx/tbot/blob/master/src/tc/uboot/tc_uboot_check_kconfig.py
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[U-Boot] [PATCH 1/6] env: ubi: KConfig: add CONFIG_ENV_UBI_VOLUME_REDUND

[Markus Klotzbuecher]

Hello Heiko

On Thu, May 09, 2019 at 01:17:06PM +0200, Heiko Schocher wrote:
>
>Am 09.05.2019 um 10:59 schrieb Markus Klotzbuecher:
>> Hello Heiko
>> 
>> On Tue, Apr 30, 2019 at 06:54:01AM +0200, Heiko Schocher wrote:
>> 
>> > Am 15.04.2019 um 17:32 schrieb Markus Klotzbuecher:
>> > > From: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
>> > 
>> > please add a commit message.
>> > 
>> > > Signed-off-by: Markus Klotzbuecher <markus.klotzbuecher@kistler.com>
>> > > Cc: Heiko Schocher <hs@denx.de>
>> > > Cc: Kyungmin Park <kmpark@infradead.org>
>> > > ---
>> > >    env/Kconfig                  | 6 ++++++
>> > >    scripts/config_whitelist.txt | 1 -
>> > >    2 files changed, 6 insertions(+), 1 deletion(-)
>> > > 
>> > > diff --git a/env/Kconfig b/env/Kconfig
>> > > index 78300660c7..44c47220c2 100644
>> > > --- a/env/Kconfig
>> > > +++ b/env/Kconfig
>> > > @@ -513,6 +513,12 @@ config ENV_UBI_VOLUME
>> > >    	help
>> > >    	  Name of the volume that you want to store the environment in.
>> > > +config ENV_UBI_VOLUME_REDUND
>> > > +	string "UBI redundant volume name"
>> > > +	depends on ENV_IS_IN_UBI
>> > > +	help
>> > > +	  Name of the redundant volume that you want to store the environment in.
>> > > +
>> > >    endif
>> > >    config USE_DEFAULT_ENV_FILE
>> > > diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
>> > > index fa98efc24c..5d76c781d3 100644
>> > > --- a/scripts/config_whitelist.txt
>> > > +++ b/scripts/config_whitelist.txt
>> > > @@ -504,7 +504,6 @@ CONFIG_ENV_SROM_BANK
>> > >    CONFIG_ENV_TOTAL_SIZE
>> > >    CONFIG_ENV_UBIFS_OPTION
>> > >    CONFIG_ENV_UBI_MTD
>> > > -CONFIG_ENV_UBI_VOLUME_REDUND
>> > >    CONFIG_ENV_VERSION
>> > >    CONFIG_EP9302
>> > >    CONFIG_EP9307
>> > > 
>> > 
>> > Please move from the config files:
>> > 
>> > ./include/configs/omap3_igep00x0.h
>> > ./include/configs/gardena-smart-gateway-at91sam.h
>> > ./include/configs/am335x_igep003x.h
>> > 
>> > also the symbols to the defconfig files, thanks.
>> 
>> I have a question: to convert these, I need to make available the
>> additional ENV_ configs to OMAP2PLUS and AT91:
>> 
>> diff --git a/env/Kconfig b/env/Kconfig
>> index 44c47220c2..1250656d74 100644
>> --- a/env/Kconfig
>> +++ b/env/Kconfig
>> @@ -470,7 +470,7 @@ config ENV_EXT4_FILE
>>   	  It's a string of the EXT4 file name. This file use to store the
>>   	  environment (explicit path to the file)
>> -if ARCH_ROCKCHIP || ARCH_SUNXI || ARCH_ZYNQ || ARCH_ZYNQMP || ARCH_VERSAL || ARC
>> +if ARCH_ROCKCHIP || ARCH_SUNXI || ARCH_ZYNQ || ARCH_ZYNQMP || ARCH_VERSAL || ARC || ARCH_OMAP2PLUS || ARCH_AT91
>> 
>> However, this "if" region contains a few other, non UBI settings such
>> as ENV_SIZE, which would become visible to a large number of OMAP2PLUS
>> and AT91 boards, which still define this in the headers.
>
>Huch?
>
>If so, than they are not converted (yet) ... :-(
>
>> I'm a bit hesitant to touch all of these. What is the suggested way to
>> solve this?
>
>I think, they should be converted too ...

OK.

>Sorry for the additional work ... I can understand your hesitantion
>to do such a conversion...

No problem, I just wasn't sure how handle this.

I'm now trying to run moveconfig as follows

$ grep -l "ARCH_OMAP2PLUS\|ARCH_AT91" configs/* | ./tools/moveconfig.py -s ENV_OFFSET ENV_SIZE ENV_SECT_SIZE -d -

but the command hangs infinitely.

If I run it for all boards without "-d", it starts processing and gets
through about 50 configs and then hangs too.

Any idea what may be the cause?

I'm using python 2.7.16rc1.

>Hmm... I used some year(s) ago tbot for checking, if a config change
>did not introduced diffs in created binaries for all boards [1] ...
>
>In principal I did:
>
>- build all boards with "SOURCE_DATE_EPOCH=0"
>  and created a md5sum from each binary
>- apply patch(es)
>- build again, create md5sums and check if mdsum is the same
>
>Unfortunately not converted this testcase to the new tbot ...
>
>But may it is possible to convert this into a script ?

Thank you, I'll take a look at it one I get that far.

Best regards
Markus

-- 
Markus Klotzbuecher
Freelancer Embedded, Distributed & Real-time Systems
Am See 28, 78465 Konstanz, Germany

[U-Boot] [PATCH 1/6] env: ubi: KConfig: add CONFIG_ENV_UBI_VOLUME_REDUND

[mk at mkio.de]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 1274 bytes --]

␎From: Markus Klotzbuecher <mk@mkio.de>
User-Agent: Mutt/1.10.1 (2018-07-13)
X-OutGoing-Spam-Status: No, score=-1.0
X-AntiAbuse: This header was added to track abuse, please include it with any abuse report
X-AntiAbuse: Primary Hostname - m2731.contabo.net
X-AntiAbuse: Original Domain - lists.denx.de
X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12]
X-AntiAbuse: Sender Address Domain - mkio.de
X-Get-Message-Sender-Via: m2731.contabo.net: authenticated_id: mk at mkio.de
X-Authenticated-Sender: m2731.contabo.net: mk at mkio.de
X-Source: 
X-Source-Args: 
X-Source-Dir: 

On Thu, May 09, 2019 at 05:49:51PM +0200, Markus Klotzbuecher wrote:
...

>I'm now trying to run moveconfig as follows
>
>$ grep -l "ARCH_OMAP2PLUS\|ARCH_AT91" configs/* | ./tools/moveconfig.py -s ENV_OFFSET ENV_SIZE ENV_SECT_SIZE -d -
>
>but the command hangs infinitely.
>
>If I run it for all boards without "-d", it starts processing and gets
>through about 50 configs and then hangs too.
>
>Any idea what may be the cause?

I found it: I didn't add a default to the new KConfig, so the script
got stuck when it was asked for the new config options.

Sorry for the noise.

Markus

-- 
Markus Klotzbuecher
Freelancer Embedded, Distributed & Real-time
Am See 28, 78465 Konstanz, Germany