[U-Boot] [PATCH V3 1/8] tools: imximage: add plugin support

Add plugin support for imximage.
Define CONFIG_USE_IMXIMG_PLUGIN in defconfig to enable using plugin.
Signed-off-by: Peng Fan peng.fan@nxp.com Cc: Stefano Babic sbabic@denx.de Cc: Eric Nelson eric@nelint.com Cc: Ye Li ye.li@nxp.com Reviewed-by: Tom Rini trini@konsulko.com ---
V3: Fix compile error.
V2: Drop the CONFIG_USE_PLUGIN, make plugin always support in imximage.
tools/imximage.c | 282 +++++++++++++++++++++++++++++++++++++++++++------------ tools/imximage.h | 7 +- 2 files changed, 229 insertions(+), 60 deletions(-)
diff --git a/tools/imximage.c b/tools/imximage.c index 092d550..7fa601e 100644 --- a/tools/imximage.c +++ b/tools/imximage.c @@ -27,6 +27,7 @@ static table_entry_t imximage_cmds[] = { {CMD_CHECK_BITS_CLR, "CHECK_BITS_CLR", "Reg Check bits clr", }, {CMD_CSF, "CSF", "Command Sequence File", }, {CMD_IMAGE_VERSION, "IMAGE_VERSION", "image version", }, + {CMD_PLUGIN, "PLUGIN", "file plugin_addr", }, {-1, "", "", }, };
@@ -80,6 +81,9 @@ static uint32_t imximage_ivt_offset = UNDEFINED; static uint32_t imximage_csf_size = UNDEFINED; /* Initial Load Region Size */ static uint32_t imximage_init_loadsize; +static uint32_t imximage_iram_free_start; +static uint32_t imximage_plugin_size; +static uint32_t plugin_image;
static set_dcd_val_t set_dcd_val; static set_dcd_param_t set_dcd_param; @@ -118,7 +122,11 @@ static uint32_t detect_imximage_version(struct imx_header *imx_hdr)
/* Try to detect V2 */ if ((fhdr_v2->header.tag == IVT_HEADER_TAG) && - (hdr_v2->dcd_table.header.tag == DCD_HEADER_TAG)) + (hdr_v2->data.dcd_table.header.tag == DCD_HEADER_TAG)) + return IMXIMAGE_V2; + + if ((fhdr_v2->header.tag == IVT_HEADER_TAG) && + hdr_v2->boot_data.plugin) return IMXIMAGE_V2;
return IMXIMAGE_VER_INVALID; @@ -165,7 +173,7 @@ static struct dcd_v2_cmd *gd_last_cmd; static void set_dcd_param_v2(struct imx_header *imxhdr, uint32_t dcd_len, int32_t cmd) { - dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.dcd_table; + dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.data.dcd_table; struct dcd_v2_cmd *d = gd_last_cmd; struct dcd_v2_cmd *d2; int len; @@ -261,21 +269,23 @@ static void set_dcd_rst_v1(struct imx_header *imxhdr, uint32_t dcd_len, static void set_dcd_rst_v2(struct imx_header *imxhdr, uint32_t dcd_len, char *name, int lineno) { - dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.dcd_table; - struct dcd_v2_cmd *d = gd_last_cmd; - int len; - - if (!d) - d = &dcd_v2->dcd_cmd; - len = be16_to_cpu(d->write_dcd_command.length); - if (len > 4) - d = (struct dcd_v2_cmd *)(((char *)d) + len); - - len = (char *)d - (char *)&dcd_v2->header; - - dcd_v2->header.tag = DCD_HEADER_TAG; - dcd_v2->header.length = cpu_to_be16(len); - dcd_v2->header.version = DCD_VERSION; + if (!imxhdr->header.hdr_v2.boot_data.plugin) { + dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.data.dcd_table; + struct dcd_v2_cmd *d = gd_last_cmd; + int len; + + if (!d) + d = &dcd_v2->dcd_cmd; + len = be16_to_cpu(d->write_dcd_command.length); + if (len > 4) + d = (struct dcd_v2_cmd *)(((char *)d) + len); + + len = (char *)d - (char *)&dcd_v2->header; + + dcd_v2->header.tag = DCD_HEADER_TAG; + dcd_v2->header.length = cpu_to_be16(len); + dcd_v2->header.version = DCD_VERSION; + } }
static void set_imx_hdr_v1(struct imx_header *imxhdr, uint32_t dcd_len, @@ -317,24 +327,93 @@ static void set_imx_hdr_v2(struct imx_header *imxhdr, uint32_t dcd_len, fhdr_v2->header.length = cpu_to_be16(sizeof(flash_header_v2_t)); fhdr_v2->header.version = IVT_VERSION; /* 0x40 */
- fhdr_v2->entry = entry_point; - fhdr_v2->reserved1 = fhdr_v2->reserved2 = 0; - hdr_base = entry_point - imximage_init_loadsize + - flash_offset; - fhdr_v2->self = hdr_base; - if (dcd_len > 0) - fhdr_v2->dcd_ptr = hdr_base - + offsetof(imx_header_v2_t, dcd_table); - else + if (!hdr_v2->boot_data.plugin) { + fhdr_v2->entry = entry_point; + fhdr_v2->reserved1 = 0; + fhdr_v2->reserved1 = 0; + hdr_base = entry_point - imximage_init_loadsize + + flash_offset; + fhdr_v2->self = hdr_base; + if (dcd_len > 0) + fhdr_v2->dcd_ptr = hdr_base + + offsetof(imx_header_v2_t, data); + else + fhdr_v2->dcd_ptr = 0; + fhdr_v2->boot_data_ptr = hdr_base + + offsetof(imx_header_v2_t, boot_data); + hdr_v2->boot_data.start = entry_point - imximage_init_loadsize; + + fhdr_v2->csf = 0; + + header_size_ptr = &hdr_v2->boot_data.size; + csf_ptr = &fhdr_v2->csf; + } else { + imx_header_v2_t *next_hdr_v2; + flash_header_v2_t *next_fhdr_v2; + + if (imximage_csf_size != 0) { + fprintf(stderr, "Error: Header v2: SECURE_BOOT is only supported in DCD mode!"); + exit(EXIT_FAILURE); + } + + fhdr_v2->entry = imximage_iram_free_start + + flash_offset + sizeof(flash_header_v2_t) + + sizeof(boot_data_t); + + fhdr_v2->reserved1 = 0; + fhdr_v2->reserved2 = 0; + fhdr_v2->self = imximage_iram_free_start + flash_offset; + fhdr_v2->dcd_ptr = 0; - fhdr_v2->boot_data_ptr = hdr_base - + offsetof(imx_header_v2_t, boot_data); - hdr_v2->boot_data.start = entry_point - imximage_init_loadsize;
- fhdr_v2->csf = 0; + fhdr_v2->boot_data_ptr = fhdr_v2->self + + offsetof(imx_header_v2_t, boot_data); + + hdr_v2->boot_data.start = imximage_iram_free_start; + /* + * The actural size of plugin image is "imximage_plugin_size + + * sizeof(flash_header_v2_t) + sizeof(boot_data_t)", plus the + * flash_offset space.The ROM code only need to copy this size + * to run the plugin code. However, later when copy the whole + * U-Boot image to DDR, the ROM code use memcpy to copy the + * first part of the image, and use the storage read function + * to get the remaining part. This requires the dividing point + * must be multiple of storage sector size. Here we set the + * first section to be 16KB for this purpose. + */ + hdr_v2->boot_data.size = MAX_PLUGIN_CODE_SIZE; + + /* Security feature are not supported */ + fhdr_v2->csf = 0; + + next_hdr_v2 = (imx_header_v2_t *)((char *)hdr_v2 + + imximage_plugin_size); + + next_fhdr_v2 = &next_hdr_v2->fhdr; + + next_fhdr_v2->header.tag = IVT_HEADER_TAG; /* 0xD1 */ + next_fhdr_v2->header.length = + cpu_to_be16(sizeof(flash_header_v2_t)); + next_fhdr_v2->header.version = IVT_VERSION; /* 0x40 */ + + next_fhdr_v2->entry = entry_point; + hdr_base = entry_point - sizeof(struct imx_header); + next_fhdr_v2->reserved1 = 0; + next_fhdr_v2->reserved2 = 0; + next_fhdr_v2->self = hdr_base + imximage_plugin_size; + + next_fhdr_v2->dcd_ptr = 0; + next_fhdr_v2->boot_data_ptr = next_fhdr_v2->self + + offsetof(imx_header_v2_t, boot_data); + + next_hdr_v2->boot_data.start = hdr_base - flash_offset; + + header_size_ptr = &next_hdr_v2->boot_data.size;
- header_size_ptr = &hdr_v2->boot_data.size; - csf_ptr = &fhdr_v2->csf; + next_hdr_v2->boot_data.plugin = 0; + + next_fhdr_v2->csf = 0; + } }
static void set_hdr_func(void) @@ -393,16 +472,19 @@ static void print_hdr_v2(struct imx_header *imx_hdr) { imx_header_v2_t *hdr_v2 = &imx_hdr->header.hdr_v2; flash_header_v2_t *fhdr_v2 = &hdr_v2->fhdr; - dcd_v2_t *dcd_v2 = &hdr_v2->dcd_table; - uint32_t size, version; + dcd_v2_t *dcd_v2 = &hdr_v2->data.dcd_table; + uint32_t size, version, plugin;
- size = be16_to_cpu(dcd_v2->header.length); - if (size > (MAX_HW_CFG_SIZE_V2 * sizeof(dcd_addr_data_t)) + 8) { - fprintf(stderr, - "Error: Image corrupt DCD size %d exceed maximum %d\n", - (uint32_t)(size / sizeof(dcd_addr_data_t)), - MAX_HW_CFG_SIZE_V2); - exit(EXIT_FAILURE); + plugin = hdr_v2->boot_data.plugin; + if (!plugin) { + size = be16_to_cpu(dcd_v2->header.length); + if (size > (MAX_HW_CFG_SIZE_V2 * sizeof(dcd_addr_data_t))) { + fprintf(stderr, + "Error: Image corrupt DCD size %d exceed maximum %d\n", + (uint32_t)(size / sizeof(dcd_addr_data_t)), + MAX_HW_CFG_SIZE_V2); + exit(EXIT_FAILURE); + } }
version = detect_imximage_version(imx_hdr); @@ -410,19 +492,81 @@ static void print_hdr_v2(struct imx_header *imx_hdr) printf("Image Type: Freescale IMX Boot Image\n"); printf("Image Ver: %x", version); printf("%s\n", get_table_entry_name(imximage_versions, NULL, version)); - printf("Data Size: "); - genimg_print_size(hdr_v2->boot_data.size); - printf("Load Address: %08x\n", (uint32_t)fhdr_v2->boot_data_ptr); - printf("Entry Point: %08x\n", (uint32_t)fhdr_v2->entry); - if (fhdr_v2->csf && (imximage_ivt_offset != UNDEFINED) && - (imximage_csf_size != UNDEFINED)) { - printf("HAB Blocks: %08x %08x %08x\n", - (uint32_t)fhdr_v2->self, 0, - hdr_v2->boot_data.size - imximage_ivt_offset - - imximage_csf_size); + printf("Mode: %s\n", plugin ? "PLUGIN" : "DCD"); + if (!plugin) { + printf("Data Size: "); + genimg_print_size(hdr_v2->boot_data.size); + printf("Load Address: %08x\n", (uint32_t)fhdr_v2->boot_data_ptr); + printf("Entry Point: %08x\n", (uint32_t)fhdr_v2->entry); + if (fhdr_v2->csf && (imximage_ivt_offset != UNDEFINED) && + (imximage_csf_size != UNDEFINED)) { + printf("HAB Blocks: %08x %08x %08x\n", + (uint32_t)fhdr_v2->self, 0, + hdr_v2->boot_data.size - imximage_ivt_offset - + imximage_csf_size); + } + } else { + imx_header_v2_t *next_hdr_v2; + flash_header_v2_t *next_fhdr_v2; + + /*First Header*/ + printf("Plugin Data Size: "); + genimg_print_size(hdr_v2->boot_data.size); + printf("Plugin Code Size: "); + genimg_print_size(imximage_plugin_size); + printf("Plugin Load Address: %08x\n", hdr_v2->boot_data.start); + printf("Plugin Entry Point: %08x\n", (uint32_t)fhdr_v2->entry); + + /*Second Header*/ + next_hdr_v2 = (imx_header_v2_t *)((char *)hdr_v2 + + imximage_plugin_size); + next_fhdr_v2 = &next_hdr_v2->fhdr; + printf("U-Boot Data Size: "); + genimg_print_size(next_hdr_v2->boot_data.size); + printf("U-Boot Load Address: %08x\n", + next_hdr_v2->boot_data.start); + printf("U-Boot Entry Point: %08x\n", + (uint32_t)next_fhdr_v2->entry); } }
+static void copy_plugin_code(struct imx_header *imxhdr, char *plugin_file) +{ + int ifd = -1; + struct stat sbuf; + char *plugin_buf = imxhdr->header.hdr_v2.data.plugin_code; + char *ptr; + + ifd = open(plugin_file, O_RDONLY|O_BINARY); + if (fstat(ifd, &sbuf) < 0) { + fprintf(stderr, "Can't stat %s: %s\n", + plugin_file, + strerror(errno)); + exit(EXIT_FAILURE); + } + + ptr = mmap(0, sbuf.st_size, PROT_READ, MAP_SHARED, ifd, 0); + if (ptr == MAP_FAILED) { + fprintf(stderr, "Can't read %s: %s\n", + plugin_file, + strerror(errno)); + exit(EXIT_FAILURE); + } + + if (sbuf.st_size > MAX_PLUGIN_CODE_SIZE) { + printf("plugin binary size too large\n"); + exit(EXIT_FAILURE); + } + + memcpy(plugin_buf, ptr, sbuf.st_size); + imximage_plugin_size = sbuf.st_size; + + (void) munmap((void *)ptr, sbuf.st_size); + (void) close(ifd); + + imxhdr->header.hdr_v2.boot_data.plugin = 1; +} + static void parse_cfg_cmd(struct imx_header *imxhdr, int32_t cmd, char *token, char *name, int lineno, int fld, int dcd_len) { @@ -497,6 +641,10 @@ static void parse_cfg_cmd(struct imx_header *imxhdr, int32_t cmd, char *token, if (unlikely(cmd_ver_first != 1)) cmd_ver_first = 0; break; + case CMD_PLUGIN: + plugin_image = 1; + copy_plugin_code(imxhdr, token); + break; } }
@@ -542,6 +690,10 @@ static void parse_cfg_fld(struct imx_header *imxhdr, int32_t *cmd, } } break; + case CMD_PLUGIN: + value = get_cfg_value(token, name, lineno); + imximage_iram_free_start = value; + break; default: break; } @@ -649,6 +801,7 @@ static void imximage_set_header(void *ptr, struct stat *sbuf, int ifd, { struct imx_header *imxhdr = (struct imx_header *)ptr; uint32_t dcd_len; + uint32_t header_size;
/* * In order to not change the old imx cfg file @@ -665,10 +818,15 @@ static void imximage_set_header(void *ptr, struct stat *sbuf, int ifd, dcd_len = parse_cfg_file(imxhdr, params->imagename);
if (imximage_version == IMXIMAGE_V2) { - if (imximage_init_loadsize < imximage_ivt_offset + - sizeof(imx_header_v2_t)) + header_size = sizeof(flash_header_v2_t) + sizeof(boot_data_t); + if (!plugin_image) + header_size += sizeof(dcd_v2_t); + else + header_size += MAX_PLUGIN_CODE_SIZE; + + if (imximage_init_loadsize < imximage_ivt_offset + header_size) imximage_init_loadsize = imximage_ivt_offset + - sizeof(imx_header_v2_t); + header_size; }
/* Set the imx header */ @@ -721,7 +879,7 @@ static int imximage_generate(struct image_tool_params *params, size_t alloc_len; struct stat sbuf; char *datafile = params->datafile; - uint32_t pad_len; + uint32_t pad_len, header_size;
memset(&imximage_header, 0, sizeof(imximage_header));
@@ -742,15 +900,21 @@ static int imximage_generate(struct image_tool_params *params, /* TODO: check i.MX image V1 handling, for now use 'old' style */ if (imximage_version == IMXIMAGE_V1) { alloc_len = 4096; + header_size = 4096; } else { - if (imximage_init_loadsize < imximage_ivt_offset + - sizeof(imx_header_v2_t)) + header_size = sizeof(flash_header_v2_t) + sizeof(boot_data_t); + if (!plugin_image) + header_size += sizeof(dcd_v2_t); + else + header_size += MAX_PLUGIN_CODE_SIZE; + + if (imximage_init_loadsize < imximage_ivt_offset + header_size) imximage_init_loadsize = imximage_ivt_offset + - sizeof(imx_header_v2_t); + header_size; alloc_len = imximage_init_loadsize - imximage_ivt_offset; }
- if (alloc_len < sizeof(struct imx_header)) { + if (alloc_len < header_size) { fprintf(stderr, "%s: header error\n", params->cmdname); exit(EXIT_FAILURE); diff --git a/tools/imximage.h b/tools/imximage.h index c7b9b5c..fe4dd89 100644 --- a/tools/imximage.h +++ b/tools/imximage.h @@ -9,6 +9,7 @@ #define _IMXIMAGE_H_
#define MAX_HW_CFG_SIZE_V2 220 /* Max number of registers imx can set for v2 */ +#define MAX_PLUGIN_CODE_SIZE (16*1024) #define MAX_HW_CFG_SIZE_V1 60 /* Max number of registers imx can set for v1 */ #define APP_CODE_BARKER 0xB1 #define DCD_BARKER 0xB17219E9 @@ -64,6 +65,7 @@ enum imximage_cmd { CMD_CHECK_BITS_SET, CMD_CHECK_BITS_CLR, CMD_CSF, + CMD_PLUGIN, };
enum imximage_fld_types { @@ -164,7 +166,10 @@ typedef struct { typedef struct { flash_header_v2_t fhdr; boot_data_t boot_data; - dcd_v2_t dcd_table; + union { + dcd_v2_t dcd_table; + char plugin_code[MAX_PLUGIN_CODE_SIZE]; + } data; } imx_header_v2_t;
/* The header must be aligned to 4k on MX53 for NAND boot */

Add mx6_plugin.S which calls boot rom setup function, generate the second ivt, and jump back to boot rom.
Signed-off-by: Peng Fan peng.fan@nxp.com Signed-off-by: Ye Li ye.li@nxp.com Signed-off-by: Utkarsh Gupta utkarsh.gupta@nxp.com Reviewed-by: Tom Rini trini@konsulko.com Acked-by: Utkarsh Gupta utkarsh.gupta@nxp.com ---
V3: None
V2: None
arch/arm/include/asm/arch-mx6/mx6_plugin.S | 159 +++++++++++++++++++++++++++++ 1 file changed, 159 insertions(+) create mode 100644 arch/arm/include/asm/arch-mx6/mx6_plugin.S
diff --git a/arch/arm/include/asm/arch-mx6/mx6_plugin.S b/arch/arm/include/asm/arch-mx6/mx6_plugin.S new file mode 100644 index 0000000..b7d1b20 --- /dev/null +++ b/arch/arm/include/asm/arch-mx6/mx6_plugin.S @@ -0,0 +1,159 @@ +/* + * Copyright (C) 2016 Freescale Semiconductor, Inc. + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <config.h> + +#ifdef CONFIG_ROM_UNIFIED_SECTIONS +#define ROM_API_TABLE_BASE_ADDR_LEGACY 0x180 +#define ROM_VERSION_OFFSET 0x80 +#else +#define ROM_API_TABLE_BASE_ADDR_LEGACY 0xC0 +#define ROM_VERSION_OFFSET 0x48 +#endif +#define ROM_API_TABLE_BASE_ADDR_MX6DQ_TO15 0xC4 +#define ROM_API_TABLE_BASE_ADDR_MX6DL_TO12 0xC4 +#define ROM_API_HWCNFG_SETUP_OFFSET 0x08 +#define ROM_VERSION_TO10 0x10 +#define ROM_VERSION_TO12 0x12 +#define ROM_VERSION_TO15 0x15 + +plugin_start: + + push {r0-r4, lr} + + imx6_ddr_setting + imx6_clock_gating + imx6_qos_setting + +/* + * The following is to fill in those arguments for this ROM function + * pu_irom_hwcnfg_setup(void **start, size_t *bytes, const void *boot_data) + * This function is used to copy data from the storage media into DDR. + * start - Initial (possibly partial) image load address on entry. + * Final image load address on exit. + * bytes - Initial (possibly partial) image size on entry. + * Final image size on exit. + * boot_data - Initial @ref ivt Boot Data load address. + */ + adr r0, boot_data2 + adr r1, image_len2 + adr r2, boot_data2 + +#ifdef CONFIG_NOR_BOOT +#ifdef CONFIG_MX6SX + ldr r3, =ROM_VERSION_OFFSET + ldr r4, [r3] + cmp r4, #ROM_VERSION_TO10 + bgt before_calling_rom___pu_irom_hwcnfg_setup + ldr r3, =0x00900b00 + ldr r4, =0x50000000 + str r4, [r3, #0x5c] +#else + ldr r3, =0x00900800 + ldr r4, =0x08000000 + str r4, [r3, #0xc0] +#endif +#endif + +/* + * check the _pu_irom_api_table for the address + */ +before_calling_rom___pu_irom_hwcnfg_setup: + ldr r3, =ROM_VERSION_OFFSET + ldr r4, [r3] +#if defined(CONFIG_MX6SOLO) || defined(CONFIG_MX6DL) + ldr r3, =ROM_VERSION_TO12 + cmp r4, r3 + ldrge r3, =ROM_API_TABLE_BASE_ADDR_MX6DL_TO12 + ldrlt r3, =ROM_API_TABLE_BASE_ADDR_LEGACY +#elif defined(CONFIG_MX6Q) + ldr r3, =ROM_VERSION_TO15 + cmp r4, r3 + ldrge r3, =ROM_API_TABLE_BASE_ADDR_MX6DQ_TO15 + ldrlt r3, =ROM_API_TABLE_BASE_ADDR_LEGACY +#else + ldr r3, =ROM_API_TABLE_BASE_ADDR_LEGACY +#endif + ldr r4, [r3, #ROM_API_HWCNFG_SETUP_OFFSET] + blx r4 +after_calling_rom___pu_irom_hwcnfg_setup: + +/* + * ROM_API_HWCNFG_SETUP function enables MMU & Caches. + * Thus disable MMU & Caches. + */ + + mrc p15, 0, r0, c1, c0, 0 /* read CP15 register 1 into r0*/ + ands r0, r0, #0x1 /* check if MMU is enabled */ + beq mmu_disable_notreq /* exit if MMU is already disabled */ + + /* Disable caches, MMU */ + mrc p15, 0, r0, c1, c0, 0 /* read CP15 register 1 into r0 */ + bic r0, r0, #(1 << 2) /* disable D Cache */ + bic r0, r0, #0x1 /* clear bit 0 ; MMU off */ + + bic r0, r0, #(0x1 << 11) /* disable Z, branch prediction */ + bic r0, r0, #(0x1 << 1) /* disable A, Strict alignment */ + /* check enabled. */ + mcr p15, 0, r0, c1, c0, 0 /* write CP15 register 1 */ + mov r0, r0 + mov r0, r0 + mov r0, r0 + mov r0, r0 + +mmu_disable_notreq: + NOP + +/* To return to ROM from plugin, we need to fill in these argument. + * Here is what need to do: + * Need to construct the paramters for this function before return to ROM: + * plugin_download(void **start, size_t *bytes, UINT32 *ivt_offset) + */ + pop {r0-r4, lr} + push {r5} + ldr r5, boot_data2 + str r5, [r0] + ldr r5, image_len2 + str r5, [r1] + ldr r5, second_ivt_offset + str r5, [r2] + mov r0, #1 + pop {r5} + + /* return back to ROM code */ + bx lr + +/* make the following data right in the end of the output*/ +.ltorg + +#if (defined(CONFIG_NOR_BOOT) || defined(CONFIG_QSPI_BOOT)) +#define FLASH_OFFSET 0x1000 +#else +#define FLASH_OFFSET 0x400 +#endif + +/* + * second_ivt_offset is the offset from the "second_ivt_header" to + * "image_copy_start", which involves FLASH_OFFSET, plus the first + * ivt_header, the plugin code size itself recorded by "ivt2_header" + */ + +second_ivt_offset: .long (ivt2_header + 0x2C + FLASH_OFFSET) + +/* + * The following is the second IVT header plus the second boot data + */ +ivt2_header: .long 0x0 +app2_code_jump_v: .long 0x0 +reserv3: .long 0x0 +dcd2_ptr: .long 0x0 +boot_data2_ptr: .long 0x0 +self_ptr2: .long 0x0 +app_code_csf2: .long 0x0 +reserv4: .long 0x0 +boot_data2: .long 0x0 +image_len2: .long 0x0 +plugin2: .long 0x0

Add mx7_plugin.S which calls boot rom setup function, generate the second ivt, and jump back to boot rom.
Signed-off-by: Peng Fan peng.fan@nxp.com Signed-off-by: Ye Li ye.li@nxp.com Cc: Stefano Babic sbabic@denx.de Reviewed-by: Tom Rini trini@konsulko.com ---
V3: None
V2: None
arch/arm/include/asm/arch-mx7/mx7_plugin.S | 111 +++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 arch/arm/include/asm/arch-mx7/mx7_plugin.S
diff --git a/arch/arm/include/asm/arch-mx7/mx7_plugin.S b/arch/arm/include/asm/arch-mx7/mx7_plugin.S new file mode 100644 index 0000000..41336b4 --- /dev/null +++ b/arch/arm/include/asm/arch-mx7/mx7_plugin.S @@ -0,0 +1,111 @@ +/* + * Copyright (C) 2016 Freescale Semiconductor, Inc. + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <config.h> + +#define ROM_API_TABLE_BASE_ADDR_LEGACY 0x180 +#define ROM_VERSION_OFFSET 0x80 +#define ROM_API_HWCNFG_SETUP_OFFSET 0x08 + +plugin_start: + + push {r0-r4, lr} + + imx7_ddr_setting + imx7_clock_gating + imx7_qos_setting + +/* + * Check if we are in USB serial download mode and immediately return to ROM + * Need to check USB CTRL clock firstly, then check the USBx_nASYNCLISTADDR + */ + ldr r0, =0x30384680 + ldr r1, [r0] + cmp r1, #0 + beq normal_boot + + ldr r0, =0x30B10158 + ldr r1, [r0] + cmp r1, #0 + beq normal_boot + + pop {r0-r4, lr} + bx lr + +normal_boot: + +/* + * The following is to fill in those arguments for this ROM function + * pu_irom_hwcnfg_setup(void **start, size_t *bytes, const void *boot_data) + * This function is used to copy data from the storage media into DDR. + * start - Initial (possibly partial) image load address on entry. + * Final image load address on exit. + * bytes - Initial (possibly partial) image size on entry. + * Final image size on exit. + * boot_data - Initial @ref ivt Boot Data load address. + */ + adr r0, boot_data2 + adr r1, image_len2 + adr r2, boot_data2 + +/* + * check the _pu_irom_api_table for the address + */ +before_calling_rom___pu_irom_hwcnfg_setup: + ldr r3, =ROM_VERSION_OFFSET + ldr r4, [r3] + ldr r3, =ROM_API_TABLE_BASE_ADDR_LEGACY + ldr r4, [r3, #ROM_API_HWCNFG_SETUP_OFFSET] + blx r4 +after_calling_rom___pu_irom_hwcnfg_setup: + + +/* To return to ROM from plugin, we need to fill in these argument. + * Here is what need to do: + * Need to construct the paramters for this function before return to ROM: + * plugin_download(void **start, size_t *bytes, UINT32 *ivt_offset) + */ + pop {r0-r4, lr} + push {r5} + ldr r5, boot_data2 + str r5, [r0] + ldr r5, image_len2 + str r5, [r1] + ldr r5, second_ivt_offset + str r5, [r2] + mov r0, #1 + pop {r5} + + /* return back to ROM code */ + bx lr + +/* make the following data right in the end of the output*/ +.ltorg + +#define FLASH_OFFSET 0x400 + +/* + * second_ivt_offset is the offset from the "second_ivt_header" to + * "image_copy_start", which involves FLASH_OFFSET, plus the first + * ivt_header, the plugin code size itself recorded by "ivt2_header" + */ + +second_ivt_offset: .long (ivt2_header + 0x2C + FLASH_OFFSET) + +/* + * The following is the second IVT header plus the second boot data + */ +ivt2_header: .long 0x0 +app2_code_jump_v: .long 0x0 +reserv3: .long 0x0 +dcd2_ptr: .long 0x0 +boot_data2_ptr: .long 0x0 +self_ptr2: .long 0x0 +app_code_csf2: .long 0x0 +reserv4: .long 0x0 +boot_data2: .long 0x0 +image_len2: .long 0x0 +plugin2: .long 0x0

Introduce USE_IMXIMG_PLUGIN Kconfig
Signed-off-by: Peng Fan peng.fan@nxp.com Cc: Stefano Babic sbabic@denx.de Reviewed-by: Tom Rini trini@konsulko.com ---
V3: None
V2: New
arch/arm/imx-common/Kconfig | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/arch/arm/imx-common/Kconfig b/arch/arm/imx-common/Kconfig index 1b7da5a..85eac57 100644 --- a/arch/arm/imx-common/Kconfig +++ b/arch/arm/imx-common/Kconfig @@ -17,3 +17,10 @@ config IMX_BOOTAUX depends on ARCH_MX7 || ARCH_MX6 help bootaux [addr] to boot auxiliary core. + +config USE_IMXIMG_PLUGIN + bool "Using imximage plugin code" + depends on ARCH_MX7 || ARCH_MX6 + help + i.MX6/7 supports DCD and Plugin. Enable this configuration + to use Plugin, otherwise DCD will be used.

Hi Peng,
Note the typo in the subject header.
On 10/08/2016 08:58 AM, Peng Fan wrote:
Introduce USE_IMXIMG_PLUGIN Kconfig
Signed-off-by: Peng Fan peng.fan@nxp.com Cc: Stefano Babic sbabic@denx.de Reviewed-by: Tom Rini trini@konsulko.com
V3: None
V2: New
arch/arm/imx-common/Kconfig | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/arch/arm/imx-common/Kconfig b/arch/arm/imx-common/Kconfig index 1b7da5a..85eac57 100644 --- a/arch/arm/imx-common/Kconfig +++ b/arch/arm/imx-common/Kconfig @@ -17,3 +17,10 @@ config IMX_BOOTAUX depends on ARCH_MX7 || ARCH_MX6 help bootaux [addr] to boot auxiliary core.
+config USE_IMXIMG_PLUGIN
- bool "Using imximage plugin code"
s/Using/Use/
- depends on ARCH_MX7 || ARCH_MX6
- help
i.MX6/7 supports DCD and Plugin. Enable this configuration
to use Plugin, otherwise DCD will be used.
MX5x also supports plugins, right?
Also, the alternative isn't really DCD is it? Most (all) of the boards using SPL now are using the essentially empty config file in arch/arm/imx-common/spl_sd.cfg.

On Sun, Oct 09, 2016 at 07:45:41AM +0200, Eric Nelson wrote:
Hi Peng,
Note the typo in the subject header.
Thanks.
On 10/08/2016 08:58 AM, Peng Fan wrote:
Introduce USE_IMXIMG_PLUGIN Kconfig
Signed-off-by: Peng Fan peng.fan@nxp.com Cc: Stefano Babic sbabic@denx.de Reviewed-by: Tom Rini trini@konsulko.com
V3: None
V2: New
arch/arm/imx-common/Kconfig | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/arch/arm/imx-common/Kconfig b/arch/arm/imx-common/Kconfig index 1b7da5a..85eac57 100644 --- a/arch/arm/imx-common/Kconfig +++ b/arch/arm/imx-common/Kconfig @@ -17,3 +17,10 @@ config IMX_BOOTAUX depends on ARCH_MX7 || ARCH_MX6 help bootaux [addr] to boot auxiliary core.
+config USE_IMXIMG_PLUGIN
- bool "Using imximage plugin code"
s/Using/Use/
Will fix this in V4.
- depends on ARCH_MX7 || ARCH_MX6
- help
i.MX6/7 supports DCD and Plugin. Enable this configuration
to use Plugin, otherwise DCD will be used.
MX5x also supports plugins, right?
Yeah. I do not have the hardware to test, so not add ARCH_MX5. If later, someone can test ARCH_MX5, then he/she could add the support.
Also, the alternative isn't really DCD is it? Most (all) of the boards using SPL now are using the essentially empty config file in arch/arm/imx-common/spl_sd.cfg.
The help msg is just to tell use plugin or DCD. spl_sd.cfg surely can have DCD DATA lines or not :)
Regards, Peng.
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

If CONFIG_USE_IMXIMG_PLUGIN is selected, plugin.bin will be generated under board/$(BOARDDIR)/.
Signed-off-by: Peng Fan peng.fan@nxp.com Cc: Stefano Babic sbabic@denx.de Reviewed-by: Tom Rini trini@konsulko.com ---
V3: None
V2: New. Make this common to all i.MX6/7, but not duplicated in board makefile.
arch/arm/imx-common/Makefile | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/arch/arm/imx-common/Makefile b/arch/arm/imx-common/Makefile index d34a784..1873185 100644 --- a/arch/arm/imx-common/Makefile +++ b/arch/arm/imx-common/Makefile @@ -38,6 +38,23 @@ obj-$(CONFIG_CMD_BMODE) += cmd_bmode.o obj-$(CONFIG_CMD_HDMIDETECT) += cmd_hdmidet.o obj-$(CONFIG_CMD_DEKBLOB) += cmd_dek.o
+PLUGIN = board/$(BOARDDIR)/plugin + +ifeq ($(CONFIG_USE_IMXIMG_PLUGIN),y) + +$(PLUGIN).o: $(PLUGIN).S FORCE + $(Q)mkdir -p $(dir $@) + $(call if_changed_dep,as_o_S) + +$(PLUGIN).bin: $(PLUGIN).o FORCE + $(Q)mkdir -p $(dir $@) + $(OBJCOPY) -O binary --gap-fill 0xff $< $@ +else + +$(PLUGIN).bin: + +endif + quiet_cmd_cpp_cfg = CFGS $@ cmd_cpp_cfg = $(CPP) $(cpp_flags) -x c -o $@ $<
@@ -47,24 +64,24 @@ $(IMX_CONFIG): %.cfgtmp: % FORCE $(Q)mkdir -p $(dir $@) $(call if_changed_dep,cpp_cfg)
-MKIMAGEFLAGS_u-boot.imx = -n $(filter-out $< $(PHONY),$^) -T imximage \ +MKIMAGEFLAGS_u-boot.imx = -n $(filter-out $(PLUGIN).bin $< $(PHONY),$^) -T imximage \ -e $(CONFIG_SYS_TEXT_BASE)
-u-boot.imx: u-boot.bin $(IMX_CONFIG) FORCE +u-boot.imx: u-boot.bin $(IMX_CONFIG) $(PLUGIN).bin FORCE $(call if_changed,mkimage)
ifeq ($(CONFIG_OF_SEPARATE),y) -MKIMAGEFLAGS_u-boot-dtb.imx = -n $(filter-out $< $(PHONY),$^) -T imximage \ +MKIMAGEFLAGS_u-boot-dtb.imx = -n $(filter-out $(PLUGIN).bin $< $(PHONY),$^) -T imximage \ -e $(CONFIG_SYS_TEXT_BASE)
-u-boot-dtb.imx: u-boot-dtb.bin $(IMX_CONFIG) FORCE +u-boot-dtb.imx: u-boot-dtb.bin $(IMX_CONFIG) $(PLUGIN).bin FORCE $(call if_changed,mkimage) endif
-MKIMAGEFLAGS_SPL = -n $(filter-out $< $(PHONY),$^) -T imximage \ +MKIMAGEFLAGS_SPL = -n $(filter-out $(PLUGIN).bin $< $(PHONY),$^) -T imximage \ -e $(CONFIG_SPL_TEXT_BASE)
-SPL: spl/u-boot-spl.bin $(IMX_CONFIG) FORCE +SPL: spl/u-boot-spl.bin $(IMX_CONFIG) $(PLUGIN).bin FORCE $(call if_changed,mkimage)
MKIMAGEFLAGS_u-boot.uim = -A arm -O U-Boot -a $(CONFIG_SYS_TEXT_BASE) \

Add plugin code for mx6ullevk. Define CONFIG_USE_IMXIMG_PLUGIN in defconfig file to use plugin code.
Signed-off-by: Peng Fan peng.fan@nxp.com Cc: Stefano Babic sbabic@denx.de ---
V3: Update commit log
V2: None
board/freescale/mx6ullevk/imximage.cfg | 2 +- board/freescale/mx6ullevk/plugin.S | 139 +++++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 board/freescale/mx6ullevk/plugin.S
diff --git a/board/freescale/mx6ullevk/imximage.cfg b/board/freescale/mx6ullevk/imximage.cfg index 4604b62..1a21b49 100644 --- a/board/freescale/mx6ullevk/imximage.cfg +++ b/board/freescale/mx6ullevk/imximage.cfg @@ -29,7 +29,7 @@ BOOT_FROM nor BOOT_FROM sd #endif
-#ifdef CONFIG_USE_PLUGIN +#ifdef CONFIG_USE_IMXIMG_PLUGIN /*PLUGIN plugin-binary-file IRAM_FREE_START_ADDR*/ PLUGIN board/freescale/mx6ullevk/plugin.bin 0x00907000 #else diff --git a/board/freescale/mx6ullevk/plugin.S b/board/freescale/mx6ullevk/plugin.S new file mode 100644 index 0000000..65a3c45 --- /dev/null +++ b/board/freescale/mx6ullevk/plugin.S @@ -0,0 +1,139 @@ +/* + * Copyright (C) 2016 Freescale Semiconductor, Inc. + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <config.h> + +/* DDR script */ +.macro imx6ull_ddr3_evk_setting + ldr r0, =IOMUXC_BASE_ADDR + ldr r1, =0x000C0000 + str r1, [r0, #0x4B4] + ldr r1, =0x00000000 + str r1, [r0, #0x4AC] + ldr r1, =0x00000030 + str r1, [r0, #0x27C] + ldr r1, =0x00000030 + str r1, [r0, #0x250] + str r1, [r0, #0x24C] + str r1, [r0, #0x490] + ldr r1, =0x000C0030 + str r1, [r0, #0x288] + + ldr r1, =0x00000000 + str r1, [r0, #0x270] + + ldr r1, =0x00000030 + str r1, [r0, #0x260] + str r1, [r0, #0x264] + str r1, [r0, #0x4A0] + + ldr r1, =0x00020000 + str r1, [r0, #0x494] + + ldr r1, =0x00000030 + str r1, [r0, #0x280] + ldr r1, =0x00000030 + str r1, [r0, #0x284] + + ldr r1, =0x00020000 + str r1, [r0, #0x4B0] + + ldr r1, =0x00000030 + str r1, [r0, #0x498] + str r1, [r0, #0x4A4] + str r1, [r0, #0x244] + str r1, [r0, #0x248] + + ldr r0, =MMDC_P0_BASE_ADDR + ldr r1, =0x00008000 + str r1, [r0, #0x1C] + ldr r1, =0xA1390003 + str r1, [r0, #0x800] + ldr r1, =0x00000004 + str r1, [r0, #0x80C] + ldr r1, =0x41640158 + str r1, [r0, #0x83C] + ldr r1, =0x40403237 + str r1, [r0, #0x848] + ldr r1, =0x40403C33 + str r1, [r0, #0x850] + ldr r1, =0x33333333 + str r1, [r0, #0x81C] + str r1, [r0, #0x820] + ldr r1, =0xF3333333 + str r1, [r0, #0x82C] + str r1, [r0, #0x830] + ldr r1, =0x00944009 + str r1, [r0, #0x8C0] + ldr r1, =0x00000800 + str r1, [r0, #0x8B8] + ldr r1, =0x0002002D + str r1, [r0, #0x004] + ldr r1, =0x1B333030 + str r1, [r0, #0x008] + ldr r1, =0x676B52F3 + str r1, [r0, #0x00C] + ldr r1, =0xB66D0B63 + str r1, [r0, #0x010] + ldr r1, =0x01FF00DB + str r1, [r0, #0x014] + ldr r1, =0x00201740 + str r1, [r0, #0x018] + ldr r1, =0x00008000 + str r1, [r0, #0x01C] + ldr r1, =0x000026D2 + str r1, [r0, #0x02C] + ldr r1, =0x006B1023 + str r1, [r0, #0x030] + ldr r1, =0x0000004F + str r1, [r0, #0x040] + ldr r1, =0x84180000 + str r1, [r0, #0x000] + ldr r1, =0x00400000 + str r1, [r0, #0x890] + ldr r1, =0x02008032 + str r1, [r0, #0x01C] + ldr r1, =0x00008033 + str r1, [r0, #0x01C] + ldr r1, =0x00048031 + str r1, [r0, #0x01C] + ldr r1, =0x15208030 + str r1, [r0, #0x01C] + ldr r1, =0x04008040 + str r1, [r0, #0x01C] + ldr r1, =0x00000800 + str r1, [r0, #0x020] + ldr r1, =0x00000227 + str r1, [r0, #0x818] + ldr r1, =0x0002552D + str r1, [r0, #0x004] + ldr r1, =0x00011006 + str r1, [r0, #0x404] + ldr r1, =0x00000000 + str r1, [r0, #0x01C] +.endm + +.macro imx6_clock_gating + ldr r0, =CCM_BASE_ADDR + ldr r1, =0xFFFFFFFF + str r1, [r0, #0x68] + str r1, [r0, #0x6C] + str r1, [r0, #0x70] + str r1, [r0, #0x74] + str r1, [r0, #0x78] + str r1, [r0, #0x7C] + str r1, [r0, #0x80] +.endm + +.macro imx6_qos_setting +.endm + +.macro imx6_ddr_setting + imx6ull_ddr3_evk_setting +.endm + +/* include the common plugin code here */ +#include <asm/arch/mx6_plugin.S>

Correct boot device macro according to kconfig entry in common/Kconfig
Signed-off-by: Peng Fan peng.fan@nxp.com Cc: Stefano Babic sbabic@denx.de ---
V3: None
V2: None
board/freescale/mx6ullevk/imximage.cfg | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/board/freescale/mx6ullevk/imximage.cfg b/board/freescale/mx6ullevk/imximage.cfg index 1a21b49..3c219fa 100644 --- a/board/freescale/mx6ullevk/imximage.cfg +++ b/board/freescale/mx6ullevk/imximage.cfg @@ -21,9 +21,9 @@ IMAGE_VERSION 2 * spi/sd/nand/onenand, qspi/nor */
-#ifdef CONFIG_SYS_BOOT_QSPI +#ifdef CONFIG_QSPI_BOOT BOOT_FROM qspi -#elif defined(CONFIG_SYS_BOOT_EIMNOR) +#elif defined(CONFIG_NOR_BOOT) BOOT_FROM nor #else BOOT_FROM sd

Add defconfig file to use plugin code.
Signed-off-by: Peng Fan peng.fan@nxp.com Cc: Stefano Babic sbabic@denx.de ---
V3: None
V2: New
configs/mx6ull_14x14_evk_plugin_defconfig | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 configs/mx6ull_14x14_evk_plugin_defconfig
diff --git a/configs/mx6ull_14x14_evk_plugin_defconfig b/configs/mx6ull_14x14_evk_plugin_defconfig new file mode 100644 index 0000000..ff15c8e --- /dev/null +++ b/configs/mx6ull_14x14_evk_plugin_defconfig @@ -0,0 +1,31 @@ +CONFIG_ARM=y +CONFIG_ARCH_MX6=y +CONFIG_TARGET_MX6ULL_14X14_EVK=y +CONFIG_USE_IMXIMG_PLUGIN=y +CONFIG_DEFAULT_DEVICE_TREE="imx6ull-14x14-evk" +CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/freescale/mx6ullevk/imximage.cfg" +CONFIG_BOOTDELAY=3 +CONFIG_HUSH_PARSER=y +CONFIG_CMD_BOOTZ=y +# CONFIG_CMD_IMLS is not set +CONFIG_CMD_MEMTEST=y +CONFIG_CMD_MMC=y +CONFIG_CMD_I2C=y +CONFIG_CMD_GPIO=y +CONFIG_CMD_DHCP=y +CONFIG_CMD_PING=y +CONFIG_CMD_CACHE=y +CONFIG_CMD_EXT2=y +CONFIG_CMD_EXT4=y +CONFIG_CMD_EXT4_WRITE=y +CONFIG_CMD_FAT=y +CONFIG_CMD_FS_GENERIC=y +CONFIG_OF_CONTROL=y +CONFIG_DM_GPIO=y +CONFIG_DM_74X164=y +CONFIG_DM_I2C=y +CONFIG_DM_MMC=y +CONFIG_PINCTRL=y +CONFIG_PINCTRL_IMX6=y +CONFIG_DM_REGULATOR=y +CONFIG_DM_SPI=y

Hi Peng,
I'm sorry for taking so long to go though this.
On 10/08/2016 08:58 AM, Peng Fan wrote:
Add plugin support for imximage.
This CONFIG setting doesn't actually affect mkimage or imximage:
Define CONFIG_USE_IMXIMG_PLUGIN in defconfig to enable using plugin.
Signed-off-by: Peng Fan peng.fan@nxp.com Cc: Stefano Babic sbabic@denx.de Cc: Eric Nelson eric@nelint.com Cc: Ye Li ye.li@nxp.com Reviewed-by: Tom Rini trini@konsulko.com
V3: Fix compile error.
V2: Drop the CONFIG_USE_PLUGIN, make plugin always support in imximage.
tools/imximage.c | 282 +++++++++++++++++++++++++++++++++++++++++++------------ tools/imximage.h | 7 +- 2 files changed, 229 insertions(+), 60 deletions(-)
diff --git a/tools/imximage.c b/tools/imximage.c index 092d550..7fa601e 100644 --- a/tools/imximage.c +++ b/tools/imximage.c @@ -27,6 +27,7 @@ static table_entry_t imximage_cmds[] = { {CMD_CHECK_BITS_CLR, "CHECK_BITS_CLR", "Reg Check bits clr", }, {CMD_CSF, "CSF", "Command Sequence File", }, {CMD_IMAGE_VERSION, "IMAGE_VERSION", "image version", },
- {CMD_PLUGIN, "PLUGIN", "file plugin_addr", }, {-1, "", "", },
};
@@ -80,6 +81,9 @@ static uint32_t imximage_ivt_offset = UNDEFINED; static uint32_t imximage_csf_size = UNDEFINED; /* Initial Load Region Size */ static uint32_t imximage_init_loadsize;
These seem very limiting.
From what I can tell, there's no reason that you can't have multiple
plugins, and the use of these variables isn't really needed.
+static uint32_t imximage_iram_free_start; +static uint32_t imximage_plugin_size; +static uint32_t plugin_image;
static set_dcd_val_t set_dcd_val; static set_dcd_param_t set_dcd_param; @@ -118,7 +122,11 @@ static uint32_t detect_imximage_version(struct imx_header *imx_hdr)
/* Try to detect V2 */ if ((fhdr_v2->header.tag == IVT_HEADER_TAG) &&
(hdr_v2->dcd_table.header.tag == DCD_HEADER_TAG))
(hdr_v2->data.dcd_table.header.tag == DCD_HEADER_TAG))
return IMXIMAGE_V2;
if ((fhdr_v2->header.tag == IVT_HEADER_TAG) &&
hdr_v2->boot_data.plugin)
return IMXIMAGE_V2;
return IMXIMAGE_VER_INVALID;
@@ -165,7 +173,7 @@ static struct dcd_v2_cmd *gd_last_cmd; static void set_dcd_param_v2(struct imx_header *imxhdr, uint32_t dcd_len, int32_t cmd) {
I also don't understand why the choice to make the union with either a DCD table or a plugin.
We should be able to have both, and this doesn't make the code any easier.
- dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.dcd_table;
- dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.data.dcd_table; struct dcd_v2_cmd *d = gd_last_cmd; struct dcd_v2_cmd *d2; int len;
@@ -261,21 +269,23 @@ static void set_dcd_rst_v1(struct imx_header *imxhdr, uint32_t dcd_len, static void set_dcd_rst_v2(struct imx_header *imxhdr, uint32_t dcd_len, char *name, int lineno) {
- dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.dcd_table;
- struct dcd_v2_cmd *d = gd_last_cmd;
- int len;
- if (!d)
d = &dcd_v2->dcd_cmd;
- len = be16_to_cpu(d->write_dcd_command.length);
- if (len > 4)
d = (struct dcd_v2_cmd *)(((char *)d) + len);
- len = (char *)d - (char *)&dcd_v2->header;
- dcd_v2->header.tag = DCD_HEADER_TAG;
- dcd_v2->header.length = cpu_to_be16(len);
- dcd_v2->header.version = DCD_VERSION;
- if (!imxhdr->header.hdr_v2.boot_data.plugin) {
dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.data.dcd_table;
struct dcd_v2_cmd *d = gd_last_cmd;
int len;
if (!d)
d = &dcd_v2->dcd_cmd;
len = be16_to_cpu(d->write_dcd_command.length);
if (len > 4)
d = (struct dcd_v2_cmd *)(((char *)d) + len);
len = (char *)d - (char *)&dcd_v2->header;
dcd_v2->header.tag = DCD_HEADER_TAG;
dcd_v2->header.length = cpu_to_be16(len);
dcd_v2->header.version = DCD_VERSION;
- }
}
static void set_imx_hdr_v1(struct imx_header *imxhdr, uint32_t dcd_len, @@ -317,24 +327,93 @@ static void set_imx_hdr_v2(struct imx_header *imxhdr, uint32_t dcd_len, fhdr_v2->header.length = cpu_to_be16(sizeof(flash_header_v2_t)); fhdr_v2->header.version = IVT_VERSION; /* 0x40 */
It seems that the reason for a lot of this special-purpose code is to add support for the entry address.
If mkimage is invoked with an entrypoint from the command-line:
~/$ ./tools/mkimage -n my.cfg -T imximage -e 0xentrypoint u-boot.img u-boot.imx
This entry point is normally placed into the header, but if we have a plugin in the .cfg file like this:
~/$ grep PLUGIN my.cfg PLUGIN path/to/board/plugin.bin 0x00907000
Then we need to use the plugin's address instead of the command line address, and use the command-line address for the file which follows.
- fhdr_v2->entry = entry_point;
- fhdr_v2->reserved1 = fhdr_v2->reserved2 = 0;
- hdr_base = entry_point - imximage_init_loadsize +
flash_offset;
- fhdr_v2->self = hdr_base;
- if (dcd_len > 0)
fhdr_v2->dcd_ptr = hdr_base
+ offsetof(imx_header_v2_t, dcd_table);
- else
- if (!hdr_v2->boot_data.plugin) {
fhdr_v2->entry = entry_point;
fhdr_v2->reserved1 = 0;
fhdr_v2->reserved1 = 0;
hdr_base = entry_point - imximage_init_loadsize +
flash_offset;
fhdr_v2->self = hdr_base;
if (dcd_len > 0)
fhdr_v2->dcd_ptr = hdr_base +
offsetof(imx_header_v2_t, data);
else
fhdr_v2->dcd_ptr = 0;
fhdr_v2->boot_data_ptr = hdr_base
+ offsetof(imx_header_v2_t, boot_data);
hdr_v2->boot_data.start = entry_point - imximage_init_loadsize;
fhdr_v2->csf = 0;
header_size_ptr = &hdr_v2->boot_data.size;
csf_ptr = &fhdr_v2->csf;
- } else {
imx_header_v2_t *next_hdr_v2;
flash_header_v2_t *next_fhdr_v2;
if (imximage_csf_size != 0) {
fprintf(stderr, "Error: Header v2: SECURE_BOOT is only supported in DCD mode!");
exit(EXIT_FAILURE);
}
I think that only ->entry needs to be different between these two blocks.
fhdr_v2->entry = imximage_iram_free_start +
flash_offset + sizeof(flash_header_v2_t) +
sizeof(boot_data_t);
fhdr_v2->reserved1 = 0;
fhdr_v2->reserved2 = 0;
fhdr_v2->self = imximage_iram_free_start + flash_offset;
- fhdr_v2->dcd_ptr = 0;
fhdr_v2->boot_data_ptr = hdr_base
+ offsetof(imx_header_v2_t, boot_data);
hdr_v2->boot_data.start = entry_point - imximage_init_loadsize;
fhdr_v2->csf = 0;
fhdr_v2->boot_data_ptr = fhdr_v2->self +
offsetof(imx_header_v2_t, boot_data);
hdr_v2->boot_data.start = imximage_iram_free_start;
/*
* The actural size of plugin image is "imximage_plugin_size +
* sizeof(flash_header_v2_t) + sizeof(boot_data_t)", plus the
* flash_offset space.The ROM code only need to copy this size
* to run the plugin code. However, later when copy the whole
* U-Boot image to DDR, the ROM code use memcpy to copy the
* first part of the image, and use the storage read function
* to get the remaining part. This requires the dividing point
* must be multiple of storage sector size. Here we set the
* first section to be 16KB for this purpose.
*/
Where is the 16k limit coming from? I don't think this is necessary, and if it is, we won't be able to load SPL using a plugin.
hdr_v2->boot_data.size = MAX_PLUGIN_CODE_SIZE;
/* Security feature are not supported */
fhdr_v2->csf = 0;
next_hdr_v2 = (imx_header_v2_t *)((char *)hdr_v2 +
imximage_plugin_size);
next_fhdr_v2 = &next_hdr_v2->fhdr;
next_fhdr_v2->header.tag = IVT_HEADER_TAG; /* 0xD1 */
next_fhdr_v2->header.length =
cpu_to_be16(sizeof(flash_header_v2_t));
next_fhdr_v2->header.version = IVT_VERSION; /* 0x40 */
next_fhdr_v2->entry = entry_point;
hdr_base = entry_point - sizeof(struct imx_header);
next_fhdr_v2->reserved1 = 0;
next_fhdr_v2->reserved2 = 0;
next_fhdr_v2->self = hdr_base + imximage_plugin_size;
next_fhdr_v2->dcd_ptr = 0;
next_fhdr_v2->boot_data_ptr = next_fhdr_v2->self +
offsetof(imx_header_v2_t, boot_data);
next_hdr_v2->boot_data.start = hdr_base - flash_offset;
header_size_ptr = &next_hdr_v2->boot_data.size;
- header_size_ptr = &hdr_v2->boot_data.size;
- csf_ptr = &fhdr_v2->csf;
next_hdr_v2->boot_data.plugin = 0;
next_fhdr_v2->csf = 0;
- }
}
static void set_hdr_func(void) @@ -393,16 +472,19 @@ static void print_hdr_v2(struct imx_header *imx_hdr) { imx_header_v2_t *hdr_v2 = &imx_hdr->header.hdr_v2; flash_header_v2_t *fhdr_v2 = &hdr_v2->fhdr;
- dcd_v2_t *dcd_v2 = &hdr_v2->dcd_table;
- uint32_t size, version;
- dcd_v2_t *dcd_v2 = &hdr_v2->data.dcd_table;
- uint32_t size, version, plugin;
- size = be16_to_cpu(dcd_v2->header.length);
- if (size > (MAX_HW_CFG_SIZE_V2 * sizeof(dcd_addr_data_t)) + 8) {
fprintf(stderr,
"Error: Image corrupt DCD size %d exceed maximum %d\n",
(uint32_t)(size / sizeof(dcd_addr_data_t)),
MAX_HW_CFG_SIZE_V2);
exit(EXIT_FAILURE);
plugin = hdr_v2->boot_data.plugin;
if (!plugin) {
size = be16_to_cpu(dcd_v2->header.length);
if (size > (MAX_HW_CFG_SIZE_V2 * sizeof(dcd_addr_data_t))) {
fprintf(stderr,
"Error: Image corrupt DCD size %d exceed maximum %d\n",
(uint32_t)(size / sizeof(dcd_addr_data_t)),
MAX_HW_CFG_SIZE_V2);
exit(EXIT_FAILURE);
}
}
version = detect_imximage_version(imx_hdr);
@@ -410,19 +492,81 @@ static void print_hdr_v2(struct imx_header *imx_hdr) printf("Image Type: Freescale IMX Boot Image\n"); printf("Image Ver: %x", version); printf("%s\n", get_table_entry_name(imximage_versions, NULL, version));
- printf("Data Size: ");
- genimg_print_size(hdr_v2->boot_data.size);
- printf("Load Address: %08x\n", (uint32_t)fhdr_v2->boot_data_ptr);
- printf("Entry Point: %08x\n", (uint32_t)fhdr_v2->entry);
- if (fhdr_v2->csf && (imximage_ivt_offset != UNDEFINED) &&
(imximage_csf_size != UNDEFINED)) {
printf("HAB Blocks: %08x %08x %08x\n",
(uint32_t)fhdr_v2->self, 0,
hdr_v2->boot_data.size - imximage_ivt_offset -
imximage_csf_size);
- printf("Mode: %s\n", plugin ? "PLUGIN" : "DCD");
- if (!plugin) {
printf("Data Size: ");
genimg_print_size(hdr_v2->boot_data.size);
printf("Load Address: %08x\n", (uint32_t)fhdr_v2->boot_data_ptr);
printf("Entry Point: %08x\n", (uint32_t)fhdr_v2->entry);
if (fhdr_v2->csf && (imximage_ivt_offset != UNDEFINED) &&
(imximage_csf_size != UNDEFINED)) {
printf("HAB Blocks: %08x %08x %08x\n",
(uint32_t)fhdr_v2->self, 0,
hdr_v2->boot_data.size - imximage_ivt_offset -
imximage_csf_size);
}
- } else {
imx_header_v2_t *next_hdr_v2;
flash_header_v2_t *next_fhdr_v2;
/*First Header*/
printf("Plugin Data Size: ");
genimg_print_size(hdr_v2->boot_data.size);
printf("Plugin Code Size: ");
genimg_print_size(imximage_plugin_size);
printf("Plugin Load Address: %08x\n", hdr_v2->boot_data.start);
printf("Plugin Entry Point: %08x\n", (uint32_t)fhdr_v2->entry);
/*Second Header*/
next_hdr_v2 = (imx_header_v2_t *)((char *)hdr_v2 +
imximage_plugin_size);
next_fhdr_v2 = &next_hdr_v2->fhdr;
printf("U-Boot Data Size: ");
genimg_print_size(next_hdr_v2->boot_data.size);
printf("U-Boot Load Address: %08x\n",
next_hdr_v2->boot_data.start);
printf("U-Boot Entry Point: %08x\n",
}(uint32_t)next_fhdr_v2->entry);
}
+static void copy_plugin_code(struct imx_header *imxhdr, char *plugin_file) +{
- int ifd = -1;
- struct stat sbuf;
- char *plugin_buf = imxhdr->header.hdr_v2.data.plugin_code;
- char *ptr;
- ifd = open(plugin_file, O_RDONLY|O_BINARY);
- if (fstat(ifd, &sbuf) < 0) {
fprintf(stderr, "Can't stat %s: %s\n",
plugin_file,
strerror(errno));
exit(EXIT_FAILURE);
- }
- ptr = mmap(0, sbuf.st_size, PROT_READ, MAP_SHARED, ifd, 0);
- if (ptr == MAP_FAILED) {
fprintf(stderr, "Can't read %s: %s\n",
plugin_file,
strerror(errno));
exit(EXIT_FAILURE);
- }
- if (sbuf.st_size > MAX_PLUGIN_CODE_SIZE) {
printf("plugin binary size too large\n");
exit(EXIT_FAILURE);
- }
- memcpy(plugin_buf, ptr, sbuf.st_size);
- imximage_plugin_size = sbuf.st_size;
- (void) munmap((void *)ptr, sbuf.st_size);
- (void) close(ifd);
- imxhdr->header.hdr_v2.boot_data.plugin = 1;
+}
static void parse_cfg_cmd(struct imx_header *imxhdr, int32_t cmd, char *token, char *name, int lineno, int fld, int dcd_len) { @@ -497,6 +641,10 @@ static void parse_cfg_cmd(struct imx_header *imxhdr, int32_t cmd, char *token, if (unlikely(cmd_ver_first != 1)) cmd_ver_first = 0; break;
This structure of parse_cfg_cmd to handle the first argument and parse_cfg_fld to handle the second argument is unfortunate.
- case CMD_PLUGIN:
plugin_image = 1;
copy_plugin_code(imxhdr, token);
}break;
}
@@ -542,6 +690,10 @@ static void parse_cfg_fld(struct imx_header *imxhdr, int32_t *cmd, } } break;
case CMD_PLUGIN:
value = get_cfg_value(token, name, lineno);
imximage_iram_free_start = value;
default: break; }break;
@@ -649,6 +801,7 @@ static void imximage_set_header(void *ptr, struct stat *sbuf, int ifd, { struct imx_header *imxhdr = (struct imx_header *)ptr; uint32_t dcd_len;
uint32_t header_size;
/*
- In order to not change the old imx cfg file
@@ -665,10 +818,15 @@ static void imximage_set_header(void *ptr, struct stat *sbuf, int ifd, dcd_len = parse_cfg_file(imxhdr, params->imagename);
if (imximage_version == IMXIMAGE_V2) {
if (imximage_init_loadsize < imximage_ivt_offset +
sizeof(imx_header_v2_t))
header_size = sizeof(flash_header_v2_t) + sizeof(boot_data_t);
if (!plugin_image)
header_size += sizeof(dcd_v2_t);
else
header_size += MAX_PLUGIN_CODE_SIZE;
if (imximage_init_loadsize < imximage_ivt_offset + header_size) imximage_init_loadsize = imximage_ivt_offset +
sizeof(imx_header_v2_t);
header_size;
}
/* Set the imx header */
@@ -721,7 +879,7 @@ static int imximage_generate(struct image_tool_params *params, size_t alloc_len; struct stat sbuf; char *datafile = params->datafile;
- uint32_t pad_len;
uint32_t pad_len, header_size;
memset(&imximage_header, 0, sizeof(imximage_header));
@@ -742,15 +900,21 @@ static int imximage_generate(struct image_tool_params *params, /* TODO: check i.MX image V1 handling, for now use 'old' style */ if (imximage_version == IMXIMAGE_V1) { alloc_len = 4096;
} else {header_size = 4096;
if (imximage_init_loadsize < imximage_ivt_offset +
sizeof(imx_header_v2_t))
header_size = sizeof(flash_header_v2_t) + sizeof(boot_data_t);
if (!plugin_image)
header_size += sizeof(dcd_v2_t);
else
header_size += MAX_PLUGIN_CODE_SIZE;
if (imximage_init_loadsize < imximage_ivt_offset + header_size) imximage_init_loadsize = imximage_ivt_offset +
sizeof(imx_header_v2_t);
alloc_len = imximage_init_loadsize - imximage_ivt_offset; }header_size;
- if (alloc_len < sizeof(struct imx_header)) {
- if (alloc_len < header_size) { fprintf(stderr, "%s: header error\n", params->cmdname); exit(EXIT_FAILURE);
diff --git a/tools/imximage.h b/tools/imximage.h index c7b9b5c..fe4dd89 100644 --- a/tools/imximage.h +++ b/tools/imximage.h @@ -9,6 +9,7 @@ #define _IMXIMAGE_H_
#define MAX_HW_CFG_SIZE_V2 220 /* Max number of registers imx can set for v2 */ +#define MAX_PLUGIN_CODE_SIZE (16*1024) #define MAX_HW_CFG_SIZE_V1 60 /* Max number of registers imx can set for v1 */ #define APP_CODE_BARKER 0xB1 #define DCD_BARKER 0xB17219E9 @@ -64,6 +65,7 @@ enum imximage_cmd { CMD_CHECK_BITS_SET, CMD_CHECK_BITS_CLR, CMD_CSF,
- CMD_PLUGIN,
};
enum imximage_fld_types { @@ -164,7 +166,10 @@ typedef struct { typedef struct { flash_header_v2_t fhdr; boot_data_t boot_data;
- dcd_v2_t dcd_table;
- union {
dcd_v2_t dcd_table;
char plugin_code[MAX_PLUGIN_CODE_SIZE];
- } data;
} imx_header_v2_t;
/* The header must be aligned to 4k on MX53 for NAND boot */
Again, I apologize for being slow to review this, but I'm hoping to use this to build a package of SPL (as a plugin) along with U-Boot and I still think it's doable.
A quick proof-of-concept shows that we can load SPL directly as a plugin just by setting byte 0x28 to 1 (the plugin flag), and if we can test for serial download mode as Stefano does in this patch, we can return to the boot loader.
http://lists.denx.de/pipermail/u-boot/2015-December/237555.html
The subsequent patches to require plugin.S seem to get in the way of that.
Regards,
Eric

On Sat, Oct 08, 2016 at 05:26:18PM +0200, Eric Nelson wrote:
Hi Peng,
I'm sorry for taking so long to go though this.
On 10/08/2016 08:58 AM, Peng Fan wrote:
Add plugin support for imximage.
This CONFIG setting doesn't actually affect mkimage or imximage:
Yeah. The host tool always support plugin code now. But need to define CONFIG_USE_IMXIMG_PLUGIN to compile plugin.S.
Define CONFIG_USE_IMXIMG_PLUGIN in defconfig to enable using plugin.
Signed-off-by: Peng Fan peng.fan@nxp.com Cc: Stefano Babic sbabic@denx.de Cc: Eric Nelson eric@nelint.com Cc: Ye Li ye.li@nxp.com Reviewed-by: Tom Rini trini@konsulko.com
V3: Fix compile error.
V2: Drop the CONFIG_USE_PLUGIN, make plugin always support in imximage.
tools/imximage.c | 282 +++++++++++++++++++++++++++++++++++++++++++------------ tools/imximage.h | 7 +- 2 files changed, 229 insertions(+), 60 deletions(-)
diff --git a/tools/imximage.c b/tools/imximage.c index 092d550..7fa601e 100644 --- a/tools/imximage.c +++ b/tools/imximage.c @@ -27,6 +27,7 @@ static table_entry_t imximage_cmds[] = { {CMD_CHECK_BITS_CLR, "CHECK_BITS_CLR", "Reg Check bits clr", }, {CMD_CSF, "CSF", "Command Sequence File", }, {CMD_IMAGE_VERSION, "IMAGE_VERSION", "image version", },
- {CMD_PLUGIN, "PLUGIN", "file plugin_addr", }, {-1, "", "", },
};
@@ -80,6 +81,9 @@ static uint32_t imximage_ivt_offset = UNDEFINED; static uint32_t imximage_csf_size = UNDEFINED; /* Initial Load Region Size */ static uint32_t imximage_init_loadsize;
These seem very limiting.
From what I can tell, there's no reason that you can't have multiple plugins, and the use of these variables isn't really needed.
I try to follow, are you talking about chained plugin images? Now we only support two IVT headers when using plugin. The first IVT header is for the plugin image, the second ivt header is for uboot image.
+static uint32_t imximage_iram_free_start; +static uint32_t imximage_plugin_size; +static uint32_t plugin_image;
static set_dcd_val_t set_dcd_val; static set_dcd_param_t set_dcd_param; @@ -118,7 +122,11 @@ static uint32_t detect_imximage_version(struct imx_header *imx_hdr)
/* Try to detect V2 */ if ((fhdr_v2->header.tag == IVT_HEADER_TAG) &&
(hdr_v2->dcd_table.header.tag == DCD_HEADER_TAG))
(hdr_v2->data.dcd_table.header.tag == DCD_HEADER_TAG))
return IMXIMAGE_V2;
if ((fhdr_v2->header.tag == IVT_HEADER_TAG) &&
hdr_v2->boot_data.plugin)
return IMXIMAGE_V2;
return IMXIMAGE_VER_INVALID;
@@ -165,7 +173,7 @@ static struct dcd_v2_cmd *gd_last_cmd; static void set_dcd_param_v2(struct imx_header *imxhdr, uint32_t dcd_len, int32_t cmd) {
I also don't understand why the choice to make the union with either a DCD table or a plugin.
Confirmed with ROM team. DCD and plugin are exclusive,
You can not have both at the same time.
We should be able to have both, and this doesn't make the code any easier.
- dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.dcd_table;
- dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.data.dcd_table; struct dcd_v2_cmd *d = gd_last_cmd; struct dcd_v2_cmd *d2; int len;
@@ -261,21 +269,23 @@ static void set_dcd_rst_v1(struct imx_header *imxhdr, uint32_t dcd_len, static void set_dcd_rst_v2(struct imx_header *imxhdr, uint32_t dcd_len, char *name, int lineno) {
- dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.dcd_table;
- struct dcd_v2_cmd *d = gd_last_cmd;
- int len;
- if (!d)
d = &dcd_v2->dcd_cmd;
- len = be16_to_cpu(d->write_dcd_command.length);
- if (len > 4)
d = (struct dcd_v2_cmd *)(((char *)d) + len);
- len = (char *)d - (char *)&dcd_v2->header;
- dcd_v2->header.tag = DCD_HEADER_TAG;
- dcd_v2->header.length = cpu_to_be16(len);
- dcd_v2->header.version = DCD_VERSION;
- if (!imxhdr->header.hdr_v2.boot_data.plugin) {
dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.data.dcd_table;
struct dcd_v2_cmd *d = gd_last_cmd;
int len;
if (!d)
d = &dcd_v2->dcd_cmd;
len = be16_to_cpu(d->write_dcd_command.length);
if (len > 4)
d = (struct dcd_v2_cmd *)(((char *)d) + len);
len = (char *)d - (char *)&dcd_v2->header;
dcd_v2->header.tag = DCD_HEADER_TAG;
dcd_v2->header.length = cpu_to_be16(len);
dcd_v2->header.version = DCD_VERSION;
- }
}
static void set_imx_hdr_v1(struct imx_header *imxhdr, uint32_t dcd_len, @@ -317,24 +327,93 @@ static void set_imx_hdr_v2(struct imx_header *imxhdr, uint32_t dcd_len, fhdr_v2->header.length = cpu_to_be16(sizeof(flash_header_v2_t)); fhdr_v2->header.version = IVT_VERSION; /* 0x40 */
It seems that the reason for a lot of this special-purpose code is to add support for the entry address.
If mkimage is invoked with an entrypoint from the command-line:
~/$ ./tools/mkimage -n my.cfg -T imximage -e 0xentrypoint u-boot.img u-boot.imx
This entry point is normally placed into the header, but if we have a plugin in the .cfg file like this:
~/$ grep PLUGIN my.cfg PLUGIN path/to/board/plugin.bin 0x00907000
Then we need to use the plugin's address instead of the command line address, and use the command-line address for the file which follows.
There are two IVT headers when using plugin, the 0x907000 is for the first IVT header, 0x87800000 in command line is for the second IVT header.
- fhdr_v2->entry = entry_point;
- fhdr_v2->reserved1 = fhdr_v2->reserved2 = 0;
- hdr_base = entry_point - imximage_init_loadsize +
flash_offset;
- fhdr_v2->self = hdr_base;
- if (dcd_len > 0)
fhdr_v2->dcd_ptr = hdr_base
+ offsetof(imx_header_v2_t, dcd_table);
- else
- if (!hdr_v2->boot_data.plugin) {
fhdr_v2->entry = entry_point;
fhdr_v2->reserved1 = 0;
fhdr_v2->reserved1 = 0;
hdr_base = entry_point - imximage_init_loadsize +
flash_offset;
fhdr_v2->self = hdr_base;
if (dcd_len > 0)
fhdr_v2->dcd_ptr = hdr_base +
offsetof(imx_header_v2_t, data);
else
fhdr_v2->dcd_ptr = 0;
fhdr_v2->boot_data_ptr = hdr_base
+ offsetof(imx_header_v2_t, boot_data);
hdr_v2->boot_data.start = entry_point - imximage_init_loadsize;
fhdr_v2->csf = 0;
header_size_ptr = &hdr_v2->boot_data.size;
csf_ptr = &fhdr_v2->csf;
- } else {
imx_header_v2_t *next_hdr_v2;
flash_header_v2_t *next_fhdr_v2;
if (imximage_csf_size != 0) {
fprintf(stderr, "Error: Header v2: SECURE_BOOT is only supported in DCD mode!");
exit(EXIT_FAILURE);
}
I think that only ->entry needs to be different between these two blocks.
There are two IVT headers for plugin. In this block, the second IVT also needs to be handled.
fhdr_v2->entry = imximage_iram_free_start +
flash_offset + sizeof(flash_header_v2_t) +
sizeof(boot_data_t);
fhdr_v2->reserved1 = 0;
fhdr_v2->reserved2 = 0;
fhdr_v2->self = imximage_iram_free_start + flash_offset;
- fhdr_v2->dcd_ptr = 0;
fhdr_v2->boot_data_ptr = hdr_base
+ offsetof(imx_header_v2_t, boot_data);
hdr_v2->boot_data.start = entry_point - imximage_init_loadsize;
fhdr_v2->csf = 0;
fhdr_v2->boot_data_ptr = fhdr_v2->self +
offsetof(imx_header_v2_t, boot_data);
hdr_v2->boot_data.start = imximage_iram_free_start;
/*
* The actural size of plugin image is "imximage_plugin_size +
* sizeof(flash_header_v2_t) + sizeof(boot_data_t)", plus the
* flash_offset space.The ROM code only need to copy this size
* to run the plugin code. However, later when copy the whole
* U-Boot image to DDR, the ROM code use memcpy to copy the
* first part of the image, and use the storage read function
* to get the remaining part. This requires the dividing point
* must be multiple of storage sector size. Here we set the
* first section to be 16KB for this purpose.
*/
Where is the 16k limit coming from? I don't think this is necessary, and if it is, we won't be able to load SPL using a plugin.
Confirmed with ROM team, there is no limitation. But SPL code needs to be small to be in OCRAM. I'll rework this piece code to drop this limitation.
hdr_v2->boot_data.size = MAX_PLUGIN_CODE_SIZE;
/* Security feature are not supported */
fhdr_v2->csf = 0;
next_hdr_v2 = (imx_header_v2_t *)((char *)hdr_v2 +
imximage_plugin_size);
next_fhdr_v2 = &next_hdr_v2->fhdr;
next_fhdr_v2->header.tag = IVT_HEADER_TAG; /* 0xD1 */
next_fhdr_v2->header.length =
cpu_to_be16(sizeof(flash_header_v2_t));
next_fhdr_v2->header.version = IVT_VERSION; /* 0x40 */
next_fhdr_v2->entry = entry_point;
hdr_base = entry_point - sizeof(struct imx_header);
next_fhdr_v2->reserved1 = 0;
next_fhdr_v2->reserved2 = 0;
next_fhdr_v2->self = hdr_base + imximage_plugin_size;
next_fhdr_v2->dcd_ptr = 0;
next_fhdr_v2->boot_data_ptr = next_fhdr_v2->self +
offsetof(imx_header_v2_t, boot_data);
next_hdr_v2->boot_data.start = hdr_base - flash_offset;
header_size_ptr = &next_hdr_v2->boot_data.size;
- header_size_ptr = &hdr_v2->boot_data.size;
- csf_ptr = &fhdr_v2->csf;
next_hdr_v2->boot_data.plugin = 0;
next_fhdr_v2->csf = 0;
- }
}
static void set_hdr_func(void) @@ -393,16 +472,19 @@ static void print_hdr_v2(struct imx_header *imx_hdr) { imx_header_v2_t *hdr_v2 = &imx_hdr->header.hdr_v2; flash_header_v2_t *fhdr_v2 = &hdr_v2->fhdr;
- dcd_v2_t *dcd_v2 = &hdr_v2->dcd_table;
- uint32_t size, version;
- dcd_v2_t *dcd_v2 = &hdr_v2->data.dcd_table;
- uint32_t size, version, plugin;
- size = be16_to_cpu(dcd_v2->header.length);
- if (size > (MAX_HW_CFG_SIZE_V2 * sizeof(dcd_addr_data_t)) + 8) {
fprintf(stderr,
"Error: Image corrupt DCD size %d exceed maximum %d\n",
(uint32_t)(size / sizeof(dcd_addr_data_t)),
MAX_HW_CFG_SIZE_V2);
exit(EXIT_FAILURE);
plugin = hdr_v2->boot_data.plugin;
if (!plugin) {
size = be16_to_cpu(dcd_v2->header.length);
if (size > (MAX_HW_CFG_SIZE_V2 * sizeof(dcd_addr_data_t))) {
fprintf(stderr,
"Error: Image corrupt DCD size %d exceed maximum %d\n",
(uint32_t)(size / sizeof(dcd_addr_data_t)),
MAX_HW_CFG_SIZE_V2);
exit(EXIT_FAILURE);
}
}
version = detect_imximage_version(imx_hdr);
@@ -410,19 +492,81 @@ static void print_hdr_v2(struct imx_header *imx_hdr) printf("Image Type: Freescale IMX Boot Image\n"); printf("Image Ver: %x", version); printf("%s\n", get_table_entry_name(imximage_versions, NULL, version));
- printf("Data Size: ");
- genimg_print_size(hdr_v2->boot_data.size);
- printf("Load Address: %08x\n", (uint32_t)fhdr_v2->boot_data_ptr);
- printf("Entry Point: %08x\n", (uint32_t)fhdr_v2->entry);
- if (fhdr_v2->csf && (imximage_ivt_offset != UNDEFINED) &&
(imximage_csf_size != UNDEFINED)) {
printf("HAB Blocks: %08x %08x %08x\n",
(uint32_t)fhdr_v2->self, 0,
hdr_v2->boot_data.size - imximage_ivt_offset -
imximage_csf_size);
- printf("Mode: %s\n", plugin ? "PLUGIN" : "DCD");
- if (!plugin) {
printf("Data Size: ");
genimg_print_size(hdr_v2->boot_data.size);
printf("Load Address: %08x\n", (uint32_t)fhdr_v2->boot_data_ptr);
printf("Entry Point: %08x\n", (uint32_t)fhdr_v2->entry);
if (fhdr_v2->csf && (imximage_ivt_offset != UNDEFINED) &&
(imximage_csf_size != UNDEFINED)) {
printf("HAB Blocks: %08x %08x %08x\n",
(uint32_t)fhdr_v2->self, 0,
hdr_v2->boot_data.size - imximage_ivt_offset -
imximage_csf_size);
}
- } else {
imx_header_v2_t *next_hdr_v2;
flash_header_v2_t *next_fhdr_v2;
/*First Header*/
printf("Plugin Data Size: ");
genimg_print_size(hdr_v2->boot_data.size);
printf("Plugin Code Size: ");
genimg_print_size(imximage_plugin_size);
printf("Plugin Load Address: %08x\n", hdr_v2->boot_data.start);
printf("Plugin Entry Point: %08x\n", (uint32_t)fhdr_v2->entry);
/*Second Header*/
next_hdr_v2 = (imx_header_v2_t *)((char *)hdr_v2 +
imximage_plugin_size);
next_fhdr_v2 = &next_hdr_v2->fhdr;
printf("U-Boot Data Size: ");
genimg_print_size(next_hdr_v2->boot_data.size);
printf("U-Boot Load Address: %08x\n",
next_hdr_v2->boot_data.start);
printf("U-Boot Entry Point: %08x\n",
}(uint32_t)next_fhdr_v2->entry);
}
+static void copy_plugin_code(struct imx_header *imxhdr, char *plugin_file) +{
- int ifd = -1;
- struct stat sbuf;
- char *plugin_buf = imxhdr->header.hdr_v2.data.plugin_code;
- char *ptr;
- ifd = open(plugin_file, O_RDONLY|O_BINARY);
- if (fstat(ifd, &sbuf) < 0) {
fprintf(stderr, "Can't stat %s: %s\n",
plugin_file,
strerror(errno));
exit(EXIT_FAILURE);
- }
- ptr = mmap(0, sbuf.st_size, PROT_READ, MAP_SHARED, ifd, 0);
- if (ptr == MAP_FAILED) {
fprintf(stderr, "Can't read %s: %s\n",
plugin_file,
strerror(errno));
exit(EXIT_FAILURE);
- }
- if (sbuf.st_size > MAX_PLUGIN_CODE_SIZE) {
printf("plugin binary size too large\n");
exit(EXIT_FAILURE);
- }
- memcpy(plugin_buf, ptr, sbuf.st_size);
- imximage_plugin_size = sbuf.st_size;
- (void) munmap((void *)ptr, sbuf.st_size);
- (void) close(ifd);
- imxhdr->header.hdr_v2.boot_data.plugin = 1;
+}
static void parse_cfg_cmd(struct imx_header *imxhdr, int32_t cmd, char *token, char *name, int lineno, int fld, int dcd_len) { @@ -497,6 +641,10 @@ static void parse_cfg_cmd(struct imx_header *imxhdr, int32_t cmd, char *token, if (unlikely(cmd_ver_first != 1)) cmd_ver_first = 0; break;
This structure of parse_cfg_cmd to handle the first argument and parse_cfg_fld to handle the second argument is unfortunate.
parse_cfg_cmd is to parse the CMD, parse_cfg_fld is to handle the second argument. This is the design of imximage.c to parse xx.cfg file. I do not want to break this.
- case CMD_PLUGIN:
plugin_image = 1;
copy_plugin_code(imxhdr, token);
}break;
}
@@ -542,6 +690,10 @@ static void parse_cfg_fld(struct imx_header *imxhdr, int32_t *cmd, } } break;
case CMD_PLUGIN:
value = get_cfg_value(token, name, lineno);
imximage_iram_free_start = value;
default: break; }break;
@@ -649,6 +801,7 @@ static void imximage_set_header(void *ptr, struct stat *sbuf, int ifd, { struct imx_header *imxhdr = (struct imx_header *)ptr; uint32_t dcd_len;
uint32_t header_size;
/*
- In order to not change the old imx cfg file
@@ -665,10 +818,15 @@ static void imximage_set_header(void *ptr, struct stat *sbuf, int ifd, dcd_len = parse_cfg_file(imxhdr, params->imagename);
if (imximage_version == IMXIMAGE_V2) {
if (imximage_init_loadsize < imximage_ivt_offset +
sizeof(imx_header_v2_t))
header_size = sizeof(flash_header_v2_t) + sizeof(boot_data_t);
if (!plugin_image)
header_size += sizeof(dcd_v2_t);
else
header_size += MAX_PLUGIN_CODE_SIZE;
if (imximage_init_loadsize < imximage_ivt_offset + header_size) imximage_init_loadsize = imximage_ivt_offset +
sizeof(imx_header_v2_t);
header_size;
}
/* Set the imx header */
@@ -721,7 +879,7 @@ static int imximage_generate(struct image_tool_params *params, size_t alloc_len; struct stat sbuf; char *datafile = params->datafile;
- uint32_t pad_len;
uint32_t pad_len, header_size;
memset(&imximage_header, 0, sizeof(imximage_header));
@@ -742,15 +900,21 @@ static int imximage_generate(struct image_tool_params *params, /* TODO: check i.MX image V1 handling, for now use 'old' style */ if (imximage_version == IMXIMAGE_V1) { alloc_len = 4096;
} else {header_size = 4096;
if (imximage_init_loadsize < imximage_ivt_offset +
sizeof(imx_header_v2_t))
header_size = sizeof(flash_header_v2_t) + sizeof(boot_data_t);
if (!plugin_image)
header_size += sizeof(dcd_v2_t);
else
header_size += MAX_PLUGIN_CODE_SIZE;
if (imximage_init_loadsize < imximage_ivt_offset + header_size) imximage_init_loadsize = imximage_ivt_offset +
sizeof(imx_header_v2_t);
alloc_len = imximage_init_loadsize - imximage_ivt_offset; }header_size;
- if (alloc_len < sizeof(struct imx_header)) {
- if (alloc_len < header_size) { fprintf(stderr, "%s: header error\n", params->cmdname); exit(EXIT_FAILURE);
diff --git a/tools/imximage.h b/tools/imximage.h index c7b9b5c..fe4dd89 100644 --- a/tools/imximage.h +++ b/tools/imximage.h @@ -9,6 +9,7 @@ #define _IMXIMAGE_H_
#define MAX_HW_CFG_SIZE_V2 220 /* Max number of registers imx can set for v2 */ +#define MAX_PLUGIN_CODE_SIZE (16*1024) #define MAX_HW_CFG_SIZE_V1 60 /* Max number of registers imx can set for v1 */ #define APP_CODE_BARKER 0xB1 #define DCD_BARKER 0xB17219E9 @@ -64,6 +65,7 @@ enum imximage_cmd { CMD_CHECK_BITS_SET, CMD_CHECK_BITS_CLR, CMD_CSF,
- CMD_PLUGIN,
};
enum imximage_fld_types { @@ -164,7 +166,10 @@ typedef struct { typedef struct { flash_header_v2_t fhdr; boot_data_t boot_data;
- dcd_v2_t dcd_table;
- union {
dcd_v2_t dcd_table;
char plugin_code[MAX_PLUGIN_CODE_SIZE];
- } data;
} imx_header_v2_t;
/* The header must be aligned to 4k on MX53 for NAND boot */
Again, I apologize for being slow to review this, but I'm hoping to use this to build a package of SPL (as a plugin) along with U-Boot and I still think it's doable.
A quick proof-of-concept shows that we can load SPL directly as a plugin just by setting byte 0x28 to 1
some more work needed to use SPL as a plugin image, I think, directly use "PLUGIN xxx/SPL" may not work.
mx6_plugin.S is needed.
Regards, Peng.
(the plugin flag), and if we can test for serial download mode as Stefano does in this patch, we can return to the boot loader.
http://lists.denx.de/pipermail/u-boot/2015-December/237555.html
The subsequent patches to require plugin.S seem to get in the way of that.
Regards,
Eric

Hi Peng,
On 10/09/2016 04:20 AM, Peng Fan wrote:
On Sat, Oct 08, 2016 at 05:26:18PM +0200, Eric Nelson wrote:
On 10/08/2016 08:58 AM, Peng Fan wrote:
<snip>
From what I can tell, there's no reason that you can't have multiple plugins, and the use of these variables isn't really needed.
I try to follow, are you talking about chained plugin images? Now we only support two IVT headers when using plugin. The first IVT header is for the plugin image, the second ivt header is for uboot image.
I understand, though the API seems to allow it (chained plugins) and I have a suspicion that the code would be cleaner without the union.
That said, I don't have a use case in mind.
+static uint32_t imximage_iram_free_start; +static uint32_t imximage_plugin_size; +static uint32_t plugin_image;
static set_dcd_val_t set_dcd_val; static set_dcd_param_t set_dcd_param; @@ -118,7 +122,11 @@ static uint32_t detect_imximage_version(struct imx_header *imx_hdr)
/* Try to detect V2 */ if ((fhdr_v2->header.tag == IVT_HEADER_TAG) &&
(hdr_v2->dcd_table.header.tag == DCD_HEADER_TAG))
(hdr_v2->data.dcd_table.header.tag == DCD_HEADER_TAG))
return IMXIMAGE_V2;
if ((fhdr_v2->header.tag == IVT_HEADER_TAG) &&
hdr_v2->boot_data.plugin)
return IMXIMAGE_V2;
return IMXIMAGE_VER_INVALID;
@@ -165,7 +173,7 @@ static struct dcd_v2_cmd *gd_last_cmd; static void set_dcd_param_v2(struct imx_header *imxhdr, uint32_t dcd_len, int32_t cmd) {
I also don't understand why the choice to make the union with either a DCD table or a plugin.
Confirmed with ROM team. DCD and plugin are exclusive,
You can not have both at the same time.
That's too bad, since porting from DCD-style to SPL can be useful (as Fabio has seen trying to keep some boards up-to-date).
If we get SPL-as-plugin working, then this would form a dividing line where you need to get rid of the DCD altogether.
What about the CSF test? Your patches say that CSF and plugins are also not supported.
We should be able to have both, and this doesn't make the code any easier.
- dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.dcd_table;
- dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.data.dcd_table; struct dcd_v2_cmd *d = gd_last_cmd; struct dcd_v2_cmd *d2; int len;
@@ -261,21 +269,23 @@ static void set_dcd_rst_v1(struct imx_header *imxhdr, uint32_t dcd_len, static void set_dcd_rst_v2(struct imx_header *imxhdr, uint32_t dcd_len, char *name, int lineno) {
- dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.dcd_table;
- struct dcd_v2_cmd *d = gd_last_cmd;
- int len;
- if (!d)
d = &dcd_v2->dcd_cmd;
- len = be16_to_cpu(d->write_dcd_command.length);
- if (len > 4)
d = (struct dcd_v2_cmd *)(((char *)d) + len);
- len = (char *)d - (char *)&dcd_v2->header;
- dcd_v2->header.tag = DCD_HEADER_TAG;
- dcd_v2->header.length = cpu_to_be16(len);
- dcd_v2->header.version = DCD_VERSION;
- if (!imxhdr->header.hdr_v2.boot_data.plugin) {
dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.data.dcd_table;
struct dcd_v2_cmd *d = gd_last_cmd;
int len;
if (!d)
d = &dcd_v2->dcd_cmd;
len = be16_to_cpu(d->write_dcd_command.length);
if (len > 4)
d = (struct dcd_v2_cmd *)(((char *)d) + len);
len = (char *)d - (char *)&dcd_v2->header;
dcd_v2->header.tag = DCD_HEADER_TAG;
dcd_v2->header.length = cpu_to_be16(len);
dcd_v2->header.version = DCD_VERSION;
- }
}
static void set_imx_hdr_v1(struct imx_header *imxhdr, uint32_t dcd_len, @@ -317,24 +327,93 @@ static void set_imx_hdr_v2(struct imx_header *imxhdr, uint32_t dcd_len, fhdr_v2->header.length = cpu_to_be16(sizeof(flash_header_v2_t)); fhdr_v2->header.version = IVT_VERSION; /* 0x40 */
It seems that the reason for a lot of this special-purpose code is to add support for the entry address.
If mkimage is invoked with an entrypoint from the command-line:
~/$ ./tools/mkimage -n my.cfg -T imximage -e 0xentrypoint u-boot.img u-boot.imx
This entry point is normally placed into the header, but if we have a plugin in the .cfg file like this:
~/$ grep PLUGIN my.cfg PLUGIN path/to/board/plugin.bin 0x00907000
Then we need to use the plugin's address instead of the command line address, and use the command-line address for the file which follows.
There are two IVT headers when using plugin, the 0x907000 is for the first IVT header, 0x87800000 in command line is for the second IVT header.
- fhdr_v2->entry = entry_point;
- fhdr_v2->reserved1 = fhdr_v2->reserved2 = 0;
- hdr_base = entry_point - imximage_init_loadsize +
flash_offset;
- fhdr_v2->self = hdr_base;
- if (dcd_len > 0)
fhdr_v2->dcd_ptr = hdr_base
+ offsetof(imx_header_v2_t, dcd_table);
- else
- if (!hdr_v2->boot_data.plugin) {
fhdr_v2->entry = entry_point;
fhdr_v2->reserved1 = 0;
fhdr_v2->reserved1 = 0;
hdr_base = entry_point - imximage_init_loadsize +
flash_offset;
fhdr_v2->self = hdr_base;
if (dcd_len > 0)
fhdr_v2->dcd_ptr = hdr_base +
offsetof(imx_header_v2_t, data);
else
fhdr_v2->dcd_ptr = 0;
fhdr_v2->boot_data_ptr = hdr_base
+ offsetof(imx_header_v2_t, boot_data);
hdr_v2->boot_data.start = entry_point - imximage_init_loadsize;
fhdr_v2->csf = 0;
header_size_ptr = &hdr_v2->boot_data.size;
csf_ptr = &fhdr_v2->csf;
- } else {
imx_header_v2_t *next_hdr_v2;
flash_header_v2_t *next_fhdr_v2;
if (imximage_csf_size != 0) {
fprintf(stderr, "Error: Header v2: SECURE_BOOT is only supported in DCD mode!");
exit(EXIT_FAILURE);
}
I think that only ->entry needs to be different between these two blocks.
There are two IVT headers for plugin. In this block, the second IVT also needs to be handled.
I understand. It just appears to me that almost all of the code that manipulates fhdr_v2 in the two blocks of code could be done before the test.
Only the processing of fhdr_v2->entry and next_fhdr_v2 needs to be different.
fhdr_v2->entry = imximage_iram_free_start +
flash_offset + sizeof(flash_header_v2_t) +
sizeof(boot_data_t);
fhdr_v2->reserved1 = 0;
fhdr_v2->reserved2 = 0;
fhdr_v2->self = imximage_iram_free_start + flash_offset;
- fhdr_v2->dcd_ptr = 0;
fhdr_v2->boot_data_ptr = hdr_base
+ offsetof(imx_header_v2_t, boot_data);
hdr_v2->boot_data.start = entry_point - imximage_init_loadsize;
fhdr_v2->csf = 0;
fhdr_v2->boot_data_ptr = fhdr_v2->self +
offsetof(imx_header_v2_t, boot_data);
hdr_v2->boot_data.start = imximage_iram_free_start;
/*
* The actural size of plugin image is "imximage_plugin_size +
* sizeof(flash_header_v2_t) + sizeof(boot_data_t)", plus the
* flash_offset space.The ROM code only need to copy this size
* to run the plugin code. However, later when copy the whole
* U-Boot image to DDR, the ROM code use memcpy to copy the
* first part of the image, and use the storage read function
* to get the remaining part. This requires the dividing point
* must be multiple of storage sector size. Here we set the
* first section to be 16KB for this purpose.
*/
Where is the 16k limit coming from? I don't think this is necessary, and if it is, we won't be able to load SPL using a plugin.
Confirmed with ROM team, there is no limitation. But SPL code needs to be small to be in OCRAM.
Whew! This would have killed the idea of SPL-as-plugin.
I'll rework this piece code to drop this limitation.
<snip>
This structure of parse_cfg_cmd to handle the first argument and parse_cfg_fld to handle the second argument is unfortunate.
parse_cfg_cmd is to parse the CMD, parse_cfg_fld is to handle the second argument. This is the design of imximage.c to parse xx.cfg file. I do not want to break this.
I'd say that it's a bit broken already, but that has nothing to do with your patch.
The parsing model seems to have outlived its' usefulness and the use of the CFG_COMMAND, CFG_REG_SIZE, et cetera to mean parameter numbers doesn't fit for many (any?) commands.
<snip>
/* The header must be aligned to 4k on MX53 for NAND boot */
Again, I apologize for being slow to review this, but I'm hoping to use this to build a package of SPL (as a plugin) along with U-Boot and I still think it's doable.
A quick proof-of-concept shows that we can load SPL directly as a plugin just by setting byte 0x28 to 1
some more work needed to use SPL as a plugin image, I think, directly use "PLUGIN xxx/SPL" may not work.
mx6_plugin.S is needed.
Well, some modification to the startup is needed to save the context for an eventual return to ROM, but if we're trying to use SPL to perform SoC and DDR initialization, it can't be a stand-alone plugin.S.
Regards,
Eric

Hi Eric, On Sun, Oct 09, 2016 at 07:42:34AM +0200, Eric Nelson wrote:
Hi Peng,
On 10/09/2016 04:20 AM, Peng Fan wrote:
On Sat, Oct 08, 2016 at 05:26:18PM +0200, Eric Nelson wrote:
On 10/08/2016 08:58 AM, Peng Fan wrote:
<snip>
From what I can tell, there's no reason that you can't have multiple plugins, and the use of these variables isn't really needed.
I try to follow, are you talking about chained plugin images? Now we only support two IVT headers when using plugin. The first IVT header is for the plugin image, the second ivt header is for uboot image.
I understand, though the API seems to allow it (chained plugins) and I have a suspicion that the code would be cleaner without the union.
That said, I don't have a use case in mind.
+static uint32_t imximage_iram_free_start; +static uint32_t imximage_plugin_size; +static uint32_t plugin_image;
static set_dcd_val_t set_dcd_val; static set_dcd_param_t set_dcd_param; @@ -118,7 +122,11 @@ static uint32_t detect_imximage_version(struct imx_header *imx_hdr)
/* Try to detect V2 */ if ((fhdr_v2->header.tag == IVT_HEADER_TAG) &&
(hdr_v2->dcd_table.header.tag == DCD_HEADER_TAG))
(hdr_v2->data.dcd_table.header.tag == DCD_HEADER_TAG))
return IMXIMAGE_V2;
if ((fhdr_v2->header.tag == IVT_HEADER_TAG) &&
hdr_v2->boot_data.plugin)
return IMXIMAGE_V2;
return IMXIMAGE_VER_INVALID;
@@ -165,7 +173,7 @@ static struct dcd_v2_cmd *gd_last_cmd; static void set_dcd_param_v2(struct imx_header *imxhdr, uint32_t dcd_len, int32_t cmd) {
I also don't understand why the choice to make the union with either a DCD table or a plugin.
Confirmed with ROM team. DCD and plugin are exclusive,
You can not have both at the same time.
That's too bad, since porting from DCD-style to SPL can be useful (as Fabio has seen trying to keep some boards up-to-date).
I saw the patches. If using plugin, there is no need to use DCD.
all the things in DCD can be done in plugin code.
If we get SPL-as-plugin working, then this would form a dividing line where you need to get rid of the DCD altogether.
What about the CSF test? Your patches say that CSF and plugins are also not supported.
CSF is supported and code in nxp community. But that piece code is not included in the nxp released uboot.
https://community.nxp.com/docs/DOC-332725
We should be able to have both, and this doesn't make the code any easier.
- dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.dcd_table;
- dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.data.dcd_table; struct dcd_v2_cmd *d = gd_last_cmd; struct dcd_v2_cmd *d2; int len;
@@ -261,21 +269,23 @@ static void set_dcd_rst_v1(struct imx_header *imxhdr, uint32_t dcd_len, static void set_dcd_rst_v2(struct imx_header *imxhdr, uint32_t dcd_len, char *name, int lineno) {
- dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.dcd_table;
- struct dcd_v2_cmd *d = gd_last_cmd;
- int len;
- if (!d)
d = &dcd_v2->dcd_cmd;
- len = be16_to_cpu(d->write_dcd_command.length);
- if (len > 4)
d = (struct dcd_v2_cmd *)(((char *)d) + len);
- len = (char *)d - (char *)&dcd_v2->header;
- dcd_v2->header.tag = DCD_HEADER_TAG;
- dcd_v2->header.length = cpu_to_be16(len);
- dcd_v2->header.version = DCD_VERSION;
- if (!imxhdr->header.hdr_v2.boot_data.plugin) {
dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.data.dcd_table;
struct dcd_v2_cmd *d = gd_last_cmd;
int len;
if (!d)
d = &dcd_v2->dcd_cmd;
len = be16_to_cpu(d->write_dcd_command.length);
if (len > 4)
d = (struct dcd_v2_cmd *)(((char *)d) + len);
len = (char *)d - (char *)&dcd_v2->header;
dcd_v2->header.tag = DCD_HEADER_TAG;
dcd_v2->header.length = cpu_to_be16(len);
dcd_v2->header.version = DCD_VERSION;
- }
}
static void set_imx_hdr_v1(struct imx_header *imxhdr, uint32_t dcd_len, @@ -317,24 +327,93 @@ static void set_imx_hdr_v2(struct imx_header *imxhdr, uint32_t dcd_len, fhdr_v2->header.length = cpu_to_be16(sizeof(flash_header_v2_t)); fhdr_v2->header.version = IVT_VERSION; /* 0x40 */
It seems that the reason for a lot of this special-purpose code is to add support for the entry address.
If mkimage is invoked with an entrypoint from the command-line:
~/$ ./tools/mkimage -n my.cfg -T imximage -e 0xentrypoint u-boot.img u-boot.imx
This entry point is normally placed into the header, but if we have a plugin in the .cfg file like this:
~/$ grep PLUGIN my.cfg PLUGIN path/to/board/plugin.bin 0x00907000
Then we need to use the plugin's address instead of the command line address, and use the command-line address for the file which follows.
There are two IVT headers when using plugin, the 0x907000 is for the first IVT header, 0x87800000 in command line is for the second IVT header.
- fhdr_v2->entry = entry_point;
- fhdr_v2->reserved1 = fhdr_v2->reserved2 = 0;
- hdr_base = entry_point - imximage_init_loadsize +
flash_offset;
- fhdr_v2->self = hdr_base;
- if (dcd_len > 0)
fhdr_v2->dcd_ptr = hdr_base
+ offsetof(imx_header_v2_t, dcd_table);
- else
- if (!hdr_v2->boot_data.plugin) {
fhdr_v2->entry = entry_point;
fhdr_v2->reserved1 = 0;
fhdr_v2->reserved1 = 0;
hdr_base = entry_point - imximage_init_loadsize +
flash_offset;
fhdr_v2->self = hdr_base;
if (dcd_len > 0)
fhdr_v2->dcd_ptr = hdr_base +
offsetof(imx_header_v2_t, data);
else
fhdr_v2->dcd_ptr = 0;
fhdr_v2->boot_data_ptr = hdr_base
+ offsetof(imx_header_v2_t, boot_data);
hdr_v2->boot_data.start = entry_point - imximage_init_loadsize;
fhdr_v2->csf = 0;
header_size_ptr = &hdr_v2->boot_data.size;
csf_ptr = &fhdr_v2->csf;
- } else {
imx_header_v2_t *next_hdr_v2;
flash_header_v2_t *next_fhdr_v2;
if (imximage_csf_size != 0) {
fprintf(stderr, "Error: Header v2: SECURE_BOOT is only supported in DCD mode!");
exit(EXIT_FAILURE);
}
I think that only ->entry needs to be different between these two blocks.
There are two IVT headers for plugin. In this block, the second IVT also needs to be handled.
I understand. It just appears to me that almost all of the code that manipulates fhdr_v2 in the two blocks of code could be done before the test.
Only the processing of fhdr_v2->entry and next_fhdr_v2 needs to be different.
fhdr_v2->entry = imximage_iram_free_start +
flash_offset + sizeof(flash_header_v2_t) +
sizeof(boot_data_t);
fhdr_v2->reserved1 = 0;
fhdr_v2->reserved2 = 0;
fhdr_v2->self = imximage_iram_free_start + flash_offset;
- fhdr_v2->dcd_ptr = 0;
fhdr_v2->boot_data_ptr = hdr_base
+ offsetof(imx_header_v2_t, boot_data);
hdr_v2->boot_data.start = entry_point - imximage_init_loadsize;
fhdr_v2->csf = 0;
fhdr_v2->boot_data_ptr = fhdr_v2->self +
offsetof(imx_header_v2_t, boot_data);
hdr_v2->boot_data.start = imximage_iram_free_start;
/*
* The actural size of plugin image is "imximage_plugin_size +
* sizeof(flash_header_v2_t) + sizeof(boot_data_t)", plus the
* flash_offset space.The ROM code only need to copy this size
* to run the plugin code. However, later when copy the whole
* U-Boot image to DDR, the ROM code use memcpy to copy the
* first part of the image, and use the storage read function
* to get the remaining part. This requires the dividing point
* must be multiple of storage sector size. Here we set the
* first section to be 16KB for this purpose.
*/
Where is the 16k limit coming from? I don't think this is necessary, and if it is, we won't be able to load SPL using a plugin.
Confirmed with ROM team, there is no limitation. But SPL code needs to be small to be in OCRAM.
Whew! This would have killed the idea of SPL-as-plugin.
SPL is designed to run in OCRAM for i.MX6, so it's still ok to let SPL as plugin image.
Regards, Peng.
I'll rework this piece code to drop this limitation.
<snip>
This structure of parse_cfg_cmd to handle the first argument and parse_cfg_fld to handle the second argument is unfortunate.
parse_cfg_cmd is to parse the CMD, parse_cfg_fld is to handle the second argument. This is the design of imximage.c to parse xx.cfg file. I do not want to break this.
I'd say that it's a bit broken already, but that has nothing to do with your patch.
The parsing model seems to have outlived its' usefulness and the use of the CFG_COMMAND, CFG_REG_SIZE, et cetera to mean parameter numbers doesn't fit for many (any?) commands.
<snip>
/* The header must be aligned to 4k on MX53 for NAND boot */
Again, I apologize for being slow to review this, but I'm hoping to use this to build a package of SPL (as a plugin) along with U-Boot and I still think it's doable.
A quick proof-of-concept shows that we can load SPL directly as a plugin just by setting byte 0x28 to 1
some more work needed to use SPL as a plugin image, I think, directly use "PLUGIN xxx/SPL" may not work.
mx6_plugin.S is needed.
Well, some modification to the startup is needed to save the context for an eventual return to ROM, but if we're trying to use SPL to perform SoC and DDR initialization, it can't be a stand-alone plugin.S.
Regards,
Eric

Hi Peng,
On 10/09/2016 08:12 AM, Peng Fan wrote:
On 10/09/2016 04:20 AM, Peng Fan wrote:
On Sat, Oct 08, 2016 at 05:26:18PM +0200, Eric Nelson wrote:
On 10/08/2016 08:58 AM, Peng Fan wrote:
<snip>
I also don't understand why the choice to make the union with either a DCD table or a plugin.
Confirmed with ROM team. DCD and plugin are exclusive,
You can not have both at the same time.
That's too bad, since porting from DCD-style to SPL can be useful (as Fabio has seen trying to keep some boards up-to-date).
I saw the patches. If using plugin, there is no need to use DCD.
all the things in DCD can be done in plugin code.
If we get SPL-as-plugin working, then this would form a dividing line where you need to get rid of the DCD altogether.
What about the CSF test? Your patches say that CSF and plugins are also not supported.
CSF is supported and code in nxp community. But that piece code is not included in the nxp released uboot.
Cool. Otherwise
This page is telling me that "Access is restricted".
<snip>
Where is the 16k limit coming from? I don't think this is necessary, and if it is, we won't be able to load SPL using a plugin.
Confirmed with ROM team, there is no limitation. But SPL code needs to be small to be in OCRAM.
Whew! This would have killed the idea of SPL-as-plugin.
SPL is designed to run in OCRAM for i.MX6, so it's still ok to let SPL as plugin image.
Cool.
I'll take a stab at instrumenting the startup code and let you know what I find.
Regards,
Eric

Hi Peng,
On 10/09/2016 06:48 PM, Eric Nelson wrote:
On 10/09/2016 08:12 AM, Peng Fan wrote:
On 10/09/2016 04:20 AM, Peng Fan wrote:
On Sat, Oct 08, 2016 at 05:26:18PM +0200, Eric Nelson wrote:
On 10/08/2016 08:58 AM, Peng Fan wrote:
<snip>
If we get SPL-as-plugin working, then this would form a dividing line where you need to get rid of the DCD altogether.
What about the CSF test? Your patches say that CSF and plugins are also not supported.
CSF is supported and code in nxp community. But that piece code is not included in the nxp released uboot.
Cool. Otherwise
Sorry. I fat-fingered the <enter> key while I went in search for a link.
What I meant to say was
"Otherwise, we couldn't use HAB and LPSR mode on i.MX7", which is another reason for getting plugins to work."
participants (3)
-
Eric Nelson
-
Peng Fan
-
Peng Fan