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

[Simon Goldschmidt]

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

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.

Regards,
Simon

> 
>>
>>>>>>> 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 ?
>>>>>
>>>>> From the CPU side, that should work, no? Of course you have to make sure
>>>>> no
>>>>> other peripheraly (including FPGA!) are using the RAM.
>>>>>
>>>>> And if this would be an explicit command, people needing this could
>>>>> experiment with it - and hopefully give better hints as to what's going
>>>>> wrong
>>>>> if we *do* see problems again.
>>>>
>>>> Maybe altera has something hidden somewhere in the bus tunables ? :)
>>>
>>> The only trick i am aware of, and Ley Foon, please comment, is isolating
>>> relevant code to the caches before executing.
>>
>> How do you make sure some DMA doesn't do something funny or some pending
>> write doesn't trigger DRAM write ? There is more than the CPU that can
>> access the DRAM and cause bus traffic.
> 
> True, and it is unclear how we could ensure there is absolutely no traffic.
> 
> --dalon
> 
>