[U-Boot] [PATCH 0/3] Boot from the bootable paritions
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(-)
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)" );
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
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 */
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 " \
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
participants (3)
-
Hans de Goede -
Sjoerd Simons -
Stephen Warren