Message ID | 1442279191-6615-1-git-send-email-troy.kisky@boundarydevices.com |
---|---|
State | Changes Requested |
Delegated to: | Stefano Babic |
Headers | show |
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); > + 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 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
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;
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