[U-Boot] [PATCH v4 0/2] Extend 'part' cmd and C API to get part info

This patch-series extends `part` cmd and disk/part C API for getting partition info by it's name:
1. part "number" sub-command serves for getting the partition index from partition name. Also it can be used to test the existence of specified partition.
2. Introduces part_get_info_by_dev_and_name_or_num() function which allows us to get partition info from its number or name. Partition of interest is specified by string like "device_num:partition_number" or "device_num#partition_name".
Initially these patches were a part of "android: implement A/B boot process" [1] patch-series, but then due to multiple requests decided to send it separately, while the work on A/B series is still in progress.
Use case:
For example, in Linaro Lab 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}
[1] https://patchwork.ozlabs.org/cover/1044152/
Ruslan Trofymenko (2): cmd: part: Add 'number' sub-command disk: part: Extend API to get partition info
cmd/part.c | 16 +++++++++++- disk/part.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/part.h | 21 ++++++++++++++++ 3 files changed, 104 insertions(+), 1 deletion(-)
-- 2.17.1

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.
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 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" );

On Fri, Jun 14, 2019 at 4:04 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.
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 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
Hello Igor,
It would be nice if you would address Eugeniy Rosca comment ( https://patchwork.ozlabs.org/patch/1044151/ ), and rename 'number' to 'index'
Anyway, thank you for the update,

Hi Roman,
On Fri, Jun 14, 2019 at 4:26 PM Roman Stratiienko roman.stratiienko@globallogic.com wrote:
On Fri, Jun 14, 2019 at 4:04 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.
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 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
Hello Igor,
It would be nice if you would address Eugeniy Rosca comment ( https://patchwork.ozlabs.org/patch/1044151/ ), and rename 'number' to 'index'
Anyway, thank you for the update,
-- Best regards, Roman Stratiienko
There is already pending discussion about re-naming "number" to "index" [1] The main point is that it's already a long-established expression, which is used everywhere within cmd/part.c and disk/part.c, so before introducing new term "index", previous instances of "number" should be replaces with "index". So renaming "number" to "index" within this patch-set will just create more confusion, IMHO.
[1] https://patchwork.ozlabs.org/patch/1115517/

Hi Roman,
On Fri, Jun 14, 2019 at 3:26 PM Roman Stratiienko roman.stratiienko@globallogic.com wrote:
On Fri, Jun 14, 2019 at 4:04 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
[..]
CMD_PART_INFO_NUMBER
[..]
"part number <interface> <dev> <part> <varname>\n"
[..]
Hello Igor,
It would be nice if you would address Eugeniy Rosca comment ( https://patchwork.ozlabs.org/patch/1044151/ ), and rename 'number' to 'index'
I believe the author and reviewers lean towards "partition number" instead of "partition index" (even if the two are used intermixed in the patch). I won't argue against that. Thanks for your comment.

From: Ruslan Trofymenko ruslan.trofymenko@linaro.org
This patch adds part_get_info_by_dev_and_name_or_num() function which allows us to get partition info from its number or name. Partition of interest is specified by string like "device_num:partition_number" or "device_num#partition_name".
The patch was extracted from [1].
[1] https://android-review.googlesource.com/c/platform/external/u-boot/+/729880/...
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 v4: None Changes in v3: None Changes in v2: * Error codes are changed to -EINVAL instead of -1
disk/part.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/part.h | 21 ++++++++++++++++ 2 files changed, 89 insertions(+)
diff --git a/disk/part.c b/disk/part.c index 98cc54db20..580604c2bb 100644 --- a/disk/part.c +++ b/disk/part.c @@ -675,6 +675,74 @@ int part_get_info_by_name(struct blk_desc *dev_desc, const char *name, return part_get_info_by_name_type(dev_desc, name, info, PART_TYPE_ALL); }
+/** + * Get partition info from device number and partition name. + * + * Parse a device number and partition name string in the form of + * "device_num#partition_name", for example "0#misc". If the partition + * is found, sets dev_desc and part_info accordingly with the information + * of the partition with the given partition_name. + * + * @param[in] dev_iface Device interface + * @param[in] dev_part_str Input string argument, like "0#misc" + * @param[out] dev_desc Place to store the device description pointer + * @param[out] part_info Place to store the partition information + * @return 0 on success, or a negative on error + */ +static int part_get_info_by_dev_and_name(const char *dev_iface, + const char *dev_part_str, + struct blk_desc **dev_desc, + disk_partition_t *part_info) +{ + char *ep; + const char *part_str; + int dev_num; + + part_str = strchr(dev_part_str, '#'); + if (!part_str || part_str == dev_part_str) + return -EINVAL; + + dev_num = simple_strtoul(dev_part_str, &ep, 16); + if (ep != part_str) { + /* Not all the first part before the # was parsed. */ + return -EINVAL; + } + part_str++; + + *dev_desc = blk_get_dev(dev_iface, dev_num); + if (!*dev_desc) { + printf("Could not find %s %d\n", dev_iface, dev_num); + return -EINVAL; + } + if (part_get_info_by_name(*dev_desc, part_str, part_info) < 0) { + printf("Could not find "%s" partition\n", part_str); + return -EINVAL; + } + return 0; +} + +int part_get_info_by_dev_and_name_or_num(const char *dev_iface, + const char *dev_part_str, + struct blk_desc **dev_desc, + disk_partition_t *part_info) +{ + /* Split the part_name if passed as "$dev_num#part_name". */ + if (!part_get_info_by_dev_and_name(dev_iface, dev_part_str, + dev_desc, part_info)) + return 0; + /* + * Couldn't lookup by name, try looking up the partition description + * directly. + */ + if (blk_get_device_part_str(dev_iface, dev_part_str, + dev_desc, part_info, 1) < 0) { + printf("Couldn't find partition %s %s\n", + dev_iface, dev_part_str); + return -EINVAL; + } + return 0; +} + void part_set_generic_name(const struct blk_desc *dev_desc, int part_num, char *name) { diff --git a/include/part.h b/include/part.h index ebca546db5..0b5cf3d5e8 100644 --- a/include/part.h +++ b/include/part.h @@ -201,6 +201,27 @@ int part_get_info_by_name_type(struct blk_desc *dev_desc, const char *name, int part_get_info_by_name(struct blk_desc *dev_desc, const char *name, disk_partition_t *info);
+/** + * Get partition info from dev number + part name, or dev number + part number. + * + * Parse a device number and partition description (either name or number) + * in the form of device number plus partition name separated by a "#" + * (like "device_num#partition_name") or a device number plus a partition number + * separated by a ":". For example both "0#misc" and "0:1" can be valid + * partition descriptions for a given interface. If the partition is found, sets + * dev_desc and part_info accordingly with the information of the partition. + * + * @param[in] dev_iface Device interface + * @param[in] dev_part_str Input partition description, like "0#misc" or "0:1" + * @param[out] dev_desc Place to store the device description pointer + * @param[out] part_info Place to store the partition information + * @return 0 on success, or a negative on error + */ +int part_get_info_by_dev_and_name_or_num(const char *dev_iface, + const char *dev_part_str, + struct blk_desc **dev_desc, + disk_partition_t *part_info); + /** * part_set_generic_name() - create generic partition like hda1 or sdb2 *

Please ignore this patch-set, "disk: part: Extend API to get partition info" won't be used and was sent by by confusion. I'll re-send a single "cmd: part: Add 'number' sub-command" as v5.
On Fri, Jun 14, 2019 at 4:04 PM Igor Opaniuk igor.opaniuk@gmail.com wrote:
This patch-series extends `part` cmd and disk/part C API for getting partition info by it's name:
- part "number" sub-command serves for getting the partition index from
partition name. Also it can be used to test the existence of specified partition.
- Introduces part_get_info_by_dev_and_name_or_num() function which
allows us to get partition info from its number or name. Partition of interest is specified by string like "device_num:partition_number" or "device_num#partition_name".
Initially these patches were a part of "android: implement A/B boot process" [1] patch-series, but then due to multiple requests decided to send it separately, while the work on A/B series is still in progress.
Use case:
For example, in Linaro Lab 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}
[1] https://patchwork.ozlabs.org/cover/1044152/
Ruslan Trofymenko (2): cmd: part: Add 'number' sub-command disk: part: Extend API to get partition info
cmd/part.c | 16 +++++++++++- disk/part.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/part.h | 21 ++++++++++++++++ 3 files changed, 104 insertions(+), 1 deletion(-)
-- 2.17.1
participants (3)
-
Eugeniu Rosca
-
Igor Opaniuk
-
Roman Stratiienko