[U-Boot] [PATCH v9 3/7] ARM: socfpga: Add FPGA drivers for Arria 10 FPGA bitstream loading

Chee, Tien Fong tien.fong.chee at intel.com
Wed Feb 27 06:35:30 UTC 2019


On Tue, 2019-02-26 at 16:46 +0100, Michal Simek wrote:
> On 26. 02. 19 15:53, Chee, Tien Fong wrote:
> > 
> > On Tue, 2019-02-26 at 15:20 +0100, Michal Simek wrote:
> > > 
> > > On 19. 02. 19 4:47, tien.fong.chee at intel.com wrote:
> > > > 
> > > > 
> > > > From: Tien Fong Chee <tien.fong.chee at intel.com>
> > > > 
> > > > Add FPGA driver to support program FPGA with FPGA bitstream
> > > > loading
> > > > from
> > > > filesystem. The driver are designed based on generic firmware
> > > > loader
> > > > framework. The driver can handle FPGA program operation from
> > > > loading FPGA
> > > > bitstream in flash to memory and then to program FPGA.
> > > > 
> > > > Signed-off-by: Tien Fong Chee <tien.fong.chee at intel.com>
> > > > 
> > > > ---
> > > > 
> > > > changes for v9
> > > > - Support data offset
> > > > - Added default DDR load address
> > > > - Squashed the image.h
> > > > - Changed to phandle
> > > > - Ensure the DDR is fully up running by checking the gd->ram
> > > > 
> > > > changes for v8
> > > > - Added codes to discern bitstream type based on fpga node
> > > > name.
> > > > 
> > > > changes for v7
> > > > - Restructure the FPGA driver to support both peripheral
> > > > bitstream
> > > > and core
> > > >   bitstream bundled into FIT image.
> > > > - Support loadable property for core bitstream. User can set
> > > > loadable
> > > >   in DDR for better performance. This loading would be done in
> > > > one
> > > > large
> > > >   chunk instead of chunk by chunk loading with small memory
> > > > buffer.
> > > > ---
> > > >  arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts       |  17 +
> > > >  .../include/mach/fpga_manager_arria10.h            |  40 +-
> > > >  drivers/fpga/socfpga_arria10.c                     | 533
> > > > ++++++++++++++++++++-
> > > >  include/image.h                                    |   4 +
> > > >  4 files changed, 571 insertions(+), 23 deletions(-)
> > > > 
> > > > diff --git a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
> > > > b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
> > > > index 998d811..9d43111 100644
> > > > --- a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
> > > > +++ b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
> > > > @@ -18,6 +18,23 @@
> > > >  /dts-v1/;
> > > >  #include "socfpga_arria10_socdk.dtsi"
> > > >  
> > > > +/ {
> > > > +	chosen {
> > > > +		firmware-loader = <&fs_loader0>;
> > > > +	};
> > > > +
> > > > +	fs_loader0: fs-loader at 0 {
> > > again @0 without reg properly is wrong.
> > Mind to explain more?
How about fs-loader-0?
> > > 
> > > 
> > > > 
> > > > 
> > > > +		u-boot,dm-pre-reloc;
> > > > +		compatible = "u-boot,fs-loader";
> > > > +		phandlepart = <&mmc 1>;
> > > > +	};
> > > I think that this will be nacked by DT guys.
> > > 
> > > > 
> > > > 
> > > > +};
> > > > +
> > > > +&fpga_mgr {
> > > > +	u-boot,dm-pre-reloc;
> > > > +	altr,bitstream = "fit_spl_fpga.itb";
> > > > +};
> > > > +
> > > >  &mmc {
> > > >  	u-boot,dm-pre-reloc;
> > > >  	status = "okay";
> > > > diff --git a/arch/arm/mach-
> > > > socfpga/include/mach/fpga_manager_arria10.h b/arch/arm/mach-
> > > > socfpga/include/mach/fpga_manager_arria10.h
> > > > index 09d13f6..7a4f723 100644
> > > > --- a/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h
> > > > +++ b/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h
> > > > @@ -1,9 +1,13 @@
> > > >  /* SPDX-License-Identifier: GPL-2.0 */
> > > >  /*
> > > > - * Copyright (C) 2017 Intel Corporation <www.intel.com>
> > > > + * Copyright (C) 2017-2019 Intel Corporation <www.intel.com>
> > > >   * All rights reserved.
> > > >   */
> > > >  
> > > > +#include <asm/cache.h>
> > > > +#include <altera.h>
> > > > +#include <image.h>
> > > > +
> > > >  #ifndef _FPGA_MANAGER_ARRIA10_H_
> > > >  #define _FPGA_MANAGER_ARRIA10_H_
> > > >  
> > > > @@ -51,6 +55,10 @@
> > > >  #define ALT_FPGAMGR_IMGCFG_CTL_02_CFGWIDTH_SET_MSK		
> > > > BIT(24)
> > > >  #define ALT_FPGAMGR_IMGCFG_CTL_02_CDRATIO_LSB			
> > > > 16
> > > >  
> > > > +#define FPGA_SOCFPGA_A10_RBF_UNENCRYPTED	0xa65c
> > > > +#define FPGA_SOCFPGA_A10_RBF_ENCRYPTED		0xa65d
> > > > +#define FPGA_SOCFPGA_A10_RBF_PERIPH		0x0001
> > > > +#define FPGA_SOCFPGA_A10_RBF_CORE		0x8001
> > > >  #ifndef __ASSEMBLY__
> > > >  
> > > >  struct socfpga_fpga_manager {
> > > > @@ -88,12 +96,40 @@ struct socfpga_fpga_manager {
> > > >  	u32  imgcfg_fifo_status;
> > > >  };
> > > >  
> > > > +enum rbf_type {
> > > > +	unknown,
> > > > +	periph_section,
> > > > +	core_section
> > > > +};
> > > > +
> > > > +enum rbf_security {
> > > > +	invalid,
> > > > +	unencrypted,
> > > > +	encrypted
> > > > +};
> > > > +
> > > > +struct rbf_info {
> > > > +	enum rbf_type section;
> > > > +	enum rbf_security security;
> > > > +};
> > > > +
> > > > +struct fpga_loadfs_info {
> > > > +	fpga_fs_info *fpga_fsinfo;
> > > > +	u32 remaining;
> > > > +	u32 offset;
> > > > +	struct rbf_info rbfinfo;
> > > > +};
> > > > +
> > > >  /* Functions */
> > > >  int fpgamgr_program_init(u32 * rbf_data, size_t rbf_size);
> > > >  int fpgamgr_program_finish(void);
> > > >  int is_fpgamgr_user_mode(void);
> > > >  int fpgamgr_wait_early_user_mode(void);
> > > > -
> > > > +int is_fpgamgr_early_user_mode(void);
> > > > +const char *get_fpga_filename(const void *fdt, int *len);
> > > > +int socfpga_loadfs(fpga_fs_info *fpga_fsinfo, const void *buf,
> > > > size_t bsize,
> > > > +		  u32 offset);
> > > > +void fpgamgr_program(const void *buf, size_t bsize, u32
> > > > offset);
> > > >  #endif /* __ASSEMBLY__ */
> > > >  
> > > >  #endif /* _FPGA_MANAGER_ARRIA10_H_ */
> > > > diff --git a/drivers/fpga/socfpga_arria10.c
> > > > b/drivers/fpga/socfpga_arria10.c
> > > > index 114dd91..9936b69 100644
> > > > --- a/drivers/fpga/socfpga_arria10.c
> > > > +++ b/drivers/fpga/socfpga_arria10.c
> > > > @@ -1,8 +1,7 @@
> > > >  // SPDX-License-Identifier: GPL-2.0
> > > >  /*
> > > > - * Copyright (C) 2017 Intel Corporation <www.intel.com>
> > > > + * Copyright (C) 2017-2019 Intel Corporation <www.intel.com>
> > > >   */
> > > > -
> > > >  #include <asm/io.h>
> > > >  #include <asm/arch/fpga_manager.h>
> > > >  #include <asm/arch/reset_manager.h>
> > > > @@ -10,8 +9,11 @@
> > > >  #include <asm/arch/sdram.h>
> > > >  #include <asm/arch/misc.h>
> > > >  #include <altera.h>
> > > > +#include <asm/arch/pinmux.h>
> > > >  #include <common.h>
> > > > +#include <dm/ofnode.h>
> > > >  #include <errno.h>
> > > > +#include <fs_loader.h>
> > > >  #include <wait_bit.h>
> > > >  #include <watchdog.h>
> > > >  
> > > > @@ -21,6 +23,9 @@
> > > >  #define COMPRESSION_OFFSET	229
> > > >  #define FPGA_TIMEOUT_MSEC	1000  /* timeout in ms */
> > > >  #define FPGA_TIMEOUT_CNT	0x1000000
> > > > +#define DEFAULT_DDR_LOAD_ADDRESS	0x400
> > > > +
> > > > +DECLARE_GLOBAL_DATA_PTR;
> > > >  
> > > >  static const struct socfpga_fpga_manager *fpga_manager_base =
> > > >  		(void *)SOCFPGA_FPGAMGRREGS_ADDRESS;
> > > > @@ -64,7 +69,7 @@ static int wait_for_user_mode(void)
> > > >  		1, FPGA_TIMEOUT_MSEC, false);
> > > >  }
> > > >  
> > > > -static int is_fpgamgr_early_user_mode(void)
> > > > +int is_fpgamgr_early_user_mode(void)
> > > This is called inside the same file that's why no reason for this
> > > change.
> > > Maybe you are using that later but it just suggest incorrect
> > > split.
> > This is part of complete fpga driver that was accepted long time
> > ago,
> > and now we just submitted the complete fpga driver. I have no
> > strong
> > opinion about this.
> > 
> > Marek, what do you think?
> > > 
> > > 
> > > 
> > > > 
> > > > 
> > > >  {
> > > >  	return (readl(&fpga_manager_base->imgcfg_stat) &
> > > >  		ALT_FPGAMGR_IMGCFG_STAT_F2S_EARLY_USERMODE_SET
> > > > _MSK
> > > > ) != 0;
> > > > @@ -94,7 +99,7 @@ int fpgamgr_wait_early_user_mode(void)
> > > >  		i++;
> > > >  	}
> > > >  
> > > > -	debug("Additional %i sync word needed\n", i);
> > > > +	debug("FPGA: Additional %i sync word needed\n", i);
> > > it should be separate patch.
> > > 
> > > > 
> > > > 
> > > >  
> > > >  	/* restoring original CDRATIO */
> > > >  	fpgamgr_set_cd_ratio(cd_ratio);
> > > > @@ -172,9 +177,10 @@ static int
> > > > fpgamgr_set_cdratio_cdwidth(unsigned int cfg_width, u32
> > > > *rbf_data,
> > > >  	compress = (rbf_data[COMPRESSION_OFFSET] >> 1) & 1;
> > > >  	compress = !compress;
> > > >  
> > > > -	debug("header word %d = %08x\n", 69, rbf_data[69]);
> > > > -	debug("header word %d = %08x\n", 229, rbf_data[229]);
> > > > -	debug("read from rbf header: encrypt=%d
> > > > compress=%d\n",
> > > > encrypt, compress);
> > > > +	debug("FPGA: Header word %d = %08x.\n", 69,
> > > > rbf_data[69]);
> > > > +	debug("FPGA: Header word %d = %08x.\n", 229,
> > > > rbf_data[229]);
> > > > +	debug("FPGA: Read from rbf header: encrypt=%d
> > > > compress=%d.\n", encrypt,
> > > > +	     compress);
> > > separate patch  - just disturbing reviewers and you are not
> > > saying
> > > nothing about it in commit message.
> > > 
> > > > 
> > > > 
> > > >  
> > > >  	/*
> > > >  	 * from the register map description of cdratio in
> > > > imgcfg_ctrl_02:
> > > > @@ -359,6 +365,7 @@ static int fpgamgr_program_poll_cd(void)
> > > >  			printf("nstatus == 0 while waiting for
> > > > condone\n");
> > > >  			return -EPERM;
> > > >  		}
> > > > +		WATCHDOG_RESET();
> > > >  	}
> > > >  
> > > >  	if (i == FPGA_TIMEOUT_CNT)
> > > > @@ -432,7 +439,6 @@ int fpgamgr_program_finish(void)
> > > >  		printf("FPGA: Poll CD failed with error code
> > > > %d\n", status);
> > > >  		return -EPERM;
> > > >  	}
> > > > -	WATCHDOG_RESET();
> > > These two looks like separate patch too.
> > > 
> > > > 
> > > > 
> > > >  
> > > >  	/* Ensure the FPGA entering user mode */
> > > >  	status = fpgamgr_program_poll_usermode();
> > > > @@ -447,27 +453,512 @@ int fpgamgr_program_finish(void)
> > > >  	return 0;
> > > >  }
> > > >  
> > > > -/*
> > > > - * FPGA Manager to program the FPGA. This is the interface
> > > > used by
> > > > FPGA driver.
> > > > - * Return 0 for sucess, non-zero for error.
> > > > - */
> > > > +ofnode get_fpga_mgr_ofnode(void)
> > > > +{
> > > > +	int node_offset;
> > > > +
> > > > +	fdtdec_find_aliases_for_id(gd->fdt_blob, "fpga_mgr",
> > > nit: using of live functions would be better to get rid of gd->.
> > Are you means using ofnode?
> > > 
> > > 
> > > > 
> > > > 
> > > > +				COMPAT_ALTERA_SOCFPGA_FPGA0,
> > > > +				&node_offset, 1);
> > > > +
> > > > +	return offset_to_ofnode(node_offset);
> > > > +}
> > > > +
> > > > +const char *get_fpga_filename(const void *fdt, int *len)
> > > > +{
> > > > +	const char *fpga_filename = NULL;
> > > > +
> > > > +	ofnode fpgamgr_node = get_fpga_mgr_ofnode();
> > > > +
> > > > +	if (ofnode_valid(fpgamgr_node))
> > > > +		fpga_filename =
> > > > ofnode_read_string(fpgamgr_node,
> > > > +						"altr,bitstrea
> > > > m");
> > > > +
> > > > +	return fpga_filename;
> > > > +}
> > > > +
> > > > +static void get_rbf_image_info(struct rbf_info *rbf, u16
> > > > *buffer)
> > > > +{
> > > > +	/*
> > > > +	 * Magic ID starting at:
> > > > +	 * -> 1st dword[15:0] in periph.rbf
> > > > +	 * -> 2nd dword[15:0] in core.rbf
> > > > +	 * Note: dword == 32 bits
> > > > +	 */
> > > > +	u32 word_reading_max = 2;
> > > > +	u32 i;
> > > > +
> > > > +	for (i = 0; i < word_reading_max; i++) {
> > > > +		if (*(buffer + i) ==
> > > > FPGA_SOCFPGA_A10_RBF_UNENCRYPTED) {
> > > > +			rbf->security = unencrypted;
> > > > +		} else if (*(buffer + i) ==
> > > > FPGA_SOCFPGA_A10_RBF_ENCRYPTED) {
> > > > +			rbf->security = encrypted;
> > > > +		} else if (*(buffer + i + 1) ==
> > > > +				FPGA_SOCFPGA_A10_RBF_UNENCRYPT
> > > > ED)
> > > > {
> > > > +			rbf->security = unencrypted;
> > > > +		} else if (*(buffer + i + 1) ==
> > > > +				FPGA_SOCFPGA_A10_RBF_ENCRYPTED
> > > > ) {
> > > > +			rbf->security = encrypted;
> > > > +		} else {
> > > > +			rbf->security = invalid;
> > > > +			continue;
> > > > +		}
> > > > +
> > > > +		/* PERIPH RBF(buffer + i + 1), CORE RBF(buffer
> > > > + i
> > > > + 2) */
> > > > +		if (*(buffer + i + 1) ==
> > > > FPGA_SOCFPGA_A10_RBF_PERIPH) {
> > > > +			rbf->section = periph_section;
> > > > +			break;
> > > > +		} else if (*(buffer + i + 1) ==
> > > > FPGA_SOCFPGA_A10_RBF_CORE) {
> > > > +			rbf->section = core_section;
> > > > +			break;
> > > > +		} else if (*(buffer + i + 2) ==
> > > > FPGA_SOCFPGA_A10_RBF_PERIPH) {
> > > > +			rbf->section = periph_section;
> > > > +			break;
> > > > +		} else if (*(buffer + i + 2) ==
> > > > FPGA_SOCFPGA_A10_RBF_CORE) {
> > > > +			rbf->section = core_section;
> > > > +			break;
> > > > +		}
> > > > +
> > > > +		rbf->section = unknown;
> > > > +		break;
> > > > +
> > > > +		WATCHDOG_RESET();
> > > > +	}
> > > > +}
> > > > +
> > > > +#ifdef CONFIG_FS_LOADER
> > > > +static int first_loading_rbf_to_buffer(struct udevice *dev,
> > > > +				struct fpga_loadfs_info
> > > > *fpga_loadfs,
> > > > +				u32 *buffer, size_t
> > > > *buffer_bsize)
> > > > +{
> > > > +	u32 *buffer_p = (u32 *)*buffer;
> > > > +	u32 *loadable = buffer_p;
> > > > +	size_t buffer_size = *buffer_bsize;
> > > > +	size_t fit_size;
> > > > +	int ret, i, count;
> > > > +	int confs_noffset, images_noffset;
> > > > +	int rbf_offset;
> > > > +	int rbf_size;
> > > put them on the same line.
> > sure.
> > > 
> > > 
> > > > 
> > > > 
> > > > +	const char *fpga_node_name = NULL;
> > > > +	const char *uname = NULL;
> > > > +
> > > > +	/* Load image header into buffer */
> > > > +	ret = request_firmware_into_buf(dev,
> > > > +					fpga_loadfs-
> > > > >fpga_fsinfo-
> > > > > 
> > > > > filename,
> > > > +					buffer_p,
> > > > +					sizeof(struct
> > > > image_header),
> > > > +					0);
> > > > +	if (ret < 0) {
> > > > +		debug("FPGA: Failed to read image header from
> > > > flash.\n");
> > > > +		return -ENOENT;
> > > > +	}
> > > > +
> > > > +	if (image_get_magic((struct image_header *)buffer_p)
> > > > !=
> > > > FDT_MAGIC) {
> > > > +		debug("FPGA: No FDT magic was found.\n");
> > > > +		return -EBADF;
> > > > +	}
> > > > +
> > > > +	fit_size = fdt_totalsize(buffer_p);
> > > > +
> > > > +	if (fit_size > buffer_size) {
> > > > +		debug("FPGA: FIT image is larger than
> > > > available
> > > > buffer.\n");
> > > > +		debug("Please use FIT external data or
> > > > increasing
> > > > buffer.\n");
> > > > +		return -ENOMEM;
> > > > +	}
> > > > +
> > > > +	/* Load entire FIT into buffer */
> > > > +	ret = request_firmware_into_buf(dev,
> > > > +					fpga_loadfs-
> > > > >fpga_fsinfo-
> > > > > 
> > > > > filename,
> > > > +					buffer_p,
> > > > +					fit_size,
> > > > +					0);
> > > nit: better  buffer_p, fit_size, 0);
> > sure.
> > > 
> > > 
> > > 
> > > > 
> > > > 
> > > > +
> > > nit: remove empty line above
> > sure.
> > > 
> > > 
> > > > 
> > > > 
> > > > +	if (ret < 0)
> > > > +		return ret;
> > > > +
> > > > +	ret = fit_check_format(buffer_p);
> > > > +	if (!ret) {
> > > > +		debug("FPGA: No valid FIT image was
> > > > found.\n");
> > > > +		return -EBADF;
> > > > +	}
> > > > +
> > > > +	confs_noffset = fdt_path_offset(buffer_p,
> > > > FIT_CONFS_PATH);
> > > > +	images_noffset = fdt_path_offset(buffer_p,
> > > > FIT_IMAGES_PATH);
> > > > +	if (confs_noffset < 0 || images_noffset < 0) {
> > > > +		debug("FPGA: No Configurations or images nodes
> > > > were found.\n");
> > > > +		return -ENOENT;
> > > > +	}
> > > > +
> > > > +	/* Get default configuration unit name from default
> > > > property */
> > > > +	confs_noffset = fit_conf_get_node(buffer_p, NULL);
> > > > +	if (confs_noffset < 0) {
> > > > +		debug("FPGA: No default configuration was
> > > > found in
> > > > config.\n");
> > > > +		return -ENOENT;
> > > > +	}
> > > > +
> > > > +	count = fit_conf_get_prop_node_count(buffer_p,
> > > > confs_noffset,
> > > > +					    FIT_FPGA_PROP);
> > > > +
> > > nit: remove empty line.
> > sure.
> > > 
> > > 
> > > > 
> > > > 
> > > > +	if (count < 0) {
> > > > +		debug("FPGA: Invalid configuration format for
> > > > FPGA
> > > > node.\n");
> > > > +		return count;
> > > > +	}
> > > > +	debug("FPGA: FPGA node count: %d\n", count);
> > > > +
> > > > +	for (i = 0; i < count; i++) {
> > > > +		images_noffset =
> > > > fit_conf_get_prop_node_index(buffer_p,
> > > > +							     c
> > > > onfs
> > > > _noffset,
> > > > +							     F
> > > > IT_F
> > > > PGA_PROP, i);
> > > > +		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
> > > > _mod
> > > > e() &&
> > > > +					!is_fpgamgr_user_mode(
> > > > )))
> > > > {
> > > > +				fpga_node_name = uname;
> > > > +				printf("FPGA: Start to program
> > > > core ");
> > > > +				printf("bitstream ...\n");
> > > > +				break;
> > > > +			}
> > > > +		}
> > > > +		WATCHDOG_RESET();
> > > > +	}
> > > > +
> > > > +	if (!fpga_node_name) {
> > > > +		debug("FPGA: No suitable bitstream was found,
> > > > count: %d.\n", i);
> > > > +		return 1;
> > > > +	}
> > > > +
> > > > +	images_noffset = fit_image_get_node(buffer_p,
> > > > fpga_node_name);
> > > > +	if (images_noffset < 0) {
> > > > +		debug("FPGA: No node '%s' was found in
> > > > FIT.\n",
> > > > +		     fpga_node_name);
> > > > +		return -ENOENT;
> > > > +	}
> > > > +
> > > > +	if (!fit_image_get_data_position(buffer_p,
> > > > images_noffset,
> > > > +					&rbf_offset)) {
> > > > +		debug("FPGA: Data position was found.\n");
> > > > +	} else if (!fit_image_get_data_offset(buffer_p,
> > > > images_noffset,
> > > > +		  &rbf_offset)) {
> > > > +		/*
> > > > +		 * For FIT with external data, figure out
> > > > where
> > > > +		 * the external images start. This is the base
> > > > +		 * for the data-offset properties in each
> > > > image.
> > > > +		 */
> > > > +		rbf_offset += ((fdt_totalsize(buffer_p) + 3) &
> > > > ~3);
> > > > +		debug("FPGA: Data offset was found.\n");
> > > > +	} else {
> > > > +		debug("FPGA: No data position/offset was
> > > > found.\n");
> > > > +		return -ENOENT;
> > > > +	}
> > > > +
> > > > +	ret = fit_image_get_data_size(buffer_p,
> > > > images_noffset,
> > > > &rbf_size);
> > > > +	if (ret < 0) {
> > > > +		debug("FPGA: No data size was found
> > > > (err=%d).\n",
> > > > ret);
> > > > +		return -ENOENT;
> > > > +	}
> > > > +
> > > > +	if (gd->ram_size < rbf_size) {
> > > > +		debug("FPGA: Using default OCRAM buffer and
> > > > size.\n");
> > > > +	} else {
> > > > +		ret = fit_image_get_load(buffer_p,
> > > > images_noffset,
> > > > +					(ulong *)loadable);
> > > > +		if (ret < 0) {
> > > > +			buffer_p = (u32
> > > > *)DEFAULT_DDR_LOAD_ADDRESS;
> > > > +			debug("FPGA: No loadable was
> > > > found.\n");
> > > > +			debug("FPGA: Using default DDR load
> > > > address: 0x%x .\n",
> > > > +			     DEFAULT_DDR_LOAD_ADDRESS);
> > > > +		} else {
> > > > +			buffer_p = (u32 *)*loadable;
> > > > +			debug("FPGA: Found loadable address =
> > > > 0x%x.\n",
> > > > +			     *loadable);
> > > > +		}
> > > > +
> > > > +		buffer_size = rbf_size;
> > > > +	}
> > > > +
> > > > +	debug("FPGA: External data: offset = 0x%x, size =
> > > > 0x%x.\n",
> > > > +	      rbf_offset, rbf_size);
> > > > +
> > > > +	fpga_loadfs->remaining = rbf_size;
> > > > +
> > > > +	/*
> > > > +	 * Determine buffer size vs bitstream size, and
> > > > calculating number of
> > > > +	 * chunk by chunk transfer is required due to smaller
> > > > buffer size
> > > > +	 * compare to bitstream
> > > > +	 */
> > > > +	if (rbf_size <= buffer_size) {
> > > > +		/* Loading whole bitstream into buffer */
> > > > +		buffer_size = rbf_size;
> > > > +		fpga_loadfs->remaining = 0;
> > > > +	} else {
> > > > +		fpga_loadfs->remaining -= buffer_size;
> > > > +	}
> > > > +
> > > > +	fpga_loadfs->offset = rbf_offset;
> > > > +	/* Loading bitstream into buffer */
> > > > +	ret = request_firmware_into_buf(dev,
> > > > +					fpga_loadfs-
> > > > >fpga_fsinfo-
> > > > > 
> > > > > filename,
> > > > +					buffer_p,
> > > > +					buffer_size,
> > > > +					fpga_loadfs->offset);
> > > > +	if (ret < 0) {
> > > > +		debug("FPGA: Failed to read bitstream from
> > > > flash.\n");
> > > > +		return -ENOENT;
> > > > +	}
> > > > +
> > > > +	/* Getting info about bitstream types */
> > > > +	get_rbf_image_info(&fpga_loadfs->rbfinfo, (u16
> > > > *)buffer_p);
> > > > +
> > > > +	/* Update next reading bitstream offset */
> > > > +	fpga_loadfs->offset += buffer_size;
> > > > +
> > > > +	/* Update the final addr for bitstream */
> > > > +	*buffer = (u32)buffer_p;
> > > > +
> > > > +	/* Update the size of bitstream to be programmed into
> > > > FPGA
> > > > */
> > > > +	*buffer_bsize = buffer_size;
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static int subsequent_loading_rbf_to_buffer(struct udevice
> > > > *dev,
> > > > +					struct
> > > > fpga_loadfs_info
> > > > *fpga_loadfs,
> > > > +					u32 *buffer, size_t
> > > > *buffer_bsize)
> > > > +{
> > > > +	int ret = 0;
> > > > +	u32 *buffer_p = (u32 *)*buffer;
> > > > +
> > > > +	/* Read the bitstream chunk by chunk. */
> > > > +	if (fpga_loadfs->remaining > *buffer_bsize) {
> > > > +		fpga_loadfs->remaining -= *buffer_bsize;
> > > > +	} else {
> > > > +		*buffer_bsize = fpga_loadfs->remaining;
> > > > +		fpga_loadfs->remaining = 0;
> > > > +	}
> > > > +
> > > > +	ret = request_firmware_into_buf(dev,
> > > > +					fpga_loadfs-
> > > > >fpga_fsinfo-
> > > > > 
> > > > > filename,
> > > > +					buffer_p,
> > > > +					*buffer_bsize,
> > > > +					fpga_loadfs->offset);
> > > > +	if (ret < 0) {
> > > > +		debug("FPGA: Failed to read bitstream from
> > > > flash.\n");
> > > > +		return -ENOENT;
> > > > +	}
> > > > +
> > > > +	/* Update next reading bitstream offset */
> > > > +	fpga_loadfs->offset += *buffer_bsize;
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +int socfpga_loadfs(fpga_fs_info *fpga_fsinfo, const void *buf,
> > > > size_t bsize,
> > > > +			u32 offset)
> > > > +{
> > > > +	struct fpga_loadfs_info fpga_loadfs;
> > > > +	int status = 0;
> > > > +	int ret = 0;
> > > no reason to initiate here.
> > sure.
> > > 
> > > 
> > > > 
> > > > 
> > > > +	u32 buffer = (uintptr_t)buf;
> > > > +	size_t buffer_sizebytes = bsize;
> > > > +	size_t buffer_sizebytes_ori = bsize;
> > > > +	size_t total_sizeof_image = 0;
> > > > +	struct udevice *dev;
> > > > +	ofnode node;
> > > > +	int size;
> > > another int - just put them on the same line.
> > sure.
> > > 
> > > 
> > > > 
> > > > 
> > > > +	const fdt32_t *phandle_p;
> > > > +	u32 phandle;
> > > > +
> > > > +	node = get_fpga_mgr_ofnode();
> > > > +
> > > > +	if (ofnode_valid(node)) {
> > > > +		phandle_p = ofnode_get_property(node,
> > > > "firmware-
> > > > loader", &size);
> > > > +		if (phandle_p) {
> > > > +			node = ofnode_path("/chosen");
> > > > +			if (!ofnode_valid(node)) {
> > > > +				debug("FPGA: /chosen node was
> > > > not
> > > > found.\n");
> > > > +				return -ENOENT;
> > > > +			}
> > > > +
> > > > +			phandle_p = ofnode_get_property(node,
> > > > "firmware-loader",
> > > > +						       &size);
> > > > +			if (!phandle_p) {
> > > > +				debug("FPGA: firmware-loader
> > > > property was not");
> > > > +				debug(" found.\n");
> > > > +				return -ENOENT;
> > > > +			}
> > > > +		}
> > > > +	} else {
> > > > +		debug("FPGA: FPGA manager node was not
> > > > found.\n");
> > > > +		return -ENOENT;
> > > > +	}
> > > > +
> > > > +	phandle = fdt32_to_cpu(*phandle_p);
> > > > +	ret =
> > > > uclass_get_device_by_phandle_id(UCLASS_FS_FIRMWARE_LOADER,
> > > > +					     phandle, &dev);
> > > > +	if (ret)
> > > > +		return ret;
> > > > +
> > > > +	memset(&fpga_loadfs, 0, sizeof(fpga_loadfs));
> > > > +
> > > > +	fpga_loadfs.fpga_fsinfo = fpga_fsinfo;
> > > > +	fpga_loadfs.offset = offset;
> > > > +
> > > > +	printf("FPGA: Checking FPGA configuration setting
> > > > ...\n");
> > > > +
> > > > +	/*
> > > > +	 * Note: Both buffer and buffer_sizebytes values can
> > > > be
> > > > altered by
> > > > +	 * function below.
> > > > +	 */
> > > > +	ret = first_loading_rbf_to_buffer(dev, &fpga_loadfs,
> > > > &buffer,
> > > > +					   &buffer_sizebytes);
> > > > +	if (ret == 1) {
> > > > +		printf("FPGA: Skipping configuration ...\n");
> > > > +		return 0;
> > > > +	} else if (ret) {
> > > > +		return ret;
> > > > +	}
> > > > +
> > > > +	if (fpga_loadfs.rbfinfo.section == core_section &&
> > > > +		!(is_fpgamgr_early_user_mode() &&
> > > > !is_fpgamgr_user_mode())) {
> > > > +		debug("FPGA : Must be in Early Release mode to
> > > > program ");
> > > > +		debug("core bitstream.\n");
> > > > +		return 0;
> > > This doesn't look like pass. 0 means pass but it should fail.
> > This is for supporting our specific use case.
> > > 
> > > 
> > > > 
> > > > 
> > > > +	}
> > > > +
> > > > +	/* Disable all signals from HPS peripheral controller
> > > > to
> > > > FPGA */
> > > > +	writel(0, &system_manager_base->fpgaintf_en_global);
> > > > +
> > > > +	/* Disable all axi bridges (hps2fpga, lwhps2fpga &
> > > > fpga2hps) */
> > > > +	socfpga_bridges_reset();
> > > > +
> > > > +	if (fpga_loadfs.rbfinfo.section == periph_section) {
> > > > +		/* Initialize the FPGA Manager */
> > > > +		status = fpgamgr_program_init((u32 *)buffer,
> > > > buffer_sizebytes);
> > > > +		if (status) {
> > > > +			debug("FPGA: Init with peripheral
> > > > bitstream failed.\n");
> > > > +			return -EPERM;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	/* Transfer bitstream to FPGA Manager */
> > > > +	fpgamgr_program_write((void *)buffer,
> > > > buffer_sizebytes);
> > > > +
> > > > +	total_sizeof_image += buffer_sizebytes;
> > > > +
> > > > +	while (fpga_loadfs.remaining) {
> > > > +		ret = subsequent_loading_rbf_to_buffer(dev,
> > > > +							&fpga_
> > > > load
> > > > fs,
> > > > +							&buffe
> > > > r,
> > > > +							&buffe
> > > > r_si
> > > > zebytes_ori);
> > > > +
> > > > +		if (ret)
> > > > +			return ret;
> > > > +
> > > > +		/* Transfer data to FPGA Manager */
> > > > +		fpgamgr_program_write((void *)buffer,
> > > > +					buffer_sizebytes_ori);
> > > > +
> > > > +		total_sizeof_image += buffer_sizebytes_ori;
> > > > +
> > > > +		WATCHDOG_RESET();
> > > > +	}
> > > > +
> > > > +	if (fpga_loadfs.rbfinfo.section == periph_section) {
> > > > +		if (fpgamgr_wait_early_user_mode() !=
> > > > -ETIMEDOUT)
> > > > {
> > > > +			config_pins(gd->fdt_blob, "shared");
> > > > +			puts("FPGA: Early Release
> > > > Succeeded.\n");
> > > > +		} else {
> > > > +			debug("FPGA: Failed to see Early
> > > > Release.\n");
> > > > +			return -EIO;
> > > > +		}
> > > > +
> > > > +		/* For monolithic bitstream */
> > > > +		if (is_fpgamgr_user_mode()) {
> > > > +			/* Ensure the FPGA entering config
> > > > done */
> > > > +			status = fpgamgr_program_finish();
> > > > +			if (status)
> > > > +				return status;
> > > > +
> > > > +			config_pins(gd->fdt_blob, "fpga");
> > > > +			puts("FPGA: Enter user mode.\n");
> > > > +		}
> > > > +	} else if (fpga_loadfs.rbfinfo.section ==
> > > > core_section) {
> > > > +		/* Ensure the FPGA entering config done */
> > > > +		status = fpgamgr_program_finish();
> > > > +		if (status)
> > > > +			return status;
> > > > +
> > > > +		config_pins(gd->fdt_blob, "fpga");
> > > > +		puts("FPGA: Enter user mode.\n");
> > > > +	} else {
> > > > +		debug("FPGA: Config Error: Unsupported
> > > > bitstream
> > > > type.\n");
> > > > +		return -ENOEXEC;
> > > > +	}
> > > > +
> > > > +	return (int)total_sizeof_image;
> > > > +}
> > > > +
> > > > +void fpgamgr_program(const void *buf, size_t bsize, u32
> > > > offset)
> > > > +{
> > > > +	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, bsize,
> > > > offset);
> > > > +}
> > > > +#endif
> > > > +
> > > > +/* This function is used to load the core bitstream from the
> > > > OCRAM. */
> > > >  int socfpga_load(Altera_desc *desc, const void *rbf_data,
> > > > size_t
> > > > rbf_size)
> > > >  {
> > > > -	int status;
> > > > +	unsigned long status;
> > > > +	struct rbf_info rbfinfo;
> > > > +
> > > > +	memset(&rbfinfo, 0, sizeof(rbfinfo));
> > > >  
> > > > -	/* disable all signals from hps peripheral controller
> > > > to
> > > > fpga */
> > > > +	/* Disable all signals from hps peripheral controller
> > > > to
> > > > fpga */
> > > >  	writel(0, &system_manager_base->fpgaintf_en_global);
> > > >  
> > > > -	/* disable all axi bridge (hps2fpga, lwhps2fpga &
> > > > fpga2hps) */
> > > > +	/* Disable all axi bridge (hps2fpga, lwhps2fpga &
> > > > fpga2hps) */
> > > separate changes.
> > > 
> > > > 
> > > > 
> > > >  	socfpga_bridges_reset();
> > > >  
> > > > -	/* Initialize the FPGA Manager */
> > > > -	status = fpgamgr_program_init((u32 *)rbf_data,
> > > > rbf_size);
> > > > -	if (status)
> > > > -		return status;
> > > > +	/* Getting info about bitstream types */
> > > > +	get_rbf_image_info(&rbfinfo, (u16 *)rbf_data);
> > > >  
> > > > -	/* Write the RBF data to FPGA Manager */
> > > > +	if (rbfinfo.section == periph_section) {
> > > > +		/* Initialize the FPGA Manager */
> > > > +		status = fpgamgr_program_init((u32 *)rbf_data,
> > > > rbf_size);
> > > > +		if (status)
> > > > +			return status;
> > > > +	}
> > > > +
> > > > +	if (rbfinfo.section == core_section &&
> > > > +		!(is_fpgamgr_early_user_mode() &&
> > > > !is_fpgamgr_user_mode())) {
> > > > +		debug("FPGA : Must be in early release mode to
> > > > program ");
> > > > +		debug("core bitstream.\n");
> > > > +		return 0;
> > > 0 is supposed to be pass. This looks like a fail.
> > This is for supporting our specific use case.
> Still if you call this function what you want to load/program
> something
> and you are not able to do it, it should return reasonable return
> value.
> I would say error value.
> Maybe you just need to improve that debug message to look more
> sensible.
> 
> M
> 
> 


More information about the U-Boot mailing list