[U-Boot] [PATCH 0/5] Add support for using an UBI volume for environment

[U-Boot] [PATCH 0/5] Add support for using an UBI volume for environment

[Joe Hershberger]

NAND is not good at handling absolute addresses to sectors for storing
particular data.  The current implementation of the NAND env support
works around this in several ways such as storing a pointer to the
sector in the OOB of the first sector (interferes with some CRC) or
supporting a range of sectors (which unless it is huge is not
guaranteed to be safe).  None of these options address wear-leveling
concerns or bad block handling.

Accessing the u-boot env from UBI eliminates these concerns.  However,
it does require some of the basic settings for finding the UBI env to
be in the default u-boot env.


Joe Hershberger (5):
  ubi: Expose a few simple functions from the cmd_ubi
  ubi: ubifs: Turn off verbose prints
  mtd: Make mtdparts work with pre-reloc env
  env: Add support for UBI environment
  env: Add redundant env support to UBI env

 README                |  21 +++++
 common/Makefile       |   1 +
 common/cmd_mtdparts.c |  23 +++++-
 common/cmd_nvedit.c   |   7 +-
 common/cmd_ubi.c      | 149 +++++++++++++++++++---------------
 common/env_ubi.c      | 218 ++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/mtdpart.c |  14 ++--
 drivers/mtd/ubi/ubi.h |   3 +-
 fs/ubifs/ubifs.h      |   2 +-
 include/environment.h |  18 +++++
 include/ubi_uboot.h   |   3 +
 tools/env/fw_env.c    |   6 +-
 12 files changed, 387 insertions(+), 78 deletions(-)
 create mode 100644 common/env_ubi.c

-- 
1.7.11.5

[U-Boot] [PATCH 1/5] ubi: Expose a few simple functions from the cmd_ubi

[Joe Hershberger]

Part, Read, and Write functionality that will be used by env_ubi.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
 common/cmd_ubi.c    | 146 ++++++++++++++++++++++++++++------------------------
 include/ubi_uboot.h |   3 ++
 2 files changed, 83 insertions(+), 66 deletions(-)

diff --git a/common/cmd_ubi.c b/common/cmd_ubi.c
index 35b1d31..01335dd 100644
--- a/common/cmd_ubi.c
+++ b/common/cmd_ubi.c
@@ -263,7 +263,7 @@ out_err:
 	return err;
 }
 
-static int ubi_volume_write(char *volume, void *buf, size_t size)
+int ubi_volume_write(char *volume, void *buf, size_t size)
 {
 	int err = 1;
 	int rsvd_bytes = 0;
@@ -308,12 +308,10 @@ static int ubi_volume_write(char *volume, void *buf, size_t size)
 		ubi_gluebi_updated(vol);
 	}
 
-	printf("%d bytes written to volume %s\n", size, volume);
-
 	return 0;
 }
 
-static int ubi_volume_read(char *volume, char *buf, size_t size)
+int ubi_volume_read(char *volume, char *buf, size_t size)
 {
 	int err, lnum, off, len, tbuf_size;
 	void *tbuf;
@@ -325,8 +323,6 @@ static int ubi_volume_read(char *volume, char *buf, size_t size)
 	if (vol == NULL)
 		return ENODEV;
 
-	printf("Read %d bytes from volume %s to %p\n", size, volume, buf);
-
 	if (vol->updating) {
 		printf("updating");
 		return EBUSY;
@@ -431,26 +427,82 @@ static int ubi_dev_scan(struct mtd_info *info, char *ubidev,
 	return 0;
 }
 
-static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
+int ubi_part(char *part_name, const char *vid_header_offset)
 {
-	size_t size = 0;
-	ulong addr = 0;
 	int err = 0;
-
-	if (argc < 2)
-		return CMD_RET_USAGE;
+	char mtd_dev[16];
+	struct mtd_device *dev;
+	struct part_info *part;
+	u8 pnum;
 
 	if (mtdparts_init() != 0) {
 		printf("Error initializing mtdparts!\n");
 		return 1;
 	}
 
+#ifdef CONFIG_CMD_UBIFS
+	/*
+	 * Automatically unmount UBIFS partition when user
+	 * changes the UBI device. Otherwise the following
+	 * UBIFS commands will crash.
+	 */
+	if (ubifs_is_mounted())
+		cmd_ubifs_umount();
+#endif
+
+	/* todo: get dev number for NAND... */
+	ubi_dev.nr = 0;
+
+	/*
+	 * Call ubi_exit() before re-initializing the UBI subsystem
+	 */
+	if (ubi_initialized) {
+		ubi_exit();
+		del_mtd_partitions(ubi_dev.mtd_info);
+	}
+
+	/*
+	 * Search the mtd device number where this partition
+	 * is located
+	 */
+	if (find_dev_and_part(part_name, &dev, &pnum, &part)) {
+		printf("Partition %s not found!\n", part_name);
+		return 1;
+	}
+	sprintf(mtd_dev, "%s%d", MTD_DEV_TYPE(dev->id->type), dev->id->num);
+	ubi_dev.mtd_info = get_mtd_device_nm(mtd_dev);
+	if (IS_ERR(ubi_dev.mtd_info)) {
+		printf("Partition %s not found on device %s!\n", part_name,
+			mtd_dev);
+		return 1;
+	}
+
+	ubi_dev.selected = 1;
+
+	strcpy(ubi_dev.part_name, part_name);
+	err = ubi_dev_scan(ubi_dev.mtd_info, ubi_dev.part_name,
+			vid_header_offset);
+	if (err) {
+		printf("UBI init error %d\n", err);
+		ubi_dev.selected = 0;
+		return err;
+	}
+
+	ubi = ubi_devices[0];
+
+	return 0;
+}
+
+static int do_ubi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	size_t size = 0;
+	ulong addr = 0;
+
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
 	if (strcmp(argv[1], "part") == 0) {
-		char mtd_dev[16];
-		struct mtd_device *dev;
-		struct part_info *part;
 		const char *vid_header_offset = NULL;
-		u8 pnum;
 
 		/* Print current partition */
 		if (argc == 2) {
@@ -467,58 +519,10 @@ static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 		if (argc < 3)
 			return CMD_RET_USAGE;
 
-#ifdef CONFIG_CMD_UBIFS
-		/*
-		 * Automatically unmount UBIFS partition when user
-		 * changes the UBI device. Otherwise the following
-		 * UBIFS commands will crash.
-		 */
-		if (ubifs_is_mounted())
-			cmd_ubifs_umount();
-#endif
-
-		/* todo: get dev number for NAND... */
-		ubi_dev.nr = 0;
-
-		/*
-		 * Call ubi_exit() before re-initializing the UBI subsystem
-		 */
-		if (ubi_initialized) {
-			ubi_exit();
-			del_mtd_partitions(ubi_dev.mtd_info);
-		}
-
-		/*
-		 * Search the mtd device number where this partition
-		 * is located
-		 */
-		if (find_dev_and_part(argv[2], &dev, &pnum, &part)) {
-			printf("Partition %s not found!\n", argv[2]);
-			return 1;
-		}
-		sprintf(mtd_dev, "%s%d", MTD_DEV_TYPE(dev->id->type), dev->id->num);
-		ubi_dev.mtd_info = get_mtd_device_nm(mtd_dev);
-		if (IS_ERR(ubi_dev.mtd_info)) {
-			printf("Partition %s not found on device %s!\n", argv[2], mtd_dev);
-			return 1;
-		}
-
-		ubi_dev.selected = 1;
-
 		if (argc > 3)
 			vid_header_offset = argv[3];
-		strcpy(ubi_dev.part_name, argv[2]);
-		err = ubi_dev_scan(ubi_dev.mtd_info, ubi_dev.part_name,
-				vid_header_offset);
-		if (err) {
-			printf("UBI init error %d\n", err);
-			ubi_dev.selected = 0;
-			return err;
-		}
 
-		ubi = ubi_devices[0];
-
-		return 0;
+		return ubi_part(argv[2], vid_header_offset);
 	}
 
 	if ((strcmp(argv[1], "part") != 0) && (!ubi_dev.selected)) {
@@ -571,6 +575,8 @@ static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 	}
 
 	if (strncmp(argv[1], "write", 5) == 0) {
+		int ret;
+
 		if (argc < 5) {
 			printf("Please see usage\n");
 			return 1;
@@ -579,7 +585,12 @@ static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 		addr = simple_strtoul(argv[2], NULL, 16);
 		size = simple_strtoul(argv[4], NULL, 16);
 
-		return ubi_volume_write(argv[3], (void *)addr, size);
+		ret = ubi_volume_write(argv[3], (void *)addr, size);
+		if (!ret)
+			printf("%d bytes written to volume %s\n", size,
+				argv[3]);
+
+		return ret;
 	}
 
 	if (strncmp(argv[1], "read", 4) == 0) {
@@ -598,6 +609,9 @@ static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 		}
 
 		if (argc == 3)
+			printf("Read %d bytes from volume %s to %lx\n", size,
+				argv[3], addr);
+
 			return ubi_volume_read(argv[3], (char *)addr, size);
 	}
 
diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h
index 69006e2..1207895 100644
--- a/include/ubi_uboot.h
+++ b/include/ubi_uboot.h
@@ -214,6 +214,9 @@ static inline long IS_ERR(const void *ptr)
 extern int ubi_mtd_param_parse(const char *val, struct kernel_param *kp);
 extern int ubi_init(void);
 extern void ubi_exit(void);
+int ubi_part(char *part_name, const char *vid_header_offset);
+extern int ubi_volume_write(char *volume, void *buf, size_t size);
+extern int ubi_volume_read(char *volume, char *buf, size_t size);
 
 extern struct ubi_device *ubi_devices[];
 
-- 
1.7.11.5

[U-Boot] [PATCH 1/5] ubi: Expose a few simple functions from the cmd_ubi

[Stefan Roese]

On 02/08/2013 09:07 PM, Joe Hershberger wrote:
> Part, Read, and Write functionality that will be used by env_ubi.
> 
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>

Some minor nitpicking comments below.

> ---
>  common/cmd_ubi.c    | 146 ++++++++++++++++++++++++++++------------------------
>  include/ubi_uboot.h |   3 ++
>  2 files changed, 83 insertions(+), 66 deletions(-)
> 
> diff --git a/common/cmd_ubi.c b/common/cmd_ubi.c
> index 35b1d31..01335dd 100644
> --- a/common/cmd_ubi.c
> +++ b/common/cmd_ubi.c
> @@ -263,7 +263,7 @@ out_err:
>  	return err;
>  }
>  
> -static int ubi_volume_write(char *volume, void *buf, size_t size)
> +int ubi_volume_write(char *volume, void *buf, size_t size)
>  {
>  	int err = 1;
>  	int rsvd_bytes = 0;
> @@ -308,12 +308,10 @@ static int ubi_volume_write(char *volume, void *buf, size_t size)
>  		ubi_gluebi_updated(vol);
>  	}
>  
> -	printf("%d bytes written to volume %s\n", size, volume);
> -
>  	return 0;
>  }
>  
> -static int ubi_volume_read(char *volume, char *buf, size_t size)
> +int ubi_volume_read(char *volume, char *buf, size_t size)
>  {
>  	int err, lnum, off, len, tbuf_size;
>  	void *tbuf;
> @@ -325,8 +323,6 @@ static int ubi_volume_read(char *volume, char *buf, size_t size)
>  	if (vol == NULL)
>  		return ENODEV;
>  
> -	printf("Read %d bytes from volume %s to %p\n", size, volume, buf);
> -
>  	if (vol->updating) {
>  		printf("updating");
>  		return EBUSY;
> @@ -431,26 +427,82 @@ static int ubi_dev_scan(struct mtd_info *info, char *ubidev,
>  	return 0;
>  }
>  
> -static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
> +int ubi_part(char *part_name, const char *vid_header_offset)
>  {
> -	size_t size = 0;
> -	ulong addr = 0;
>  	int err = 0;
> -
> -	if (argc < 2)
> -		return CMD_RET_USAGE;
> +	char mtd_dev[16];
> +	struct mtd_device *dev;
> +	struct part_info *part;
> +	u8 pnum;
>  
>  	if (mtdparts_init() != 0) {
>  		printf("Error initializing mtdparts!\n");
>  		return 1;
>  	}
>  
> +#ifdef CONFIG_CMD_UBIFS
> +	/*
> +	 * Automatically unmount UBIFS partition when user
> +	 * changes the UBI device. Otherwise the following
> +	 * UBIFS commands will crash.
> +	 */
> +	if (ubifs_is_mounted())
> +		cmd_ubifs_umount();
> +#endif
> +
> +	/* todo: get dev number for NAND... */
> +	ubi_dev.nr = 0;
> +
> +	/*
> +	 * Call ubi_exit() before re-initializing the UBI subsystem
> +	 */
> +	if (ubi_initialized) {
> +		ubi_exit();
> +		del_mtd_partitions(ubi_dev.mtd_info);
> +	}
> +
> +	/*
> +	 * Search the mtd device number where this partition
> +	 * is located
> +	 */
> +	if (find_dev_and_part(part_name, &dev, &pnum, &part)) {
> +		printf("Partition %s not found!\n", part_name);
> +		return 1;
> +	}
> +	sprintf(mtd_dev, "%s%d", MTD_DEV_TYPE(dev->id->type), dev->id->num);
> +	ubi_dev.mtd_info = get_mtd_device_nm(mtd_dev);
> +	if (IS_ERR(ubi_dev.mtd_info)) {
> +		printf("Partition %s not found on device %s!\n", part_name,
> +			mtd_dev);
> +		return 1;
> +	}
> +
> +	ubi_dev.selected = 1;
> +
> +	strcpy(ubi_dev.part_name, part_name);
> +	err = ubi_dev_scan(ubi_dev.mtd_info, ubi_dev.part_name,
> +			vid_header_offset);
> +	if (err) {
> +		printf("UBI init error %d\n", err);
> +		ubi_dev.selected = 0;
> +		return err;
> +	}
> +
> +	ubi = ubi_devices[0];
> +
> +	return 0;
> +}
> +
> +static int do_ubi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> +	size_t size = 0;
> +	ulong addr = 0;
> +
> +	if (argc < 2)
> +		return CMD_RET_USAGE;
> +
>  	if (strcmp(argv[1], "part") == 0) {
> -		char mtd_dev[16];
> -		struct mtd_device *dev;
> -		struct part_info *part;
>  		const char *vid_header_offset = NULL;
> -		u8 pnum;
>  
>  		/* Print current partition */
>  		if (argc == 2) {
> @@ -467,58 +519,10 @@ static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
>  		if (argc < 3)
>  			return CMD_RET_USAGE;
>  
> -#ifdef CONFIG_CMD_UBIFS
> -		/*
> -		 * Automatically unmount UBIFS partition when user
> -		 * changes the UBI device. Otherwise the following
> -		 * UBIFS commands will crash.
> -		 */
> -		if (ubifs_is_mounted())
> -			cmd_ubifs_umount();
> -#endif
> -
> -		/* todo: get dev number for NAND... */
> -		ubi_dev.nr = 0;
> -
> -		/*
> -		 * Call ubi_exit() before re-initializing the UBI subsystem
> -		 */
> -		if (ubi_initialized) {
> -			ubi_exit();
> -			del_mtd_partitions(ubi_dev.mtd_info);
> -		}
> -
> -		/*
> -		 * Search the mtd device number where this partition
> -		 * is located
> -		 */
> -		if (find_dev_and_part(argv[2], &dev, &pnum, &part)) {
> -			printf("Partition %s not found!\n", argv[2]);
> -			return 1;
> -		}
> -		sprintf(mtd_dev, "%s%d", MTD_DEV_TYPE(dev->id->type), dev->id->num);
> -		ubi_dev.mtd_info = get_mtd_device_nm(mtd_dev);
> -		if (IS_ERR(ubi_dev.mtd_info)) {
> -			printf("Partition %s not found on device %s!\n", argv[2], mtd_dev);
> -			return 1;
> -		}
> -
> -		ubi_dev.selected = 1;
> -
>  		if (argc > 3)
>  			vid_header_offset = argv[3];
> -		strcpy(ubi_dev.part_name, argv[2]);
> -		err = ubi_dev_scan(ubi_dev.mtd_info, ubi_dev.part_name,
> -				vid_header_offset);
> -		if (err) {
> -			printf("UBI init error %d\n", err);
> -			ubi_dev.selected = 0;
> -			return err;
> -		}
>  
> -		ubi = ubi_devices[0];
> -
> -		return 0;
> +		return ubi_part(argv[2], vid_header_offset);
>  	}
>  
>  	if ((strcmp(argv[1], "part") != 0) && (!ubi_dev.selected)) {
> @@ -571,6 +575,8 @@ static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
>  	}
>  
>  	if (strncmp(argv[1], "write", 5) == 0) {
> +		int ret;
> +
>  		if (argc < 5) {
>  			printf("Please see usage\n");
>  			return 1;
> @@ -579,7 +585,12 @@ static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
>  		addr = simple_strtoul(argv[2], NULL, 16);
>  		size = simple_strtoul(argv[4], NULL, 16);
>  
> -		return ubi_volume_write(argv[3], (void *)addr, size);
> +		ret = ubi_volume_write(argv[3], (void *)addr, size);
> +		if (!ret)
> +			printf("%d bytes written to volume %s\n", size,
> +				argv[3]);

Use parentheses on multi-line statements please.

> +
> +		return ret;
>  	}
>  
>  	if (strncmp(argv[1], "read", 4) == 0) {
> @@ -598,6 +609,9 @@ static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
>  		}
>  
>  		if (argc == 3)
> +			printf("Read %d bytes from volume %s to %lx\n", size,
> +				argv[3], addr);

Again.

> +
>  			return ubi_volume_read(argv[3], (char *)addr, size);
>  	}
>  
> diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h
> index 69006e2..1207895 100644
> --- a/include/ubi_uboot.h
> +++ b/include/ubi_uboot.h
> @@ -214,6 +214,9 @@ static inline long IS_ERR(const void *ptr)
>  extern int ubi_mtd_param_parse(const char *val, struct kernel_param *kp);
>  extern int ubi_init(void);
>  extern void ubi_exit(void);
> +int ubi_part(char *part_name, const char *vid_header_offset);
> +extern int ubi_volume_write(char *volume, void *buf, size_t size);
> +extern int ubi_volume_read(char *volume, char *buf, size_t size);

Why do you add one function prototype without "extern" and 2 with?
Please handle this consistently.

Thanks,
Stefan

[U-Boot] [PATCH 2/5] ubi: ubifs: Turn off verbose prints

[Joe Hershberger]

The prints are out of control.  SILENCE!

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
 common/cmd_ubi.c      |  3 +++
 drivers/mtd/mtdpart.c | 14 ++++++++------
 drivers/mtd/ubi/ubi.h |  3 ++-
 fs/ubifs/ubifs.h      |  2 +-
 4 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/common/cmd_ubi.c b/common/cmd_ubi.c
index 01335dd..02b6b81 100644
--- a/common/cmd_ubi.c
+++ b/common/cmd_ubi.c
@@ -23,6 +23,9 @@
 #include <asm/errno.h>
 #include <jffs2/load_kernel.h>
 
+#undef ubi_msg
+#define ubi_msg(fmt, ...) printk(KERN_NOTICE "UBI: " fmt "\n", ##__VA_ARGS__)
+
 #define DEV_TYPE_NONE		0
 #define DEV_TYPE_NAND		1
 #define DEV_TYPE_ONENAND	2
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 96dcda2..2c60293 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -347,16 +347,18 @@ static struct mtd_part *add_one_partition(struct mtd_info *master,
 		if (mtd_mod_by_eb(cur_offset, master) != 0) {
 			/* Round up to next erasesize */
 			slave->offset = (mtd_div_by_eb(cur_offset, master) + 1) * master->erasesize;
-			printk(KERN_NOTICE "Moving partition %d: "
-			       "0x%012llx -> 0x%012llx\n", partno,
-			       (unsigned long long)cur_offset, (unsigned long long)slave->offset);
+			debug("Moving partition %d: 0x%012llx -> 0x%012llx\n",
+				partno, (unsigned long long)cur_offset,
+				(unsigned long long)slave->offset);
 		}
 	}
 	if (slave->mtd.size == MTDPART_SIZ_FULL)
 		slave->mtd.size = master->size - slave->offset;
 
-	printk(KERN_NOTICE "0x%012llx-0x%012llx : \"%s\"\n", (unsigned long long)slave->offset,
-		(unsigned long long)(slave->offset + slave->mtd.size), slave->mtd.name);
+	debug("0x%012llx-0x%012llx : \"%s\"\n",
+		(unsigned long long)slave->offset,
+		(unsigned long long)(slave->offset + slave->mtd.size),
+		slave->mtd.name);
 
 	/* let's do some sanity checks */
 	if (slave->offset >= master->size) {
@@ -463,7 +465,7 @@ int add_mtd_partitions(struct mtd_info *master,
 	if (mtd_partitions.next == NULL)
 		INIT_LIST_HEAD(&mtd_partitions);
 
-	printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
+	debug("Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
 
 	for (i = 0; i < nbparts; i++) {
 		slave = add_one_partition(master, parts + i, i, cur_offset);
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index 14c3a5f..f4ed7d8 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -59,7 +59,8 @@
 #define UBI_NAME_STR "ubi"
 
 /* Normal UBI messages */
-#define ubi_msg(fmt, ...) printk(KERN_NOTICE "UBI: " fmt "\n", ##__VA_ARGS__)
+#define ubi_msg(fmt, ...) /*printk(KERN_NOTICE "UBI: " fmt "\n", \
+				##__VA_ARGS__)*/
 /* UBI warning messages */
 #define ubi_warn(fmt, ...) printk(KERN_WARNING "UBI warning: %s: " fmt "\n", \
 				  __func__, ##__VA_ARGS__)
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 0af471a..4ab1cbd 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -464,7 +464,7 @@ static inline ino_t parent_ino(struct dentry *dentry)
 
 /* Normal UBIFS messages */
 #define ubifs_msg(fmt, ...) \
-		printk(KERN_NOTICE "UBIFS: " fmt "\n", ##__VA_ARGS__)
+		/*printk(KERN_NOTICE "UBIFS: " fmt "\n", ##__VA_ARGS__)*/
 /* UBIFS error messages */
 #define ubifs_err(fmt, ...)                                                  \
 	printk(KERN_ERR "UBIFS error (pid %d): %s: " fmt "\n", 0, \
-- 
1.7.11.5

[U-Boot] [PATCH 2/5] ubi: ubifs: Turn off verbose prints

[Stefan Roese]

On 02/08/2013 09:07 PM, Joe Hershberger wrote:
> The prints are out of control.  SILENCE!
> 
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
> ---
>  common/cmd_ubi.c      |  3 +++
>  drivers/mtd/mtdpart.c | 14 ++++++++------
>  drivers/mtd/ubi/ubi.h |  3 ++-
>  fs/ubifs/ubifs.h      |  2 +-
>  4 files changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/common/cmd_ubi.c b/common/cmd_ubi.c
> index 01335dd..02b6b81 100644
> --- a/common/cmd_ubi.c
> +++ b/common/cmd_ubi.c
> @@ -23,6 +23,9 @@
>  #include <asm/errno.h>
>  #include <jffs2/load_kernel.h>
>  
> +#undef ubi_msg
> +#define ubi_msg(fmt, ...) printk(KERN_NOTICE "UBI: " fmt "\n", ##__VA_ARGS__)
> +
>  #define DEV_TYPE_NONE		0
>  #define DEV_TYPE_NAND		1
>  #define DEV_TYPE_ONENAND	2
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index 96dcda2..2c60293 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -347,16 +347,18 @@ static struct mtd_part *add_one_partition(struct mtd_info *master,
>  		if (mtd_mod_by_eb(cur_offset, master) != 0) {
>  			/* Round up to next erasesize */
>  			slave->offset = (mtd_div_by_eb(cur_offset, master) + 1) * master->erasesize;
> -			printk(KERN_NOTICE "Moving partition %d: "
> -			       "0x%012llx -> 0x%012llx\n", partno,
> -			       (unsigned long long)cur_offset, (unsigned long long)slave->offset);
> +			debug("Moving partition %d: 0x%012llx -> 0x%012llx\n",
> +				partno, (unsigned long long)cur_offset,
> +				(unsigned long long)slave->offset);
>  		}
>  	}
>  	if (slave->mtd.size == MTDPART_SIZ_FULL)
>  		slave->mtd.size = master->size - slave->offset;
>  
> -	printk(KERN_NOTICE "0x%012llx-0x%012llx : \"%s\"\n", (unsigned long long)slave->offset,
> -		(unsigned long long)(slave->offset + slave->mtd.size), slave->mtd.name);
> +	debug("0x%012llx-0x%012llx : \"%s\"\n",
> +		(unsigned long long)slave->offset,
> +		(unsigned long long)(slave->offset + slave->mtd.size),
> +		slave->mtd.name);
>  
>  	/* let's do some sanity checks */
>  	if (slave->offset >= master->size) {
> @@ -463,7 +465,7 @@ int add_mtd_partitions(struct mtd_info *master,
>  	if (mtd_partitions.next == NULL)
>  		INIT_LIST_HEAD(&mtd_partitions);
>  
> -	printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
> +	debug("Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);

Not so sure about this one. Other users might find this message quite
useful. Does this output really interfere with your UBI env handling?

>  	for (i = 0; i < nbparts; i++) {
>  		slave = add_one_partition(master, parts + i, i, cur_offset);
> diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
> index 14c3a5f..f4ed7d8 100644
> --- a/drivers/mtd/ubi/ubi.h
> +++ b/drivers/mtd/ubi/ubi.h
> @@ -59,7 +59,8 @@
>  #define UBI_NAME_STR "ubi"
>  
>  /* Normal UBI messages */
> -#define ubi_msg(fmt, ...) printk(KERN_NOTICE "UBI: " fmt "\n", ##__VA_ARGS__)
> +#define ubi_msg(fmt, ...) /*printk(KERN_NOTICE "UBI: " fmt "\n", \
> +				##__VA_ARGS__)*/

Hmmm....

>  /* UBI warning messages */
>  #define ubi_warn(fmt, ...) printk(KERN_WARNING "UBI warning: %s: " fmt "\n", \
>  				  __func__, ##__VA_ARGS__)
> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
> index 0af471a..4ab1cbd 100644
> --- a/fs/ubifs/ubifs.h
> +++ b/fs/ubifs/ubifs.h
> @@ -464,7 +464,7 @@ static inline ino_t parent_ino(struct dentry *dentry)
>  
>  /* Normal UBIFS messages */
>  #define ubifs_msg(fmt, ...) \
> -		printk(KERN_NOTICE "UBIFS: " fmt "\n", ##__VA_ARGS__)
> +		/*printk(KERN_NOTICE "UBIFS: " fmt "\n", ##__VA_ARGS__)*/

... Not sure again about this silencing. And these "removed" printk's
are really ugly.

Could you please give an example of UBI usage (message logs from "ubi
create ..., ubi part ..., ubi read ...") without this patch (or complete
patchset) and with it? So that we see the real difference?

Thanks,
Stefan

[U-Boot] [PATCH 2/5] ubi: ubifs: Turn off verbose prints

[Joe Hershberger]

Hi Stefan,

Sorry for the delay.

On Mon, Feb 11, 2013 at 4:52 AM, Stefan Roese <sr@denx.de> wrote:
> On 02/08/2013 09:07 PM, Joe Hershberger wrote:
>> The prints are out of control.  SILENCE!
>>
>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>> ---
>>  common/cmd_ubi.c      |  3 +++
>>  drivers/mtd/mtdpart.c | 14 ++++++++------
>>  drivers/mtd/ubi/ubi.h |  3 ++-
>>  fs/ubifs/ubifs.h      |  2 +-
>>  4 files changed, 14 insertions(+), 8 deletions(-)
>>
>> diff --git a/common/cmd_ubi.c b/common/cmd_ubi.c
>> index 01335dd..02b6b81 100644
>> --- a/common/cmd_ubi.c
>> +++ b/common/cmd_ubi.c
>> @@ -23,6 +23,9 @@
>>  #include <asm/errno.h>
>>  #include <jffs2/load_kernel.h>
>>
>> +#undef ubi_msg
>> +#define ubi_msg(fmt, ...) printk(KERN_NOTICE "UBI: " fmt "\n", ##__VA_ARGS__)
>> +
>>  #define DEV_TYPE_NONE                0
>>  #define DEV_TYPE_NAND                1
>>  #define DEV_TYPE_ONENAND     2
>> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
>> index 96dcda2..2c60293 100644
>> --- a/drivers/mtd/mtdpart.c
>> +++ b/drivers/mtd/mtdpart.c
>> @@ -347,16 +347,18 @@ static struct mtd_part *add_one_partition(struct mtd_info *master,
>>               if (mtd_mod_by_eb(cur_offset, master) != 0) {
>>                       /* Round up to next erasesize */
>>                       slave->offset = (mtd_div_by_eb(cur_offset, master) + 1) * master->erasesize;
>> -                     printk(KERN_NOTICE "Moving partition %d: "
>> -                            "0x%012llx -> 0x%012llx\n", partno,
>> -                            (unsigned long long)cur_offset, (unsigned long long)slave->offset);
>> +                     debug("Moving partition %d: 0x%012llx -> 0x%012llx\n",
>> +                             partno, (unsigned long long)cur_offset,
>> +                             (unsigned long long)slave->offset);
>>               }
>>       }
>>       if (slave->mtd.size == MTDPART_SIZ_FULL)
>>               slave->mtd.size = master->size - slave->offset;
>>
>> -     printk(KERN_NOTICE "0x%012llx-0x%012llx : \"%s\"\n", (unsigned long long)slave->offset,
>> -             (unsigned long long)(slave->offset + slave->mtd.size), slave->mtd.name);
>> +     debug("0x%012llx-0x%012llx : \"%s\"\n",
>> +             (unsigned long long)slave->offset,
>> +             (unsigned long long)(slave->offset + slave->mtd.size),
>> +             slave->mtd.name);
>>
>>       /* let's do some sanity checks */
>>       if (slave->offset >= master->size) {
>> @@ -463,7 +465,7 @@ int add_mtd_partitions(struct mtd_info *master,
>>       if (mtd_partitions.next == NULL)
>>               INIT_LIST_HEAD(&mtd_partitions);
>>
>> -     printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
>> +     debug("Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
>
> Not so sure about this one. Other users might find this message quite
> useful. Does this output really interfere with your UBI env handling?

It makes it quite chatty.  I'll show an example below.

>>       for (i = 0; i < nbparts; i++) {
>>               slave = add_one_partition(master, parts + i, i, cur_offset);
>> diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
>> index 14c3a5f..f4ed7d8 100644
>> --- a/drivers/mtd/ubi/ubi.h
>> +++ b/drivers/mtd/ubi/ubi.h
>> @@ -59,7 +59,8 @@
>>  #define UBI_NAME_STR "ubi"
>>
>>  /* Normal UBI messages */
>> -#define ubi_msg(fmt, ...) printk(KERN_NOTICE "UBI: " fmt "\n", ##__VA_ARGS__)
>> +#define ubi_msg(fmt, ...) /*printk(KERN_NOTICE "UBI: " fmt "\n", \
>> +                             ##__VA_ARGS__)*/
>
> Hmmm....

Yes... this is ugly... I'll clean it up.

>>  /* UBI warning messages */
>>  #define ubi_warn(fmt, ...) printk(KERN_WARNING "UBI warning: %s: " fmt "\n", \
>>                                 __func__, ##__VA_ARGS__)
>> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
>> index 0af471a..4ab1cbd 100644
>> --- a/fs/ubifs/ubifs.h
>> +++ b/fs/ubifs/ubifs.h
>> @@ -464,7 +464,7 @@ static inline ino_t parent_ino(struct dentry *dentry)
>>
>>  /* Normal UBIFS messages */
>>  #define ubifs_msg(fmt, ...) \
>> -             printk(KERN_NOTICE "UBIFS: " fmt "\n", ##__VA_ARGS__)
>> +             /*printk(KERN_NOTICE "UBIFS: " fmt "\n", ##__VA_ARGS__)*/
>
> ... Not sure again about this silencing. And these "removed" printk's
> are really ugly.

Agreed... I'll clean it up.

> Could you please give an example of UBI usage (message logs from "ubi
> create ..., ubi part ..., ubi read ...") without this patch (or complete
> patchset) and with it? So that we see the real difference?

Here is boot:

<snip>
NAND:   1024 MiB
MMC:   SDHCI: 0
UBI: attaching mtd1 to ubi0
UBI: physical eraseblock size:   131072 bytes (128 KiB)
UBI: logical eraseblock size:    129024 bytes
UBI: smallest flash I/O unit:    2048
UBI: sub-page size:              512
UBI: VID header offset:          512 (aligned 512)
UBI: data offset:                2048
UBI: attached mtd1 to ubi0
UBI: MTD device name:            "mtd=2"
UBI: MTD device size:            70 MiB
UBI: number of good PEBs:        560
UBI: number of bad PEBs:         0
UBI: max. allowed volumes:       128
UBI: wear-leveling threshold:    4096
UBI: number of internal volumes: 1
UBI: number of user volumes:     4
UBI: available PEBs:             0
UBI: total number of reserved PEBs: 560
UBI: number of PEBs reserved for bad PEB handling: 80
UBI: max/mean erase counter: 7/1
In:    serial
Out:   serial
Err:   serial
<snip>


And here is a "env save"


Saving Environment to UBI...
UBI: mtd1 is detached from ubi0
UBI: attaching mtd1 to ubi0
UBI: physical eraseblock size:   131072 bytes (128 KiB)
UBI: logical eraseblock size:    129024 bytes
UBI: smallest flash I/O unit:    2048
UBI: sub-page size:              512
UBI: VID header offset:          512 (aligned 512)
UBI: data offset:                2048
UBI: attached mtd1 to ubi0
UBI: MTD device name:            "mtd=2"
UBI: MTD device size:            70 MiB
UBI: number of good PEBs:        560
UBI: number of bad PEBs:         0
UBI: max. allowed volumes:       128
UBI: wear-leveling threshold:    4096
UBI: number of internal volumes: 1
UBI: number of user volumes:     4
UBI: available PEBs:             0
UBI: total number of reserved PEBs: 560
UBI: number of PEBs reserved for bad PEB handling: 80
UBI: max/mean erase counter: 5/1
Writing to UBI... done


It's pretty out of control.

With this patch everything that starts with "UBI:" is gone.  Only
errors remain (if any).

Cheers,
-Joe

[U-Boot] [PATCH 2/5] ubi: ubifs: Turn off verbose prints

[Stefan Roese]

Hi Joe,

On 20.03.2013 11:07, Joe Hershberger wrote:

<snip>

>>>  /* Normal UBI messages */
>>> -#define ubi_msg(fmt, ...) printk(KERN_NOTICE "UBI: " fmt "\n", ##__VA_ARGS__)
>>> +#define ubi_msg(fmt, ...) /*printk(KERN_NOTICE "UBI: " fmt "\n", \
>>> +                             ##__VA_ARGS__)*/
>>
>> Hmmm....
> 
> Yes... this is ugly... I'll clean it up.

Thanks.

>>>  /* UBI warning messages */
>>>  #define ubi_warn(fmt, ...) printk(KERN_WARNING "UBI warning: %s: " fmt "\n", \
>>>                                 __func__, ##__VA_ARGS__)
>>> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
>>> index 0af471a..4ab1cbd 100644
>>> --- a/fs/ubifs/ubifs.h
>>> +++ b/fs/ubifs/ubifs.h
>>> @@ -464,7 +464,7 @@ static inline ino_t parent_ino(struct dentry *dentry)
>>>
>>>  /* Normal UBIFS messages */
>>>  #define ubifs_msg(fmt, ...) \
>>> -             printk(KERN_NOTICE "UBIFS: " fmt "\n", ##__VA_ARGS__)
>>> +             /*printk(KERN_NOTICE "UBIFS: " fmt "\n", ##__VA_ARGS__)*/
>>
>> ... Not sure again about this silencing. And these "removed" printk's
>> are really ugly.
> 
> Agreed... I'll clean it up.
> 
>> Could you please give an example of UBI usage (message logs from "ubi
>> create ..., ubi part ..., ubi read ...") without this patch (or complete
>> patchset) and with it? So that we see the real difference?
> 
> Here is boot:
> 
> <snip>
> NAND:   1024 MiB
> MMC:   SDHCI: 0
> UBI: attaching mtd1 to ubi0
> UBI: physical eraseblock size:   131072 bytes (128 KiB)
> UBI: logical eraseblock size:    129024 bytes
> UBI: smallest flash I/O unit:    2048
> UBI: sub-page size:              512
> UBI: VID header offset:          512 (aligned 512)
> UBI: data offset:                2048
> UBI: attached mtd1 to ubi0
> UBI: MTD device name:            "mtd=2"
> UBI: MTD device size:            70 MiB
> UBI: number of good PEBs:        560
> UBI: number of bad PEBs:         0
> UBI: max. allowed volumes:       128
> UBI: wear-leveling threshold:    4096
> UBI: number of internal volumes: 1
> UBI: number of user volumes:     4
> UBI: available PEBs:             0
> UBI: total number of reserved PEBs: 560
> UBI: number of PEBs reserved for bad PEB handling: 80
> UBI: max/mean erase counter: 7/1
> In:    serial
> Out:   serial
> Err:   serial
> <snip>
> 
> 
> And here is a "env save"
> 
> 
> Saving Environment to UBI...
> UBI: mtd1 is detached from ubi0
> UBI: attaching mtd1 to ubi0
> UBI: physical eraseblock size:   131072 bytes (128 KiB)
> UBI: logical eraseblock size:    129024 bytes
> UBI: smallest flash I/O unit:    2048
> UBI: sub-page size:              512
> UBI: VID header offset:          512 (aligned 512)
> UBI: data offset:                2048
> UBI: attached mtd1 to ubi0
> UBI: MTD device name:            "mtd=2"
> UBI: MTD device size:            70 MiB
> UBI: number of good PEBs:        560
> UBI: number of bad PEBs:         0
> UBI: max. allowed volumes:       128
> UBI: wear-leveling threshold:    4096
> UBI: number of internal volumes: 1
> UBI: number of user volumes:     4
> UBI: available PEBs:             0
> UBI: total number of reserved PEBs: 560
> UBI: number of PEBs reserved for bad PEB handling: 80
> UBI: max/mean erase counter: 5/1
> Writing to UBI... done
> 
> 
> It's pretty out of control.
> 
> With this patch everything that starts with "UBI:" is gone.  Only
> errors remain (if any).

I see. This is definitely helpful for your use-case, env in UBI. But I
would like to keep the UBI printf's for all other use cases. Or at least
make it configurable.

How about adding a switch/define, to optionally disable the UBI output?
This seems to be the most flexible option to me.

Thanks,
Stefan

[U-Boot] [PATCH 2/5] ubi: ubifs: Turn off verbose prints

[Joe Hershberger]

Hi Stefan,

On Wed, Mar 20, 2013 at 5:14 AM, Stefan Roese <sr@denx.de> wrote:
<snip>
> I see. This is definitely helpful for your use-case, env in UBI. But I
> would like to keep the UBI printf's for all other use cases. Or at least
> make it configurable.

That makes sense.

> How about adding a switch/define, to optionally disable the UBI output?
> This seems to be the most flexible option to me.


Will do!

-Joe

[U-Boot] [PATCH 2/5] ubi: ubifs: Turn off verbose prints

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/20/2013 06:19 AM, Joe Hershberger wrote:
> Hi Stefan,
> 
> On Wed, Mar 20, 2013 at 5:14 AM, Stefan Roese <sr@denx.de> wrote: 
> <snip>
>> I see. This is definitely helpful for your use-case, env in UBI.
>> But I would like to keep the UBI printf's for all other use
>> cases. Or at least make it configurable.
> 
> That makes sense.
> 
>> How about adding a switch/define, to optionally disable the UBI
>> output? This seems to be the most flexible option to me.
> 
> 
> Will do!

Or a "verbose" flag for some of the ubi commands?  Or ubi info even?
Just thinking out loud here (and I need to throw a UBI/UBIFS onto
something), but all of those prints are handy for making sure you're
trying to mount things right, etc.  But yes, lots of prints are bad
for time and shouldn't be the default.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRSbV+AAoJENk4IS6UOR1WCOUQAKN741OE2IV+f8EcKyC9JaP9
CuN7ZovoOFaq1yxjnM4HhWBinpntN+PeEI7HEnEeX/y8elnXlpuSknJqJ6ZvpgHs
yRhnv2NGYCjQ9imVQ+99HNBJB14EDO/zKruw+0ztSmSVvIJ9j4aGFWXr28ot28je
zQsbIgEK2kyJ1nBl70L8iSQOw45pTt9Ho4IxP9pofMzyZsf7hxLN1iYvmJ1V59A7
11XMgAFKvn8Lh6k2Ao/R/BfsQBhrFO0I1KcRH3ZX8u1ki0B8GL95UAgt/9L38nXl
apRMRuwBRUQEb9Qyg3OlktF29D11z9IDhTu4dcTRKdCW4iOATtXWarOmgDuudwaQ
yRvZMM6e9OYtbZJbIbSTRpJra7vh1gL45baGHSx3o1rUdYmOpOPTZg7dw0O9kepf
oPzT5iDFY1rHOK6pv/ETPTjA6aeVPXoOaMFkDRq1Kifr4KmXry2Lrs2espbxvZ/W
4e3KoyBdPMqJnb4QbAKrkOekbr1iQjGBzThX08z0YJHW5KbxCwLed5dczKckHZx9
I+R7aCtx9bSTtCMBf7cL40owMdN7BOm3mWzFQqOPieFxxh0+N3af8UJyFTFZY9Bf
zS/c/IFgCllqLPlznWUzc0RMLuoie+QU7FGskwYO6ayvjJO1zwRMrQ1zVflFBg9x
Quq0moE72+fvfNi+7WZ6
=n3wP
-----END PGP SIGNATURE-----

[U-Boot] [PATCH 3/5] mtd: Make mtdparts work with pre-reloc env

[Joe Hershberger]

The env in UBI needs to look up the mtd partition as part of relocation,
which happens before relocation.  Make the mtdparts code capable of
working on the default env to start with.

The code tries to set values in the env as well, but again, the env
isn't there yet, so add a check to setenv to not allow sets before the
env is relocated.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
 common/cmd_mtdparts.c | 23 +++++++++++++++++++++--
 common/cmd_nvedit.c   |  4 ++++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/common/cmd_mtdparts.c b/common/cmd_mtdparts.c
index 06fc171..e72dc38 100644
--- a/common/cmd_mtdparts.c
+++ b/common/cmd_mtdparts.c
@@ -106,6 +106,8 @@
 #include <onenand_uboot.h>
 #endif
 
+DECLARE_GLOBAL_DATA_PTR;
+
 /* special size referring to all the remaining space in a partition */
 #define SIZE_REMAINING		0xFFFFFFFF
 
@@ -1539,6 +1541,7 @@ static int parse_mtdparts(const char *const mtdparts)
 	const char *p = mtdparts;
 	struct mtd_device *dev;
 	int err = 1;
+	char tmp_parts[MTDPARTS_MAXLEN];
 
 	debug("\n---parse_mtdparts---\nmtdparts = %s\n\n", p);
 
@@ -1549,7 +1552,12 @@ static int parse_mtdparts(const char *const mtdparts)
 	}
 
 	/* re-read 'mtdparts' variable, mtd_devices_init may be updating env */
-	p = getenv("mtdparts");
+	if (gd->flags & GD_FLG_ENV_READY) {
+		p = getenv("mtdparts");
+	} else {
+		p = tmp_parts;
+		getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN);
+	}
 
 	if (strncmp(p, "mtdparts=", 9) != 0) {
 		printf("mtdparts variable doesn't start with 'mtdparts='\n");
@@ -1707,6 +1715,7 @@ int mtdparts_init(void)
 	const char *current_partition;
 	int ids_changed;
 	char tmp_ep[PARTITION_MAXLEN];
+	char tmp_parts[MTDPARTS_MAXLEN];
 
 	debug("\n---mtdparts_init---\n");
 	if (!initialized) {
@@ -1720,7 +1729,17 @@ int mtdparts_init(void)
 
 	/* get variables */
 	ids = getenv("mtdids");
-	parts = getenv("mtdparts");
+	/*
+	 * The mtdparts variable tends to be long. If we need to access it
+	 * before the env is relocated, then we need to use our own stack
+	 * buffer.  gd->env_buf will be too small.
+	 */
+	if (gd->flags & GD_FLG_ENV_READY) {
+		parts = getenv("mtdparts");
+	} else {
+		parts = tmp_parts;
+		getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN);
+	}
 	current_partition = getenv("partition");
 
 	/* save it for later parsing, cannot rely on current partition pointer
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 7633f0c..c7c6440 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -273,6 +273,10 @@ int setenv(const char *varname, const char *varvalue)
 {
 	const char * const argv[4] = { "setenv", varname, varvalue, NULL };
 
+	/* before import into hashtable */
+	if (!(gd->flags & GD_FLG_ENV_READY))
+		return 1;
+
 	if (varvalue == NULL || varvalue[0] == '\0')
 		return _do_env_set(0, 2, (char * const *)argv);
 	else
-- 
1.7.11.5

[U-Boot] [PATCH 3/5] mtd: Make mtdparts work with pre-reloc env

[Stefan Roese]

On 02/08/2013 09:07 PM, Joe Hershberger wrote:
> The env in UBI needs to look up the mtd partition as part of relocation,
> which happens before relocation.  Make the mtdparts code capable of
> working on the default env to start with.
> 
> The code tries to set values in the env as well, but again, the env
> isn't there yet, so add a check to setenv to not allow sets before the
> env is relocated.
> 
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>

Looks good:

Acked-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH 4/5] env: Add support for UBI environment

[Joe Hershberger]

UBI is a better place for the environment on NAND devices because it
handles wear-leveling and bad blocks.

Gluebi is needed in Linux to access the env as an MTD partition.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
 README                |  15 ++++++++
 common/Makefile       |   1 +
 common/cmd_nvedit.c   |   3 +-
 common/env_ubi.c      | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/environment.h |  15 ++++++++
 tools/env/fw_env.c    |   3 +-
 6 files changed, 138 insertions(+), 2 deletions(-)
 create mode 100644 common/env_ubi.c

diff --git a/README b/README
index 2352e38..c46ca3a 100644
--- a/README
+++ b/README
@@ -3442,6 +3442,21 @@ but it can not erase, write this NOR flash by SRIO or PCIE interface.
 	environment. If redundant environment is used, it will be copied to
 	CONFIG_NAND_ENV_DST + CONFIG_ENV_SIZE.
 
+- CONFIG_ENV_IS_IN_UBI:
+
+	Define this if you have an UBI volume that you want to use for the
+	environment.  This has the benefit of wear-leveling the environment
+	accesses, which is important on NAND.
+
+	- CONFIG_ENV_UBI_PART:
+
+	  Define this to a string that is the mtd partition containing the UBI.
+
+	- CONFIG_ENV_UBI_VOLUME:
+
+	  Define this to the name of the volume that you want to store the
+	  environment in.
+
 - CONFIG_SYS_SPI_INIT_OFFSET
 
 	Defines offset to the initial SPI buffer area in DPRAM. The
diff --git a/common/Makefile b/common/Makefile
index 54fcc81..9ff59c5 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -62,6 +62,7 @@ COBJS-$(CONFIG_ENV_IS_IN_NVRAM) += env_nvram.o
 COBJS-$(CONFIG_ENV_IS_IN_ONENAND) += env_onenand.o
 COBJS-$(CONFIG_ENV_IS_IN_SPI_FLASH) += env_sf.o
 COBJS-$(CONFIG_ENV_IS_IN_REMOTE) += env_remote.o
+COBJS-$(CONFIG_ENV_IS_IN_UBI) += env_ubi.o
 COBJS-$(CONFIG_ENV_IS_NOWHERE) += env_nowhere.o
 
 # command
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index c7c6440..960b22c 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -62,9 +62,10 @@ DECLARE_GLOBAL_DATA_PTR;
 	!defined(CONFIG_ENV_IS_IN_ONENAND)	&& \
 	!defined(CONFIG_ENV_IS_IN_SPI_FLASH)	&& \
 	!defined(CONFIG_ENV_IS_IN_REMOTE)	&& \
+	!defined(CONFIG_ENV_IS_IN_UBI)		&& \
 	!defined(CONFIG_ENV_IS_NOWHERE)
 # error Define one of CONFIG_ENV_IS_IN_{EEPROM|FLASH|DATAFLASH|ONENAND|\
-SPI_FLASH|NVRAM|MMC|FAT|REMOTE} or CONFIG_ENV_IS_NOWHERE
+SPI_FLASH|NVRAM|MMC|FAT|REMOTE|UBI} or CONFIG_ENV_IS_NOWHERE
 #endif
 
 /*
diff --git a/common/env_ubi.c b/common/env_ubi.c
new file mode 100644
index 0000000..9a47fd2
--- /dev/null
+++ b/common/env_ubi.c
@@ -0,0 +1,103 @@
+/*
+ * (c) Copyright 2012 by National Instruments,
+ *        Joe Hershberger <joe.hershberger@ni.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+
+#include <command.h>
+#include <environment.h>
+#include <errno.h>
+#include <malloc.h>
+#include <search.h>
+#include <ubi_uboot.h>
+#undef crc32
+
+char *env_name_spec = "UBI";
+
+env_t *env_ptr;
+
+DECLARE_GLOBAL_DATA_PTR;
+
+int env_init(void)
+{
+	/* use default */
+	gd->env_addr = (ulong)&default_environment[0];
+	gd->env_valid = 1;
+
+	return 0;
+}
+
+#ifdef CONFIG_CMD_SAVEENV
+int saveenv(void)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(env_t, env_new, 1);
+	ssize_t	len;
+	char *res;
+
+	res = (char *)&env_new->data;
+	len = hexport_r(&env_htab, '\0', 0, &res, ENV_SIZE, 0, NULL);
+	if (len < 0) {
+		error("Cannot export environment: errno = %d\n", errno);
+		return 1;
+	}
+
+	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
+		printf("\n** Cannot find mtd partition \"%s\"\n",
+			CONFIG_ENV_UBI_PART);
+		return 1;
+	}
+
+	env_new->crc = crc32(0, env_new->data, ENV_SIZE);
+
+	if (ubi_volume_write(CONFIG_ENV_UBI_VOLUME, (void *)env_new,
+	    CONFIG_ENV_SIZE)) {
+		printf("\n** Unable to write env to %s:%s **\n",
+			CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME);
+		return 1;
+	}
+
+	puts("done\n");
+	return 0;
+}
+#endif /* CONFIG_CMD_SAVEENV */
+
+void env_relocate_spec(void)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE);
+
+	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
+		printf("\n** Cannot find mtd partition \"%s\"\n",
+			CONFIG_ENV_UBI_PART);
+		set_default_env(NULL);
+		return;
+	}
+
+	if (ubi_volume_read(CONFIG_ENV_UBI_VOLUME, (void *)&buf,
+	    CONFIG_ENV_SIZE)) {
+		printf("\n** Unable to read env from %s:%s **\n",
+			CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME);
+		set_default_env(NULL);
+		return;
+	}
+
+	env_import(buf, 1);
+}
diff --git a/include/environment.h b/include/environment.h
index e64b43d..ece073f 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -96,6 +96,21 @@ extern unsigned long nand_env_oob_offset;
 # endif
 #endif /* CONFIG_ENV_IS_IN_NAND */
 
+#if defined(CONFIG_ENV_IS_IN_UBI)
+# ifndef CONFIG_ENV_UBI_PART
+#  error "Need to define CONFIG_ENV_UBI_PART when using CONFIG_ENV_IS_IN_UBI"
+# endif
+# ifndef CONFIG_ENV_UBI_VOLUME
+#  error "Need to define CONFIG_ENV_UBI_VOLUME when using CONFIG_ENV_IS_IN_UBI"
+# endif
+# ifndef CONFIG_ENV_SIZE
+#  error "Need to define CONFIG_ENV_SIZE when using CONFIG_ENV_IS_IN_UBI"
+# endif
+# ifndef CONFIG_CMD_UBI
+#  error "Need to define CONFIG_CMD_UBI when using CONFIG_ENV_IS_IN_UBI"
+# endif
+#endif /* CONFIG_ENV_IS_IN_UBI */
+
 /* Embedded env is only supported for some flash types */
 #ifdef CONFIG_ENV_IS_EMBEDDED
 # if	!defined(CONFIG_ENV_IS_IN_FLASH)	&& \
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 37b60b8..88e184f 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -959,7 +959,8 @@ static int flash_read (int fd)
 
 	if (mtdinfo.type != MTD_NORFLASH &&
 	    mtdinfo.type != MTD_NANDFLASH &&
-	    mtdinfo.type != MTD_DATAFLASH) {
+	    mtdinfo.type != MTD_DATAFLASH &&
+	    mtdinfo.type != MTD_UBIVOLUME) {
 		fprintf (stderr, "Unsupported flash type %u\n", mtdinfo.type);
 		return -1;
 	}
-- 
1.7.11.5

[U-Boot] [PATCH 4/5] env: Add support for UBI environment

[Stefan Roese]

On 02/08/2013 09:07 PM, Joe Hershberger wrote:
> UBI is a better place for the environment on NAND devices because it
> handles wear-leveling and bad blocks.
> 
> Gluebi is needed in Linux to access the env as an MTD partition.
> 
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>

Looks good:

Acked-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH 5/5] env: Add redundant env support to UBI env

[Joe Hershberger]

Allow the user to specify two UBI volumes to use for the environment

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
 README                |   6 +++
 common/env_ubi.c      | 115 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/environment.h |   3 ++
 tools/env/fw_env.c    |   3 ++
 4 files changed, 127 insertions(+)

diff --git a/README b/README
index c46ca3a..baf828a 100644
--- a/README
+++ b/README
@@ -3457,6 +3457,12 @@ but it can not erase, write this NOR flash by SRIO or PCIE interface.
 	  Define this to the name of the volume that you want to store the
 	  environment in.
 
+	- CONFIG_ENV_UBI_VOLUME_REDUND:
+
+	  Define this to the name of another volume to store a second copy of
+	  the environment in.  This will enable redundant environments in UBI.
+	  It is assumed that both volumes are in the same MTD partition.
+
 - CONFIG_SYS_SPI_INIT_OFFSET
 
 	Defines offset to the initial SPI buffer area in DPRAM. The
diff --git a/common/env_ubi.c b/common/env_ubi.c
index 9a47fd2..48f0bcb 100644
--- a/common/env_ubi.c
+++ b/common/env_ubi.c
@@ -47,6 +47,58 @@ int env_init(void)
 }
 
 #ifdef CONFIG_CMD_SAVEENV
+#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
+static unsigned char env_flags;
+
+int saveenv(void)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(env_t, env_new, 1);
+	ssize_t	len;
+	char *res;
+
+	res = (char *)&env_new->data;
+	len = hexport_r(&env_htab, '\0', 0, &res, ENV_SIZE, 0, NULL);
+	if (len < 0) {
+		error("Cannot export environment: errno = %d\n", errno);
+		return 1;
+	}
+
+	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
+		printf("\n** Cannot find mtd partition \"%s\"\n",
+			CONFIG_ENV_UBI_PART);
+		return 1;
+	}
+
+	env_new->crc = crc32(0, env_new->data, ENV_SIZE);
+	env_new->flags = ++env_flags; /* increase the serial */
+
+	if (gd->env_valid == 1) {
+		puts("Writing to redundant UBI... ");
+		if (ubi_volume_write(CONFIG_ENV_UBI_VOLUME_REDUND,
+		    (void *)env_new, CONFIG_ENV_SIZE)) {
+			printf("\n** Unable to write env to %s:%s **\n",
+				CONFIG_ENV_UBI_PART,
+				CONFIG_ENV_UBI_VOLUME_REDUND);
+			return 1;
+		}
+	} else {
+		puts("Writing to UBI... ");
+		if (ubi_volume_write(CONFIG_ENV_UBI_VOLUME,
+		    (void *)env_new, CONFIG_ENV_SIZE)) {
+			printf("\n** Unable to write env to %s:%s **\n",
+				CONFIG_ENV_UBI_PART,
+				CONFIG_ENV_UBI_VOLUME);
+			return 1;
+		}
+	}
+
+	puts("done\n");
+
+	gd->env_valid = gd->env_valid == 2 ? 1 : 2;
+
+	return 0;
+}
+#else /* ! CONFIG_SYS_REDUNDAND_ENVIRONMENT */
 int saveenv(void)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(env_t, env_new, 1);
@@ -78,8 +130,70 @@ int saveenv(void)
 	puts("done\n");
 	return 0;
 }
+#endif /* CONFIG_SYS_REDUNDAND_ENVIRONMENT */
 #endif /* CONFIG_CMD_SAVEENV */
 
+#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
+void env_relocate_spec(void)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(char, env1_buf, CONFIG_ENV_SIZE);
+	ALLOC_CACHE_ALIGN_BUFFER(char, env2_buf, CONFIG_ENV_SIZE);
+	int crc1_ok = 0, crc2_ok = 0;
+	env_t *ep, *tmp_env1, *tmp_env2;
+
+	tmp_env1 = (env_t *)env1_buf;
+	tmp_env2 = (env_t *)env2_buf;
+
+	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
+		printf("\n** Cannot find mtd partition \"%s\"\n",
+			CONFIG_ENV_UBI_PART);
+		set_default_env(NULL);
+		return;
+	}
+
+	if (ubi_volume_read(CONFIG_ENV_UBI_VOLUME, (void *)tmp_env1,
+	    CONFIG_ENV_SIZE))
+		printf("\n** Unable to read env from %s:%s **\n",
+			CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME);
+
+	if (ubi_volume_read(CONFIG_ENV_UBI_VOLUME_REDUND, (void *)tmp_env2,
+	    CONFIG_ENV_SIZE))
+		printf("\n** Unable to read redundant env from %s:%s **\n",
+			CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME_REDUND);
+
+	crc1_ok = crc32(0, tmp_env1->data, ENV_SIZE) == tmp_env1->crc;
+	crc2_ok = crc32(0, tmp_env2->data, ENV_SIZE) == tmp_env2->crc;
+
+	if (!crc1_ok && !crc2_ok) {
+		set_default_env("!bad CRC");
+		return;
+	} else if (crc1_ok && !crc2_ok) {
+		gd->env_valid = 1;
+	} else if (!crc1_ok && crc2_ok) {
+		gd->env_valid = 2;
+	} else {
+		/* both ok - check serial */
+		if (tmp_env1->flags == 255 && tmp_env2->flags == 0)
+			gd->env_valid = 2;
+		else if (tmp_env2->flags == 255 && tmp_env1->flags == 0)
+			gd->env_valid = 1;
+		else if (tmp_env1->flags > tmp_env2->flags)
+			gd->env_valid = 1;
+		else if (tmp_env2->flags > tmp_env1->flags)
+			gd->env_valid = 2;
+		else /* flags are equal - almost impossible */
+			gd->env_valid = 1;
+	}
+
+	if (gd->env_valid == 1)
+		ep = tmp_env1;
+	else
+		ep = tmp_env2;
+
+	env_flags = ep->flags;
+	env_import((char *)ep, 0);
+}
+#else /* ! CONFIG_SYS_REDUNDAND_ENVIRONMENT */
 void env_relocate_spec(void)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE);
@@ -101,3 +215,4 @@ void env_relocate_spec(void)
 
 	env_import(buf, 1);
 }
+#endif /* CONFIG_SYS_REDUNDAND_ENVIRONMENT */
diff --git a/include/environment.h b/include/environment.h
index ece073f..4c6a37b 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -103,6 +103,9 @@ extern unsigned long nand_env_oob_offset;
 # ifndef CONFIG_ENV_UBI_VOLUME
 #  error "Need to define CONFIG_ENV_UBI_VOLUME when using CONFIG_ENV_IS_IN_UBI"
 # endif
+# if defined(CONFIG_ENV_UBI_VOLUME_REDUND)
+#  define CONFIG_SYS_REDUNDAND_ENVIRONMENT
+# endif
 # ifndef CONFIG_ENV_SIZE
 #  error "Need to define CONFIG_ENV_SIZE when using CONFIG_ENV_IS_IN_UBI"
 # endif
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 88e184f..7654b1e 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -1135,6 +1135,9 @@ int fw_env_open(void)
 		} else if (DEVTYPE(dev_current) == MTD_DATAFLASH &&
 			   DEVTYPE(!dev_current) == MTD_DATAFLASH) {
 			environment.flag_scheme = FLAG_BOOLEAN;
+		} else if (DEVTYPE(dev_current) == MTD_UBIVOLUME &&
+			   DEVTYPE(!dev_current) == MTD_UBIVOLUME) {
+			environment.flag_scheme = FLAG_INCREMENTAL;
 		} else {
 			fprintf (stderr, "Incompatible flash types!\n");
 			return -1;
-- 
1.7.11.5

[U-Boot] [PATCH 5/5] env: Add redundant env support to UBI env

[Stefan Roese]

On 02/08/2013 09:07 PM, Joe Hershberger wrote:
> Allow the user to specify two UBI volumes to use for the environment
> 
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>

Some minor comments below.

> ---
>  README                |   6 +++
>  common/env_ubi.c      | 115 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/environment.h |   3 ++
>  tools/env/fw_env.c    |   3 ++
>  4 files changed, 127 insertions(+)
> 
> diff --git a/README b/README
> index c46ca3a..baf828a 100644
> --- a/README
> +++ b/README
> @@ -3457,6 +3457,12 @@ but it can not erase, write this NOR flash by SRIO or PCIE interface.
>  	  Define this to the name of the volume that you want to store the
>  	  environment in.
>  
> +	- CONFIG_ENV_UBI_VOLUME_REDUND:
> +
> +	  Define this to the name of another volume to store a second copy of
> +	  the environment in.  This will enable redundant environments in UBI.
> +	  It is assumed that both volumes are in the same MTD partition.
> +
>  - CONFIG_SYS_SPI_INIT_OFFSET
>  
>  	Defines offset to the initial SPI buffer area in DPRAM. The
> diff --git a/common/env_ubi.c b/common/env_ubi.c
> index 9a47fd2..48f0bcb 100644
> --- a/common/env_ubi.c
> +++ b/common/env_ubi.c
> @@ -47,6 +47,58 @@ int env_init(void)
>  }
>  
>  #ifdef CONFIG_CMD_SAVEENV
> +#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
> +static unsigned char env_flags;
> +
> +int saveenv(void)
> +{
> +	ALLOC_CACHE_ALIGN_BUFFER(env_t, env_new, 1);
> +	ssize_t	len;
> +	char *res;
> +
> +	res = (char *)&env_new->data;
> +	len = hexport_r(&env_htab, '\0', 0, &res, ENV_SIZE, 0, NULL);
> +	if (len < 0) {
> +		error("Cannot export environment: errno = %d\n", errno);
> +		return 1;
> +	}
> +
> +	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
> +		printf("\n** Cannot find mtd partition \"%s\"\n",
> +			CONFIG_ENV_UBI_PART);
> +		return 1;
> +	}
> +
> +	env_new->crc = crc32(0, env_new->data, ENV_SIZE);
> +	env_new->flags = ++env_flags; /* increase the serial */
> +
> +	if (gd->env_valid == 1) {
> +		puts("Writing to redundant UBI... ");
> +		if (ubi_volume_write(CONFIG_ENV_UBI_VOLUME_REDUND,
> +		    (void *)env_new, CONFIG_ENV_SIZE)) {
> +			printf("\n** Unable to write env to %s:%s **\n",
> +				CONFIG_ENV_UBI_PART,
> +				CONFIG_ENV_UBI_VOLUME_REDUND);
> +			return 1;
> +		}
> +	} else {
> +		puts("Writing to UBI... ");
> +		if (ubi_volume_write(CONFIG_ENV_UBI_VOLUME,
> +		    (void *)env_new, CONFIG_ENV_SIZE)) {
> +			printf("\n** Unable to write env to %s:%s **\n",
> +				CONFIG_ENV_UBI_PART,
> +				CONFIG_ENV_UBI_VOLUME);
> +			return 1;
> +		}
> +	}
> +
> +	puts("done\n");
> +
> +	gd->env_valid = gd->env_valid == 2 ? 1 : 2;
> +
> +	return 0;
> +}
> +#else /* ! CONFIG_SYS_REDUNDAND_ENVIRONMENT */
>  int saveenv(void)
>  {
>  	ALLOC_CACHE_ALIGN_BUFFER(env_t, env_new, 1);
> @@ -78,8 +130,70 @@ int saveenv(void)
>  	puts("done\n");
>  	return 0;
>  }
> +#endif /* CONFIG_SYS_REDUNDAND_ENVIRONMENT */
>  #endif /* CONFIG_CMD_SAVEENV */
>  
> +#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
> +void env_relocate_spec(void)
> +{
> +	ALLOC_CACHE_ALIGN_BUFFER(char, env1_buf, CONFIG_ENV_SIZE);
> +	ALLOC_CACHE_ALIGN_BUFFER(char, env2_buf, CONFIG_ENV_SIZE);
> +	int crc1_ok = 0, crc2_ok = 0;
> +	env_t *ep, *tmp_env1, *tmp_env2;
> +
> +	tmp_env1 = (env_t *)env1_buf;
> +	tmp_env2 = (env_t *)env2_buf;
> +
> +	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
> +		printf("\n** Cannot find mtd partition \"%s\"\n",
> +			CONFIG_ENV_UBI_PART);
> +		set_default_env(NULL);
> +		return;
> +	}
> +
> +	if (ubi_volume_read(CONFIG_ENV_UBI_VOLUME, (void *)tmp_env1,
> +	    CONFIG_ENV_SIZE))
> +		printf("\n** Unable to read env from %s:%s **\n",
> +			CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME);

Parentheses for multi-line statements.

> +
> +	if (ubi_volume_read(CONFIG_ENV_UBI_VOLUME_REDUND, (void *)tmp_env2,
> +	    CONFIG_ENV_SIZE))
> +		printf("\n** Unable to read redundant env from %s:%s **\n",
> +			CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME_REDUND);

Here too.

> +
> +	crc1_ok = crc32(0, tmp_env1->data, ENV_SIZE) == tmp_env1->crc;
> +	crc2_ok = crc32(0, tmp_env2->data, ENV_SIZE) == tmp_env2->crc;
> +
> +	if (!crc1_ok && !crc2_ok) {
> +		set_default_env("!bad CRC");
> +		return;
> +	} else if (crc1_ok && !crc2_ok) {
> +		gd->env_valid = 1;
> +	} else if (!crc1_ok && crc2_ok) {
> +		gd->env_valid = 2;
> +	} else {
> +		/* both ok - check serial */
> +		if (tmp_env1->flags == 255 && tmp_env2->flags == 0)
> +			gd->env_valid = 2;
> +		else if (tmp_env2->flags == 255 && tmp_env1->flags == 0)
> +			gd->env_valid = 1;
> +		else if (tmp_env1->flags > tmp_env2->flags)
> +			gd->env_valid = 1;
> +		else if (tmp_env2->flags > tmp_env1->flags)
> +			gd->env_valid = 2;
> +		else /* flags are equal - almost impossible */
> +			gd->env_valid = 1;
> +	}
> +
> +	if (gd->env_valid == 1)
> +		ep = tmp_env1;
> +	else
> +		ep = tmp_env2;
> +
> +	env_flags = ep->flags;
> +	env_import((char *)ep, 0);

I might be wrong, but these lines above are most likely copied from
other env handling source files, correct? It would be great to extract
this code into some common file then. What do you think?


Other that that:

Acked-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH 0/5] Add support for using an UBI volume for environment

[Stefan Roese]

Hi Joe,

On 02/08/2013 09:07 PM, Joe Hershberger wrote:
> NAND is not good at handling absolute addresses to sectors for storing
> particular data.  The current implementation of the NAND env support
> works around this in several ways such as storing a pointer to the
> sector in the OOB of the first sector (interferes with some CRC) or
> supporting a range of sectors (which unless it is huge is not
> guaranteed to be safe).  None of these options address wear-leveling
> concerns or bad block handling.
> 
> Accessing the u-boot env from UBI eliminates these concerns.  However,
> it does require some of the basic settings for finding the UBI env to
> be in the default u-boot env.

Great. I'll review the patches in a bit and will send some comments.

Thanks,
Stefan

[U-Boot] [PATCH 0/5] Add support for using an UBI volume for environment

[Scott Wood]

On 02/08/2013 02:07:21 PM, Joe Hershberger wrote:
> NAND is not good at handling absolute addresses to sectors for storing
> particular data.  The current implementation of the NAND env support
> works around this in several ways such as storing a pointer to the
> sector in the OOB of the first sector (interferes with some CRC) or
> supporting a range of sectors (which unless it is huge is not
> guaranteed to be safe).  None of these options address wear-leveling
> concerns or bad block handling.
> 
> Accessing the u-boot env from UBI eliminates these concerns.  However,
> it does require some of the basic settings for finding the UBI env to
> be in the default u-boot env.

The downside is this moves us further away from having an environment  
available before relocation (e.g. loaded by SPL), which is important  
not just for serial config but also hwconfig, which can affect how RAM  
is set up among other things.

Maybe the "OOB of first sector" approach could be changed to be more  
like how bad block tables are allocated, with a special marker in the  
env block's own OOB that we scan for.

-Scott

[U-Boot] [PATCH 0/5] Add support for using an UBI volume for environment

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/11/2013 09:37 PM, Scott Wood wrote:
> On 02/08/2013 02:07:21 PM, Joe Hershberger wrote:
>> NAND is not good at handling absolute addresses to sectors for 
>> storing particular data.  The current implementation of the NAND 
>> env support works around this in several ways such as storing a 
>> pointer to the sector in the OOB of the first sector (interferes 
>> with some CRC) or supporting a range of sectors (which unless it 
>> is huge is not guaranteed to be safe).  None of these options 
>> address wear-leveling concerns or bad block handling.
>> 
>> Accessing the u-boot env from UBI eliminates these concerns. 
>> However, it does require some of the basic settings for finding 
>> the UBI env to be in the default u-boot env.
> 
> The downside is this moves us further away from having an 
> environment available before relocation (e.g. loaded by SPL),
> which is important not just for serial config but also hwconfig,
> which can affect how RAM is set up among other things.
> 
> Maybe the "OOB of first sector" approach could be changed to be 
> more like how bad block tables are allocated, with a special
> marker in the env block's own OOB that we scan for.

There's pluses and minuses to "throw more stuff in UBI".  So long as
it doesn't break the ability to have env pre-relocation (and it
shouldn't, we already support env in a file), is there a problem here?

Or just hoping to encourage other robust methods?

I think somewhere there's a wishlist for UBI in SPL (for Falcon mode),
for example..

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRGmf4AAoJENk4IS6UOR1W4IUP/0bcsyrI+ufUPf99ME/bkQq0
NhCP3/DH/CxtXrTzsl4mkPOv/C0ziMmQiGpz58k63ITqSPJL/pAZgVqmy1DwhIB/
u/y2F3IYZTwllw06OykesKd8GlP4+oiQokYDAHywfOzw/SyUI3t4jZCGTR2BOvbt
TWw6XL8Rm7sGi3nWRhFlQt4AfRvdidlzcOFppjrQsf7wS39Ho6ALRs0kSh/pVy7h
MoQcICC/+vRc/qDXV6QKlQ3EHK3+3+/3HKUQiKbsw+hrPJua6Ur8+6PpLS96Qc8F
r7+hnOHS+pPVuoIzOfRBn8+R3LDccPskirFGlT0uHXMpph/nQBohLXFMt0CMe9/9
8CwXyMSckpZdPM+2EeGURqQdJZ/fBL+WJEmke6BUnLdOdw8Ks2PuIGQWonlrsLkq
Q7ZHyqN2RtC9veGadRvKhR8GOZP7bQPLMiIWpThNQQTaSUKKYoUFjlxgcaiLAsoC
grO2S0AIBTQx3D0Vzfrka1shttYBd5vqs85bRt/EZAi5YDMeeSTkc3D/wiZST3bP
q8GNyM6p7QNXMnM0hNgQRZIhw5iStaMohh517h84JN69RkQ4TwRUt8HdJPXnPPHq
F1+gmWLKeJRSSza9uIWsUlU4VTE1vVLo7H3hIq1EigERLdg3Z3g9WirkPuSMYEjX
XuvGODaEQ4wqM6yH1XFQ
=rkT9
-----END PGP SIGNATURE-----

[U-Boot] [PATCH 0/5] Add support for using an UBI volume for environment

[Scott Wood]

On 02/12/2013 10:04:09 AM, Tom Rini wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 02/11/2013 09:37 PM, Scott Wood wrote:
> > On 02/08/2013 02:07:21 PM, Joe Hershberger wrote:
> >> NAND is not good at handling absolute addresses to sectors for
> >> storing particular data.  The current implementation of the NAND
> >> env support works around this in several ways such as storing a
> >> pointer to the sector in the OOB of the first sector (interferes
> >> with some CRC) or supporting a range of sectors (which unless it
> >> is huge is not guaranteed to be safe).  None of these options
> >> address wear-leveling concerns or bad block handling.
> >>
> >> Accessing the u-boot env from UBI eliminates these concerns.
> >> However, it does require some of the basic settings for finding
> >> the UBI env to be in the default u-boot env.
> >
> > The downside is this moves us further away from having an
> > environment available before relocation (e.g. loaded by SPL),
> > which is important not just for serial config but also hwconfig,
> > which can affect how RAM is set up among other things.
> >
> > Maybe the "OOB of first sector" approach could be changed to be
> > more like how bad block tables are allocated, with a special
> > marker in the env block's own OOB that we scan for.
> 
> There's pluses and minuses to "throw more stuff in UBI".  So long as
> it doesn't break the ability to have env pre-relocation (and it
> shouldn't, we already support env in a file), is there a problem here?
> 
> Or just hoping to encourage other robust methods?

The latter.

-Scott

[U-Boot] [PATCH 0/5] Add support for using an UBI volume for environment

[Tom Rini]

On Fri, Feb 08, 2013 at 02:07:21PM -0600, Joe Hershberger wrote:

> NAND is not good at handling absolute addresses to sectors for storing
> particular data.  The current implementation of the NAND env support
> works around this in several ways such as storing a pointer to the
> sector in the OOB of the first sector (interferes with some CRC) or
> supporting a range of sectors (which unless it is huge is not
> guaranteed to be safe).  None of these options address wear-leveling
> concerns or bad block handling.
> 
> Accessing the u-boot env from UBI eliminates these concerns.  However,
> it does require some of the basic settings for finding the UBI env to
> be in the default u-boot env.
> 
> 
> Joe Hershberger (5):
>   ubi: Expose a few simple functions from the cmd_ubi
>   ubi: ubifs: Turn off verbose prints
>   mtd: Make mtdparts work with pre-reloc env
>   env: Add support for UBI environment
>   env: Add redundant env support to UBI env
> 
>  README                |  21 +++++
>  common/Makefile       |   1 +
>  common/cmd_mtdparts.c |  23 +++++-
>  common/cmd_nvedit.c   |   7 +-
>  common/cmd_ubi.c      | 149 +++++++++++++++++++---------------
>  common/env_ubi.c      | 218 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/mtd/mtdpart.c |  14 ++--
>  drivers/mtd/ubi/ubi.h |   3 +-
>  fs/ubifs/ubifs.h      |   2 +-
>  include/environment.h |  18 +++++
>  include/ubi_uboot.h   |   3 +
>  tools/env/fw_env.c    |   6 +-
>  12 files changed, 387 insertions(+), 78 deletions(-)
>  create mode 100644 common/env_ubi.c

Please add a patch 6 which enables all of these options for some board,
preferably the one you've been testing this with.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130218/70b040ab/attachment.pgp>

[U-Boot] [PATCH v2 0/6] Add support for using an UBI volume for environment

[Joe Hershberger]

NAND is not good at handling absolute addresses to sectors for storing
particular data.  The current implementation of the NAND env support
works around this in several ways such as storing a pointer to the
sector in the OOB of the first sector (interferes with some CRC) or
supporting a range of sectors (which unless it is huge is not
guaranteed to be safe).  None of these options address wear-leveling
concerns or bad block handling.

Accessing the u-boot env from UBI eliminates these concerns.  However,
it does require some of the basic settings for finding the UBI env to
be in the default u-boot env.

Changes in v2:
- Fixed error handling bug that prevents fail-over to default env on error
- Added curly braces on "multi-line" statements
- Added extern consistently in header
- Cleaned up the msg print silencing
- Added curly braces on "multi-line" statements

Joe Hershberger (6):
  ubi: Fix broken cleanup code in attach_by_scanning
  ubi: Expose a few simple functions from the cmd_ubi
  ubi: ubifs: Turn off verbose prints
  mtd: Make mtdparts work with pre-reloc env
  env: Add support for UBI environment
  env: Add redundant env support to UBI env

 README                  |  21 +++++
 common/Makefile         |   1 +
 common/cmd_mtdparts.c   |  23 ++++-
 common/cmd_nvedit.c     |   7 +-
 common/cmd_ubi.c        | 153 ++++++++++++++++++---------------
 common/env_ubi.c        | 220 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/mtdpart.c   |  14 +--
 drivers/mtd/ubi/build.c |   8 +-
 drivers/mtd/ubi/ubi.h   |   4 +
 drivers/mtd/ubi/wl.c    |   1 +
 fs/ubifs/ubifs.h        |   4 +
 include/environment.h   |  18 ++++
 include/ubi_uboot.h     |   3 +
 tools/env/fw_env.c      |   6 +-
 14 files changed, 402 insertions(+), 81 deletions(-)
 create mode 100644 common/env_ubi.c

-- 
1.7.11.5

[U-Boot] [PATCH v2 1/6] ubi: Fix broken cleanup code in attach_by_scanning

[Joe Hershberger]

The unwind code was not reversing operations correctly and was causing
a hang on any error condition.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
Changes in v2:
- Fixed error handling bug that prevents fail-over to default env on error

 drivers/mtd/ubi/build.c | 8 ++++----
 drivers/mtd/ubi/wl.c    | 1 +
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index d144ac2..a708162 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -478,19 +478,19 @@ static int attach_by_scanning(struct ubi_device *ubi)
 
 	err = ubi_eba_init_scan(ubi, si);
 	if (err)
-		goto out_wl;
+		goto out_vtbl;
 
 	err = ubi_wl_init_scan(ubi, si);
 	if (err)
-		goto out_vtbl;
+		goto out_eba;
 
 	ubi_scan_destroy_si(si);
 	return 0;
 
+out_eba:
+	ubi_eba_close(ubi);
 out_vtbl:
 	vfree(ubi->vtbl);
-out_wl:
-	ubi_wl_close(ubi);
 out_si:
 	ubi_scan_destroy_si(si);
 	return err;
diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
index 88b867a..d1ba722 100644
--- a/drivers/mtd/ubi/wl.c
+++ b/drivers/mtd/ubi/wl.c
@@ -1538,6 +1538,7 @@ int ubi_wl_init_scan(struct ubi_device *ubi, struct ubi_scan_info *si)
 	if (ubi->avail_pebs < WL_RESERVED_PEBS) {
 		ubi_err("no enough physical eraseblocks (%d, need %d)",
 			ubi->avail_pebs, WL_RESERVED_PEBS);
+		err = -ENOSPC;
 		goto out_free;
 	}
 	ubi->avail_pebs -= WL_RESERVED_PEBS;
-- 
1.7.11.5

[U-Boot] [PATCH v2 2/6] ubi: Expose a few simple functions from the cmd_ubi

[Joe Hershberger]

Part, Read, and Write functionality that will be used by env_ubi.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
Changes in v2:
- Added curly braces on "multi-line" statements
- Added extern consistently in header

 common/cmd_ubi.c    | 150 +++++++++++++++++++++++++++++-----------------------
 include/ubi_uboot.h |   3 ++
 2 files changed, 86 insertions(+), 67 deletions(-)

diff --git a/common/cmd_ubi.c b/common/cmd_ubi.c
index 35b1d31..7ca3bfa 100644
--- a/common/cmd_ubi.c
+++ b/common/cmd_ubi.c
@@ -263,7 +263,7 @@ out_err:
 	return err;
 }
 
-static int ubi_volume_write(char *volume, void *buf, size_t size)
+int ubi_volume_write(char *volume, void *buf, size_t size)
 {
 	int err = 1;
 	int rsvd_bytes = 0;
@@ -308,12 +308,10 @@ static int ubi_volume_write(char *volume, void *buf, size_t size)
 		ubi_gluebi_updated(vol);
 	}
 
-	printf("%d bytes written to volume %s\n", size, volume);
-
 	return 0;
 }
 
-static int ubi_volume_read(char *volume, char *buf, size_t size)
+int ubi_volume_read(char *volume, char *buf, size_t size)
 {
 	int err, lnum, off, len, tbuf_size;
 	void *tbuf;
@@ -325,8 +323,6 @@ static int ubi_volume_read(char *volume, char *buf, size_t size)
 	if (vol == NULL)
 		return ENODEV;
 
-	printf("Read %d bytes from volume %s to %p\n", size, volume, buf);
-
 	if (vol->updating) {
 		printf("updating");
 		return EBUSY;
@@ -431,26 +427,82 @@ static int ubi_dev_scan(struct mtd_info *info, char *ubidev,
 	return 0;
 }
 
-static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
+int ubi_part(char *part_name, const char *vid_header_offset)
 {
-	size_t size = 0;
-	ulong addr = 0;
 	int err = 0;
-
-	if (argc < 2)
-		return CMD_RET_USAGE;
+	char mtd_dev[16];
+	struct mtd_device *dev;
+	struct part_info *part;
+	u8 pnum;
 
 	if (mtdparts_init() != 0) {
 		printf("Error initializing mtdparts!\n");
 		return 1;
 	}
 
+#ifdef CONFIG_CMD_UBIFS
+	/*
+	 * Automatically unmount UBIFS partition when user
+	 * changes the UBI device. Otherwise the following
+	 * UBIFS commands will crash.
+	 */
+	if (ubifs_is_mounted())
+		cmd_ubifs_umount();
+#endif
+
+	/* todo: get dev number for NAND... */
+	ubi_dev.nr = 0;
+
+	/*
+	 * Call ubi_exit() before re-initializing the UBI subsystem
+	 */
+	if (ubi_initialized) {
+		ubi_exit();
+		del_mtd_partitions(ubi_dev.mtd_info);
+	}
+
+	/*
+	 * Search the mtd device number where this partition
+	 * is located
+	 */
+	if (find_dev_and_part(part_name, &dev, &pnum, &part)) {
+		printf("Partition %s not found!\n", part_name);
+		return 1;
+	}
+	sprintf(mtd_dev, "%s%d", MTD_DEV_TYPE(dev->id->type), dev->id->num);
+	ubi_dev.mtd_info = get_mtd_device_nm(mtd_dev);
+	if (IS_ERR(ubi_dev.mtd_info)) {
+		printf("Partition %s not found on device %s!\n", part_name,
+			mtd_dev);
+		return 1;
+	}
+
+	ubi_dev.selected = 1;
+
+	strcpy(ubi_dev.part_name, part_name);
+	err = ubi_dev_scan(ubi_dev.mtd_info, ubi_dev.part_name,
+			vid_header_offset);
+	if (err) {
+		printf("UBI init error %d\n", err);
+		ubi_dev.selected = 0;
+		return err;
+	}
+
+	ubi = ubi_devices[0];
+
+	return 0;
+}
+
+static int do_ubi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	size_t size = 0;
+	ulong addr = 0;
+
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
 	if (strcmp(argv[1], "part") == 0) {
-		char mtd_dev[16];
-		struct mtd_device *dev;
-		struct part_info *part;
 		const char *vid_header_offset = NULL;
-		u8 pnum;
 
 		/* Print current partition */
 		if (argc == 2) {
@@ -467,58 +519,10 @@ static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 		if (argc < 3)
 			return CMD_RET_USAGE;
 
-#ifdef CONFIG_CMD_UBIFS
-		/*
-		 * Automatically unmount UBIFS partition when user
-		 * changes the UBI device. Otherwise the following
-		 * UBIFS commands will crash.
-		 */
-		if (ubifs_is_mounted())
-			cmd_ubifs_umount();
-#endif
-
-		/* todo: get dev number for NAND... */
-		ubi_dev.nr = 0;
-
-		/*
-		 * Call ubi_exit() before re-initializing the UBI subsystem
-		 */
-		if (ubi_initialized) {
-			ubi_exit();
-			del_mtd_partitions(ubi_dev.mtd_info);
-		}
-
-		/*
-		 * Search the mtd device number where this partition
-		 * is located
-		 */
-		if (find_dev_and_part(argv[2], &dev, &pnum, &part)) {
-			printf("Partition %s not found!\n", argv[2]);
-			return 1;
-		}
-		sprintf(mtd_dev, "%s%d", MTD_DEV_TYPE(dev->id->type), dev->id->num);
-		ubi_dev.mtd_info = get_mtd_device_nm(mtd_dev);
-		if (IS_ERR(ubi_dev.mtd_info)) {
-			printf("Partition %s not found on device %s!\n", argv[2], mtd_dev);
-			return 1;
-		}
-
-		ubi_dev.selected = 1;
-
 		if (argc > 3)
 			vid_header_offset = argv[3];
-		strcpy(ubi_dev.part_name, argv[2]);
-		err = ubi_dev_scan(ubi_dev.mtd_info, ubi_dev.part_name,
-				vid_header_offset);
-		if (err) {
-			printf("UBI init error %d\n", err);
-			ubi_dev.selected = 0;
-			return err;
-		}
-
-		ubi = ubi_devices[0];
 
-		return 0;
+		return ubi_part(argv[2], vid_header_offset);
 	}
 
 	if ((strcmp(argv[1], "part") != 0) && (!ubi_dev.selected)) {
@@ -571,6 +575,8 @@ static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 	}
 
 	if (strncmp(argv[1], "write", 5) == 0) {
+		int ret;
+
 		if (argc < 5) {
 			printf("Please see usage\n");
 			return 1;
@@ -579,7 +585,13 @@ static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 		addr = simple_strtoul(argv[2], NULL, 16);
 		size = simple_strtoul(argv[4], NULL, 16);
 
-		return ubi_volume_write(argv[3], (void *)addr, size);
+		ret = ubi_volume_write(argv[3], (void *)addr, size);
+		if (!ret) {
+			printf("%d bytes written to volume %s\n", size,
+				argv[3]);
+		}
+
+		return ret;
 	}
 
 	if (strncmp(argv[1], "read", 4) == 0) {
@@ -597,8 +609,12 @@ static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 			argc--;
 		}
 
-		if (argc == 3)
+		if (argc == 3) {
+			printf("Read %d bytes from volume %s to %lx\n", size,
+				argv[3], addr);
+
 			return ubi_volume_read(argv[3], (char *)addr, size);
+		}
 	}
 
 	printf("Please see usage\n");
diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h
index 69006e2..7f72022 100644
--- a/include/ubi_uboot.h
+++ b/include/ubi_uboot.h
@@ -214,6 +214,9 @@ static inline long IS_ERR(const void *ptr)
 extern int ubi_mtd_param_parse(const char *val, struct kernel_param *kp);
 extern int ubi_init(void);
 extern void ubi_exit(void);
+extern int ubi_part(char *part_name, const char *vid_header_offset);
+extern int ubi_volume_write(char *volume, void *buf, size_t size);
+extern int ubi_volume_read(char *volume, char *buf, size_t size);
 
 extern struct ubi_device *ubi_devices[];
 
-- 
1.7.11.5

[U-Boot] [PATCH v2 3/6] ubi: ubifs: Turn off verbose prints

[Joe Hershberger]

The prints are out of control.  SILENCE!

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
Changes in v2:
- Cleaned up the msg print silencing

 common/cmd_ubi.c      |  3 +++
 drivers/mtd/mtdpart.c | 14 ++++++++------
 drivers/mtd/ubi/ubi.h |  4 ++++
 fs/ubifs/ubifs.h      |  4 ++++
 4 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/common/cmd_ubi.c b/common/cmd_ubi.c
index 7ca3bfa..70a8019 100644
--- a/common/cmd_ubi.c
+++ b/common/cmd_ubi.c
@@ -23,6 +23,9 @@
 #include <asm/errno.h>
 #include <jffs2/load_kernel.h>
 
+#undef ubi_msg
+#define ubi_msg(fmt, ...) printk(KERN_NOTICE "UBI: " fmt "\n", ##__VA_ARGS__)
+
 #define DEV_TYPE_NONE		0
 #define DEV_TYPE_NAND		1
 #define DEV_TYPE_ONENAND	2
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 96dcda2..2c60293 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -347,16 +347,18 @@ static struct mtd_part *add_one_partition(struct mtd_info *master,
 		if (mtd_mod_by_eb(cur_offset, master) != 0) {
 			/* Round up to next erasesize */
 			slave->offset = (mtd_div_by_eb(cur_offset, master) + 1) * master->erasesize;
-			printk(KERN_NOTICE "Moving partition %d: "
-			       "0x%012llx -> 0x%012llx\n", partno,
-			       (unsigned long long)cur_offset, (unsigned long long)slave->offset);
+			debug("Moving partition %d: 0x%012llx -> 0x%012llx\n",
+				partno, (unsigned long long)cur_offset,
+				(unsigned long long)slave->offset);
 		}
 	}
 	if (slave->mtd.size == MTDPART_SIZ_FULL)
 		slave->mtd.size = master->size - slave->offset;
 
-	printk(KERN_NOTICE "0x%012llx-0x%012llx : \"%s\"\n", (unsigned long long)slave->offset,
-		(unsigned long long)(slave->offset + slave->mtd.size), slave->mtd.name);
+	debug("0x%012llx-0x%012llx : \"%s\"\n",
+		(unsigned long long)slave->offset,
+		(unsigned long long)(slave->offset + slave->mtd.size),
+		slave->mtd.name);
 
 	/* let's do some sanity checks */
 	if (slave->offset >= master->size) {
@@ -463,7 +465,7 @@ int add_mtd_partitions(struct mtd_info *master,
 	if (mtd_partitions.next == NULL)
 		INIT_LIST_HEAD(&mtd_partitions);
 
-	printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
+	debug("Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
 
 	for (i = 0; i < nbparts; i++) {
 		slave = add_one_partition(master, parts + i, i, cur_offset);
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index 14c3a5f..e2c416f 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -59,7 +59,11 @@
 #define UBI_NAME_STR "ubi"
 
 /* Normal UBI messages */
+#if DEBUG
 #define ubi_msg(fmt, ...) printk(KERN_NOTICE "UBI: " fmt "\n", ##__VA_ARGS__)
+#else
+#define ubi_msg(fmt, ...)
+#endif
 /* UBI warning messages */
 #define ubi_warn(fmt, ...) printk(KERN_WARNING "UBI warning: %s: " fmt "\n", \
 				  __func__, ##__VA_ARGS__)
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 0af471a..11dd170 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -463,8 +463,12 @@ static inline ino_t parent_ino(struct dentry *dentry)
 #define UBIFS_VERSION 1
 
 /* Normal UBIFS messages */
+#if DEBUG
 #define ubifs_msg(fmt, ...) \
 		printk(KERN_NOTICE "UBIFS: " fmt "\n", ##__VA_ARGS__)
+#else
+#define ubifs_msg(fmt, ...)
+#endif
 /* UBIFS error messages */
 #define ubifs_err(fmt, ...)                                                  \
 	printk(KERN_ERR "UBIFS error (pid %d): %s: " fmt "\n", 0, \
-- 
1.7.11.5

[U-Boot] [PATCH v2 3/6] ubi: ubifs: Turn off verbose prints

[Stefan Roese]

Hi Joe,

On 26.03.2013 22:53, Joe Hershberger wrote:
> The prints are out of control.  SILENCE!
> 
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
> ---
> Changes in v2:
> - Cleaned up the msg print silencing

Thanks. But could you please make these printf's configurable? Either
silence them with an config option (something like
CONFIG_SYS_UBI_SILENCE_OUTPUT). Or create a "verbose" parameter for the
"ubi/ubifs" commands instead?

Otherwise all current users of these UBI/UBIFS functions/commands have
no chance to see the output as they are used to.

Thanks,
Stefan

[U-Boot] [PATCH v2 3/6] ubi: ubifs: Turn off verbose prints

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/02/2013 06:46 AM, Stefan Roese wrote:
> Hi Joe,
> 
> On 26.03.2013 22:53, Joe Hershberger wrote:
>> The prints are out of control.  SILENCE!
>> 
>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> --- 
>> Changes in v2: - Cleaned up the msg print silencing
> 
> Thanks. But could you please make these printf's configurable? 
> Either silence them with an config option (something like 
> CONFIG_SYS_UBI_SILENCE_OUTPUT). Or create a "verbose" parameter
> for the "ubi/ubifs" commands instead?
> 
> Otherwise all current users of these UBI/UBIFS functions/commands 
> have no chance to see the output as they are used to.

Agreed.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRWyKmAAoJENk4IS6UOR1Wc7IP/2G8l2DLxvk0ZAdXyO/51Vy1
2atD1dDPpfARuXCoPw0qgmqtwGRE8qqiFwlUphVXmHsBHUYVQ0TI5irHALh/CeiH
TUDyOk9PSpVIB6XcM2APp5QROyjL13Uyg/QnXyp3Pp28QwGpoomvE2xQQU9jNzca
qc2sAC6t2UaydD4Aouq3uFAAM+QwU1o3Fvca5TZ58nZXpiWDAUBV7cbvxQ5nhaRv
7XoirJ8Vko13V96K0Ty/4fy+2ZPszj73OqgQaDduP5I/7DDADWBWjx4rBkPhY17c
tNh+c2+v873M0yzfmK++cY3FXs6+vkbOa/0zFyR5JJl3lau9GpAetVoZCeIr4fTO
zc3y7Y1r3y0c7DAFJOzWp811QcItDuzT7DEpv/KltDe6N2NM5mzzbba9q+rD3Xk3
1QGyxVP+c5S5tCYqKSIz5FdnKTBZr5MqTDRgB2hbsH5Vob1/rR58inpba+Sl+V7P
p+fIW0L7VSjoiVKdEWDR/XE1SiQmOX6bt8ZuZdE8xPqcOHDq2LeHSlQ9Y5vHTjdF
7znEcZRMcWW/26yUBdnMcHfDL3oddL9CZmFNEGRtqcfHKf1guCmnlLuLcqGRJsBb
5OhnFGhngtsRnjR71fUXlMP/ZJKhm21H/iK1rKE7uUGGrEU5KuREa6+sPgJNFFGu
5D0TncRlgQZtrh2o10JW
=vn7W
-----END PGP SIGNATURE-----

[U-Boot] [PATCH v2 4/6] mtd: Make mtdparts work with pre-reloc env

[Joe Hershberger]

The env in UBI needs to look up the mtd partition as part of relocation,
which happens before relocation.  Make the mtdparts code capable of
working on the default env to start with.

The code tries to set values in the env as well, but again, the env
isn't there yet, so add a check to setenv to not allow sets before the
env is relocated.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
Changes in v2: None

 common/cmd_mtdparts.c | 23 +++++++++++++++++++++--
 common/cmd_nvedit.c   |  4 ++++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/common/cmd_mtdparts.c b/common/cmd_mtdparts.c
index 06fc171..e72dc38 100644
--- a/common/cmd_mtdparts.c
+++ b/common/cmd_mtdparts.c
@@ -106,6 +106,8 @@
 #include <onenand_uboot.h>
 #endif
 
+DECLARE_GLOBAL_DATA_PTR;
+
 /* special size referring to all the remaining space in a partition */
 #define SIZE_REMAINING		0xFFFFFFFF
 
@@ -1539,6 +1541,7 @@ static int parse_mtdparts(const char *const mtdparts)
 	const char *p = mtdparts;
 	struct mtd_device *dev;
 	int err = 1;
+	char tmp_parts[MTDPARTS_MAXLEN];
 
 	debug("\n---parse_mtdparts---\nmtdparts = %s\n\n", p);
 
@@ -1549,7 +1552,12 @@ static int parse_mtdparts(const char *const mtdparts)
 	}
 
 	/* re-read 'mtdparts' variable, mtd_devices_init may be updating env */
-	p = getenv("mtdparts");
+	if (gd->flags & GD_FLG_ENV_READY) {
+		p = getenv("mtdparts");
+	} else {
+		p = tmp_parts;
+		getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN);
+	}
 
 	if (strncmp(p, "mtdparts=", 9) != 0) {
 		printf("mtdparts variable doesn't start with 'mtdparts='\n");
@@ -1707,6 +1715,7 @@ int mtdparts_init(void)
 	const char *current_partition;
 	int ids_changed;
 	char tmp_ep[PARTITION_MAXLEN];
+	char tmp_parts[MTDPARTS_MAXLEN];
 
 	debug("\n---mtdparts_init---\n");
 	if (!initialized) {
@@ -1720,7 +1729,17 @@ int mtdparts_init(void)
 
 	/* get variables */
 	ids = getenv("mtdids");
-	parts = getenv("mtdparts");
+	/*
+	 * The mtdparts variable tends to be long. If we need to access it
+	 * before the env is relocated, then we need to use our own stack
+	 * buffer.  gd->env_buf will be too small.
+	 */
+	if (gd->flags & GD_FLG_ENV_READY) {
+		parts = getenv("mtdparts");
+	} else {
+		parts = tmp_parts;
+		getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN);
+	}
 	current_partition = getenv("partition");
 
 	/* save it for later parsing, cannot rely on current partition pointer
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 7633f0c..c7c6440 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -273,6 +273,10 @@ int setenv(const char *varname, const char *varvalue)
 {
 	const char * const argv[4] = { "setenv", varname, varvalue, NULL };
 
+	/* before import into hashtable */
+	if (!(gd->flags & GD_FLG_ENV_READY))
+		return 1;
+
 	if (varvalue == NULL || varvalue[0] == '\0')
 		return _do_env_set(0, 2, (char * const *)argv);
 	else
-- 
1.7.11.5

[U-Boot] [PATCH v2 5/6] env: Add support for UBI environment

[Joe Hershberger]

UBI is a better place for the environment on NAND devices because it
handles wear-leveling and bad blocks.

Gluebi is needed in Linux to access the env as an MTD partition.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
Changes in v2: None

 README                |  15 ++++++++
 common/Makefile       |   1 +
 common/cmd_nvedit.c   |   3 +-
 common/env_ubi.c      | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/environment.h |  15 ++++++++
 tools/env/fw_env.c    |   3 +-
 6 files changed, 138 insertions(+), 2 deletions(-)
 create mode 100644 common/env_ubi.c

diff --git a/README b/README
index d8cb394..1a389e2 100644
--- a/README
+++ b/README
@@ -3442,6 +3442,21 @@ but it can not erase, write this NOR flash by SRIO or PCIE interface.
 	environment. If redundant environment is used, it will be copied to
 	CONFIG_NAND_ENV_DST + CONFIG_ENV_SIZE.
 
+- CONFIG_ENV_IS_IN_UBI:
+
+	Define this if you have an UBI volume that you want to use for the
+	environment.  This has the benefit of wear-leveling the environment
+	accesses, which is important on NAND.
+
+	- CONFIG_ENV_UBI_PART:
+
+	  Define this to a string that is the mtd partition containing the UBI.
+
+	- CONFIG_ENV_UBI_VOLUME:
+
+	  Define this to the name of the volume that you want to store the
+	  environment in.
+
 - CONFIG_SYS_SPI_INIT_OFFSET
 
 	Defines offset to the initial SPI buffer area in DPRAM. The
diff --git a/common/Makefile b/common/Makefile
index 54fcc81..9ff59c5 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -62,6 +62,7 @@ COBJS-$(CONFIG_ENV_IS_IN_NVRAM) += env_nvram.o
 COBJS-$(CONFIG_ENV_IS_IN_ONENAND) += env_onenand.o
 COBJS-$(CONFIG_ENV_IS_IN_SPI_FLASH) += env_sf.o
 COBJS-$(CONFIG_ENV_IS_IN_REMOTE) += env_remote.o
+COBJS-$(CONFIG_ENV_IS_IN_UBI) += env_ubi.o
 COBJS-$(CONFIG_ENV_IS_NOWHERE) += env_nowhere.o
 
 # command
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index c7c6440..960b22c 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -62,9 +62,10 @@ DECLARE_GLOBAL_DATA_PTR;
 	!defined(CONFIG_ENV_IS_IN_ONENAND)	&& \
 	!defined(CONFIG_ENV_IS_IN_SPI_FLASH)	&& \
 	!defined(CONFIG_ENV_IS_IN_REMOTE)	&& \
+	!defined(CONFIG_ENV_IS_IN_UBI)		&& \
 	!defined(CONFIG_ENV_IS_NOWHERE)
 # error Define one of CONFIG_ENV_IS_IN_{EEPROM|FLASH|DATAFLASH|ONENAND|\
-SPI_FLASH|NVRAM|MMC|FAT|REMOTE} or CONFIG_ENV_IS_NOWHERE
+SPI_FLASH|NVRAM|MMC|FAT|REMOTE|UBI} or CONFIG_ENV_IS_NOWHERE
 #endif
 
 /*
diff --git a/common/env_ubi.c b/common/env_ubi.c
new file mode 100644
index 0000000..9a47fd2
--- /dev/null
+++ b/common/env_ubi.c
@@ -0,0 +1,103 @@
+/*
+ * (c) Copyright 2012 by National Instruments,
+ *        Joe Hershberger <joe.hershberger@ni.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+
+#include <command.h>
+#include <environment.h>
+#include <errno.h>
+#include <malloc.h>
+#include <search.h>
+#include <ubi_uboot.h>
+#undef crc32
+
+char *env_name_spec = "UBI";
+
+env_t *env_ptr;
+
+DECLARE_GLOBAL_DATA_PTR;
+
+int env_init(void)
+{
+	/* use default */
+	gd->env_addr = (ulong)&default_environment[0];
+	gd->env_valid = 1;
+
+	return 0;
+}
+
+#ifdef CONFIG_CMD_SAVEENV
+int saveenv(void)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(env_t, env_new, 1);
+	ssize_t	len;
+	char *res;
+
+	res = (char *)&env_new->data;
+	len = hexport_r(&env_htab, '\0', 0, &res, ENV_SIZE, 0, NULL);
+	if (len < 0) {
+		error("Cannot export environment: errno = %d\n", errno);
+		return 1;
+	}
+
+	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
+		printf("\n** Cannot find mtd partition \"%s\"\n",
+			CONFIG_ENV_UBI_PART);
+		return 1;
+	}
+
+	env_new->crc = crc32(0, env_new->data, ENV_SIZE);
+
+	if (ubi_volume_write(CONFIG_ENV_UBI_VOLUME, (void *)env_new,
+	    CONFIG_ENV_SIZE)) {
+		printf("\n** Unable to write env to %s:%s **\n",
+			CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME);
+		return 1;
+	}
+
+	puts("done\n");
+	return 0;
+}
+#endif /* CONFIG_CMD_SAVEENV */
+
+void env_relocate_spec(void)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE);
+
+	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
+		printf("\n** Cannot find mtd partition \"%s\"\n",
+			CONFIG_ENV_UBI_PART);
+		set_default_env(NULL);
+		return;
+	}
+
+	if (ubi_volume_read(CONFIG_ENV_UBI_VOLUME, (void *)&buf,
+	    CONFIG_ENV_SIZE)) {
+		printf("\n** Unable to read env from %s:%s **\n",
+			CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME);
+		set_default_env(NULL);
+		return;
+	}
+
+	env_import(buf, 1);
+}
diff --git a/include/environment.h b/include/environment.h
index e64b43d..ece073f 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -96,6 +96,21 @@ extern unsigned long nand_env_oob_offset;
 # endif
 #endif /* CONFIG_ENV_IS_IN_NAND */
 
+#if defined(CONFIG_ENV_IS_IN_UBI)
+# ifndef CONFIG_ENV_UBI_PART
+#  error "Need to define CONFIG_ENV_UBI_PART when using CONFIG_ENV_IS_IN_UBI"
+# endif
+# ifndef CONFIG_ENV_UBI_VOLUME
+#  error "Need to define CONFIG_ENV_UBI_VOLUME when using CONFIG_ENV_IS_IN_UBI"
+# endif
+# ifndef CONFIG_ENV_SIZE
+#  error "Need to define CONFIG_ENV_SIZE when using CONFIG_ENV_IS_IN_UBI"
+# endif
+# ifndef CONFIG_CMD_UBI
+#  error "Need to define CONFIG_CMD_UBI when using CONFIG_ENV_IS_IN_UBI"
+# endif
+#endif /* CONFIG_ENV_IS_IN_UBI */
+
 /* Embedded env is only supported for some flash types */
 #ifdef CONFIG_ENV_IS_EMBEDDED
 # if	!defined(CONFIG_ENV_IS_IN_FLASH)	&& \
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 37b60b8..88e184f 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -959,7 +959,8 @@ static int flash_read (int fd)
 
 	if (mtdinfo.type != MTD_NORFLASH &&
 	    mtdinfo.type != MTD_NANDFLASH &&
-	    mtdinfo.type != MTD_DATAFLASH) {
+	    mtdinfo.type != MTD_DATAFLASH &&
+	    mtdinfo.type != MTD_UBIVOLUME) {
 		fprintf (stderr, "Unsupported flash type %u\n", mtdinfo.type);
 		return -1;
 	}
-- 
1.7.11.5

[U-Boot] [PATCH v2 6/6] env: Add redundant env support to UBI env

[Joe Hershberger]

Allow the user to specify two UBI volumes to use for the environment

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
Changes in v2:
- Added curly braces on "multi-line" statements

 README                |   6 +++
 common/env_ubi.c      | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/environment.h |   3 ++
 tools/env/fw_env.c    |   3 ++
 4 files changed, 129 insertions(+)

diff --git a/README b/README
index 1a389e2..f241d54 100644
--- a/README
+++ b/README
@@ -3457,6 +3457,12 @@ but it can not erase, write this NOR flash by SRIO or PCIE interface.
 	  Define this to the name of the volume that you want to store the
 	  environment in.
 
+	- CONFIG_ENV_UBI_VOLUME_REDUND:
+
+	  Define this to the name of another volume to store a second copy of
+	  the environment in.  This will enable redundant environments in UBI.
+	  It is assumed that both volumes are in the same MTD partition.
+
 - CONFIG_SYS_SPI_INIT_OFFSET
 
 	Defines offset to the initial SPI buffer area in DPRAM. The
diff --git a/common/env_ubi.c b/common/env_ubi.c
index 9a47fd2..ab647ff 100644
--- a/common/env_ubi.c
+++ b/common/env_ubi.c
@@ -47,6 +47,58 @@ int env_init(void)
 }
 
 #ifdef CONFIG_CMD_SAVEENV
+#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
+static unsigned char env_flags;
+
+int saveenv(void)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(env_t, env_new, 1);
+	ssize_t	len;
+	char *res;
+
+	res = (char *)&env_new->data;
+	len = hexport_r(&env_htab, '\0', 0, &res, ENV_SIZE, 0, NULL);
+	if (len < 0) {
+		error("Cannot export environment: errno = %d\n", errno);
+		return 1;
+	}
+
+	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
+		printf("\n** Cannot find mtd partition \"%s\"\n",
+			CONFIG_ENV_UBI_PART);
+		return 1;
+	}
+
+	env_new->crc = crc32(0, env_new->data, ENV_SIZE);
+	env_new->flags = ++env_flags; /* increase the serial */
+
+	if (gd->env_valid == 1) {
+		puts("Writing to redundant UBI... ");
+		if (ubi_volume_write(CONFIG_ENV_UBI_VOLUME_REDUND,
+		    (void *)env_new, CONFIG_ENV_SIZE)) {
+			printf("\n** Unable to write env to %s:%s **\n",
+				CONFIG_ENV_UBI_PART,
+				CONFIG_ENV_UBI_VOLUME_REDUND);
+			return 1;
+		}
+	} else {
+		puts("Writing to UBI... ");
+		if (ubi_volume_write(CONFIG_ENV_UBI_VOLUME,
+		    (void *)env_new, CONFIG_ENV_SIZE)) {
+			printf("\n** Unable to write env to %s:%s **\n",
+				CONFIG_ENV_UBI_PART,
+				CONFIG_ENV_UBI_VOLUME);
+			return 1;
+		}
+	}
+
+	puts("done\n");
+
+	gd->env_valid = gd->env_valid == 2 ? 1 : 2;
+
+	return 0;
+}
+#else /* ! CONFIG_SYS_REDUNDAND_ENVIRONMENT */
 int saveenv(void)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(env_t, env_new, 1);
@@ -78,8 +130,72 @@ int saveenv(void)
 	puts("done\n");
 	return 0;
 }
+#endif /* CONFIG_SYS_REDUNDAND_ENVIRONMENT */
 #endif /* CONFIG_CMD_SAVEENV */
 
+#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
+void env_relocate_spec(void)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(char, env1_buf, CONFIG_ENV_SIZE);
+	ALLOC_CACHE_ALIGN_BUFFER(char, env2_buf, CONFIG_ENV_SIZE);
+	int crc1_ok = 0, crc2_ok = 0;
+	env_t *ep, *tmp_env1, *tmp_env2;
+
+	tmp_env1 = (env_t *)env1_buf;
+	tmp_env2 = (env_t *)env2_buf;
+
+	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
+		printf("\n** Cannot find mtd partition \"%s\"\n",
+			CONFIG_ENV_UBI_PART);
+		set_default_env(NULL);
+		return;
+	}
+
+	if (ubi_volume_read(CONFIG_ENV_UBI_VOLUME, (void *)tmp_env1,
+	    CONFIG_ENV_SIZE)) {
+		printf("\n** Unable to read env from %s:%s **\n",
+			CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME);
+	}
+
+	if (ubi_volume_read(CONFIG_ENV_UBI_VOLUME_REDUND, (void *)tmp_env2,
+	    CONFIG_ENV_SIZE)) {
+		printf("\n** Unable to read redundant env from %s:%s **\n",
+			CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME_REDUND);
+	}
+
+	crc1_ok = crc32(0, tmp_env1->data, ENV_SIZE) == tmp_env1->crc;
+	crc2_ok = crc32(0, tmp_env2->data, ENV_SIZE) == tmp_env2->crc;
+
+	if (!crc1_ok && !crc2_ok) {
+		set_default_env("!bad CRC");
+		return;
+	} else if (crc1_ok && !crc2_ok) {
+		gd->env_valid = 1;
+	} else if (!crc1_ok && crc2_ok) {
+		gd->env_valid = 2;
+	} else {
+		/* both ok - check serial */
+		if (tmp_env1->flags == 255 && tmp_env2->flags == 0)
+			gd->env_valid = 2;
+		else if (tmp_env2->flags == 255 && tmp_env1->flags == 0)
+			gd->env_valid = 1;
+		else if (tmp_env1->flags > tmp_env2->flags)
+			gd->env_valid = 1;
+		else if (tmp_env2->flags > tmp_env1->flags)
+			gd->env_valid = 2;
+		else /* flags are equal - almost impossible */
+			gd->env_valid = 1;
+	}
+
+	if (gd->env_valid == 1)
+		ep = tmp_env1;
+	else
+		ep = tmp_env2;
+
+	env_flags = ep->flags;
+	env_import((char *)ep, 0);
+}
+#else /* ! CONFIG_SYS_REDUNDAND_ENVIRONMENT */
 void env_relocate_spec(void)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE);
@@ -101,3 +217,4 @@ void env_relocate_spec(void)
 
 	env_import(buf, 1);
 }
+#endif /* CONFIG_SYS_REDUNDAND_ENVIRONMENT */
diff --git a/include/environment.h b/include/environment.h
index ece073f..4c6a37b 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -103,6 +103,9 @@ extern unsigned long nand_env_oob_offset;
 # ifndef CONFIG_ENV_UBI_VOLUME
 #  error "Need to define CONFIG_ENV_UBI_VOLUME when using CONFIG_ENV_IS_IN_UBI"
 # endif
+# if defined(CONFIG_ENV_UBI_VOLUME_REDUND)
+#  define CONFIG_SYS_REDUNDAND_ENVIRONMENT
+# endif
 # ifndef CONFIG_ENV_SIZE
 #  error "Need to define CONFIG_ENV_SIZE when using CONFIG_ENV_IS_IN_UBI"
 # endif
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 88e184f..7654b1e 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -1135,6 +1135,9 @@ int fw_env_open(void)
 		} else if (DEVTYPE(dev_current) == MTD_DATAFLASH &&
 			   DEVTYPE(!dev_current) == MTD_DATAFLASH) {
 			environment.flag_scheme = FLAG_BOOLEAN;
+		} else if (DEVTYPE(dev_current) == MTD_UBIVOLUME &&
+			   DEVTYPE(!dev_current) == MTD_UBIVOLUME) {
+			environment.flag_scheme = FLAG_INCREMENTAL;
 		} else {
 			fprintf (stderr, "Incompatible flash types!\n");
 			return -1;
-- 
1.7.11.5

[U-Boot] [PATCH v3 0/7] Add support for using an UBI volume for environment

[Joe Hershberger]

NAND is not good at handling absolute addresses to sectors for storing
particular data.  The current implementation of the NAND env support
works around this in several ways such as storing a pointer to the
sector in the OOB of the first sector (interferes with some CRC) or
supporting a range of sectors (which unless it is huge is not
guaranteed to be safe).  None of these options address wear-leveling
concerns or bad block handling.

Accessing the u-boot env from UBI eliminates these concerns.  However,
it does require some of the basic settings for finding the UBI env to
be in the default u-boot env.

Changes in v3:
- Added documentation for UBI and UBIFS to README
- Changed the silence to opt-in and added the options to README
- Added comment to README about using _SILENCE_MSG options

Changes in v2:
- Fixed error handling bug that prevents fail-over to default env on error
- Added curly braces on "multi-line" statements
- Added extern consistently in header
- Cleaned up the msg print silencing
- Added curly braces on "multi-line" statements

Joe Hershberger (7):
  ubi: Fix broken cleanup code in attach_by_scanning
  ubi: Expose a few simple functions from the cmd_ubi
  ubi: ubifs: Add documentation for README
  ubi: ubifs: Turn off verbose prints
  mtd: Make mtdparts work with pre-reloc env
  env: Add support for UBI environment
  env: Add redundant env support to UBI env

 README                  |  53 ++++++++++++
 common/Makefile         |   1 +
 common/cmd_mtdparts.c   |  23 ++++-
 common/cmd_nvedit.c     |   7 +-
 common/cmd_ubi.c        | 153 ++++++++++++++++++---------------
 common/env_ubi.c        | 220 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/mtdpart.c   |  14 +--
 drivers/mtd/ubi/build.c |   8 +-
 drivers/mtd/ubi/ubi.h   |   4 +
 drivers/mtd/ubi/wl.c    |   1 +
 fs/ubifs/ubifs.h        |   4 +
 include/environment.h   |  18 ++++
 include/ubi_uboot.h     |   3 +
 tools/env/fw_env.c      |   6 +-
 14 files changed, 434 insertions(+), 81 deletions(-)
 create mode 100644 common/env_ubi.c

-- 
1.7.11.5

[U-Boot] [PATCH v3 1/7] ubi: Fix broken cleanup code in attach_by_scanning

[Joe Hershberger]

The unwind code was not reversing operations correctly and was causing
a hang on any error condition.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
Changes in v3: None
Changes in v2:
- Fixed error handling bug that prevents fail-over to default env on error

 drivers/mtd/ubi/build.c | 8 ++++----
 drivers/mtd/ubi/wl.c    | 1 +
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index d144ac2..a708162 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -478,19 +478,19 @@ static int attach_by_scanning(struct ubi_device *ubi)
 
 	err = ubi_eba_init_scan(ubi, si);
 	if (err)
-		goto out_wl;
+		goto out_vtbl;
 
 	err = ubi_wl_init_scan(ubi, si);
 	if (err)
-		goto out_vtbl;
+		goto out_eba;
 
 	ubi_scan_destroy_si(si);
 	return 0;
 
+out_eba:
+	ubi_eba_close(ubi);
 out_vtbl:
 	vfree(ubi->vtbl);
-out_wl:
-	ubi_wl_close(ubi);
 out_si:
 	ubi_scan_destroy_si(si);
 	return err;
diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
index 88b867a..d1ba722 100644
--- a/drivers/mtd/ubi/wl.c
+++ b/drivers/mtd/ubi/wl.c
@@ -1538,6 +1538,7 @@ int ubi_wl_init_scan(struct ubi_device *ubi, struct ubi_scan_info *si)
 	if (ubi->avail_pebs < WL_RESERVED_PEBS) {
 		ubi_err("no enough physical eraseblocks (%d, need %d)",
 			ubi->avail_pebs, WL_RESERVED_PEBS);
+		err = -ENOSPC;
 		goto out_free;
 	}
 	ubi->avail_pebs -= WL_RESERVED_PEBS;
-- 
1.7.11.5

[U-Boot] [PATCH v3 2/7] ubi: Expose a few simple functions from the cmd_ubi

[Joe Hershberger]

Part, Read, and Write functionality that will be used by env_ubi.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
Changes in v3: None
Changes in v2:
- Added curly braces on "multi-line" statements
- Added extern consistently in header

 common/cmd_ubi.c    | 150 +++++++++++++++++++++++++++++-----------------------
 include/ubi_uboot.h |   3 ++
 2 files changed, 86 insertions(+), 67 deletions(-)

diff --git a/common/cmd_ubi.c b/common/cmd_ubi.c
index 35b1d31..41fbae7 100644
--- a/common/cmd_ubi.c
+++ b/common/cmd_ubi.c
@@ -263,7 +263,7 @@ out_err:
 	return err;
 }
 
-static int ubi_volume_write(char *volume, void *buf, size_t size)
+int ubi_volume_write(char *volume, void *buf, size_t size)
 {
 	int err = 1;
 	int rsvd_bytes = 0;
@@ -308,12 +308,10 @@ static int ubi_volume_write(char *volume, void *buf, size_t size)
 		ubi_gluebi_updated(vol);
 	}
 
-	printf("%d bytes written to volume %s\n", size, volume);
-
 	return 0;
 }
 
-static int ubi_volume_read(char *volume, char *buf, size_t size)
+int ubi_volume_read(char *volume, char *buf, size_t size)
 {
 	int err, lnum, off, len, tbuf_size;
 	void *tbuf;
@@ -325,8 +323,6 @@ static int ubi_volume_read(char *volume, char *buf, size_t size)
 	if (vol == NULL)
 		return ENODEV;
 
-	printf("Read %d bytes from volume %s to %p\n", size, volume, buf);
-
 	if (vol->updating) {
 		printf("updating");
 		return EBUSY;
@@ -431,26 +427,82 @@ static int ubi_dev_scan(struct mtd_info *info, char *ubidev,
 	return 0;
 }
 
-static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
+int ubi_part(char *part_name, const char *vid_header_offset)
 {
-	size_t size = 0;
-	ulong addr = 0;
 	int err = 0;
-
-	if (argc < 2)
-		return CMD_RET_USAGE;
+	char mtd_dev[16];
+	struct mtd_device *dev;
+	struct part_info *part;
+	u8 pnum;
 
 	if (mtdparts_init() != 0) {
 		printf("Error initializing mtdparts!\n");
 		return 1;
 	}
 
+#ifdef CONFIG_CMD_UBIFS
+	/*
+	 * Automatically unmount UBIFS partition when user
+	 * changes the UBI device. Otherwise the following
+	 * UBIFS commands will crash.
+	 */
+	if (ubifs_is_mounted())
+		cmd_ubifs_umount();
+#endif
+
+	/* todo: get dev number for NAND... */
+	ubi_dev.nr = 0;
+
+	/*
+	 * Call ubi_exit() before re-initializing the UBI subsystem
+	 */
+	if (ubi_initialized) {
+		ubi_exit();
+		del_mtd_partitions(ubi_dev.mtd_info);
+	}
+
+	/*
+	 * Search the mtd device number where this partition
+	 * is located
+	 */
+	if (find_dev_and_part(part_name, &dev, &pnum, &part)) {
+		printf("Partition %s not found!\n", part_name);
+		return 1;
+	}
+	sprintf(mtd_dev, "%s%d", MTD_DEV_TYPE(dev->id->type), dev->id->num);
+	ubi_dev.mtd_info = get_mtd_device_nm(mtd_dev);
+	if (IS_ERR(ubi_dev.mtd_info)) {
+		printf("Partition %s not found on device %s!\n", part_name,
+		       mtd_dev);
+		return 1;
+	}
+
+	ubi_dev.selected = 1;
+
+	strcpy(ubi_dev.part_name, part_name);
+	err = ubi_dev_scan(ubi_dev.mtd_info, ubi_dev.part_name,
+			vid_header_offset);
+	if (err) {
+		printf("UBI init error %d\n", err);
+		ubi_dev.selected = 0;
+		return err;
+	}
+
+	ubi = ubi_devices[0];
+
+	return 0;
+}
+
+static int do_ubi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	size_t size = 0;
+	ulong addr = 0;
+
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
 	if (strcmp(argv[1], "part") == 0) {
-		char mtd_dev[16];
-		struct mtd_device *dev;
-		struct part_info *part;
 		const char *vid_header_offset = NULL;
-		u8 pnum;
 
 		/* Print current partition */
 		if (argc == 2) {
@@ -467,58 +519,10 @@ static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 		if (argc < 3)
 			return CMD_RET_USAGE;
 
-#ifdef CONFIG_CMD_UBIFS
-		/*
-		 * Automatically unmount UBIFS partition when user
-		 * changes the UBI device. Otherwise the following
-		 * UBIFS commands will crash.
-		 */
-		if (ubifs_is_mounted())
-			cmd_ubifs_umount();
-#endif
-
-		/* todo: get dev number for NAND... */
-		ubi_dev.nr = 0;
-
-		/*
-		 * Call ubi_exit() before re-initializing the UBI subsystem
-		 */
-		if (ubi_initialized) {
-			ubi_exit();
-			del_mtd_partitions(ubi_dev.mtd_info);
-		}
-
-		/*
-		 * Search the mtd device number where this partition
-		 * is located
-		 */
-		if (find_dev_and_part(argv[2], &dev, &pnum, &part)) {
-			printf("Partition %s not found!\n", argv[2]);
-			return 1;
-		}
-		sprintf(mtd_dev, "%s%d", MTD_DEV_TYPE(dev->id->type), dev->id->num);
-		ubi_dev.mtd_info = get_mtd_device_nm(mtd_dev);
-		if (IS_ERR(ubi_dev.mtd_info)) {
-			printf("Partition %s not found on device %s!\n", argv[2], mtd_dev);
-			return 1;
-		}
-
-		ubi_dev.selected = 1;
-
 		if (argc > 3)
 			vid_header_offset = argv[3];
-		strcpy(ubi_dev.part_name, argv[2]);
-		err = ubi_dev_scan(ubi_dev.mtd_info, ubi_dev.part_name,
-				vid_header_offset);
-		if (err) {
-			printf("UBI init error %d\n", err);
-			ubi_dev.selected = 0;
-			return err;
-		}
-
-		ubi = ubi_devices[0];
 
-		return 0;
+		return ubi_part(argv[2], vid_header_offset);
 	}
 
 	if ((strcmp(argv[1], "part") != 0) && (!ubi_dev.selected)) {
@@ -571,6 +575,8 @@ static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 	}
 
 	if (strncmp(argv[1], "write", 5) == 0) {
+		int ret;
+
 		if (argc < 5) {
 			printf("Please see usage\n");
 			return 1;
@@ -579,7 +585,13 @@ static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 		addr = simple_strtoul(argv[2], NULL, 16);
 		size = simple_strtoul(argv[4], NULL, 16);
 
-		return ubi_volume_write(argv[3], (void *)addr, size);
+		ret = ubi_volume_write(argv[3], (void *)addr, size);
+		if (!ret) {
+			printf("%d bytes written to volume %s\n", size,
+			       argv[3]);
+		}
+
+		return ret;
 	}
 
 	if (strncmp(argv[1], "read", 4) == 0) {
@@ -597,8 +609,12 @@ static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 			argc--;
 		}
 
-		if (argc == 3)
+		if (argc == 3) {
+			printf("Read %d bytes from volume %s to %lx\n", size,
+			       argv[3], addr);
+
 			return ubi_volume_read(argv[3], (char *)addr, size);
+		}
 	}
 
 	printf("Please see usage\n");
diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h
index 69006e2..7f72022 100644
--- a/include/ubi_uboot.h
+++ b/include/ubi_uboot.h
@@ -214,6 +214,9 @@ static inline long IS_ERR(const void *ptr)
 extern int ubi_mtd_param_parse(const char *val, struct kernel_param *kp);
 extern int ubi_init(void);
 extern void ubi_exit(void);
+extern int ubi_part(char *part_name, const char *vid_header_offset);
+extern int ubi_volume_write(char *volume, void *buf, size_t size);
+extern int ubi_volume_read(char *volume, char *buf, size_t size);
 
 extern struct ubi_device *ubi_devices[];
 
-- 
1.7.11.5

[U-Boot] [PATCH v3 3/7] ubi: ubifs: Add documentation for README

[Joe Hershberger]

Describe the needed CONFIG tokens to enable UBI and UBIFS support.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
Changes in v3:
- Added documentation for UBI and UBIFS to README

Changes in v2: None

 README | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/README b/README
index 5701016..6d45111 100644
--- a/README
+++ b/README
@@ -2783,6 +2783,22 @@ FIT uImage format:
 		Adds the MTD partitioning infrastructure from the Linux
 		kernel. Needed for UBI support.
 
+- UBI support
+		CONFIG_CMD_UBI
+
+		Adds commands for interacting with MTD partitions formatted
+		with the UBI flash translation layer
+
+		Requires also defining CONFIG_RBTREE
+
+- UBIFS support
+		CONFIG_CMD_UBIFS
+
+		Adds commands for interacting with UBI volumes formatted as
+		UBIFS.  UBIFS is read-only in u-boot.
+
+		Requires UBI support as well as CONFIG_LZO
+
 - SPL framework
 		CONFIG_SPL
 		Enable building of SPL globally.
-- 
1.7.11.5

[U-Boot] [PATCH v3 4/7] ubi: ubifs: Turn off verbose prints

[Joe Hershberger]

The prints are out of control.  SILENCE!

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
Changes in v3:
- Changed the silence to opt-in and added the options to README

Changes in v2:
- Cleaned up the msg print silencing

 README                | 10 ++++++++++
 common/cmd_ubi.c      |  3 +++
 drivers/mtd/mtdpart.c | 14 ++++++++------
 drivers/mtd/ubi/ubi.h |  4 ++++
 fs/ubifs/ubifs.h      |  4 ++++
 5 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/README b/README
index 6d45111..8235470 100644
--- a/README
+++ b/README
@@ -2791,6 +2791,11 @@ FIT uImage format:
 
 		Requires also defining CONFIG_RBTREE
 
+		CONFIG_UBI_SILENCE_MSG
+
+		Make the verbose messages from UBI stop printing.  This leaves
+		warnings and errors enabled.
+
 - UBIFS support
 		CONFIG_CMD_UBIFS
 
@@ -2799,6 +2804,11 @@ FIT uImage format:
 
 		Requires UBI support as well as CONFIG_LZO
 
+		CONFIG_UBIFS_SILENCE_MSG
+
+		Make the verbose messages from UBIFS stop printing.  This leaves
+		warnings and errors enabled.
+
 - SPL framework
 		CONFIG_SPL
 		Enable building of SPL globally.
diff --git a/common/cmd_ubi.c b/common/cmd_ubi.c
index 41fbae7..5ba4feb 100644
--- a/common/cmd_ubi.c
+++ b/common/cmd_ubi.c
@@ -23,6 +23,9 @@
 #include <asm/errno.h>
 #include <jffs2/load_kernel.h>
 
+#undef ubi_msg
+#define ubi_msg(fmt, ...) printf("UBI: " fmt "\n", ##__VA_ARGS__)
+
 #define DEV_TYPE_NONE		0
 #define DEV_TYPE_NAND		1
 #define DEV_TYPE_ONENAND	2
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 96dcda2..cbfc679 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -347,16 +347,18 @@ static struct mtd_part *add_one_partition(struct mtd_info *master,
 		if (mtd_mod_by_eb(cur_offset, master) != 0) {
 			/* Round up to next erasesize */
 			slave->offset = (mtd_div_by_eb(cur_offset, master) + 1) * master->erasesize;
-			printk(KERN_NOTICE "Moving partition %d: "
-			       "0x%012llx -> 0x%012llx\n", partno,
-			       (unsigned long long)cur_offset, (unsigned long long)slave->offset);
+			debug("Moving partition %d: 0x%012llx -> 0x%012llx\n",
+			      partno, (unsigned long long)cur_offset,
+			      (unsigned long long)slave->offset);
 		}
 	}
 	if (slave->mtd.size == MTDPART_SIZ_FULL)
 		slave->mtd.size = master->size - slave->offset;
 
-	printk(KERN_NOTICE "0x%012llx-0x%012llx : \"%s\"\n", (unsigned long long)slave->offset,
-		(unsigned long long)(slave->offset + slave->mtd.size), slave->mtd.name);
+	debug("0x%012llx-0x%012llx : \"%s\"\n",
+	      (unsigned long long)slave->offset,
+	      (unsigned long long)(slave->offset + slave->mtd.size),
+	      slave->mtd.name);
 
 	/* let's do some sanity checks */
 	if (slave->offset >= master->size) {
@@ -463,7 +465,7 @@ int add_mtd_partitions(struct mtd_info *master,
 	if (mtd_partitions.next == NULL)
 		INIT_LIST_HEAD(&mtd_partitions);
 
-	printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
+	debug("Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
 
 	for (i = 0; i < nbparts; i++) {
 		slave = add_one_partition(master, parts + i, i, cur_offset);
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index 14c3a5f..044e849 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -59,7 +59,11 @@
 #define UBI_NAME_STR "ubi"
 
 /* Normal UBI messages */
+#ifdef CONFIG_UBI_SILENCE_MSG
+#define ubi_msg(fmt, ...)
+#else
 #define ubi_msg(fmt, ...) printk(KERN_NOTICE "UBI: " fmt "\n", ##__VA_ARGS__)
+#endif
 /* UBI warning messages */
 #define ubi_warn(fmt, ...) printk(KERN_WARNING "UBI warning: %s: " fmt "\n", \
 				  __func__, ##__VA_ARGS__)
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 0af471a..633631e 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -463,8 +463,12 @@ static inline ino_t parent_ino(struct dentry *dentry)
 #define UBIFS_VERSION 1
 
 /* Normal UBIFS messages */
+#ifdef CONFIG_UBIFS_SILENCE_MSG
+#define ubifs_msg(fmt, ...)
+#else
 #define ubifs_msg(fmt, ...) \
 		printk(KERN_NOTICE "UBIFS: " fmt "\n", ##__VA_ARGS__)
+#endif
 /* UBIFS error messages */
 #define ubifs_err(fmt, ...)                                                  \
 	printk(KERN_ERR "UBIFS error (pid %d): %s: " fmt "\n", 0, \
-- 
1.7.11.5

[U-Boot] [PATCH v3 5/7] mtd: Make mtdparts work with pre-reloc env

[Joe Hershberger]

The env in UBI needs to look up the mtd partition as part of relocation,
which happens before relocation.  Make the mtdparts code capable of
working on the default env to start with.

The code tries to set values in the env as well, but again, the env
isn't there yet, so add a check to setenv to not allow sets before the
env is relocated.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
Changes in v3: None
Changes in v2: None

 common/cmd_mtdparts.c | 23 +++++++++++++++++++++--
 common/cmd_nvedit.c   |  4 ++++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/common/cmd_mtdparts.c b/common/cmd_mtdparts.c
index 5192dee..1c35f9d 100644
--- a/common/cmd_mtdparts.c
+++ b/common/cmd_mtdparts.c
@@ -106,6 +106,8 @@
 #include <onenand_uboot.h>
 #endif
 
+DECLARE_GLOBAL_DATA_PTR;
+
 /* special size referring to all the remaining space in a partition */
 #define SIZE_REMAINING		0xFFFFFFFF
 
@@ -1537,6 +1539,7 @@ static int parse_mtdparts(const char *const mtdparts)
 	const char *p = mtdparts;
 	struct mtd_device *dev;
 	int err = 1;
+	char tmp_parts[MTDPARTS_MAXLEN];
 
 	debug("\n---parse_mtdparts---\nmtdparts = %s\n\n", p);
 
@@ -1547,7 +1550,12 @@ static int parse_mtdparts(const char *const mtdparts)
 	}
 
 	/* re-read 'mtdparts' variable, mtd_devices_init may be updating env */
-	p = getenv("mtdparts");
+	if (gd->flags & GD_FLG_ENV_READY) {
+		p = getenv("mtdparts");
+	} else {
+		p = tmp_parts;
+		getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN);
+	}
 
 	if (strncmp(p, "mtdparts=", 9) != 0) {
 		printf("mtdparts variable doesn't start with 'mtdparts='\n");
@@ -1705,6 +1713,7 @@ int mtdparts_init(void)
 	const char *current_partition;
 	int ids_changed;
 	char tmp_ep[PARTITION_MAXLEN];
+	char tmp_parts[MTDPARTS_MAXLEN];
 
 	debug("\n---mtdparts_init---\n");
 	if (!initialized) {
@@ -1718,7 +1727,17 @@ int mtdparts_init(void)
 
 	/* get variables */
 	ids = getenv("mtdids");
-	parts = getenv("mtdparts");
+	/*
+	 * The mtdparts variable tends to be long. If we need to access it
+	 * before the env is relocated, then we need to use our own stack
+	 * buffer.  gd->env_buf will be too small.
+	 */
+	if (gd->flags & GD_FLG_ENV_READY) {
+		parts = getenv("mtdparts");
+	} else {
+		parts = tmp_parts;
+		getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN);
+	}
 	current_partition = getenv("partition");
 
 	/* save it for later parsing, cannot rely on current partition pointer
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 947d6c4..fab8694 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -273,6 +273,10 @@ int setenv(const char *varname, const char *varvalue)
 {
 	const char * const argv[4] = { "setenv", varname, varvalue, NULL };
 
+	/* before import into hashtable */
+	if (!(gd->flags & GD_FLG_ENV_READY))
+		return 1;
+
 	if (varvalue == NULL || varvalue[0] == '\0')
 		return _do_env_set(0, 2, (char * const *)argv);
 	else
-- 
1.7.11.5

[U-Boot] [PATCH v3 6/7] env: Add support for UBI environment

[Joe Hershberger]

UBI is a better place for the environment on NAND devices because it
handles wear-leveling and bad blocks.

Gluebi is needed in Linux to access the env as an MTD partition.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
Changes in v3:
- Added comment to README about using _SILENCE_MSG options

Changes in v2: None

 README                |  21 ++++++++++
 common/Makefile       |   1 +
 common/cmd_nvedit.c   |   3 +-
 common/env_ubi.c      | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/environment.h |  15 ++++++++
 tools/env/fw_env.c    |   3 +-
 6 files changed, 144 insertions(+), 2 deletions(-)
 create mode 100644 common/env_ubi.c

diff --git a/README b/README
index 8235470..37ef694 100644
--- a/README
+++ b/README
@@ -3525,6 +3525,27 @@ but it can not erase, write this NOR flash by SRIO or PCIE interface.
 	environment. If redundant environment is used, it will be copied to
 	CONFIG_NAND_ENV_DST + CONFIG_ENV_SIZE.
 
+- CONFIG_ENV_IS_IN_UBI:
+
+	Define this if you have an UBI volume that you want to use for the
+	environment.  This has the benefit of wear-leveling the environment
+	accesses, which is important on NAND.
+
+	- CONFIG_ENV_UBI_PART:
+
+	  Define this to a string that is the mtd partition containing the UBI.
+
+	- CONFIG_ENV_UBI_VOLUME:
+
+	  Define this to the name of the volume that you want to store the
+	  environment in.
+
+	- CONFIG_UBI_SILENCE_MSG
+	- CONFIG_UBIFS_SILENCE_MSG
+
+	  You will probably want to define these to avoid a really noisy system
+	  when storing the env in UBI.
+
 - CONFIG_SYS_SPI_INIT_OFFSET
 
 	Defines offset to the initial SPI buffer area in DPRAM. The
diff --git a/common/Makefile b/common/Makefile
index f631311..0e0fff1 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -66,6 +66,7 @@ COBJS-$(CONFIG_ENV_IS_IN_NVRAM) += env_nvram.o
 COBJS-$(CONFIG_ENV_IS_IN_ONENAND) += env_onenand.o
 COBJS-$(CONFIG_ENV_IS_IN_SPI_FLASH) += env_sf.o
 COBJS-$(CONFIG_ENV_IS_IN_REMOTE) += env_remote.o
+COBJS-$(CONFIG_ENV_IS_IN_UBI) += env_ubi.o
 COBJS-$(CONFIG_ENV_IS_NOWHERE) += env_nowhere.o
 
 # command
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index fab8694..afa128e 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -62,9 +62,10 @@ DECLARE_GLOBAL_DATA_PTR;
 	!defined(CONFIG_ENV_IS_IN_ONENAND)	&& \
 	!defined(CONFIG_ENV_IS_IN_SPI_FLASH)	&& \
 	!defined(CONFIG_ENV_IS_IN_REMOTE)	&& \
+	!defined(CONFIG_ENV_IS_IN_UBI)		&& \
 	!defined(CONFIG_ENV_IS_NOWHERE)
 # error Define one of CONFIG_ENV_IS_IN_{EEPROM|FLASH|DATAFLASH|ONENAND|\
-SPI_FLASH|NVRAM|MMC|FAT|REMOTE} or CONFIG_ENV_IS_NOWHERE
+SPI_FLASH|NVRAM|MMC|FAT|REMOTE|UBI} or CONFIG_ENV_IS_NOWHERE
 #endif
 
 /*
diff --git a/common/env_ubi.c b/common/env_ubi.c
new file mode 100644
index 0000000..0c592a6
--- /dev/null
+++ b/common/env_ubi.c
@@ -0,0 +1,103 @@
+/*
+ * (c) Copyright 2012 by National Instruments,
+ *        Joe Hershberger <joe.hershberger@ni.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+
+#include <command.h>
+#include <environment.h>
+#include <errno.h>
+#include <malloc.h>
+#include <search.h>
+#include <ubi_uboot.h>
+#undef crc32
+
+char *env_name_spec = "UBI";
+
+env_t *env_ptr;
+
+DECLARE_GLOBAL_DATA_PTR;
+
+int env_init(void)
+{
+	/* use default */
+	gd->env_addr = (ulong)&default_environment[0];
+	gd->env_valid = 1;
+
+	return 0;
+}
+
+#ifdef CONFIG_CMD_SAVEENV
+int saveenv(void)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(env_t, env_new, 1);
+	ssize_t	len;
+	char *res;
+
+	res = (char *)&env_new->data;
+	len = hexport_r(&env_htab, '\0', 0, &res, ENV_SIZE, 0, NULL);
+	if (len < 0) {
+		error("Cannot export environment: errno = %d\n", errno);
+		return 1;
+	}
+
+	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
+		printf("\n** Cannot find mtd partition \"%s\"\n",
+		       CONFIG_ENV_UBI_PART);
+		return 1;
+	}
+
+	env_new->crc = crc32(0, env_new->data, ENV_SIZE);
+
+	if (ubi_volume_write(CONFIG_ENV_UBI_VOLUME, (void *)env_new,
+			     CONFIG_ENV_SIZE)) {
+		printf("\n** Unable to write env to %s:%s **\n",
+		       CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME);
+		return 1;
+	}
+
+	puts("done\n");
+	return 0;
+}
+#endif /* CONFIG_CMD_SAVEENV */
+
+void env_relocate_spec(void)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE);
+
+	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
+		printf("\n** Cannot find mtd partition \"%s\"\n",
+		       CONFIG_ENV_UBI_PART);
+		set_default_env(NULL);
+		return;
+	}
+
+	if (ubi_volume_read(CONFIG_ENV_UBI_VOLUME, (void *)&buf,
+			    CONFIG_ENV_SIZE)) {
+		printf("\n** Unable to read env from %s:%s **\n",
+		       CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME);
+		set_default_env(NULL);
+		return;
+	}
+
+	env_import(buf, 1);
+}
diff --git a/include/environment.h b/include/environment.h
index e64b43d..ece073f 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -96,6 +96,21 @@ extern unsigned long nand_env_oob_offset;
 # endif
 #endif /* CONFIG_ENV_IS_IN_NAND */
 
+#if defined(CONFIG_ENV_IS_IN_UBI)
+# ifndef CONFIG_ENV_UBI_PART
+#  error "Need to define CONFIG_ENV_UBI_PART when using CONFIG_ENV_IS_IN_UBI"
+# endif
+# ifndef CONFIG_ENV_UBI_VOLUME
+#  error "Need to define CONFIG_ENV_UBI_VOLUME when using CONFIG_ENV_IS_IN_UBI"
+# endif
+# ifndef CONFIG_ENV_SIZE
+#  error "Need to define CONFIG_ENV_SIZE when using CONFIG_ENV_IS_IN_UBI"
+# endif
+# ifndef CONFIG_CMD_UBI
+#  error "Need to define CONFIG_CMD_UBI when using CONFIG_ENV_IS_IN_UBI"
+# endif
+#endif /* CONFIG_ENV_IS_IN_UBI */
+
 /* Embedded env is only supported for some flash types */
 #ifdef CONFIG_ENV_IS_EMBEDDED
 # if	!defined(CONFIG_ENV_IS_IN_FLASH)	&& \
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index bf30234..0dee682 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -968,7 +968,8 @@ static int flash_read (int fd)
 		}
 		if (mtdinfo.type != MTD_NORFLASH &&
 		    mtdinfo.type != MTD_NANDFLASH &&
-		    mtdinfo.type != MTD_DATAFLASH) {
+		    mtdinfo.type != MTD_DATAFLASH &&
+		    mtdinfo.type != MTD_UBIVOLUME) {
 			fprintf (stderr, "Unsupported flash type %u on %s\n",
 				 mtdinfo.type, DEVNAME(dev_current));
 			return -1;
-- 
1.7.11.5

[U-Boot] [PATCH v3 7/7] env: Add redundant env support to UBI env

[Joe Hershberger]

Allow the user to specify two UBI volumes to use for the environment

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
Changes in v3: None
Changes in v2:
- Added curly braces on "multi-line" statements

 README                |   6 +++
 common/env_ubi.c      | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/environment.h |   3 ++
 tools/env/fw_env.c    |   3 ++
 4 files changed, 129 insertions(+)

diff --git a/README b/README
index 37ef694..598b674 100644
--- a/README
+++ b/README
@@ -3540,6 +3540,12 @@ but it can not erase, write this NOR flash by SRIO or PCIE interface.
 	  Define this to the name of the volume that you want to store the
 	  environment in.
 
+	- CONFIG_ENV_UBI_VOLUME_REDUND:
+
+	  Define this to the name of another volume to store a second copy of
+	  the environment in.  This will enable redundant environments in UBI.
+	  It is assumed that both volumes are in the same MTD partition.
+
 	- CONFIG_UBI_SILENCE_MSG
 	- CONFIG_UBIFS_SILENCE_MSG
 
diff --git a/common/env_ubi.c b/common/env_ubi.c
index 0c592a6..1ed8b02 100644
--- a/common/env_ubi.c
+++ b/common/env_ubi.c
@@ -47,6 +47,58 @@ int env_init(void)
 }
 
 #ifdef CONFIG_CMD_SAVEENV
+#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
+static unsigned char env_flags;
+
+int saveenv(void)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(env_t, env_new, 1);
+	ssize_t	len;
+	char *res;
+
+	res = (char *)&env_new->data;
+	len = hexport_r(&env_htab, '\0', 0, &res, ENV_SIZE, 0, NULL);
+	if (len < 0) {
+		error("Cannot export environment: errno = %d\n", errno);
+		return 1;
+	}
+
+	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
+		printf("\n** Cannot find mtd partition \"%s\"\n",
+		       CONFIG_ENV_UBI_PART);
+		return 1;
+	}
+
+	env_new->crc = crc32(0, env_new->data, ENV_SIZE);
+	env_new->flags = ++env_flags; /* increase the serial */
+
+	if (gd->env_valid == 1) {
+		puts("Writing to redundant UBI... ");
+		if (ubi_volume_write(CONFIG_ENV_UBI_VOLUME_REDUND,
+				     (void *)env_new, CONFIG_ENV_SIZE)) {
+			printf("\n** Unable to write env to %s:%s **\n",
+			       CONFIG_ENV_UBI_PART,
+			       CONFIG_ENV_UBI_VOLUME_REDUND);
+			return 1;
+		}
+	} else {
+		puts("Writing to UBI... ");
+		if (ubi_volume_write(CONFIG_ENV_UBI_VOLUME,
+				     (void *)env_new, CONFIG_ENV_SIZE)) {
+			printf("\n** Unable to write env to %s:%s **\n",
+			       CONFIG_ENV_UBI_PART,
+			       CONFIG_ENV_UBI_VOLUME);
+			return 1;
+		}
+	}
+
+	puts("done\n");
+
+	gd->env_valid = gd->env_valid == 2 ? 1 : 2;
+
+	return 0;
+}
+#else /* ! CONFIG_SYS_REDUNDAND_ENVIRONMENT */
 int saveenv(void)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(env_t, env_new, 1);
@@ -78,8 +130,72 @@ int saveenv(void)
 	puts("done\n");
 	return 0;
 }
+#endif /* CONFIG_SYS_REDUNDAND_ENVIRONMENT */
 #endif /* CONFIG_CMD_SAVEENV */
 
+#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
+void env_relocate_spec(void)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(char, env1_buf, CONFIG_ENV_SIZE);
+	ALLOC_CACHE_ALIGN_BUFFER(char, env2_buf, CONFIG_ENV_SIZE);
+	int crc1_ok = 0, crc2_ok = 0;
+	env_t *ep, *tmp_env1, *tmp_env2;
+
+	tmp_env1 = (env_t *)env1_buf;
+	tmp_env2 = (env_t *)env2_buf;
+
+	if (ubi_part(CONFIG_ENV_UBI_PART, NULL)) {
+		printf("\n** Cannot find mtd partition \"%s\"\n",
+		       CONFIG_ENV_UBI_PART);
+		set_default_env(NULL);
+		return;
+	}
+
+	if (ubi_volume_read(CONFIG_ENV_UBI_VOLUME, (void *)tmp_env1,
+			    CONFIG_ENV_SIZE)) {
+		printf("\n** Unable to read env from %s:%s **\n",
+		       CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME);
+	}
+
+	if (ubi_volume_read(CONFIG_ENV_UBI_VOLUME_REDUND, (void *)tmp_env2,
+			    CONFIG_ENV_SIZE)) {
+		printf("\n** Unable to read redundant env from %s:%s **\n",
+		       CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME_REDUND);
+	}
+
+	crc1_ok = crc32(0, tmp_env1->data, ENV_SIZE) == tmp_env1->crc;
+	crc2_ok = crc32(0, tmp_env2->data, ENV_SIZE) == tmp_env2->crc;
+
+	if (!crc1_ok && !crc2_ok) {
+		set_default_env("!bad CRC");
+		return;
+	} else if (crc1_ok && !crc2_ok) {
+		gd->env_valid = 1;
+	} else if (!crc1_ok && crc2_ok) {
+		gd->env_valid = 2;
+	} else {
+		/* both ok - check serial */
+		if (tmp_env1->flags == 255 && tmp_env2->flags == 0)
+			gd->env_valid = 2;
+		else if (tmp_env2->flags == 255 && tmp_env1->flags == 0)
+			gd->env_valid = 1;
+		else if (tmp_env1->flags > tmp_env2->flags)
+			gd->env_valid = 1;
+		else if (tmp_env2->flags > tmp_env1->flags)
+			gd->env_valid = 2;
+		else /* flags are equal - almost impossible */
+			gd->env_valid = 1;
+	}
+
+	if (gd->env_valid == 1)
+		ep = tmp_env1;
+	else
+		ep = tmp_env2;
+
+	env_flags = ep->flags;
+	env_import((char *)ep, 0);
+}
+#else /* ! CONFIG_SYS_REDUNDAND_ENVIRONMENT */
 void env_relocate_spec(void)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE);
@@ -101,3 +217,4 @@ void env_relocate_spec(void)
 
 	env_import(buf, 1);
 }
+#endif /* CONFIG_SYS_REDUNDAND_ENVIRONMENT */
diff --git a/include/environment.h b/include/environment.h
index ece073f..4c6a37b 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -103,6 +103,9 @@ extern unsigned long nand_env_oob_offset;
 # ifndef CONFIG_ENV_UBI_VOLUME
 #  error "Need to define CONFIG_ENV_UBI_VOLUME when using CONFIG_ENV_IS_IN_UBI"
 # endif
+# if defined(CONFIG_ENV_UBI_VOLUME_REDUND)
+#  define CONFIG_SYS_REDUNDAND_ENVIRONMENT
+# endif
 # ifndef CONFIG_ENV_SIZE
 #  error "Need to define CONFIG_ENV_SIZE when using CONFIG_ENV_IS_IN_UBI"
 # endif
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 0dee682..01fc1d4 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -1149,6 +1149,9 @@ int fw_env_open(void)
 		} else if (DEVTYPE(dev_current) == MTD_DATAFLASH &&
 			   DEVTYPE(!dev_current) == MTD_DATAFLASH) {
 			environment.flag_scheme = FLAG_BOOLEAN;
+		} else if (DEVTYPE(dev_current) == MTD_UBIVOLUME &&
+			   DEVTYPE(!dev_current) == MTD_UBIVOLUME) {
+			environment.flag_scheme = FLAG_INCREMENTAL;
 		} else {
 			fprintf (stderr, "Incompatible flash types!\n");
 			return -1;
-- 
1.7.11.5

[U-Boot] [PATCH v3 0/7] Add support for using an UBI volume for environment

[Tom Rini]

On Mon, Apr 08, 2013 at 03:32:45PM -0500, Joe Hershberger wrote:

> NAND is not good at handling absolute addresses to sectors for storing
> particular data.  The current implementation of the NAND env support
> works around this in several ways such as storing a pointer to the
> sector in the OOB of the first sector (interferes with some CRC) or
> supporting a range of sectors (which unless it is huge is not
> guaranteed to be safe).  None of these options address wear-leveling
> concerns or bad block handling.
> 
> Accessing the u-boot env from UBI eliminates these concerns.  However,
> it does require some of the basic settings for finding the UBI env to
> be in the default u-boot env.
> 
> Changes in v3:
> - Added documentation for UBI and UBIFS to README
> - Changed the silence to opt-in and added the options to README
> - Added comment to README about using _SILENCE_MSG options
> 
> Changes in v2:
> - Fixed error handling bug that prevents fail-over to default env on error
> - Added curly braces on "multi-line" statements
> - Added extern consistently in header
> - Cleaned up the msg print silencing
> - Added curly braces on "multi-line" statements
> 
> Joe Hershberger (7):
>   ubi: Fix broken cleanup code in attach_by_scanning
>   ubi: Expose a few simple functions from the cmd_ubi
>   ubi: ubifs: Add documentation for README
>   ubi: ubifs: Turn off verbose prints
>   mtd: Make mtdparts work with pre-reloc env
>   env: Add support for UBI environment
>   env: Add redundant env support to UBI env

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130411/bda7e537/attachment.pgp>

[U-Boot] [PATCH v3 0/7] Add support for using an UBI volume for environment

[Stefan Roese]

Hi Joe,

On 12.04.2013 00:26, Tom Rini wrote:
> On Mon, Apr 08, 2013 at 03:32:45PM -0500, Joe Hershberger wrote:
> 
>> NAND is not good at handling absolute addresses to sectors for storing
>> particular data.  The current implementation of the NAND env support
>> works around this in several ways such as storing a pointer to the
>> sector in the OOB of the first sector (interferes with some CRC) or
>> supporting a range of sectors (which unless it is huge is not
>> guaranteed to be safe).  None of these options address wear-leveling
>> concerns or bad block handling.
>>
>> Accessing the u-boot env from UBI eliminates these concerns.  However,
>> it does require some of the basic settings for finding the UBI env to
>> be in the default u-boot env.

One question: Do you plan to support the fw_env Linux tools with this
env-in-UBI feature as well?

Thanks,
Stefan

[U-Boot] [PATCH v3 0/7] Add support for using an UBI volume for environment

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/12/2013 02:19 AM, Stefan Roese wrote:
> Hi Joe,
> 
> On 12.04.2013 00:26, Tom Rini wrote:
>> On Mon, Apr 08, 2013 at 03:32:45PM -0500, Joe Hershberger wrote:
>> 
>>> NAND is not good at handling absolute addresses to sectors for
>>> storing particular data.  The current implementation of the
>>> NAND env support works around this in several ways such as
>>> storing a pointer to the sector in the OOB of the first sector
>>> (interferes with some CRC) or supporting a range of sectors
>>> (which unless it is huge is not guaranteed to be safe).  None
>>> of these options address wear-leveling concerns or bad block
>>> handling.
>>> 
>>> Accessing the u-boot env from UBI eliminates these concerns.
>>> However, it does require some of the basic settings for finding
>>> the UBI env to be in the default u-boot env.
> 
> One question: Do you plan to support the fw_env Linux tools with
> this env-in-UBI feature as well?

That is what the 'gluebi' comment is about.  It could however stand to
be documented more explicitly in the README or similar.  You use
CONFIG_MTD_UBI_GLUEBI in the kernel to allow you to map UBI volumes
(which the env(s) are) as regular flash devices.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRZ/BQAAoJENk4IS6UOR1WVRMP/iCs2le379RI3hjHl96pf9dg
owK56SjtQ1pXAnF15fWYbecMPmkbkqseV3Jcstq0XUvEtCrn6YoLnUMp6Pzm2Gfp
yn5ylFI5ofbHhtvj5l350exsV41lTIEb4qeGXmqsenfJdsxo7ddbhXgs+VULJ3eb
DdBEmCv86LAxmhe4Te7g+/JReF6c6wfgV6vMibR54KI5coRHf/MKxhIO0/d+puGL
V35pcH3lTD3iddhXxPqa5mF8EHHCRed35D/uVkdTKoOJLaIjgsHdHSgvnLnKBoyl
swLq82Et2jKUVf1n/co/fi1WC4ymZ+RaOV/qUT5ca+brzsAg9Es0l+41frkKNBAl
sA4sA6TuFszfNe/JdtUnPnUh3xDA9ruPPeuU3ZotQs81So4zPHapc9dRNR+chZcL
gYHPKoiec/7m/t9ccDILz53OKfCwj7IGEK6Z9oFUn1Hq6pP21UybGTB8wSIUMpXr
py9tmhLCSThMzzqLeo09iUvqOgwKmW9oXH1SNZV/LsK7Xlna1qNZSo/uabuDID/n
Y57XLuUqzvtrK0FMtixA/yfufAWwTpky+0UAGxDqAIjK/9JMZjIgMDbkWs30znh+
t4SQnaf5yoafGHHmabZo2NBAIRQc9FCjMXxfVGF0EW1diw6VZt17z4psemoKKz18
7obHBEsHfMBa455Iwzi5
=woZY
-----END PGP SIGNATURE-----

[U-Boot] [PATCH v3 0/7] Add support for using an UBI volume for environment

[Joe Hershberger]

Hi Stefan,

On Fri, Apr 12, 2013 at 6:30 AM, Tom Rini <trini@ti.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 04/12/2013 02:19 AM, Stefan Roese wrote:
>> Hi Joe,
>>
>> On 12.04.2013 00:26, Tom Rini wrote:
>>> On Mon, Apr 08, 2013 at 03:32:45PM -0500, Joe Hershberger wrote:
>>>
>>>> NAND is not good at handling absolute addresses to sectors for
>>>> storing particular data.  The current implementation of the
>>>> NAND env support works around this in several ways such as
>>>> storing a pointer to the sector in the OOB of the first sector
>>>> (interferes with some CRC) or supporting a range of sectors
>>>> (which unless it is huge is not guaranteed to be safe).  None
>>>> of these options address wear-leveling concerns or bad block
>>>> handling.
>>>>
>>>> Accessing the u-boot env from UBI eliminates these concerns.
>>>> However, it does require some of the basic settings for finding
>>>> the UBI env to be in the default u-boot env.
>>
>> One question: Do you plan to support the fw_env Linux tools with
>> this env-in-UBI feature as well?
>
> That is what the 'gluebi' comment is about.  It could however stand to
> be documented more explicitly in the README or similar.  You use
> CONFIG_MTD_UBI_GLUEBI in the kernel to allow you to map UBI volumes
> (which the env(s) are) as regular flash devices.

Sorry that wasn't clearer.  In the last two patches you can see where
I have to add a few changes to the fw_setenv to make it function
properly on the GLUEBI mtd devices.

I guess I didn't add much to the READMEs about it.  I'll try to
remember to push a patch to comment on it more there.

Cheers,
-Joe

[U-Boot] [PATCH v3 0/7] Add support for using an UBI volume for environment

[Stefan Roese]

Hi Joe,

On 12.04.2013 14:52, Joe Hershberger wrote:
> Sorry that wasn't clearer.  In the last two patches you can see where
> I have to add a few changes to the fw_setenv to make it function
> properly on the GLUEBI mtd devices.
> 
> I guess I didn't add much to the READMEs about it.  I'll try to
> remember to push a patch to comment on it more there.

That would be great. Thanks.

Cheers,
Stefan