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

[Marek Vasut]

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.

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

> Oh, and by now I'm glad our own design connects to DDR via the main bus ;-)

Right.

-- 
Best regards,
Marek Vasut