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

[Marek Vasut]

On 3/9/20 3:10 PM, Simon Goldschmidt wrote:
> On Mon, Mar 9, 2020 at 1:57 PM Marek Vasut <marex at denx.de> wrote:
>>
>> On 3/9/20 9:50 AM, Ley Foon Tan wrote:
>>> On Thu, Feb 13, 2020 at 2:52 AM Dalon L Westergreen
>>> <dalon.westergreen at linux.intel.com> wrote:
>>>>
>>>> I am reading through this thread, and want to point out that it is not that the
>>>> FPGA bridge need be actively used in the fpga, but
>>>> rather that this port be configured in the FPGA configuration.  This is an
>>>> important distinction, ecery FPGA design that
>>>> instantiates the HPS does configure the F2S Bridge.  it drives select pins,
>>>> control pins, data pins, etc, on the interface to known values,
>>>> and so the apply can always be done as all SoC FPGA designs have the soc
>>>> instantiated.  If someone boots the arm, and uses an
>>>> FPGA design without having the soc instantiated, it may then cause issues, but i
>>>> would argue that is not a supported use
>>>> in the first place.
>>>>
>>>> --dalon
>>>>
>>>
>>> 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?

Yes

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

Well, but later we have SDRAM running and we cannot make it go idle, can we?

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

Can you make SDRAM bus really idle ?