| Message ID | 20191204174439.69934-3-giulio.benetti@benettiengineering.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add i.MXRT family support | expand |
On Wed, 4 Dec 2019 18:44:21 +0100 Giulio Benetti <giulio.benetti@benettiengineering.com> wrote: > At the moment entry_point is set to image_get_load(header) that sets > it to "load address" instead of "entry point", assuming entry_point is > equal to load_addr, but it's not true. Then load_addr is set to > "entry_point - header_size", but this is wrong too since load_addr is > not an entry point. > > So use image_get_ep() for entry_point assignment and image_get_load() > for load_addr assignment. > > Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com> > --- > common/spl/spl.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/common/spl/spl.c b/common/spl/spl.c > index d51dbe9942..24da164b43 100644 > --- a/common/spl/spl.c > +++ b/common/spl/spl.c > @@ -264,9 +264,9 @@ int spl_parse_image_header(struct spl_image_info > *spl_image, spl_image->entry_point = image_get_ep(header); > spl_image->size = > image_get_data_size(header); } else { > - spl_image->entry_point = > image_get_load(header); > + spl_image->entry_point = > image_get_ep(header); /* Load including the header */ > - spl_image->load_addr = > spl_image->entry_point - > + spl_image->load_addr = > image_get_load(header) - header_size; > spl_image->size = > image_get_data_size(header) + header_size; This may not be the case - but I do recall that there was a similar issue between u-boot.bin and u-boot.imx being loaded. What is the format of i.MXRT? I'm also concerned about breaking already supported in-tree boards. Why i.MXRT needs to make this change? And why other boards don't need that fix? (do they all have load address equal to entry point ?) Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Hi Lukasz, First of all thank you for reviewing my patches and... On 12/8/19 3:37 PM, Lukasz Majewski wrote: > On Wed, 4 Dec 2019 18:44:21 +0100 > Giulio Benetti <giulio.benetti@benettiengineering.com> wrote: > >> At the moment entry_point is set to image_get_load(header) that sets >> it to "load address" instead of "entry point", assuming entry_point is >> equal to load_addr, but it's not true. Then load_addr is set to >> "entry_point - header_size", but this is wrong too since load_addr is >> not an entry point. >> >> So use image_get_ep() for entry_point assignment and image_get_load() >> for load_addr assignment. >> >> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com> >> --- >> common/spl/spl.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/common/spl/spl.c b/common/spl/spl.c >> index d51dbe9942..24da164b43 100644 >> --- a/common/spl/spl.c >> +++ b/common/spl/spl.c >> @@ -264,9 +264,9 @@ int spl_parse_image_header(struct spl_image_info >> *spl_image, spl_image->entry_point = image_get_ep(header); >> spl_image->size = >> image_get_data_size(header); } else { >> - spl_image->entry_point = >> image_get_load(header); >> + spl_image->entry_point = >> image_get_ep(header); /* Load including the header */ >> - spl_image->load_addr = >> spl_image->entry_point - >> + spl_image->load_addr = >> image_get_load(header) - header_size; >> spl_image->size = >> image_get_data_size(header) + header_size; > > This may not be the case - but I do recall that there was a similar > issue between u-boot.bin and u-boot.imx being loaded. ...I erroneously sent this patch again in this series but it's been already committed and then reverted at the moment and will be committed on next version since it breaks a lot of boards that assume that "load address" and "entry point" are the same. Here is the discussion: https://lists.denx.de/pipermail/u-boot/2019-December/393010.html > What is the format of i.MXRT? i.MXRT is a cortex-M7, so it needs "load address" to be different from "entry point", since entry point is offset of 0x3FD from "load address". This is done in board defconfig with: CONFIG_SYS_UBOOT_START different from: CONFIG_SYS_TEXT_BASE > I'm also concerned about breaking already supported in-tree boards. Why > i.MXRT needs to make this change? And why other boards don't need that > fix? (do they all have load address equal to entry point ?) Indeed, all these boards assume load address and entry point to be the same. Best regards
diff --git a/common/spl/spl.c b/common/spl/spl.c index d51dbe9942..24da164b43 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -264,9 +264,9 @@ int spl_parse_image_header(struct spl_image_info *spl_image, spl_image->entry_point = image_get_ep(header); spl_image->size = image_get_data_size(header); } else { - spl_image->entry_point = image_get_load(header); + spl_image->entry_point = image_get_ep(header); /* Load including the header */ - spl_image->load_addr = spl_image->entry_point - + spl_image->load_addr = image_get_load(header) - header_size; spl_image->size = image_get_data_size(header) + header_size;
At the moment entry_point is set to image_get_load(header) that sets it to "load address" instead of "entry point", assuming entry_point is equal to load_addr, but it's not true. Then load_addr is set to "entry_point - header_size", but this is wrong too since load_addr is not an entry point. So use image_get_ep() for entry_point assignment and image_get_load() for load_addr assignment. Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com> --- common/spl/spl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)