[U-Boot] [RFC PATCH] ARM: zynq: Replace dram_init* functions with board_init_f safe ones

[Michal Simek]

Hi Nathan,

On 4.12.2016 10:33, Nathan Rossi wrote:
> The dram_init* functions for the zynq board are not safe for use from
> the board_init_f stage due to its use of the 'tmp' static variable.
> 
> This incorrect use of a static variable was causing rare issues where
> the dram_init function would overwrite some parts the __rel_dyn section
> which caused obscure failures.
> 
> Using the zynq_zybo configuration, U-Boot would generate the following
> error during image load. This was caused due to dram_init overwriting
> the relocations for the "image" variable within the do_bootm function.
> Out of coincidence the un-initialized memory has a compression type
> which is the same as the value for the relocation type R_ARM_RELATIVE.
> 
>    Uncompressing Invalid Image ... Unimplemented compression type 23

First of all I didn't see any issue like this before but it is serious
issue.

> 
> It should be noted that this is just one way the issue could surface,
> other cases my not be observed in normal boot flow.
> 
> This change removes the existing code and copies the implementation of
> the dram_init and dram_init_banksize from the
> arch/arm/mach-uniphier/dram_init.c source. This version of these
> functions does not use static variables and behaves the same (reading
> banks from fdt, and using the first bank as the ram_size).
> 
> Signed-off-by: Nathan Rossi <nathan at nathanrossi.com>
> Fixes: 758f29d0f8 ("ARM: zynq: Support systems with more memory banks")
> Cc: Michal Simek <monstr at monstr.eu>
> ---
> I am sending this as an RFC first to query whether it would make sense
> to have the setup of memory info based on FDT information as a common
> function that could be re-used or maybe always used (if OF enabled and
> SDRAM_SIZE not used) from board_init_f. And in that regard get some
> feedback on the best way to implement such.
> 
> This change would also need to be applied to the zynqmp dram_init*
> functions.

Yes - the same stuff is used by zynqmp. TBH I would prefer to get this
code to generic location and use it.
It will be necessary to test zynq and zynqmp code and I am happy if this
is working for all valid configurations.
address cells, size-cells
1	1
2	1
1	2
2	2

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP SoCs


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161206/cd43a57a/attachment.sig>