[U-Boot] [PATCH 0/9] Add support for loading FPGA bitstream

[U-Boot] [PATCH 0/9] Add support for loading FPGA bitstream

[tien.fong.chee at intel.com]

From: Tien Fong Chee <tien.fong.chee@intel.com>

These series of patches enable peripheral bitstream being programmed into FPGA
to get the DDR up running. This's also called early IO release, because the
peripheral bitstream is only initializing FPGA IOs, PLL, IO48 and DDR.

Once DDR is up running, core bitstream from FIT image which contains user FPGA
design would be loaded into DDR location, where it's defined as 0x1000 by
default in fit_spl_fpga.its. fpga load would be called to program core bitstream
into FPGA and entering user mode.

Lastly, U-Boot from FIT image would be loaded to DDR, and up running from there.

For the whole mechanism to work, the SDMMC flash layout would be designed as
shown in below:

RAW partition:
1. spl_w_dtb-mkpimage.bin
mkpimage -hv 1 -o spl/spl_w_dtb-mkpimage.bin spl/u-boot-spl-dtb.bin
 spl/u-boot-spl-dtb.bin spl/u-boot-spl-dtb.bin spl/u-boot-spl-dtb.bin

FAT partition:
1. u-boot.itb(default build)
	- uboot
	- fdt
	- fpga
2. ghrd_10as066n2.periph.rbf.mkimage
mkimage -A arm -T firmware -C none -O u-boot -a 0 -e 0 -n \"RBF\" -d
 ghrd_10as066n2.periph.rbf ghrd_10as066n2.periph.rbf.mkimage

This series is working on top of u-boot.git -
 http://git.denx.de/u-boot.git .

Tien Fong Chee (9):
  ARM: socfpga: Description on FPGA bitstream type and file name for
    Arria 10
  ARM: socfpga: Add FPGA drivers for Arria 10 FPGA bitstream loading
  spl : socfpga: Implement fpga bitstream loading with socfpga loadfs
  ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10
  ARM: socfpga: Add SPL fitImage config match
  ARM: socfpga: Set default DTB address on A10
  ARM: socfpga: Use custom header target buffer in SPL
  ARM: socfpga: Add default fitImage for Arria10 SoCDK
  ARM: socfpga: Synchronize the configuration for A10 SoCDK

 Makefile                                           |    9 +-
 arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts       |   18 +
 arch/arm/mach-socfpga/board.c                      |    8 +
 .../include/mach/fpga_manager_arria10.h            |   30 ++-
 arch/arm/mach-socfpga/spl_a10.c                    |   70 ++++-
 board/altera/arria10-socdk/fit_spl_fpga.its        |   54 +++
 common/spl/spl_mmc.c                               |    2 +-
 configs/socfpga_arria10_defconfig                  |   51 +++-
 .../fpga/altera-socfpga-a10-fpga-mgr.txt           |    6 +
 drivers/fpga/Kconfig                               |    9 +
 drivers/fpga/socfpga_arria10.c                     |  417 +++++++++++++++++++-
 include/configs/socfpga_common.h                   |    4 +
 include/mmc.h                                      |    1 +
 13 files changed, 657 insertions(+), 22 deletions(-)
 create mode 100644 board/altera/arria10-socdk/fit_spl_fpga.its

-- 
1.7.7.4

[U-Boot] [PATCH 1/9] ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

[tien.fong.chee at intel.com]

From: Tien Fong Chee <tien.fong.chee@intel.com>

This patch adds description on properties about file name used for both
peripheral bitstream and core bitstream.

Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
---
 .../fpga/altera-socfpga-a10-fpga-mgr.txt           |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt b/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt
index 2fd8e7a..010322a 100644
--- a/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt
+++ b/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt
@@ -7,6 +7,10 @@ Required properties:
                - The second index is for writing FPGA configuration data.
 - resets     : Phandle and reset specifier for the device's reset.
 - clocks     : Clocks used by the device.
+- altr,bitstream_periph : File name for FPGA peripheral raw binary which is used
+			  to initialize FPGA IOs, PLL, IO48 and DDR.
+- altr,bitstream_core : File name for core raw binary which contains FPGA design
+			which is used to program FPGA CRAM and ERAM.
 
 Example:
 
@@ -16,4 +20,6 @@ Example:
 		       0xffcfe400 0x20>;
 		clocks = <&l4_mp_clk>;
 		resets = <&rst FPGAMGR_RESET>;
+		altr,bitstream_periph = "ghrd_10as066n2.periph.rbf.mkimage";
+		altr,bitstream_core = "ghrd_10as066n2.core.rbf.mkimage";
 	};
-- 
1.7.7.4

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

[tien.fong.chee at intel.com]

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>
---
 arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts       |   18 +
 .../include/mach/fpga_manager_arria10.h            |   30 ++-
 configs/socfpga_arria10_defconfig                  |    9 +
 drivers/fpga/Kconfig                               |    9 +
 drivers/fpga/socfpga_arria10.c                     |  417 +++++++++++++++++++-
 5 files changed, 471 insertions(+), 12 deletions(-)

diff --git a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
index 998d811..f4764d4 100644
--- a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
+++ b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
@@ -18,6 +18,24 @@
 /dts-v1/;
 #include "socfpga_arria10_socdk.dtsi"
 
+/ {
+	chosen {
+		firmware-loader = &fs_loader0;
+	};
+
+	fs_loader0: fs-loader at 0 {
+		u-boot,dm-pre-reloc;
+		compatible = "u-boot,fs-loader";
+		phandlepart = <&mmc 1>;
+	};
+};
+
+&fpga_mgr {
+	u-boot,dm-pre-reloc;
+	altr,bitstream_periph = "ghrd_10as066n2.periph.rbf.mkimage";
+	altr,bitstream_core = "ghrd_10as066n2.core.rbf.mkimage";
+};
+
 &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..b48bc76 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,12 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 /*
- * Copyright (C) 2017 Intel Corporation <www.intel.com>
+ * Copyright (C) 2017-2018 Intel Corporation <www.intel.com>
  * All rights reserved.
  */
 
+#include <altera.h>
+#include <image.h>
+
 #ifndef _FPGA_MANAGER_ARRIA10_H_
 #define _FPGA_MANAGER_ARRIA10_H_
 
@@ -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
 #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};
+
+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
 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
+	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
 
 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)
+{
+	const char *fpga_filename = NULL;
+	const char *cell;
+	int nodeoffset;
+
+	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;
+
+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);
+	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);
+#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;
+}
+
+int subsequent_loading_rbf_to_buffer(struct device_platdata *plat,
+					struct fpga_loadfs_info *fpga_loadfs,
+					u32 *buffer, u32 *buffer_bsize)
+{
+	int ret = 0;
+	u32 *buffer_p = (u32 *)*buffer;
+
+	/* Read the RBF 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(plat,
+					fpga_loadfs->fpga_fsinfo->filename,
+					buffer_p,
+					*buffer_bsize,
+					fpga_loadfs->offset,
+					&fw);
+	if (ret < 0) {
+		debug("FPGA: Failed to read RBF from flash.\n");
+		return -ENOENT;
+	}
+
+#ifdef CONFIG_CHECK_FPGA_DATA_CRC
+	fpga_loadfs->datacrc = crc32(fpga_loadfs->datacrc,
+					(unsigned char *)buffer_p,
+					*buffer_bsize);
+#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
+}
+
+	/* Update next reading RBF 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;
+	u32 buffer = (u32) buf;
+	u32 buffer_ori = (u32) buf;
+	size_t buffer_sizebytes = bsize;
+	size_t buffer_sizebytes_ori = bsize;
+	size_t total_sizeof_mkimage = sizeof(struct image_header);
+	struct udevice *dev;
+
+	ret = uclass_get_device(UCLASS_FS_FIRMWARE_LOADER, 0, &dev);
+	if (ret)
+		return ret;
+
+	memset(&fpga_loadfs, 0, sizeof(fpga_loadfs));
+
+	WATCHDOG_RESET();
+
+	fpga_loadfs.fpga_fsinfo = fpga_fsinfo;
+	fpga_loadfs.offset = offset;
+
+	printf("Start to program FPGA ...\n");
+
+	/*
+	 * Note: Both buffer and buffer_sizebytes values can be altered by
+	 * function below.
+	 */
+	ret = first_loading_rbf_to_buffer(dev->platdata, &fpga_loadfs, &buffer,
+					   &buffer_sizebytes);
+	if (ret) {
+		release_firmware(fw);
+		fw = NULL;
+		return ret;
+	}
+
+	if ((fpga_loadfs.rbfinfo.section == core_section) &&
+		!(is_fpgamgr_early_user_mode() && !is_fpgamgr_user_mode())) {
+		debug("FPGA : FPGA must be in Early Release mode to program ");
+		debug("core.\n");
+		release_firmware(fw);
+		fw = NULL;
+		return 0;
+	}
+
+	/* 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 periph rbf failed.\n");
+			release_firmware(fw);
+			fw = NULL;
+			return -EPERM;
+		}
+	}
+
+	WATCHDOG_RESET();
+
+	/* Transfer RBF to FPGA Manager */
+	fpgamgr_program_write((void *)buffer, buffer_sizebytes);
+
+	total_sizeof_mkimage += buffer_sizebytes;
+
+	WATCHDOG_RESET();
+
+	while (fpga_loadfs.remaining) {
+		ret = subsequent_loading_rbf_to_buffer(dev->platdata,
+							&fpga_loadfs,
+							&buffer_ori,
+							&buffer_sizebytes_ori);
+
+		if (ret) {
+			release_firmware(fw);
+			fw = NULL;
+			return ret;
+		}
+
+		/* Transfer data to FPGA Manager */
+		fpgamgr_program_write((void *)buffer_ori,
+					buffer_sizebytes_ori);
+
+		total_sizeof_mkimage += 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");
+			release_firmware(fw);
+			fw = NULL;
+			return -EIO;
+		}
+
+		/* For monolithic bitstream */
+		if (is_fpgamgr_user_mode()) {
+			/* Ensure the FPGA entering config done */
+			status = fpgamgr_program_finish();
+			if (status) {
+				release_firmware(fw);
+				fw = NULL;
+				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) {
+			release_firmware(fw);
+			fw = NULL;
+			return status;
+		}
+
+		config_pins(gd->fdt_blob, "fpga");
+		puts("FPGA: Enter user mode.\n");
+	} else {
+		debug("Config Error: Unsupported FGPA RBF type.\n");
+		release_firmware(fw);
+		fw = NULL;
+		return -ENOEXEC;
+	}
+
+	release_firmware(fw);
+	fw = NULL;
+
+	return (int)total_sizeof_mkimage;
+}
+
 /*
- * FPGA Manager to program the FPGA. This is the interface used by FPGA driver.
- * Return 0 for sucess, non-zero for error.
+ * This function is used to load the core RBF from the OCRAM where the location
+ * of the image is defined in the load property in the FIT image source file
+ * (.its)
  */
 int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size)
 {
-	int status;
+	unsigned long status;
+	struct rbf_info rbfinfo;
 
 	/* disable all signals from hps peripheral controller to fpga */
 	writel(0, &system_manager_base->fpgaintf_en_global);
@@ -461,13 +847,24 @@ int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size)
 	/* disable all axi bridge (hps2fpga, lwhps2fpga & fpga2hps) */
 	socfpga_bridges_reset();
 
-	/* Initialize the FPGA Manager */
-	status = fpgamgr_program_init((u32 *)rbf_data, rbf_size);
-	if (status)
-		return status;
+	/* Getting info about RBF types */
+	get_rbf_image_info(&rbfinfo, (u16 *)rbf_data);
+
+	if (rbfinfo.section == periph_section) {
+		/* Initialize the FPGA Manager */
+		status = fpgamgr_program_init((u32 *)rbf_data, rbf_size);
+		if (status)
+			return status;
+	}
 
 	/* Write the RBF data to FPGA Manager */
 	fpgamgr_program_write(rbf_data, rbf_size);
 
-	return fpgamgr_program_finish();
+	status = fpgamgr_program_finish();
+	if (status) {
+		config_pins(gd->fdt_blob, "fpga");
+		puts("FPGA: Enter user mode.\n");
+	}
+
+	return status;
 }
-- 
1.7.7.4

[U-Boot] [PATCH 3/9] spl : socfpga: Implement fpga bitstream loading with socfpga loadfs

[tien.fong.chee at intel.com]

From: Tien Fong Chee <tien.fong.chee@intel.com>

Add support for loading FPGA bitstream to get DDR up running before
U-Boot is loaded into DDR. Boot device initialization, generic firmware
loader and SPL FAT support are required for this whole mechanism to work.

Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
---
 arch/arm/mach-socfpga/spl_a10.c |   49 ++++++++++++++++++++++++++++++++++++++-
 common/spl/spl_mmc.c            |    2 +-
 include/mmc.h                   |    1 +
 3 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-socfpga/spl_a10.c
index 3ea64f7..67a4fac 100644
--- a/arch/arm/mach-socfpga/spl_a10.c
+++ b/arch/arm/mach-socfpga/spl_a10.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- *  Copyright (C) 2012 Altera Corporation <www.altera.com>
+ *  Copyright (C) 2012-2018 Altera Corporation <www.altera.com>
  */
 
 #include <common.h>
@@ -23,6 +23,10 @@
 #include <fdtdec.h>
 #include <watchdog.h>
 #include <asm/arch/pinmux.h>
+#include <asm/arch/fpga_manager.h>
+#include <mmc.h>
+
+#define RBF	0
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -73,6 +77,49 @@ void spl_board_init(void)
 	WATCHDOG_RESET();
 
 	arch_early_init_r();
+
+	/* If the full FPGA is already loaded, ie.from EPCQ, config fpga pins */
+	if (is_fpgamgr_user_mode()) {
+		config_pins(gd->fdt_blob, "shared");
+		config_pins(gd->fdt_blob, "fpga");
+	} else if (!is_fpgamgr_early_user_mode()) {
+		/* Program IOSSM(early IO release) or full FPGA */
+		fpga_fs_info fpga_fsinfo;
+		char buf[16 * 1024] __aligned(ARCH_DMA_MINALIGN);
+		struct spl_boot_device bootdev;
+		int len = 0;
+
+		bootdev.boot_device = spl_boot_device();
+
+		/* Init MMC driver before reading FPGA bitstream from flash */
+		if (bootdev.boot_device == BOOT_DEVICE_MMC1) {
+			struct mmc *mmc = NULL;
+			int err = 0;
+
+			err = spl_mmc_find_device(&mmc, bootdev.boot_device);
+			if (err)
+				return;
+
+			err = mmc_init(mmc);
+
+			if (err) {
+				debug("spl: mmc init failed with error: %d\n",
+					err);
+
+				return;
+			}
+		}
+
+		fpga_fsinfo.filename = (char *)get_fpga_filename(gd->fdt_blob,
+								 &len,
+								 RBF);
+
+		socfpga_loadfs(&fpga_fsinfo, buf, sizeof(buf), 0);
+	}
+
+	/* If the IOSSM/full FPGA is already loaded, start DDR */
+	if (is_fpgamgr_early_user_mode() || is_fpgamgr_user_mode())
+		ddr_calibration_sequence();
 }
 
 void board_init_f(ulong dummy)
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index 4d55dcc..b85e146 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -111,7 +111,7 @@ static int spl_mmc_get_device_index(u32 boot_device)
 	return -ENODEV;
 }
 
-static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device)
+int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device)
 {
 #if CONFIG_IS_ENABLED(DM_MMC)
 	struct udevice *dev;
diff --git a/include/mmc.h b/include/mmc.h
index 95548e9..de92909 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -829,6 +829,7 @@ int board_mmc_init(bd_t *bis);
 int cpu_mmc_init(bd_t *bis);
 int mmc_get_env_addr(struct mmc *mmc, int copy, u32 *env_addr);
 int mmc_get_env_dev(void);
+int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device);
 
 /* Set block count limit because of 16 bit register limit on some hardware*/
 #ifndef CONFIG_SYS_MMC_MAX_BLK_COUNT
-- 
1.7.7.4

[U-Boot] [PATCH 4/9] ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10

[tien.fong.chee at intel.com]

From: Tien Fong Chee <tien.fong.chee@intel.com>

Bundle U-Boot fitImage containing U-Boot and FPGA bitstream into the
u-boot-with-spl.sfp on Arria10. This lets U-Boot operate in a very
similar fashion to Gen5, where the U-Boot binary got loaded by the
SPL from a uImage concatenated at the end of the SPL SFP image. On
Gen10, the U-Boot is in fitImage which contains the FPGA bitstream
as well. In this case, the SPL can load the FPGA bitstream first and
load the U-Boot afterward in the same manner. This is nonetheless a
stopgap measure until there is a proper firmware loader in U-Boot.

Signed-off-by: Marek Vasut <marex@denx.de>
Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
---
 Makefile                         |    9 +++++++--
 include/configs/socfpga_common.h |    4 ++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index a55915d..4ecc19d 100644
--- a/Makefile
+++ b/Makefile
@@ -1212,9 +1212,14 @@ ifneq ($(CONFIG_ARCH_SOCFPGA),)
 quiet_cmd_socboot = SOCBOOT $@
 cmd_socboot = cat	spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
 			spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
-			u-boot.img > $@ || rm -f $@
+			$2 > $@ || rm -f $@
+ifdef CONFIG_TARGET_SOCFPGA_ARRIA10
+u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-boot.itb FORCE
+	$(call if_changed,socboot,u-boot.itb)
+else
 u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
-	$(call if_changed,socboot)
+	$(call if_changed,socboot,u-boot.img)
+endif
 endif
 
 ifeq ($(CONFIG_MPC85xx)$(CONFIG_OF_SEPARATE),yy)
diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h
index bd8f5c8..ffdc6eb 100644
--- a/include/configs/socfpga_common.h
+++ b/include/configs/socfpga_common.h
@@ -268,7 +268,11 @@ unsigned int cm_get_qspi_controller_clk_hz(void);
 /* SPL SDMMC boot support */
 #ifdef CONFIG_SPL_MMC_SUPPORT
 #if defined(CONFIG_SPL_FAT_SUPPORT) || defined(CONFIG_SPL_EXT_SUPPORT)
+#if CONFIG_SPL_FIT
+#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME		"u-boot.itb"
+#else
 #define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME		"u-boot-dtb.img"
+#endif
 #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION	1
 #endif
 #else
-- 
1.7.7.4

[U-Boot] [PATCH 5/9] ARM: socfpga: Add SPL fitImage config match

[tien.fong.chee at intel.com]

From: Tien Fong Chee <tien.fong.chee@intel.com>

Add empty SPL fitImage configuration match. This can be extended
if there is ever need to support multiple boards with single SFP
image.

Signed-off-by: Marek Vasut <marex@denx.de>
---
 arch/arm/mach-socfpga/spl_a10.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-socfpga/spl_a10.c
index 67a4fac..2baeba6 100644
--- a/arch/arm/mach-socfpga/spl_a10.c
+++ b/arch/arm/mach-socfpga/spl_a10.c
@@ -148,3 +148,13 @@ void board_init_f(ulong dummy)
 	config_dedicated_pins(gd->fdt_blob);
 	WATCHDOG_RESET();
 }
+
+#ifdef CONFIG_SPL_LOAD_FIT
+int board_fit_config_name_match(const char *name)
+{
+	/* Just empty function now - can't decide what to choose */
+	debug("%s: %s\n", __func__, name);
+
+	return 0;
+}
+#endif
-- 
1.7.7.4

[U-Boot] [PATCH 6/9] ARM: socfpga: Set default DTB address on A10

[tien.fong.chee at intel.com]

From: Tien Fong Chee <tien.fong.chee@intel.com>

Set default DT blob address on A10 SoCDK, since this SoC uses OF
separate configuration. The 0xf0000 address is just below the text
base and still leaves enough room for the DT to grow.

Signed-off-by: Marek Vasut <marex@denx.de>
Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
---
 arch/arm/mach-socfpga/board.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-socfpga/board.c b/arch/arm/mach-socfpga/board.c
index 7c8c05c..436a8a8 100644
--- a/arch/arm/mach-socfpga/board.c
+++ b/arch/arm/mach-socfpga/board.c
@@ -86,3 +86,11 @@ int g_dnl_board_usb_cable_connected(void)
 	return 1;
 }
 #endif
+
+#if defined(CONFIG_TARGET_SOCFPGA_ARRIA10) && \
+!defined(CONFIG_SPL_BUILD) && defined(CONFIG_OF_SEPARATE)
+void *board_fdt_blob_setup(void)
+{
+	return (void *)0xf00000;
+}
+#endif
-- 
1.7.7.4

[U-Boot] [PATCH 7/9] ARM: socfpga: Use custom header target buffer in SPL

[tien.fong.chee at intel.com]

From: Tien Fong Chee <tien.fong.chee@intel.com>

Allocate buffers from OCRAM heap for the image headers in SPL on
Arria10, since DRAM is not available at that point. This allows
U-Boot to load the fitImage header, parse it, extract the FPGA
bitstream section from it, program the FPGA and make DRAM available.

Signed-off-by: Marek Vasut <marex@denx.de>
---
 arch/arm/mach-socfpga/spl_a10.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-socfpga/spl_a10.c
index 2baeba6..7fe9790 100644
--- a/arch/arm/mach-socfpga/spl_a10.c
+++ b/arch/arm/mach-socfpga/spl_a10.c
@@ -9,6 +9,7 @@
 #include <asm/u-boot.h>
 #include <asm/utils.h>
 #include <image.h>
+#include <malloc.h>
 #include <asm/arch/reset_manager.h>
 #include <spl.h>
 #include <asm/arch/system_manager.h>
@@ -157,4 +158,14 @@ int board_fit_config_name_match(const char *name)
 
 	return 0;
 }
+
+struct image_header *spl_get_load_buffer(int offset, size_t size)
+{
+	struct image_header *mem = memalign(4, size);
+
+	if (!mem)
+		hang();
+
+	return mem;
+}
 #endif
-- 
1.7.7.4

[U-Boot] [PATCH 8/9] ARM: socfpga: Add default fitImage for Arria10 SoCDK

[tien.fong.chee at intel.com]

From: Tien Fong Chee <tien.fong.chee@intel.com>

Add default fitImage file bundling U-Boot and FPGA bitstream for Arria10.

Signed-off-by: Marek Vasut <marex@denx.de>
Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
---
 board/altera/arria10-socdk/fit_spl_fpga.its |   54 +++++++++++++++++++++++++++
 1 files changed, 54 insertions(+), 0 deletions(-)
 create mode 100644 board/altera/arria10-socdk/fit_spl_fpga.its

diff --git a/board/altera/arria10-socdk/fit_spl_fpga.its b/board/altera/arria10-socdk/fit_spl_fpga.its
new file mode 100644
index 0000000..7782740
--- /dev/null
+++ b/board/altera/arria10-socdk/fit_spl_fpga.its
@@ -0,0 +1,54 @@
+/*
+ * The fitImage source for Arria 10 SoCDK loading FPGA bitstream and U-Boot
+ *
+ * Copyright (C) 2018 Marek Vasut <marex@denx.de>
+ *
+ * SPDX-License-Identifier: GPL-2.0+  X11
+ */
+
+/dts-v1/;
+
+/ {
+	description = "FIT image with U-Boot proper and FPGA bitstream";
+	#address-cells = <1>;
+
+	images {
+		uboot {
+			description = "U-Boot (32-bit)";
+			data = /incbin/("../../../u-boot-nodtb.bin");
+			type = "standalone";
+			os = "U-Boot";
+			arch = "arm";
+			compression = "none";
+			load = <0x01000040>;
+			entry = <0x01000040>;
+		};
+
+		fdt {
+			description = "Arria10 SoCDK flat device-tree";
+			data = /incbin/("../../../u-boot.dtb");
+			type = "flat_dt";
+			arch = "arm";
+			compression = "none";
+			load = <0x00f00000>;
+		};
+
+		fpga {
+		        description = "FPGA bitstream";
+			data = /incbin/("../../../ghrd_10as066n2.core.rbf");
+			type = "fpga";
+			compression = "none";
+			load = <0x1000>;
+                };
+	};
+
+	configurations {
+		default = "conf";
+		conf {
+			description = "Altera Arria10 SoCDK";
+			loadables = "uboot";
+			fdt = "fdt";
+			fpga = "fpga";
+		};
+	};
+};
-- 
1.7.7.4

[U-Boot] [PATCH 9/9] ARM: socfpga: Synchronize the configuration for A10 SoCDK

[tien.fong.chee at intel.com]

From: Tien Fong Chee <tien.fong.chee@intel.com>

Update the default configuration file to enable the necessary functionality
the get the kit working. That includes fitImage loading in SPL, SPL SD/MMC
support, USB, I2C.

Signed-off-by: Marek Vasut <marex@denx.de>
Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
---
 configs/socfpga_arria10_defconfig |   42 +++++++++++++++++++++++++++++++-----
 1 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/configs/socfpga_arria10_defconfig b/configs/socfpga_arria10_defconfig
index f88910c..86f078b 100644
--- a/configs/socfpga_arria10_defconfig
+++ b/configs/socfpga_arria10_defconfig
@@ -1,35 +1,48 @@
 CONFIG_ARM=y
 CONFIG_ARCH_SOCFPGA=y
 CONFIG_SYS_TEXT_BASE=0x01000040
-CONFIG_SYS_MALLOC_F_LEN=0x2000
+CONFIG_SYS_MALLOC_F_LEN=0x8000
 CONFIG_TARGET_SOCFPGA_ARRIA10_SOCDK=y
 CONFIG_SPL=y
 CONFIG_IDENT_STRING="socfpga_arria10"
 CONFIG_DISTRO_DEFAULTS=y
 CONFIG_NR_DRAM_BANKS=1
+CONFIG_FIT=y
+CONFIG_FIT_VERBOSE=y
+CONFIG_SPL_LOAD_FIT=y
+CONFIG_SPL_FIT_SOURCE="board/altera/arria10-socdk/fit_spl_fpga.its"
 CONFIG_USE_BOOTARGS=y
 CONFIG_BOOTARGS="console=ttyS0,115200"
 # CONFIG_USE_BOOTCOMMAND is not set
+CONFIG_SYS_CONSOLE_IS_IN_ENV=y
+CONFIG_SYS_CONSOLE_OVERWRITE_ROUTINE=y
+CONFIG_SYS_CONSOLE_ENV_OVERWRITE=y
 CONFIG_DEFAULT_FDT_FILE="socfpga_arria10_socdk_sdmmc.dtb"
+CONFIG_VERSION_VARIABLE=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x800
 CONFIG_SPL_FPGA_SUPPORT=y
-CONFIG_SPL_SPI_LOAD=y
 CONFIG_CMD_ASKENV=y
 CONFIG_CMD_GREPENV=y
+CONFIG_CMD_DFU=y
 # CONFIG_CMD_FLASH is not set
 CONFIG_CMD_GPIO=y
+CONFIG_CMD_I2C=y
 CONFIG_CMD_MMC=y
+CONFIG_CMD_SF=y
+CONFIG_CMD_SPI=y
+CONFIG_CMD_USB=y
+CONFIG_CMD_USB_MASS_STORAGE=y
 CONFIG_CMD_CACHE=y
 CONFIG_CMD_EXT4_WRITE=y
 CONFIG_MTDIDS_DEFAULT="nor0=ff705000.spi.0"
-# CONFIG_SPL_DOS_PARTITION is not set
-# CONFIG_ISO_PARTITION is not set
-# CONFIG_EFI_PARTITION is not set
+CONFIG_OF_SPL_REMOVE_PROPS="interrupts interrupt-parent dmas dma-names"
 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_DFU_MMC=y
 CONFIG_SPL_DM_MMC=y
 CONFIG_SPL_MMC_SUPPORT=y
 CONFIG_SPL_FS_SUPPORT=y
@@ -41,13 +54,30 @@ CONFIG_FS_LOADER=y
 CONFIG_FPGA_SOCFPGA=y
 CONFIG_DM_GPIO=y
 CONFIG_DWAPB_GPIO=y
+CONFIG_SYS_I2C_DW=y
 CONFIG_DM_MMC=y
 CONFIG_MTD_DEVICE=y
+CONFIG_MTD_PARTITIONS=y
+CONFIG_MMC_DW=y
+CONFIG_SPI_FLASH=y
+CONFIG_SPI_FLASH_BAR=y
+CONFIG_SPI_FLASH_SPANSION=y
+CONFIG_SPI_FLASH_STMICRO=y
+CONFIG_PHY_MICREL=y
+CONFIG_PHY_MICREL_KSZ90X1=y
 CONFIG_DM_ETH=y
 CONFIG_ETH_DESIGNWARE=y
 CONFIG_MII=y
+CONFIG_SYS_NS16550=y
 CONFIG_SPI=y
 CONFIG_TIMER=y
 CONFIG_SPL_TIMER=y
 CONFIG_DESIGNWARE_APB_TIMER=y
-CONFIG_USE_TINY_PRINTF=y
+CONFIG_DESIGNWARE_SPI=y
+CONFIG_USB=y
+CONFIG_DM_USB=y
+CONFIG_USB_DWC2=y
+CONFIG_USB_STORAGE=y
+CONFIG_USB_GADGET=y
+CONFIG_USB_GADGET_DWC2_OTG=y
+CONFIG_USB_GADGET_DOWNLOAD=y
-- 
1.7.7.4

[U-Boot] [PATCH 1/9] ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

[Marek Vasut]

On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
> From: Tien Fong Chee <tien.fong.chee@intel.com>
> 
> This patch adds description on properties about file name used for both
> peripheral bitstream and core bitstream.
> 
> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> ---
>  .../fpga/altera-socfpga-a10-fpga-mgr.txt           |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt b/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt
> index 2fd8e7a..010322a 100644
> --- a/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt
> +++ b/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt
> @@ -7,6 +7,10 @@ Required properties:
>                 - The second index is for writing FPGA configuration data.
>  - resets     : Phandle and reset specifier for the device's reset.
>  - clocks     : Clocks used by the device.
> +- altr,bitstream_periph : File name for FPGA peripheral raw binary which is used
> +			  to initialize FPGA IOs, PLL, IO48 and DDR.
> +- altr,bitstream_core : File name for core raw binary which contains FPGA design
> +			which is used to program FPGA CRAM and ERAM.

bitstream- instead of bitstream_

btw can we get something that works with full bitstream too ?

>  Example:
>  
> @@ -16,4 +20,6 @@ Example:
>  		       0xffcfe400 0x20>;
>  		clocks = <&l4_mp_clk>;
>  		resets = <&rst FPGAMGR_RESET>;
> +		altr,bitstream_periph = "ghrd_10as066n2.periph.rbf.mkimage";
> +		altr,bitstream_core = "ghrd_10as066n2.core.rbf.mkimage";
>  	};
> 


-- 
Best regards,
Marek Vasut

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

[Marek Vasut]

On 11/21/2018 11:41 AM, tien.fong.chee at 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;
> +}

[...]
-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 3/9] spl : socfpga: Implement fpga bitstream loading with socfpga loadfs

[Marek Vasut]

On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
> From: Tien Fong Chee <tien.fong.chee@intel.com>
> 
> Add support for loading FPGA bitstream to get DDR up running before
> U-Boot is loaded into DDR. Boot device initialization, generic firmware
> loader and SPL FAT support are required for this whole mechanism to work.
> 
> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> ---
>  arch/arm/mach-socfpga/spl_a10.c |   49 ++++++++++++++++++++++++++++++++++++++-
>  common/spl/spl_mmc.c            |    2 +-
>  include/mmc.h                   |    1 +
>  3 files changed, 50 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-socfpga/spl_a10.c
> index 3ea64f7..67a4fac 100644
> --- a/arch/arm/mach-socfpga/spl_a10.c
> +++ b/arch/arm/mach-socfpga/spl_a10.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  /*
> - *  Copyright (C) 2012 Altera Corporation <www.altera.com>
> + *  Copyright (C) 2012-2018 Altera Corporation <www.altera.com>
>   */
>  
>  #include <common.h>
> @@ -23,6 +23,10 @@
>  #include <fdtdec.h>
>  #include <watchdog.h>
>  #include <asm/arch/pinmux.h>
> +#include <asm/arch/fpga_manager.h>
> +#include <mmc.h>
> +
> +#define RBF	0
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> @@ -73,6 +77,49 @@ void spl_board_init(void)
>  	WATCHDOG_RESET();
>  
>  	arch_early_init_r();
> +
> +	/* If the full FPGA is already loaded, ie.from EPCQ, config fpga pins */
> +	if (is_fpgamgr_user_mode()) {
> +		config_pins(gd->fdt_blob, "shared");
> +		config_pins(gd->fdt_blob, "fpga");
> +	} else if (!is_fpgamgr_early_user_mode()) {
> +		/* Program IOSSM(early IO release) or full FPGA */
> +		fpga_fs_info fpga_fsinfo;
> +		char buf[16 * 1024] __aligned(ARCH_DMA_MINALIGN);
> +		struct spl_boot_device bootdev;
> +		int len = 0;
> +
> +		bootdev.boot_device = spl_boot_device();
> +
> +		/* Init MMC driver before reading FPGA bitstream from flash */
> +		if (bootdev.boot_device == BOOT_DEVICE_MMC1) {
> +			struct mmc *mmc = NULL;
> +			int err = 0;
> +
> +			err = spl_mmc_find_device(&mmc, bootdev.boot_device);
> +			if (err)
> +				return;
> +
> +			err = mmc_init(mmc);

I thought all this backend specific stuff would be hidden in the FW loader.

> +			if (err) {
> +				debug("spl: mmc init failed with error: %d\n",
> +					err);
> +
> +				return;
> +			}
> +		}
> +
> +		fpga_fsinfo.filename = (char *)get_fpga_filename(gd->fdt_blob,
> +								 &len,
> +								 RBF);
> +
> +		socfpga_loadfs(&fpga_fsinfo, buf, sizeof(buf), 0);
> +	}
> +
> +	/* If the IOSSM/full FPGA is already loaded, start DDR */
> +	if (is_fpgamgr_early_user_mode() || is_fpgamgr_user_mode())
> +		ddr_calibration_sequence();
>  }
>  
>  void board_init_f(ulong dummy)
> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> index 4d55dcc..b85e146 100644
> --- a/common/spl/spl_mmc.c
> +++ b/common/spl/spl_mmc.c
> @@ -111,7 +111,7 @@ static int spl_mmc_get_device_index(u32 boot_device)
>  	return -ENODEV;
>  }
>  
> -static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device)
> +int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device)
>  {
>  #if CONFIG_IS_ENABLED(DM_MMC)
>  	struct udevice *dev;
> diff --git a/include/mmc.h b/include/mmc.h
> index 95548e9..de92909 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -829,6 +829,7 @@ int board_mmc_init(bd_t *bis);
>  int cpu_mmc_init(bd_t *bis);
>  int mmc_get_env_addr(struct mmc *mmc, int copy, u32 *env_addr);
>  int mmc_get_env_dev(void);
> +int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device);
>  
>  /* Set block count limit because of 16 bit register limit on some hardware*/
>  #ifndef CONFIG_SYS_MMC_MAX_BLK_COUNT
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 4/9] ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10

[Marek Vasut]

On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
> From: Tien Fong Chee <tien.fong.chee@intel.com>

Did you change Author:ship of the patch ?

> Bundle U-Boot fitImage containing U-Boot and FPGA bitstream into the
> u-boot-with-spl.sfp on Arria10. This lets U-Boot operate in a very
> similar fashion to Gen5, where the U-Boot binary got loaded by the
> SPL from a uImage concatenated at the end of the SPL SFP image. On
> Gen10, the U-Boot is in fitImage which contains the FPGA bitstream
> as well. In this case, the SPL can load the FPGA bitstream first and
> load the U-Boot afterward in the same manner. This is nonetheless a
> stopgap measure until there is a proper firmware loader in U-Boot.

Right, this is a stopgap measure until FW loader is present. Why is this
patch needed at all in this series ?

> Signed-off-by: Marek Vasut <marex@denx.de>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> ---
>  Makefile                         |    9 +++++++--
>  include/configs/socfpga_common.h |    4 ++++
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index a55915d..4ecc19d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1212,9 +1212,14 @@ ifneq ($(CONFIG_ARCH_SOCFPGA),)
>  quiet_cmd_socboot = SOCBOOT $@
>  cmd_socboot = cat	spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
>  			spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
> -			u-boot.img > $@ || rm -f $@
> +			$2 > $@ || rm -f $@
> +ifdef CONFIG_TARGET_SOCFPGA_ARRIA10
> +u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-boot.itb FORCE
> +	$(call if_changed,socboot,u-boot.itb)
> +else
>  u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
> -	$(call if_changed,socboot)
> +	$(call if_changed,socboot,u-boot.img)
> +endif
>  endif
>  
>  ifeq ($(CONFIG_MPC85xx)$(CONFIG_OF_SEPARATE),yy)
> diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h
> index bd8f5c8..ffdc6eb 100644
> --- a/include/configs/socfpga_common.h
> +++ b/include/configs/socfpga_common.h
> @@ -268,7 +268,11 @@ unsigned int cm_get_qspi_controller_clk_hz(void);
>  /* SPL SDMMC boot support */
>  #ifdef CONFIG_SPL_MMC_SUPPORT
>  #if defined(CONFIG_SPL_FAT_SUPPORT) || defined(CONFIG_SPL_EXT_SUPPORT)
> +#if CONFIG_SPL_FIT
> +#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME		"u-boot.itb"
> +#else
>  #define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME		"u-boot-dtb.img"
> +#endif
>  #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION	1
>  #endif
>  #else
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 5/9] ARM: socfpga: Add SPL fitImage config match

[Marek Vasut]

On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
> From: Tien Fong Chee <tien.fong.chee@intel.com>
> 
> Add empty SPL fitImage configuration match. This can be extended
> if there is ever need to support multiple boards with single SFP
> image.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>

It's missing your SoB line, but again, why is this patch needed ?

> ---
>  arch/arm/mach-socfpga/spl_a10.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-socfpga/spl_a10.c
> index 67a4fac..2baeba6 100644
> --- a/arch/arm/mach-socfpga/spl_a10.c
> +++ b/arch/arm/mach-socfpga/spl_a10.c
> @@ -148,3 +148,13 @@ void board_init_f(ulong dummy)
>  	config_dedicated_pins(gd->fdt_blob);
>  	WATCHDOG_RESET();
>  }
> +
> +#ifdef CONFIG_SPL_LOAD_FIT
> +int board_fit_config_name_match(const char *name)
> +{
> +	/* Just empty function now - can't decide what to choose */
> +	debug("%s: %s\n", __func__, name);
> +
> +	return 0;
> +}
> +#endif
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 6/9] ARM: socfpga: Set default DTB address on A10

[Marek Vasut]

On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
> From: Tien Fong Chee <tien.fong.chee@intel.com>

Again, not your patch, please don't change authorship.

http://git.denx.de/?p=u-boot/u-boot-socfpga.git;a=commit;h=eb32e568869b5f91fe34efb2642875a8da5f0ebd

> Set default DT blob address on A10 SoCDK, since this SoC uses OF
> separate configuration. The 0xf0000 address is just below the text
> base and still leaves enough room for the DT to grow.

Why is this needed ?

> Signed-off-by: Marek Vasut <marex@denx.de>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> ---
>  arch/arm/mach-socfpga/board.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-socfpga/board.c b/arch/arm/mach-socfpga/board.c
> index 7c8c05c..436a8a8 100644
> --- a/arch/arm/mach-socfpga/board.c
> +++ b/arch/arm/mach-socfpga/board.c
> @@ -86,3 +86,11 @@ int g_dnl_board_usb_cable_connected(void)
>  	return 1;
>  }
>  #endif
> +
> +#if defined(CONFIG_TARGET_SOCFPGA_ARRIA10) && \
> +!defined(CONFIG_SPL_BUILD) && defined(CONFIG_OF_SEPARATE)
> +void *board_fdt_blob_setup(void)
> +{
> +	return (void *)0xf00000;
> +}
> +#endif
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 1/9] ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

[Chee, Tien Fong]

On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote:
> On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
> > 
> > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > 
> > This patch adds description on properties about file name used for
> > both
> > peripheral bitstream and core bitstream.
> > 
> > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > ---
> >  .../fpga/altera-socfpga-a10-fpga-mgr.txt           |    6 ++++++
> >  1 files changed, 6 insertions(+), 0 deletions(-)
> > 
> > diff --git a/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-
> > mgr.txt b/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-
> > mgr.txt
> > index 2fd8e7a..010322a 100644
> > --- a/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt
> > +++ b/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt
> > @@ -7,6 +7,10 @@ Required properties:
> >                 - The second index is for writing FPGA
> > configuration data.
> >  - resets     : Phandle and reset specifier for the device's reset.
> >  - clocks     : Clocks used by the device.
> > +- altr,bitstream_periph : File name for FPGA peripheral raw binary
> > which is used
> > +			  to initialize FPGA IOs, PLL, IO48 and
> > DDR.
> > +- altr,bitstream_core : File name for core raw binary which
> > contains FPGA design
> > +			which is used to program FPGA CRAM and
> > ERAM.
> bitstream- instead of bitstream_
Noted.
> 
> btw can we get something that works with full bitstream too ?
This patchset actually support the full bitstream too, unfortunately it
is blocked by hardware MPFE issue. The patchset for the MPFE workaround
would come after this patchset. I would advice to use the early IO
release method for the sake of performance.

For details of issue, you can read the from the link https://github.com
/altera-opensource/u-boot-socfpga/commits/socfpga_v2014.10_arria10_brin
gup
FogBugz #410989-6: Masking hardware sequenced warm reset for logic
in…  …

Tien Fong Chee
Tien Fong Chee committed on Feb 16, 2017
 
FogBugz #410989-5: Enable RAM boot  …

Tien Fong Chee
Tien Fong Chee committed on Feb 16, 2017
 
FogBugz #410989-4: Added software reset for QSPI  …

Tien Fong Chee
Tien Fong Chee committed on Feb 8, 2017
 
FogBugz #410989-3: Disable redundant redundant messages after a warm
…  …

Tien Fong Chee
Tien Fong Chee committed on Dec 21, 2016
 
FogBugz #410989-2: Reset MPFE NoC after programming periperal rbf  …

Tien Fong Chee
Tien Fong Chee committed on Dec 21, 2016
 
FogBugz #410989-1: Functions for setting/checking magic no. to
isw_ha…  …

Tien Fong Chee
Tien Fong Chee committed on Dec 21, 2016
> 
> > 
> >  Example:
> >  
> > @@ -16,4 +20,6 @@ Example:
> >  		       0xffcfe400 0x20>;
> >  		clocks = <&l4_mp_clk>;
> >  		resets = <&rst FPGAMGR_RESET>;
> > +		altr,bitstream_periph =
> > "ghrd_10as066n2.periph.rbf.mkimage";
> > +		altr,bitstream_core =
> > "ghrd_10as066n2.core.rbf.mkimage";
> >  	};
> > 
> 

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

[Chee, Tien Fong]

On Wed, 2018-11-21 at 15:18 +0100, Marek Vasut wrote:
> On 11/21/2018 11:41 AM, tien.fong.chee at 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.
Noted.
> 
> > 
> >  #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.
Noted.
> 
> > 
> > +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
Okay.
> 
> > 
> >  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 ?
Both periph.rbf or full.rbf are wrapped with mkimage. So, this CRC
checking can be used to check the integrity of the loading bitstream
data against checksum from mkimage. It is off for the sake of
performance.
> 
> > 
> > +	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.
PERIPH_RBF and CORE_RBF are the flags, so i can change them to enum.
But above #define are magic content used to identify the bistream type.
If above #define are not suitable, what can you suggest?
> 
> > 
> >  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.
Noted.
> 
> > 
> > +	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 ?
A place for storing firmware and its attribute data. This would be
shared across a few helper functions which contain firmware loader.
> 
> > 
> > +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.
This function is mainly for checking the mkimage header, searching the
location of the bitstream image from the 1st chunk of reading data and
updating the read location.
The remaining of the bitstream data after 1st chunk would be processed
by the function called subsequent_loading_rbf_to_buffer().
> 
> > 
> > +	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 ?
It is off for the sake of performance.
> 
> > 
> > +#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;
> > +}
> [...]

[U-Boot] [PATCH 3/9] spl : socfpga: Implement fpga bitstream loading with socfpga loadfs

[Chee, Tien Fong]

On Wed, 2018-11-21 at 15:19 +0100, Marek Vasut wrote:
> On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
> > 
> > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > 
> > Add support for loading FPGA bitstream to get DDR up running before
> > U-Boot is loaded into DDR. Boot device initialization, generic
> > firmware
> > loader and SPL FAT support are required for this whole mechanism to
> > work.
> > 
> > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > ---
> >  arch/arm/mach-socfpga/spl_a10.c |   49
> > ++++++++++++++++++++++++++++++++++++++-
> >  common/spl/spl_mmc.c            |    2 +-
> >  include/mmc.h                   |    1 +
> >  3 files changed, 50 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-
> > socfpga/spl_a10.c
> > index 3ea64f7..67a4fac 100644
> > --- a/arch/arm/mach-socfpga/spl_a10.c
> > +++ b/arch/arm/mach-socfpga/spl_a10.c
> > @@ -1,6 +1,6 @@
> >  // SPDX-License-Identifier: GPL-2.0+
> >  /*
> > - *  Copyright (C) 2012 Altera Corporation <www.altera.com>
> > + *  Copyright (C) 2012-2018 Altera Corporation <www.altera.com>
> >   */
> >  
> >  #include <common.h>
> > @@ -23,6 +23,10 @@
> >  #include <fdtdec.h>
> >  #include <watchdog.h>
> >  #include <asm/arch/pinmux.h>
> > +#include <asm/arch/fpga_manager.h>
> > +#include <mmc.h>
> > +
> > +#define RBF	0
> >  
> >  DECLARE_GLOBAL_DATA_PTR;
> >  
> > @@ -73,6 +77,49 @@ void spl_board_init(void)
> >  	WATCHDOG_RESET();
> >  
> >  	arch_early_init_r();
> > +
> > +	/* If the full FPGA is already loaded, ie.from EPCQ,
> > config fpga pins */
> > +	if (is_fpgamgr_user_mode()) {
> > +		config_pins(gd->fdt_blob, "shared");
> > +		config_pins(gd->fdt_blob, "fpga");
> > +	} else if (!is_fpgamgr_early_user_mode()) {
> > +		/* Program IOSSM(early IO release) or full FPGA */
> > +		fpga_fs_info fpga_fsinfo;
> > +		char buf[16 * 1024] __aligned(ARCH_DMA_MINALIGN);
> > +		struct spl_boot_device bootdev;
> > +		int len = 0;
> > +
> > +		bootdev.boot_device = spl_boot_device();
> > +
> > +		/* Init MMC driver before reading FPGA bitstream
> > from flash */
> > +		if (bootdev.boot_device == BOOT_DEVICE_MMC1) {
> > +			struct mmc *mmc = NULL;
> > +			int err = 0;
> > +
> > +			err = spl_mmc_find_device(&mmc,
> > bootdev.boot_device);
> > +			if (err)
> > +				return;
> > +
> > +			err = mmc_init(mmc);
> I thought all this backend specific stuff would be hidden in the FW
> loader.
The backend supported by FW loader is up to generic file system
interface layer. flash driver init is expected done by SPL/U-Boot
common init sequence framwork or user. Unfortunately, fw loader need to
access flash before init sequence.
> 
> > 
> > +			if (err) {
> > +				debug("spl: mmc init failed with
> > error: %d\n",
> > +					err);
> > +
> > +				return;
> > +			}
> > +		}
> > +
> > +		fpga_fsinfo.filename = (char
> > *)get_fpga_filename(gd->fdt_blob,
> > +								 &
> > len,
> > +								 R
> > BF);
> > +
> > +		socfpga_loadfs(&fpga_fsinfo, buf, sizeof(buf), 0);
> > +	}
> > +
> > +	/* If the IOSSM/full FPGA is already loaded, start DDR */
> > +	if (is_fpgamgr_early_user_mode() ||
> > is_fpgamgr_user_mode())
> > +		ddr_calibration_sequence();
> >  }
> >  
> >  void board_init_f(ulong dummy)
> > diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> > index 4d55dcc..b85e146 100644
> > --- a/common/spl/spl_mmc.c
> > +++ b/common/spl/spl_mmc.c
> > @@ -111,7 +111,7 @@ static int spl_mmc_get_device_index(u32
> > boot_device)
> >  	return -ENODEV;
> >  }
> >  
> > -static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device)
> > +int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device)
> >  {
> >  #if CONFIG_IS_ENABLED(DM_MMC)
> >  	struct udevice *dev;
> > diff --git a/include/mmc.h b/include/mmc.h
> > index 95548e9..de92909 100644
> > --- a/include/mmc.h
> > +++ b/include/mmc.h
> > @@ -829,6 +829,7 @@ int board_mmc_init(bd_t *bis);
> >  int cpu_mmc_init(bd_t *bis);
> >  int mmc_get_env_addr(struct mmc *mmc, int copy, u32 *env_addr);
> >  int mmc_get_env_dev(void);
> > +int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device);
> >  
> >  /* Set block count limit because of 16 bit register limit on some
> > hardware*/
> >  #ifndef CONFIG_SYS_MMC_MAX_BLK_COUNT
> > 
> 

[U-Boot] [PATCH 4/9] ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10

[Chee, Tien Fong]

On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote:
> On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
> > 
> > From: Tien Fong Chee <tien.fong.chee@intel.com>
> Did you change Author:ship of the patch ?
> 
> > 
> > Bundle U-Boot fitImage containing U-Boot and FPGA bitstream into
> > the
> > u-boot-with-spl.sfp on Arria10. This lets U-Boot operate in a very
> > similar fashion to Gen5, where the U-Boot binary got loaded by the
> > SPL from a uImage concatenated at the end of the SPL SFP image. On
> > Gen10, the U-Boot is in fitImage which contains the FPGA bitstream
> > as well. In this case, the SPL can load the FPGA bitstream first
> > and
> > load the U-Boot afterward in the same manner. This is nonetheless a
> > stopgap measure until there is a proper firmware loader in U-Boot.
> Right, this is a stopgap measure until FW loader is present. Why is
> this
> patch needed at all in this series ?
This patch is cherry picked from the sdmmc_next custodian, so this
patch is required for generating FIT image. I can remove the stopgap
comment to avoid confusing.
> 
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > ---
> >  Makefile                         |    9 +++++++--
> >  include/configs/socfpga_common.h |    4 ++++
> >  2 files changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index a55915d..4ecc19d 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1212,9 +1212,14 @@ ifneq ($(CONFIG_ARCH_SOCFPGA),)
> >  quiet_cmd_socboot = SOCBOOT $@
> >  cmd_socboot = cat	spl/u-boot-spl.sfp spl/u-boot-spl.sfp	
> > \
> >  			spl/u-boot-spl.sfp spl/u-boot-spl.sfp	
> > \
> > -			u-boot.img > $@ || rm -f $@
> > +			$2 > $@ || rm -f $@
> > +ifdef CONFIG_TARGET_SOCFPGA_ARRIA10
> > +u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-boot.itb FORCE
> > +	$(call if_changed,socboot,u-boot.itb)
> > +else
> >  u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
> > -	$(call if_changed,socboot)
> > +	$(call if_changed,socboot,u-boot.img)
> > +endif
> >  endif
> >  
> >  ifeq ($(CONFIG_MPC85xx)$(CONFIG_OF_SEPARATE),yy)
> > diff --git a/include/configs/socfpga_common.h
> > b/include/configs/socfpga_common.h
> > index bd8f5c8..ffdc6eb 100644
> > --- a/include/configs/socfpga_common.h
> > +++ b/include/configs/socfpga_common.h
> > @@ -268,7 +268,11 @@ unsigned int
> > cm_get_qspi_controller_clk_hz(void);
> >  /* SPL SDMMC boot support */
> >  #ifdef CONFIG_SPL_MMC_SUPPORT
> >  #if defined(CONFIG_SPL_FAT_SUPPORT) ||
> > defined(CONFIG_SPL_EXT_SUPPORT)
> > +#if CONFIG_SPL_FIT
> > +#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME		"u-
> > boot.itb"
> > +#else
> >  #define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME		"u-boot-
> > dtb.img"
> > +#endif
> >  #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION	1
> >  #endif
> >  #else
> > 
> 

[U-Boot] [PATCH 5/9] ARM: socfpga: Add SPL fitImage config match

[Chee, Tien Fong]

On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote:
> On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
> > 
> > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > 
> > Add empty SPL fitImage configuration match. This can be extended
> > if there is ever need to support multiple boards with single SFP
> > image.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> It's missing your SoB line, but again, why is this patch needed ?
This patch i also cherry picked from sdmmc_next custodian, and i didn't
made any changes, so i still need SoB?
Without this patch, compiling failed with error"/uboot-
socfpga/common/common_fit.c:66: undefined reference to
`board_fit_config_name_match'"
> 
> > 
> > ---
> >  arch/arm/mach-socfpga/spl_a10.c |   10 ++++++++++
> >  1 files changed, 10 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-
> > socfpga/spl_a10.c
> > index 67a4fac..2baeba6 100644
> > --- a/arch/arm/mach-socfpga/spl_a10.c
> > +++ b/arch/arm/mach-socfpga/spl_a10.c
> > @@ -148,3 +148,13 @@ void board_init_f(ulong dummy)
> >  	config_dedicated_pins(gd->fdt_blob);
> >  	WATCHDOG_RESET();
> >  }
> > +
> > +#ifdef CONFIG_SPL_LOAD_FIT
> > +int board_fit_config_name_match(const char *name)
> > +{
> > +	/* Just empty function now - can't decide what to choose
> > */
> > +	debug("%s: %s\n", __func__, name);
> > +
> > +	return 0;
> > +}
> > +#endif
> > 
> 

[U-Boot] [PATCH 6/9] ARM: socfpga: Set default DTB address on A10

[Chee, Tien Fong]

On Wed, 2018-11-21 at 15:22 +0100, Marek Vasut wrote:
> On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
> > 
> > From: Tien Fong Chee <tien.fong.chee@intel.com>
> Again, not your patch, please don't change authorship.
> 
> http://git.denx.de/?p=u-boot/u-boot-socfpga.git;a=commit;h=eb32e56886
> 9b5f91fe34efb2642875a8da5f0ebd
> 
> > 
> > Set default DT blob address on A10 SoCDK, since this SoC uses OF
> > separate configuration. The 0xf0000 address is just below the text
> > base and still leaves enough room for the DT to grow.
> Why is this needed ?
This patch i also cherry picked from sdmmc_next custodian. I think you
put this for FIT implementation? I saw there is load property
"0xf00000" defined for fdt in .its.
> 
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > ---
> >  arch/arm/mach-socfpga/board.c |    8 ++++++++
> >  1 files changed, 8 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/mach-socfpga/board.c b/arch/arm/mach-
> > socfpga/board.c
> > index 7c8c05c..436a8a8 100644
> > --- a/arch/arm/mach-socfpga/board.c
> > +++ b/arch/arm/mach-socfpga/board.c
> > @@ -86,3 +86,11 @@ int g_dnl_board_usb_cable_connected(void)
> >  	return 1;
> >  }
> >  #endif
> > +
> > +#if defined(CONFIG_TARGET_SOCFPGA_ARRIA10) && \
> > +!defined(CONFIG_SPL_BUILD) && defined(CONFIG_OF_SEPARATE)
> > +void *board_fdt_blob_setup(void)
> > +{
> > +	return (void *)0xf00000;
> > +}
> > +#endif
> > 
> 

[U-Boot] [PATCH 1/9] ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

[Marek Vasut]

On 11/23/2018 10:19 AM, Chee, Tien Fong wrote:
> On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote:
>> On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
>>>
>>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>>
>>> This patch adds description on properties about file name used for
>>> both
>>> peripheral bitstream and core bitstream.
>>>
>>> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
>>> ---
>>>  .../fpga/altera-socfpga-a10-fpga-mgr.txt           |    6 ++++++
>>>  1 files changed, 6 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-
>>> mgr.txt b/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-
>>> mgr.txt
>>> index 2fd8e7a..010322a 100644
>>> --- a/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt
>>> +++ b/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt
>>> @@ -7,6 +7,10 @@ Required properties:
>>>                 - The second index is for writing FPGA
>>> configuration data.
>>>  - resets     : Phandle and reset specifier for the device's reset.
>>>  - clocks     : Clocks used by the device.
>>> +- altr,bitstream_periph : File name for FPGA peripheral raw binary
>>> which is used
>>> +			  to initialize FPGA IOs, PLL, IO48 and
>>> DDR.
>>> +- altr,bitstream_core : File name for core raw binary which
>>> contains FPGA design
>>> +			which is used to program FPGA CRAM and
>>> ERAM.
>> bitstream- instead of bitstream_
> Noted.
>>
>> btw can we get something that works with full bitstream too ?
> This patchset actually support the full bitstream too, unfortunately it
> is blocked by hardware MPFE issue. The patchset for the MPFE workaround
> would come after this patchset. I would advice to use the early IO
> release method for the sake of performance.
> 
> For details of issue, you can read the from the link https://github.com
> /altera-opensource/u-boot-socfpga/commits/socfpga_v2014.10_arria10_brin
> gup
> FogBugz #410989-6: Masking hardware sequenced warm reset for logic
> in…  …
Does that work on ES2 ? I don't think so ...

-- 
Best regards,
Marek Vasut

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

[Marek Vasut]

On 11/23/2018 10:43 AM, Chee, Tien Fong wrote:
> On Wed, 2018-11-21 at 15:18 +0100, Marek Vasut wrote:
>> On 11/21/2018 11:41 AM, tien.fong.chee at 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.
> Noted.
>>
>>>
>>>  #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.
> Noted.
>>
>>>
>>> +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
> Okay.
>>
>>>
>>>  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 ?
> Both periph.rbf or full.rbf are wrapped with mkimage. So, this CRC
> checking can be used to check the integrity of the loading bitstream
> data against checksum from mkimage. It is off for the sake of
> performance.

Is there a measurable performance degradation ? I presume that's because
caches are disabled at that point, yes? Enable caches and see if that helps.

>>>
>>> +	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.
> PERIPH_RBF and CORE_RBF are the flags, so i can change them to enum.
> But above #define are magic content used to identify the bistream type.
> If above #define are not suitable, what can you suggest?

Maybe you can just align those two to avoid duplication ?

>>
>>>
>>>  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.
> Noted.
>>
>>>
>>> +	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 ?
> A place for storing firmware and its attribute data. This would be
> shared across a few helper functions which contain firmware loader.

Why is it not in the device data instead ? If you had multiple FPGAs in
the system, this would likely be randomly overwritten . Such static
variables shouldn't be needed in DM/DT capable system.

>>> +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.
> This function is mainly for checking the mkimage header, searching the
> location of the bitstream image from the 1st chunk of reading data and
> updating the read location.
> The remaining of the bitstream data after 1st chunk would be processed
> by the function called subsequent_loading_rbf_to_buffer().

I wonder if this can be somehow optimized, so it's not such a long
function with repeated pieces of code.

>>>
>>> +	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 ?
> It is off for the sake of performance.

Do you have some hard numbers to support this claim ?

>>
>>>
>>> +#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;
>>> +}
>> [...]


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 3/9] spl : socfpga: Implement fpga bitstream loading with socfpga loadfs

[Marek Vasut]

On 11/23/2018 10:51 AM, Chee, Tien Fong wrote:
> On Wed, 2018-11-21 at 15:19 +0100, Marek Vasut wrote:
>> On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
>>>
>>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>>
>>> Add support for loading FPGA bitstream to get DDR up running before
>>> U-Boot is loaded into DDR. Boot device initialization, generic
>>> firmware
>>> loader and SPL FAT support are required for this whole mechanism to
>>> work.
>>>
>>> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
>>> ---
>>>  arch/arm/mach-socfpga/spl_a10.c |   49
>>> ++++++++++++++++++++++++++++++++++++++-
>>>  common/spl/spl_mmc.c            |    2 +-
>>>  include/mmc.h                   |    1 +
>>>  3 files changed, 50 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-
>>> socfpga/spl_a10.c
>>> index 3ea64f7..67a4fac 100644
>>> --- a/arch/arm/mach-socfpga/spl_a10.c
>>> +++ b/arch/arm/mach-socfpga/spl_a10.c
>>> @@ -1,6 +1,6 @@
>>>  // SPDX-License-Identifier: GPL-2.0+
>>>  /*
>>> - *  Copyright (C) 2012 Altera Corporation <www.altera.com>
>>> + *  Copyright (C) 2012-2018 Altera Corporation <www.altera.com>
>>>   */
>>>  
>>>  #include <common.h>
>>> @@ -23,6 +23,10 @@
>>>  #include <fdtdec.h>
>>>  #include <watchdog.h>
>>>  #include <asm/arch/pinmux.h>
>>> +#include <asm/arch/fpga_manager.h>
>>> +#include <mmc.h>
>>> +
>>> +#define RBF	0
>>>  
>>>  DECLARE_GLOBAL_DATA_PTR;
>>>  
>>> @@ -73,6 +77,49 @@ void spl_board_init(void)
>>>  	WATCHDOG_RESET();
>>>  
>>>  	arch_early_init_r();
>>> +
>>> +	/* If the full FPGA is already loaded, ie.from EPCQ,
>>> config fpga pins */
>>> +	if (is_fpgamgr_user_mode()) {
>>> +		config_pins(gd->fdt_blob, "shared");
>>> +		config_pins(gd->fdt_blob, "fpga");
>>> +	} else if (!is_fpgamgr_early_user_mode()) {
>>> +		/* Program IOSSM(early IO release) or full FPGA */
>>> +		fpga_fs_info fpga_fsinfo;
>>> +		char buf[16 * 1024] __aligned(ARCH_DMA_MINALIGN);
>>> +		struct spl_boot_device bootdev;
>>> +		int len = 0;
>>> +
>>> +		bootdev.boot_device = spl_boot_device();
>>> +
>>> +		/* Init MMC driver before reading FPGA bitstream
>>> from flash */
>>> +		if (bootdev.boot_device == BOOT_DEVICE_MMC1) {
>>> +			struct mmc *mmc = NULL;
>>> +			int err = 0;
>>> +
>>> +			err = spl_mmc_find_device(&mmc,
>>> bootdev.boot_device);
>>> +			if (err)
>>> +				return;
>>> +
>>> +			err = mmc_init(mmc);
>> I thought all this backend specific stuff would be hidden in the FW
>> loader.
> The backend supported by FW loader is up to generic file system
> interface layer. flash driver init is expected done by SPL/U-Boot
> common init sequence framwork or user. Unfortunately, fw loader need to
> access flash before init sequence.

This is actually accessing eMMC though , not flash . If we need this
huge boilerplate code every time we use the FW loader, than the FW
loader needs fixing. I can understand the spl_boot_device() being
outside of the FW loader, but not the mmc_init() and co.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 5/9] ARM: socfpga: Add SPL fitImage config match

[Marek Vasut]

On 11/23/2018 11:05 AM, Chee, Tien Fong wrote:
> On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote:
>> On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
>>>
>>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>>
>>> Add empty SPL fitImage configuration match. This can be extended
>>> if there is ever need to support multiple boards with single SFP
>>> image.
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>> It's missing your SoB line, but again, why is this patch needed ?
> This patch i also cherry picked from sdmmc_next custodian, and i didn't
> made any changes, so i still need SoB?

Yes

> Without this patch, compiling failed with error"/uboot-
> socfpga/common/common_fit.c:66: undefined reference to
> `board_fit_config_name_match'"

Sure, it will fail. Let's continue the discussion at 4/9.

>>> ---
>>>  arch/arm/mach-socfpga/spl_a10.c |   10 ++++++++++
>>>  1 files changed, 10 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-
>>> socfpga/spl_a10.c
>>> index 67a4fac..2baeba6 100644
>>> --- a/arch/arm/mach-socfpga/spl_a10.c
>>> +++ b/arch/arm/mach-socfpga/spl_a10.c
>>> @@ -148,3 +148,13 @@ void board_init_f(ulong dummy)
>>>  	config_dedicated_pins(gd->fdt_blob);
>>>  	WATCHDOG_RESET();
>>>  }
>>> +
>>> +#ifdef CONFIG_SPL_LOAD_FIT
>>> +int board_fit_config_name_match(const char *name)
>>> +{
>>> +	/* Just empty function now - can't decide what to choose
>>> */
>>> +	debug("%s: %s\n", __func__, name);
>>> +
>>> +	return 0;
>>> +}
>>> +#endif
>>>


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 6/9] ARM: socfpga: Set default DTB address on A10

[Marek Vasut]

On 11/23/2018 11:10 AM, Chee, Tien Fong wrote:
> On Wed, 2018-11-21 at 15:22 +0100, Marek Vasut wrote:
>> On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
>>>
>>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>> Again, not your patch, please don't change authorship.
>>
>> http://git.denx.de/?p=u-boot/u-boot-socfpga.git;a=commit;h=eb32e56886
>> 9b5f91fe34efb2642875a8da5f0ebd
>>
>>>
>>> Set default DT blob address on A10 SoCDK, since this SoC uses OF
>>> separate configuration. The 0xf0000 address is just below the text
>>> base and still leaves enough room for the DT to grow.
>> Why is this needed ?
> This patch i also cherry picked from sdmmc_next custodian. I think you
> put this for FIT implementation? I saw there is load property
> "0xf00000" defined for fdt in .its.

Thanks for confirming that I am the original author of some of these
patches and you just changed that field when submitting them. That's a
big no-no.

Let's continue the discussion under patch 4/9.

>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
>>> ---
>>>  arch/arm/mach-socfpga/board.c |    8 ++++++++
>>>  1 files changed, 8 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-socfpga/board.c b/arch/arm/mach-
>>> socfpga/board.c
>>> index 7c8c05c..436a8a8 100644
>>> --- a/arch/arm/mach-socfpga/board.c
>>> +++ b/arch/arm/mach-socfpga/board.c
>>> @@ -86,3 +86,11 @@ int g_dnl_board_usb_cable_connected(void)
>>>  	return 1;
>>>  }
>>>  #endif
>>> +
>>> +#if defined(CONFIG_TARGET_SOCFPGA_ARRIA10) && \
>>> +!defined(CONFIG_SPL_BUILD) && defined(CONFIG_OF_SEPARATE)
>>> +void *board_fdt_blob_setup(void)
>>> +{
>>> +	return (void *)0xf00000;
>>> +}
>>> +#endif
>>>


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 4/9] ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10

[Marek Vasut]

On 11/23/2018 10:54 AM, Chee, Tien Fong wrote:
> On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote:
>> On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
>>>
>>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>> Did you change Author:ship of the patch ?

I believe you did, so please fix that.

>>> Bundle U-Boot fitImage containing U-Boot and FPGA bitstream into
>>> the
>>> u-boot-with-spl.sfp on Arria10. This lets U-Boot operate in a very
>>> similar fashion to Gen5, where the U-Boot binary got loaded by the
>>> SPL from a uImage concatenated at the end of the SPL SFP image. On
>>> Gen10, the U-Boot is in fitImage which contains the FPGA bitstream
>>> as well. In this case, the SPL can load the FPGA bitstream first
>>> and
>>> load the U-Boot afterward in the same manner. This is nonetheless a
>>> stopgap measure until there is a proper firmware loader in U-Boot.
>> Right, this is a stopgap measure until FW loader is present. Why is
>> this
>> patch needed at all in this series ?
> This patch is cherry picked from the sdmmc_next custodian, so this
> patch is required for generating FIT image. I can remove the stopgap
> comment to avoid confusing.

But why is this patch needed at all ? You use the firmware loader to
load the FPGA bitstream. Where does the fitImage come into play ?

The fitImage was used to circumvent the missing FW loader, when I needed
to load multiple files (bitstream and u-boot binary). Now there is no
such requirement anymore, so the entire fitImage machinery is probably
not needed ?

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 1/9] ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

[Chee, Tien Fong]

On Fri, 2018-11-23 at 13:23 +0100, Marek Vasut wrote:
> On 11/23/2018 10:19 AM, Chee, Tien Fong wrote:
> > 
> > On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote:
> > > 
> > > On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
> > > > 
> > > > 
> > > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > 
> > > > This patch adds description on properties about file name used
> > > > for
> > > > both
> > > > peripheral bitstream and core bitstream.
> > > > 
> > > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > ---
> > > >  .../fpga/altera-socfpga-a10-fpga-mgr.txt           |    6
> > > > ++++++
> > > >  1 files changed, 6 insertions(+), 0 deletions(-)
> > > > 
> > > > diff --git a/doc/device-tree-bindings/fpga/altera-socfpga-a10-
> > > > fpga-
> > > > mgr.txt b/doc/device-tree-bindings/fpga/altera-socfpga-a10-
> > > > fpga-
> > > > mgr.txt
> > > > index 2fd8e7a..010322a 100644
> > > > --- a/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-
> > > > mgr.txt
> > > > +++ b/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-
> > > > mgr.txt
> > > > @@ -7,6 +7,10 @@ Required properties:
> > > >                 - The second index is for writing FPGA
> > > > configuration data.
> > > >  - resets     : Phandle and reset specifier for the device's
> > > > reset.
> > > >  - clocks     : Clocks used by the device.
> > > > +- altr,bitstream_periph : File name for FPGA peripheral raw
> > > > binary
> > > > which is used
> > > > +			  to initialize FPGA IOs, PLL, IO48
> > > > and
> > > > DDR.
> > > > +- altr,bitstream_core : File name for core raw binary which
> > > > contains FPGA design
> > > > +			which is used to program FPGA CRAM and
> > > > ERAM.
> > > bitstream- instead of bitstream_
> > Noted.
> > > 
> > > 
> > > btw can we get something that works with full bitstream too ?
> > This patchset actually support the full bitstream too,
> > unfortunately it
> > is blocked by hardware MPFE issue. The patchset for the MPFE
> > workaround
> > would come after this patchset. I would advice to use the early IO
> > release method for the sake of performance.
> > 
> > For details of issue, you can read the from the link https://github
> > .com
> > /altera-opensource/u-boot-
> > socfpga/commits/socfpga_v2014.10_arria10_brin
> > gup
> > FogBugz #410989-6: Masking hardware sequenced warm reset for logic
> > in…  …
> Does that work on ES2 ? I don't think so ...
Why you think it doesn't work, using early IO or full rbf? The
bitstream limitation? What you see from the print out?
> 

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

[Chee, Tien Fong]

On Fri, 2018-11-23 at 13:28 +0100, Marek Vasut wrote:
> On 11/23/2018 10:43 AM, Chee, Tien Fong wrote:
> > 
> > On Wed, 2018-11-21 at 15:18 +0100, Marek Vasut wrote:
> > > 
> > > On 11/21/2018 11:41 AM, tien.fong.chee at 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.
> > Noted.
> > > 
> > > 
> > > > 
> > > > 
> > > >  #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.
> > Noted.
> > > 
> > > 
> > > > 
> > > > 
> > > > +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
> > Okay.
> > > 
> > > 
> > > > 
> > > > 
> > > >  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 ?
> > Both periph.rbf or full.rbf are wrapped with mkimage. So, this CRC
> > checking can be used to check the integrity of the loading
> > bitstream
> > data against checksum from mkimage. It is off for the sake of
> > performance.
> Is there a measurable performance degradation ? I presume that's
> because
> caches are disabled at that point, yes? Enable caches and see if that
> helps.
Just logical sense, performance sure getting degraded, especially
loading full rbf, but may be not that obvious for periph.rbf because of
very small size, i can try to measure. If not much difference, i can
enable checking in 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.
> > PERIPH_RBF and CORE_RBF are the flags, so i can change them to
> > enum.
> > But above #define are magic content used to identify the bistream
> > type.
> > If above #define are not suitable, what can you suggest?
> Maybe you can just align those two to avoid duplication ?
What's you means with duplication, they are different thing.
How about i change the name to ARRIA10RBF_PERIPH_TYPE
and ARRIA10RBF_CORE_TYPE.
> 
> > 
> > > 
> > > 
> > > > 
> > > > 
> > > >  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.
> > Noted.
> > > 
> > > 
> > > > 
> > > > 
> > > > +	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 ?
> > A place for storing firmware and its attribute data. This would be
> > shared across a few helper functions which contain firmware loader.
> Why is it not in the device data instead ? If you had multiple FPGAs
> in
> the system, this would likely be randomly overwritten . Such static
> variables shouldn't be needed in DM/DT capable system.
Noted. Made sense.
> 
> > 
> > > 
> > > > 
> > > > +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.
> > This function is mainly for checking the mkimage header, searching
> > the
> > location of the bitstream image from the 1st chunk of reading data
> > and
> > updating the read location.
> > The remaining of the bitstream data after 1st chunk would be
> > processed
> > by the function called subsequent_loading_rbf_to_buffer().
> I wonder if this can be somehow optimized, so it's not such a long
> function with repeated pieces of code.
I already try my best to optimize them without compromising consistent
implementation for periph rbf, core rbf and full rbf.
> 
> > 
> > > 
> > > > 
> > > > 
> > > > +	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 ?
> > It is off for the sake of performance.
> Do you have some hard numbers to support this claim ?
> 
> > 
> > > 
> > > 
> > > > 
> > > > 
> > > > +#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;
> > > > +}
> > > [...]
> 

[U-Boot] [PATCH 3/9] spl : socfpga: Implement fpga bitstream loading with socfpga loadfs

[Chee, Tien Fong]

On Fri, 2018-11-23 at 13:31 +0100, Marek Vasut wrote:
> On 11/23/2018 10:51 AM, Chee, Tien Fong wrote:
> > 
> > On Wed, 2018-11-21 at 15:19 +0100, Marek Vasut wrote:
> > > 
> > > On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
> > > > 
> > > > 
> > > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > 
> > > > Add support for loading FPGA bitstream to get DDR up running
> > > > before
> > > > U-Boot is loaded into DDR. Boot device initialization, generic
> > > > firmware
> > > > loader and SPL FAT support are required for this whole
> > > > mechanism to
> > > > work.
> > > > 
> > > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > ---
> > > >  arch/arm/mach-socfpga/spl_a10.c |   49
> > > > ++++++++++++++++++++++++++++++++++++++-
> > > >  common/spl/spl_mmc.c            |    2 +-
> > > >  include/mmc.h                   |    1 +
> > > >  3 files changed, 50 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-
> > > > socfpga/spl_a10.c
> > > > index 3ea64f7..67a4fac 100644
> > > > --- a/arch/arm/mach-socfpga/spl_a10.c
> > > > +++ b/arch/arm/mach-socfpga/spl_a10.c
> > > > @@ -1,6 +1,6 @@
> > > >  // SPDX-License-Identifier: GPL-2.0+
> > > >  /*
> > > > - *  Copyright (C) 2012 Altera Corporation <www.altera.com>
> > > > + *  Copyright (C) 2012-2018 Altera Corporation <www.altera.com
> > > > >
> > > >   */
> > > >  
> > > >  #include <common.h>
> > > > @@ -23,6 +23,10 @@
> > > >  #include <fdtdec.h>
> > > >  #include <watchdog.h>
> > > >  #include <asm/arch/pinmux.h>
> > > > +#include <asm/arch/fpga_manager.h>
> > > > +#include <mmc.h>
> > > > +
> > > > +#define RBF	0
> > > >  
> > > >  DECLARE_GLOBAL_DATA_PTR;
> > > >  
> > > > @@ -73,6 +77,49 @@ void spl_board_init(void)
> > > >  	WATCHDOG_RESET();
> > > >  
> > > >  	arch_early_init_r();
> > > > +
> > > > +	/* If the full FPGA is already loaded, ie.from EPCQ,
> > > > config fpga pins */
> > > > +	if (is_fpgamgr_user_mode()) {
> > > > +		config_pins(gd->fdt_blob, "shared");
> > > > +		config_pins(gd->fdt_blob, "fpga");
> > > > +	} else if (!is_fpgamgr_early_user_mode()) {
> > > > +		/* Program IOSSM(early IO release) or full
> > > > FPGA */
> > > > +		fpga_fs_info fpga_fsinfo;
> > > > +		char buf[16 * 1024]
> > > > __aligned(ARCH_DMA_MINALIGN);
> > > > +		struct spl_boot_device bootdev;
> > > > +		int len = 0;
> > > > +
> > > > +		bootdev.boot_device = spl_boot_device();
> > > > +
> > > > +		/* Init MMC driver before reading FPGA
> > > > bitstream
> > > > from flash */
> > > > +		if (bootdev.boot_device == BOOT_DEVICE_MMC1) {
> > > > +			struct mmc *mmc = NULL;
> > > > +			int err = 0;
> > > > +
> > > > +			err = spl_mmc_find_device(&mmc,
> > > > bootdev.boot_device);
> > > > +			if (err)
> > > > +				return;
> > > > +
> > > > +			err = mmc_init(mmc);
> > > I thought all this backend specific stuff would be hidden in the
> > > FW
> > > loader.
> > The backend supported by FW loader is up to generic file system
> > interface layer. flash driver init is expected done by SPL/U-Boot
> > common init sequence framwork or user. Unfortunately, fw loader
> > need to
> > access flash before init sequence.
> This is actually accessing eMMC though , not flash . If we need this
> huge boilerplate code every time we use the FW loader, than the FW
> loader needs fixing. I can understand the spl_boot_device() being
> outside of the FW loader, but not the mmc_init() and co.
I can explore the posibility of adding the flash int mechanism into the
fm loader probe function.
> 

[U-Boot] [PATCH 5/9] ARM: socfpga: Add SPL fitImage config match

[Chee, Tien Fong]

On Fri, 2018-11-23 at 13:34 +0100, Marek Vasut wrote:
> On 11/23/2018 11:05 AM, Chee, Tien Fong wrote:
> > 
> > On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote:
> > > 
> > > On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
> > > > 
> > > > 
> > > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > 
> > > > Add empty SPL fitImage configuration match. This can be
> > > > extended
> > > > if there is ever need to support multiple boards with single
> > > > SFP
> > > > image.
> > > > 
> > > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > It's missing your SoB line, but again, why is this patch needed ?
> > This patch i also cherry picked from sdmmc_next custodian, and i
> > didn't
> > made any changes, so i still need SoB?
> Yes
Noted.
> 
> > 
> > Without this patch, compiling failed with error"/uboot-
> > socfpga/common/common_fit.c:66: undefined reference to
> > `board_fit_config_name_match'"
> Sure, it will fail. Let's continue the discussion at 4/9.
Okay.
> 
> > 
> > > 
> > > > 
> > > > ---
> > > >  arch/arm/mach-socfpga/spl_a10.c |   10 ++++++++++
> > > >  1 files changed, 10 insertions(+), 0 deletions(-)
> > > > 
> > > > diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-
> > > > socfpga/spl_a10.c
> > > > index 67a4fac..2baeba6 100644
> > > > --- a/arch/arm/mach-socfpga/spl_a10.c
> > > > +++ b/arch/arm/mach-socfpga/spl_a10.c
> > > > @@ -148,3 +148,13 @@ void board_init_f(ulong dummy)
> > > >  	config_dedicated_pins(gd->fdt_blob);
> > > >  	WATCHDOG_RESET();
> > > >  }
> > > > +
> > > > +#ifdef CONFIG_SPL_LOAD_FIT
> > > > +int board_fit_config_name_match(const char *name)
> > > > +{
> > > > +	/* Just empty function now - can't decide what to
> > > > choose
> > > > */
> > > > +	debug("%s: %s\n", __func__, name);
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +#endif
> > > > 
> 

[U-Boot] [PATCH 4/9] ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10

[Chee, Tien Fong]

On Fri, 2018-11-23 at 13:40 +0100, Marek Vasut wrote:
> On 11/23/2018 10:54 AM, Chee, Tien Fong wrote:
> > 
> > On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote:
> > > 
> > > On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
> > > > 
> > > > 
> > > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > > Did you change Author:ship of the patch ?
> I believe you did, so please fix that.
Very sorry. I din't realize the author name was changed.
> 
> > 
> > > 
> > > > 
> > > > Bundle U-Boot fitImage containing U-Boot and FPGA bitstream
> > > > into
> > > > the
> > > > u-boot-with-spl.sfp on Arria10. This lets U-Boot operate in a
> > > > very
> > > > similar fashion to Gen5, where the U-Boot binary got loaded by
> > > > the
> > > > SPL from a uImage concatenated at the end of the SPL SFP image.
> > > > On
> > > > Gen10, the U-Boot is in fitImage which contains the FPGA
> > > > bitstream
> > > > as well. In this case, the SPL can load the FPGA bitstream
> > > > first
> > > > and
> > > > load the U-Boot afterward in the same manner. This is
> > > > nonetheless a
> > > > stopgap measure until there is a proper firmware loader in U-
> > > > Boot.
> > > Right, this is a stopgap measure until FW loader is present. Why
> > > is
> > > this
> > > patch needed at all in this series ?
> > This patch is cherry picked from the sdmmc_next custodian, so this
> > patch is required for generating FIT image. I can remove the
> > stopgap
> > comment to avoid confusing.
> But why is this patch needed at all ? You use the firmware loader to
> load the FPGA bitstream. Where does the fitImage come into play ?
> 
> The fitImage was used to circumvent the missing FW loader, when I
> needed
> to load multiple files (bitstream and u-boot binary). Now there is no
> such requirement anymore, so the entire fitImage machinery is
> probably
> not needed ?
Loading issue is not the reason we choose the fitImage. We choose it
because it allows more flexibility in handling various type images,
especially it allows user more choices to enhance integrity and
security protection.

Although we plan to use fitImage as our default implementation, but
this series of patches are still allow fw loading individual bitstream
image in filesystem partition. So, it is up to user to made the choice.
> 

[U-Boot] [PATCH 1/9] ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

[Marek Vasut]

On 11/26/2018 10:44 AM, Chee, Tien Fong wrote:
> On Fri, 2018-11-23 at 13:23 +0100, Marek Vasut wrote:
>> On 11/23/2018 10:19 AM, Chee, Tien Fong wrote:
>>>
>>> On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote:
>>>>
>>>> On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
>>>>>
>>>>>
>>>>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>>>>
>>>>> This patch adds description on properties about file name used
>>>>> for
>>>>> both
>>>>> peripheral bitstream and core bitstream.
>>>>>
>>>>> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
>>>>> ---
>>>>>  .../fpga/altera-socfpga-a10-fpga-mgr.txt           |    6
>>>>> ++++++
>>>>>  1 files changed, 6 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/doc/device-tree-bindings/fpga/altera-socfpga-a10-
>>>>> fpga-
>>>>> mgr.txt b/doc/device-tree-bindings/fpga/altera-socfpga-a10-
>>>>> fpga-
>>>>> mgr.txt
>>>>> index 2fd8e7a..010322a 100644
>>>>> --- a/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-
>>>>> mgr.txt
>>>>> +++ b/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-
>>>>> mgr.txt
>>>>> @@ -7,6 +7,10 @@ Required properties:
>>>>>                 - The second index is for writing FPGA
>>>>> configuration data.
>>>>>  - resets     : Phandle and reset specifier for the device's
>>>>> reset.
>>>>>  - clocks     : Clocks used by the device.
>>>>> +- altr,bitstream_periph : File name for FPGA peripheral raw
>>>>> binary
>>>>> which is used
>>>>> +			  to initialize FPGA IOs, PLL, IO48
>>>>> and
>>>>> DDR.
>>>>> +- altr,bitstream_core : File name for core raw binary which
>>>>> contains FPGA design
>>>>> +			which is used to program FPGA CRAM and
>>>>> ERAM.
>>>> bitstream- instead of bitstream_
>>> Noted.
>>>>
>>>>
>>>> btw can we get something that works with full bitstream too ?
>>> This patchset actually support the full bitstream too,
>>> unfortunately it
>>> is blocked by hardware MPFE issue. The patchset for the MPFE
>>> workaround
>>> would come after this patchset. I would advice to use the early IO
>>> release method for the sake of performance.
>>>
>>> For details of issue, you can read the from the link https://github
>>> .com
>>> /altera-opensource/u-boot-
>>> socfpga/commits/socfpga_v2014.10_arria10_brin
>>> gup
>>> FogBugz #410989-6: Masking hardware sequenced warm reset for logic
>>> in…  …
>> Does that work on ES2 ? I don't think so ...
> Why you think it doesn't work, using early IO or full rbf? The
> bitstream limitation? What you see from the print out?

ES2 can only use full RBF, I don't think this is handled in this
patchset at all.

-- 
Best regards,
Marek Vasut

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

[Marek Vasut]

On 11/26/2018 11:09 AM, Chee, Tien Fong wrote:
[...]
>>>>> 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 ?
>>> Both periph.rbf or full.rbf are wrapped with mkimage. So, this CRC
>>> checking can be used to check the integrity of the loading
>>> bitstream
>>> data against checksum from mkimage. It is off for the sake of
>>> performance.
>> Is there a measurable performance degradation ? I presume that's
>> because
>> caches are disabled at that point, yes? Enable caches and see if that
>> helps.
> Just logical sense, performance sure getting degraded, especially
> loading full rbf, but may be not that obvious for periph.rbf because of
> very small size, i can try to measure. If not much difference, i can
> enable checking in default.

Hard numbers are the only relevant argument here, please measure.
And try it with caches enabled as much as possible, you want users to
boot fast. Arria10 is particularly annoyingly slow at booting.

>>
>>>
>>>>
>>>>>
>>>>>
>>>>> +	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.
>>> PERIPH_RBF and CORE_RBF are the flags, so i can change them to
>>> enum.
>>> But above #define are magic content used to identify the bistream
>>> type.
>>> If above #define are not suitable, what can you suggest?
>> Maybe you can just align those two to avoid duplication ?
> What's you means with duplication, they are different thing.
> How about i change the name to ARRIA10RBF_PERIPH_TYPE
> and ARRIA10RBF_CORE_TYPE.

ARRIA10RBF_PERIPH = (PERIPH_RBF << 15) | 1

same for ... _CORE ... is that a coincidence ?

[...]

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 3/9] spl : socfpga: Implement fpga bitstream loading with socfpga loadfs

[Marek Vasut]

On 11/26/2018 11:10 AM, Chee, Tien Fong wrote:
> On Fri, 2018-11-23 at 13:31 +0100, Marek Vasut wrote:
>> On 11/23/2018 10:51 AM, Chee, Tien Fong wrote:
>>>
>>> On Wed, 2018-11-21 at 15:19 +0100, Marek Vasut wrote:
>>>>
>>>> On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
>>>>>
>>>>>
>>>>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>>>>
>>>>> Add support for loading FPGA bitstream to get DDR up running
>>>>> before
>>>>> U-Boot is loaded into DDR. Boot device initialization, generic
>>>>> firmware
>>>>> loader and SPL FAT support are required for this whole
>>>>> mechanism to
>>>>> work.
>>>>>
>>>>> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
>>>>> ---
>>>>>  arch/arm/mach-socfpga/spl_a10.c |   49
>>>>> ++++++++++++++++++++++++++++++++++++++-
>>>>>  common/spl/spl_mmc.c            |    2 +-
>>>>>  include/mmc.h                   |    1 +
>>>>>  3 files changed, 50 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-
>>>>> socfpga/spl_a10.c
>>>>> index 3ea64f7..67a4fac 100644
>>>>> --- a/arch/arm/mach-socfpga/spl_a10.c
>>>>> +++ b/arch/arm/mach-socfpga/spl_a10.c
>>>>> @@ -1,6 +1,6 @@
>>>>>  // SPDX-License-Identifier: GPL-2.0+
>>>>>  /*
>>>>> - *  Copyright (C) 2012 Altera Corporation <www.altera.com>
>>>>> + *  Copyright (C) 2012-2018 Altera Corporation <www.altera.com
>>>>>>
>>>>>   */
>>>>>  
>>>>>  #include <common.h>
>>>>> @@ -23,6 +23,10 @@
>>>>>  #include <fdtdec.h>
>>>>>  #include <watchdog.h>
>>>>>  #include <asm/arch/pinmux.h>
>>>>> +#include <asm/arch/fpga_manager.h>
>>>>> +#include <mmc.h>
>>>>> +
>>>>> +#define RBF	0
>>>>>  
>>>>>  DECLARE_GLOBAL_DATA_PTR;
>>>>>  
>>>>> @@ -73,6 +77,49 @@ void spl_board_init(void)
>>>>>  	WATCHDOG_RESET();
>>>>>  
>>>>>  	arch_early_init_r();
>>>>> +
>>>>> +	/* If the full FPGA is already loaded, ie.from EPCQ,
>>>>> config fpga pins */
>>>>> +	if (is_fpgamgr_user_mode()) {
>>>>> +		config_pins(gd->fdt_blob, "shared");
>>>>> +		config_pins(gd->fdt_blob, "fpga");
>>>>> +	} else if (!is_fpgamgr_early_user_mode()) {
>>>>> +		/* Program IOSSM(early IO release) or full
>>>>> FPGA */
>>>>> +		fpga_fs_info fpga_fsinfo;
>>>>> +		char buf[16 * 1024]
>>>>> __aligned(ARCH_DMA_MINALIGN);
>>>>> +		struct spl_boot_device bootdev;
>>>>> +		int len = 0;
>>>>> +
>>>>> +		bootdev.boot_device = spl_boot_device();
>>>>> +
>>>>> +		/* Init MMC driver before reading FPGA
>>>>> bitstream
>>>>> from flash */
>>>>> +		if (bootdev.boot_device == BOOT_DEVICE_MMC1) {
>>>>> +			struct mmc *mmc = NULL;
>>>>> +			int err = 0;
>>>>> +
>>>>> +			err = spl_mmc_find_device(&mmc,
>>>>> bootdev.boot_device);
>>>>> +			if (err)
>>>>> +				return;
>>>>> +
>>>>> +			err = mmc_init(mmc);
>>>> I thought all this backend specific stuff would be hidden in the
>>>> FW
>>>> loader.
>>> The backend supported by FW loader is up to generic file system
>>> interface layer. flash driver init is expected done by SPL/U-Boot
>>> common init sequence framwork or user. Unfortunately, fw loader
>>> need to
>>> access flash before init sequence.
>> This is actually accessing eMMC though , not flash . If we need this
>> huge boilerplate code every time we use the FW loader, than the FW
>> loader needs fixing. I can understand the spl_boot_device() being
>> outside of the FW loader, but not the mmc_init() and co.
> I can explore the posibility of adding the flash int mechanism into the
> fm loader probe function.

What do you mean by "flash int" ? Note that we're talking about eMMC
here, not flash. Unless you mean "backend init" by all that, in which
case that'd only make sense, thanks.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 4/9] ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10

[Marek Vasut]

On 11/26/2018 11:30 AM, Chee, Tien Fong wrote:
> On Fri, 2018-11-23 at 13:40 +0100, Marek Vasut wrote:
>> On 11/23/2018 10:54 AM, Chee, Tien Fong wrote:
>>>
>>> On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote:
>>>>
>>>> On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
>>>>>
>>>>>
>>>>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>>> Did you change Author:ship of the patch ?
>> I believe you did, so please fix that.
> Very sorry. I din't realize the author name was changed.

Please be careful next time.

>>
>>>
>>>>
>>>>>
>>>>> Bundle U-Boot fitImage containing U-Boot and FPGA bitstream
>>>>> into
>>>>> the
>>>>> u-boot-with-spl.sfp on Arria10. This lets U-Boot operate in a
>>>>> very
>>>>> similar fashion to Gen5, where the U-Boot binary got loaded by
>>>>> the
>>>>> SPL from a uImage concatenated at the end of the SPL SFP image.
>>>>> On
>>>>> Gen10, the U-Boot is in fitImage which contains the FPGA
>>>>> bitstream
>>>>> as well. In this case, the SPL can load the FPGA bitstream
>>>>> first
>>>>> and
>>>>> load the U-Boot afterward in the same manner. This is
>>>>> nonetheless a
>>>>> stopgap measure until there is a proper firmware loader in U-
>>>>> Boot.
>>>> Right, this is a stopgap measure until FW loader is present. Why
>>>> is
>>>> this
>>>> patch needed at all in this series ?
>>> This patch is cherry picked from the sdmmc_next custodian, so this
>>> patch is required for generating FIT image. I can remove the
>>> stopgap
>>> comment to avoid confusing.
>> But why is this patch needed at all ? You use the firmware loader to
>> load the FPGA bitstream. Where does the fitImage come into play ?
>>
>> The fitImage was used to circumvent the missing FW loader, when I
>> needed
>> to load multiple files (bitstream and u-boot binary). Now there is no
>> such requirement anymore, so the entire fitImage machinery is
>> probably
>> not needed ?
> Loading issue is not the reason we choose the fitImage. We choose it
> because it allows more flexibility in handling various type images,
> especially it allows user more choices to enhance integrity and
> security protection.

Do you need to load multiple images at all ? Do you need the extra
flexibility or does it only bloat and slow down the boot process for no
benefit at all? If a user needs it, they can enable it, but do we need
it by default ?

> Although we plan to use fitImage as our default implementation, but
> this series of patches are still allow fw loading individual bitstream
> image in filesystem partition. So, it is up to user to made the choice.

Right, so is the fitImage needed at all ?

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 1/9] ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

[Chee, Tien Fong]

On Mon, 2018-11-26 at 12:15 +0100, Marek Vasut wrote:
> On 11/26/2018 10:44 AM, Chee, Tien Fong wrote:
> > 
> > On Fri, 2018-11-23 at 13:23 +0100, Marek Vasut wrote:
> > > 
> > > On 11/23/2018 10:19 AM, Chee, Tien Fong wrote:
> > > > 
> > > > 
> > > > On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote:
> > > > > 
> > > > > 
> > > > > On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > > > 
> > > > > > This patch adds description on properties about file name
> > > > > > used
> > > > > > for
> > > > > > both
> > > > > > peripheral bitstream and core bitstream.
> > > > > > 
> > > > > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > > > ---
> > > > > >  .../fpga/altera-socfpga-a10-fpga-mgr.txt           |    6
> > > > > > ++++++
> > > > > >  1 files changed, 6 insertions(+), 0 deletions(-)
> > > > > > 
> > > > > > diff --git a/doc/device-tree-bindings/fpga/altera-socfpga-
> > > > > > a10-
> > > > > > fpga-
> > > > > > mgr.txt b/doc/device-tree-bindings/fpga/altera-socfpga-a10-
> > > > > > fpga-
> > > > > > mgr.txt
> > > > > > index 2fd8e7a..010322a 100644
> > > > > > --- a/doc/device-tree-bindings/fpga/altera-socfpga-a10-
> > > > > > fpga-
> > > > > > mgr.txt
> > > > > > +++ b/doc/device-tree-bindings/fpga/altera-socfpga-a10-
> > > > > > fpga-
> > > > > > mgr.txt
> > > > > > @@ -7,6 +7,10 @@ Required properties:
> > > > > >                 - The second index is for writing FPGA
> > > > > > configuration data.
> > > > > >  - resets     : Phandle and reset specifier for the
> > > > > > device's
> > > > > > reset.
> > > > > >  - clocks     : Clocks used by the device.
> > > > > > +- altr,bitstream_periph : File name for FPGA peripheral
> > > > > > raw
> > > > > > binary
> > > > > > which is used
> > > > > > +			  to initialize FPGA IOs, PLL,
> > > > > > IO48
> > > > > > and
> > > > > > DDR.
> > > > > > +- altr,bitstream_core : File name for core raw binary
> > > > > > which
> > > > > > contains FPGA design
> > > > > > +			which is used to program FPGA CRAM
> > > > > > and
> > > > > > ERAM.
> > > > > bitstream- instead of bitstream_
> > > > Noted.
> > > > > 
> > > > > 
> > > > > 
> > > > > btw can we get something that works with full bitstream too ?
> > > > This patchset actually support the full bitstream too,
> > > > unfortunately it
> > > > is blocked by hardware MPFE issue. The patchset for the MPFE
> > > > workaround
> > > > would come after this patchset. I would advice to use the early
> > > > IO
> > > > release method for the sake of performance.
> > > > 
> > > > For details of issue, you can read the from the link https://gi
> > > > thub
> > > > .com
> > > > /altera-opensource/u-boot-
> > > > socfpga/commits/socfpga_v2014.10_arria10_brin
> > > > gup
> > > > FogBugz #410989-6: Masking hardware sequenced warm reset for
> > > > logic
> > > > in…  …
> > > Does that work on ES2 ? I don't think so ...
> > Why you think it doesn't work, using early IO or full rbf? The
> > bitstream limitation? What you see from the print out?
> ES2 can only use full RBF, I don't think this is handled in this
> patchset at all.
i did testing the full rbf loading, but in the end i removed that
portion of codes because it stuck in DDR calibration due to MPFE HW
issue. So, i would put back that portion of codes after MPFE HW
workaround. My plan is to let early IO release up 1st.

Actually ES2 board also support early IO release, just you need ACDS
and SOCEDS version 17.1 onward to rebuild your hardware, and choosing
the early IO release setting inside the tool. We can discuss this more
if you need our help for early IO release RBFs.
> 

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

[Chee, Tien Fong]

On Mon, 2018-11-26 at 12:18 +0100, Marek Vasut wrote:
> On 11/26/2018 11:09 AM, Chee, Tien Fong wrote:
> [...]
> > 
> > > 
> > > > 
> > > > > 
> > > > > > 
> > > > > > 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 ?
> > > > Both periph.rbf or full.rbf are wrapped with mkimage. So, this
> > > > CRC
> > > > checking can be used to check the integrity of the loading
> > > > bitstream
> > > > data against checksum from mkimage. It is off for the sake of
> > > > performance.
> > > Is there a measurable performance degradation ? I presume that's
> > > because
> > > caches are disabled at that point, yes? Enable caches and see if
> > > that
> > > helps.
> > Just logical sense, performance sure getting degraded, especially
> > loading full rbf, but may be not that obvious for periph.rbf
> > because of
> > very small size, i can try to measure. If not much difference, i
> > can
> > enable checking in default.
> Hard numbers are the only relevant argument here, please measure.
> And try it with caches enabled as much as possible, you want users to
> boot fast. Arria10 is particularly annoyingly slow at booting.
sure.
> 
> > 
> > > 
> > > 
> > > > 
> > > > 
> > > > > 
> > > > > 
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > +	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.co
> > > > > > m>
> > > > > >   */
> > > > > >  
> > > > > >  #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.
> > > > PERIPH_RBF and CORE_RBF are the flags, so i can change them to
> > > > enum.
> > > > But above #define are magic content used to identify the
> > > > bistream
> > > > type.
> > > > If above #define are not suitable, what can you suggest?
> > > Maybe you can just align those two to avoid duplication ?
> > What's you means with duplication, they are different thing.
> > How about i change the name to ARRIA10RBF_PERIPH_TYPE
> > and ARRIA10RBF_CORE_TYPE.
> ARRIA10RBF_PERIPH = (PERIPH_RBF << 15) | 1
We can't use the flag PERIPH_RBF(similar TRUE/FALSE) for magic content,
because they are not related each other. Magic content is defined by HW
design.

We identify the type of rbf such as periph, and core through this magic
content within the rbf. After we getting the type, then only we setting
the flag such as PERIPH_RBF to the function.
> 
> same for ... _CORE ... is that a coincidence ?
> 
> [...]
> 

[U-Boot] [PATCH 3/9] spl : socfpga: Implement fpga bitstream loading with socfpga loadfs

[Chee, Tien Fong]

On Mon, 2018-11-26 at 12:20 +0100, Marek Vasut wrote:
> On 11/26/2018 11:10 AM, Chee, Tien Fong wrote:
> > 
> > On Fri, 2018-11-23 at 13:31 +0100, Marek Vasut wrote:
> > > 
> > > On 11/23/2018 10:51 AM, Chee, Tien Fong wrote:
> > > > 
> > > > 
> > > > On Wed, 2018-11-21 at 15:19 +0100, Marek Vasut wrote:
> > > > > 
> > > > > 
> > > > > On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > > > 
> > > > > > Add support for loading FPGA bitstream to get DDR up
> > > > > > running
> > > > > > before
> > > > > > U-Boot is loaded into DDR. Boot device initialization,
> > > > > > generic
> > > > > > firmware
> > > > > > loader and SPL FAT support are required for this whole
> > > > > > mechanism to
> > > > > > work.
> > > > > > 
> > > > > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > > > ---
> > > > > >  arch/arm/mach-socfpga/spl_a10.c |   49
> > > > > > ++++++++++++++++++++++++++++++++++++++-
> > > > > >  common/spl/spl_mmc.c            |    2 +-
> > > > > >  include/mmc.h                   |    1 +
> > > > > >  3 files changed, 50 insertions(+), 2 deletions(-)
> > > > > > 
> > > > > > diff --git a/arch/arm/mach-socfpga/spl_a10.c
> > > > > > b/arch/arm/mach-
> > > > > > socfpga/spl_a10.c
> > > > > > index 3ea64f7..67a4fac 100644
> > > > > > --- a/arch/arm/mach-socfpga/spl_a10.c
> > > > > > +++ b/arch/arm/mach-socfpga/spl_a10.c
> > > > > > @@ -1,6 +1,6 @@
> > > > > >  // SPDX-License-Identifier: GPL-2.0+
> > > > > >  /*
> > > > > > - *  Copyright (C) 2012 Altera Corporation <www.altera.com>
> > > > > > + *  Copyright (C) 2012-2018 Altera Corporation <www.altera
> > > > > > .com
> > > > > > > 
> > > > > > > 
> > > > > >   */
> > > > > >  
> > > > > >  #include <common.h>
> > > > > > @@ -23,6 +23,10 @@
> > > > > >  #include <fdtdec.h>
> > > > > >  #include <watchdog.h>
> > > > > >  #include <asm/arch/pinmux.h>
> > > > > > +#include <asm/arch/fpga_manager.h>
> > > > > > +#include <mmc.h>
> > > > > > +
> > > > > > +#define RBF	0
> > > > > >  
> > > > > >  DECLARE_GLOBAL_DATA_PTR;
> > > > > >  
> > > > > > @@ -73,6 +77,49 @@ void spl_board_init(void)
> > > > > >  	WATCHDOG_RESET();
> > > > > >  
> > > > > >  	arch_early_init_r();
> > > > > > +
> > > > > > +	/* If the full FPGA is already loaded, ie.from
> > > > > > EPCQ,
> > > > > > config fpga pins */
> > > > > > +	if (is_fpgamgr_user_mode()) {
> > > > > > +		config_pins(gd->fdt_blob, "shared");
> > > > > > +		config_pins(gd->fdt_blob, "fpga");
> > > > > > +	} else if (!is_fpgamgr_early_user_mode()) {
> > > > > > +		/* Program IOSSM(early IO release) or full
> > > > > > FPGA */
> > > > > > +		fpga_fs_info fpga_fsinfo;
> > > > > > +		char buf[16 * 1024]
> > > > > > __aligned(ARCH_DMA_MINALIGN);
> > > > > > +		struct spl_boot_device bootdev;
> > > > > > +		int len = 0;
> > > > > > +
> > > > > > +		bootdev.boot_device = spl_boot_device();
> > > > > > +
> > > > > > +		/* Init MMC driver before reading FPGA
> > > > > > bitstream
> > > > > > from flash */
> > > > > > +		if (bootdev.boot_device ==
> > > > > > BOOT_DEVICE_MMC1) {
> > > > > > +			struct mmc *mmc = NULL;
> > > > > > +			int err = 0;
> > > > > > +
> > > > > > +			err = spl_mmc_find_device(&mmc,
> > > > > > bootdev.boot_device);
> > > > > > +			if (err)
> > > > > > +				return;
> > > > > > +
> > > > > > +			err = mmc_init(mmc);
> > > > > I thought all this backend specific stuff would be hidden in
> > > > > the
> > > > > FW
> > > > > loader.
> > > > The backend supported by FW loader is up to generic file system
> > > > interface layer. flash driver init is expected done by SPL/U-
> > > > Boot
> > > > common init sequence framwork or user. Unfortunately, fw loader
> > > > need to
> > > > access flash before init sequence.
> > > This is actually accessing eMMC though , not flash . If we need
> > > this
> > > huge boilerplate code every time we use the FW loader, than the
> > > FW
> > > loader needs fixing. I can understand the spl_boot_device() being
> > > outside of the FW loader, but not the mmc_init() and co.
> > I can explore the posibility of adding the flash int mechanism into
> > the
> > fm loader probe function.
> What do you mean by "flash int" ? Note that we're talking about eMMC
> here, not flash. Unless you mean "backend init" by all that, in which
> case that'd only make sense, thanks.
I means backend init such as MMC, and NAND driver init.
> 

[U-Boot] [PATCH 4/9] ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10

[Chee, Tien Fong]

On Mon, 2018-11-26 at 12:22 +0100, Marek Vasut wrote:
> On 11/26/2018 11:30 AM, Chee, Tien Fong wrote:
> > 
> > On Fri, 2018-11-23 at 13:40 +0100, Marek Vasut wrote:
> > > 
> > > On 11/23/2018 10:54 AM, Chee, Tien Fong wrote:
> > > > 
> > > > 
> > > > On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote:
> > > > > 
> > > > > 
> > > > > On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > > Did you change Author:ship of the patch ?
> > > I believe you did, so please fix that.
> > Very sorry. I din't realize the author name was changed.
> Please be careful next time.
Sure.
> 
> > 
> > > 
> > > 
> > > > 
> > > > 
> > > > > 
> > > > > 
> > > > > > 
> > > > > > 
> > > > > > Bundle U-Boot fitImage containing U-Boot and FPGA bitstream
> > > > > > into
> > > > > > the
> > > > > > u-boot-with-spl.sfp on Arria10. This lets U-Boot operate in
> > > > > > a
> > > > > > very
> > > > > > similar fashion to Gen5, where the U-Boot binary got loaded
> > > > > > by
> > > > > > the
> > > > > > SPL from a uImage concatenated at the end of the SPL SFP
> > > > > > image.
> > > > > > On
> > > > > > Gen10, the U-Boot is in fitImage which contains the FPGA
> > > > > > bitstream
> > > > > > as well. In this case, the SPL can load the FPGA bitstream
> > > > > > first
> > > > > > and
> > > > > > load the U-Boot afterward in the same manner. This is
> > > > > > nonetheless a
> > > > > > stopgap measure until there is a proper firmware loader in
> > > > > > U-
> > > > > > Boot.
> > > > > Right, this is a stopgap measure until FW loader is present.
> > > > > Why
> > > > > is
> > > > > this
> > > > > patch needed at all in this series ?
> > > > This patch is cherry picked from the sdmmc_next custodian, so
> > > > this
> > > > patch is required for generating FIT image. I can remove the
> > > > stopgap
> > > > comment to avoid confusing.
> > > But why is this patch needed at all ? You use the firmware loader
> > > to
> > > load the FPGA bitstream. Where does the fitImage come into play ?
> > > 
> > > The fitImage was used to circumvent the missing FW loader, when I
> > > needed
> > > to load multiple files (bitstream and u-boot binary). Now there
> > > is no
> > > such requirement anymore, so the entire fitImage machinery is
> > > probably
> > > not needed ?
> > Loading issue is not the reason we choose the fitImage. We choose
> > it
> > because it allows more flexibility in handling various type images,
> > especially it allows user more choices to enhance integrity and
> > security protection.
> Do you need to load multiple images at all ? Do you need the extra
> flexibility or does it only bloat and slow down the boot process for
> no
> benefit at all? If a user needs it, they can enable it, but do we
> need
> it by default ?
Okay, then we add in the fitImage support and let user to enable it.
So, without CONFIG_SPL_FIT is defined, then the boot process would be
with individual files such as u-boot-dtb.img instead of u-boot.itb.
> 
> > 
> > Although we plan to use fitImage as our default implementation, but
> > this series of patches are still allow fw loading individual
> > bitstream
> > image in filesystem partition. So, it is up to user to made the
> > choice.
> Right, so is the fitImage needed at all ?
> 

[U-Boot] [PATCH 1/9] ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

[Marek Vasut]

On 11/27/2018 09:45 AM, Chee, Tien Fong wrote:
> On Mon, 2018-11-26 at 12:15 +0100, Marek Vasut wrote:
>> On 11/26/2018 10:44 AM, Chee, Tien Fong wrote:
>>>
>>> On Fri, 2018-11-23 at 13:23 +0100, Marek Vasut wrote:
>>>>
>>>> On 11/23/2018 10:19 AM, Chee, Tien Fong wrote:
>>>>>
>>>>>
>>>>> On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote:
>>>>>>
>>>>>>
>>>>>> On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>>>>>>
>>>>>>> This patch adds description on properties about file name
>>>>>>> used
>>>>>>> for
>>>>>>> both
>>>>>>> peripheral bitstream and core bitstream.
>>>>>>>
>>>>>>> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
>>>>>>> ---
>>>>>>>  .../fpga/altera-socfpga-a10-fpga-mgr.txt           |    6
>>>>>>> ++++++
>>>>>>>  1 files changed, 6 insertions(+), 0 deletions(-)
>>>>>>>
>>>>>>> diff --git a/doc/device-tree-bindings/fpga/altera-socfpga-
>>>>>>> a10-
>>>>>>> fpga-
>>>>>>> mgr.txt b/doc/device-tree-bindings/fpga/altera-socfpga-a10-
>>>>>>> fpga-
>>>>>>> mgr.txt
>>>>>>> index 2fd8e7a..010322a 100644
>>>>>>> --- a/doc/device-tree-bindings/fpga/altera-socfpga-a10-
>>>>>>> fpga-
>>>>>>> mgr.txt
>>>>>>> +++ b/doc/device-tree-bindings/fpga/altera-socfpga-a10-
>>>>>>> fpga-
>>>>>>> mgr.txt
>>>>>>> @@ -7,6 +7,10 @@ Required properties:
>>>>>>>                 - The second index is for writing FPGA
>>>>>>> configuration data.
>>>>>>>  - resets     : Phandle and reset specifier for the
>>>>>>> device's
>>>>>>> reset.
>>>>>>>  - clocks     : Clocks used by the device.
>>>>>>> +- altr,bitstream_periph : File name for FPGA peripheral
>>>>>>> raw
>>>>>>> binary
>>>>>>> which is used
>>>>>>> +			  to initialize FPGA IOs, PLL,
>>>>>>> IO48
>>>>>>> and
>>>>>>> DDR.
>>>>>>> +- altr,bitstream_core : File name for core raw binary
>>>>>>> which
>>>>>>> contains FPGA design
>>>>>>> +			which is used to program FPGA CRAM
>>>>>>> and
>>>>>>> ERAM.
>>>>>> bitstream- instead of bitstream_
>>>>> Noted.
>>>>>>
>>>>>>
>>>>>>
>>>>>> btw can we get something that works with full bitstream too ?
>>>>> This patchset actually support the full bitstream too,
>>>>> unfortunately it
>>>>> is blocked by hardware MPFE issue. The patchset for the MPFE
>>>>> workaround
>>>>> would come after this patchset. I would advice to use the early
>>>>> IO
>>>>> release method for the sake of performance.
>>>>>
>>>>> For details of issue, you can read the from the link https://gi
>>>>> thub
>>>>> .com
>>>>> /altera-opensource/u-boot-
>>>>> socfpga/commits/socfpga_v2014.10_arria10_brin
>>>>> gup
>>>>> FogBugz #410989-6: Masking hardware sequenced warm reset for
>>>>> logic
>>>>> in…  …
>>>> Does that work on ES2 ? I don't think so ...
>>> Why you think it doesn't work, using early IO or full rbf? The
>>> bitstream limitation? What you see from the print out?
>> ES2 can only use full RBF, I don't think this is handled in this
>> patchset at all.
> i did testing the full rbf loading, but in the end i removed that
> portion of codes because it stuck in DDR calibration due to MPFE HW
> issue. So, i would put back that portion of codes after MPFE HW
> workaround. My plan is to let early IO release up 1st.

Can you describe that workaround ? The code worked on the A10ES2 kit I
have back around v2018.09, so what's the problem ?

> Actually ES2 board also support early IO release, just you need ACDS
> and SOCEDS version 17.1 onward to rebuild your hardware, and choosing
> the early IO release setting inside the tool. We can discuss this more
> if you need our help for early IO release RBFs.

I do, otherwise I won't be able to test anything, so make sure this is
supported.

-- 
Best regards,
Marek Vasut

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

[Marek Vasut]

On 11/27/2018 09:54 AM, Chee, Tien Fong wrote:
> On Mon, 2018-11-26 at 12:18 +0100, Marek Vasut wrote:
>> On 11/26/2018 11:09 AM, Chee, Tien Fong wrote:
>> [...]
>>>
>>>>
>>>>>
>>>>>>
>>>>>>>
>>>>>>> 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 ?
>>>>> Both periph.rbf or full.rbf are wrapped with mkimage. So, this
>>>>> CRC
>>>>> checking can be used to check the integrity of the loading
>>>>> bitstream
>>>>> data against checksum from mkimage. It is off for the sake of
>>>>> performance.
>>>> Is there a measurable performance degradation ? I presume that's
>>>> because
>>>> caches are disabled at that point, yes? Enable caches and see if
>>>> that
>>>> helps.
>>> Just logical sense, performance sure getting degraded, especially
>>> loading full rbf, but may be not that obvious for periph.rbf
>>> because of
>>> very small size, i can try to measure. If not much difference, i
>>> can
>>> enable checking in default.
>> Hard numbers are the only relevant argument here, please measure.
>> And try it with caches enabled as much as possible, you want users to
>> boot fast. Arria10 is particularly annoyingly slow at booting.
> sure.
>>
>>>
>>>>
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> +	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.co
>>>>>>> m>
>>>>>>>   */
>>>>>>>  
>>>>>>>  #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.
>>>>> PERIPH_RBF and CORE_RBF are the flags, so i can change them to
>>>>> enum.
>>>>> But above #define are magic content used to identify the
>>>>> bistream
>>>>> type.
>>>>> If above #define are not suitable, what can you suggest?
>>>> Maybe you can just align those two to avoid duplication ?
>>> What's you means with duplication, they are different thing.
>>> How about i change the name to ARRIA10RBF_PERIPH_TYPE
>>> and ARRIA10RBF_CORE_TYPE.
>> ARRIA10RBF_PERIPH = (PERIPH_RBF << 15) | 1
> We can't use the flag PERIPH_RBF(similar TRUE/FALSE) for magic content,
> because they are not related each other. Magic content is defined by HW
> design.

You can define the flags to match the HW design, which is probably a
good idea ?

> We identify the type of rbf such as periph, and core through this magic
> content within the rbf. After we getting the type, then only we setting
> the flag such as PERIPH_RBF to the function.
>>
>> same for ... _CORE ... is that a coincidence ?
>>
>> [...]


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 3/9] spl : socfpga: Implement fpga bitstream loading with socfpga loadfs

[Marek Vasut]

On 11/27/2018 09:55 AM, Chee, Tien Fong wrote:
> On Mon, 2018-11-26 at 12:20 +0100, Marek Vasut wrote:
>> On 11/26/2018 11:10 AM, Chee, Tien Fong wrote:
>>>
>>> On Fri, 2018-11-23 at 13:31 +0100, Marek Vasut wrote:
>>>>
>>>> On 11/23/2018 10:51 AM, Chee, Tien Fong wrote:
>>>>>
>>>>>
>>>>> On Wed, 2018-11-21 at 15:19 +0100, Marek Vasut wrote:
>>>>>>
>>>>>>
>>>>>> On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>>>>>>
>>>>>>> Add support for loading FPGA bitstream to get DDR up
>>>>>>> running
>>>>>>> before
>>>>>>> U-Boot is loaded into DDR. Boot device initialization,
>>>>>>> generic
>>>>>>> firmware
>>>>>>> loader and SPL FAT support are required for this whole
>>>>>>> mechanism to
>>>>>>> work.
>>>>>>>
>>>>>>> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
>>>>>>> ---
>>>>>>>  arch/arm/mach-socfpga/spl_a10.c |   49
>>>>>>> ++++++++++++++++++++++++++++++++++++++-
>>>>>>>  common/spl/spl_mmc.c            |    2 +-
>>>>>>>  include/mmc.h                   |    1 +
>>>>>>>  3 files changed, 50 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/arm/mach-socfpga/spl_a10.c
>>>>>>> b/arch/arm/mach-
>>>>>>> socfpga/spl_a10.c
>>>>>>> index 3ea64f7..67a4fac 100644
>>>>>>> --- a/arch/arm/mach-socfpga/spl_a10.c
>>>>>>> +++ b/arch/arm/mach-socfpga/spl_a10.c
>>>>>>> @@ -1,6 +1,6 @@
>>>>>>>  // SPDX-License-Identifier: GPL-2.0+
>>>>>>>  /*
>>>>>>> - *  Copyright (C) 2012 Altera Corporation <www.altera.com>
>>>>>>> + *  Copyright (C) 2012-2018 Altera Corporation <www.altera
>>>>>>> .com
>>>>>>>>
>>>>>>>>
>>>>>>>   */
>>>>>>>  
>>>>>>>  #include <common.h>
>>>>>>> @@ -23,6 +23,10 @@
>>>>>>>  #include <fdtdec.h>
>>>>>>>  #include <watchdog.h>
>>>>>>>  #include <asm/arch/pinmux.h>
>>>>>>> +#include <asm/arch/fpga_manager.h>
>>>>>>> +#include <mmc.h>
>>>>>>> +
>>>>>>> +#define RBF	0
>>>>>>>  
>>>>>>>  DECLARE_GLOBAL_DATA_PTR;
>>>>>>>  
>>>>>>> @@ -73,6 +77,49 @@ void spl_board_init(void)
>>>>>>>  	WATCHDOG_RESET();
>>>>>>>  
>>>>>>>  	arch_early_init_r();
>>>>>>> +
>>>>>>> +	/* If the full FPGA is already loaded, ie.from
>>>>>>> EPCQ,
>>>>>>> config fpga pins */
>>>>>>> +	if (is_fpgamgr_user_mode()) {
>>>>>>> +		config_pins(gd->fdt_blob, "shared");
>>>>>>> +		config_pins(gd->fdt_blob, "fpga");
>>>>>>> +	} else if (!is_fpgamgr_early_user_mode()) {
>>>>>>> +		/* Program IOSSM(early IO release) or full
>>>>>>> FPGA */
>>>>>>> +		fpga_fs_info fpga_fsinfo;
>>>>>>> +		char buf[16 * 1024]
>>>>>>> __aligned(ARCH_DMA_MINALIGN);
>>>>>>> +		struct spl_boot_device bootdev;
>>>>>>> +		int len = 0;
>>>>>>> +
>>>>>>> +		bootdev.boot_device = spl_boot_device();
>>>>>>> +
>>>>>>> +		/* Init MMC driver before reading FPGA
>>>>>>> bitstream
>>>>>>> from flash */
>>>>>>> +		if (bootdev.boot_device ==
>>>>>>> BOOT_DEVICE_MMC1) {
>>>>>>> +			struct mmc *mmc = NULL;
>>>>>>> +			int err = 0;
>>>>>>> +
>>>>>>> +			err = spl_mmc_find_device(&mmc,
>>>>>>> bootdev.boot_device);
>>>>>>> +			if (err)
>>>>>>> +				return;
>>>>>>> +
>>>>>>> +			err = mmc_init(mmc);
>>>>>> I thought all this backend specific stuff would be hidden in
>>>>>> the
>>>>>> FW
>>>>>> loader.
>>>>> The backend supported by FW loader is up to generic file system
>>>>> interface layer. flash driver init is expected done by SPL/U-
>>>>> Boot
>>>>> common init sequence framwork or user. Unfortunately, fw loader
>>>>> need to
>>>>> access flash before init sequence.
>>>> This is actually accessing eMMC though , not flash . If we need
>>>> this
>>>> huge boilerplate code every time we use the FW loader, than the
>>>> FW
>>>> loader needs fixing. I can understand the spl_boot_device() being
>>>> outside of the FW loader, but not the mmc_init() and co.
>>> I can explore the posibility of adding the flash int mechanism into
>>> the
>>> fm loader probe function.
>> What do you mean by "flash int" ? Note that we're talking about eMMC
>> here, not flash. Unless you mean "backend init" by all that, in which
>> case that'd only make sense, thanks.
> I means backend init such as MMC, and NAND driver init.

Right, backend init, that should be part of the FW loader.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 4/9] ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10

[Marek Vasut]

On 11/27/2018 10:00 AM, Chee, Tien Fong wrote:
> On Mon, 2018-11-26 at 12:22 +0100, Marek Vasut wrote:
>> On 11/26/2018 11:30 AM, Chee, Tien Fong wrote:
>>>
>>> On Fri, 2018-11-23 at 13:40 +0100, Marek Vasut wrote:
>>>>
>>>> On 11/23/2018 10:54 AM, Chee, Tien Fong wrote:
>>>>>
>>>>>
>>>>> On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote:
>>>>>>
>>>>>>
>>>>>> On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>>>>> Did you change Author:ship of the patch ?
>>>> I believe you did, so please fix that.
>>> Very sorry. I din't realize the author name was changed.
>> Please be careful next time.
> Sure.
>>
>>>
>>>>
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Bundle U-Boot fitImage containing U-Boot and FPGA bitstream
>>>>>>> into
>>>>>>> the
>>>>>>> u-boot-with-spl.sfp on Arria10. This lets U-Boot operate in
>>>>>>> a
>>>>>>> very
>>>>>>> similar fashion to Gen5, where the U-Boot binary got loaded
>>>>>>> by
>>>>>>> the
>>>>>>> SPL from a uImage concatenated at the end of the SPL SFP
>>>>>>> image.
>>>>>>> On
>>>>>>> Gen10, the U-Boot is in fitImage which contains the FPGA
>>>>>>> bitstream
>>>>>>> as well. In this case, the SPL can load the FPGA bitstream
>>>>>>> first
>>>>>>> and
>>>>>>> load the U-Boot afterward in the same manner. This is
>>>>>>> nonetheless a
>>>>>>> stopgap measure until there is a proper firmware loader in
>>>>>>> U-
>>>>>>> Boot.
>>>>>> Right, this is a stopgap measure until FW loader is present.
>>>>>> Why
>>>>>> is
>>>>>> this
>>>>>> patch needed at all in this series ?
>>>>> This patch is cherry picked from the sdmmc_next custodian, so
>>>>> this
>>>>> patch is required for generating FIT image. I can remove the
>>>>> stopgap
>>>>> comment to avoid confusing.
>>>> But why is this patch needed at all ? You use the firmware loader
>>>> to
>>>> load the FPGA bitstream. Where does the fitImage come into play ?
>>>>
>>>> The fitImage was used to circumvent the missing FW loader, when I
>>>> needed
>>>> to load multiple files (bitstream and u-boot binary). Now there
>>>> is no
>>>> such requirement anymore, so the entire fitImage machinery is
>>>> probably
>>>> not needed ?
>>> Loading issue is not the reason we choose the fitImage. We choose
>>> it
>>> because it allows more flexibility in handling various type images,
>>> especially it allows user more choices to enhance integrity and
>>> security protection.
>> Do you need to load multiple images at all ? Do you need the extra
>> flexibility or does it only bloat and slow down the boot process for
>> no
>> benefit at all? If a user needs it, they can enable it, but do we
>> need
>> it by default ?
> Okay, then we add in the fitImage support and let user to enable it.
> So, without CONFIG_SPL_FIT is defined, then the boot process would be
> with individual files such as u-boot-dtb.img instead of u-boot.itb.

Yes, so all these fitImage patches can be dropped for now ?

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 4/9] ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10

[Chee, Tien Fong]

On Tue, 2018-11-27 at 13:09 +0100, Marek Vasut wrote:
> On 11/27/2018 10:00 AM, Chee, Tien Fong wrote:
> > 
> > On Mon, 2018-11-26 at 12:22 +0100, Marek Vasut wrote:
> > > 
> > > On 11/26/2018 11:30 AM, Chee, Tien Fong wrote:
> > > > 
> > > > 
> > > > On Fri, 2018-11-23 at 13:40 +0100, Marek Vasut wrote:
> > > > > 
> > > > > 
> > > > > On 11/23/2018 10:54 AM, Chee, Tien Fong wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > > > > Did you change Author:ship of the patch ?
> > > > > I believe you did, so please fix that.
> > > > Very sorry. I din't realize the author name was changed.
> > > Please be careful next time.
> > Sure.
> > > 
> > > 
> > > > 
> > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > Bundle U-Boot fitImage containing U-Boot and FPGA
> > > > > > > > bitstream
> > > > > > > > into
> > > > > > > > the
> > > > > > > > u-boot-with-spl.sfp on Arria10. This lets U-Boot
> > > > > > > > operate in
> > > > > > > > a
> > > > > > > > very
> > > > > > > > similar fashion to Gen5, where the U-Boot binary got
> > > > > > > > loaded
> > > > > > > > by
> > > > > > > > the
> > > > > > > > SPL from a uImage concatenated at the end of the SPL
> > > > > > > > SFP
> > > > > > > > image.
> > > > > > > > On
> > > > > > > > Gen10, the U-Boot is in fitImage which contains the
> > > > > > > > FPGA
> > > > > > > > bitstream
> > > > > > > > as well. In this case, the SPL can load the FPGA
> > > > > > > > bitstream
> > > > > > > > first
> > > > > > > > and
> > > > > > > > load the U-Boot afterward in the same manner. This is
> > > > > > > > nonetheless a
> > > > > > > > stopgap measure until there is a proper firmware loader
> > > > > > > > in
> > > > > > > > U-
> > > > > > > > Boot.
> > > > > > > Right, this is a stopgap measure until FW loader is
> > > > > > > present.
> > > > > > > Why
> > > > > > > is
> > > > > > > this
> > > > > > > patch needed at all in this series ?
> > > > > > This patch is cherry picked from the sdmmc_next custodian,
> > > > > > so
> > > > > > this
> > > > > > patch is required for generating FIT image. I can remove
> > > > > > the
> > > > > > stopgap
> > > > > > comment to avoid confusing.
> > > > > But why is this patch needed at all ? You use the firmware
> > > > > loader
> > > > > to
> > > > > load the FPGA bitstream. Where does the fitImage come into
> > > > > play ?
> > > > > 
> > > > > The fitImage was used to circumvent the missing FW loader,
> > > > > when I
> > > > > needed
> > > > > to load multiple files (bitstream and u-boot binary). Now
> > > > > there
> > > > > is no
> > > > > such requirement anymore, so the entire fitImage machinery is
> > > > > probably
> > > > > not needed ?
> > > > Loading issue is not the reason we choose the fitImage. We
> > > > choose
> > > > it
> > > > because it allows more flexibility in handling various type
> > > > images,
> > > > especially it allows user more choices to enhance integrity and
> > > > security protection.
> > > Do you need to load multiple images at all ? Do you need the
> > > extra
> > > flexibility or does it only bloat and slow down the boot process
> > > for
> > > no
> > > benefit at all? If a user needs it, they can enable it, but do we
> > > need
> > > it by default ?
> > Okay, then we add in the fitImage support and let user to enable
> > it.
> > So, without CONFIG_SPL_FIT is defined, then the boot process would
> > be
> > with individual files such as u-boot-dtb.img instead of u-boot.itb.
> Yes, so all these fitImage patches can be dropped for now ?
This patch can be dropped. But i don't know it is good idea to reserve
the patch 5-8, this would be easier for user to enable CONFIG_SPL_FIT
in future.
> 

[U-Boot] [PATCH 1/9] ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

[Chee, Tien Fong]

On Tue, 2018-11-27 at 13:07 +0100, Marek Vasut wrote:
> On 11/27/2018 09:45 AM, Chee, Tien Fong wrote:
> > 
> > On Mon, 2018-11-26 at 12:15 +0100, Marek Vasut wrote:
> > > 
> > > On 11/26/2018 10:44 AM, Chee, Tien Fong wrote:
> > > > 
> > > > 
> > > > On Fri, 2018-11-23 at 13:23 +0100, Marek Vasut wrote:
> > > > > 
> > > > > 
> > > > > On 11/23/2018 10:19 AM, Chee, Tien Fong wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > > > > > 
> > > > > > > > This patch adds description on properties about file
> > > > > > > > name
> > > > > > > > used
> > > > > > > > for
> > > > > > > > both
> > > > > > > > peripheral bitstream and core bitstream.
> > > > > > > > 
> > > > > > > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com
> > > > > > > > >
> > > > > > > > ---
> > > > > > > >  .../fpga/altera-socfpga-a10-fpga-
> > > > > > > > mgr.txt           |    6
> > > > > > > > ++++++
> > > > > > > >  1 files changed, 6 insertions(+), 0 deletions(-)
> > > > > > > > 
> > > > > > > > diff --git a/doc/device-tree-bindings/fpga/altera-
> > > > > > > > socfpga-
> > > > > > > > a10-
> > > > > > > > fpga-
> > > > > > > > mgr.txt b/doc/device-tree-bindings/fpga/altera-socfpga-
> > > > > > > > a10-
> > > > > > > > fpga-
> > > > > > > > mgr.txt
> > > > > > > > index 2fd8e7a..010322a 100644
> > > > > > > > --- a/doc/device-tree-bindings/fpga/altera-socfpga-a10-
> > > > > > > > fpga-
> > > > > > > > mgr.txt
> > > > > > > > +++ b/doc/device-tree-bindings/fpga/altera-socfpga-a10-
> > > > > > > > fpga-
> > > > > > > > mgr.txt
> > > > > > > > @@ -7,6 +7,10 @@ Required properties:
> > > > > > > >                 - The second index is for writing FPGA
> > > > > > > > configuration data.
> > > > > > > >  - resets     : Phandle and reset specifier for the
> > > > > > > > device's
> > > > > > > > reset.
> > > > > > > >  - clocks     : Clocks used by the device.
> > > > > > > > +- altr,bitstream_periph : File name for FPGA
> > > > > > > > peripheral
> > > > > > > > raw
> > > > > > > > binary
> > > > > > > > which is used
> > > > > > > > +			  to initialize FPGA IOs, PLL,
> > > > > > > > IO48
> > > > > > > > and
> > > > > > > > DDR.
> > > > > > > > +- altr,bitstream_core : File name for core raw binary
> > > > > > > > which
> > > > > > > > contains FPGA design
> > > > > > > > +			which is used to program FPGA
> > > > > > > > CRAM
> > > > > > > > and
> > > > > > > > ERAM.
> > > > > > > bitstream- instead of bitstream_
> > > > > > Noted.
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > btw can we get something that works with full bitstream
> > > > > > > too ?
> > > > > > This patchset actually support the full bitstream too,
> > > > > > unfortunately it
> > > > > > is blocked by hardware MPFE issue. The patchset for the
> > > > > > MPFE
> > > > > > workaround
> > > > > > would come after this patchset. I would advice to use the
> > > > > > early
> > > > > > IO
> > > > > > release method for the sake of performance.
> > > > > > 
> > > > > > For details of issue, you can read the from the link https:
> > > > > > //gi
> > > > > > thub
> > > > > > .com
> > > > > > /altera-opensource/u-boot-
> > > > > > socfpga/commits/socfpga_v2014.10_arria10_brin
> > > > > > gup
> > > > > > FogBugz #410989-6: Masking hardware sequenced warm reset
> > > > > > for
> > > > > > logic
> > > > > > in…  …
> > > > > Does that work on ES2 ? I don't think so ...
> > > > Why you think it doesn't work, using early IO or full rbf? The
> > > > bitstream limitation? What you see from the print out?
> > > ES2 can only use full RBF, I don't think this is handled in this
> > > patchset at all.
> > i did testing the full rbf loading, but in the end i removed that
> > portion of codes because it stuck in DDR calibration due to MPFE HW
> > issue. So, i would put back that portion of codes after MPFE HW
> > workaround. My plan is to let early IO release up 1st.
> Can you describe that workaround ? The code worked on the A10ES2 kit
> I
> have back around v2018.09, so what's the problem ?
> 
> > 
> > Actually ES2 board also support early IO release, just you need
> > ACDS
> > and SOCEDS version 17.1 onward to rebuild your hardware, and
> > choosing
> > the early IO release setting inside the tool. We can discuss this
> > more
> > if you need our help for early IO release RBFs.
> I do, otherwise I won't be able to test anything, so make sure this
> is
> supported.
Great, i would send out seprate email discussion with you, and inviting
our aplication and hardware engineer into the pool.

By the way, how you get the full rbf or you build it yourself? Do you
have the hardware design? What are the tools you have? what version?
> 

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

[Chee, Tien Fong]

On Tue, 2018-11-27 at 13:08 +0100, Marek Vasut wrote:
> On 11/27/2018 09:54 AM, Chee, Tien Fong wrote:
> > 
> > On Mon, 2018-11-26 at 12:18 +0100, Marek Vasut wrote:
> > > 
> > > On 11/26/2018 11:09 AM, Chee, Tien Fong wrote:
> > > [...]
> > > > 
> > > > 
> > > > > 
> > > > > 
> > > > > > 
> > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 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
> > > > > > > ?
> > > > > > Both periph.rbf or full.rbf are wrapped with mkimage. So,
> > > > > > this
> > > > > > CRC
> > > > > > checking can be used to check the integrity of the loading
> > > > > > bitstream
> > > > > > data against checksum from mkimage. It is off for the sake
> > > > > > of
> > > > > > performance.
> > > > > Is there a measurable performance degradation ? I presume
> > > > > that's
> > > > > because
> > > > > caches are disabled at that point, yes? Enable caches and see
> > > > > if
> > > > > that
> > > > > helps.
> > > > Just logical sense, performance sure getting degraded,
> > > > especially
> > > > loading full rbf, but may be not that obvious for periph.rbf
> > > > because of
> > > > very small size, i can try to measure. If not much difference,
> > > > i
> > > > can
> > > > enable checking in default.
> > > Hard numbers are the only relevant argument here, please measure.
> > > And try it with caches enabled as much as possible, you want
> > > users to
> > > boot fast. Arria10 is particularly annoyingly slow at booting.
> > sure.
> > > 
> > > 
> > > > 
> > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > +	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.inte
> > > > > > > > l.co
> > > > > > > > m>
> > > > > > > >   */
> > > > > > > >  
> > > > > > > >  #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.
> > > > > > PERIPH_RBF and CORE_RBF are the flags, so i can change them
> > > > > > to
> > > > > > enum.
> > > > > > But above #define are magic content used to identify the
> > > > > > bistream
> > > > > > type.
> > > > > > If above #define are not suitable, what can you suggest?
> > > > > Maybe you can just align those two to avoid duplication ?
> > > > What's you means with duplication, they are different thing.
> > > > How about i change the name to ARRIA10RBF_PERIPH_TYPE
> > > > and ARRIA10RBF_CORE_TYPE.
> > > ARRIA10RBF_PERIPH = (PERIPH_RBF << 15) | 1
> > We can't use the flag PERIPH_RBF(similar TRUE/FALSE) for magic
> > content,
> > because they are not related each other. Magic content is defined
> > by HW
> > design.
> You can define the flags to match the HW design, which is probably a
> good idea ?
I have no strong opinion of this, i can do it.
> 
> > 
> > We identify the type of rbf such as periph, and core through this
> > magic
> > content within the rbf. After we getting the type, then only we
> > setting
> > the flag such as PERIPH_RBF to the function.
> > > 
> > > 
> > > same for ... _CORE ... is that a coincidence ?
> > > 
> > > [...]
> 

[U-Boot] [PATCH 1/9] ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

[Marek Vasut]

On 11/28/2018 03:49 PM, Chee, Tien Fong wrote:
> On Tue, 2018-11-27 at 13:07 +0100, Marek Vasut wrote:
>> On 11/27/2018 09:45 AM, Chee, Tien Fong wrote:
>>>
>>> On Mon, 2018-11-26 at 12:15 +0100, Marek Vasut wrote:
>>>>
>>>> On 11/26/2018 10:44 AM, Chee, Tien Fong wrote:
>>>>>
>>>>>
>>>>> On Fri, 2018-11-23 at 13:23 +0100, Marek Vasut wrote:
>>>>>>
>>>>>>
>>>>>> On 11/23/2018 10:19 AM, Chee, Tien Fong wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>>>>>>>>
>>>>>>>>> This patch adds description on properties about file
>>>>>>>>> name
>>>>>>>>> used
>>>>>>>>> for
>>>>>>>>> both
>>>>>>>>> peripheral bitstream and core bitstream.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com
>>>>>>>>>>
>>>>>>>>> ---
>>>>>>>>>  .../fpga/altera-socfpga-a10-fpga-
>>>>>>>>> mgr.txt           |    6
>>>>>>>>> ++++++
>>>>>>>>>  1 files changed, 6 insertions(+), 0 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/doc/device-tree-bindings/fpga/altera-
>>>>>>>>> socfpga-
>>>>>>>>> a10-
>>>>>>>>> fpga-
>>>>>>>>> mgr.txt b/doc/device-tree-bindings/fpga/altera-socfpga-
>>>>>>>>> a10-
>>>>>>>>> fpga-
>>>>>>>>> mgr.txt
>>>>>>>>> index 2fd8e7a..010322a 100644
>>>>>>>>> --- a/doc/device-tree-bindings/fpga/altera-socfpga-a10-
>>>>>>>>> fpga-
>>>>>>>>> mgr.txt
>>>>>>>>> +++ b/doc/device-tree-bindings/fpga/altera-socfpga-a10-
>>>>>>>>> fpga-
>>>>>>>>> mgr.txt
>>>>>>>>> @@ -7,6 +7,10 @@ Required properties:
>>>>>>>>>                 - The second index is for writing FPGA
>>>>>>>>> configuration data.
>>>>>>>>>  - resets     : Phandle and reset specifier for the
>>>>>>>>> device's
>>>>>>>>> reset.
>>>>>>>>>  - clocks     : Clocks used by the device.
>>>>>>>>> +- altr,bitstream_periph : File name for FPGA
>>>>>>>>> peripheral
>>>>>>>>> raw
>>>>>>>>> binary
>>>>>>>>> which is used
>>>>>>>>> +			  to initialize FPGA IOs, PLL,
>>>>>>>>> IO48
>>>>>>>>> and
>>>>>>>>> DDR.
>>>>>>>>> +- altr,bitstream_core : File name for core raw binary
>>>>>>>>> which
>>>>>>>>> contains FPGA design
>>>>>>>>> +			which is used to program FPGA
>>>>>>>>> CRAM
>>>>>>>>> and
>>>>>>>>> ERAM.
>>>>>>>> bitstream- instead of bitstream_
>>>>>>> Noted.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> btw can we get something that works with full bitstream
>>>>>>>> too ?
>>>>>>> This patchset actually support the full bitstream too,
>>>>>>> unfortunately it
>>>>>>> is blocked by hardware MPFE issue. The patchset for the
>>>>>>> MPFE
>>>>>>> workaround
>>>>>>> would come after this patchset. I would advice to use the
>>>>>>> early
>>>>>>> IO
>>>>>>> release method for the sake of performance.
>>>>>>>
>>>>>>> For details of issue, you can read the from the link https:
>>>>>>> //gi
>>>>>>> thub
>>>>>>> .com
>>>>>>> /altera-opensource/u-boot-
>>>>>>> socfpga/commits/socfpga_v2014.10_arria10_brin
>>>>>>> gup
>>>>>>> FogBugz #410989-6: Masking hardware sequenced warm reset
>>>>>>> for
>>>>>>> logic
>>>>>>> in…  …
>>>>>> Does that work on ES2 ? I don't think so ...
>>>>> Why you think it doesn't work, using early IO or full rbf? The
>>>>> bitstream limitation? What you see from the print out?
>>>> ES2 can only use full RBF, I don't think this is handled in this
>>>> patchset at all.
>>> i did testing the full rbf loading, but in the end i removed that
>>> portion of codes because it stuck in DDR calibration due to MPFE HW
>>> issue. So, i would put back that portion of codes after MPFE HW
>>> workaround. My plan is to let early IO release up 1st.
>> Can you describe that workaround ? The code worked on the A10ES2 kit
>> I
>> have back around v2018.09, so what's the problem ?
>>
>>>
>>> Actually ES2 board also support early IO release, just you need
>>> ACDS
>>> and SOCEDS version 17.1 onward to rebuild your hardware, and
>>> choosing
>>> the early IO release setting inside the tool. We can discuss this
>>> more
>>> if you need our help for early IO release RBFs.
>> I do, otherwise I won't be able to test anything, so make sure this
>> is
>> supported.
> Great, i would send out seprate email discussion with you, and inviting
> our aplication and hardware engineer into the pool.
> 
> By the way, how you get the full rbf or you build it yourself? Do you
> have the hardware design? What are the tools you have? what version?

I'm using the GHRD from altera wiki.

-- 
Best regards,
Marek Vasut

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

[Marek Vasut]

On 11/28/2018 03:53 PM, Chee, Tien Fong wrote:
> On Tue, 2018-11-27 at 13:08 +0100, Marek Vasut wrote:
>> On 11/27/2018 09:54 AM, Chee, Tien Fong wrote:
>>>
>>> On Mon, 2018-11-26 at 12:18 +0100, Marek Vasut wrote:
>>>>
>>>> On 11/26/2018 11:09 AM, Chee, Tien Fong wrote:
>>>> [...]
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 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
>>>>>>>> ?
>>>>>>> Both periph.rbf or full.rbf are wrapped with mkimage. So,
>>>>>>> this
>>>>>>> CRC
>>>>>>> checking can be used to check the integrity of the loading
>>>>>>> bitstream
>>>>>>> data against checksum from mkimage. It is off for the sake
>>>>>>> of
>>>>>>> performance.
>>>>>> Is there a measurable performance degradation ? I presume
>>>>>> that's
>>>>>> because
>>>>>> caches are disabled at that point, yes? Enable caches and see
>>>>>> if
>>>>>> that
>>>>>> helps.
>>>>> Just logical sense, performance sure getting degraded,
>>>>> especially
>>>>> loading full rbf, but may be not that obvious for periph.rbf
>>>>> because of
>>>>> very small size, i can try to measure. If not much difference,
>>>>> i
>>>>> can
>>>>> enable checking in default.
>>>> Hard numbers are the only relevant argument here, please measure.
>>>> And try it with caches enabled as much as possible, you want
>>>> users to
>>>> boot fast. Arria10 is particularly annoyingly slow at booting.
>>> sure.
>>>>
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> +	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.inte
>>>>>>>>> l.co
>>>>>>>>> m>
>>>>>>>>>   */
>>>>>>>>>  
>>>>>>>>>  #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.
>>>>>>> PERIPH_RBF and CORE_RBF are the flags, so i can change them
>>>>>>> to
>>>>>>> enum.
>>>>>>> But above #define are magic content used to identify the
>>>>>>> bistream
>>>>>>> type.
>>>>>>> If above #define are not suitable, what can you suggest?
>>>>>> Maybe you can just align those two to avoid duplication ?
>>>>> What's you means with duplication, they are different thing.
>>>>> How about i change the name to ARRIA10RBF_PERIPH_TYPE
>>>>> and ARRIA10RBF_CORE_TYPE.
>>>> ARRIA10RBF_PERIPH = (PERIPH_RBF << 15) | 1
>>> We can't use the flag PERIPH_RBF(similar TRUE/FALSE) for magic
>>> content,
>>> because they are not related each other. Magic content is defined
>>> by HW
>>> design.
>> You can define the flags to match the HW design, which is probably a
>> good idea ?
> I have no strong opinion of this, i can do it.

If it can deduplicate things, that'd be good.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 4/9] ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10

[Marek Vasut]

On 11/28/2018 03:43 PM, Chee, Tien Fong wrote:
> On Tue, 2018-11-27 at 13:09 +0100, Marek Vasut wrote:
>> On 11/27/2018 10:00 AM, Chee, Tien Fong wrote:
>>>
>>> On Mon, 2018-11-26 at 12:22 +0100, Marek Vasut wrote:
>>>>
>>>> On 11/26/2018 11:30 AM, Chee, Tien Fong wrote:
>>>>>
>>>>>
>>>>> On Fri, 2018-11-23 at 13:40 +0100, Marek Vasut wrote:
>>>>>>
>>>>>>
>>>>>> On 11/23/2018 10:54 AM, Chee, Tien Fong wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 11/21/2018 11:41 AM, tien.fong.chee at intel.com wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>>>>>>> Did you change Author:ship of the patch ?
>>>>>> I believe you did, so please fix that.
>>>>> Very sorry. I din't realize the author name was changed.
>>>> Please be careful next time.
>>> Sure.
>>>>
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Bundle U-Boot fitImage containing U-Boot and FPGA
>>>>>>>>> bitstream
>>>>>>>>> into
>>>>>>>>> the
>>>>>>>>> u-boot-with-spl.sfp on Arria10. This lets U-Boot
>>>>>>>>> operate in
>>>>>>>>> a
>>>>>>>>> very
>>>>>>>>> similar fashion to Gen5, where the U-Boot binary got
>>>>>>>>> loaded
>>>>>>>>> by
>>>>>>>>> the
>>>>>>>>> SPL from a uImage concatenated at the end of the SPL
>>>>>>>>> SFP
>>>>>>>>> image.
>>>>>>>>> On
>>>>>>>>> Gen10, the U-Boot is in fitImage which contains the
>>>>>>>>> FPGA
>>>>>>>>> bitstream
>>>>>>>>> as well. In this case, the SPL can load the FPGA
>>>>>>>>> bitstream
>>>>>>>>> first
>>>>>>>>> and
>>>>>>>>> load the U-Boot afterward in the same manner. This is
>>>>>>>>> nonetheless a
>>>>>>>>> stopgap measure until there is a proper firmware loader
>>>>>>>>> in
>>>>>>>>> U-
>>>>>>>>> Boot.
>>>>>>>> Right, this is a stopgap measure until FW loader is
>>>>>>>> present.
>>>>>>>> Why
>>>>>>>> is
>>>>>>>> this
>>>>>>>> patch needed at all in this series ?
>>>>>>> This patch is cherry picked from the sdmmc_next custodian,
>>>>>>> so
>>>>>>> this
>>>>>>> patch is required for generating FIT image. I can remove
>>>>>>> the
>>>>>>> stopgap
>>>>>>> comment to avoid confusing.
>>>>>> But why is this patch needed at all ? You use the firmware
>>>>>> loader
>>>>>> to
>>>>>> load the FPGA bitstream. Where does the fitImage come into
>>>>>> play ?
>>>>>>
>>>>>> The fitImage was used to circumvent the missing FW loader,
>>>>>> when I
>>>>>> needed
>>>>>> to load multiple files (bitstream and u-boot binary). Now
>>>>>> there
>>>>>> is no
>>>>>> such requirement anymore, so the entire fitImage machinery is
>>>>>> probably
>>>>>> not needed ?
>>>>> Loading issue is not the reason we choose the fitImage. We
>>>>> choose
>>>>> it
>>>>> because it allows more flexibility in handling various type
>>>>> images,
>>>>> especially it allows user more choices to enhance integrity and
>>>>> security protection.
>>>> Do you need to load multiple images at all ? Do you need the
>>>> extra
>>>> flexibility or does it only bloat and slow down the boot process
>>>> for
>>>> no
>>>> benefit at all? If a user needs it, they can enable it, but do we
>>>> need
>>>> it by default ?
>>> Okay, then we add in the fitImage support and let user to enable
>>> it.
>>> So, without CONFIG_SPL_FIT is defined, then the boot process would
>>> be
>>> with individual files such as u-boot-dtb.img instead of u-boot.itb.
>> Yes, so all these fitImage patches can be dropped for now ?
> This patch can be dropped. But i don't know it is good idea to reserve
> the patch 5-8, this would be easier for user to enable CONFIG_SPL_FIT
> in future.

Drop them for now, let's revisit this later.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 1/9] ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

[Chee, Tien Fong]

On Wed, 2018-11-28 at 16:10 +0100, Marek Vasut wrote:
> On 11/28/2018 03:49 PM, Chee, Tien Fong wrote:
> > 
> > On Tue, 2018-11-27 at 13:07 +0100, Marek Vasut wrote:
> > > 
> > > On 11/27/2018 09:45 AM, Chee, Tien Fong wrote:
> > > > 
> > > > 
> > > > On Mon, 2018-11-26 at 12:15 +0100, Marek Vasut wrote:
> > > > > 
> > > > > 
> > > > > On 11/26/2018 10:44 AM, Chee, Tien Fong wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On Fri, 2018-11-23 at 13:23 +0100, Marek Vasut wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > On 11/23/2018 10:19 AM, Chee, Tien Fong wrote:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote:
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > On 11/21/2018 11:41 AM, tien.fong.chee at intel.com
> > > > > > > > > wrote:
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > > > > > > > 
> > > > > > > > > > This patch adds description on properties about
> > > > > > > > > > file
> > > > > > > > > > name
> > > > > > > > > > used
> > > > > > > > > > for
> > > > > > > > > > both
> > > > > > > > > > peripheral bitstream and core bitstream.
> > > > > > > > > > 
> > > > > > > > > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel
> > > > > > > > > > .com
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > ---
> > > > > > > > > >  .../fpga/altera-socfpga-a10-fpga-
> > > > > > > > > > mgr.txt           |    6
> > > > > > > > > > ++++++
> > > > > > > > > >  1 files changed, 6 insertions(+), 0 deletions(-)
> > > > > > > > > > 
> > > > > > > > > > diff --git a/doc/device-tree-bindings/fpga/altera-
> > > > > > > > > > socfpga-
> > > > > > > > > > a10-
> > > > > > > > > > fpga-
> > > > > > > > > > mgr.txt b/doc/device-tree-bindings/fpga/altera-
> > > > > > > > > > socfpga-
> > > > > > > > > > a10-
> > > > > > > > > > fpga-
> > > > > > > > > > mgr.txt
> > > > > > > > > > index 2fd8e7a..010322a 100644
> > > > > > > > > > --- a/doc/device-tree-bindings/fpga/altera-socfpga-
> > > > > > > > > > a10-
> > > > > > > > > > fpga-
> > > > > > > > > > mgr.txt
> > > > > > > > > > +++ b/doc/device-tree-bindings/fpga/altera-socfpga-
> > > > > > > > > > a10-
> > > > > > > > > > fpga-
> > > > > > > > > > mgr.txt
> > > > > > > > > > @@ -7,6 +7,10 @@ Required properties:
> > > > > > > > > >                 - The second index is for writing
> > > > > > > > > > FPGA
> > > > > > > > > > configuration data.
> > > > > > > > > >  - resets     : Phandle and reset specifier for the
> > > > > > > > > > device's
> > > > > > > > > > reset.
> > > > > > > > > >  - clocks     : Clocks used by the device.
> > > > > > > > > > +- altr,bitstream_periph : File name for FPGA
> > > > > > > > > > peripheral
> > > > > > > > > > raw
> > > > > > > > > > binary
> > > > > > > > > > which is used
> > > > > > > > > > +			  to initialize FPGA IOs,
> > > > > > > > > > PLL,
> > > > > > > > > > IO48
> > > > > > > > > > and
> > > > > > > > > > DDR.
> > > > > > > > > > +- altr,bitstream_core : File name for core raw
> > > > > > > > > > binary
> > > > > > > > > > which
> > > > > > > > > > contains FPGA design
> > > > > > > > > > +			which is used to program
> > > > > > > > > > FPGA
> > > > > > > > > > CRAM
> > > > > > > > > > and
> > > > > > > > > > ERAM.
> > > > > > > > > bitstream- instead of bitstream_
> > > > > > > > Noted.
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > btw can we get something that works with full
> > > > > > > > > bitstream
> > > > > > > > > too ?
> > > > > > > > This patchset actually support the full bitstream too,
> > > > > > > > unfortunately it
> > > > > > > > is blocked by hardware MPFE issue. The patchset for the
> > > > > > > > MPFE
> > > > > > > > workaround
> > > > > > > > would come after this patchset. I would advice to use
> > > > > > > > the
> > > > > > > > early
> > > > > > > > IO
> > > > > > > > release method for the sake of performance.
> > > > > > > > 
> > > > > > > > For details of issue, you can read the from the
> > > > > > > > link https:
> > > > > > > > //gi
> > > > > > > > thub
> > > > > > > > .com
> > > > > > > > /altera-opensource/u-boot-
> > > > > > > > socfpga/commits/socfpga_v2014.10_arria10_brin
> > > > > > > > gup
> > > > > > > > FogBugz #410989-6: Masking hardware sequenced warm
> > > > > > > > reset
> > > > > > > > for
> > > > > > > > logic
> > > > > > > > in…  …
> > > > > > > Does that work on ES2 ? I don't think so ...
> > > > > > Why you think it doesn't work, using early IO or full rbf?
> > > > > > The
> > > > > > bitstream limitation? What you see from the print out?
> > > > > ES2 can only use full RBF, I don't think this is handled in
> > > > > this
> > > > > patchset at all.
> > > > i did testing the full rbf loading, but in the end i removed
> > > > that
> > > > portion of codes because it stuck in DDR calibration due to
> > > > MPFE HW
> > > > issue. So, i would put back that portion of codes after MPFE HW
> > > > workaround. My plan is to let early IO release up 1st.
> > > Can you describe that workaround ? The code worked on the A10ES2
> > > kit
> > > I
> > > have back around v2018.09, so what's the problem ?
> > > 
> > > > 
> > > > 
> > > > Actually ES2 board also support early IO release, just you need
> > > > ACDS
> > > > and SOCEDS version 17.1 onward to rebuild your hardware, and
> > > > choosing
> > > > the early IO release setting inside the tool. We can discuss
> > > > this
> > > > more
> > > > if you need our help for early IO release RBFs.
> > > I do, otherwise I won't be able to test anything, so make sure
> > > this
> > > is
> > > supported.
> > Great, i would send out seprate email discussion with you, and
> > inviting
> > our aplication and hardware engineer into the pool.
> > 
> > By the way, how you get the full rbf or you build it yourself? Do
> > you
> > have the hardware design? What are the tools you have? what
> > version?
> I'm using the GHRD from altera wiki.
Do you have the link?
> 

[U-Boot] [PATCH 1/9] ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

[Chee, Tien Fong]

On Wed, 2018-11-28 at 16:10 +0100, Marek Vasut wrote:
> On 11/28/2018 03:49 PM, Chee, Tien Fong wrote:
> > 
> > On Tue, 2018-11-27 at 13:07 +0100, Marek Vasut wrote:
> > > 
> > > On 11/27/2018 09:45 AM, Chee, Tien Fong wrote:
> > > > 
> > > > 
> > > > On Mon, 2018-11-26 at 12:15 +0100, Marek Vasut wrote:
> > > > > 
> > > > > 
> > > > > On 11/26/2018 10:44 AM, Chee, Tien Fong wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On Fri, 2018-11-23 at 13:23 +0100, Marek Vasut wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > On 11/23/2018 10:19 AM, Chee, Tien Fong wrote:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote:
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > On 11/21/2018 11:41 AM, tien.fong.chee at intel.com
> > > > > > > > > wrote:
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > > > > > > > 
> > > > > > > > > > This patch adds description on properties about
> > > > > > > > > > file
> > > > > > > > > > name
> > > > > > > > > > used
> > > > > > > > > > for
> > > > > > > > > > both
> > > > > > > > > > peripheral bitstream and core bitstream.
> > > > > > > > > > 
> > > > > > > > > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel
> > > > > > > > > > .com
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > ---
> > > > > > > > > >  .../fpga/altera-socfpga-a10-fpga-
> > > > > > > > > > mgr.txt           |    6
> > > > > > > > > > ++++++
> > > > > > > > > >  1 files changed, 6 insertions(+), 0 deletions(-)
> > > > > > > > > > 
> > > > > > > > > > diff --git a/doc/device-tree-bindings/fpga/altera-
> > > > > > > > > > socfpga-
> > > > > > > > > > a10-
> > > > > > > > > > fpga-
> > > > > > > > > > mgr.txt b/doc/device-tree-bindings/fpga/altera-
> > > > > > > > > > socfpga-
> > > > > > > > > > a10-
> > > > > > > > > > fpga-
> > > > > > > > > > mgr.txt
> > > > > > > > > > index 2fd8e7a..010322a 100644
> > > > > > > > > > --- a/doc/device-tree-bindings/fpga/altera-socfpga-
> > > > > > > > > > a10-
> > > > > > > > > > fpga-
> > > > > > > > > > mgr.txt
> > > > > > > > > > +++ b/doc/device-tree-bindings/fpga/altera-socfpga-
> > > > > > > > > > a10-
> > > > > > > > > > fpga-
> > > > > > > > > > mgr.txt
> > > > > > > > > > @@ -7,6 +7,10 @@ Required properties:
> > > > > > > > > >                 - The second index is for writing
> > > > > > > > > > FPGA
> > > > > > > > > > configuration data.
> > > > > > > > > >  - resets     : Phandle and reset specifier for the
> > > > > > > > > > device's
> > > > > > > > > > reset.
> > > > > > > > > >  - clocks     : Clocks used by the device.
> > > > > > > > > > +- altr,bitstream_periph : File name for FPGA
> > > > > > > > > > peripheral
> > > > > > > > > > raw
> > > > > > > > > > binary
> > > > > > > > > > which is used
> > > > > > > > > > +			  to initialize FPGA IOs,
> > > > > > > > > > PLL,
> > > > > > > > > > IO48
> > > > > > > > > > and
> > > > > > > > > > DDR.
> > > > > > > > > > +- altr,bitstream_core : File name for core raw
> > > > > > > > > > binary
> > > > > > > > > > which
> > > > > > > > > > contains FPGA design
> > > > > > > > > > +			which is used to program
> > > > > > > > > > FPGA
> > > > > > > > > > CRAM
> > > > > > > > > > and
> > > > > > > > > > ERAM.
> > > > > > > > > bitstream- instead of bitstream_
> > > > > > > > Noted.
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > btw can we get something that works with full
> > > > > > > > > bitstream
> > > > > > > > > too ?
> > > > > > > > This patchset actually support the full bitstream too,
> > > > > > > > unfortunately it
> > > > > > > > is blocked by hardware MPFE issue. The patchset for the
> > > > > > > > MPFE
> > > > > > > > workaround
> > > > > > > > would come after this patchset. I would advice to use
> > > > > > > > the
> > > > > > > > early
> > > > > > > > IO
> > > > > > > > release method for the sake of performance.
> > > > > > > > 
> > > > > > > > For details of issue, you can read the from the
> > > > > > > > link https:
> > > > > > > > //gi
> > > > > > > > thub
> > > > > > > > .com
> > > > > > > > /altera-opensource/u-boot-
> > > > > > > > socfpga/commits/socfpga_v2014.10_arria10_brin
> > > > > > > > gup
> > > > > > > > FogBugz #410989-6: Masking hardware sequenced warm
> > > > > > > > reset
> > > > > > > > for
> > > > > > > > logic
> > > > > > > > in…  …
> > > > > > > Does that work on ES2 ? I don't think so ...
> > > > > > Why you think it doesn't work, using early IO or full rbf?
> > > > > > The
> > > > > > bitstream limitation? What you see from the print out?
> > > > > ES2 can only use full RBF, I don't think this is handled in
> > > > > this
> > > > > patchset at all.
> > > > i did testing the full rbf loading, but in the end i removed
> > > > that
> > > > portion of codes because it stuck in DDR calibration due to
> > > > MPFE HW
> > > > issue. So, i would put back that portion of codes after MPFE HW
> > > > workaround. My plan is to let early IO release up 1st.
> > > Can you describe that workaround ? The code worked on the A10ES2
> > > kit
> > > I
> > > have back around v2018.09, so what's the problem ?
There is a corruption to MPFE NoC(which function like ethernet QOS, but
its main function for priotizing and controlling traffic to access DDR
from MPU and FPGA) due to high fequency transient clock out from HPS
EMIF IOPLL at VCO startup. The corruption happens intermittent on some
boards. The workaround is to trigger warm reset to recover MPFE NoC
from corruption after programing the periph rbf or full rbf. Once U-
Boot reentrance after warm reset, only core rbf is allowed to
configured FPGA.
> > > 
> > > > 
> > > > 
> > > > Actually ES2 board also support early IO release, just you need
> > > > ACDS
> > > > and SOCEDS version 17.1 onward to rebuild your hardware, and
> > > > choosing
> > > > the early IO release setting inside the tool. We can discuss
> > > > this
> > > > more
> > > > if you need our help for early IO release RBFs.
> > > I do, otherwise I won't be able to test anything, so make sure
> > > this
> > > is
> > > supported.
> > Great, i would send out seprate email discussion with you, and
> > inviting
> > our aplication and hardware engineer into the pool.
> > 
> > By the way, how you get the full rbf or you build it yourself? Do
> > you
> > have the hardware design? What are the tools you have? what
> > version?
> I'm using the GHRD from altera wiki.
> 

[U-Boot] [PATCH 1/9] ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

[Marek Vasut]

On 11/28/2018 05:17 PM, Chee, Tien Fong wrote:
> On Wed, 2018-11-28 at 16:10 +0100, Marek Vasut wrote:
>> On 11/28/2018 03:49 PM, Chee, Tien Fong wrote:
>>>
>>> On Tue, 2018-11-27 at 13:07 +0100, Marek Vasut wrote:
>>>>
>>>> On 11/27/2018 09:45 AM, Chee, Tien Fong wrote:
>>>>>
>>>>>
>>>>> On Mon, 2018-11-26 at 12:15 +0100, Marek Vasut wrote:
>>>>>>
>>>>>>
>>>>>> On 11/26/2018 10:44 AM, Chee, Tien Fong wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Fri, 2018-11-23 at 13:23 +0100, Marek Vasut wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 11/23/2018 10:19 AM, Chee, Tien Fong wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 11/21/2018 11:41 AM, tien.fong.chee at intel.com
>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>>>>>>>>>>
>>>>>>>>>>> This patch adds description on properties about
>>>>>>>>>>> file
>>>>>>>>>>> name
>>>>>>>>>>> used
>>>>>>>>>>> for
>>>>>>>>>>> both
>>>>>>>>>>> peripheral bitstream and core bitstream.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel
>>>>>>>>>>> .com
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>> ---
>>>>>>>>>>>  .../fpga/altera-socfpga-a10-fpga-
>>>>>>>>>>> mgr.txt           |    6
>>>>>>>>>>> ++++++
>>>>>>>>>>>  1 files changed, 6 insertions(+), 0 deletions(-)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/doc/device-tree-bindings/fpga/altera-
>>>>>>>>>>> socfpga-
>>>>>>>>>>> a10-
>>>>>>>>>>> fpga-
>>>>>>>>>>> mgr.txt b/doc/device-tree-bindings/fpga/altera-
>>>>>>>>>>> socfpga-
>>>>>>>>>>> a10-
>>>>>>>>>>> fpga-
>>>>>>>>>>> mgr.txt
>>>>>>>>>>> index 2fd8e7a..010322a 100644
>>>>>>>>>>> --- a/doc/device-tree-bindings/fpga/altera-socfpga-
>>>>>>>>>>> a10-
>>>>>>>>>>> fpga-
>>>>>>>>>>> mgr.txt
>>>>>>>>>>> +++ b/doc/device-tree-bindings/fpga/altera-socfpga-
>>>>>>>>>>> a10-
>>>>>>>>>>> fpga-
>>>>>>>>>>> mgr.txt
>>>>>>>>>>> @@ -7,6 +7,10 @@ Required properties:
>>>>>>>>>>>                 - The second index is for writing
>>>>>>>>>>> FPGA
>>>>>>>>>>> configuration data.
>>>>>>>>>>>  - resets     : Phandle and reset specifier for the
>>>>>>>>>>> device's
>>>>>>>>>>> reset.
>>>>>>>>>>>  - clocks     : Clocks used by the device.
>>>>>>>>>>> +- altr,bitstream_periph : File name for FPGA
>>>>>>>>>>> peripheral
>>>>>>>>>>> raw
>>>>>>>>>>> binary
>>>>>>>>>>> which is used
>>>>>>>>>>> +			  to initialize FPGA IOs,
>>>>>>>>>>> PLL,
>>>>>>>>>>> IO48
>>>>>>>>>>> and
>>>>>>>>>>> DDR.
>>>>>>>>>>> +- altr,bitstream_core : File name for core raw
>>>>>>>>>>> binary
>>>>>>>>>>> which
>>>>>>>>>>> contains FPGA design
>>>>>>>>>>> +			which is used to program
>>>>>>>>>>> FPGA
>>>>>>>>>>> CRAM
>>>>>>>>>>> and
>>>>>>>>>>> ERAM.
>>>>>>>>>> bitstream- instead of bitstream_
>>>>>>>>> Noted.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> btw can we get something that works with full
>>>>>>>>>> bitstream
>>>>>>>>>> too ?
>>>>>>>>> This patchset actually support the full bitstream too,
>>>>>>>>> unfortunately it
>>>>>>>>> is blocked by hardware MPFE issue. The patchset for the
>>>>>>>>> MPFE
>>>>>>>>> workaround
>>>>>>>>> would come after this patchset. I would advice to use
>>>>>>>>> the
>>>>>>>>> early
>>>>>>>>> IO
>>>>>>>>> release method for the sake of performance.
>>>>>>>>>
>>>>>>>>> For details of issue, you can read the from the
>>>>>>>>> link https:
>>>>>>>>> //gi
>>>>>>>>> thub
>>>>>>>>> .com
>>>>>>>>> /altera-opensource/u-boot-
>>>>>>>>> socfpga/commits/socfpga_v2014.10_arria10_brin
>>>>>>>>> gup
>>>>>>>>> FogBugz #410989-6: Masking hardware sequenced warm
>>>>>>>>> reset
>>>>>>>>> for
>>>>>>>>> logic
>>>>>>>>> in…  …
>>>>>>>> Does that work on ES2 ? I don't think so ...
>>>>>>> Why you think it doesn't work, using early IO or full rbf?
>>>>>>> The
>>>>>>> bitstream limitation? What you see from the print out?
>>>>>> ES2 can only use full RBF, I don't think this is handled in
>>>>>> this
>>>>>> patchset at all.
>>>>> i did testing the full rbf loading, but in the end i removed
>>>>> that
>>>>> portion of codes because it stuck in DDR calibration due to
>>>>> MPFE HW
>>>>> issue. So, i would put back that portion of codes after MPFE HW
>>>>> workaround. My plan is to let early IO release up 1st.
>>>> Can you describe that workaround ? The code worked on the A10ES2
>>>> kit
>>>> I
>>>> have back around v2018.09, so what's the problem ?
> There is a corruption to MPFE NoC(which function like ethernet QOS, but
> its main function for priotizing and controlling traffic to access DDR
> from MPU and FPGA) due to high fequency transient clock out from HPS
> EMIF IOPLL at VCO startup. The corruption happens intermittent on some
> boards. The workaround is to trigger warm reset to recover MPFE NoC
> from corruption after programing the periph rbf or full rbf. Once U-
> Boot reentrance after warm reset, only core rbf is allowed to
> configured FPGA.

And this is present in ES1 or ES2 ? Or is it also in PS ?

I wonder whether we want to scrap the ES support afterall.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 1/9] ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

[Chee, Tien Fong]

On Wed, 2018-11-28 at 18:55 +0100, Marek Vasut wrote:
> On 11/28/2018 05:17 PM, Chee, Tien Fong wrote:
> > 
> > On Wed, 2018-11-28 at 16:10 +0100, Marek Vasut wrote:
> > > 
> > > On 11/28/2018 03:49 PM, Chee, Tien Fong wrote:
> > > > 
> > > > 
> > > > On Tue, 2018-11-27 at 13:07 +0100, Marek Vasut wrote:
> > > > > 
> > > > > 
> > > > > On 11/27/2018 09:45 AM, Chee, Tien Fong wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On Mon, 2018-11-26 at 12:15 +0100, Marek Vasut wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > On 11/26/2018 10:44 AM, Chee, Tien Fong wrote:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > On Fri, 2018-11-23 at 13:23 +0100, Marek Vasut wrote:
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > On 11/23/2018 10:19 AM, Chee, Tien Fong wrote:
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut
> > > > > > > > > > wrote:
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > On 11/21/2018 11:41 AM, tien.fong.chee at intel.com
> > > > > > > > > > > wrote:
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > > > > > > > > > 
> > > > > > > > > > > > This patch adds description on properties about
> > > > > > > > > > > > file
> > > > > > > > > > > > name
> > > > > > > > > > > > used
> > > > > > > > > > > > for
> > > > > > > > > > > > both
> > > > > > > > > > > > peripheral bitstream and core bitstream.
> > > > > > > > > > > > 
> > > > > > > > > > > > Signed-off-by: Tien Fong Chee <tien.fong.chee@i
> > > > > > > > > > > > ntel
> > > > > > > > > > > > .com
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > ---
> > > > > > > > > > > >  .../fpga/altera-socfpga-a10-fpga-
> > > > > > > > > > > > mgr.txt           |    6
> > > > > > > > > > > > ++++++
> > > > > > > > > > > >  1 files changed, 6 insertions(+), 0
> > > > > > > > > > > > deletions(-)
> > > > > > > > > > > > 
> > > > > > > > > > > > diff --git a/doc/device-tree-
> > > > > > > > > > > > bindings/fpga/altera-
> > > > > > > > > > > > socfpga-
> > > > > > > > > > > > a10-
> > > > > > > > > > > > fpga-
> > > > > > > > > > > > mgr.txt b/doc/device-tree-bindings/fpga/altera-
> > > > > > > > > > > > socfpga-
> > > > > > > > > > > > a10-
> > > > > > > > > > > > fpga-
> > > > > > > > > > > > mgr.txt
> > > > > > > > > > > > index 2fd8e7a..010322a 100644
> > > > > > > > > > > > --- a/doc/device-tree-bindings/fpga/altera-
> > > > > > > > > > > > socfpga-
> > > > > > > > > > > > a10-
> > > > > > > > > > > > fpga-
> > > > > > > > > > > > mgr.txt
> > > > > > > > > > > > +++ b/doc/device-tree-bindings/fpga/altera-
> > > > > > > > > > > > socfpga-
> > > > > > > > > > > > a10-
> > > > > > > > > > > > fpga-
> > > > > > > > > > > > mgr.txt
> > > > > > > > > > > > @@ -7,6 +7,10 @@ Required properties:
> > > > > > > > > > > >                 - The second index is for
> > > > > > > > > > > > writing
> > > > > > > > > > > > FPGA
> > > > > > > > > > > > configuration data.
> > > > > > > > > > > >  - resets     : Phandle and reset specifier for
> > > > > > > > > > > > the
> > > > > > > > > > > > device's
> > > > > > > > > > > > reset.
> > > > > > > > > > > >  - clocks     : Clocks used by the device.
> > > > > > > > > > > > +- altr,bitstream_periph : File name for FPGA
> > > > > > > > > > > > peripheral
> > > > > > > > > > > > raw
> > > > > > > > > > > > binary
> > > > > > > > > > > > which is used
> > > > > > > > > > > > +			  to initialize FPGA
> > > > > > > > > > > > IOs,
> > > > > > > > > > > > PLL,
> > > > > > > > > > > > IO48
> > > > > > > > > > > > and
> > > > > > > > > > > > DDR.
> > > > > > > > > > > > +- altr,bitstream_core : File name for core raw
> > > > > > > > > > > > binary
> > > > > > > > > > > > which
> > > > > > > > > > > > contains FPGA design
> > > > > > > > > > > > +			which is used to
> > > > > > > > > > > > program
> > > > > > > > > > > > FPGA
> > > > > > > > > > > > CRAM
> > > > > > > > > > > > and
> > > > > > > > > > > > ERAM.
> > > > > > > > > > > bitstream- instead of bitstream_
> > > > > > > > > > Noted.
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > btw can we get something that works with full
> > > > > > > > > > > bitstream
> > > > > > > > > > > too ?
> > > > > > > > > > This patchset actually support the full bitstream
> > > > > > > > > > too,
> > > > > > > > > > unfortunately it
> > > > > > > > > > is blocked by hardware MPFE issue. The patchset for
> > > > > > > > > > the
> > > > > > > > > > MPFE
> > > > > > > > > > workaround
> > > > > > > > > > would come after this patchset. I would advice to
> > > > > > > > > > use
> > > > > > > > > > the
> > > > > > > > > > early
> > > > > > > > > > IO
> > > > > > > > > > release method for the sake of performance.
> > > > > > > > > > 
> > > > > > > > > > For details of issue, you can read the from the
> > > > > > > > > > link https:
> > > > > > > > > > //gi
> > > > > > > > > > thub
> > > > > > > > > > .com
> > > > > > > > > > /altera-opensource/u-boot-
> > > > > > > > > > socfpga/commits/socfpga_v2014.10_arria10_brin
> > > > > > > > > > gup
> > > > > > > > > > FogBugz #410989-6: Masking hardware sequenced warm
> > > > > > > > > > reset
> > > > > > > > > > for
> > > > > > > > > > logic
> > > > > > > > > > in…  …
> > > > > > > > > Does that work on ES2 ? I don't think so ...
> > > > > > > > Why you think it doesn't work, using early IO or full
> > > > > > > > rbf?
> > > > > > > > The
> > > > > > > > bitstream limitation? What you see from the print out?
> > > > > > > ES2 can only use full RBF, I don't think this is handled
> > > > > > > in
> > > > > > > this
> > > > > > > patchset at all.
> > > > > > i did testing the full rbf loading, but in the end i
> > > > > > removed
> > > > > > that
> > > > > > portion of codes because it stuck in DDR calibration due to
> > > > > > MPFE HW
> > > > > > issue. So, i would put back that portion of codes after
> > > > > > MPFE HW
> > > > > > workaround. My plan is to let early IO release up 1st.
> > > > > Can you describe that workaround ? The code worked on the
> > > > > A10ES2
> > > > > kit
> > > > > I
> > > > > have back around v2018.09, so what's the problem ?
> > There is a corruption to MPFE NoC(which function like ethernet QOS,
> > but
> > its main function for priotizing and controlling traffic to access
> > DDR
> > from MPU and FPGA) due to high fequency transient clock out from
> > HPS
> > EMIF IOPLL at VCO startup. The corruption happens intermittent on
> > some
> > boards. The workaround is to trigger warm reset to recover MPFE NoC
> > from corruption after programing the periph rbf or full rbf. Once
> > U-
> > Boot reentrance after warm reset, only core rbf is allowed to
> > configured FPGA.
> And this is present in ES1 or ES2 ? Or is it also in PS ?
I have no idea would this present in ES1 or ES2, but this happened in
some PS.
> 
> I wonder whether we want to scrap the ES support afterall.
We actually no longer support ES.
>