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

[Marek Vasut]

On 3/16/20 8:04 PM, Simon Goldschmidt wrote:
> Am 16.03.2020 um 19:04 schrieb Dalon L Westergreen:
>>
>>
>> On Thu, 2020-03-12 at 16:57 +0100, Marek Vasut wrote:
>>> On 3/12/20 4:54 PM, Dalon L Westergreen wrote:
>>> [...]
>>>
>>> (thanks for fixing your mailer :))
>>>
>>>>>>>>> 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?
>>>>>>>
>>>>
>>>> Unfortunately the sdram cfg apply must occur AFTER the fpga is
>>>> configured.  This
>>>> is because the FPGA drives configuration bits, around the interfaces
>>>> datawidth
>>>> for example, that are used in setting up the dram interface.  I believe the
>>>> intent is for the command to only run when the ridge enable function is
>>>> called.
>>>
>>> So that's one part of the fix -- only run this apply_static_cfg if the
>>> bitstream uses the F2S bridge.
>>
>> actually, the restriction is to apply this only if the FPGA is configured,
>> whether the bridge is used is irrelevant.  you can further restrict this
>> to only when the bridge is used, but to me that means devicetree entries
>> or something to determine whether it is used.
> 
> In my opinion, we need an FPGA-specific devicetree (or something
> similar) to describe the FPGA image, anyway.

Like a DTO ?

> Today, too much
> configuration is applied at compile time (or when programming SPL to
> flash at latest) - there's currently no way to switch peripherals to
> FPGA for one image but not for another, for example.

Yes

> Worse: if you enable bridges but there's no slave attached, the CPU can
> be stuck. That would need to be described with the FPGA image as well.

Can you propose a patch ?

-- 
Best regards,
Marek Vasut