| Message ID | 1329987019-24379-1-git-send-email-dirk.behme@de.bosch.com |
|---|---|
| State | Accepted |
| Commit | 49d3e2721164eaef5df702a26cfca6efd430be30 |
| Delegated to: | Stefano Babic |
| Headers | show |
On 23/02/2012 09:50, Dirk Behme wrote: > The flash header supports different flash offsets for different > boot devices. E.g. parallel NOR or OneNAND use a different offset > than FLASH_OFFSET_STANDARD (== 0x400). > > The flash offset is correctly read from the configuration in > parse_cfg_cmd(). But is then overwritten wrongly in set_imx_hdr_v1/2(). > > Fix this by removing this overwriting. Use the flash offset > correctly read from the configuration, instead. > > If there is no flash_offset read from the configuration file, i.e. > the BOOT_FROM tag is missing, exit with an error message. > > Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com> > CC: Jason Liu <liu.h.jason@gmail.com> > CC: Stefano Babic <sbabic@denx.de> > --- Tested on ima-mx53 with MX53 (header V2), booting from NOR. Tested-by: Stefano Babic <sbabic@denx.de> Dirk, apart this patch that fixes in any case a wrong offset in header, I have investigated in more detail this issue. The bug sets a wrong value in the boot_data.start field that inform the processor which is the start address of the image to be copied into the RAM. All other fields are still correct. However, if we boot from NOR we have a XIP device and we do not need this first copy into the RAM. We could skip it if we set CONFIG_SYS_TEXT_BASE to point to a region in the NOR flash, as it is usually done when booting from NOR (see for example PowerPc boards). I understand that this is the reason why my board has always booted, while yours not. Is it right ? Where do you set CONFIG_SYS_TEXT_BASE ? As optimization, we could also avoid that the processor starts to copy the image by setting boot_data.size=0 in the header when NOR device is set, but only if we are sure that no copy into RAM is required. Best regards, Stefano Babic
On 23.02.2012 14:10, Stefano Babic wrote: > On 23/02/2012 09:50, Dirk Behme wrote: >> The flash header supports different flash offsets for different >> boot devices. E.g. parallel NOR or OneNAND use a different offset >> than FLASH_OFFSET_STANDARD (== 0x400). >> >> The flash offset is correctly read from the configuration in >> parse_cfg_cmd(). But is then overwritten wrongly in set_imx_hdr_v1/2(). >> >> Fix this by removing this overwriting. Use the flash offset >> correctly read from the configuration, instead. >> >> If there is no flash_offset read from the configuration file, i.e. >> the BOOT_FROM tag is missing, exit with an error message. >> >> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com> >> CC: Jason Liu <liu.h.jason@gmail.com> >> CC: Stefano Babic <sbabic@denx.de> >> --- > > Tested on ima-mx53 with MX53 (header V2), booting from NOR. > > Tested-by: Stefano Babic <sbabic@denx.de> > > > Dirk, > > apart this patch that fixes in any case a wrong offset in header, I have > investigated in more detail this issue. The bug sets a wrong value in > the boot_data.start field that inform the processor which is the start > address of the image to be copied into the RAM. All other fields are > still correct. > > However, if we boot from NOR we have a XIP device and we do not need > this first copy into the RAM. We could skip it if we set > CONFIG_SYS_TEXT_BASE to point to a region in the NOR flash, as it is > usually done when booting from NOR (see for example PowerPc boards). > I understand that this is the reason why my board has always booted, > while yours not. Is it right ? Ah, XIP. Yes, this does make sense! :) > Where do you set CONFIG_SYS_TEXT_BASE ? To main memory, i.e. no XIP. We just took the configuration as it is in mainline and normally is used to start from SD card: Initialize the main memory using the DCD data. And then copy the image to main memory, e.g. 0x17800000, and jump to it. And this jump fails if boot_data.start is wrong. Many thanks for this update! Best regards Dirk
On 23/02/2012 09:50, Dirk Behme wrote: > The flash header supports different flash offsets for different > boot devices. E.g. parallel NOR or OneNAND use a different offset > than FLASH_OFFSET_STANDARD (== 0x400). > > The flash offset is correctly read from the configuration in > parse_cfg_cmd(). But is then overwritten wrongly in set_imx_hdr_v1/2(). > > Fix this by removing this overwriting. Use the flash offset > correctly read from the configuration, instead. > > If there is no flash_offset read from the configuration file, i.e. > the BOOT_FROM tag is missing, exit with an error message. > > Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com> > CC: Jason Liu <liu.h.jason@gmail.com> > CC: Stefano Babic <sbabic@denx.de> > --- Applied to u-boot-imx, thanks. Best regards, Stefano Babic
On 23/02/2012 09:50, Dirk Behme wrote: > The flash header supports different flash offsets for different > boot devices. E.g. parallel NOR or OneNAND use a different offset > than FLASH_OFFSET_STANDARD (== 0x400). > > The flash offset is correctly read from the configuration in > parse_cfg_cmd(). But is then overwritten wrongly in set_imx_hdr_v1/2(). > > Fix this by removing this overwriting. Use the flash offset > correctly read from the configuration, instead. > > If there is no flash_offset read from the configuration file, i.e. > the BOOT_FROM tag is missing, exit with an error message. > > Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com> > CC: Jason Liu <liu.h.jason@gmail.com> > CC: Stefano Babic <sbabic@denx.de> > --- Applied to u-boot-imx, thanks. Best regards, Stefano Babic
diff --git a/tools/imximage.c b/tools/imximage.c index 1e0f5d4..03a7716 100644 --- a/tools/imximage.c +++ b/tools/imximage.c @@ -216,8 +216,12 @@ static void set_imx_hdr_v1(struct imx_header *imxhdr, uint32_t dcd_len, dcd_v1_t *dcd_v1 = &hdr_v1->dcd_table; uint32_t base_offset; - /* Set default offset */ - imxhdr->flash_offset = FLASH_OFFSET_STANDARD; + /* Exit if there is no BOOT_FROM field specifying the flash_offset */ + if(imxhdr->flash_offset == FLASH_OFFSET_UNDEFINED) { + fprintf(stderr, "Error: Header v1: No BOOT_FROM tag in %s\n", + params->imagename); + exit(EXIT_FAILURE); + } /* Set magic number */ fhdr_v1->app_code_barker = APP_CODE_BARKER; @@ -253,8 +257,12 @@ static void set_imx_hdr_v2(struct imx_header *imxhdr, uint32_t dcd_len, imx_header_v2_t *hdr_v2 = &imxhdr->header.hdr_v2; flash_header_v2_t *fhdr_v2 = &hdr_v2->fhdr; - /* Set default offset */ - imxhdr->flash_offset = FLASH_OFFSET_STANDARD; + /* Exit if there is no BOOT_FROM field specifying the flash_offset */ + if(imxhdr->flash_offset == FLASH_OFFSET_UNDEFINED) { + fprintf(stderr, "Error: Header v2: No BOOT_FROM tag in %s\n", + params->imagename); + exit(EXIT_FAILURE); + } /* Set magic number */ fhdr_v2->header.tag = IVT_HEADER_TAG; /* 0xD1 */ @@ -525,6 +533,8 @@ static void imximage_set_header(void *ptr, struct stat *sbuf, int ifd, * set up function ptr group to V1 by default. */ imximage_version = IMXIMAGE_V1; + /* Be able to detect if the cfg file has no BOOT_FROM tag */ + imxhdr->flash_offset = FLASH_OFFSET_UNDEFINED; set_hdr_func(imxhdr); /* Parse dcd configuration file */ diff --git a/tools/imximage.h b/tools/imximage.h index d784a8d..34f293d 100644 --- a/tools/imximage.h +++ b/tools/imximage.h @@ -32,6 +32,7 @@ #define HEADER_OFFSET 0x400 #define CMD_DATA_STR "DATA" +#define FLASH_OFFSET_UNDEFINED 0xFFFFFFFF #define FLASH_OFFSET_STANDARD 0x400 #define FLASH_OFFSET_NAND FLASH_OFFSET_STANDARD #define FLASH_OFFSET_SD FLASH_OFFSET_STANDARD
The flash header supports different flash offsets for different boot devices. E.g. parallel NOR or OneNAND use a different offset than FLASH_OFFSET_STANDARD (== 0x400). The flash offset is correctly read from the configuration in parse_cfg_cmd(). But is then overwritten wrongly in set_imx_hdr_v1/2(). Fix this by removing this overwriting. Use the flash offset correctly read from the configuration, instead. If there is no flash_offset read from the configuration file, i.e. the BOOT_FROM tag is missing, exit with an error message. Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com> CC: Jason Liu <liu.h.jason@gmail.com> CC: Stefano Babic <sbabic@denx.de> --- Changes in v2: Make the BOOT_FROM field mandatory, i.e. exit with an error message if there is no valid flash_offset set. Do the same handling in header v1 and v2, not only in v2. Note: This is tested only for header v2. tools/imximage.c | 18 ++++++++++++++---- tools/imximage.h | 1 + 2 files changed, 15 insertions(+), 4 deletions(-)