Message ID | 1437000559-3208-1-git-send-email-aalonso@freescale.com |
---|---|
State | Changes Requested |
Headers | show |
Hi Adrian, hi Peng, On 16/07/2015 00:49, Adrian Alonso wrote: > * Extend imximage DCD version 2 to support DCD commands > CMD_WRITE_CLR_BIT 4 [address] [mask bit] means: > while ((*address & ~mask) != 0); > CMD_CHECK_BITS_SET 4 [address] [mask bit] means: > while ((*address & mask) != mask); > CMD_CHECK_BITS_CLR 4 [address] [mask bit] means: > *address = *address & ~mask; > * Add set_dcd_param_v2 helper function to set DCD > command parameters > > Signed-off-by: Adrian Alonso <aalonso@freescale.com> > Signed-off-by: Peng Fan <Peng.Fan@freescale.com> > --- > Changes for V2 > - Add set_dcd_param_v2 helper function to set DCD command tag > and parameters > > tools/imximage.c | 99 ++++++++++++++++++++++++++++++++++++++++++++------------ > tools/imximage.h | 25 ++++++++++---- > 2 files changed, 97 insertions(+), 27 deletions(-) > > diff --git a/tools/imximage.c b/tools/imximage.c > index 6f469ae..cc0392f 100644 > --- a/tools/imximage.c > +++ b/tools/imximage.c > @@ -21,7 +21,10 @@ > static table_entry_t imximage_cmds[] = { > {CMD_BOOT_FROM, "BOOT_FROM", "boot command", }, > {CMD_BOOT_OFFSET, "BOOT_OFFSET", "Boot offset", }, > - {CMD_DATA, "DATA", "Reg Write Data", }, > + {CMD_WRITE_DATA, "DATA", "Reg Write Data", }, > + {CMD_WRITE_CLR_BIT, "CLR_BIT", "Reg clear bit", }, > + {CMD_CHECK_BITS_SET, "CHECK_BITS_SET", "Reg Check bits set", }, > + {CMD_CHECK_BITS_CLR, "CHECK_BITS_CLR", "Reg Check bits clr", }, > {CMD_CSF, "CSF", "Command Sequence File", }, > {CMD_IMAGE_VERSION, "IMAGE_VERSION", "image version", }, > {-1, "", "", }, > @@ -62,7 +65,7 @@ static table_entry_t imximage_boot_loadsize[] = { > */ > static table_entry_t imximage_versions[] = { > {IMXIMAGE_V1, "", " (i.MX25/35/51 compatible)", }, > - {IMXIMAGE_V2, "", " (i.MX53/6 compatible)", }, > + {IMXIMAGE_V2, "", " (i.MX53/6/7 compatible)", }, > {-1, "", " (Invalid)", }, > }; > > @@ -79,6 +82,7 @@ static uint32_t imximage_csf_size = UNDEFINED; > static uint32_t imximage_init_loadsize; > > static set_dcd_val_t set_dcd_val; > +static set_dcd_param_t set_dcd_param; > static set_dcd_rst_t set_dcd_rst; > static set_imx_hdr_t set_imx_hdr; > static uint32_t max_dcd_entries; > @@ -128,6 +132,12 @@ static void err_imximage_version(int version) > exit(EXIT_FAILURE); > } > > +static void set_dcd_param_v1(struct imx_header *imxhdr, uint32_t dcd_len, > + int32_t cmd) > +{ > + /* DCD V1 no parameter settings */ > +} It is better you check if you drop this and you check if the pointer is set to NULL before calling. > + > static void set_dcd_val_v1(struct imx_header *imxhdr, char *name, int lineno, > int fld, uint32_t value, uint32_t off) > { > @@ -156,6 +166,43 @@ static void set_dcd_val_v1(struct imx_header *imxhdr, char *name, int lineno, > } > } > > +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; > + > + switch (cmd) { > + case CMD_WRITE_DATA: > + dcd_v2->write_dcd_command.tag = DCD_WRITE_DATA_COMMAND_TAG; > + dcd_v2->write_dcd_command.length = cpu_to_be16( > + dcd_len * sizeof(dcd_addr_data_t) + 4); > + dcd_v2->write_dcd_command.param = DCD_WRITE_DATA_PARAM; > + break; > + case CMD_WRITE_CLR_BIT: > + dcd_v2->write_dcd_command.tag = DCD_WRITE_DATA_COMMAND_TAG; > + dcd_v2->write_dcd_command.length = cpu_to_be16( > + dcd_len * sizeof(dcd_addr_data_t) + 4); > + dcd_v2->write_dcd_command.param = DCD_WRITE_CLR_BIT_PARAM; > + break; > + case CMD_CHECK_BITS_SET: > + dcd_v2->write_dcd_command.tag = DCD_CHECK_DATA_COMMAND_TAG; > + /* > + * Check data command only supports one entry, > + * so use 0xC = size(address + value + command). > + */ Comment applied to both CHECK_BITS, you could move it before (or put it also in CMD_CHECK_BITS_CLR case). > + dcd_v2->write_dcd_command.length = cpu_to_be16(0xC); > + dcd_v2->write_dcd_command.param = DCD_CHECK_BITS_SET_PARAM; > + break; > + case CMD_CHECK_BITS_CLR: > + dcd_v2->write_dcd_command.tag = DCD_CHECK_DATA_COMMAND_TAG; > + dcd_v2->write_dcd_command.length = cpu_to_be16(0xC); > + dcd_v2->write_dcd_command.param = DCD_CHECK_BITS_SET_PARAM; > + break; > + default: > + break; > + } > +} > + > static void set_dcd_val_v2(struct imx_header *imxhdr, char *name, int lineno, > int fld, uint32_t value, uint32_t off) > { > @@ -200,10 +247,7 @@ static void set_dcd_rst_v2(struct imx_header *imxhdr, uint32_t dcd_len, > dcd_v2->header.length = cpu_to_be16( > dcd_len * sizeof(dcd_addr_data_t) + 8); > dcd_v2->header.version = DCD_VERSION; > - dcd_v2->write_dcd_command.tag = DCD_COMMAND_TAG; > - dcd_v2->write_dcd_command.length = cpu_to_be16( > - dcd_len * sizeof(dcd_addr_data_t) + 4); > - dcd_v2->write_dcd_command.param = DCD_COMMAND_PARAM; > + set_dcd_param_v2(imxhdr, dcd_len, CMD_WRITE_DATA); > } > > static void set_imx_hdr_v1(struct imx_header *imxhdr, uint32_t dcd_len, > @@ -266,12 +310,14 @@ static void set_hdr_func(void) > switch (imximage_version) { > case IMXIMAGE_V1: > set_dcd_val = set_dcd_val_v1; > + set_dcd_param = set_dcd_param_v1; > set_dcd_rst = set_dcd_rst_v1; > set_imx_hdr = set_imx_hdr_v1; > max_dcd_entries = MAX_HW_CFG_SIZE_V1; > break; > case IMXIMAGE_V2: > set_dcd_val = set_dcd_val_v2; > + set_dcd_param = set_dcd_param_v2; > set_dcd_rst = set_dcd_rst_v2; > set_imx_hdr = set_imx_hdr_v2; > max_dcd_entries = MAX_HW_CFG_SIZE_V2; > @@ -396,8 +442,12 @@ 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_DATA: > + case CMD_WRITE_DATA: > + case CMD_WRITE_CLR_BIT: > + case CMD_CHECK_BITS_SET: > + case CMD_CHECK_BITS_CLR: > value = get_cfg_value(token, name, lineno); > + (*set_dcd_param)(imxhdr, dcd_len, cmd); > (*set_dcd_val)(imxhdr, name, lineno, fld, value, dcd_len); > if (unlikely(cmd_ver_first != 1)) > cmd_ver_first = 0; > @@ -436,20 +486,29 @@ static void parse_cfg_fld(struct imx_header *imxhdr, int32_t *cmd, > break; > case CFG_REG_ADDRESS: > case CFG_REG_VALUE: > - if (*cmd != CMD_DATA) > - return; > - > - value = get_cfg_value(token, name, lineno); > - (*set_dcd_val)(imxhdr, name, lineno, fld, value, *dcd_len); > - > - if (fld == CFG_REG_VALUE) { > - (*dcd_len)++; > - if (*dcd_len > max_dcd_entries) { > - fprintf(stderr, "Error: %s[%d] -" > - "DCD table exceeds maximum size(%d)\n", > - name, lineno, max_dcd_entries); > - exit(EXIT_FAILURE); > + switch(*cmd) { > + case CMD_WRITE_DATA: > + case CMD_WRITE_CLR_BIT: > + case CMD_CHECK_BITS_SET: > + case CMD_CHECK_BITS_CLR: > + > + value = get_cfg_value(token, name, lineno); > + (*set_dcd_param)(imxhdr, *dcd_len, *cmd); What I meant before was: if (set_dcd_param) (*set_dcd_param)(imxhdr, *dcd_len, *cmd); and then case IMXIMAGE_V1: set_dcd_val = set_dcd_val_v1; + set_dcd_param = NULL; > + (*set_dcd_val)(imxhdr, name, lineno, fld, value, > + *dcd_len); > + > + if (fld == CFG_REG_VALUE) { > + (*dcd_len)++; > + if (*dcd_len > max_dcd_entries) { > + fprintf(stderr, "Error: %s[%d] -" > + "DCD table exceeds maximum size(%d)\n", > + name, lineno, max_dcd_entries); > + exit(EXIT_FAILURE); > + } > } > + break; > + default: > + break; > } > break; > default: > diff --git a/tools/imximage.h b/tools/imximage.h > index 36fe095..c199814 100644 > --- a/tools/imximage.h > +++ b/tools/imximage.h > @@ -42,19 +42,27 @@ > #define FLASH_LOADSIZE_SATA FLASH_LOADSIZE_STANDARD > #define FLASH_LOADSIZE_QSPI 0x0 /* entire image */ > > -#define IVT_HEADER_TAG 0xD1 > -#define IVT_VERSION 0x40 > -#define DCD_HEADER_TAG 0xD2 > -#define DCD_COMMAND_TAG 0xCC > -#define DCD_VERSION 0x40 > -#define DCD_COMMAND_PARAM 0x4 > +/* Command tags and parameters */ > +#define IVT_HEADER_TAG 0xD1 > +#define IVT_VERSION 0x40 > +#define DCD_HEADER_TAG 0xD2 > +#define DCD_VERSION 0x40 > +#define DCD_WRITE_DATA_COMMAND_TAG 0xCC > +#define DCD_WRITE_DATA_PARAM 0x4 > +#define DCD_WRITE_CLR_BIT_PARAM 0xC > +#define DCD_CHECK_DATA_COMMAND_TAG 0xCF > +#define DCD_CHECK_BITS_SET_PARAM 0x14 > +#define DCD_CHECK_BITS_CLR_PARAM 0x04 > > enum imximage_cmd { > CMD_INVALID, > CMD_IMAGE_VERSION, > CMD_BOOT_FROM, > CMD_BOOT_OFFSET, > - CMD_DATA, > + CMD_WRITE_DATA, > + CMD_WRITE_CLR_BIT, > + CMD_CHECK_BITS_SET, > + CMD_CHECK_BITS_CLR, > CMD_CSF, > }; > > @@ -167,6 +175,9 @@ typedef void (*set_dcd_val_t)(struct imx_header *imxhdr, > int fld, uint32_t value, > uint32_t off); > > +typedef void (*set_dcd_param_t)(struct imx_header *imxhdr, uint32_t dcd_len, > + int32_t cmd); > + > typedef void (*set_dcd_rst_t)(struct imx_header *imxhdr, > uint32_t dcd_len, > char *name, int lineno); > V2 looks like much more easier to understand - apart the very small comments, it is ok for me. Best regards, Stefano Babic
diff --git a/tools/imximage.c b/tools/imximage.c index 6f469ae..cc0392f 100644 --- a/tools/imximage.c +++ b/tools/imximage.c @@ -21,7 +21,10 @@ static table_entry_t imximage_cmds[] = { {CMD_BOOT_FROM, "BOOT_FROM", "boot command", }, {CMD_BOOT_OFFSET, "BOOT_OFFSET", "Boot offset", }, - {CMD_DATA, "DATA", "Reg Write Data", }, + {CMD_WRITE_DATA, "DATA", "Reg Write Data", }, + {CMD_WRITE_CLR_BIT, "CLR_BIT", "Reg clear bit", }, + {CMD_CHECK_BITS_SET, "CHECK_BITS_SET", "Reg Check bits set", }, + {CMD_CHECK_BITS_CLR, "CHECK_BITS_CLR", "Reg Check bits clr", }, {CMD_CSF, "CSF", "Command Sequence File", }, {CMD_IMAGE_VERSION, "IMAGE_VERSION", "image version", }, {-1, "", "", }, @@ -62,7 +65,7 @@ static table_entry_t imximage_boot_loadsize[] = { */ static table_entry_t imximage_versions[] = { {IMXIMAGE_V1, "", " (i.MX25/35/51 compatible)", }, - {IMXIMAGE_V2, "", " (i.MX53/6 compatible)", }, + {IMXIMAGE_V2, "", " (i.MX53/6/7 compatible)", }, {-1, "", " (Invalid)", }, }; @@ -79,6 +82,7 @@ static uint32_t imximage_csf_size = UNDEFINED; static uint32_t imximage_init_loadsize; static set_dcd_val_t set_dcd_val; +static set_dcd_param_t set_dcd_param; static set_dcd_rst_t set_dcd_rst; static set_imx_hdr_t set_imx_hdr; static uint32_t max_dcd_entries; @@ -128,6 +132,12 @@ static void err_imximage_version(int version) exit(EXIT_FAILURE); } +static void set_dcd_param_v1(struct imx_header *imxhdr, uint32_t dcd_len, + int32_t cmd) +{ + /* DCD V1 no parameter settings */ +} + static void set_dcd_val_v1(struct imx_header *imxhdr, char *name, int lineno, int fld, uint32_t value, uint32_t off) { @@ -156,6 +166,43 @@ static void set_dcd_val_v1(struct imx_header *imxhdr, char *name, int lineno, } } +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; + + switch (cmd) { + case CMD_WRITE_DATA: + dcd_v2->write_dcd_command.tag = DCD_WRITE_DATA_COMMAND_TAG; + dcd_v2->write_dcd_command.length = cpu_to_be16( + dcd_len * sizeof(dcd_addr_data_t) + 4); + dcd_v2->write_dcd_command.param = DCD_WRITE_DATA_PARAM; + break; + case CMD_WRITE_CLR_BIT: + dcd_v2->write_dcd_command.tag = DCD_WRITE_DATA_COMMAND_TAG; + dcd_v2->write_dcd_command.length = cpu_to_be16( + dcd_len * sizeof(dcd_addr_data_t) + 4); + dcd_v2->write_dcd_command.param = DCD_WRITE_CLR_BIT_PARAM; + break; + case CMD_CHECK_BITS_SET: + dcd_v2->write_dcd_command.tag = DCD_CHECK_DATA_COMMAND_TAG; + /* + * Check data command only supports one entry, + * so use 0xC = size(address + value + command). + */ + dcd_v2->write_dcd_command.length = cpu_to_be16(0xC); + dcd_v2->write_dcd_command.param = DCD_CHECK_BITS_SET_PARAM; + break; + case CMD_CHECK_BITS_CLR: + dcd_v2->write_dcd_command.tag = DCD_CHECK_DATA_COMMAND_TAG; + dcd_v2->write_dcd_command.length = cpu_to_be16(0xC); + dcd_v2->write_dcd_command.param = DCD_CHECK_BITS_SET_PARAM; + break; + default: + break; + } +} + static void set_dcd_val_v2(struct imx_header *imxhdr, char *name, int lineno, int fld, uint32_t value, uint32_t off) { @@ -200,10 +247,7 @@ static void set_dcd_rst_v2(struct imx_header *imxhdr, uint32_t dcd_len, dcd_v2->header.length = cpu_to_be16( dcd_len * sizeof(dcd_addr_data_t) + 8); dcd_v2->header.version = DCD_VERSION; - dcd_v2->write_dcd_command.tag = DCD_COMMAND_TAG; - dcd_v2->write_dcd_command.length = cpu_to_be16( - dcd_len * sizeof(dcd_addr_data_t) + 4); - dcd_v2->write_dcd_command.param = DCD_COMMAND_PARAM; + set_dcd_param_v2(imxhdr, dcd_len, CMD_WRITE_DATA); } static void set_imx_hdr_v1(struct imx_header *imxhdr, uint32_t dcd_len, @@ -266,12 +310,14 @@ static void set_hdr_func(void) switch (imximage_version) { case IMXIMAGE_V1: set_dcd_val = set_dcd_val_v1; + set_dcd_param = set_dcd_param_v1; set_dcd_rst = set_dcd_rst_v1; set_imx_hdr = set_imx_hdr_v1; max_dcd_entries = MAX_HW_CFG_SIZE_V1; break; case IMXIMAGE_V2: set_dcd_val = set_dcd_val_v2; + set_dcd_param = set_dcd_param_v2; set_dcd_rst = set_dcd_rst_v2; set_imx_hdr = set_imx_hdr_v2; max_dcd_entries = MAX_HW_CFG_SIZE_V2; @@ -396,8 +442,12 @@ 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_DATA: + case CMD_WRITE_DATA: + case CMD_WRITE_CLR_BIT: + case CMD_CHECK_BITS_SET: + case CMD_CHECK_BITS_CLR: value = get_cfg_value(token, name, lineno); + (*set_dcd_param)(imxhdr, dcd_len, cmd); (*set_dcd_val)(imxhdr, name, lineno, fld, value, dcd_len); if (unlikely(cmd_ver_first != 1)) cmd_ver_first = 0; @@ -436,20 +486,29 @@ static void parse_cfg_fld(struct imx_header *imxhdr, int32_t *cmd, break; case CFG_REG_ADDRESS: case CFG_REG_VALUE: - if (*cmd != CMD_DATA) - return; - - value = get_cfg_value(token, name, lineno); - (*set_dcd_val)(imxhdr, name, lineno, fld, value, *dcd_len); - - if (fld == CFG_REG_VALUE) { - (*dcd_len)++; - if (*dcd_len > max_dcd_entries) { - fprintf(stderr, "Error: %s[%d] -" - "DCD table exceeds maximum size(%d)\n", - name, lineno, max_dcd_entries); - exit(EXIT_FAILURE); + switch(*cmd) { + case CMD_WRITE_DATA: + case CMD_WRITE_CLR_BIT: + case CMD_CHECK_BITS_SET: + case CMD_CHECK_BITS_CLR: + + value = get_cfg_value(token, name, lineno); + (*set_dcd_param)(imxhdr, *dcd_len, *cmd); + (*set_dcd_val)(imxhdr, name, lineno, fld, value, + *dcd_len); + + if (fld == CFG_REG_VALUE) { + (*dcd_len)++; + if (*dcd_len > max_dcd_entries) { + fprintf(stderr, "Error: %s[%d] -" + "DCD table exceeds maximum size(%d)\n", + name, lineno, max_dcd_entries); + exit(EXIT_FAILURE); + } } + break; + default: + break; } break; default: diff --git a/tools/imximage.h b/tools/imximage.h index 36fe095..c199814 100644 --- a/tools/imximage.h +++ b/tools/imximage.h @@ -42,19 +42,27 @@ #define FLASH_LOADSIZE_SATA FLASH_LOADSIZE_STANDARD #define FLASH_LOADSIZE_QSPI 0x0 /* entire image */ -#define IVT_HEADER_TAG 0xD1 -#define IVT_VERSION 0x40 -#define DCD_HEADER_TAG 0xD2 -#define DCD_COMMAND_TAG 0xCC -#define DCD_VERSION 0x40 -#define DCD_COMMAND_PARAM 0x4 +/* Command tags and parameters */ +#define IVT_HEADER_TAG 0xD1 +#define IVT_VERSION 0x40 +#define DCD_HEADER_TAG 0xD2 +#define DCD_VERSION 0x40 +#define DCD_WRITE_DATA_COMMAND_TAG 0xCC +#define DCD_WRITE_DATA_PARAM 0x4 +#define DCD_WRITE_CLR_BIT_PARAM 0xC +#define DCD_CHECK_DATA_COMMAND_TAG 0xCF +#define DCD_CHECK_BITS_SET_PARAM 0x14 +#define DCD_CHECK_BITS_CLR_PARAM 0x04 enum imximage_cmd { CMD_INVALID, CMD_IMAGE_VERSION, CMD_BOOT_FROM, CMD_BOOT_OFFSET, - CMD_DATA, + CMD_WRITE_DATA, + CMD_WRITE_CLR_BIT, + CMD_CHECK_BITS_SET, + CMD_CHECK_BITS_CLR, CMD_CSF, }; @@ -167,6 +175,9 @@ typedef void (*set_dcd_val_t)(struct imx_header *imxhdr, int fld, uint32_t value, uint32_t off); +typedef void (*set_dcd_param_t)(struct imx_header *imxhdr, uint32_t dcd_len, + int32_t cmd); + typedef void (*set_dcd_rst_t)(struct imx_header *imxhdr, uint32_t dcd_len, char *name, int lineno);