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

[Tom Rini]

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 at linaro.org> wrote:
> >
> > Hi Igor,
> >
> > Once again:
> >
> > Reviewed-by: Sam Protsenko <semen.protsenko at linaro.org>
> >
> > Thanks!
> >
> > On Fri, Jun 14, 2019 at 5:01 PM Igor Opaniuk <igor.opaniuk at gmail.com> wrote:
> > >
> > > From: Ruslan Trofymenko <ruslan.trofymenko at 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 at linaro.org>
> > > Signed-off-by: Igor Opaniuk <igor.opaniuk at gmail.com>
> > > Reviewed-by: Alistair Strachan <astrachan at google.com>
> > > Reviewed-by: Sam Protsenko <semen.protsenko at linaro.org>
> > > Reviewed-by: Simon Glass <sjg at 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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190715/ad7c9ec1/attachment.sig>