[U-Boot] [PATCH] imximage: fix commands other than write_data

When CHECK_BITS_SET was added, they forgot to add a new command table, and instead overwrote the previous table.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
---
Note: this needs tested to make sure imx7dsabresd still boots as its dcd header has changed
diff --git a/tools/imximage.c b/tools/imximage.c index 0da48a7..97a6880 100644 --- a/tools/imximage.c +++ b/tools/imximage.c @@ -160,54 +160,80 @@ static void set_dcd_val_v1(struct imx_header *imxhdr, char *name, int lineno, } }
+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; + struct dcd_v2_cmd *d = gd_last_cmd; + struct dcd_v2_cmd *d2; + int len; + + if (!d) + d = &dcd_v2->dcd_cmd; + d2 = d; + len = be16_to_cpu(d->write_dcd_command.length); + if (len > 4) + d2 = (struct dcd_v2_cmd *)(((char *)d) + len);
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; + if ((d->write_dcd_command.tag == DCD_WRITE_DATA_COMMAND_TAG) && + (d->write_dcd_command.param == DCD_WRITE_DATA_PARAM)) + break; + d = d2; + d->write_dcd_command.tag = DCD_WRITE_DATA_COMMAND_TAG; + d->write_dcd_command.length = cpu_to_be16(4); + d->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; + if ((d->write_dcd_command.tag == DCD_WRITE_DATA_COMMAND_TAG) && + (d->write_dcd_command.param == DCD_WRITE_CLR_BIT_PARAM)) + break; + d = d2; + d->write_dcd_command.tag = DCD_WRITE_DATA_COMMAND_TAG; + d->write_dcd_command.length = cpu_to_be16(4); + d->write_dcd_command.param = DCD_WRITE_CLR_BIT_PARAM; break; /* * Check data command only supports one entry, - * so use 0xC = size(address + value + command). */ case CMD_CHECK_BITS_SET: - 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; + d = d2; + d->write_dcd_command.tag = DCD_CHECK_DATA_COMMAND_TAG; + d->write_dcd_command.length = cpu_to_be16(4); + d->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; + d = d2; + d->write_dcd_command.tag = DCD_CHECK_DATA_COMMAND_TAG; + d->write_dcd_command.length = cpu_to_be16(4); + d->write_dcd_command.param = DCD_CHECK_BITS_SET_PARAM; break; default: break; } + gd_last_cmd = d; }
static void set_dcd_val_v2(struct imx_header *imxhdr, char *name, int lineno, int fld, uint32_t value, uint32_t off) { - dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.dcd_table; + struct dcd_v2_cmd *d = gd_last_cmd; + int len; + + len = be16_to_cpu(d->write_dcd_command.length); + off = (len - 4) >> 3;
switch (fld) { case CFG_REG_ADDRESS: - dcd_v2->addr_data[off].addr = cpu_to_be32(value); + d->addr_data[off].addr = cpu_to_be32(value); break; case CFG_REG_VALUE: - dcd_v2->addr_data[off].value = cpu_to_be32(value); + d->addr_data[off].value = cpu_to_be32(value); + off++; + d->write_dcd_command.length = cpu_to_be16((off << 3) + 4); break; default: break; @@ -236,12 +262,20 @@ 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( - dcd_len * sizeof(dcd_addr_data_t) + 8); + dcd_v2->header.length = cpu_to_be16(len); dcd_v2->header.version = DCD_VERSION; - set_dcd_param_v2(imxhdr, dcd_len, CMD_WRITE_DATA); }
static void set_imx_hdr_v1(struct imx_header *imxhdr, uint32_t dcd_len, @@ -314,6 +348,7 @@ static void set_hdr_func(void) max_dcd_entries = MAX_HW_CFG_SIZE_V1; break; case IMXIMAGE_V2: + gd_last_cmd = NULL; set_dcd_val = set_dcd_val_v2; set_dcd_param = set_dcd_param_v2; set_dcd_rst = set_dcd_rst_v2; diff --git a/tools/imximage.h b/tools/imximage.h index d41c74f..c7b9b5c 100644 --- a/tools/imximage.h +++ b/tools/imximage.h @@ -133,10 +133,14 @@ typedef struct { uint8_t param; } __attribute__((packed)) write_dcd_command_t;
-typedef struct { - ivt_header_t header; +struct dcd_v2_cmd { write_dcd_command_t write_dcd_command; dcd_addr_data_t addr_data[MAX_HW_CFG_SIZE_V2]; +}; + +typedef struct { + ivt_header_t header; + struct dcd_v2_cmd dcd_cmd; uint32_t padding[1]; /* end up on an 8-byte boundary */ } dcd_v2_t;

On Mon, Sep 14, 2015 at 10:06 PM, Troy Kisky troy.kisky@boundarydevices.com wrote:
When CHECK_BITS_SET was added, they forgot to add a new command table, and instead overwrote the previous table.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
Note: this needs tested to make sure imx7dsabresd still boots as its dcd header has changed
Boots fine on mx7dsabres, thanks:
Tested-by: Fabio Estevam fabio.estevam@freescale.com

On 15/09/2015 03:06, Troy Kisky wrote:
When CHECK_BITS_SET was added, they forgot to add a new command table, and instead overwrote the previous table.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
Applied to u-boot-imx, thanks !
Best regards, Stefano Babic

Hi Troy,
On 15/09/2015 03:06, Troy Kisky wrote:
When CHECK_BITS_SET was added, they forgot to add a new command table, and instead overwrote the previous table.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
This patch breaks building boards with SPL:
Building current source for 85 boards (6 threads, 1 job per thread) arm: + colibri_vf_dtb +Error: Image corrupt DCD size 536870911 exceed maximum 220 +make[2]: *** [u-boot-dtb.imx] Error 1 +make[1]: *** [u-boot-dtb.imx] Error 2 +make: *** [sub-make] Error 2 arm: + mx6sabresd_spl +Error: Image corrupt DCD size 536870911 exceed maximum 220 +make[2]: *** [SPL] Error 1 +make[1]: *** [SPL] Error 2 +make: *** [sub-make] Error 2 arm: + colibri_vf +Error: Image corrupt DCD size 536870911 exceed maximum 220 +make[2]: *** [u-boot.imx] Error 1 +make[1]: *** [u-boot.imx] Error 2 +make: *** [sub-make] Error 2 arm: + vf610twr +Error: Image corrupt DCD size 536870911 exceed maximum 220 +make[2]: *** [u-boot.imx] Error 1 +make[1]: *** [u-boot.imx] Error 2 +make: *** [sub-make] Error 2 arm: + cm_fx6 +Error: Image corrupt DCD size 536870911 exceed maximum 220 +make[2]: *** [SPL] Error 1 +make[1]: *** [SPL] Error 2 +make: *** [sub-make] Error 2 arm: + vf610twr_nand +Error: Image corrupt DCD size 536870911 exceed maximum 220 +make[2]: *** [u-boot.imx] Error 1 +make[1]: *** [u-boot.imx] Error 2 +make: *** [sub-make] Error 2 arm: + mx6cuboxi +Error: Image corrupt DCD size 536870911 exceed maximum 220 +make[2]: *** [SPL] Error 1 +make[1]: *** [SPL] Error 2 +make: *** [sub-make] Error 2
Can you take a look ? Thanks !
Best regards, Stefano Babic
Note: this needs tested to make sure imx7dsabresd still boots as its dcd header has changed
diff --git a/tools/imximage.c b/tools/imximage.c index 0da48a7..97a6880 100644 --- a/tools/imximage.c +++ b/tools/imximage.c @@ -160,54 +160,80 @@ static void set_dcd_val_v1(struct imx_header *imxhdr, char *name, int lineno, } }
+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;
struct dcd_v2_cmd *d = gd_last_cmd;
struct dcd_v2_cmd *d2;
int len;
if (!d)
d = &dcd_v2->dcd_cmd;
d2 = d;
len = be16_to_cpu(d->write_dcd_command.length);
if (len > 4)
d2 = (struct dcd_v2_cmd *)(((char *)d) + len);
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;
if ((d->write_dcd_command.tag == DCD_WRITE_DATA_COMMAND_TAG) &&
(d->write_dcd_command.param == DCD_WRITE_DATA_PARAM))
break;
d = d2;
d->write_dcd_command.tag = DCD_WRITE_DATA_COMMAND_TAG;
d->write_dcd_command.length = cpu_to_be16(4);
break; case CMD_WRITE_CLR_BIT:d->write_dcd_command.param = DCD_WRITE_DATA_PARAM;
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;
if ((d->write_dcd_command.tag == DCD_WRITE_DATA_COMMAND_TAG) &&
(d->write_dcd_command.param == DCD_WRITE_CLR_BIT_PARAM))
break;
d = d2;
d->write_dcd_command.tag = DCD_WRITE_DATA_COMMAND_TAG;
d->write_dcd_command.length = cpu_to_be16(4);
break; /*d->write_dcd_command.param = DCD_WRITE_CLR_BIT_PARAM;
- Check data command only supports one entry,
*/ case CMD_CHECK_BITS_SET:* so use 0xC = size(address + value + command).
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;
d = d2;
d->write_dcd_command.tag = DCD_CHECK_DATA_COMMAND_TAG;
d->write_dcd_command.length = cpu_to_be16(4);
break; case CMD_CHECK_BITS_CLR:d->write_dcd_command.param = DCD_CHECK_BITS_SET_PARAM;
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;
d = d2;
d->write_dcd_command.tag = DCD_CHECK_DATA_COMMAND_TAG;
d->write_dcd_command.length = cpu_to_be16(4);
break; default: break; }d->write_dcd_command.param = DCD_CHECK_BITS_SET_PARAM;
- gd_last_cmd = d;
}
static void set_dcd_val_v2(struct imx_header *imxhdr, char *name, int lineno, int fld, uint32_t value, uint32_t off) {
- dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.dcd_table;
struct dcd_v2_cmd *d = gd_last_cmd;
int len;
len = be16_to_cpu(d->write_dcd_command.length);
off = (len - 4) >> 3;
switch (fld) { case CFG_REG_ADDRESS:
dcd_v2->addr_data[off].addr = cpu_to_be32(value);
break; case CFG_REG_VALUE:d->addr_data[off].addr = cpu_to_be32(value);
dcd_v2->addr_data[off].value = cpu_to_be32(value);
d->addr_data[off].value = cpu_to_be32(value);
off++;
break; default: break;d->write_dcd_command.length = cpu_to_be16((off << 3) + 4);
@@ -236,12 +262,20 @@ 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(
dcd_len * sizeof(dcd_addr_data_t) + 8);
- dcd_v2->header.length = cpu_to_be16(len); dcd_v2->header.version = DCD_VERSION;
- set_dcd_param_v2(imxhdr, dcd_len, CMD_WRITE_DATA);
}
static void set_imx_hdr_v1(struct imx_header *imxhdr, uint32_t dcd_len, @@ -314,6 +348,7 @@ static void set_hdr_func(void) 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;gd_last_cmd = NULL;
diff --git a/tools/imximage.h b/tools/imximage.h index d41c74f..c7b9b5c 100644 --- a/tools/imximage.h +++ b/tools/imximage.h @@ -133,10 +133,14 @@ typedef struct { uint8_t param; } __attribute__((packed)) write_dcd_command_t;
-typedef struct {
- ivt_header_t header;
+struct dcd_v2_cmd { write_dcd_command_t write_dcd_command; dcd_addr_data_t addr_data[MAX_HW_CFG_SIZE_V2]; +};
+typedef struct {
- ivt_header_t header;
- struct dcd_v2_cmd dcd_cmd; uint32_t padding[1]; /* end up on an 8-byte boundary */
} dcd_v2_t;

On 9/20/2015 1:59 AM, Stefano Babic wrote:
Hi Troy,
On 15/09/2015 03:06, Troy Kisky wrote:
When CHECK_BITS_SET was added, they forgot to add a new command table, and instead overwrote the previous table.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
This patch breaks building boards with SPL: arm: + mx6sabresd_spl +Error: Image corrupt DCD size 536870911 exceed maximum 220 +make[2]: *** [SPL] Error 1 +make[1]: *** [SPL] Error 2 +make: *** [sub-make] Error 2
Looks like the error check is wrong. So previously, it was probably writing 0 to address 0. I'll send a patch.
Troy

Hi Troy,
On 15/09/2015 03:06, Troy Kisky wrote:
When CHECK_BITS_SET was added, they forgot to add a new command table, and instead overwrote the previous table.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
Applied to u-boot-imx (fix), thanks !
Best regards, Stefano Babic

On 10/7/2015 5:05 AM, Stefano Babic wrote:
Hi Troy,
On 15/09/2015 03:06, Troy Kisky wrote:
When CHECK_BITS_SET was added, they forgot to add a new command table, and instead overwrote the previous table.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
Applied to u-boot-imx (fix), thanks !
Best regards, Stefano Babic
Could someone verify that one of the following boards still boot with this patch?
colibri_vf_dtb mx6sabresd_spl colibri_vf vf610twr cm_fx6 vf610twr_nand mx6cuboxi
Just in case the ROM has a need for a non-empty DCD table.
Thanks Troy

On Wed, Oct 7, 2015 at 2:54 PM, Troy Kisky troy.kisky@boundarydevices.com wrote:
Could someone verify that one of the following boards still boot with this patch?
colibri_vf_dtb mx6sabresd_spl
Tested with mx6sabresd_spl and it still boots.

On 10/7/2015 11:25 AM, Fabio Estevam wrote:
On Wed, Oct 7, 2015 at 2:54 PM, Troy Kisky troy.kisky@boundarydevices.com wrote:
Could someone verify that one of the following boards still boot with this patch?
colibri_vf_dtb mx6sabresd_spl
Tested with mx6sabresd_spl and it still boots.
Thanks Fabio

Hi Troy,
On 07/10/2015 19:54, Troy Kisky wrote:
On 10/7/2015 5:05 AM, Stefano Babic wrote:
Hi Troy,
On 15/09/2015 03:06, Troy Kisky wrote:
When CHECK_BITS_SET was added, they forgot to add a new command table, and instead overwrote the previous table.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
Applied to u-boot-imx (fix), thanks !
Best regards, Stefano Babic
Could someone verify that one of the following boards still boot with this patch?
colibri_vf_dtb mx6sabresd_spl colibri_vf vf610twr cm_fx6 vf610twr_nand mx6cuboxi
Tested on mx6cuboxi, it works fine.
Best regards, Stefano
participants (3)
-
Fabio Estevam
-
Stefano Babic
-
Troy Kisky