[U-Boot] [PATCH] ARM: socfpga: Remove socfpga_sdram_apply_static_cfg()

[Marek Vasut]

The usage of socfpga_sdram_apply_static_cfg() seems rather dubious and
is confirmed to lead to a rare system hang when enabling bridges. This
patch removes the socfpga_sdram_apply_static_cfg() altogether, because
it's use seems unjustified and problematic.

The socfpga_sdram_apply_static_cfg() triggers write to SDRAM staticcfg
register to set the applycfg bit, which according to old vendor U-Boot
sources can only be written when there is no traffic between the SDRAM
controller and the rest of the system. Empirical measurements confirm
this, setting the applycfg bit when there is traffic between the SDRAM
controller and CPU leads to the SDRAM controller accesses being blocked
shortly after.

Altera originally solved this by moving the entire code which sets the
staticcfg register to OCRAM [1]. The commit message claims that the
applycfg bit needs to be set after write to fpgaportrst register. This
is however inverted by Altera shortly after in [2], where the order
becomes the exact opposite of what commit message [1] claims to be the
required order. The explanation points to a possible problem in AMP
use-case, where the FPGA might be sending transactions through the F2S
bridge.

However, the AMP is only the tip of the iceberg here. Any of the other
L2, L3 or L4 masters can trigger transactions to the SDRAM. It becomes
rather non-trivial to guarantee there are no transactions to the SDRAM
controller.

The SoCFPGA SDRAM driver always writes the applycfg bit in SPL. Thus,
writing the applycfg again in bridge enable code seems redundant and
can presumably be dropped.

[1] https://github.com/altera-opensource/u-boot-socfpga/commit/75905816ec95b0ccd515700b922628d7aa9036f8
[2] https://github.com/altera-opensource/u-boot-socfpga/commit/8ba6986b04a91d23c7adf529186b34c8d2967ad5

Signed-off-by: Marek Vasut <marex at denx.de>
Cc: Chin Liang See <chin.liang.see at intel.com>
Cc: Dinh Nguyen <dinguyen at kernel.org>
Cc: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
Cc: Tien Fong Chee <tien.fong.chee at intel.com>
---
 arch/arm/mach-socfpga/misc_gen5.c | 31 -------------------------------
 1 file changed, 31 deletions(-)

diff --git a/arch/arm/mach-socfpga/misc_gen5.c b/arch/arm/mach-socfpga/misc_gen5.c
index 7876953595..eeba199edc 100644
--- a/arch/arm/mach-socfpga/misc_gen5.c
+++ b/arch/arm/mach-socfpga/misc_gen5.c
@@ -220,35 +220,6 @@ static struct socfpga_reset_manager *reset_manager_base =
 static struct socfpga_sdr_ctrl *sdr_ctrl =
 	(struct socfpga_sdr_ctrl *)SDR_CTRLGRP_ADDRESS;
 
-static void socfpga_sdram_apply_static_cfg(void)
-{
-	const u32 applymask = 0x8;
-	u32 val = readl(&sdr_ctrl->static_cfg) | applymask;
-
-	/*
-	 * SDRAM staticcfg register specific:
-	 * When applying the register setting, the CPU must not access
-	 * SDRAM. Luckily for us, we can abuse i-cache here to help us
-	 * circumvent the SDRAM access issue. The idea is to make sure
-	 * that the code is in one full i-cache line by branching past
-	 * it and back. Once it is in the i-cache, we execute the core
-	 * of the code and apply the register settings.
-	 *
-	 * The code below uses 7 instructions, while the Cortex-A9 has
-	 * 32-byte cachelines, thus the limit is 8 instructions total.
-	 */
-	asm volatile(
-		".align	5			\n"
-		"	b	2f		\n"
-		"1:	str	%0,	[%1]	\n"
-		"	dsb			\n"
-		"	isb			\n"
-		"	b	3f		\n"
-		"2:	b	1b		\n"
-		"3:	nop			\n"
-	: : "r"(val), "r"(&sdr_ctrl->static_cfg) : "memory", "cc");
-}
-
 void do_bridge_reset(int enable, unsigned int mask)
 {
 	int i;
@@ -263,14 +234,12 @@ void do_bridge_reset(int enable, unsigned int mask)
 		}
 
 		writel(iswgrp_handoff[2], &sysmgr_regs->fpgaintfgrp_module);
-		socfpga_sdram_apply_static_cfg();
 		writel(iswgrp_handoff[3], &sdr_ctrl->fpgaport_rst);
 		writel(iswgrp_handoff[0], &reset_manager_base->brg_mod_reset);
 		writel(iswgrp_handoff[1], &nic301_regs->remap);
 	} else {
 		writel(0, &sysmgr_regs->fpgaintfgrp_module);
 		writel(0, &sdr_ctrl->fpgaport_rst);
-		socfpga_sdram_apply_static_cfg();
 		writel(0, &reset_manager_base->brg_mod_reset);
 		writel(1, &nic301_regs->remap);
 	}
-- 
2.20.1