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.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
- 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
1) you could only call the fpga-core programming if needed 2) 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?
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