From mboxrd@z Thu Jan  1 00:00:00 1970
From: Igor Opaniuk <igor.opaniuk@gmail.com>
Date: Fri, 14 Jun 2019 16:52:59 +0300
Subject: [U-Boot] [PATCH v4 1/2] cmd: part: Add 'number' sub-command
In-Reply-To: <CAODwZ7taJ5qxJkNCWNBFChpC0oBfuq_-EF5nf=qNSCVvA5dvCA@mail.gmail.com>
References: <20190614130454.26797-1-igor.opaniuk@gmail.com>
 <20190614130454.26797-2-igor.opaniuk@gmail.com>
 <CAODwZ7taJ5qxJkNCWNBFChpC0oBfuq_-EF5nf=qNSCVvA5dvCA@mail.gmail.com>
Message-ID: <CAByghJbjDNLZcO7U4DPiA0_xCVtz+CDRyzo_t8Yn4-=uhuMa-A@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
To: u-boot@lists.denx.de

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> wro=
te:
> >
> > 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 =3D 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 arg=
v[])
> >         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 (i=
n 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 t=
he 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/

--=20
Best regards - Freundliche Gr=C3=BCsse - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk at gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk