Re: [U-Boot] [PATCH 1/3] arm: socfpga: Remove unused FPGA feature from gen5 SPL

27 Jul 2017

On 07/27/2017 11:21 AM, Chee, Tien Fong wrote:
...
On Kha, 2017-07-27 at 10:21 +0200, Marek Vasut wrote:
...
On 07/27/2017 06:36 AM, tien.fong.chee@intel.com wrote:
...
From: Tien Fong Chee tien.fong.chee@intel.com
Remove unused FPGA feature for saving some space in gen5 SPL.
I really dislike the ifdefery. Why is this patch even needed?
I thought we had no space problems in the Gen5 SPL?
I can remove those codes within ifdefery because they are no longer
required.
Why ?
...
I keep them here just for one day we can use if we need to.
Do you remember we have consent to clean up those useless codes for
SPL, all fpga related drivers will be moved into one driver location.
No, sorry.
...
...
...
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
arch/arm/mach-socfpga/reset_manager_gen5.c  |    9 +++++++++
 arch/arm/mach-socfpga/system_manager_gen5.c |    6 ------
 drivers/ddr/altera/sdram.c                  |    8 +++++++-
 3 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-socfpga/reset_manager_gen5.c
b/arch/arm/mach-socfpga/reset_manager_gen5.c
index aa88adb..c954063 100644
--- a/arch/arm/mach-socfpga/reset_manager_gen5.c
+++ b/arch/arm/mach-socfpga/reset_manager_gen5.c
@@ -97,6 +97,14 @@ void socfpga_bridges_reset(int enable)
   	writel(0, &sysmgr_regs->iswgrp_handoff[0]);
   	writel(l3mask, &sysmgr_regs->iswgrp_handoff[1]);
+/*


FPGA feature is not required in SPL, so FPGA bridges release



from reset


should not be supported in SPL, but some FPGA releated setting



can be stored


in handoff registers as SPL to U-boot/OS handoff information.



These


handoff information can be used in later phase such as feeding



handoff


information to bridge command when user want to enable FPGA



feature in U-boot

*/

+#ifndef CONFIG_SPL_BUILD
   	/* Check signal from FPGA. */
   	if (!fpgamgr_test_fpga_ready()) {
   		/* FPGA not ready, do nothing. We allow
system to boot
@@ -110,6 +118,7 @@ void socfpga_bridges_reset(int enable)
/* Remap the bridges into memory map */
writel(l3mask, SOCFPGA_L3REGS_ADDRESS);

I believe the L3REGS needs to be programmed on Gen5.
Yes, this is done when calling command "bridge
enable". iswgrp_handoff[1] contains l3mask, which will be written
into SOCFPGA_L3REGS_ADDRESS.
I think this needs to be done earlier, otherwise the first 1 MiB or so
of DRAM isn't accessible.
...
Snippet from misc_gen5.c: 
int do_bridge(cmd_tbl_t *cmdtp, int flag, int argc, char * const
argv[])
{
   if (argc != 2)
   	return CMD_RET_USAGE;
argv++;
switch (*argv[0]) {
   case 'e':	/* Enable */
   	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);
break;


...
...
+#endif
   }
   return;
 }
diff --git a/arch/arm/mach-socfpga/system_manager_gen5.c
b/arch/arm/mach-socfpga/system_manager_gen5.c
index 3588a57..ee25496 100644
--- a/arch/arm/mach-socfpga/system_manager_gen5.c
+++ b/arch/arm/mach-socfpga/system_manager_gen5.c
@@ -43,12 +43,6 @@ static void
populate_sysmgr_fpgaintf_module(void)
   /* populate (not writing) the value for
SYSMGR.FPGAINTF.MODULE
   based on pinmux setting */
   setbits_le32(&sysmgr_regs->iswgrp_handoff[2],
handoff_val);


handoff_val = readl(&sysmgr_regs->iswgrp_handoff[2]);
if (fpgamgr_test_fpga_ready()) {
/* Enable the required signals only */


writel(handoff_val, &sysmgr_regs-



...
fpgaintfgrp_module);

}

}
/*
diff --git a/drivers/ddr/altera/sdram.c
b/drivers/ddr/altera/sdram.c
index e74c5b0..df16102 100644
--- a/drivers/ddr/altera/sdram.c
+++ b/drivers/ddr/altera/sdram.c
@@ -233,6 +233,7 @@ static void sdram_dump_protection_config(void)
   }
 }
+#ifndef CONFIG_SPL_BUILD
 /**

sdram_write_verify() - write to register and verify the write.
@addr:	Register address

@@ -259,6 +260,7 @@ static unsigned sdram_write_verify(const u32
*addr, const u32 val)
   debug("correct!\n");
   return 0;
 }
+#endif
/**

sdr_get_ctrlcfg() - Get the value of DRAM CTRLCFG register

@@ -435,7 +437,6 @@ int sdram_mmr_init_full(unsigned int
sdr_phy_reg)
   const unsigned int rows =
   	(cfg->dram_addrw &
SDR_CTRLGRP_DRAMADDRW_ROWBITS_MASK) >>
   		SDR_CTRLGRP_DRAMADDRW_ROWBITS_LSB;

int ret;
writel(rows, &sysmgr_regs->iswgrp_handoff[4]);


@@ -444,13 +445,18 @@ int sdram_mmr_init_full(unsigned int
sdr_phy_reg)
   /* saving this value to SYSMGR.ISWGRP.HANDOFF.FPGA2SDR */
   writel(cfg->fpgaport_rst, &sysmgr_regs-
...
iswgrp_handoff[3]);

/* FPGA feature is not required in SPL, so no test on FPGA

reset in SPL */
+#ifndef CONFIG_SPL_BUILD
   /* only enable if the FPGA is programmed */

int ret;
if (fpgamgr_test_fpga_ready()) {
  ret = sdram_write_verify(&sdr_ctrl->fpgaport_rst,
  			 cfg->fpgaport_rst);
  if (ret)
  	return ret;
 }

+#endif
/* Restore the SDR PHY Register if valid */
   if (sdr_phy_reg != 0xffffffff)
-- 
Best regards,
Marek Vasut

    