[U-Boot] [PATCH] ARM: socfpga: Remove socfpga_sdram_apply_static_cfg()

[Marek Vasut]

On 3/11/20 10:54 AM, Ley Foon Tan wrote:
> On Mon, Mar 9, 2020 at 10:10 PM Simon Goldschmidt
> <simon.k.r.goldschmidt at gmail.com> wrote:
>>
>> On Mon, Mar 9, 2020 at 1:57 PM Marek Vasut <marex at denx.de> wrote:
>>>
> 
>>>>>
>>>>
>>>> I can reproduce the issue if without setting applycfg bit. Access to
>>>> F2sdram interface will cause system hang.
>>>>
>>>> From the Cyclone 5 Soc datasheet:
>>>> applycfg - Write with this bit set to apply all the settings loaded in
>>>> SDR registers to the memory interface. This bit is write-only and
>>>> always returns 0 if read.
>>>>
>>>> Software should set applycfg bit if change any register under SDR
>>>> register range. SW is allowed to set this bit multiple times, the only
>>>> condition is SDRAM need to be idle.
>>>> My suggestion is we add back socfpga_sdram_apply_static_cfg(), but
>>>> change the sequence to below:
>>>> writel(iswgrp_handoff[3], &sdr_ctrl->fpgaport_rst);
>>>> socfpga_sdram_apply_static_cfg();
>>>>
>>>> Marek and Simon, are you okay with this? If yes, I will submit patch for this.
>>>
>>> The problem was that this was causing weird sporadic hangs on Gen5,
>>> which is why it was removed. So until there is an explanation for those
>>> hangs, I'm not really OK with this.
>>
>> These sporadic hangs you saw were on devices without an FPGA image directly
>> accessing the SDRAM ports, right?
>>
>>>
>>> Maybe the application of static config should happen in SPL, before the
>>> DRAM is running, or something like that ?
>>
>> Thinking this further, limiting it to applying in SPL is not a good idea since
>> that prevents us from implementing different FPGA images/configs by loading a
>> config later in the boot (i.e. in U-Boot from a FIT-image).
>>
>> Would it work to make setting this optional, i.e. only write the bit if an FPGA
>> config actually uses these ports? Then it couldn't lead to problems on other
>> hardware...
> 
> If the sporadic hangs issue happen on FPGA image that doesn't connect
> to f2sdram interface, we can add the checking to only apply static cfg
> when it has f2sdram port enabled.
> It can send a patch with this checking for you to try if want.
Well, what could cause that sporadic hang ?
Yes, the hang happens on image which doesn't access the SDRAM IIRC.

-- 
Best regards,
Marek Vasut