[U-Boot] [PATCH v4 2/2] cmd: Add dtimg command

[Roman Stratiienko]

On Tue, Nov 12, 2019 at 8:18 PM Eugeniu Rosca <erosca at de.adit-jv.com> wrote:
>
> Hello Sam,
>
> On Thu, Aug 16, 2018 at 11:34:13PM +0300, Sam Protsenko wrote:
> > dtimg command allows user to work with Android DTB/DTBO image format.
> > Such as, getting the address of desired DTB/DTBO file, printing the dump
> > of the image in U-Boot shell, etc.
> >
> > This command is needed to provide Android boot with new Android DT image
> > format further.
> >
> > Signed-off-by: Sam Protsenko <semen.protsenko at linaro.org>
> > Reviewed-by: Tom Rini <trini at konsulko.com>
>
> [..]
>
> > +U_BOOT_CMD(
> > +     dtimg, CONFIG_SYS_MAXARGS, 0, do_dtimg,
> > +     "manipulate dtb/dtbo Android image",
> > +     "dump <addr>\n"
> > +     "    - parse specified image and print its structure info\n"
> > +     "      <addr>: image address in RAM, in hex\n"
> > +     "dtimg start <addr> <index> <varname>\n"
> > +     "    - get address (hex) of FDT in the image, by index\n"
> > +     "      <addr>: image address in RAM, in hex\n"
> > +     "      <index>: index of desired FDT in the image\n"
> > +     "      <varname>: name of variable where to store address of FDT\n"
> > +     "dtimg size <addr> <index> <varname>\n"
> > +     "    - get size (hex, bytes) of FDT in the image, by index\n"
> > +     "      <addr>: image address in RAM, in hex\n"
> > +     "      <index>: index of desired FDT in the image\n"
> > +     "      <varname>: name of variable where to store size of FDT"
> > +);
>
> Since you are the author and the main stakeholder of "dtimg", could you
> kindly feedback the command usage you envision for getting the start and
> size of dtb/dtbo blob given a certain "id" and "rev" fields used by
> mkdtboimg.py [1] and visible in the output of U-Boot's "dtimg dump" [2]?
>
> One option would be to extend the existing "dtimg {start|size}" to
> accept an argument like "id:<val>" and "rev:<val>".
>
> Another possibility is to create brand new dtimg sub-command.
> What would be your preference? TIA.
>
> [1] https://android.googlesource.com/platform/system/libufdt/+/master/utils/src/mkdtboimg.py
> [2] https://gitlab.denx.de/u-boot/u-boot/commit/e63bf1b13b3a7a

Let me add some background information to clarify why new command was suggested.
We came up with this during brainstorming on what is the best way to
implement lookup fdt by id and rev fields.

First suggestion was to implement separate lookup command, e.g.:

--> dtimg lookup id:<board_id> <dt_image_index_variable>

Second one was to integrate it into existing start/size command to
make command look more natural:

--> dtimg start|size <addr> [<index>|id:<id>] <varname>

Then after some time I suggested to combine 'start/size' subcommands
into single 'range' subcommand:

--> dtimg range <addr> id:<id> [rev:<rev>] [<start_varname> [<size_varname>]]
--> dtimg range <addr> index:<index> [<start_varname> [<size_varname>]]

Benefits of such combining:
- Reduce chance of human mistake in between of this commands (for
example different values for start and size).
- Increase readability and slightly reduce parsing time.

Downsides:
This solution is a little revolutionary since it requires to start
long-term deprecation process of start/size subcommands.

So the main question to the community is next:
Are we ready to make long term deprecation in the name of benefits
mentioned above?

In case not - we have no other choice but to extend existing
start/size subcommands.

-- 
Best regards,
Roman Stratiienko
Global Logic Inc.