Hi, Wolfgang,
2011/1/18 Wolfgang Denk wd@denx.de:
Dear Jason Liu,
In message 1294129662-680-7-git-send-email-r64343@freescale.com you wrote:
This patch add the MX53 boot image support.
This patch has been tested on Freescale MX53EVK board and MX51EVK board.
Signed-off-by: Jason Liu r64343@freescale.com
...
static table_entry_t imximage_cmds[] = {
- {CMD_IMAGE_VERSION, "IMAGE_VERSION", "image version", },
{CMD_BOOT_FROM, "BOOT_FROM", "boot command", }, {CMD_DATA, "DATA", "Reg Write Data", }, {-1, "", "", },
Can we please keep the table sorted?
what does the "keep the table sorted" mean?
{-1, "", "Invalid", }, };
+/*
- IMXIMAGE version definition for i.MX chips
- */
+static table_entry_t imximage_versions[] = {
- {IMXIMAGE_V1, "", " (i.MX25/35/51 compatible)", },
- {IMXIMAGE_V2, "", " (i.MX53 compatible)", },
- {-1, "", " (Invalid)", },
+};
static struct imx_header imximage_header; +static uint32_t imximage_version;
+static set_dcd_val_t set_dcd_val; +static set_dcd_rst_t set_dcd_rst; +static set_imx_hdr_t set_imx_hdr;
static uint32_t get_cfg_value(char *token, char *name, int linenr) { @@ -71,58 +85,264 @@ static uint32_t get_cfg_value(char *token, char *name, int linenr) return value;
- /* Try to detect V1 */
- if (fhdr_v1->app_code_barker == APP_CODE_BARKER &&
- imx_hdr->header.hdr_v1.dcd_table.preamble.barker == DCD_BARKER)
- return IMXIMAGE_V1;
This needs braces. Please fix globally.
+static void set_dcd_rst_v1(struct imx_header *imxhdr, uint32_t dcd_len,
- char *name, int lineno)
Could you please add a comment what the somewhat cryptic name "set_dcd_rst_v1" is supposed to mean? Similar for the other functions like set_dcd_rst_v2() etc.?
OK, I will add some comments for this function.
- if (dcd_len > MAX_HW_CFG_SIZE_V1) {
- fprintf(stderr, "Error: %s[%d] -"
- "DCD table exceeds maximum size(%d)\n",
- name, lineno, MAX_HW_CFG_SIZE_V1);
- }
- dcd_v1->preamble.barker = DCD_BARKER;
- dcd_v1->preamble.length = dcd_len * sizeof(dcd_type_addr_data_t);
+}
It seems these functions can run into error situations - yet they are of type void, i. e. they don't return any information about such errors to their callers. In the end, you cannot test the result oif running the command to find out if it had worked or not.
This must be fixed (globally).
I think this function does not need return something, I will fix as following, is it OK,
static void set_dcd_rst_v1(struct imx_header *imxhdr, uint32_t dcd_len, char *name, int lineno) { dcd_v1_t *dcd_v1 = &imxhdr->header.hdr_v1.dcd_table;
if (dcd_len > MAX_HW_CFG_SIZE_V1) { fprintf(stderr, "Error: %s[%d] -" "DCD table exceeds maximum size(%d)\n", name, lineno, MAX_HW_CFG_SIZE_V1); + exit(EXIT_FAILURE); }
dcd_v1->preamble.barker = DCD_BARKER; dcd_v1->preamble.length = dcd_len * sizeof(dcd_type_addr_data_t); }
- case CMD_IMAGE_VERSION:
- imximage_version =
- get_cfg_value(token,
- name, lineno);
- if (cmd_ver_first == 0) {
- fprintf(stderr,
- "Error: %s[%d] - "
- "IMAGE_VERSION command "
- "need appear the first "
- "before other valid "
- "command in the file\n",
- name, lineno);
- exit(EXIT_FAILURE);
- }
Your nesting is too deep. Consider reorganizing the code.
In fact, I did not change the nesting deep, which is the same deep as the original code, I will reorganizing it.
Stefano, Albert - I apologize for the late review. Please don't pull this into mainline as is. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot