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

[Simon Goldschmidt]

On Fri, Feb 7, 2020 at 8:56 AM Marek Vasut <marex at denx.de> wrote:
>
> On 2/6/20 3:57 PM, Simon Goldschmidt wrote:
> > On Thu, Feb 6, 2020 at 3:41 PM Nico Becker <n.becker at ic-automation.de> wrote:
> >>
> >> Am 06.02.2020 um 14:00 schrieb Marek Vasut:
> >>> On 2/6/20 1:57 PM, Nico Becker wrote:
> >>>> Am 06.02.2020 um 12:53 schrieb Marek Vasut:
> >>>>> On 2/6/20 11:50 AM, Nico Becker wrote:
> >>>>>> Hello,
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>>> after removing the function socfpga_sdram_apply_static_cfg() in
> >>>>>> misc_gen5 we can not use the FPGA2SDRAM bridge.
> >>>>>>
> >>>>>> Without the apply static cfg the kernel crash every time,
> >>>>>> if we try to write @ the fpga2sdram bridge. After an soft reset
> >>>>>> everything is working.
> >>>>>>
> >>>>>> If we add the socfpga_sdram_apply_static_cfg() in the
> >>>>>> u-boot source code, everything is fine.
> >>>>>> Now we can use the fpga2sdram bridge after power on.
> >>>>>>
> >>>>>> Our setup:
> >>>>>> - u-boot v2020.01
> >>>>>>     - load and write fpga firmware
> >>>>>>     - enable bridges
> >>>>>> - boot linux
> >>>>>>
> >>>>>> I have found some information at
> >>>>>> <https://support.criticallink.com/redmine/projects/mityarm-5cs/wiki/Important_Note_about_FPGAHPS_SDRAM_Bridge>
> >>>>>>
> >>>>>>
> >>>>>> <http://u-boot.10912.n7.nabble.com/WG-Linux-hang-td314276.html>
> >>>>>
> >>>>> Can you send a patch which fixes this for you, with Fixes: tag ?
> >>>>> Then it would be clear what you changed.
> >>>>>
> >>>>> Thanks
> >>>>>
> >>>>
> >>>> Hello,
> >>>> the code was removed @ commit c5f4b805.
> >>>
> >>> Did you read the commit message of that commit and what problem that was
> >>> solving ? Clearly, reverting the commit isn't the way to go. We need to
> >>> find a way to unbreak this for you, while not break other platforms.
> >>>
> >>>> I attached my patch, sorry for the format, i am new in this.
> >>>
> >>> [...]
> >>>
> >> Hi,
> >> yes i read the commit message.
> >>
> >> but i found no other option to enable the sdram bridges,
> >> without crashes/hanging up linux, with the removed source code.
> >>
> >> i ve found some more information @intel
> >> <https://www.intel.com/content/www/us/en/programmable/support/support-resources/knowledge-base/embedded/2016/how-and-when-can-i-enable-the-fpga2sdram-bridge-on-cyclone-v-soc.html>
> >>
> >> Intel talk about an bridge_enable_handoff in my opionion
> >> the cmd set the sram aply cfg
> >>
> >> /* add signle command to enable all bridges based on handoff */
> >> setenv("bridge_enable_handoff",
> >>         "mw $fpgaintf ${fpgaintf_handoff}; "
> >>         "go $fpga2sdram_apply; "
> >>         "mw $fpga2sdram ${fpga2sdram_handoff}; "
> >>         "mw $axibridge ${axibridge_handoff}; "
> >>         "mw $l3remap ${l3remap_handoff} ");
> >>
> >> setenv_addr("fpga2sdram_apply", (void *)sdram_applycfg_uboot);
> >>
> >> /*
> >>   * Relocate the sdram_applycfg_ocram function to OCRAM and call it
> >>   */
> >> ENTRY(sdram_applycfg_uboot)
> >>         PUSH    {r4-r11, lr}            /* save registers per AAPCS */
> >>
> >>         ldr     r1, =sdram_applycfg_ocram
> >>         ldr     r2, =CONFIG_SYS_INIT_RAM_ADDR
> >>         mov     r3, r2
> >>         ldmia   r1!, {r4 - r11}
> >>         stmia   r3!, {r4 - r11}
> >>         ldmia   r1!, {r4 - r11}         /* copy more in case code added */
> >>         stmia   r3!, {r4 - r11}         /* in the future */
> >>         ldmia   r1!, {r4 - r11}         /* copy more in case code added */
> >>         stmia   r3!, {r4 - r11}         /* in the future */
> >>         dsb
> >>         isb
> >>         blx     r2                      /* jump to OCRAM */
> >>         POP     {r4-r11, pc}
> >> ENDPROC(sdram_applycfg_uboot)
> >>
> >>
> >> it could be an option to write the fpga firmware with u-boot,
> >> and do a soft reset.
> >> boot u-boot
> >> check fpga configuration state
> >> not configured write firmware reset
> >> if configured boot linux
> >>
> >> i ll check howto to determine the fpga configuration state
> >> and try this.
> >
> > That doesn't look like a safe plan: what if you explicitly *want* a reboot
> > and want to reconfigure the FPGA then to ensure it is in a well-known state?
> >
> > Marek, what were the problems why this has been removed? Aside from "is
> > confirmed to lead to a rare system hang when enabling bridges", the commit
> > message stays rather vague.
>
> That's exactly what the problem was. I didn't manage to get further
> details from Altera on this.

Hrmpf :-(

>
> > Maybe that "apply config" bit should only be written
> > if explicitly configured so, but that would mean we need some kind of config
> > options included/coming with the FPGA image we program.
>
> Maybe the apply config should only be used if the F2S bridge is being used ?

Yes, well how do you know after programming an FPGA that this bridge is in use?
This depends on the FPGA image, while currently the Altera work flow compiles
it into the U-Boot binary.While I'm still working on moving this into the U-Boot
dts (I still have size issues there), it's still not encoded with the FPGA.

What we would need is some kind of meta info with the FPGA image that tells us
how to configure the bridges (and maybe some clocks or hardware handed off into
the FPGA) after programming the FPGA.

Only then, we could write the apply config bit after programming the FPGA.

As to the "SDRAM access must be idle", I think that should work from U-Boot. All
peripherals should be idle, unless the FPGA accesses SDRAM right after being
configured. Maybe we'd need to wait a bit in case the cache is filling a line
or so...

Regards,
Simon

>
> > Oh, and by now I'm glad our own design connects to DDR via the main bus ;-)
>
> Right.
>
> --
> Best regards,
> Marek Vasut