[U-Boot] [PATCH v8 6/8] spl : socfpga: Implement fpga bitstream loading with socfpga loadfs

[Marek Vasut]

On 2/14/19 12:38 PM, Chee, Tien Fong wrote:
> On Thu, 2019-02-14 at 11:42 +0100, Marek Vasut wrote:
>> On 2/14/19 7:50 AM, Chee, Tien Fong wrote:
>>>
>>> On Wed, 2019-02-13 at 17:25 +0100, Marek Vasut wrote:
>>>>
>>>> On 2/13/19 3:18 PM, tien.fong.chee at intel.com wrote:
>>>>>
>>>>>
>>>>> From: Tien Fong Chee <tien.fong.chee at intel.com>
>>>>>
>>>>> Add support for loading FPGA bitstream to get DDR up running
>>>>> before
>>>>> U-Boot is loaded into DDR. Boot device initialization, generic
>>>>> firmware
>>>>> loader and SPL FAT support are required for this whole
>>>>> mechanism to
>>>>> work.
>>>>>
>>>>> Signed-off-by: Tien Fong Chee <tien.fong.chee at intel.com>
>>>>>
>>>>> ---
>>>>>
>>>>> changes for v7
>>>>> - Removed casting for get_fpga_filename
>>>>> - Removed hard coding DDR address for loading core bistream,
>>>>> using
>>>>> loadable
>>>>>   property from FIT.
>>>>> - Added checking for config_pins, return if error.
>>>>> ---
>>>>>  arch/arm/mach-socfpga/spl_a10.c | 41
>>>>> ++++++++++++++++++++++++++++++++++++++++-
>>>>>  1 file changed, 40 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-
>>>>> socfpga/spl_a10.c
>>>>> index c97eacb..36003b1 100644
>>>>> --- a/arch/arm/mach-socfpga/spl_a10.c
>>>>> +++ b/arch/arm/mach-socfpga/spl_a10.c
>>>>> @@ -1,6 +1,6 @@
>>>>>  // SPDX-License-Identifier: GPL-2.0+
>>>>>  /*
>>>>> - *  Copyright (C) 2012 Altera Corporation <www.altera.com>
>>>>> + *  Copyright (C) 2012-2019 Altera Corporation <www.altera.com
>>>>>>
>>>>>   */
>>>>>  
>>>>>  #include <common.h>
>>>>> @@ -23,6 +23,8 @@
>>>>>  #include <fdtdec.h>
>>>>>  #include <watchdog.h>
>>>>>  #include <asm/arch/pinmux.h>
>>>>> +#include <asm/arch/fpga_manager.h>
>>>>> +#include <mmc.h>
>>>>>  
>>>>>  DECLARE_GLOBAL_DATA_PTR;
>>>>>  
>>>>> @@ -68,11 +70,48 @@ u32 spl_boot_mode(const u32 boot_device)
>>>>>  
>>>>>  void spl_board_init(void)
>>>>>  {
>>>>> +	char buf[16 * 1024] __aligned(ARCH_DMA_MINALIGN);
>>>> ALLOC_CACHE_ALIGN_BUFFER()
>>> #define ARCH_DMA_MINALIGN	CONFIG_SYS_CACHELINE_SIZE
>>> They are not same thing?
>> See include/memalign.h and other drivers, the macro is preferred as
>> it
>> hides the details.
> Okay.
>>
>>>
>>>>
>>>>>
>>>>>
>>>>> +
>>>>>  	/* enable console uart printing */
>>>>>  	preloader_console_init();
>>>>>  	WATCHDOG_RESET();
>>>>>  
>>>>>  	arch_early_init_r();
>>>>> +
>>>>> +	/* If the full FPGA is already loaded, ie.from EPCQ,
>>>>> config fpga pins */
>>>>> +	if (is_fpgamgr_user_mode()) {
>>>>> +		int ret = config_pins(gd->fdt_blob, "shared");
>>>>> +
>>>>> +		if (ret)
>>>>> +			return;
>>>>> +
>>>>> +		ret = config_pins(gd->fdt_blob, "fpga");
>>>>> +		if (ret)
>>>>> +			return;
>>>>> +	} else if (!is_fpgamgr_early_user_mode()) {
>>>>> +		/* Program IOSSM(early IO release) or full
>>>>> FPGA */
>>>>> +		fpga_fs_info fpga_fsinfo;
>>>>> +		int len;
>>>>> +
>>>>> +		fpga_fsinfo.filename = get_fpga_filename(gd-
>>>>>>
>>>>>> fdt_blob, &len);
>>>>> +
>>>>> +		if (fpga_fsinfo.filename)
>>>>> +			socfpga_loadfs(&fpga_fsinfo, buf,
>>>>> sizeof(buf), 0);
>>>> Why is this code here twice ? The same code seems to be below ...
>>> The 1st calling for periph program, then running ddr calibration,
>>> then
>>> 2nd calling for core program.
>> Then maybe the code can be deduplicated ?
> Hmm...seems cannot, because
> 1. DDR calibration is not part of fpga code.
> 2. fpga driver can only be used to process one bistream at a time,
> because different mode has different handling.

+	} else if (!is_fpgamgr_early_user_mode()) {
+		/* Program IOSSM(early IO release) or full FPGA */
+		fpga_fs_info fpga_fsinfo;
+		int len;
+
+		fpga_fsinfo.filename = get_fpga_filename(gd->fdt_blob, &len);
+
+		if (fpga_fsinfo.filename)
+			socfpga_loadfs(&fpga_fsinfo, buf, sizeof(buf), 0);
...
+	if (!is_fpgamgr_user_mode()) {
+		fpga_fs_info fpga_fsinfo;
+		int len;
+
+		fpga_fsinfo.filename = get_fpga_filename(gd->fdt_blob, &len);
+
+		if (fpga_fsinfo.filename)
+			socfpga_loadfs(&fpga_fsinfo, buf, sizeof(buf), 0);

These two chunks look the same to me , no ?

-- 
Best regards,
Marek Vasut