| Message ID | 20190614130454.26797-2-igor.opaniuk@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Extend 'part' cmd and C API to get part info | expand |
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.
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" );