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

[Marek Vasut]

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 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.

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 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.

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