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

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

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