On Tue, Aug 14, 2018 at 10:37 AM Marek Vasut <ma...@denx.de> wrote: > > On 08/14/2018 08:09 AM, Simon Goldschmidt wrote: > > > > > > Marek Vasut <ma...@denx.de <mailto:ma...@denx.de>> schrieb am Mo., 13. > > Aug. 2018, 22:36: > > > > On 08/13/2018 09:34 PM, 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.goldschm...@gmail.com > > <mailto:simon.k.r.goldschm...@gmail.com>> > > > --- > > > > > > Changes in v4: enable this fix for all socfpga, not for gen5 only > > > Changes in v3: this patch is new in v3 > > > Changes in v2: None > > > > > > include/configs/socfpga_common.h | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/include/configs/socfpga_common.h > > b/include/configs/socfpga_common.h > > > index 8ebf6b85fe..d1148b838b 100644 > > > --- a/include/configs/socfpga_common.h > > > +++ b/include/configs/socfpga_common.h > > > @@ -284,6 +284,12 @@ unsigned int cm_get_qspi_controller_clk_hz(void); > > > #define CONFIG_SPL_STACK CONFIG_SYS_SPL_MALLOC_START > > > #endif > > > > > > +/* When U-Boot is started from FPGA, prevent gd->env_addr to > > point into > > > > Multi-line comment should have this format > > /* > > * foo > > * bar > > */ > > > > > > Right, of course. I wonder why patman didn't warm me about that... > > > > > > > + * FPGA OnChip RAM after relocation > > > + */ > > > +#define CONFIG_SYS_EXTRA_ENV_RELOC > > > +#define CONFIG_SYS_MONITOR_BASE CONFIG_SYS_TEXT_BASE /* > > start of monitor */ > > > > What you don't explain in the commit message is this last line. Why is > > this needed ? > > > > > > The code enabled by CONFIG_SYS_EXTRA_ENV_RELOC used this to calculate > > the relocation offset. I do think that's a bit strange, but I wouldn't > > change it with this patchset, or should I? > > You should document _why_ this is needed. Not "because the code enabled > by foo needed this", but why that code enabled this and why setting it > to SYS_TEXT_BASE is correct.
Yes, I wouldn't have sent a patch like that. I rather wanted to phrase that I don't know why this is needed for env relocation, as fdt relocation just uses gd->reloc_off. That might work for env relocation, too, but changing that seems out of scope for this patchset. Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
