[U-Boot] [PATCH 0/3] Boot from the bootable paritions

[U-Boot] [PATCH 0/3] Boot from the bootable paritions

[Sjoerd Simons]

In the discussion after the submission of the "Let the distro boot command scan
all partitions" patchset it became clear the general consensus was that
u-boot should not simply scan all partitions but instead only partitions with
the bootable flag set, falling back to parition 1 as was previously done.

This set of patches implements that


Sjoerd Simons (3):
  part: Add support for list filtering on bootable partitions
  config_cmd_default.h: Add 'env exists' command
  config_distro_bootcmd.h: Prefer booting from bootable paritions

 common/cmd_part.c               | 48 ++++++++++++++++++++++++++++++-----------
 include/config_cmd_default.h    |  1 +
 include/config_distro_bootcmd.h |  3 ++-
 3 files changed, 39 insertions(+), 13 deletions(-)

-- 
2.1.4

[U-Boot] [PATCH 1/3] part: Add support for list filtering on bootable partitions

[Sjoerd Simons]

Add an optional -bootable parameter to the part list commands to only
put the list of bootable partitions in the environment variable

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
---
 common/cmd_part.c | 48 ++++++++++++++++++++++++++++++++++++------------
 1 file changed, 36 insertions(+), 12 deletions(-)

diff --git a/common/cmd_part.c b/common/cmd_part.c
index c99f527..e1fe641 100644
--- a/common/cmd_part.c
+++ b/common/cmd_part.c
@@ -53,29 +53,52 @@ static int do_part_list(int argc, char * const argv[])
 {
 	int ret;
 	block_dev_desc_t *desc;
+	char *var = NULL;
+	bool bootable = false;
+	int i;
 
-	if (argc < 2 || argc > 3)
+	if (argc < 2)
 		return CMD_RET_USAGE;
 
+	for (i = 2; i < argc; i++) {
+		if (argv[i][0] == '-') {
+			if (!strcmp(argv[i], "-bootable")) {
+				bootable = true;
+			} else {
+				printf("Unknown option %s\n", argv[i]);
+				return CMD_RET_USAGE;
+			}
+		} else if (var == NULL) {
+			var = argv[i];
+		} else {
+			printf("duplicated varname\n");
+			return CMD_RET_USAGE;
+		}
+	}
+
 	ret = get_device(argv[0], argv[1], &desc);
 	if (ret < 0)
 		return 1;
 
-	if (argc == 3) {
+	if (var != NULL) {
 		int p;
-		char str[512] = { 0, };
+		char str[512] = { '\0', };
 	  disk_partition_t info;
 
 		for (p = 1; p < 128; p++) {
+			char t[5];
 			int r = get_partition_info(desc, p, &info);
 
-			if (r == 0) {
-				char t[5];
-				sprintf(t, "%s%d", str[0] ? " " : "", p);
-				strcat(str, t);
-			}
+			if (r != 0)
+				continue;
+
+			if (bootable && !info.bootable)
+				continue;
+
+			sprintf(t, "%s%d", str[0] ? " " : "", p);
+			strcat(str, t);
 		}
-		setenv(argv[2], str);
+		setenv(var, str);
 		return 0;
 	}
 
@@ -98,7 +121,7 @@ static int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 }
 
 U_BOOT_CMD(
-	part,	5,	1,	do_part,
+	part,	CONFIG_SYS_MAXARGS,	1,	do_part,
 	"disk partition related commands",
 	"part uuid <interface> <dev>:<part>\n"
 	"    - print partition UUID\n"
@@ -106,6 +129,7 @@ U_BOOT_CMD(
 	"    - set environment variable to partition UUID\n"
 	"part list <interface> <dev>\n"
 	"    - print a device's partition table\n"
-	"part list <interface> <dev> <varname>\n"
-	"    - set environment variable to the list of partitions"
+	"part list <interface> <dev> [flags] <varname>\n"
+	"    - set environment variable to the list of partitions\n"
+	"      flags can be -bootable (list only bootable partitions)"
 );
-- 
2.1.4

[U-Boot] [PATCH 2/3] config_cmd_default.h: Add 'env exists' command

[Sjoerd Simons]

env exists allows scripts to query whether an environment variable
exists. Enable by default as it adds only a trivial amount of code and
can be useful in scripts.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
---
 include/config_cmd_default.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/config_cmd_default.h b/include/config_cmd_default.h
index 73c9544..e79a13b 100644
--- a/include/config_cmd_default.h
+++ b/include/config_cmd_default.h
@@ -21,6 +21,7 @@
 #define CONFIG_CMD_CONSOLE	/* coninfo			*/
 #define CONFIG_CMD_ECHO		/* echo arguments		*/
 #define CONFIG_CMD_EDITENV	/* editenv			*/
+#define CONFIG_CMD_ENV_EXISTS	/* query whether env variables exists */
 #define CONFIG_CMD_FPGA		/* FPGA configuration Support	*/
 #define CONFIG_CMD_IMI		/* iminfo			*/
 #define CONFIG_CMD_ITEST	/* Integer (and string) test	*/
-- 
2.1.4

[U-Boot] [PATCH 3/3] config_distro_bootcmd.h: Prefer booting from bootable paritions

[Sjoerd Simons]

List bootable partitions and only scan those for bootable files, falling
back to partition 1 if there are no bootable partitions

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
---
 include/config_distro_bootcmd.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
index 07a0b3b..ad2dda1 100644
--- a/include/config_distro_bootcmd.h
+++ b/include/config_distro_bootcmd.h
@@ -197,7 +197,8 @@
 		"done\0"                                                  \
 	\
 	"scan_dev_for_boot_part="                                         \
-		"part list ${devtype} ${devnum} devplist; "               \
+		"part list ${devtype} ${devnum} -bootable devplist; "     \
+		"env exists devplist || setenv devplist 1; "              \
 		"for bootpart in ${devplist}; do "                        \
 			"if fstype ${devtype} ${devnum}:${bootpart} "     \
 					"bootfstype; then "               \
-- 
2.1.4

[U-Boot] [PATCH 0/3] Boot from the bootable paritions

[Hans de Goede]

Hi,

On 19-02-15 21:53, Sjoerd Simons wrote:
> In the discussion after the submission of the "Let the distro boot command scan
> all partitions" patchset it became clear the general consensus was that
> u-boot should not simply scan all partitions but instead only partitions with
> the bootable flag set, falling back to parition 1 as was previously done.
>
> This set of patches implements that

Thanks for working on this. Since the initial scan all partitions patch-set
is in v2015.04, I believe that we should add this one to v2015.04 too, so
that we do not change behavior after the official v2015.04 release.

So I've taken the liberty to review this, to expedite the process of getting
this upstream. All 3 patches look good to me and are:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

[U-Boot] [PATCH 1/3] part: Add support for list filtering on bootable partitions

[Stephen Warren]

On 02/19/2015 01:53 PM, Sjoerd Simons wrote:
> Add an optional -bootable parameter to the part list commands to only
> put the list of bootable partitions in the environment variable

> diff --git a/common/cmd_part.c b/common/cmd_part.c

> +	for (i = 2; i < argc; i++) {
> +		if (argv[i][0] == '-') {
> +			if (!strcmp(argv[i], "-bootable")) {
> +				bootable = true;
> +			} else {
> +				printf("Unknown option %s\n", argv[i]);
> +				return CMD_RET_USAGE;
> +			}
> +		} else if (var == NULL) {
> +			var = argv[i];
> +		} else {
> +			printf("duplicated varname\n");
> +			return CMD_RET_USAGE;
> +		}
> +	}

I'd prefer that to validate the command-line doesn't have multiple 
variable names, or the variable name specified before the -bootable 
flag, so that only the following options are valid:

... var
... -bootable var

and not:

... var1 var2
... var -bootable

etc.

This could be tightened up later I suppose. Other than that, this series 
looks good at a quick glance, so:
Acked-by: Stephen Warren <swarren@nvidia.com>