[PATCH v2 0/2] tools: mkimage: cleanups + allow to create legacy image with type flat_dt
Hello,
the first patch cleans up the error message output if struct image_type_params::set_header is set, the other adds support for creating legacy images with type flat_dt.
regards, Marc
Changes since v1: - introduce new image type (suggested by Sean Anderson) instead of adding new cmd line parameter
In case there's no struct image_type_params::set_header callback, no "errno" will be set. Don't fail with an error message, followed by "Success". Remove the printing of the human readable "errno" value.
Signed-off-by: Marc Kleine-Budde mkl@pengutronix.de --- tools/mkimage.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/mkimage.c b/tools/mkimage.c index 30c6df77081f..35a6b1fb799c 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -760,8 +760,8 @@ int main(int argc, char **argv) if (tparams->set_header) tparams->set_header (ptr, &sbuf, ifd, ¶ms); else { - fprintf (stderr, "%s: Can't set header for %s: %s\n", - params.cmdname, tparams->name, strerror(errno)); + fprintf (stderr, "%s: Can't set header for %s\n", + params.cmdname, tparams->name); exit (EXIT_FAILURE); }
On Mon, 31 Oct 2022 at 08:51, Marc Kleine-Budde mkl@pengutronix.de wrote:
In case there's no struct image_type_params::set_header callback, no "errno" will be set. Don't fail with an error message, followed by "Success". Remove the printing of the human readable "errno" value.
Signed-off-by: Marc Kleine-Budde mkl@pengutronix.de
tools/mkimage.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On 31.10.2022 13:27:10, Simon Glass wrote:
On Mon, 31 Oct 2022 at 08:51, Marc Kleine-Budde mkl@pengutronix.de wrote:
In case there's no struct image_type_params::set_header callback, no "errno" will be set. Don't fail with an error message, followed by "Success". Remove the printing of the human readable "errno" value.
Signed-off-by: Marc Kleine-Budde mkl@pengutronix.de
tools/mkimage.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Has this patch already been applied?
regards, Marc
+Tom Rini
Hi Marc,
On Wed, 16 Nov 2022 at 03:42, Marc Kleine-Budde mkl@pengutronix.de wrote:
On 31.10.2022 13:27:10, Simon Glass wrote:
On Mon, 31 Oct 2022 at 08:51, Marc Kleine-Budde mkl@pengutronix.de wrote:
In case there's no struct image_type_params::set_header callback, no "errno" will be set. Don't fail with an error message, followed by "Success". Remove the printing of the human readable "errno" value.
Signed-off-by: Marc Kleine-Budde mkl@pengutronix.de
tools/mkimage.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Has this patch already been applied?
No, you can see the status here:
https://patchwork.ozlabs.org/project/uboot/patch/20221031145121.236877-2-mkl...
Regards, SImon
If the user select the image type "flat_dt" a FIT image will be build. This breaks the legacy use case of putting a Flat Device Tree into a legacy u-boot image.
Add a new image type "flat_dt_legacy" to build a legacy u-boot image with a "flat_dt" type.
Link: https://lore.kernel.org/all/20221028155205.ojw6tcso2fofgnhm@pengutronix.de Signed-off-by: Marc Kleine-Budde mkl@pengutronix.de --- boot/image.c | 1 + include/image.h | 1 + tools/default_image.c | 11 +++++++++-- 3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/boot/image.c b/boot/image.c index 9f95b3260a80..9d7e57dee985 100644 --- a/boot/image.c +++ b/boot/image.c @@ -180,6 +180,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_COPRO, "copro", "Coprocessor Image"}, { IH_TYPE_SUNXI_EGON, "sunxi_egon", "Allwinner eGON Boot Image" }, { IH_TYPE_SUNXI_TOC0, "sunxi_toc0", "Allwinner TOC0 Boot Image" }, + { IH_TYPE_FLATDT_LEGACY, "flat_dt_legacy", "Flat Device Tree legacy Image", }, { -1, "", "", }, };
diff --git a/include/image.h b/include/image.h index d7d6a3fe5b81..e578e2c5f1fd 100644 --- a/include/image.h +++ b/include/image.h @@ -229,6 +229,7 @@ enum { IH_TYPE_COPRO, /* Coprocessor Image for remoteproc*/ IH_TYPE_SUNXI_EGON, /* Allwinner eGON Boot Image */ IH_TYPE_SUNXI_TOC0, /* Allwinner TOC0 Boot Image */ + IH_TYPE_FLATDT_LEGACY, /* Binary Flat Device Tree Blob in a legacy image */
IH_TYPE_COUNT, /* Number of image types */ }; diff --git a/tools/default_image.c b/tools/default_image.c index 4a067e65862e..3b49f0d70e29 100644 --- a/tools/default_image.c +++ b/tools/default_image.c @@ -27,7 +27,8 @@ static struct legacy_img_hdr header; static int image_check_image_types(uint8_t type) { if (((type > IH_TYPE_INVALID) && (type < IH_TYPE_FLATDT)) || - (type == IH_TYPE_KERNEL_NOLOAD) || (type == IH_TYPE_FIRMWARE_IVT)) + (type == IH_TYPE_KERNEL_NOLOAD) || (type == IH_TYPE_FIRMWARE_IVT) || + (type == IH_TYPE_FLATDT_LEGACY)) return EXIT_SUCCESS; else return EXIT_FAILURE; @@ -94,6 +95,7 @@ static void image_set_header(void *ptr, struct stat *sbuf, int ifd, uint32_t imagesize; uint32_t ep; uint32_t addr; + int type; struct legacy_img_hdr *hdr = (struct legacy_img_hdr *)ptr;
checksum = crc32(0, @@ -113,6 +115,11 @@ static void image_set_header(void *ptr, struct stat *sbuf, int ifd, else imagesize = sbuf->st_size - sizeof(struct legacy_img_hdr);
+ if (params->type == IH_TYPE_FLATDT_LEGACY) + type = IH_TYPE_FLATDT; + else + type = params->type; + if (params->os == IH_OS_TEE) { addr = optee_image_get_load_addr(hdr); ep = optee_image_get_entry_point(hdr); @@ -127,7 +134,7 @@ static void image_set_header(void *ptr, struct stat *sbuf, int ifd, image_set_dcrc(hdr, checksum); image_set_os(hdr, params->os); image_set_arch(hdr, params->arch); - image_set_type(hdr, params->type); + image_set_type(hdr, type); image_set_comp(hdr, params->comp);
image_set_name(hdr, params->imagename);
On 31.10.2022 15:51:21, Marc Kleine-Budde wrote:
If the user select the image type "flat_dt" a FIT image will be build. This breaks the legacy use case of putting a Flat Device Tree into a legacy u-boot image.
Add a new image type "flat_dt_legacy" to build a legacy u-boot image with a "flat_dt" type.
Link: https://lore.kernel.org/all/20221028155205.ojw6tcso2fofgnhm@pengutronix.de Signed-off-by: Marc Kleine-Budde mkl@pengutronix.de
Sean, what about this approach compared to adding the new command line parameter?
Marc
boot/image.c | 1 + include/image.h | 1 + tools/default_image.c | 11 +++++++++-- 3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/boot/image.c b/boot/image.c index 9f95b3260a80..9d7e57dee985 100644 --- a/boot/image.c +++ b/boot/image.c @@ -180,6 +180,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_COPRO, "copro", "Coprocessor Image"}, { IH_TYPE_SUNXI_EGON, "sunxi_egon", "Allwinner eGON Boot Image" }, { IH_TYPE_SUNXI_TOC0, "sunxi_toc0", "Allwinner TOC0 Boot Image" },
- { IH_TYPE_FLATDT_LEGACY, "flat_dt_legacy", "Flat Device Tree legacy Image", }, { -1, "", "", },
};
diff --git a/include/image.h b/include/image.h index d7d6a3fe5b81..e578e2c5f1fd 100644 --- a/include/image.h +++ b/include/image.h @@ -229,6 +229,7 @@ enum { IH_TYPE_COPRO, /* Coprocessor Image for remoteproc*/ IH_TYPE_SUNXI_EGON, /* Allwinner eGON Boot Image */ IH_TYPE_SUNXI_TOC0, /* Allwinner TOC0 Boot Image */
IH_TYPE_FLATDT_LEGACY, /* Binary Flat Device Tree Blob in a legacy image */
IH_TYPE_COUNT, /* Number of image types */
}; diff --git a/tools/default_image.c b/tools/default_image.c index 4a067e65862e..3b49f0d70e29 100644 --- a/tools/default_image.c +++ b/tools/default_image.c @@ -27,7 +27,8 @@ static struct legacy_img_hdr header; static int image_check_image_types(uint8_t type) { if (((type > IH_TYPE_INVALID) && (type < IH_TYPE_FLATDT)) ||
(type == IH_TYPE_KERNEL_NOLOAD) || (type == IH_TYPE_FIRMWARE_IVT))
(type == IH_TYPE_KERNEL_NOLOAD) || (type == IH_TYPE_FIRMWARE_IVT) ||(type == IH_TYPE_FLATDT_LEGACY))@@ -94,6 +95,7 @@ static void image_set_header(void *ptr, struct stat *sbuf, int ifd, uint32_t imagesize; uint32_t ep; uint32_t addr;
int type; struct legacy_img_hdr *hdr = (struct legacy_img_hdr *)ptr;
checksum = crc32(0,
@@ -113,6 +115,11 @@ static void image_set_header(void *ptr, struct stat *sbuf, int ifd, else imagesize = sbuf->st_size - sizeof(struct legacy_img_hdr);
- if (params->type == IH_TYPE_FLATDT_LEGACY)
type = IH_TYPE_FLATDT;- else
type = params->type;- if (params->os == IH_OS_TEE) { addr = optee_image_get_load_addr(hdr); ep = optee_image_get_entry_point(hdr);
@@ -127,7 +134,7 @@ static void image_set_header(void *ptr, struct stat *sbuf, int ifd, image_set_dcrc(hdr, checksum); image_set_os(hdr, params->os); image_set_arch(hdr, params->arch);
- image_set_type(hdr, params->type);
image_set_type(hdr, type); image_set_comp(hdr, params->comp);
image_set_name(hdr, params->imagename);
-- 2.35.1
On 11/16/22 05:49, Marc Kleine-Budde wrote:
On 31.10.2022 15:51:21, Marc Kleine-Budde wrote:
If the user select the image type "flat_dt" a FIT image will be build. This breaks the legacy use case of putting a Flat Device Tree into a legacy u-boot image.
Add a new image type "flat_dt_legacy" to build a legacy u-boot image with a "flat_dt" type.
Link: https://lore.kernel.org/all/20221028155205.ojw6tcso2fofgnhm@pengutronix.de Signed-off-by: Marc Kleine-Budde mkl@pengutronix.de
Sean, what about this approach compared to adding the new command line parameter?
This is good. Maybe we should just name it fdt?
--Sean
boot/image.c | 1 + include/image.h | 1 + tools/default_image.c | 11 +++++++++-- 3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/boot/image.c b/boot/image.c index 9f95b3260a80..9d7e57dee985 100644 --- a/boot/image.c +++ b/boot/image.c @@ -180,6 +180,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_COPRO, "copro", "Coprocessor Image"}, { IH_TYPE_SUNXI_EGON, "sunxi_egon", "Allwinner eGON Boot Image" }, { IH_TYPE_SUNXI_TOC0, "sunxi_toc0", "Allwinner TOC0 Boot Image" },
- { IH_TYPE_FLATDT_LEGACY, "flat_dt_legacy", "Flat Device Tree legacy Image", }, { -1, "", "", }, };
diff --git a/include/image.h b/include/image.h index d7d6a3fe5b81..e578e2c5f1fd 100644 --- a/include/image.h +++ b/include/image.h @@ -229,6 +229,7 @@ enum { IH_TYPE_COPRO, /* Coprocessor Image for remoteproc*/ IH_TYPE_SUNXI_EGON, /* Allwinner eGON Boot Image */ IH_TYPE_SUNXI_TOC0, /* Allwinner TOC0 Boot Image */
IH_TYPE_FLATDT_LEGACY, /* Binary Flat Device Tree Blob in a legacy image */
IH_TYPE_COUNT, /* Number of image types */ };
diff --git a/tools/default_image.c b/tools/default_image.c index 4a067e65862e..3b49f0d70e29 100644 --- a/tools/default_image.c +++ b/tools/default_image.c @@ -27,7 +27,8 @@ static struct legacy_img_hdr header; static int image_check_image_types(uint8_t type) { if (((type > IH_TYPE_INVALID) && (type < IH_TYPE_FLATDT)) ||
(type == IH_TYPE_KERNEL_NOLOAD) || (type == IH_TYPE_FIRMWARE_IVT))
(type == IH_TYPE_KERNEL_NOLOAD) || (type == IH_TYPE_FIRMWARE_IVT) ||(type == IH_TYPE_FLATDT_LEGACY))@@ -94,6 +95,7 @@ static void image_set_header(void *ptr, struct stat *sbuf, int ifd, uint32_t imagesize; uint32_t ep; uint32_t addr;
int type; struct legacy_img_hdr *hdr = (struct legacy_img_hdr *)ptr;
checksum = crc32(0,
@@ -113,6 +115,11 @@ static void image_set_header(void *ptr, struct stat *sbuf, int ifd, else imagesize = sbuf->st_size - sizeof(struct legacy_img_hdr);
- if (params->type == IH_TYPE_FLATDT_LEGACY)
type = IH_TYPE_FLATDT;- else
type = params->type;- if (params->os == IH_OS_TEE) { addr = optee_image_get_load_addr(hdr); ep = optee_image_get_entry_point(hdr);
@@ -127,7 +134,7 @@ static void image_set_header(void *ptr, struct stat *sbuf, int ifd, image_set_dcrc(hdr, checksum); image_set_os(hdr, params->os); image_set_arch(hdr, params->arch);
- image_set_type(hdr, params->type);
image_set_type(hdr, type); image_set_comp(hdr, params->comp);
image_set_name(hdr, params->imagename);
-- 2.35.1
On 17.11.2022 00:01:19, Sean Anderson wrote:
On 11/16/22 05:49, Marc Kleine-Budde wrote:
On 31.10.2022 15:51:21, Marc Kleine-Budde wrote:
If the user select the image type "flat_dt" a FIT image will be build. This breaks the legacy use case of putting a Flat Device Tree into a legacy u-boot image.
Add a new image type "flat_dt_legacy" to build a legacy u-boot image with a "flat_dt" type.
Link: https://lore.kernel.org/all/20221028155205.ojw6tcso2fofgnhm@pengutronix.de Signed-off-by: Marc Kleine-Budde mkl@pengutronix.de
Sean, what about this approach compared to adding the new command line parameter?
This is good. Maybe we should just name it fdt?
There is already the "flat_dt" in boot/image.c, which is the "new" image type:
{ IH_TYPE_FLATDT, "flat_dt", "Flat Device Tree", },
[...]
{ IH_TYPE_FLATDT_LEGACY, "flat_dt_legacy", "Flat Device Tree legacy Image", },
I need a legacy image, where the type is set to IH_TYPE_FLATDT. Maybe "legacy_flat_dt" or "legacy_fdt" would be an appropriate name, too.
As this string is user facing I think it should have "legacy" in it. I think "flat_dt" and "fdt" is just too similar and the user can't see the difference, IMHO.
--- a/boot/image.c +++ b/boot/image.c @@ -180,6 +180,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_COPRO, "copro", "Coprocessor Image"}, { IH_TYPE_SUNXI_EGON, "sunxi_egon", "Allwinner eGON Boot Image" }, { IH_TYPE_SUNXI_TOC0, "sunxi_toc0", "Allwinner TOC0 Boot Image" },
{ IH_TYPE_FLATDT_LEGACY, "flat_dt_legacy", "Flat Device Tree legacy Image", },
regards, Marc
On 11/17/22 06:30, Marc Kleine-Budde wrote:
On 17.11.2022 00:01:19, Sean Anderson wrote:
On 11/16/22 05:49, Marc Kleine-Budde wrote:
On 31.10.2022 15:51:21, Marc Kleine-Budde wrote:
If the user select the image type "flat_dt" a FIT image will be build. This breaks the legacy use case of putting a Flat Device Tree into a legacy u-boot image.
Add a new image type "flat_dt_legacy" to build a legacy u-boot image with a "flat_dt" type.
Link: https://lore.kernel.org/all/20221028155205.ojw6tcso2fofgnhm@pengutronix.de Signed-off-by: Marc Kleine-Budde mkl@pengutronix.de
Sean, what about this approach compared to adding the new command line parameter?
This is good. Maybe we should just name it fdt?
There is already the "flat_dt" in boot/image.c, which is the "new" image type:
{ IH_TYPE_FLATDT, "flat_dt", "Flat Device Tree", },
[...]
{ IH_TYPE_FLATDT_LEGACY, "flat_dt_legacy", "Flat Device Tree legacy Image", },I need a legacy image, where the type is set to IH_TYPE_FLATDT. Maybe "legacy_flat_dt" or "legacy_fdt" would be an appropriate name, too.
As this string is user facing I think it should have "legacy" in it.
fdt_legacy?
I just think flat_dt_legacy is quite wordy.
--Sean
I think "flat_dt" and "fdt" is just too similar and the user can't see the difference, IMHO.
--- a/boot/image.c +++ b/boot/image.c @@ -180,6 +180,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_COPRO, "copro", "Coprocessor Image"}, { IH_TYPE_SUNXI_EGON, "sunxi_egon", "Allwinner eGON Boot Image" }, { IH_TYPE_SUNXI_TOC0, "sunxi_toc0", "Allwinner TOC0 Boot Image" },
{ IH_TYPE_FLATDT_LEGACY, "flat_dt_legacy", "Flat Device Tree legacy Image", },regards, Marc
participants (3)
-
Marc Kleine-Budde -
Sean Anderson -
Simon Glass