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

[Marek Vasut]

On 2/14/19 4:37 PM, Chee, Tien Fong wrote:
> On Thu, 2019-02-14 at 16:21 +0100, Marek Vasut wrote:
>> 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.al
>>>>>>>>> tera
>>>>>>>>> .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 ?
> de-duplicated means calling for one time only?
> If that's the case, how de-deplicated? One call for processing one
> bistream, and this driver has different handling for different FPGA
> mode and different sequence(before and after DDR calibration).

I mean, make the two copies of code a single function and call that
function twice.

-- 
Best regards,
Marek Vasut