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

[Marek Vasut]

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 ?

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

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

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