* [PATCH 0/3] cmd: dtimg: Rename to adtimg and refactor usage style
@ 2019-12-24 16:51 Eugeniu Rosca
2019-12-24 16:51 ` [PATCH 1/3] dtimg/am57xx_evm_defconfig: Rename dtimg to adtimg Eugeniu Rosca
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Eugeniu Rosca @ 2019-12-24 16:51 UTC (permalink / raw)
To: u-boot
The main focus of this series is to prepare the ground for:
- adding the id/rev-based DT lookup in dtimg/adtimg [1]
- adding the abootimg command [2]
To be more clear, the above topics are handled _outside_ of this series,
but they treat this series as hard dependency.
The per-patch motivation and testing are described meticulously
in each commit.
[1] https://patchwork.ozlabs.org/patch/1202580/
("cmd: dtimg: Get start and size based on --id and --rev")
[2] https://patchwork.ozlabs.org/patch/1182212/
("cmd: bootimg: Add bootimg command")
Eugeniu Rosca (3):
dtimg/am57xx_evm_defconfig: Rename dtimg to adtimg
cmd: adtimg: Rename internal symbols
cmd: adtimg: Refactor usage style
cmd/Kconfig | 4 +-
cmd/Makefile | 2 +-
cmd/adtimg.c | 242 ++++++++++++++++++++++++++++
cmd/dtimg.c | 142 ----------------
common/Makefile | 2 +-
configs/am57xx_evm_defconfig | 2 +-
configs/am57xx_hs_evm_defconfig | 2 +-
configs/am57xx_hs_evm_usb_defconfig | 2 +-
8 files changed, 249 insertions(+), 149 deletions(-)
create mode 100644 cmd/adtimg.c
delete mode 100644 cmd/dtimg.c
--
2.24.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] dtimg/am57xx_evm_defconfig: Rename dtimg to adtimg
2019-12-24 16:51 [PATCH 0/3] cmd: dtimg: Rename to adtimg and refactor usage style Eugeniu Rosca
@ 2019-12-24 16:51 ` Eugeniu Rosca
2020-01-07 16:49 ` Simon Glass
2020-01-10 21:50 ` Tom Rini
2019-12-24 16:51 ` [PATCH 2/3] cmd: adtimg: Rename internal symbols Eugeniu Rosca
` (2 subsequent siblings)
3 siblings, 2 replies; 17+ messages in thread
From: Eugeniu Rosca @ 2019-12-24 16:51 UTC (permalink / raw)
To: u-boot
Rename the existing 'dtimg' command to 'adtimg', in order to:
- Suggest the Android origins and scope
- Be consistent with the upcoming 'abootimg' command (naming
suggested by Simon [*])
The change in _not_ backward compatible, but its benefits outweigh its
downsides, given that we don't expect active users of 'dtimg' today.
Perform the rename in several steps:
1. Rename *.c file and Kconfig symbol. This should allow
'git log --follow' to properly track the history of 'adtimg.c'
2. 's/dtimg/adtimg/g' in the internal namespace of 'adtimg.c'
ELF comparison [**] before and after shows no functional change.
[*] https://patchwork.ozlabs.org/patch/1182212/#2291600
[**] diff -u <(objdump -d cmd/dtimg.o) <(objdump -d cmd/adtimg.o)
Cc: Tom Rini <trini@konsulko.com>
Cc: Sam Protsenko <semen.protsenko@linaro.org>
Cc: Simon Glass <sjg@chromium.org>
Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
---
cmd/Kconfig | 4 ++--
cmd/Makefile | 2 +-
cmd/{dtimg.c => adtimg.c} | 0
common/Makefile | 2 +-
configs/am57xx_evm_defconfig | 2 +-
configs/am57xx_hs_evm_defconfig | 2 +-
configs/am57xx_hs_evm_usb_defconfig | 2 +-
7 files changed, 7 insertions(+), 7 deletions(-)
rename cmd/{dtimg.c => adtimg.c} (100%)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 1e4cf146c509..f63adbdc3a31 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -348,8 +348,8 @@ config CMD_BOOTMENU
help
Add an ANSI terminal boot menu command.
-config CMD_DTIMG
- bool "dtimg"
+config CMD_ADTIMG
+ bool "adtimg"
help
Android DTB/DTBO image manipulation commands. Read dtb/dtbo files from
image into RAM, dump image structure information, etc. Those dtb/dtbo
diff --git a/cmd/Makefile b/cmd/Makefile
index 3ac710454652..c17ee20b25c6 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -47,7 +47,7 @@ obj-$(CONFIG_CMD_SOUND) += sound.o
ifdef CONFIG_POST
obj-$(CONFIG_CMD_DIAG) += diag.o
endif
-obj-$(CONFIG_CMD_DTIMG) += dtimg.o
+obj-$(CONFIG_CMD_ADTIMG) += adtimg.o
obj-$(CONFIG_CMD_ECHO) += echo.o
obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o
obj-$(CONFIG_CMD_EEPROM) += eeprom.o
diff --git a/cmd/dtimg.c b/cmd/adtimg.c
similarity index 100%
rename from cmd/dtimg.c
rename to cmd/adtimg.c
diff --git a/common/Makefile b/common/Makefile
index 302d8beaf356..029cc0f2ce6b 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -117,7 +117,7 @@ obj-$(CONFIG_IO_TRACE) += iotrace.o
obj-y += memsize.o
obj-y += stdio.o
-obj-$(CONFIG_CMD_DTIMG) += image-android-dt.o
+obj-$(CONFIG_CMD_ADTIMG) += image-android-dt.o
ifdef CONFIG_CMD_EEPROM_LAYOUT
obj-y += eeprom/eeprom_field.o eeprom/eeprom_layout.o
diff --git a/configs/am57xx_evm_defconfig b/configs/am57xx_evm_defconfig
index ae183e9b562c..0c6a2e9193b9 100644
--- a/configs/am57xx_evm_defconfig
+++ b/configs/am57xx_evm_defconfig
@@ -29,7 +29,7 @@ CONFIG_SPL_OS_BOOT=y
CONFIG_SPL_SPI_LOAD=y
CONFIG_SYS_SPI_U_BOOT_OFFS=0x40000
CONFIG_SPL_YMODEM_SUPPORT=y
-CONFIG_CMD_DTIMG=y
+CONFIG_CMD_ADTIMG=y
CONFIG_CMD_SPL=y
CONFIG_CMD_BCB=y
# CONFIG_CMD_FLASH is not set
diff --git a/configs/am57xx_hs_evm_defconfig b/configs/am57xx_hs_evm_defconfig
index 800ec6c70b92..3c57dfb031a9 100644
--- a/configs/am57xx_hs_evm_defconfig
+++ b/configs/am57xx_hs_evm_defconfig
@@ -32,7 +32,7 @@ CONFIG_SPL_DMA_SUPPORT=y
# CONFIG_SPL_NAND_SUPPORT is not set
CONFIG_SPL_SPI_LOAD=y
CONFIG_SYS_SPI_U_BOOT_OFFS=0x40000
-CONFIG_CMD_DTIMG=y
+CONFIG_CMD_ADTIMG=y
CONFIG_CMD_BCB=y
# CONFIG_CMD_FLASH is not set
# CONFIG_CMD_SETEXPR is not set
diff --git a/configs/am57xx_hs_evm_usb_defconfig b/configs/am57xx_hs_evm_usb_defconfig
index f2cbf2fe2ba1..87f391c2b029 100644
--- a/configs/am57xx_hs_evm_usb_defconfig
+++ b/configs/am57xx_hs_evm_usb_defconfig
@@ -37,7 +37,7 @@ CONFIG_SYS_SPI_U_BOOT_OFFS=0x40000
CONFIG_SPL_USB_GADGET=y
CONFIG_SPL_DFU=y
CONFIG_SPL_YMODEM_SUPPORT=y
-CONFIG_CMD_DTIMG=y
+CONFIG_CMD_ADTIMG=y
CONFIG_CMD_BCB=y
# CONFIG_CMD_FLASH is not set
# CONFIG_CMD_SETEXPR is not set
--
2.24.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/3] cmd: adtimg: Rename internal symbols
2019-12-24 16:51 [PATCH 0/3] cmd: dtimg: Rename to adtimg and refactor usage style Eugeniu Rosca
2019-12-24 16:51 ` [PATCH 1/3] dtimg/am57xx_evm_defconfig: Rename dtimg to adtimg Eugeniu Rosca
@ 2019-12-24 16:51 ` Eugeniu Rosca
2020-01-08 17:39 ` Simon Glass
2020-01-10 21:50 ` Tom Rini
2019-12-24 16:51 ` [PATCH 3/3] cmd: adtimg: Refactor usage style Eugeniu Rosca
2019-12-24 19:44 ` [PATCH 0/3] cmd: dtimg: Rename to adtimg and refactor " Sam Protsenko
3 siblings, 2 replies; 17+ messages in thread
From: Eugeniu Rosca @ 2019-12-24 16:51 UTC (permalink / raw)
To: u-boot
With 'dtimg.c' renamed to 'adtimg.c', now ensure the naming
consistency in the internal implementation of 'adtimg.c'.
No functional change intended.
Cc: Sam Protsenko <semen.protsenko@linaro.org>
Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
---
cmd/adtimg.c | 51 ++++++++++++++++++++++++++-------------------------
1 file changed, 26 insertions(+), 25 deletions(-)
diff --git a/cmd/adtimg.c b/cmd/adtimg.c
index 6c5d53cc6808..22b4f5e1a83f 100644
--- a/cmd/adtimg.c
+++ b/cmd/adtimg.c
@@ -8,13 +8,13 @@
#include <image-android-dt.h>
#include <common.h>
-enum cmd_dtimg_info {
- CMD_DTIMG_START = 0,
- CMD_DTIMG_SIZE,
+enum cmd_adtimg_info {
+ CMD_ADTIMG_START = 0,
+ CMD_ADTIMG_SIZE,
};
-static int do_dtimg_dump(cmd_tbl_t *cmdtp, int flag, int argc,
- char * const argv[])
+static int do_adtimg_dump(cmd_tbl_t *cmdtp, int flag, int argc,
+ char * const argv[])
{
char *endp;
ulong hdr_addr;
@@ -38,7 +38,8 @@ static int do_dtimg_dump(cmd_tbl_t *cmdtp, int flag, int argc,
return CMD_RET_SUCCESS;
}
-static int dtimg_get_fdt(int argc, char * const argv[], enum cmd_dtimg_info cmd)
+static int adtimg_get_fdt(int argc, char * const argv[],
+ enum cmd_adtimg_info cmd)
{
ulong hdr_addr;
u32 index;
@@ -71,14 +72,14 @@ static int dtimg_get_fdt(int argc, char * const argv[], enum cmd_dtimg_info cmd)
return CMD_RET_FAILURE;
switch (cmd) {
- case CMD_DTIMG_START:
+ case CMD_ADTIMG_START:
snprintf(buf, sizeof(buf), "%lx", fdt_addr);
break;
- case CMD_DTIMG_SIZE:
+ case CMD_ADTIMG_SIZE:
snprintf(buf, sizeof(buf), "%x", fdt_size);
break;
default:
- printf("Error: Unknown cmd_dtimg_info value: %d\n", cmd);
+ printf("Error: Unknown cmd_adtimg_info value: %d\n", cmd);
return CMD_RET_FAILURE;
}
@@ -87,31 +88,31 @@ static int dtimg_get_fdt(int argc, char * const argv[], enum cmd_dtimg_info cmd)
return CMD_RET_SUCCESS;
}
-static int do_dtimg_start(cmd_tbl_t *cmdtp, int flag, int argc,
- char * const argv[])
+static int do_adtimg_start(cmd_tbl_t *cmdtp, int flag, int argc,
+ char * const argv[])
{
- return dtimg_get_fdt(argc, argv, CMD_DTIMG_START);
+ return adtimg_get_fdt(argc, argv, CMD_ADTIMG_START);
}
-static int do_dtimg_size(cmd_tbl_t *cmdtp, int flag, int argc,
- char * const argv[])
+static int do_adtimg_size(cmd_tbl_t *cmdtp, int flag, int argc,
+ char * const argv[])
{
- return dtimg_get_fdt(argc, argv, CMD_DTIMG_SIZE);
+ return adtimg_get_fdt(argc, argv, CMD_ADTIMG_SIZE);
}
-static cmd_tbl_t cmd_dtimg_sub[] = {
- U_BOOT_CMD_MKENT(dump, 2, 0, do_dtimg_dump, "", ""),
- U_BOOT_CMD_MKENT(start, 4, 0, do_dtimg_start, "", ""),
- U_BOOT_CMD_MKENT(size, 4, 0, do_dtimg_size, "", ""),
+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, "", ""),
};
-static int do_dtimg(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+static int do_adtimg(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
{
cmd_tbl_t *cp;
- cp = find_cmd_tbl(argv[1], cmd_dtimg_sub, ARRAY_SIZE(cmd_dtimg_sub));
+ cp = find_cmd_tbl(argv[1], cmd_adtimg_sub, ARRAY_SIZE(cmd_adtimg_sub));
- /* Strip off leading 'dtimg' command argument */
+ /* Strip off leading 'adtimg' command argument */
argc--;
argv++;
@@ -124,17 +125,17 @@ static int do_dtimg(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
}
U_BOOT_CMD(
- dtimg, CONFIG_SYS_MAXARGS, 0, do_dtimg,
+ 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"
- "dtimg start <addr> <index> <varname>\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"
- "dtimg size <addr> <index> <varname>\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"
--
2.24.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/3] cmd: adtimg: Refactor usage style
2019-12-24 16:51 [PATCH 0/3] cmd: dtimg: Rename to adtimg and refactor usage style Eugeniu Rosca
2019-12-24 16:51 ` [PATCH 1/3] dtimg/am57xx_evm_defconfig: Rename dtimg to adtimg Eugeniu Rosca
2019-12-24 16:51 ` [PATCH 2/3] cmd: adtimg: Rename internal symbols Eugeniu Rosca
@ 2019-12-24 16:51 ` Eugeniu Rosca
2020-01-08 17:39 ` Simon Glass
2020-01-10 21:50 ` Tom Rini
2019-12-24 19:44 ` [PATCH 0/3] cmd: dtimg: Rename to adtimg and refactor " Sam Protsenko
3 siblings, 2 replies; 17+ messages in thread
From: Eugeniu Rosca @ 2019-12-24 16:51 UTC (permalink / raw)
To: u-boot
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(-)
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)"
);
--
2.24.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 0/3] cmd: dtimg: Rename to adtimg and refactor usage style
2019-12-24 16:51 [PATCH 0/3] cmd: dtimg: Rename to adtimg and refactor usage style Eugeniu Rosca
` (2 preceding siblings ...)
2019-12-24 16:51 ` [PATCH 3/3] cmd: adtimg: Refactor usage style Eugeniu Rosca
@ 2019-12-24 19:44 ` Sam Protsenko
3 siblings, 0 replies; 17+ messages in thread
From: Sam Protsenko @ 2019-12-24 19:44 UTC (permalink / raw)
To: u-boot
Hi
On Tue, Dec 24, 2019 at 6:51 PM Eugeniu Rosca <roscaeugeniu@gmail.com> wrote:
>
> The main focus of this series is to prepare the ground for:
> - adding the id/rev-based DT lookup in dtimg/adtimg [1]
> - adding the abootimg command [2]
>
> To be more clear, the above topics are handled _outside_ of this series,
> but they treat this series as hard dependency.
>
> The per-patch motivation and testing are described meticulously
> in each commit.
>
> [1] https://patchwork.ozlabs.org/patch/1202580/
> ("cmd: dtimg: Get start and size based on --id and --rev")
> [2] https://patchwork.ozlabs.org/patch/1182212/
> ("cmd: bootimg: Add bootimg command")
>
> Eugeniu Rosca (3):
> dtimg/am57xx_evm_defconfig: Rename dtimg to adtimg
> cmd: adtimg: Rename internal symbols
> cmd: adtimg: Refactor usage style
>
For the whole series:
Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
> cmd/Kconfig | 4 +-
> cmd/Makefile | 2 +-
> cmd/adtimg.c | 242 ++++++++++++++++++++++++++++
> cmd/dtimg.c | 142 ----------------
> common/Makefile | 2 +-
> configs/am57xx_evm_defconfig | 2 +-
> configs/am57xx_hs_evm_defconfig | 2 +-
> configs/am57xx_hs_evm_usb_defconfig | 2 +-
> 8 files changed, 249 insertions(+), 149 deletions(-)
> create mode 100644 cmd/adtimg.c
> delete mode 100644 cmd/dtimg.c
>
> --
> 2.24.0
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] dtimg/am57xx_evm_defconfig: Rename dtimg to adtimg
2019-12-24 16:51 ` [PATCH 1/3] dtimg/am57xx_evm_defconfig: Rename dtimg to adtimg Eugeniu Rosca
@ 2020-01-07 16:49 ` Simon Glass
2020-01-07 17:13 ` Eugeniu Rosca
2020-01-10 21:50 ` Tom Rini
1 sibling, 1 reply; 17+ messages in thread
From: Simon Glass @ 2020-01-07 16:49 UTC (permalink / raw)
To: u-boot
On Tue, 24 Dec 2019 at 09:51, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote:
>
> Rename the existing 'dtimg' command to 'adtimg', in order to:
> - Suggest the Android origins and scope
> - Be consistent with the upcoming 'abootimg' command (naming
> suggested by Simon [*])
>
> The change in _not_ backward compatible, but its benefits outweigh its
> downsides, given that we don't expect active users of 'dtimg' today.
>
> Perform the rename in several steps:
> 1. Rename *.c file and Kconfig symbol. This should allow
> 'git log --follow' to properly track the history of 'adtimg.c'
> 2. 's/dtimg/adtimg/g' in the internal namespace of 'adtimg.c'
>
> ELF comparison [**] before and after shows no functional change.
>
> [*] https://patchwork.ozlabs.org/patch/1182212/#2291600
> [**] diff -u <(objdump -d cmd/dtimg.o) <(objdump -d cmd/adtimg.o)
>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Sam Protsenko <semen.protsenko@linaro.org>
> Cc: Simon Glass <sjg@chromium.org>
> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> ---
> cmd/Kconfig | 4 ++--
> cmd/Makefile | 2 +-
> cmd/{dtimg.c => adtimg.c} | 0
> common/Makefile | 2 +-
> configs/am57xx_evm_defconfig | 2 +-
> configs/am57xx_hs_evm_defconfig | 2 +-
> configs/am57xx_hs_evm_usb_defconfig | 2 +-
> 7 files changed, 7 insertions(+), 7 deletions(-)
> rename cmd/{dtimg.c => adtimg.c} (100%)
>
Reviewed-by: Simon Glass<sjg@chromium.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] dtimg/am57xx_evm_defconfig: Rename dtimg to adtimg
2020-01-07 16:49 ` Simon Glass
@ 2020-01-07 17:13 ` Eugeniu Rosca
0 siblings, 0 replies; 17+ messages in thread
From: Eugeniu Rosca @ 2020-01-07 17:13 UTC (permalink / raw)
To: u-boot
Hi Simon,
On Tue, Jan 07, 2020 at 09:49:45AM -0700, Simon Glass wrote:
> On Tue, 24 Dec 2019 at 09:51, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote:
> >
> > Rename the existing 'dtimg' command to 'adtimg', in order to:
> > - Suggest the Android origins and scope
> > - Be consistent with the upcoming 'abootimg' command (naming
> > suggested by Simon [*])
> >
> > The change in _not_ backward compatible, but its benefits outweigh its
> > downsides, given that we don't expect active users of 'dtimg' today.
> >
> > Perform the rename in several steps:
> > 1. Rename *.c file and Kconfig symbol. This should allow
> > 'git log --follow' to properly track the history of 'adtimg.c'
> > 2. 's/dtimg/adtimg/g' in the internal namespace of 'adtimg.c'
> >
> > ELF comparison [**] before and after shows no functional change.
> >
> > [*] https://patchwork.ozlabs.org/patch/1182212/#2291600
> > [**] diff -u <(objdump -d cmd/dtimg.o) <(objdump -d cmd/adtimg.o)
> >
> > Cc: Tom Rini <trini@konsulko.com>
> > Cc: Sam Protsenko <semen.protsenko@linaro.org>
> > Cc: Simon Glass <sjg@chromium.org>
> > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> > ---
> > cmd/Kconfig | 4 ++--
> > cmd/Makefile | 2 +-
> > cmd/{dtimg.c => adtimg.c} | 0
> > common/Makefile | 2 +-
> > configs/am57xx_evm_defconfig | 2 +-
> > configs/am57xx_hs_evm_defconfig | 2 +-
> > configs/am57xx_hs_evm_usb_defconfig | 2 +-
> > 7 files changed, 7 insertions(+), 7 deletions(-)
> > rename cmd/{dtimg.c => adtimg.c} (100%)
> >
>
> Reviewed-by: Simon Glass<sjg@chromium.org>
Happy new year and thank you very much for the review!
Do you see any blocking points in the other patches from this series?
- https://patchwork.ozlabs.org/patch/1215258/
- https://patchwork.ozlabs.org/patch/1215259/
If not, would you kindly provide your Reviewed-by signature?
This would unblock Sam with the recent 'abootimg' series:
- https://patchwork.ozlabs.org/cover/1215282/
This would also unblock me with some upcoming adtimg patches.
Thanks in advance for your feedback!
--
Best Regards,
Eugeniu
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/3] cmd: adtimg: Rename internal symbols
2019-12-24 16:51 ` [PATCH 2/3] cmd: adtimg: Rename internal symbols Eugeniu Rosca
@ 2020-01-08 17:39 ` Simon Glass
2020-01-10 21:50 ` Tom Rini
1 sibling, 0 replies; 17+ messages in thread
From: Simon Glass @ 2020-01-08 17:39 UTC (permalink / raw)
To: u-boot
On Tue, 24 Dec 2019 at 09:51, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote:
>
> With 'dtimg.c' renamed to 'adtimg.c', now ensure the naming
> consistency in the internal implementation of 'adtimg.c'.
>
> No functional change intended.
>
> Cc: Sam Protsenko <semen.protsenko@linaro.org>
> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> ---
> cmd/adtimg.c | 51 ++++++++++++++++++++++++++-------------------------
> 1 file changed, 26 insertions(+), 25 deletions(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/3] cmd: adtimg: Refactor usage style
2019-12-24 16:51 ` [PATCH 3/3] cmd: adtimg: Refactor usage style Eugeniu Rosca
@ 2020-01-08 17:39 ` Simon Glass
2020-01-08 18:12 ` Eugeniu Rosca
2020-01-10 21:50 ` Tom Rini
1 sibling, 1 reply; 17+ messages in thread
From: Simon Glass @ 2020-01-08 17:39 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/3] cmd: adtimg: Refactor usage style
2020-01-08 17:39 ` Simon Glass
@ 2020-01-08 18:12 ` Eugeniu Rosca
0 siblings, 0 replies; 17+ messages in thread
From: Eugeniu Rosca @ 2020-01-08 18:12 UTC (permalink / raw)
To: u-boot
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/
--
Best Regards,
Eugeniu
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] dtimg/am57xx_evm_defconfig: Rename dtimg to adtimg
2019-12-24 16:51 ` [PATCH 1/3] dtimg/am57xx_evm_defconfig: Rename dtimg to adtimg Eugeniu Rosca
2020-01-07 16:49 ` Simon Glass
@ 2020-01-10 21:50 ` Tom Rini
1 sibling, 0 replies; 17+ messages in thread
From: Tom Rini @ 2020-01-10 21:50 UTC (permalink / raw)
To: u-boot
On Tue, Dec 24, 2019 at 05:51:06PM +0100, Eugeniu Rosca wrote:
> Rename the existing 'dtimg' command to 'adtimg', in order to:
> - Suggest the Android origins and scope
> - Be consistent with the upcoming 'abootimg' command (naming
> suggested by Simon [*])
>
> The change in _not_ backward compatible, but its benefits outweigh its
> downsides, given that we don't expect active users of 'dtimg' today.
>
> Perform the rename in several steps:
> 1. Rename *.c file and Kconfig symbol. This should allow
> 'git log --follow' to properly track the history of 'adtimg.c'
> 2. 's/dtimg/adtimg/g' in the internal namespace of 'adtimg.c'
>
> ELF comparison [**] before and after shows no functional change.
>
> [*] https://patchwork.ozlabs.org/patch/1182212/#2291600
> [**] diff -u <(objdump -d cmd/dtimg.o) <(objdump -d cmd/adtimg.o)
>
> Cc: Tom Rini <trini@konsulko.com>
> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> Reviewed-by: Simon Glass<sjg@chromium.org>
> Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
Applied to u-boot/master, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200110/013fd03f/attachment.sig>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/3] cmd: adtimg: Rename internal symbols
2019-12-24 16:51 ` [PATCH 2/3] cmd: adtimg: Rename internal symbols Eugeniu Rosca
2020-01-08 17:39 ` Simon Glass
@ 2020-01-10 21:50 ` Tom Rini
1 sibling, 0 replies; 17+ messages in thread
From: Tom Rini @ 2020-01-10 21:50 UTC (permalink / raw)
To: u-boot
On Tue, Dec 24, 2019 at 05:51:07PM +0100, Eugeniu Rosca wrote:
> With 'dtimg.c' renamed to 'adtimg.c', now ensure the naming
> consistency in the internal implementation of 'adtimg.c'.
>
> No functional change intended.
>
> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
Applied to u-boot/master, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200110/c6092e36/attachment.sig>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/3] cmd: adtimg: Refactor usage style
2019-12-24 16:51 ` [PATCH 3/3] cmd: adtimg: Refactor usage style Eugeniu Rosca
2020-01-08 17:39 ` Simon Glass
@ 2020-01-10 21:50 ` Tom Rini
2020-01-10 23:29 ` Eugeniu Rosca
1 sibling, 1 reply; 17+ messages in thread
From: Tom Rini @ 2020-01-10 21:50 UTC (permalink / raw)
To: u-boot
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!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200110/30d94b3b/attachment.sig>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/3] cmd: adtimg: Refactor usage style
2020-01-10 21:50 ` Tom Rini
@ 2020-01-10 23:29 ` Eugeniu Rosca
2020-07-03 16:40 ` Patrick DELAUNAY
0 siblings, 1 reply; 17+ messages in thread
From: Eugeniu Rosca @ 2020-01-10 23:29 UTC (permalink / raw)
To: u-boot
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!
--
Best Regards,
Eugeniu
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/3] cmd: adtimg: Refactor usage style
2020-01-10 23:29 ` Eugeniu Rosca
@ 2020-07-03 16:40 ` Patrick DELAUNAY
2020-07-06 6:46 ` Eugeniu Rosca
0 siblings, 1 reply; 17+ messages in thread
From: Patrick DELAUNAY @ 2020-07-03 16:40 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/3] cmd: adtimg: Refactor usage style
2020-07-03 16:40 ` Patrick DELAUNAY
@ 2020-07-06 6:46 ` Eugeniu Rosca
2020-07-08 17:26 ` Patrick DELAUNAY
0 siblings, 1 reply; 17+ messages in thread
From: Eugeniu Rosca @ 2020-07-06 6:46 UTC (permalink / raw)
To: u-boot
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
--
Best regards,
Eugeniu Rosca
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/3] cmd: adtimg: Refactor usage style
2020-07-06 6:46 ` Eugeniu Rosca
@ 2020-07-08 17:26 ` Patrick DELAUNAY
0 siblings, 0 replies; 17+ messages in thread
From: Patrick DELAUNAY @ 2020-07-08 17:26 UTC (permalink / raw)
To: u-boot
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 at st.com/
> --
> Best regards,
> Eugeniu Rosca
Best regards,
Patrick
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-07-08 17:26 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-24 16:51 [PATCH 0/3] cmd: dtimg: Rename to adtimg and refactor usage style Eugeniu Rosca
2019-12-24 16:51 ` [PATCH 1/3] dtimg/am57xx_evm_defconfig: Rename dtimg to adtimg Eugeniu Rosca
2020-01-07 16:49 ` Simon Glass
2020-01-07 17:13 ` Eugeniu Rosca
2020-01-10 21:50 ` Tom Rini
2019-12-24 16:51 ` [PATCH 2/3] cmd: adtimg: Rename internal symbols Eugeniu Rosca
2020-01-08 17:39 ` Simon Glass
2020-01-10 21:50 ` Tom Rini
2019-12-24 16:51 ` [PATCH 3/3] cmd: adtimg: Refactor usage style Eugeniu Rosca
2020-01-08 17:39 ` Simon Glass
2020-01-08 18:12 ` Eugeniu Rosca
2020-01-10 21:50 ` Tom Rini
2020-01-10 23:29 ` Eugeniu Rosca
2020-07-03 16:40 ` Patrick DELAUNAY
2020-07-06 6:46 ` Eugeniu Rosca
2020-07-08 17:26 ` Patrick DELAUNAY
2019-12-24 19:44 ` [PATCH 0/3] cmd: dtimg: Rename to adtimg and refactor " Sam Protsenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.