Re: [U-Boot] [PATCH 2/9] ARM: socfpga: Add FPGA drivers for Arria 10 FPGA bitstream loading

On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote:

From: Tien Fong Chee tien.fong.chee@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@intel.com

[...]

@@ -51,6 +54,8 @@ #define ALT_FPGAMGR_IMGCFG_CTL_02_CFGWIDTH_SET_MSK BIT(24) #define ALT_FPGAMGR_IMGCFG_CTL_02_CDRATIO_LSB 16

+#define PERIPH_RBF 0 +#define CORE_RBF 1

Enum, use something with specific prefix.

#ifndef __ASSEMBLY__

struct socfpga_fpga_manager { @@ -88,12 +93,33 @@ struct socfpga_fpga_manager { u32 imgcfg_fifo_status; };

+enum rbf_type {unknown, periph_section, core_section}; +enum rbf_security {invalid, unencrypted, encrypted};

enum should use one line per entry.

+struct rbf_info {

• enum rbf_type section;
• enum rbf_security security;

+};

+struct fpga_loadfs_info {

• fpga_fs_info *fpga_fsinfo;
• u32 remaining;
• u32 offset;
• u32 datacrc;
• struct rbf_info rbfinfo;
• struct image_header header;

+};

/* 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, u32 core); +void get_rbf_image_info(struct rbf_info *rbf, u16 *buffer); +int socfpga_loadfs(fpga_fs_info *fpga_fsinfo, const void *buf, size_t bsize,

• 	u32 offset);
  

#endif /* __ASSEMBLY__ */

#endif /* _FPGA_MANAGER_ARRIA10_H_ */ diff --git a/configs/socfpga_arria10_defconfig b/configs/socfpga_arria10_defconfig index 6ebda81..f88910c 100644 --- a/configs/socfpga_arria10_defconfig +++ b/configs/socfpga_arria10_defconfig @@ -27,8 +27,17 @@ CONFIG_MTDIDS_DEFAULT="nor0=ff705000.spi.0" # CONFIG_EFI_PARTITION is not set CONFIG_DEFAULT_DEVICE_TREE="socfpga_arria10_socdk_sdmmc" CONFIG_ENV_IS_IN_MMC=y +CONFIG_SPL_ENV_SUPPORT=y CONFIG_SPL_DM=y CONFIG_SPL_DM_SEQ_ALIAS=y +CONFIG_SPL_DM_MMC=y +CONFIG_SPL_MMC_SUPPORT=y +CONFIG_SPL_FS_SUPPORT=y +CONFIG_SPL_EXT_SUPPORT=y +CONFIG_SPL_FAT_SUPPORT=y +CONFIG_SPL_DRIVERS_MISC_SUPPORT=y +CONFIG_FS_FAT_MAX_CLUSTSIZE=16384 +CONFIG_FS_LOADER=y

Separate patch

CONFIG_FPGA_SOCFPGA=y CONFIG_DM_GPIO=y CONFIG_DWAPB_GPIO=y diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig index 50e9019..06a8204 100644 --- a/drivers/fpga/Kconfig +++ b/drivers/fpga/Kconfig @@ -21,6 +21,15 @@ config FPGA_SOCFPGA

 This provides common functionality for Gen5 and Arria10 devices.

+config CHECK_FPGA_DATA_CRC

config FPGA_SOCFPGA_A10_CRC_CHECK

What is this for and why shouldn't this be ON by default ?

• bool "Enable CRC cheking on Arria10 FPGA bistream"
• depends on FPGA_SOCFPGA
• help

• Say Y here to enable the CRC checking on Arria 10 FPGA bitstream
  

• This provides CRC checking to ensure integrated of Arria 10 FPGA
  

• bitstream is programmed into FPGA.
  

config FPGA_CYCLON2 bool "Enable Altera FPGA driver for Cyclone II" depends on FPGA_ALTERA diff --git a/drivers/fpga/socfpga_arria10.c b/drivers/fpga/socfpga_arria10.c index 114dd91..d9ad237 100644 --- a/drivers/fpga/socfpga_arria10.c +++ b/drivers/fpga/socfpga_arria10.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 /*

• • Copyright (C) 2017 Intel Corporation <www.intel.com>

• • Copyright (C) 2017-2018 Intel Corporation <www.intel.com>
  */

#include <asm/io.h> @@ -10,8 +10,10 @@ #include <asm/arch/sdram.h> #include <asm/arch/misc.h> #include <altera.h> +#include <asm/arch/pinmux.h> #include <common.h> #include <errno.h> +#include <fs_loader.h> #include <wait_bit.h> #include <watchdog.h>

@@ -21,6 +23,10 @@ #define COMPRESSION_OFFSET 229 #define FPGA_TIMEOUT_MSEC 1000 /* timeout in ms */ #define FPGA_TIMEOUT_CNT 0x1000000 +#define RBF_UNENCRYPTED 0xa65c +#define RBF_ENCRYPTED 0xa65d +#define ARRIA10RBF_PERIPH 0x0001 +#define ARRIA10RBF_CORE 0x8001

This looks awfully similar to the PERIPH_RBF and CORE_RBF above.

static const struct socfpga_fpga_manager *fpga_manager_base = (void *)SOCFPGA_FPGAMGRREGS_ADDRESS; @@ -64,7 +70,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) { return (readl(&fpga_manager_base->imgcfg_stat) & ALT_FPGAMGR_IMGCFG_STAT_F2S_EARLY_USERMODE_SET_MSK) != 0; @@ -447,13 +453,393 @@ int fpgamgr_program_finish(void) return 0; }

+const char *get_fpga_filename(const void *fdt, int *len, u32 core)

static, fix globally .

+{

• const char *fpga_filename = NULL;
• const char *cell;
• int nodeoffset;

ofnode_read_string() , use ofnode globally.

• nodeoffset = fdtdec_next_compatible(fdt, 0,

• 	 COMPAT_ALTERA_SOCFPGA_FPGA0);
  

• if (nodeoffset >= 0) {

• if (core) {
  

• 	cell = fdt_getprop(fdt,
  

• 			nodeoffset,
  

• 			"altr,bitstream_core",
  

• 			len);
  

• } else {
  

• 	cell = fdt_getprop(fdt, nodeoffset,
  

• 			"altr,bitstream_periph", len);
  

• }
  

• if (cell)
  

• 	fpga_filename = cell;
  

• }
• return fpga_filename;

+}

+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) == RBF_UNENCRYPTED) {
  

• 	rbf->security = unencrypted;
  

• } else if (*(buffer + i) == RBF_ENCRYPTED) {
  

• 	rbf->security = encrypted;
  

• } else if (*(buffer + i + 1) == RBF_UNENCRYPTED) {
  

• 	rbf->security = unencrypted;
  

• } else if (*(buffer + i + 1) == RBF_ENCRYPTED) {
  

• 	rbf->security = encrypted;
  

• } else {
  

• 	rbf->security = invalid;
  

• 	continue;
  

• }
  

• /* PERIPH RBF(buffer + i + 1), CORE RBF(buffer + i + 2) */
  

• if (*(buffer + i + 1) == ARRIA10RBF_PERIPH) {
  

• 	rbf->section = periph_section;
  

• 	break;
  

• } else if (*(buffer + i + 1) == ARRIA10RBF_CORE) {
  

• 	rbf->section = core_section;
  

• 	break;
  

• } else if (*(buffer + i + 2) == ARRIA10RBF_PERIPH) {
  

• 	rbf->section = periph_section;
  

• 	break;
  

• } else if (*(buffer + i + 2) == ARRIA10RBF_CORE) {
  

• 	rbf->section = core_section;
  

• 	break;
  

• }
  

• rbf->section = unknown;
  

• break;
  

• WATCHDOG_RESET();
  

• }

+}

+static struct firmware *fw;

What is this static variable doing here ?

+int first_loading_rbf_to_buffer(struct device_platdata *plat,

• 		struct fpga_loadfs_info *fpga_loadfs,
  

• 		u32 *buffer, u32 *buffer_bsize)
  

+{

• u32 *buffer_p_after_header = NULL;
• u32 buffersz_after_header = 0;
• u32 totalsz_header_rbf = 0;
• u32 *buffer_p = (u32 *)*buffer;
• struct image_header *header = &(fpga_loadfs->header);
• size_t buffer_size = *buffer_bsize;
• int ret = 0;
• /* Load mkimage header into buffer */
• ret = request_firmware_into_buf(plat,

• 			fpga_loadfs->fpga_fsinfo->filename,
  

• 			header,
  

• 			sizeof(struct image_header),
  

• 			fpga_loadfs->offset,
  

• 			&fw);
  

• if (ret < 0) {

• debug("FPGA: Failed to read RBF mkimage header from flash.\n");
  

• return -ENOENT;
  

• }
• WATCHDOG_RESET();
• if (!image_check_magic(header)) {

• debug("FPGA: Bad Magic Number.\n");
  

• return -EBADF;
  

• }
• if (!image_check_hcrc(header)) {

• debug("FPGA: Bad Header Checksum.\n");
  

• return -EPERM;
  

• }
• /* Getting RBF data size from mkimage header */
• fpga_loadfs->remaining = image_get_data_size(header);
• /* Calculate total size of both RBF with mkimage header */
• totalsz_header_rbf = fpga_loadfs->remaining +

• 		sizeof(struct image_header);
  

• /*

• * Determine buffer size vs RBF size, and calculating number of
  

• * chunk by chunk transfer is required due to smaller buffer size
  

• * compare to RBF
  

• */
  

• if (totalsz_header_rbf > buffer_size) {

• /* Calculate size of RBF in the buffer */
  

• buffersz_after_header =
  

• 	buffer_size - sizeof(struct image_header);
  

• fpga_loadfs->remaining -= buffersz_after_header;
  

• } else {

• /* Loading whole RBF into buffer */
  

• buffer_size = totalsz_header_rbf;
  

• /* Calculate size of RBF in the buffer */
  

• buffersz_after_header =
  

• 	buffer_size - sizeof(struct image_header);
  

• fpga_loadfs->remaining = 0;
  

• }
• /* Loading mkimage header and RBFinto buffer */
• ret = request_firmware_into_buf(plat,

• 			fpga_loadfs->fpga_fsinfo->filename,
  

• 			buffer_p,
  

• 			buffer_size,
  

• 			fpga_loadfs->offset,
  

• 			&fw);
  

This looks like some unwound loop , since the request_firmware_into_buf() is called twice. This probably works for the 300 kiB core RBF, but doesn't work for the 16 MiB full RBF. Also, for() cycle should be used somehow.

• if (ret < 0) {

• debug("FPGA: Failed to read RBF mkimage header and RBF from ");
  

• debug("flash.\n");
  

• return -ENOENT;
  

• }
• /*

• * Getting pointer of RBF starting address where it's
  

• * right after mkimage header
  

• */
  

• buffer_p_after_header =

• (u32 *)((u_char *)buffer_p + sizeof(struct image_header));
  

• /* Update next reading RBF offset */
• fpga_loadfs->offset += buffer_size;
• /* Getting info about RBF types */
• get_rbf_image_info(&fpga_loadfs->rbfinfo, (u16 *)buffer_p_after_header);
• /*

• * Update the starting addr of RBF to init FPGA & programming RBF
  

• * into FPGA
  

• */
  

• *buffer = (u32)buffer_p_after_header;
• /* Update the size of RBF to be programmed into FPGA */
• *buffer_bsize = buffersz_after_header;

+#ifdef CONFIG_CHECK_FPGA_DATA_CRC

• fpga_loadfs->datacrc = crc32(fpga_loadfs->datacrc,

• 			(u_char *)buffer_p_after_header,
  

• 			buffersz_after_header);
  

Why is this not ON by default ?

+#endif

+if (fpga_loadfs->remaining == 0) { +#ifdef CONFIG_CHECK_FPGA_DATA_CRC

• if (fpga_loadfs->datacrc != image_get_dcrc(&(fpga_loadfs->header))) {

• debug("FPGA: Bad Data Checksum.\n");
  

• return -EPERM;
  

• }

+#endif +}

• return 0;

+}

[...]