[U-Boot] [PATCH 0/3] Provide partition names to commands in environment
This patch series makes it possible to pass partition name to "part start" and "part size" commands, and fixes corresponding magic numbers (partition numbers) in TI boot environment.
Sam Protsenko (3): cmd: part: Allow passing partition name to start and size cmd: part: Extract common code to separate function env: ti: boot: Get rid of magic numbers
cmd/part.c | 75 +++++++++++++++++++++---------------------- include/environment/ti/boot.h | 10 +++--- 2 files changed, 41 insertions(+), 44 deletions(-)
Allow passing the partition name to "part start" and "part size" commands, so we can avoid magic numbers in the environment.
Consider one real use-case: in include/environment/ti/boot.h we have commands like these:
setenv boot_part 9 part start mmc ${mmcdev} ${boot_part} boot_start part size mmc ${mmcdev} ${boot_part} boot_size mmc read ${loadaddr} ${boot_start} ${boot_size}
Now suppose that we have changed the partition table and boot_part now is 10. We will need to fix commands above. And anyone who relies on these boot commands, will need to change them accordingly, too (this was an actual case in our lab while testing Linux boot on Android environment).
By providing the option to pass partition name instead, we fix mentioned issue, by eliminating the necessity to use magic numbers.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org --- cmd/part.c | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/cmd/part.c b/cmd/part.c index 746bf40b2d..fd8825a7ec 100644 --- a/cmd/part.c +++ b/cmd/part.c @@ -113,6 +113,7 @@ static int do_part_start(int argc, char * const argv[]) struct blk_desc *desc; disk_partition_t info; char buf[512] = { 0 }; + char *endp; int part; int err; int ret; @@ -122,15 +123,20 @@ static int do_part_start(int argc, char * const argv[]) if (argc > 4) return CMD_RET_USAGE;
- part = simple_strtoul(argv[2], NULL, 0); - ret = blk_get_device_by_str(argv[0], argv[1], &desc); if (ret < 0) return 1;
- err = part_get_info(desc, part, &info); - if (err) - return 1; + part = simple_strtoul(argv[2], &endp, 0); + if (*endp == '\0') { + err = part_get_info(desc, part, &info); + if (err) + return 1; + } else { + part = part_get_info_by_name(desc, argv[2], &info); + if (part == -1) + return 1; + }
snprintf(buf, sizeof(buf), LBAF, info.start);
@@ -147,6 +153,7 @@ static int do_part_size(int argc, char * const argv[]) struct blk_desc *desc; disk_partition_t info; char buf[512] = { 0 }; + char *endp; int part; int err; int ret; @@ -156,15 +163,20 @@ static int do_part_size(int argc, char * const argv[]) if (argc > 4) return CMD_RET_USAGE;
- part = simple_strtoul(argv[2], NULL, 0); - ret = blk_get_device_by_str(argv[0], argv[1], &desc); if (ret < 0) return 1;
- err = part_get_info(desc, part, &info); - if (err) - return 1; + part = simple_strtoul(argv[2], &endp, 0); + if (*endp == '\0') { + err = part_get_info(desc, part, &info); + if (err) + return 1; + } else { + part = part_get_info_by_name(desc, argv[2], &info); + if (part == -1) + return 1; + }
snprintf(buf, sizeof(buf), LBAF, info.size);
@@ -207,6 +219,8 @@ U_BOOT_CMD( " flags can be -bootable (list only bootable partitions)\n" "part start <interface> <dev> <part> <varname>\n" " - set environment variable to the start of the partition (in blocks)\n" + " part can be either partition number or partition name\n" "part size <interface> <dev> <part> <varname>\n" - " - set environment variable to the size of the partition (in blocks)" + " - set environment variable to the size of the partition (in blocks)\n" + " part can be either partition number or partition name" );
On Mon, 26 Feb 2018 23:17:59 +0200 Sam Protsenko semen.protsenko@linaro.org wrote:
Allow passing the partition name to "part start" and "part size" commands, so we can avoid magic numbers in the environment.
Consider one real use-case: in include/environment/ti/boot.h we have commands like these:
setenv boot_part 9 part start mmc ${mmcdev} ${boot_part} boot_start part size mmc ${mmcdev} ${boot_part} boot_size mmc read ${loadaddr} ${boot_start} ${boot_size}Now suppose that we have changed the partition table and boot_part now is 10. We will need to fix commands above. And anyone who relies on these boot commands, will need to change them accordingly, too (this was an actual case in our lab while testing Linux boot on Android environment).
By providing the option to pass partition name instead, we fix mentioned issue, by eliminating the necessity to use magic numbers.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
cmd/part.c | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/cmd/part.c b/cmd/part.c index 746bf40b2d..fd8825a7ec 100644 --- a/cmd/part.c +++ b/cmd/part.c @@ -113,6 +113,7 @@ static int do_part_start(int argc, char * const argv[]) struct blk_desc *desc; disk_partition_t info; char buf[512] = { 0 };
- char *endp; int part; int err; int ret;
@@ -122,15 +123,20 @@ static int do_part_start(int argc, char * const argv[]) if (argc > 4) return CMD_RET_USAGE;
part = simple_strtoul(argv[2], NULL, 0);
ret = blk_get_device_by_str(argv[0], argv[1], &desc); if (ret < 0) return 1;
err = part_get_info(desc, part, &info);
if (err)
return 1;
part = simple_strtoul(argv[2], &endp, 0);
if (*endp == '\0') {
err = part_get_info(desc, part, &info);if (err)return 1;} else {
part = part_get_info_by_name(desc, argv[2], &info);if (part == -1)return 1;}
snprintf(buf, sizeof(buf), LBAF, info.start);
@@ -147,6 +153,7 @@ static int do_part_size(int argc, char * const argv[]) struct blk_desc *desc; disk_partition_t info; char buf[512] = { 0 };
- char *endp; int part; int err; int ret;
@@ -156,15 +163,20 @@ static int do_part_size(int argc, char * const argv[]) if (argc > 4) return CMD_RET_USAGE;
part = simple_strtoul(argv[2], NULL, 0);
ret = blk_get_device_by_str(argv[0], argv[1], &desc); if (ret < 0) return 1;
err = part_get_info(desc, part, &info);
if (err)
return 1;
part = simple_strtoul(argv[2], &endp, 0);
if (*endp == '\0') {
err = part_get_info(desc, part, &info);if (err)return 1;} else {
part = part_get_info_by_name(desc, argv[2], &info);if (part == -1)return 1;}
snprintf(buf, sizeof(buf), LBAF, info.size);
@@ -207,6 +219,8 @@ U_BOOT_CMD( " flags can be -bootable (list only bootable partitions)\n" "part start <interface> <dev> <part> <varname>\n" " - set environment variable to the start of the partition (in blocks)\n"
- " part can be either partition number or partition
name\n" "part size <interface> <dev> <part> <varname>\n"
- " - set environment variable to the size of the partition
(in blocks)"
- " - set environment variable to the size of the partition
(in blocks)\n"
- " part can be either partition number or partition name"
);
Reviewed-by: Lukasz Majewski lukma@denx.de
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
On Mon, Feb 26, 2018 at 11:17:59PM +0200, Sam Protsenko wrote:
Allow passing the partition name to "part start" and "part size" commands, so we can avoid magic numbers in the environment.
Consider one real use-case: in include/environment/ti/boot.h we have commands like these:
setenv boot_part 9 part start mmc ${mmcdev} ${boot_part} boot_start part size mmc ${mmcdev} ${boot_part} boot_size mmc read ${loadaddr} ${boot_start} ${boot_size}Now suppose that we have changed the partition table and boot_part now is 10. We will need to fix commands above. And anyone who relies on these boot commands, will need to change them accordingly, too (this was an actual case in our lab while testing Linux boot on Android environment).
By providing the option to pass partition name instead, we fix mentioned issue, by eliminating the necessity to use magic numbers.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org Reviewed-by: Lukasz Majewski lukma@denx.de
Applied to u-boot/master, thanks!
Refactor the code for "part start" and "part size" commands to avoid code duplication.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org --- cmd/part.c | 61 +++++++++++++++++++++++-------------------------------------- 1 file changed, 23 insertions(+), 38 deletions(-)
diff --git a/cmd/part.c b/cmd/part.c index fd8825a7ec..ec791fdc5d 100644 --- a/cmd/part.c +++ b/cmd/part.c @@ -22,6 +22,11 @@ #include <part.h> #include <vsprintf.h>
+enum cmd_part_info { + CMD_PART_INFO_START = 0, + CMD_PART_INFO_SIZE, +}; + static int do_part_uuid(int argc, char * const argv[]) { int part; @@ -108,7 +113,7 @@ static int do_part_list(int argc, char * const argv[]) return 0; }
-static int do_part_start(int argc, char * const argv[]) +static int do_part_info(int argc, char * const argv[], enum cmd_part_info param) { struct blk_desc *desc; disk_partition_t info; @@ -138,7 +143,17 @@ static int do_part_start(int argc, char * const argv[]) return 1; }
- snprintf(buf, sizeof(buf), LBAF, info.start); + switch (param) { + case CMD_PART_INFO_START: + snprintf(buf, sizeof(buf), LBAF, info.start); + break; + case CMD_PART_INFO_SIZE: + snprintf(buf, sizeof(buf), LBAF, info.size); + break; + default: + printf("** Unknown cmd_part_info value: %d\n", param); + return 1; + }
if (argc > 3) env_set(argv[3], buf); @@ -148,44 +163,14 @@ static int do_part_start(int argc, char * const argv[]) return 0; }
-static int do_part_size(int argc, char * const argv[]) +static int do_part_start(int argc, char * const argv[]) { - struct blk_desc *desc; - disk_partition_t info; - char buf[512] = { 0 }; - char *endp; - int part; - int err; - int ret; - - if (argc < 3) - return CMD_RET_USAGE; - if (argc > 4) - return CMD_RET_USAGE; - - ret = blk_get_device_by_str(argv[0], argv[1], &desc); - if (ret < 0) - return 1; - - part = simple_strtoul(argv[2], &endp, 0); - if (*endp == '\0') { - err = part_get_info(desc, part, &info); - if (err) - return 1; - } else { - part = part_get_info_by_name(desc, argv[2], &info); - if (part == -1) - return 1; - } - - snprintf(buf, sizeof(buf), LBAF, info.size); - - if (argc > 3) - env_set(argv[3], buf); - else - printf("%s\n", buf); + return do_part_info(argc, argv, CMD_PART_INFO_START); +}
- return 0; +static int do_part_size(int argc, char * const argv[]) +{ + return do_part_info(argc, argv, CMD_PART_INFO_SIZE); }
static int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
On Mon, 26 Feb 2018 23:18:00 +0200 Sam Protsenko semen.protsenko@linaro.org wrote:
Refactor the code for "part start" and "part size" commands to avoid code duplication.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
cmd/part.c | 61 +++++++++++++++++++++++-------------------------------------- 1 file changed, 23 insertions(+), 38 deletions(-)
diff --git a/cmd/part.c b/cmd/part.c index fd8825a7ec..ec791fdc5d 100644 --- a/cmd/part.c +++ b/cmd/part.c @@ -22,6 +22,11 @@ #include <part.h> #include <vsprintf.h>
+enum cmd_part_info {
- CMD_PART_INFO_START = 0,
- CMD_PART_INFO_SIZE,
+};
static int do_part_uuid(int argc, char * const argv[]) { int part; @@ -108,7 +113,7 @@ static int do_part_list(int argc, char * const argv[]) return 0; }
-static int do_part_start(int argc, char * const argv[]) +static int do_part_info(int argc, char * const argv[], enum cmd_part_info param) { struct blk_desc *desc; disk_partition_t info; @@ -138,7 +143,17 @@ static int do_part_start(int argc, char * const argv[]) return 1; }
- snprintf(buf, sizeof(buf), LBAF, info.start);
- switch (param) {
- case CMD_PART_INFO_START:
snprintf(buf, sizeof(buf), LBAF, info.start);break;- case CMD_PART_INFO_SIZE:
snprintf(buf, sizeof(buf), LBAF, info.size);break;- default:
printf("** Unknown cmd_part_info value: %d\n",param);
return 1;}
if (argc > 3) env_set(argv[3], buf);
@@ -148,44 +163,14 @@ static int do_part_start(int argc, char * const argv[]) return 0; }
-static int do_part_size(int argc, char * const argv[]) +static int do_part_start(int argc, char * const argv[]) {
- struct blk_desc *desc;
- disk_partition_t info;
- char buf[512] = { 0 };
- char *endp;
- int part;
- int err;
- int ret;
- if (argc < 3)
return CMD_RET_USAGE;- if (argc > 4)
return CMD_RET_USAGE;- ret = blk_get_device_by_str(argv[0], argv[1], &desc);
- if (ret < 0)
return 1;- part = simple_strtoul(argv[2], &endp, 0);
- if (*endp == '\0') {
err = part_get_info(desc, part, &info);if (err)return 1;- } else {
part = part_get_info_by_name(desc, argv[2], &info);if (part == -1)return 1;- }
- snprintf(buf, sizeof(buf), LBAF, info.size);
- if (argc > 3)
env_set(argv[3], buf);- else
printf("%s\n", buf);
- return do_part_info(argc, argv, CMD_PART_INFO_START);
+}
- return 0;
+static int do_part_size(int argc, char * const argv[]) +{
- return do_part_info(argc, argv, CMD_PART_INFO_SIZE);
}
static int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
Reviewed-by: Lukasz Majewski lukma@denx.de
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
On Mon, Feb 26, 2018 at 11:18:00PM +0200, Sam Protsenko wrote:
Refactor the code for "part start" and "part size" commands to avoid code duplication.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org Reviewed-by: Lukasz Majewski lukma@denx.de
Applied to u-boot/master, thanks!
Get the start address and the size of partitions using partition names rather than partition numbers. This way we can change the partition table further without changing the boot code.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org --- include/environment/ti/boot.h | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/include/environment/ti/boot.h b/include/environment/ti/boot.h index 0a2342061c..24b7783f88 100644 --- a/include/environment/ti/boot.h +++ b/include/environment/ti/boot.h @@ -40,15 +40,13 @@ "setenv eval_bootargs setenv bootargs $bootargs; " \ "run eval_bootargs; " \ "setenv mmcdev 1; " \ - "setenv fdt_part 3; " \ - "setenv boot_part 9; " \ "setenv machid fe6; " \ "mmc dev $mmcdev; " \ "mmc rescan; " \ - "part start mmc ${mmcdev} ${fdt_part} fdt_start; " \ - "part size mmc ${mmcdev} ${fdt_part} fdt_size; " \ - "part start mmc ${mmcdev} ${boot_part} boot_start; " \ - "part size mmc ${mmcdev} ${boot_part} boot_size; " \ + "part start mmc ${mmcdev} environment fdt_start; " \ + "part size mmc ${mmcdev} environment fdt_size; " \ + "part start mmc ${mmcdev} boot boot_start; " \ + "part size mmc ${mmcdev} boot boot_size; " \ "mmc read ${fdtaddr} ${fdt_start} ${fdt_size}; " \ "mmc read ${loadaddr} ${boot_start} ${boot_size}; " \ "bootm $loadaddr $loadaddr $fdtaddr;\0"
On Mon, Feb 26, 2018 at 11:18:01PM +0200, Sam Protsenko wrote:
Get the start address and the size of partitions using partition names rather than partition numbers. This way we can change the partition table further without changing the boot code.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
Applied to u-boot/master, thanks!
participants (3)
-
Lukasz Majewski -
Sam Protsenko -
Tom Rini