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

[Simon Goldschmidt]

Am 16.03.2020 um 20:55 schrieb Dalon L Westergreen:
> 
> 
> On Mon, 2020-03-16 at 20:06 +0100, Marek Vasut wrote:
>> 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 ?
> 
> DTOs are how i suggest solving this in linux, so i would assume a dto would
> be best here too.

Yes. That DTO should be beside the FPGA image, either in a FIT image or
just loaded separately. It should contain pin settings, bridge settings etc.

However, to ensure the bus is idle, we still would have to limit
applying that config bit to before RAM is set up, so quite early in SPL,
right? I cannot see how that would work, given the limit of 64K. Plus
it's kind of a bad boot flow to configure the FPGA before even starting
U-Boot...

Regards,
Simon

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