Hi Jonas,
On 2/5/25 8:36 PM, Jonas Karlman wrote:
Hi Quentin,
On 2025-02-05 16:57, Quentin Schulz wrote:
Hi Jonas,
On 1/29/25 11:36 PM, Jonas Karlman wrote:
[...]
--- a/tools/rkcommon.c
+++ b/tools/rkcommon.c
@@ -331,8 +331,6 @@ static void rkcommon_set_header0_v2(void *buf, struct
image_tool_params *params)
uint8_t *image_ptr = NULL;
int i;
- printf("Image Type: Rockchip %s boot image\n",
- rkcommon_get_spl_hdr(params));
Not sure this change is related? It's also not replaced by anything if
I'm not mistaken, hence why I'm wondering why it's in this patch.
Following was meant as a replacement for this, in rkcommon_print_header_v2():
printf("Rockchip Boot Image (v2)\n");
The old printf() was incorrectly done at set_header, not in print_header,
that is called after set_header or when you try to "mkimage -l <file>".
Ah, I see, thanks.
So this is a bugfix because it doesn't show with mkimage -l, separate
commit then.
memset(buf, '\0', RK_INIT_OFFSET * RK_BLK_SIZE);
hdr->magic = cpu_to_le32(RK_MAGIC_V2);
hdr->boot_flag = cpu_to_le32(HASH_SHA256);
@@ -486,6 +484,29 @@ int rkcommon_verify_header(unsigned char *buf, int size,
return -ENOENT;
}
+static void rkcommon_print_header_v2(const struct header0_info_v2 *hdr)
+{
+ uint32_t val;
+ int i;
+
+ printf("Rockchip Boot Image (v2)\n");
+
+ for (i = 0; i < le16_to_cpu(hdr->num_images); i++) {
+ printf("Image %u: %u @ 0x%x\n",
+ le32_to_cpu(hdr->images[i].counter),
+ le16_to_cpu(hdr->images[i].size) * RK_BLK_SIZE,
+ le16_to_cpu(hdr->images[i].offset) * RK_BLK_SIZE);
+
+ val = le32_to_cpu(hdr->images[i].address);
+ if (val != 0xFFFFFFFF)
Can you explain why this value is explicitly excluded? I know this is
the 4GiB boundary but why does it matter?
It is used as the default value, unknown why, see:
@address: load address (default as 0xFFFFFFFF)
Can probably add a code comment here as well.
Or maybe a constant to highlight the relation between both. Though one
would need to give it an appropriate name... Here comes the most
difficult part of SW development.
[...]
@@ -521,15 +541,16 @@ void rkcommon_print_header(const void *buf, struct
image_tool_params *params)
boot_size = le16_to_cpu(header0.init_boot_size) * RK_BLK_SIZE -
init_size;
- printf("Image Type: Rockchip %s (%s) boot image\n",
- spl_info->spl_hdr,
+ printf("Rockchip %s (%s) Boot Image\n", spl_info->spl_hdr,
(image_type == IH_TYPE_RKSD) ? "SD/MMC" : "SPI");
Please keep "Image Type:" this is what's used for other SoC vendors
also, I assume some tooling could be parsing it.
Sure, will restore the "Image Type:" prefix here and for "Rockchip Boot
Image (v2)" above. Should probably also adjust "tools: mkimage: Add
Amlogic Boot Image type" [1] to do the same.
Could even have another callback e.g. get_image_type() which returns a
const char* to print after "Image Type: " and move that to
tools/mkimage.c to have consistent behavior for that part. Probably
over-engineering this though :) (I think we'd need to update imagetool too).
Cheers,
Quentin
