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