On Thu, 2019-02-14 at 16:33 +0000, Westergreen, Dalon wrote:
On Thu, 2019-02-14 at 15:15 +0000, 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@intel.com wrote: > > > > > > > > From: Tien Fong Chee tien.fong.chee@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@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
- DDR calibration is not part of fpga code.
- 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.
I believe i understand the issue here. The code is a little more convoluted then it should be. The issue actually stems from first_loading_rbf_to_buffer which behaves differently depending on the state of the fpga. ... + uname = fit_get_name(buffer_p, images_noffset, NULL); + if (uname) { + debug("FPGA: %s\n", uname);
+ if (strstr(uname, "fpga-periph") && + (!is_fpgamgr_early_user_mode() || + is_fpgamgr_user_mode())) { + fpga_node_name = uname; + printf("FPGA: Start to program "); + printf("peripheral/full bitstream ...\n"); + break; + } else if (strstr(uname, "fpga-core") && + (is_fpgamgr_early_user_mode() && + !is_fpgamgr_user_mode())) { + fpga_node_name = uname; + printf("FPGA: Start to program core "); + printf("bitstream ...\n"); + break; + } + } ... so when called with an unprogrammed fpga or a fully programmed fpga, it will program the fpga-periph. When called with a programmed periph image and unprogrammed core is will program the the core.
so socfpga_loadfs needs to be called twice to fully program the fpga.
My question is, for a configuration where only the periph image is programmed, would this not result in an erroneous error message?
+ debug("FPGA: No suitable bitstream was found, count: %d.\n", i);
I wounder if it would not be better to move the fpga image fit parsing out of the socfpga_loadfs function so
- you could only call the fpga-core programming if needed
- socfpga_loadfs is explicitly called to program either the core
or the periph image
alternately, perhaps a call to socfpga_loadfs should program both images if present, and initialize the ddr if required?
DDR calibration is not a FPGA stuff, so i think it should not be part of FPGA driver code. I believe a call to socfpga_loadfs would be more complicated for both images. What about for the case only periph program is required?
I am not so keen on socfpga_loadfs behaving differently like this with the same function call.
--dalon
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot