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

[Marek Vasut]

On 2/14/19 4:15 PM, Chee, Tien Fong wrote:
> On Thu, 2019-02-14 at 13:28 +0100, Marek Vasut wrote:
>> 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 ?
> Yes, they are being called twice at different fpga mode, and different
> sequence, before and after DDR calibration.

So why can't these two chunks of code be de-duplicated ?

-- 
Best regards,
Marek Vasut