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.c...@intel.com wrote:
>>>
>>> From: Tien Fong Chee <tien.fong.c...@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.c...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
