imximage: fix commands other than write_data

PATCH New Troy Kisky 3 participants 10 messages Sep 15, 2015 – Oct 8, 2015 Arm i.MX and Layerscape

lore

Share & tools

lore

Troy Kisky

author

2015-09-15 01:06 UTC #0 lore raw

PATCH 1/1 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 <[email protected]>

---

Note: this needs tested to make sure imx7dsabresd still boots as its dcd header has changed

tools/imximage.c +56 ~~-21~~

index 0da48a7..97a6880 100644
@@ -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;

tools/imximage.h +6 -2

index d41c74f..c7b9b5c 100644
@@ -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;

Signature · 1 line

1.9.1

Fabio Estevam

reviewer

2015-09-15 01:16 UTC #1 lore raw

PATCH 1/1 imximage: fix commands other than write_data

Quoted 10 lines · Troy Kisky

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 <[email protected]>
---
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 <[email protected]>

Stefano Babic

2015-09-20 08:23 UTC #2 lore raw

PATCH 1/1 imximage: fix commands other than write_data

In reply to Troy Kisky

Quoted 7 lines · Troy Kisky

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 <[email protected]>
---

Applied to u-boot-imx, thanks !

Best regards,
Stefano Babic

Signature · 5 lines

=====================================================================
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: sbabic at denx.de
=====================================================================

Stefano Babic

2015-09-20 08:59 UTC #3 lore raw

PATCH 1/1 imximage: fix commands other than write_data

In reply to Troy Kisky

Hi Troy,

Quoted 8 lines · Troy Kisky

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 <[email protected]>
---

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

Quoted 161 lines

Note: this needs tested to make sure imx7dsabresd still boots as its dcd header has changed

tools/imximage.c +56 ~~-21~~

index 0da48a7..97a6880 100644
@@ -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;

tools/imximage.h +6 -2

index d41c74f..c7b9b5c 100644
@@ -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;

Signature · 5 lines

=====================================================================
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: sbabic at denx.de
=====================================================================

Troy Kisky

author

2015-09-21 21:01 UTC #4 lore raw

PATCH 1/1 imximage: fix commands other than write_data

Quoted 14 lines · Stefano Babic

Hi Troy,
Quoted 8 lines · Troy Kisky
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 <[email protected]>
---
This patch breaks building boards with SPL:
arm: + mx6sabresd_spl

Review

Quoted 4 lines

(quoted diff) +4 -0

+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

Stefano Babic

2015-10-07 12:05 UTC #5 lore raw

PATCH 1/1 imximage: fix commands other than write_data

In reply to Troy Kisky

Hi Troy,

Quoted 7 lines · Troy Kisky

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 <[email protected]>
---

Applied to u-boot-imx (fix), thanks !

Best regards,
Stefano Babic

Signature · 5 lines

=====================================================================
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: sbabic at denx.de
=====================================================================

Troy Kisky

author

2015-10-07 17:54 UTC #6 lore raw

PATCH 1/1 imximage: fix commands other than write_data

Quoted 7 lines · Stefano Babic

Hi Troy,
Quoted 4 lines · Troy Kisky
When CHECK_BITS_SET was added, they forgot to add
a new command table, and instead overwrote the
previous table.

Quoted 9 lines

Quoted 3 lines
Signed-off-by Troy Kisky <[email protected]>
---
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

Fabio Estevam

reviewer

2015-10-07 18:25 UTC #7 lore raw

PATCH 1/1 imximage: fix commands other than write_data

Quoted 7 lines · Troy Kisky

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.

Troy Kisky

author

2015-10-07 21:07 UTC #8 lore raw

PATCH 1/1 imximage: fix commands other than write_data

Quoted 8 lines · Fabio Estevam

Quoted 5 lines · Troy Kisky
Could someone verify that one of the following boards still boot with this patch?

Quoted 5 lines

Quoted 2 lines
colibri_vf_dtb
mx6sabresd_spl
Tested with mx6sabresd_spl and it still boots.

Thanks Fabio

Stefano Babic

2015-10-08 08:46 UTC #9 lore raw

PATCH 1/1 imximage: fix commands other than write_data

In reply to Troy Kisky

Hi Troy,

Quoted 23 lines · Troy Kisky

Quoted 7 lines · Stefano Babic
Hi Troy,
Quoted 4 lines · Troy Kisky
When CHECK_BITS_SET was added, they forgot to add
a new command table, and instead overwrote the
previous table.
Quoted 9 lines
Quoted 3 lines
Signed-off-by Troy Kisky <[email protected]>
---
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?

Quoted 7 lines

colibri_vf_dtb
mx6sabresd_spl
colibri_vf
vf610twr
cm_fx6
vf610twr_nand
mx6cuboxi

Tested on mx6cuboxi, it works fine.

Best regards,
Stefano

Signature · 5 lines

=====================================================================
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: sbabic at denx.de
=====================================================================