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.c...@intel.com wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> From: Tien Fong Chee <tien.fong.c...@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.c...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot