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

Michal Simek monstr at monstr.eu
Tue Feb 26 15:46:21 UTC 2019


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?
>>
>>>
>>> +		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,bitstream");
>>> +
>>> +	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_UNENCRYPTED)
>>> {
>>> +			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,
>>> +							     confs
>>> _noffset,
>>> +							     FIT_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,
>>> +							&buffer,
>>> +							&buffer_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


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190226/2dc8f4d1/attachment.sig>


More information about the U-Boot mailing list