[U-Boot] [PATCH v5 1/1] cmd: part: Add 'number' sub-command

From: Ruslan Trofymenko ruslan.trofymenko@linaro.org
This sub-command serves for getting the partition index from partition name. Also it can be used to test the existence of specified partition.
Use case: For example, in most CI environments this U-Boot command for automatic testing of Linux rootfs is used:
=> setenv bootpart 1:f
where 0xf is "userdata" partition. But the number of "userdata" partition can be changed any time, when partition table is changed.
So it would be nice to get rid of that 0xf magic number and use partition name instead, like this:
=> part number mmc 1 userdata part_num => setenv bootpart 1:${part_num}
Signed-off-by: Ruslan Trofymenko ruslan.trofymenko@linaro.org Signed-off-by: Igor Opaniuk igor.opaniuk@gmail.com Reviewed-by: Alistair Strachan astrachan@google.com Reviewed-by: Sam Protsenko semen.protsenko@linaro.org Reviewed-by: Simon Glass sjg@chromium.org ---
Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: None
cmd/part.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/cmd/part.c b/cmd/part.c index bfb6488b0f..653e13ced1 100644 --- a/cmd/part.c +++ b/cmd/part.c @@ -24,6 +24,7 @@ enum cmd_part_info { CMD_PART_INFO_START = 0, CMD_PART_INFO_SIZE, + CMD_PART_INFO_NUMBER };
static int do_part_uuid(int argc, char * const argv[]) @@ -149,6 +150,9 @@ static int do_part_info(int argc, char * const argv[], enum cmd_part_info param) case CMD_PART_INFO_SIZE: snprintf(buf, sizeof(buf), LBAF, info.size); break; + case CMD_PART_INFO_NUMBER: + snprintf(buf, sizeof(buf), "%d", part); + break; default: printf("** Unknown cmd_part_info value: %d\n", param); return 1; @@ -172,6 +176,11 @@ static int do_part_size(int argc, char * const argv[]) return do_part_info(argc, argv, CMD_PART_INFO_SIZE); }
+static int do_part_number(int argc, char * const argv[]) +{ + return do_part_info(argc, argv, CMD_PART_INFO_NUMBER); +} + static int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { if (argc < 2) @@ -185,6 +194,8 @@ static int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return do_part_start(argc - 2, argv + 2); else if (!strcmp(argv[1], "size")) return do_part_size(argc - 2, argv + 2); + else if (!strcmp(argv[1], "number")) + return do_part_number(argc - 2, argv + 2);
return CMD_RET_USAGE; } @@ -206,5 +217,8 @@ U_BOOT_CMD( " 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)\n" - " part can be either partition number or partition name" + " part can be either partition number or partition name\n" + "part number <interface> <dev> <part> <varname>\n" + " - set environment variable to the partition number using the partition name\n" + " part must be specified as partition name" );

Hi Igor,
I believe it's common sense, best practice and it is generally polite to include in CC anybody who contributed with review comments during the previous patch iterations. Otherwise, it raises questions, mistrust and all kind of weird assumptions, which may hurt the community in the long run. I believe the beauty of community-driven development is being straight to each other and keeping things transparent. So, if you don't mind, please include me in CC if I've already commented on your patches.

Hi Eugeniu,
On Fri, Jun 14, 2019 at 5:08 PM Eugeniu Rosca roscaeugeniu@gmail.com wrote:
Hi Igor,
I believe it's common sense, best practice and it is generally polite to include in CC anybody who contributed with review comments during the previous patch iterations. Otherwise, it raises questions, mistrust and all kind of weird assumptions, which may hurt the community in the long run. I believe the beauty of community-driven development is being straight to each other and keeping things transparent. So, if you don't mind, please include me in CC if I've already commented on your patches.
-- Best regards, Eugeniu.
Sorry, it's just my inattention (Friday evening, you know :) ), I've copied-pasted (yes, I still can't force myself to use patman and somehow automate this process) the list of people from CC headers from v3 patch [1], so please don't take it personally.
Thanks for all your effort and reviews of my patches, I really appreciate it.
[1] https://patchwork.ozlabs.org/patch/1044151/

Hi Igor,
Once again:
Reviewed-by: Sam Protsenko semen.protsenko@linaro.org
Thanks!
On Fri, Jun 14, 2019 at 5:01 PM Igor Opaniuk igor.opaniuk@gmail.com wrote:
From: Ruslan Trofymenko ruslan.trofymenko@linaro.org
This sub-command serves for getting the partition index from partition name. Also it can be used to test the existence of specified partition.
Use case: For example, in most CI environments this U-Boot command for automatic testing of Linux rootfs is used:
=> setenv bootpart 1:f
where 0xf is "userdata" partition. But the number of "userdata" partition can be changed any time, when partition table is changed.
So it would be nice to get rid of that 0xf magic number and use partition name instead, like this:
=> part number mmc 1 userdata part_num => setenv bootpart 1:${part_num}
Signed-off-by: Ruslan Trofymenko ruslan.trofymenko@linaro.org Signed-off-by: Igor Opaniuk igor.opaniuk@gmail.com Reviewed-by: Alistair Strachan astrachan@google.com Reviewed-by: Sam Protsenko semen.protsenko@linaro.org Reviewed-by: Simon Glass sjg@chromium.org
Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: None
cmd/part.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/cmd/part.c b/cmd/part.c index bfb6488b0f..653e13ced1 100644 --- a/cmd/part.c +++ b/cmd/part.c @@ -24,6 +24,7 @@ enum cmd_part_info { CMD_PART_INFO_START = 0, CMD_PART_INFO_SIZE,
CMD_PART_INFO_NUMBER
};
static int do_part_uuid(int argc, char * const argv[]) @@ -149,6 +150,9 @@ static int do_part_info(int argc, char * const argv[], enum cmd_part_info param) case CMD_PART_INFO_SIZE: snprintf(buf, sizeof(buf), LBAF, info.size); break;
case CMD_PART_INFO_NUMBER:
snprintf(buf, sizeof(buf), "%d", part);
break; default: printf("** Unknown cmd_part_info value: %d\n", param); return 1;
@@ -172,6 +176,11 @@ static int do_part_size(int argc, char * const argv[]) return do_part_info(argc, argv, CMD_PART_INFO_SIZE); }
+static int do_part_number(int argc, char * const argv[]) +{
return do_part_info(argc, argv, CMD_PART_INFO_NUMBER);
+}
static int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { if (argc < 2) @@ -185,6 +194,8 @@ static int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return do_part_start(argc - 2, argv + 2); else if (!strcmp(argv[1], "size")) return do_part_size(argc - 2, argv + 2);
else if (!strcmp(argv[1], "number"))
return do_part_number(argc - 2, argv + 2); return CMD_RET_USAGE;
} @@ -206,5 +217,8 @@ U_BOOT_CMD( " 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)\n"
" part can be either partition number or partition name"
" part can be either partition number or partition name\n"
"part number <interface> <dev> <part> <varname>\n"
" - set environment variable to the partition number using the partition name\n"
" part must be specified as partition name"
);
2.17.1

Hi Tom,
On Fri, Jun 14, 2019 at 5:22 PM Sam Protsenko semen.protsenko@linaro.org wrote:
Hi Igor,
Once again:
Reviewed-by: Sam Protsenko semen.protsenko@linaro.org
Thanks!
On Fri, Jun 14, 2019 at 5:01 PM Igor Opaniuk igor.opaniuk@gmail.com wrote:
From: Ruslan Trofymenko ruslan.trofymenko@linaro.org
This sub-command serves for getting the partition index from partition name. Also it can be used to test the existence of specified partition.
Use case: For example, in most CI environments this U-Boot command for automatic testing of Linux rootfs is used:
=> setenv bootpart 1:f
where 0xf is "userdata" partition. But the number of "userdata" partition can be changed any time, when partition table is changed.
So it would be nice to get rid of that 0xf magic number and use partition name instead, like this:
=> part number mmc 1 userdata part_num => setenv bootpart 1:${part_num}
Signed-off-by: Ruslan Trofymenko ruslan.trofymenko@linaro.org Signed-off-by: Igor Opaniuk igor.opaniuk@gmail.com Reviewed-by: Alistair Strachan astrachan@google.com Reviewed-by: Sam Protsenko semen.protsenko@linaro.org Reviewed-by: Simon Glass sjg@chromium.org
Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: None
cmd/part.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/cmd/part.c b/cmd/part.c index bfb6488b0f..653e13ced1 100644 --- a/cmd/part.c +++ b/cmd/part.c @@ -24,6 +24,7 @@ enum cmd_part_info { CMD_PART_INFO_START = 0, CMD_PART_INFO_SIZE,
CMD_PART_INFO_NUMBER
};
static int do_part_uuid(int argc, char * const argv[]) @@ -149,6 +150,9 @@ static int do_part_info(int argc, char * const argv[], enum cmd_part_info param) case CMD_PART_INFO_SIZE: snprintf(buf, sizeof(buf), LBAF, info.size); break;
case CMD_PART_INFO_NUMBER:
snprintf(buf, sizeof(buf), "%d", part);
break; default: printf("** Unknown cmd_part_info value: %d\n", param); return 1;
@@ -172,6 +176,11 @@ static int do_part_size(int argc, char * const argv[]) return do_part_info(argc, argv, CMD_PART_INFO_SIZE); }
+static int do_part_number(int argc, char * const argv[]) +{
return do_part_info(argc, argv, CMD_PART_INFO_NUMBER);
+}
static int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { if (argc < 2) @@ -185,6 +194,8 @@ static int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return do_part_start(argc - 2, argv + 2); else if (!strcmp(argv[1], "size")) return do_part_size(argc - 2, argv + 2);
else if (!strcmp(argv[1], "number"))
return do_part_number(argc - 2, argv + 2); return CMD_RET_USAGE;
} @@ -206,5 +217,8 @@ U_BOOT_CMD( " 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)\n"
" part can be either partition number or partition name"
" part can be either partition number or partition name\n"
"part number <interface> <dev> <part> <varname>\n"
" - set environment variable to the partition number using the partition name\n"
" part must be specified as partition name"
);
2.17.1
Are there any objections from your side to get this patch applied? If there are any comments/suggestions, please let me know!
Thanks!

On Fri, Jul 12, 2019 at 12:25:50PM +0300, Igor Opaniuk wrote:
Hi Tom,
On Fri, Jun 14, 2019 at 5:22 PM Sam Protsenko semen.protsenko@linaro.org wrote:
Hi Igor,
Once again:
Reviewed-by: Sam Protsenko semen.protsenko@linaro.org
Thanks!
On Fri, Jun 14, 2019 at 5:01 PM Igor Opaniuk igor.opaniuk@gmail.com wrote:
From: Ruslan Trofymenko ruslan.trofymenko@linaro.org
This sub-command serves for getting the partition index from partition name. Also it can be used to test the existence of specified partition.
Use case: For example, in most CI environments this U-Boot command for automatic testing of Linux rootfs is used:
=> setenv bootpart 1:f
where 0xf is "userdata" partition. But the number of "userdata" partition can be changed any time, when partition table is changed.
So it would be nice to get rid of that 0xf magic number and use partition name instead, like this:
=> part number mmc 1 userdata part_num => setenv bootpart 1:${part_num}
Signed-off-by: Ruslan Trofymenko ruslan.trofymenko@linaro.org Signed-off-by: Igor Opaniuk igor.opaniuk@gmail.com Reviewed-by: Alistair Strachan astrachan@google.com Reviewed-by: Sam Protsenko semen.protsenko@linaro.org Reviewed-by: Simon Glass sjg@chromium.org
Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: None
cmd/part.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/cmd/part.c b/cmd/part.c index bfb6488b0f..653e13ced1 100644 --- a/cmd/part.c +++ b/cmd/part.c @@ -24,6 +24,7 @@ enum cmd_part_info { CMD_PART_INFO_START = 0, CMD_PART_INFO_SIZE,
CMD_PART_INFO_NUMBER
};
static int do_part_uuid(int argc, char * const argv[]) @@ -149,6 +150,9 @@ static int do_part_info(int argc, char * const argv[], enum cmd_part_info param) case CMD_PART_INFO_SIZE: snprintf(buf, sizeof(buf), LBAF, info.size); break;
case CMD_PART_INFO_NUMBER:
snprintf(buf, sizeof(buf), "%d", part);
break; default: printf("** Unknown cmd_part_info value: %d\n", param); return 1;
@@ -172,6 +176,11 @@ static int do_part_size(int argc, char * const argv[]) return do_part_info(argc, argv, CMD_PART_INFO_SIZE); }
+static int do_part_number(int argc, char * const argv[]) +{
return do_part_info(argc, argv, CMD_PART_INFO_NUMBER);
+}
static int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { if (argc < 2) @@ -185,6 +194,8 @@ static int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return do_part_start(argc - 2, argv + 2); else if (!strcmp(argv[1], "size")) return do_part_size(argc - 2, argv + 2);
else if (!strcmp(argv[1], "number"))
return do_part_number(argc - 2, argv + 2); return CMD_RET_USAGE;
} @@ -206,5 +217,8 @@ U_BOOT_CMD( " 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)\n"
" part can be either partition number or partition name"
" part can be either partition number or partition name\n"
"part number <interface> <dev> <part> <varname>\n"
" - set environment variable to the partition number using the partition name\n"
" part must be specified as partition name"
);
2.17.1
Are there any objections from your side to get this patch applied? If there are any comments/suggestions, please let me know!
I'll need to grab this by itself and see what the overall size growth from this code is, since it's adding new unconditional commands. While I don't think every new sub-command needs a CONFIG option, one of the common critiques of how U-Boot is going these days is that size is always growing on platforms that don't make use of this new functionality.

On Fri, Jun 14, 2019 at 05:01:26PM +0300, Igor Opaniuk wrote:
From: Ruslan Trofymenko ruslan.trofymenko@linaro.org
This sub-command serves for getting the partition index from partition name. Also it can be used to test the existence of specified partition.
Use case: For example, in most CI environments this U-Boot command for automatic testing of Linux rootfs is used:
=> setenv bootpart 1:f
where 0xf is "userdata" partition. But the number of "userdata" partition can be changed any time, when partition table is changed.
So it would be nice to get rid of that 0xf magic number and use partition name instead, like this:
=> part number mmc 1 userdata part_num => setenv bootpart 1:${part_num}
Signed-off-by: Ruslan Trofymenko ruslan.trofymenko@linaro.org Signed-off-by: Igor Opaniuk igor.opaniuk@gmail.com Reviewed-by: Alistair Strachan astrachan@google.com Reviewed-by: Sam Protsenko semen.protsenko@linaro.org Reviewed-by: Simon Glass sjg@chromium.org Reviewed-by: Sam Protsenko semen.protsenko@linaro.org
Applied to u-boot/master, thanks!
participants (4)
-
Eugeniu Rosca
-
Igor Opaniuk
-
Sam Protsenko
-
Tom Rini