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

[Ley Foon Tan]

On Fri, Mar 20, 2020 at 3:52 PM Ley Foon Tan <lftan.linux at gmail.com> wrote:

> > >>>>>> 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...
> There is usecase user may want to program fpga image in Uboot command
> prompt. It will have problem too.
>
> socfpga_sdram_apply_static_cfg() is written in assembly code and the
> code is fit to one cacheline size (32 bytes). This at least make sure
> CPU no access to SDRAM when run apply static cfg code in uboot.
> Frankly, this can't prevent external master access to SDRAM.
>
> Marek,
> I found code in drivers/fpga/socfpga_gen5.c:socfpga_load() write to
> SDR_CTRLGRP_FPGAPORTRST_ADDRESS register, but doesn't write to call to
> socfpga_sdram_apply_static_cfg() function. This means FPGA port is not
> put into reset before program fpga image. Suspect this might reason it
> cause sporadic hangs.
>
> Stay safe everyone.
>
> Regards
> Ley Foon

Hi

Cyclone5 F2sdram interface is totally broken in uboot now, can we
revert back the socfpga_sdram_apply_static_cfg() but add checking if
f2sdram bridge is enabled. Also add call to
socfpga_sdram_apply_static_cfg() in socfpga_load().
Simon's enhancement for DTO might not getting in soon.

Thanks.

Regards
Ley Foon