From: Marek Vasut Date: Tue, 23 Apr 2019 15:24:22 +0000 (+0200) Subject: ARM: socfpga: Remove socfpga_sdram_apply_static_cfg() X-Git-Url: http://git.dujemihanovic.xyz/img/static/git-logo.png?a=commitdiff_plain;h=c5f4b805755912a3d2fe20f014b6b6ab0473bd73;p=u-boot.git ARM: socfpga: Remove socfpga_sdram_apply_static_cfg() 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 Cc: Chin Liang See Cc: Dinh Nguyen Cc: Simon Goldschmidt Cc: Tien Fong Chee --- diff --git a/arch/arm/mach-socfpga/misc_gen5.c b/arch/arm/mach-socfpga/misc_gen5.c index dda95e8237..71547d81ab 100644 --- a/arch/arm/mach-socfpga/misc_gen5.c +++ b/arch/arm/mach-socfpga/misc_gen5.c @@ -210,35 +210,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; @@ -253,14 +224,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); }