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 <aalo...@freescale.com> > Signed-off-by: Peng Fan <peng....@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 -- ===================================================================== DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de ===================================================================== _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot