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

[Chee, Tien Fong]

On Kha, 2017-07-27 at 10:21 +0200, Marek Vasut wrote:
> On 07/27/2017 06:36 AM, tien.fong.chee at intel.com wrote:
> > 
> > From: Tien Fong Chee <tien.fong.chee at 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. 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.
> > 
> > Signed-off-by: Tien Fong Chee <tien.fong.chee at 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.
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)
> > 
>