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

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

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