diff mbox series

[3/3] cmd: adtimg: Refactor usage style

Message ID 20191224165108.2836-4-erosca@de.adit-jv.com
State Accepted
Commit 4f731c795d75aee548f6886f4f0da0a49a99354d
Delegated to: Tom Rini
Headers show
Series cmd: dtimg: Rename to adtimg and refactor usage style | expand

Commit Message

Eugeniu Rosca Dec. 24, 2019, 4:51 p.m. UTC
Trying to extend 'adtimg' functionality [1], we've been severely hit
by a major limitation in the command's usage scheme. Specifically, the
command's user interface appears to be too centric to getting the
DTB/DTBO entry [3] based on the index of the desired DT in the image,
which makes it really difficult retrieving the DT entry based on
alternative criteria (e.g. filtering by id/rev fields), the
latter being demanded by real life customer use-cases [1].

This went to the point of receiving below feedback from Sam [2]:

 -- snip --
 As for 'dtimg' command: after giving it some thought, I think not much
 people using it yet. So in this particular case I don't have some
 strong preference, and if you think the 'dtimg' interface is ugly, and
 it overcomes "don't break interfaces" rule, maybe now is a good time
 to rework it (before it gets widely used).
 -- snip --

Given the above, rework the usage pattern from [4] to [5], in order to
allow an intuitive enablement of "by id|rev" DT search [6].

[1] https://patchwork.ozlabs.org/cover/1202575/
    ("cmd: dtimg: Enhance with --id and --rev options (take #1)")
[2] https://patchwork.ozlabs.org/patch/1182207/#2317020
[3] https://source.android.com/devices/architecture/dto/partitions
[4] Old usage
adtimg dump <addr>                    - Print image contents
adtimg start <addr> <index> <varname> - Get DT address by index
adtimg size <addr> <index> <varname>  - Get DT size by index

[5] New usage
adtimg addr <addr>                      - Set image location to <addr>
adtimg dump                             - Print out image contents
adtimg get dt --index=<i> [avar [svar]] - Get DT address and size by index

[6] Soon-to-be-provided "by id|rev" add-on functionality
adtimg get dt --id=<id> --rev=<rev> [avar [svar [ivar]]]
 - Get DT address/size/index by id|rev fields

Cc: Sam Protsenko <semen.protsenko@linaro.org>
Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
---
 cmd/adtimg.c | 217 +++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 158 insertions(+), 59 deletions(-)

Comments

Simon Glass Jan. 8, 2020, 5:39 p.m. UTC | #1
Hi Eugeniu,

On Tue, 24 Dec 2019 at 09:52, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote:
>
> Trying to extend 'adtimg' functionality [1], we've been severely hit
> by a major limitation in the command's usage scheme. Specifically, the
> command's user interface appears to be too centric to getting the
> DTB/DTBO entry [3] based on the index of the desired DT in the image,
> which makes it really difficult retrieving the DT entry based on
> alternative criteria (e.g. filtering by id/rev fields), the
> latter being demanded by real life customer use-cases [1].
>
> This went to the point of receiving below feedback from Sam [2]:
>
>  -- snip --
>  As for 'dtimg' command: after giving it some thought, I think not much
>  people using it yet. So in this particular case I don't have some
>  strong preference, and if you think the 'dtimg' interface is ugly, and
>  it overcomes "don't break interfaces" rule, maybe now is a good time
>  to rework it (before it gets widely used).
>  -- snip --
>
> Given the above, rework the usage pattern from [4] to [5], in order to
> allow an intuitive enablement of "by id|rev" DT search [6].
>
> [1] https://patchwork.ozlabs.org/cover/1202575/
>     ("cmd: dtimg: Enhance with --id and --rev options (take #1)")
> [2] https://patchwork.ozlabs.org/patch/1182207/#2317020
> [3] https://source.android.com/devices/architecture/dto/partitions
> [4] Old usage
> adtimg dump <addr>                    - Print image contents
> adtimg start <addr> <index> <varname> - Get DT address by index
> adtimg size <addr> <index> <varname>  - Get DT size by index
>
> [5] New usage
> adtimg addr <addr>                      - Set image location to <addr>
> adtimg dump                             - Print out image contents
> adtimg get dt --index=<i> [avar [svar]] - Get DT address and size by index
>
> [6] Soon-to-be-provided "by id|rev" add-on functionality
> adtimg get dt --id=<id> --rev=<rev> [avar [svar [ivar]]]
>  - Get DT address/size/index by id|rev fields
>
> Cc: Sam Protsenko <semen.protsenko@linaro.org>
> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> ---
>  cmd/adtimg.c | 217 +++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 158 insertions(+), 59 deletions(-)

Can you please add a test for this command?

Regards,
Simon
Eugeniu Rosca Jan. 8, 2020, 6:12 p.m. UTC | #2
Hi Simon,

On Wed, Jan 08, 2020 at 10:39:34AM -0700, Simon Glass wrote:
> On Tue, 24 Dec 2019 at 09:52, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote:
> > [5] New usage
> > adtimg addr <addr>                      - Set image location to <addr>
> > adtimg dump                             - Print out image contents
> > adtimg get dt --index=<i> [avar [svar]] - Get DT address and size by index
> >
> > [6] Soon-to-be-provided "by id|rev" add-on functionality
> > adtimg get dt --id=<id> --rev=<rev> [avar [svar [ivar]]]
> >  - Get DT address/size/index by id|rev fields
> >
> > Cc: Sam Protsenko <semen.protsenko@linaro.org>
> > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> > ---
> >  cmd/adtimg.c | 217 +++++++++++++++++++++++++++++++++++++--------------
> >  1 file changed, 158 insertions(+), 59 deletions(-)
> 
> Can you please add a test for this command?

Many thanks for the inputs. Two questions:

 - The binary which adtimg operates on is generated by means of a host
   tooling [1] which is actively developed and hence continuously
   incorporates new features. Only the recent versions of [1]
   (obsoleting Debian packages like [2]) may be used to generate a
   valid test image for the adtimg U-Boot command. I think Sam
   found an elegant solution in [3] to make the hex dump of the
   test image part of the test itself, as opposed to below:
   - require the users to install the correct tool version on the host,
   - embed the required tool version into U-Boot and track its version,
   I plan to go the same route and just want to make sure we all agree
   on the approach just described.

 - Since this series is already reviewed, are you fine if the test is
   submitted in a follow-up series, accompanied by a number of new
   adtimg features sitting in my queue?

[1] https://android.googlesource.com/platform/system/tools/mkbootimg/
[2] https://packages.debian.org/sid/android-tools-mkbootimg
[3] https://patchwork.ozlabs.org/patch/1215287/
Tom Rini Jan. 10, 2020, 9:50 p.m. UTC | #3
On Tue, Dec 24, 2019 at 05:51:08PM +0100, Eugeniu Rosca wrote:

> Trying to extend 'adtimg' functionality [1], we've been severely hit
> by a major limitation in the command's usage scheme. Specifically, the
> command's user interface appears to be too centric to getting the
> DTB/DTBO entry [3] based on the index of the desired DT in the image,
> which makes it really difficult retrieving the DT entry based on
> alternative criteria (e.g. filtering by id/rev fields), the
> latter being demanded by real life customer use-cases [1].
> 
> This went to the point of receiving below feedback from Sam [2]:
> 
>  -- snip --
>  As for 'dtimg' command: after giving it some thought, I think not much
>  people using it yet. So in this particular case I don't have some
>  strong preference, and if you think the 'dtimg' interface is ugly, and
>  it overcomes "don't break interfaces" rule, maybe now is a good time
>  to rework it (before it gets widely used).
>  -- snip --
> 
> Given the above, rework the usage pattern from [4] to [5], in order to
> allow an intuitive enablement of "by id|rev" DT search [6].
> 
> [1] https://patchwork.ozlabs.org/cover/1202575/
>     ("cmd: dtimg: Enhance with --id and --rev options (take #1)")
> [2] https://patchwork.ozlabs.org/patch/1182207/#2317020
> [3] https://source.android.com/devices/architecture/dto/partitions
> [4] Old usage
> adtimg dump <addr>                    - Print image contents
> adtimg start <addr> <index> <varname> - Get DT address by index
> adtimg size <addr> <index> <varname>  - Get DT size by index
> 
> [5] New usage
> adtimg addr <addr>                      - Set image location to <addr>
> adtimg dump                             - Print out image contents
> adtimg get dt --index=<i> [avar [svar]] - Get DT address and size by index
> 
> [6] Soon-to-be-provided "by id|rev" add-on functionality
> adtimg get dt --id=<id> --rev=<rev> [avar [svar [ivar]]]
>  - Get DT address/size/index by id|rev fields
> 
> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>

Applied to u-boot/master, thanks!
Eugeniu Rosca Jan. 10, 2020, 11:29 p.m. UTC | #4
Hi Tom,

On Fri, Jan 10, 2020 at 04:50:52PM -0500, Tom Rini wrote:
> On Tue, Dec 24, 2019 at 05:51:08PM +0100, Eugeniu Rosca wrote:
> > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> > Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
> 
> Applied to u-boot/master, thanks!

Thanks for merging!
Patrick DELAUNAY July 3, 2020, 4:40 p.m. UTC | #5
Hi Eugeniu,

> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Eugeniu Rosca
> Sent: samedi 11 janvier 2020 00:30
> 
> Hi Tom,
> 
> On Fri, Jan 10, 2020 at 04:50:52PM -0500, Tom Rini wrote:
> > On Tue, Dec 24, 2019 at 05:51:08PM +0100, Eugeniu Rosca wrote:
> > > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> > > Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
> >
> > Applied to u-boot/master, thanks!
> 
> Thanks for merging!

For the planned update of the command adtimg:

> [6] Soon-to-be-provided "by id|rev" add-on functionality adtimg get dt 
> --id=<id> --rev=<rev> [avar [svar [ivar]]]
>  - Get DT address/size/index by id|rev fields

Do you have plan to upstream this code or it is available  in downstream ?

Because stm32mp1 platform needs this feature

We code in downstream [1] based on v2020.01 almost the same command
but based on the old usage = [2]

	dtimg getindex <addr> <board_id> <board_rev> [varname] 
	    - get index of FDT in the image, by board identifier and revision
	      <addr>: image address in RAM, in hex
	      <board_id>: board identifier
	      <board_rev>: board revision (0 if not used)
	      [varname]: name of variable where to store index of FDT

So if you have nothing ready, I can update my code with your proposed parameters.

[1] https://github.com/STMicroelectronics/u-boot/blob/v2020.01-stm32mp/cmd/dtimg.c
[2] https://github.com/STMicroelectronics/u-boot/commit/21b57a166e73a8457d4caea6a1d37f1f3fda3d45


> --
> Best Regards,
> Eugeniu

Best Regards

Patrick
Eugeniu Rosca July 6, 2020, 6:46 a.m. UTC | #6
Hi Patrick,

On Fri, Jul 03, 2020 at 04:40:28PM +0000, Patrick DELAUNAY wrote:
> > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Eugeniu Rosca
> > Sent: samedi 11 janvier 2020 00:30
> > 
> > Hi Tom,
> > 
> > On Fri, Jan 10, 2020 at 04:50:52PM -0500, Tom Rini wrote:
> > > On Tue, Dec 24, 2019 at 05:51:08PM +0100, Eugeniu Rosca wrote:
> > > > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> > > > Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
> > >
> > > Applied to u-boot/master, thanks!
> > 
> > Thanks for merging!
> 
> For the planned update of the command adtimg:
> 
> > [6] Soon-to-be-provided "by id|rev" add-on functionality adtimg get dt 
> > --id=<id> --rev=<rev> [avar [svar [ivar]]]
> >  - Get DT address/size/index by id|rev fields
> 
> Do you have plan to upstream this code or it is available  in downstream ?
> 
> Because stm32mp1 platform needs this feature

Good to hear that the features are used by people out there.

> We code in downstream [1] based on v2020.01 almost the same command
> but based on the old usage = [2]

After a quick look, this is certainly missing the latest vanilla
patches, so is out of sync with mainline.

> 	dtimg getindex <addr> <board_id> <board_rev> [varname] 
> 	    - get index of FDT in the image, by board identifier and revision
> 	      <addr>: image address in RAM, in hex
> 	      <board_id>: board identifier
> 	      <board_rev>: board revision (0 if not used)
> 	      [varname]: name of variable where to store index of FDT
> 
> So if you have nothing ready, I can update my code with your proposed parameters.
> 
> [1] https://github.com/STMicroelectronics/u-boot/blob/v2020.01-stm32mp/cmd/dtimg.c
> [2] https://github.com/STMicroelectronics/u-boot/commit/21b57a166e73a8457d4caea6a1d37f1f3fda3d45

You would need to rebase these commits first.
A quicker route is to just take my patches available below:
 https://github.com/erosca/u-boot/commits/master_adtimg_id_rev

These have been extensively tested and are now running in
development/production systems for months w/o any issues.

Do you have any comments concerning the two commits below?
 https://github.com/erosca/u-boot/commit/936a08faaf3f485527e2d1cfb8a53dcf44e17b3a
 https://github.com/erosca/u-boot/commit/e37d090dabb6973762376adca5ae10292ca3ba3d
Patrick DELAUNAY July 8, 2020, 5:26 p.m. UTC | #7
Thanks,

> From: Eugeniu Rosca <erosca@de.adit-jv.com>
> Sent: lundi 6 juillet 2020 08:47
> 
> Hi Patrick,
> 
> On Fri, Jul 03, 2020 at 04:40:28PM +0000, Patrick DELAUNAY wrote:
> > > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Eugeniu
> > > Rosca
> > > Sent: samedi 11 janvier 2020 00:30
> > >
> > > Hi Tom,
> > >
> > > On Fri, Jan 10, 2020 at 04:50:52PM -0500, Tom Rini wrote:
> > > > On Tue, Dec 24, 2019 at 05:51:08PM +0100, Eugeniu Rosca wrote:
> > > > > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> > > > > Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
> > > >
> > > > Applied to u-boot/master, thanks!
> > >
> > > Thanks for merging!
> >
> > For the planned update of the command adtimg:
> >
> > > [6] Soon-to-be-provided "by id|rev" add-on functionality adtimg get
> > > dt --id=<id> --rev=<rev> [avar [svar [ivar]]]
> > >  - Get DT address/size/index by id|rev fields
> >
> > Do you have plan to upstream this code or it is available  in downstream ?
> >
> > Because stm32mp1 platform needs this feature
> 
> Good to hear that the features are used by people out there.
> 
> > We code in downstream [1] based on v2020.01 almost the same command
> > but based on the old usage = [2]
> 
> After a quick look, this is certainly missing the latest vanilla patches, so is out of
> sync with mainline.
>
> > 	dtimg getindex <addr> <board_id> <board_rev> [varname]
> > 	    - get index of FDT in the image, by board identifier and revision
> > 	      <addr>: image address in RAM, in hex
> > 	      <board_id>: board identifier
> > 	      <board_rev>: board revision (0 if not used)
> > 	      [varname]: name of variable where to store index of FDT
> >
> > So if you have nothing ready, I can update my code with your proposed
> parameters.
> >
> > [1]
> > https://github.com/STMicroelectronics/u-boot/blob/v2020.01-stm32mp/cmd
> > /dtimg.c [2]
> > https://github.com/STMicroelectronics/u-boot/commit/21b57a166e73a8457d
> > 4caea6a1d37f1f3fda3d45
> 
> You would need to rebase these commits first.
> A quicker route is to just take my patches available below:
>  https://github.com/erosca/u-boot/commits/master_adtimg_id_rev
> 
> These have been extensively tested and are now running in
> development/production systems for months w/o any issues.
> 
> Do you have any comments concerning the two commits below?
>  https://github.com/erosca/u-
> boot/commit/936a08faaf3f485527e2d1cfb8a53dcf44e17b3a
>  https://github.com/erosca/u-
> boot/commit/e37d090dabb6973762376adca5ae10292ca3ba3d

I check the 2 patches, they seems OK.

I just surprized by the '--' for the command parameter
because it is unusual in U-Boot commands.

It wasn't enough to have .....<parameter>=<val> as other command (unzip for example)
but it is perhaps to late to change again the commands adtimg and it is aligned with "abootimg.c"

For information, I will activate the command ADTIMG for stm32mp32 [1] 
and update the stm32mp1 downstream android bootcmd to use when they will be available.

But I have don't yet tested them

We do you plan to rebase and upstream these 2 commits ?

[1] = http://patchwork.ozlabs.org/project/uboot/patch/20200706130046.22446-1-patrick.delaunay@st.com/


> --
> Best regards,
> Eugeniu Rosca

Best regards,

Patrick
diff mbox series

Patch

diff --git a/cmd/adtimg.c b/cmd/adtimg.c
index 22b4f5e1a83f..60bb01c3498a 100644
--- a/cmd/adtimg.c
+++ b/cmd/adtimg.c
@@ -2,18 +2,22 @@ 
 /*
  * (C) Copyright 2018 Linaro Ltd.
  * Sam Protsenko <semen.protsenko@linaro.org>
+ * Eugeniu Rosca <rosca.eugeniu@gmail.com>
  */
 
 #include <env.h>
 #include <image-android-dt.h>
 #include <common.h>
 
-enum cmd_adtimg_info {
-	CMD_ADTIMG_START = 0,
-	CMD_ADTIMG_SIZE,
-};
+#define OPT_INDEX	"--index"
 
-static int do_adtimg_dump(cmd_tbl_t *cmdtp, int flag, int argc,
+/*
+ * Current/working DTB/DTBO Android image address.
+ * Similar to 'working_fdt' variable in 'fdt' command.
+ */
+static ulong working_img;
+
+static int do_adtimg_addr(cmd_tbl_t *cmdtp, int flag, int argc,
 			  char * const argv[])
 {
 	char *endp;
@@ -24,86 +28,185 @@  static int do_adtimg_dump(cmd_tbl_t *cmdtp, int flag, int argc,
 
 	hdr_addr = simple_strtoul(argv[1], &endp, 16);
 	if (*endp != '\0') {
-		printf("Error: Wrong image address\n");
+		printf("Error: Wrong image address '%s'\n", argv[1]);
 		return CMD_RET_FAILURE;
 	}
 
-	if (!android_dt_check_header(hdr_addr)) {
-		printf("Error: DT image header is incorrect\n");
+	/*
+	 * Allow users to set an address prior to copying the DTB/DTBO
+	 * image to that same address, i.e. skip header verification.
+	 */
+
+	working_img = hdr_addr;
+	return CMD_RET_SUCCESS;
+}
+
+static int adtimg_check_working_img(void)
+{
+	if (!working_img) {
+		printf("Error: Please, call 'adtimg addr <addr>'. Aborting!\n");
 		return CMD_RET_FAILURE;
 	}
 
-	android_dt_print_contents(hdr_addr);
+	if (!android_dt_check_header(working_img)) {
+		printf("Error: Invalid image header at 0x%lx\n", working_img);
+		return CMD_RET_FAILURE;
+	}
 
 	return CMD_RET_SUCCESS;
 }
 
-static int adtimg_get_fdt(int argc, char * const argv[],
-			  enum cmd_adtimg_info cmd)
+static int do_adtimg_dump(cmd_tbl_t *cmdtp, int flag, int argc,
+			  char * const argv[])
 {
-	ulong hdr_addr;
-	u32 index;
-	char *endp;
-	ulong fdt_addr;
-	u32 fdt_size;
-	char buf[65];
-
-	if (argc != 4)
+	if (argc != 1)
 		return CMD_RET_USAGE;
 
-	hdr_addr = simple_strtoul(argv[1], &endp, 16);
-	if (*endp != '\0') {
-		printf("Error: Wrong image address\n");
+	if (adtimg_check_working_img() != CMD_RET_SUCCESS)
+		return CMD_RET_FAILURE;
+
+	android_dt_print_contents(working_img);
+
+	return CMD_RET_SUCCESS;
+}
+
+static int adtimg_getopt_u32(char * const opt, char * const name, u32 *optval)
+{
+	char *endp, *str;
+	u32 val;
+
+	if (!opt || !name || !optval)
+		return CMD_RET_FAILURE;
+
+	str = strchr(opt, '=');
+	if (!str) {
+		printf("Error: Option '%s' not followed by '='\n", name);
 		return CMD_RET_FAILURE;
 	}
 
-	if (!android_dt_check_header(hdr_addr)) {
-		printf("Error: DT image header is incorrect\n");
+	if (*++str == '\0') {
+		printf("Error: Option '%s=' not followed by value\n", name);
 		return CMD_RET_FAILURE;
 	}
 
-	index = simple_strtoul(argv[2], &endp, 0);
+	val = simple_strtoul(str, &endp, 0);
 	if (*endp != '\0') {
-		printf("Error: Wrong index\n");
+		printf("Error: Wrong integer value '%s=%s'\n", name, str);
 		return CMD_RET_FAILURE;
 	}
 
-	if (!android_dt_get_fdt_by_index(hdr_addr, index, &fdt_addr, &fdt_size))
+	*optval = val;
+	return CMD_RET_SUCCESS;
+}
+
+static int adtimg_getopt_index(int argc, char * const argv[], u32 *index,
+			       char **avar, char **svar)
+{
+	int ret;
+
+	if (!argv || !avar || !svar)
 		return CMD_RET_FAILURE;
 
-	switch (cmd) {
-	case CMD_ADTIMG_START:
-		snprintf(buf, sizeof(buf), "%lx", fdt_addr);
-		break;
-	case CMD_ADTIMG_SIZE:
-		snprintf(buf, sizeof(buf), "%x", fdt_size);
-		break;
-	default:
-		printf("Error: Unknown cmd_adtimg_info value: %d\n", cmd);
+	if (argc > 3) {
+		printf("Error: Unexpected argument '%s'\n", argv[3]);
 		return CMD_RET_FAILURE;
 	}
 
-	env_set(argv[3], buf);
+	ret = adtimg_getopt_u32(argv[0], OPT_INDEX, index);
+	if (ret != CMD_RET_SUCCESS)
+		return ret;
+
+	if (argc > 1)
+		*avar = argv[1];
+	if (argc > 2)
+		*svar = argv[2];
 
 	return CMD_RET_SUCCESS;
 }
 
-static int do_adtimg_start(cmd_tbl_t *cmdtp, int flag, int argc,
-			   char * const argv[])
+static int adtimg_get_dt_by_index(int argc, char * const argv[])
 {
-	return adtimg_get_fdt(argc, argv, CMD_ADTIMG_START);
+	ulong addr;
+	u32 index, size;
+	int ret;
+	char *avar = NULL, *svar = NULL;
+
+	ret = adtimg_getopt_index(argc, argv, &index, &avar, &svar);
+	if (ret != CMD_RET_SUCCESS)
+		return ret;
+
+	if (!android_dt_get_fdt_by_index(working_img, index, &addr, &size))
+		return CMD_RET_FAILURE;
+
+	if (avar && svar) {
+		ret = env_set_hex(avar, addr);
+		if (ret) {
+			printf("Error: Can't set '%s' to 0x%lx\n", avar, addr);
+			return CMD_RET_FAILURE;
+		}
+		ret = env_set_hex(svar, size);
+		if (ret) {
+			printf("Error: Can't set '%s' to 0x%x\n", svar, size);
+			return CMD_RET_FAILURE;
+		}
+	} else if (avar) {
+		ret = env_set_hex(avar, addr);
+		if (ret) {
+			printf("Error: Can't set '%s' to 0x%lx\n", avar, addr);
+			return CMD_RET_FAILURE;
+		}
+		printf("0x%x (%d)\n", size, size);
+	} else {
+		printf("0x%lx, 0x%x (%d)\n", addr, size, size);
+	}
+
+	return CMD_RET_SUCCESS;
 }
 
-static int do_adtimg_size(cmd_tbl_t *cmdtp, int flag, int argc,
-			  char * const argv[])
+static int adtimg_get_dt(int argc, char * const argv[])
 {
-	return adtimg_get_fdt(argc, argv, CMD_ADTIMG_SIZE);
+	if (argc < 2) {
+		printf("Error: No options passed to '%s'\n", argv[0]);
+		return CMD_RET_FAILURE;
+	}
+
+	/* Strip off leading 'dt' command argument */
+	argc--;
+	argv++;
+
+	if (!strncmp(argv[0], OPT_INDEX, sizeof(OPT_INDEX) - 1))
+		return adtimg_get_dt_by_index(argc, argv);
+
+	printf("Error: Option '%s' not supported\n", argv[0]);
+	return CMD_RET_FAILURE;
+}
+
+static int do_adtimg_get(cmd_tbl_t *cmdtp, int flag, int argc,
+			 char * const argv[])
+{
+	if (argc < 2) {
+		printf("Error: No arguments passed to '%s'\n", argv[0]);
+		return CMD_RET_FAILURE;
+	}
+
+	if (adtimg_check_working_img() != CMD_RET_SUCCESS)
+		return CMD_RET_FAILURE;
+
+	/* Strip off leading 'get' command argument */
+	argc--;
+	argv++;
+
+	if (!strcmp(argv[0], "dt"))
+		return adtimg_get_dt(argc, argv);
+
+	printf("Error: Wrong argument '%s'\n", argv[0]);
+	return CMD_RET_FAILURE;
 }
 
 static cmd_tbl_t cmd_adtimg_sub[] = {
-	U_BOOT_CMD_MKENT(dump, 2, 0, do_adtimg_dump, "", ""),
-	U_BOOT_CMD_MKENT(start, 4, 0, do_adtimg_start, "", ""),
-	U_BOOT_CMD_MKENT(size, 4, 0, do_adtimg_size, "", ""),
+	U_BOOT_CMD_MKENT(addr, CONFIG_SYS_MAXARGS, 1, do_adtimg_addr, "", ""),
+	U_BOOT_CMD_MKENT(dump, CONFIG_SYS_MAXARGS, 1, do_adtimg_dump, "", ""),
+	U_BOOT_CMD_MKENT(get, CONFIG_SYS_MAXARGS, 1, do_adtimg_get, "", ""),
 };
 
 static int do_adtimg(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
@@ -127,17 +230,13 @@  static int do_adtimg(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 U_BOOT_CMD(
 	adtimg, CONFIG_SYS_MAXARGS, 0, do_adtimg,
 	"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"
-	"adtimg 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"
-	"adtimg 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"
+	"addr <addr> - Set image location to <addr>\n"
+	"adtimg dump        - Print out image contents\n"
+	"adtimg get dt --index=<index> [avar [svar]]         - Get DT address/size by index\n"
+	"\n"
+	"Legend:\n"
+	"  - <addr>: DTB/DTBO image address (hex) in RAM\n"
+	"  - <index>: index (hex/dec) of desired DT in the image\n"
+	"  - <avar>: variable name to contain DT address (hex)\n"
+	"  - <svar>: variable name to contain DT size (hex)"
 );