[U-Boot] [PATCH] ARM: socfpga: Remove socfpga_sdram_apply_static_cfg()
Simon Goldschmidt
simon.k.r.goldschmidt at gmail.com
Fri Feb 7 14:44:23 CET 2020
On Fri, Feb 7, 2020 at 1:27 PM Marek Vasut <marex at denx.de> wrote:
>
> On 2/7/20 1:09 PM, Simon Goldschmidt wrote:
> > 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?
>
> From the handoff files ?
Yeah, well, I know that. I meant this is not available in U-Boot, see below.
>
> > 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.
>
> Well you can dig this information out of the handoff files , so you can
> also make this somehow configurable via either DT or bridge command args.
My point was this can differ between FPGA images. Once you load an FPGA image
that doesn't match what you expected when building (or flashing) U-Boot, the
system may lock up again.
DT is not an option, as you hard-code it when flashing U-Boot. The loaded FPGA
image can still change.Bridge command args is a way of how to get this info into
the code that runs, but from where do these command args come? You need to
somehow parse this info from an FPGA image file read from storage.
>
> > 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.
>
> fitImage ? :)
I thought of that, too. Maybe a short dts (maybe overlay) beside the FPGA. But
then it would again not work when loading a pure rbf. But then again, that might
be ok...
So I could imagine this to "just work" when someday I finally have finished my
series of moving this handoff info into dts and then including an overlay dts
into a fit image that gets applied after programming the FPGA (and by/after
applying it, the code that applies bridge settings would run). Would that work?
Regards,
Simon
>
> > 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...
>
> ... or something is queued up in some buffer somewhere in the bridge.
More information about the U-Boot
mailing list