On 08/17/2018 08:56 AM, Simon Goldschmidt wrote:
On Fri, Aug 17, 2018 at 1:57 AM Marek Vasut marex@denx.de wrote:
On 08/16/2018 09:44 PM, Simon Goldschmidt wrote:
On Thu, Aug 16, 2018 at 4:04 PM Marek Vasut marex@denx.de wrote:
On 08/16/2018 03:50 PM, Simon Goldschmidt wrote:
On Thu, Aug 16, 2018 at 3:15 PM Marek Vasut marex@denx.de wrote:
On 08/16/2018 03:12 PM, Simon Goldschmidt wrote: > > > Marek Vasut <marex@denx.de mailto:marex@denx.de> schrieb am Do., 16. > Aug. 2018, 15:06: > > On 08/16/2018 03:00 PM, Simon Goldschmidt wrote: > > On Thu, Aug 16, 2018 at 1:18 PM Marek Vasut <marex@denx.de > mailto:marex@denx.de> wrote: > >> > >> On 08/16/2018 09:38 AM, Simon Goldschmidt wrote: > >>> gd->env_addr points to pre-relocation address even after > >>> relocation. This leads to an abort in env_callback_init > >>> when loading the environment. > >>> > >>> Fix this by enabling CONFIG_SYS_EXTRA_ENV_RELOC. > >>> > >>> Signed-off-by: Simon Goldschmidt > <simon.k.r.goldschmidt@gmail.com > mailto:simon.k.r.goldschmidt@gmail.com> > >> > >> I have one last question -- does this somehow influence SPL ? > > > > No, it doesn't. The code that gets enabled by this define is in > > common/board_r.c, which is not linked for SPL. > > Ah, thanks for checking. > > btw do you think it'd make sense to just enable this by default on all > systems and zap the EXTRA_ENV_RELOC macro altogether ? > > > Yes, that's what I have thought about already. Just like the for the > embedded device tree relocation, we could then probably use > gd->reloc_off instead of CONFIG_SYS_MONITOR_BASE. I'm just not sure this > really works for all boards, but it would be worth a try to push after > this release is out.
I think so too. I cannot think of a reason why this shouldn't be enabled in fact.
Exactly. Too me it seems like a leftover, especially given the use of CONFIG_SYS_MONITOR_BASE, which seems a little outdated, too. I've set up a reminder for a patch to remove it after the release.
Feel free to send it now.
OK, I have tried, but it seems it's not that easy: some boards override the initial gd->env_addr by setting CONFIG_ENV_ADDR. So if this is outside of U-Boot's pre-relocation range, it clearly should not be relocated. One might find an improved way to relocate gd->env_addr if it is internal (e.g. checking the range to be in pre-relocation?). But simply removing the EXTRA_ENV_RELOC does not seem to work.
Shouldn't most of those boards be easily fixable ?
Well, if we unconditionally alter gd->env_addr by gd->reloc_off, boards that have their initial gd->env_addr outside of the initial binary can be fixed only by changing their behaviour. I don't know how widely used this feature is, but since it's a config option (CONFIG_ENV_ADDR), how would we know?
git grep ? But aren't you mixing CONFIG_ENV_ADDR and CONFIG_ENV_RELOC ?
So to me that means we still have to make this overridable and could change the "default" state of such an option only. Meaning that the default is "relocate gd->env_addr" with an option to leave it. But is this really worth breaking existing boards?
I think you do want to relocate the env alongside U-Boot, always, no ?