[U-Boot] [PATCH v4 0/4] Stratix10 FPGA reconfiguration support

[U-Boot] [PATCH v4 0/4] Stratix10 FPGA reconfiguration support

[chee.hong.ang at intel.com]

From: "Ang, Chee Hong" <chee.hong.ang@intel.com>

Summary of v4 changes:
- Patch 1/4, 2/4 and 4/4 are unchanged
- Patch 3/4:
  - implement socfpga_fpga_add() as global weak function.
  - add arch/arm/mach-socfpga/fpga_device.c to override
    socfpga_fpga_add() for Cyclone/Stratix platforms

v3 patchsets:
https://lists.denx.de/pipermail/u-boot/2018-October/343561.html

Ang, Chee Hong (4):
  arm: socfpga: stratix10: Add macros for mailbox's arguments
  arm: socfpga: stratix10: Add Stratix 10 FPGA Reconfiguration Driver
  arm: socfpga: stratix10: Add Stratix10 FPGA into FPGA device table
  arm: socfpga: stratix10: Enable Stratix10 FPGA Reconfiguration

 arch/arm/mach-socfpga/Makefile                   |   4 +
 arch/arm/mach-socfpga/fpga_device.c              |  59 +++++
 arch/arm/mach-socfpga/include/mach/mailbox_s10.h |   6 +
 arch/arm/mach-socfpga/include/mach/misc.h        |   4 -
 arch/arm/mach-socfpga/misc.c                     |  31 +--
 arch/arm/mach-socfpga/misc_s10.c                 |   2 +
 configs/socfpga_stratix10_defconfig              |   1 +
 drivers/fpga/Kconfig                             |  11 +
 drivers/fpga/Makefile                            |   1 +
 drivers/fpga/altera.c                            |   6 +
 drivers/fpga/stratix10.c                         | 288 +++++++++++++++++++++++
 include/altera.h                                 |   8 +
 12 files changed, 387 insertions(+), 34 deletions(-)
 create mode 100644 arch/arm/mach-socfpga/fpga_device.c
 create mode 100644 drivers/fpga/stratix10.c

-- 
2.7.4

[U-Boot] [PATCH v4 1/4] arm: socfpga: stratix10: Add macros for mailbox's arguments

[chee.hong.ang at intel.com]

From: "Ang, Chee Hong" <chee.hong.ang@intel.com>

Add macros for specifying number of arguments in mailbox command.

Signed-off-by: Ang, Chee Hong <chee.hong.ang@intel.com>
---
 arch/arm/mach-socfpga/include/mach/mailbox_s10.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/mach-socfpga/include/mach/mailbox_s10.h b/arch/arm/mach-socfpga/include/mach/mailbox_s10.h
index 660df35..ae728a5 100644
--- a/arch/arm/mach-socfpga/include/mach/mailbox_s10.h
+++ b/arch/arm/mach-socfpga/include/mach/mailbox_s10.h
@@ -107,6 +107,12 @@ enum ALT_SDM_MBOX_RESP_CODE {
 #define RECONFIG_STATUS_PIN_STATUS			2
 #define RECONFIG_STATUS_SOFTFUNC_STATUS			3
 
+/* Macros for specifying number of arguments in mailbox command */
+#define MBOX_NUM_ARGS(n, b)				(((n) & 0xFF) << (b))
+#define MBOX_DIRECT_COUNT(n)				MBOX_NUM_ARGS((n), 0)
+#define MBOX_ARG_DESC_COUNT(n)				MBOX_NUM_ARGS((n), 8)
+#define MBOX_RESP_DESC_COUNT(n)				MBOX_NUM_ARGS((n), 16)
+
 #define MBOX_CFGSTAT_STATE_IDLE				0x00000000
 #define MBOX_CFGSTAT_STATE_CONFIG			0x10000000
 #define MBOX_CFGSTAT_STATE_FAILACK			0x08000000
-- 
2.7.4

[U-Boot] [PATCH v4 2/4] arm: socfpga: stratix10: Add Stratix 10 FPGA Reconfiguration Driver

[chee.hong.ang at intel.com]

From: "Ang, Chee Hong" <chee.hong.ang@intel.com>

Enable FPGA reconfiguration support for Stratix 10 SoC.

Signed-off-by: Ang, Chee Hong <chee.hong.ang@intel.com>
---
 drivers/fpga/Kconfig     |  11 ++
 drivers/fpga/Makefile    |   1 +
 drivers/fpga/stratix10.c | 288 +++++++++++++++++++++++++++++++++++++++++++++++
 include/altera.h         |   4 +
 4 files changed, 304 insertions(+)
 create mode 100644 drivers/fpga/stratix10.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 50e9019..8f59193 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -31,6 +31,17 @@ config FPGA_CYCLON2
 	  Enable FPGA driver for loading bitstream in BIT and BIN format
 	  on Altera Cyclone II device.
 
+config FPGA_STRATIX10
+	bool "Enable Altera FPGA driver for Stratix 10"
+	depends on TARGET_SOCFPGA_STRATIX10
+	select FPGA_ALTERA
+	help
+	  Say Y here to enable the Altera Stratix 10 FPGA specific driver
+
+	  This provides common functionality for Altera Stratix 10 devices.
+	  Enable FPGA driver for writing bitstream into Altera Stratix10
+	  device.
+
 config FPGA_XILINX
 	bool "Enable Xilinx FPGA drivers"
 	select FPGA
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 97d7d5d..5a778c1 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_FPGA_ACEX1K) += ACEX1K.o
 obj-$(CONFIG_FPGA_CYCLON2) += cyclon2.o
 obj-$(CONFIG_FPGA_STRATIX_II) += stratixII.o
 obj-$(CONFIG_FPGA_STRATIX_V) += stratixv.o
+obj-$(CONFIG_FPGA_STRATIX10) += stratix10.o
 obj-$(CONFIG_FPGA_SOCFPGA) += socfpga.o
 obj-$(CONFIG_TARGET_SOCFPGA_GEN5) += socfpga_gen5.o
 obj-$(CONFIG_TARGET_SOCFPGA_ARRIA10) += socfpga_arria10.o
diff --git a/drivers/fpga/stratix10.c b/drivers/fpga/stratix10.c
new file mode 100644
index 0000000..6728291
--- /dev/null
+++ b/drivers/fpga/stratix10.c
@@ -0,0 +1,288 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018 Intel Corporation <www.intel.com>
+ */
+
+#include <common.h>
+#include <altera.h>
+#include <asm/arch/mailbox_s10.h>
+
+#define RECONFIG_STATUS_POLL_RESP_TIMEOUT_MS		60000
+#define RECONFIG_STATUS_INTERVAL_DELAY_US		1000000
+
+static const struct mbox_cfgstat_state {
+	int			err_no;
+	const char		*error_name;
+} mbox_cfgstat_state[] = {
+	{MBOX_CFGSTAT_STATE_IDLE, "FPGA in idle mode."},
+	{MBOX_CFGSTAT_STATE_CONFIG, "FPGA in config mode."},
+	{MBOX_CFGSTAT_STATE_FAILACK, "Acknowledgment failed!"},
+	{MBOX_CFGSTAT_STATE_ERROR_INVALID, "Invalid bitstream!"},
+	{MBOX_CFGSTAT_STATE_ERROR_CORRUPT, "Corrupted bitstream!"},
+	{MBOX_CFGSTAT_STATE_ERROR_AUTH, "Authentication failed!"},
+	{MBOX_CFGSTAT_STATE_ERROR_CORE_IO, "I/O error!"},
+	{MBOX_CFGSTAT_STATE_ERROR_HARDWARE, "Hardware error!"},
+	{MBOX_CFGSTAT_STATE_ERROR_FAKE, "Fake error!"},
+	{MBOX_CFGSTAT_STATE_ERROR_BOOT_INFO, "Error in boot info!"},
+	{MBOX_CFGSTAT_STATE_ERROR_QSPI_ERROR, "Error in QSPI!"},
+	{MBOX_RESP_ERROR, "Mailbox general error!"},
+	{-ETIMEDOUT, "I/O timeout error"},
+	{-1, "Unknown error!"}
+};
+
+#define MBOX_CFGSTAT_MAX ARRAY_SIZE(mbox_cfgstat_state)
+
+static const char *mbox_cfgstat_to_str(int err)
+{
+	int i;
+
+	for (i = 0; i < MBOX_CFGSTAT_MAX - 1; i++) {
+		if (mbox_cfgstat_state[i].err_no == err)
+			return mbox_cfgstat_state[i].error_name;
+	}
+
+	return mbox_cfgstat_state[MBOX_CFGSTAT_MAX - 1].error_name;
+}
+
+/*
+ * Add the ongoing transaction's command ID into pending list and return
+ * the command ID for next transfer.
+ */
+static u8 add_transfer(u32 *xfer_pending_list, size_t list_size, u8 id)
+{
+	int i;
+
+	for (i = 0; i < list_size; i++) {
+		if (xfer_pending_list[i])
+			continue;
+		xfer_pending_list[i] = id;
+		debug("ID(%d) added to transaction pending list\n", id);
+		/*
+		 * Increment command ID for next transaction.
+		 * Valid command ID (4 bits) is from 1 to 15.
+		 */
+		id = (id % 15) + 1;
+		break;
+	}
+
+	return id;
+}
+
+/*
+ * Check whether response ID match the command ID in the transfer
+ * pending list. If a match is found in the transfer pending list,
+ * it clears the transfer pending list and return the matched
+ * command ID.
+ */
+static int get_and_clr_transfer(u32 *xfer_pending_list, size_t list_size,
+				u8 id)
+{
+	int i;
+
+	for (i = 0; i < list_size; i++) {
+		if (id != xfer_pending_list[i])
+			continue;
+		xfer_pending_list[i] = 0;
+		return id;
+	}
+
+	return 0;
+}
+
+/*
+ * Polling the FPGA configuration status.
+ * Return 0 for success, non-zero for error.
+ */
+static int reconfig_status_polling_resp(void)
+{
+	int ret;
+	unsigned long start = get_timer(0);
+
+	while (1) {
+		ret = mbox_get_fpga_config_status(MBOX_RECONFIG_STATUS);
+		if (!ret)
+			return 0;	/* configuration success */
+
+		if (ret != MBOX_CFGSTAT_STATE_CONFIG)
+			return ret;
+
+		if (get_timer(start) > RECONFIG_STATUS_POLL_RESP_TIMEOUT_MS)
+			break;	/* time out */
+
+		puts(".");
+		udelay(RECONFIG_STATUS_INTERVAL_DELAY_US);
+	}
+
+	return -ETIMEDOUT;
+}
+
+static u32 get_resp_hdr(u32 *r_index, u32 *w_index, u32 *resp_count,
+			u32 *resp_buf, u32 buf_size, u32 client_id)
+{
+	u32 buf[MBOX_RESP_BUFFER_SIZE];
+	u32 mbox_hdr;
+	u32 resp_len;
+	u32 hdr_len;
+	u32 i;
+
+	if (*resp_count < buf_size) {
+		u32 rcv_len_max = buf_size - *resp_count;
+
+		if (rcv_len_max > MBOX_RESP_BUFFER_SIZE)
+			rcv_len_max = MBOX_RESP_BUFFER_SIZE;
+		resp_len = mbox_rcv_resp(buf, rcv_len_max);
+
+		for (i = 0; i < resp_len; i++) {
+			resp_buf[(*w_index)++] = buf[i];
+			*w_index %= buf_size;
+			*resp_count++;
+		}
+	}
+
+	/* No response in buffer */
+	if (*resp_count == 0)
+		return 0;
+
+	mbox_hdr = resp_buf[*r_index];
+
+	hdr_len = MBOX_RESP_LEN_GET(mbox_hdr);
+
+	/* Insufficient header length to return a mailbox header */
+	if ((*resp_count - 1) < hdr_len)
+		return 0;
+
+	*r_index += (hdr_len + 1);
+	*r_index %= buf_size;
+	*resp_count -= (hdr_len + 1);
+
+	/* Make sure response belongs to us */
+	if (MBOX_RESP_CLIENT_GET(mbox_hdr) != client_id)
+		return 0;
+
+	return mbox_hdr;
+}
+
+/* Send bit stream data to SDM via RECONFIG_DATA mailbox command */
+static int send_reconfig_data(const void *rbf_data, size_t rbf_size,
+			      u32 xfer_max, u32 buf_size_max)
+{
+	u32 response_buffer[MBOX_RESP_BUFFER_SIZE];
+	u32 xfer_pending[MBOX_RESP_BUFFER_SIZE];
+	u32 resp_rindex = 0;
+	u32 resp_windex = 0;
+	u32 resp_count = 0;
+	u32 xfer_count = 0;
+	u8 resp_err = 0;
+	u8 cmd_id = 1;
+	u32 args[3];
+	int ret;
+
+	debug("SDM xfer_max = %d\n", xfer_max);
+	debug("SDM buf_size_max = %x\n\n", buf_size_max);
+
+	memset(xfer_pending, 0, sizeof(xfer_pending));
+
+	while (rbf_size || xfer_count) {
+		if (!resp_err && rbf_size && xfer_count < xfer_max) {
+			args[0] = MBOX_ARG_DESC_COUNT(1);
+			args[1] = (u32)rbf_data;
+			if (rbf_size >= buf_size_max) {
+				args[2] = buf_size_max;
+				rbf_size -= buf_size_max;
+				rbf_data += buf_size_max;
+			} else {
+				args[2] = (u32)rbf_size;
+				rbf_size = 0;
+			}
+
+			ret = mbox_send_cmd_only(cmd_id, MBOX_RECONFIG_DATA,
+						 MBOX_CMD_INDIRECT, 3, args);
+			if (ret) {
+				resp_err = 1;
+			} else {
+				xfer_count++;
+				cmd_id = add_transfer(xfer_pending,
+						      MBOX_RESP_BUFFER_SIZE,
+						      cmd_id);
+			}
+			puts(".");
+		} else {
+			u32 resp_hdr = get_resp_hdr(&resp_rindex, &resp_windex,
+						    &resp_count,
+						    response_buffer,
+						    MBOX_RESP_BUFFER_SIZE,
+						    MBOX_CLIENT_ID_UBOOT);
+
+			/*
+			 * If no valid response header found or
+			 * non-zero length from RECONFIG_DATA
+			 */
+			if (!resp_hdr || MBOX_RESP_LEN_GET(resp_hdr))
+				continue;
+
+			/* Check for response's status */
+			if (!resp_err) {
+				ret = MBOX_RESP_ERR_GET(resp_hdr);
+				debug("Response error code: %08x\n", ret);
+				/* Error in response */
+				if (ret)
+					resp_err = 1;
+			}
+
+			ret = get_and_clr_transfer(xfer_pending,
+						   MBOX_RESP_BUFFER_SIZE,
+						   MBOX_RESP_ID_GET(resp_hdr));
+			if (ret) {
+				/* Claim and reuse the ID */
+				cmd_id = (u8)ret;
+				xfer_count--;
+			}
+
+			if (resp_err && !xfer_count)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * This is the interface used by FPGA driver.
+ * Return 0 for success, non-zero for error.
+ */
+int stratix10_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size)
+{
+	int ret;
+	u32 resp_len = 2;
+	u32 resp_buf[2];
+
+	debug("Sending MBOX_RECONFIG...\n");
+	ret = mbox_send_cmd(MBOX_ID_UBOOT, MBOX_RECONFIG, MBOX_CMD_DIRECT, 0,
+			    NULL, 0, &resp_len, resp_buf);
+	if (ret) {
+		puts("Failure in RECONFIG mailbox command!\n");
+		return ret;
+	}
+
+	ret = send_reconfig_data(rbf_data, rbf_size, resp_buf[0], resp_buf[1]);
+	if (ret) {
+		printf("RECONFIG_DATA error: %08x, %s\n", ret,
+		       mbox_cfgstat_to_str(ret));
+		return ret;
+	}
+
+	/* Make sure we don't send MBOX_RECONFIG_STATUS too fast */
+	udelay(RECONFIG_STATUS_INTERVAL_DELAY_US);
+
+	debug("Polling with MBOX_RECONFIG_STATUS...\n");
+	ret = reconfig_status_polling_resp();
+	if (ret) {
+		printf("RECONFIG_STATUS Error: %08x, %s\n", ret,
+		       mbox_cfgstat_to_str(ret));
+		return ret;
+	}
+
+	puts("FPGA reconfiguration OK!\n");
+
+	return ret;
+}
diff --git a/include/altera.h b/include/altera.h
index ead5d3d..233b467 100644
--- a/include/altera.h
+++ b/include/altera.h
@@ -116,4 +116,8 @@ int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size);
 int stratixv_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size);
 #endif
 
+#ifdef CONFIG_FPGA_STRATIX10
+int stratix10_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size);
+#endif
+
 #endif /* _ALTERA_H_ */
-- 
2.7.4

[U-Boot] [PATCH v4 3/4] arm: socfpga: stratix10: Add Stratix10 FPGA into FPGA device table

[chee.hong.ang at intel.com]

From: "Ang, Chee Hong" <chee.hong.ang@intel.com>

Enable 'fpga' command in u-boot. User will be able to use the FPGA
command to program the FPGA on Stratix10 SoC.

Signed-off-by: Ang, Chee Hong <chee.hong.ang@intel.com>
---
 arch/arm/mach-socfpga/Makefile            |  4 +++
 arch/arm/mach-socfpga/fpga_device.c       | 59 +++++++++++++++++++++++++++++++
 arch/arm/mach-socfpga/include/mach/misc.h |  4 ---
 arch/arm/mach-socfpga/misc.c              | 31 +---------------
 arch/arm/mach-socfpga/misc_s10.c          |  2 ++
 drivers/fpga/altera.c                     |  6 ++++
 include/altera.h                          |  4 +++
 7 files changed, 76 insertions(+), 34 deletions(-)
 create mode 100644 arch/arm/mach-socfpga/fpga_device.c

diff --git a/arch/arm/mach-socfpga/Makefile b/arch/arm/mach-socfpga/Makefile
index e667204..2ff1b3f 100644
--- a/arch/arm/mach-socfpga/Makefile
+++ b/arch/arm/mach-socfpga/Makefile
@@ -10,6 +10,10 @@ obj-y	+= clock_manager.o
 obj-y	+= misc.o
 obj-y	+= reset_manager.o
 
+ifdef CONFIG_FPGA
+obj-y	+= fpga_device.o
+endif
+
 ifdef CONFIG_TARGET_SOCFPGA_GEN5
 obj-y	+= clock_manager_gen5.o
 obj-y	+= misc_gen5.o
diff --git a/arch/arm/mach-socfpga/fpga_device.c b/arch/arm/mach-socfpga/fpga_device.c
new file mode 100644
index 0000000..97b27eb
--- /dev/null
+++ b/arch/arm/mach-socfpga/fpga_device.c
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018 Intel Corporation <www.intel.com>
+ */
+
+#include <common.h>
+#include <altera.h>
+
+#ifdef CONFIG_FPGA_STRATIX10
+/*
+ * FPGA programming support for SoC FPGA Stratix 10
+ */
+static Altera_desc altera_fpga[] = {
+	{
+		/* Family */
+		Intel_FPGA_Stratix10,
+		/* Interface type */
+		secure_device_manager_mailbox,
+		/* No limitation as additional data will be ignored */
+		-1,
+		/* No device function table */
+		NULL,
+		/* Base interface address specified in driver */
+		NULL,
+		/* No cookie implementation */
+		0
+	},
+};
+#else
+/*
+ * FPGA programming support for SoC FPGA Cyclone V
+ */
+static Altera_desc altera_fpga[] = {
+	{
+		/* Family */
+		Altera_SoCFPGA,
+		/* Interface type */
+		fast_passive_parallel,
+		/* No limitation as additional data will be ignored */
+		-1,
+		/* No device function table */
+		NULL,
+		/* Base interface address specified in driver */
+		NULL,
+		/* No cookie implementation */
+		0
+	},
+};
+#endif
+
+/* add device descriptor to FPGA device table */
+void socfpga_fpga_add(void)
+{
+	int i;
+
+	fpga_init();
+	for (i = 0; i < ARRAY_SIZE(altera_fpga); i++)
+		fpga_add(fpga_altera, &altera_fpga[i]);
+}
diff --git a/arch/arm/mach-socfpga/include/mach/misc.h b/arch/arm/mach-socfpga/include/mach/misc.h
index 4fc9570..6fa9495 100644
--- a/arch/arm/mach-socfpga/include/mach/misc.h
+++ b/arch/arm/mach-socfpga/include/mach/misc.h
@@ -15,11 +15,7 @@ struct bsel {
 
 extern struct bsel bsel_str[];
 
-#ifdef CONFIG_FPGA
 void socfpga_fpga_add(void);
-#else
-static inline void socfpga_fpga_add(void) {}
-#endif
 
 #ifdef CONFIG_TARGET_SOCFPGA_GEN5
 void socfpga_sdram_remap_zero(void);
diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-socfpga/misc.c
index a4f6d5c..55f846a 100644
--- a/arch/arm/mach-socfpga/misc.c
+++ b/arch/arm/mach-socfpga/misc.c
@@ -87,36 +87,7 @@ int overwrite_console(void)
 }
 #endif
 
-#ifdef CONFIG_FPGA
-/*
- * FPGA programming support for SoC FPGA Cyclone V
- */
-static Altera_desc altera_fpga[] = {
-	{
-		/* Family */
-		Altera_SoCFPGA,
-		/* Interface type */
-		fast_passive_parallel,
-		/* No limitation as additional data will be ignored */
-		-1,
-		/* No device function table */
-		NULL,
-		/* Base interface address specified in driver */
-		NULL,
-		/* No cookie implementation */
-		0
-	},
-};
-
-/* add device descriptor to FPGA device table */
-void socfpga_fpga_add(void)
-{
-	int i;
-	fpga_init();
-	for (i = 0; i < ARRAY_SIZE(altera_fpga); i++)
-		fpga_add(fpga_altera, &altera_fpga[i]);
-}
-#endif
+__weak void socfpga_fpga_add(void) {}
 
 int arch_cpu_init(void)
 {
diff --git a/arch/arm/mach-socfpga/misc_s10.c b/arch/arm/mach-socfpga/misc_s10.c
index e599362..e611d2c 100644
--- a/arch/arm/mach-socfpga/misc_s10.c
+++ b/arch/arm/mach-socfpga/misc_s10.c
@@ -125,6 +125,8 @@ int arch_misc_init(void)
 
 int arch_early_init_r(void)
 {
+	socfpga_fpga_add();
+
 	return 0;
 }
 
diff --git a/drivers/fpga/altera.c b/drivers/fpga/altera.c
index 9605554..7c8f518 100644
--- a/drivers/fpga/altera.c
+++ b/drivers/fpga/altera.c
@@ -39,6 +39,9 @@ static const struct altera_fpga {
 #if defined(CONFIG_FPGA_STRATIX_V)
 	{ Altera_StratixV, "StratixV", stratixv_load, NULL, NULL },
 #endif
+#if defined(CONFIG_FPGA_STRATIX10)
+	{ Intel_FPGA_Stratix10, "Stratix10", stratix10_load, NULL, NULL },
+#endif
 #if defined(CONFIG_FPGA_SOCFPGA)
 	{ Altera_SoCFPGA, "SoC FPGA", socfpga_load, NULL, NULL },
 #endif
@@ -154,6 +157,9 @@ int altera_info(Altera_desc *desc)
 	case fast_passive_parallel_security:
 		printf("Fast Passive Parallel with Security (FPPS)\n");
 		break;
+	case secure_device_manager_mailbox:
+		puts("Secure Device Manager (SDM) Mailbox\n");
+		break;
 		/* Add new interface types here */
 	default:
 		printf("Unsupported interface type, %d\n", desc->iface);
diff --git a/include/altera.h b/include/altera.h
index 233b467..22d55cf 100644
--- a/include/altera.h
+++ b/include/altera.h
@@ -39,6 +39,8 @@ enum altera_iface {
 	fast_passive_parallel,
 	/* fast passive parallel with security (FPPS) */
 	fast_passive_parallel_security,
+	/* secure device manager (SDM) mailbox */
+	secure_device_manager_mailbox,
 	/* insert all new types before this */
 	max_altera_iface_type,
 };
@@ -54,6 +56,8 @@ enum altera_family {
 	Altera_StratixII,
 	/* StratixV Family */
 	Altera_StratixV,
+	/* Stratix10 Family */
+	Intel_FPGA_Stratix10,
 	/* SoCFPGA Family */
 	Altera_SoCFPGA,
 
-- 
2.7.4

[U-Boot] [PATCH v4 4/4] arm: socfpga: stratix10: Enable Stratix10 FPGA Reconfiguration

[chee.hong.ang at intel.com]

From: "Ang, Chee Hong" <chee.hong.ang@intel.com>

Enable Stratix10 FPGA reconfiguration support in defconfig.

Signed-off-by: Ang, Chee Hong <chee.hong.ang@intel.com>
---
 configs/socfpga_stratix10_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/socfpga_stratix10_defconfig b/configs/socfpga_stratix10_defconfig
index 5f3d733..155c406 100644
--- a/configs/socfpga_stratix10_defconfig
+++ b/configs/socfpga_stratix10_defconfig
@@ -10,6 +10,7 @@ CONFIG_NR_DRAM_BANKS=1
 CONFIG_BOOTDELAY=5
 CONFIG_SPL_SPI_LOAD=y
 CONFIG_HUSH_PARSER=y
+CONFIG_FPGA_STRATIX10=y
 CONFIG_SYS_PROMPT="SOCFPGA_STRATIX10 # "
 CONFIG_CMD_MEMTEST=y
 # CONFIG_CMD_FLASH is not set
-- 
2.7.4

[U-Boot] [PATCH v4 3/4] arm: socfpga: stratix10: Add Stratix10 FPGA into FPGA device table

[Simon Goldschmidt]

On Mon, Nov 19, 2018 at 10:46 AM <chee.hong.ang@intel.com> wrote:
>
> From: "Ang, Chee Hong" <chee.hong.ang@intel.com>
>
> Enable 'fpga' command in u-boot. User will be able to use the FPGA
> command to program the FPGA on Stratix10 SoC.
>
> Signed-off-by: Ang, Chee Hong <chee.hong.ang@intel.com>
> ---
>  arch/arm/mach-socfpga/Makefile            |  4 +++
>  arch/arm/mach-socfpga/fpga_device.c       | 59 +++++++++++++++++++++++++++++++
>  arch/arm/mach-socfpga/include/mach/misc.h |  4 ---
>  arch/arm/mach-socfpga/misc.c              | 31 +---------------
>  arch/arm/mach-socfpga/misc_s10.c          |  2 ++
>  drivers/fpga/altera.c                     |  6 ++++
>  include/altera.h                          |  4 +++
>  7 files changed, 76 insertions(+), 34 deletions(-)
>  create mode 100644 arch/arm/mach-socfpga/fpga_device.c
>
> diff --git a/arch/arm/mach-socfpga/Makefile b/arch/arm/mach-socfpga/Makefile
> index e667204..2ff1b3f 100644
> --- a/arch/arm/mach-socfpga/Makefile
> +++ b/arch/arm/mach-socfpga/Makefile
> @@ -10,6 +10,10 @@ obj-y        += clock_manager.o
>  obj-y  += misc.o
>  obj-y  += reset_manager.o
>
> +ifdef CONFIG_FPGA
> +obj-y  += fpga_device.o
> +endif
> +
>  ifdef CONFIG_TARGET_SOCFPGA_GEN5
>  obj-y  += clock_manager_gen5.o
>  obj-y  += misc_gen5.o
> diff --git a/arch/arm/mach-socfpga/fpga_device.c b/arch/arm/mach-socfpga/fpga_device.c
> new file mode 100644
> index 0000000..97b27eb
> --- /dev/null
> +++ b/arch/arm/mach-socfpga/fpga_device.c
> @@ -0,0 +1,59 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2018 Intel Corporation <www.intel.com>
> + */
> +
> +#include <common.h>
> +#include <altera.h>
> +
> +#ifdef CONFIG_FPGA_STRATIX10
> +/*
> + * FPGA programming support for SoC FPGA Stratix 10
> + */
> +static Altera_desc altera_fpga[] = {
> +       {
> +               /* Family */
> +               Intel_FPGA_Stratix10,
> +               /* Interface type */
> +               secure_device_manager_mailbox,
> +               /* No limitation as additional data will be ignored */
> +               -1,
> +               /* No device function table */
> +               NULL,
> +               /* Base interface address specified in driver */
> +               NULL,
> +               /* No cookie implementation */
> +               0
> +       },
> +};
> +#else
> +/*
> + * FPGA programming support for SoC FPGA Cyclone V
> + */
> +static Altera_desc altera_fpga[] = {
> +       {
> +               /* Family */
> +               Altera_SoCFPGA,
> +               /* Interface type */
> +               fast_passive_parallel,
> +               /* No limitation as additional data will be ignored */
> +               -1,
> +               /* No device function table */
> +               NULL,
> +               /* Base interface address specified in driver */
> +               NULL,
> +               /* No cookie implementation */
> +               0
> +       },
> +};
> +#endif
> +
> +/* add device descriptor to FPGA device table */
> +void socfpga_fpga_add(void)
> +{
> +       int i;
> +
> +       fpga_init();
> +       for (i = 0; i < ARRAY_SIZE(altera_fpga); i++)
> +               fpga_add(fpga_altera, &altera_fpga[i]);
> +}
> diff --git a/arch/arm/mach-socfpga/include/mach/misc.h b/arch/arm/mach-socfpga/include/mach/misc.h
> index 4fc9570..6fa9495 100644
> --- a/arch/arm/mach-socfpga/include/mach/misc.h
> +++ b/arch/arm/mach-socfpga/include/mach/misc.h
> @@ -15,11 +15,7 @@ struct bsel {
>
>  extern struct bsel bsel_str[];
>
> -#ifdef CONFIG_FPGA
>  void socfpga_fpga_add(void);
> -#else
> -static inline void socfpga_fpga_add(void) {}
> -#endif
>
>  #ifdef CONFIG_TARGET_SOCFPGA_GEN5
>  void socfpga_sdram_remap_zero(void);
> diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-socfpga/misc.c
> index a4f6d5c..55f846a 100644
> --- a/arch/arm/mach-socfpga/misc.c
> +++ b/arch/arm/mach-socfpga/misc.c
> @@ -87,36 +87,7 @@ int overwrite_console(void)
>  }
>  #endif
>
> -#ifdef CONFIG_FPGA
> -/*
> - * FPGA programming support for SoC FPGA Cyclone V
> - */
> -static Altera_desc altera_fpga[] = {
> -       {
> -               /* Family */
> -               Altera_SoCFPGA,
> -               /* Interface type */
> -               fast_passive_parallel,
> -               /* No limitation as additional data will be ignored */
> -               -1,
> -               /* No device function table */
> -               NULL,
> -               /* Base interface address specified in driver */
> -               NULL,
> -               /* No cookie implementation */
> -               0
> -       },
> -};
> -
> -/* add device descriptor to FPGA device table */
> -void socfpga_fpga_add(void)
> -{
> -       int i;
> -       fpga_init();
> -       for (i = 0; i < ARRAY_SIZE(altera_fpga); i++)
> -               fpga_add(fpga_altera, &altera_fpga[i]);
> -}
> -#endif
> +__weak void socfpga_fpga_add(void) {}

I'm not sure I completely followed the discussion of previous versions
of this series, but is this really the coding style U-Boot wants?
I would have thought weak functions are used for unknown extension
points (multiple architectures or boards), but this is a config option
in a defined set of files. In my opinion, using weak here is not the
right thing to do.

I'd rather add a header file fpga_device.h that declares this function
depending on CONFIG_FPGA.

Simon

>
>  int arch_cpu_init(void)
>  {
> diff --git a/arch/arm/mach-socfpga/misc_s10.c b/arch/arm/mach-socfpga/misc_s10.c
> index e599362..e611d2c 100644
> --- a/arch/arm/mach-socfpga/misc_s10.c
> +++ b/arch/arm/mach-socfpga/misc_s10.c
> @@ -125,6 +125,8 @@ int arch_misc_init(void)
>
>  int arch_early_init_r(void)
>  {
> +       socfpga_fpga_add();
> +
>         return 0;
>  }
>
> diff --git a/drivers/fpga/altera.c b/drivers/fpga/altera.c
> index 9605554..7c8f518 100644
> --- a/drivers/fpga/altera.c
> +++ b/drivers/fpga/altera.c
> @@ -39,6 +39,9 @@ static const struct altera_fpga {
>  #if defined(CONFIG_FPGA_STRATIX_V)
>         { Altera_StratixV, "StratixV", stratixv_load, NULL, NULL },
>  #endif
> +#if defined(CONFIG_FPGA_STRATIX10)
> +       { Intel_FPGA_Stratix10, "Stratix10", stratix10_load, NULL, NULL },
> +#endif
>  #if defined(CONFIG_FPGA_SOCFPGA)
>         { Altera_SoCFPGA, "SoC FPGA", socfpga_load, NULL, NULL },
>  #endif
> @@ -154,6 +157,9 @@ int altera_info(Altera_desc *desc)
>         case fast_passive_parallel_security:
>                 printf("Fast Passive Parallel with Security (FPPS)\n");
>                 break;
> +       case secure_device_manager_mailbox:
> +               puts("Secure Device Manager (SDM) Mailbox\n");
> +               break;
>                 /* Add new interface types here */
>         default:
>                 printf("Unsupported interface type, %d\n", desc->iface);
> diff --git a/include/altera.h b/include/altera.h
> index 233b467..22d55cf 100644
> --- a/include/altera.h
> +++ b/include/altera.h
> @@ -39,6 +39,8 @@ enum altera_iface {
>         fast_passive_parallel,
>         /* fast passive parallel with security (FPPS) */
>         fast_passive_parallel_security,
> +       /* secure device manager (SDM) mailbox */
> +       secure_device_manager_mailbox,
>         /* insert all new types before this */
>         max_altera_iface_type,
>  };
> @@ -54,6 +56,8 @@ enum altera_family {
>         Altera_StratixII,
>         /* StratixV Family */
>         Altera_StratixV,
> +       /* Stratix10 Family */
> +       Intel_FPGA_Stratix10,
>         /* SoCFPGA Family */
>         Altera_SoCFPGA,
>
> --
> 2.7.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v4 3/4] arm: socfpga: stratix10: Add Stratix10 FPGA into FPGA device table

[Marek Vasut]

On 11/19/2018 10:57 AM, Simon Goldschmidt wrote:
> On Mon, Nov 19, 2018 at 10:46 AM <chee.hong.ang@intel.com> wrote:
>>
>> From: "Ang, Chee Hong" <chee.hong.ang@intel.com>
>>
>> Enable 'fpga' command in u-boot. User will be able to use the FPGA
>> command to program the FPGA on Stratix10 SoC.
>>
>> Signed-off-by: Ang, Chee Hong <chee.hong.ang@intel.com>
>> ---
>>  arch/arm/mach-socfpga/Makefile            |  4 +++
>>  arch/arm/mach-socfpga/fpga_device.c       | 59 +++++++++++++++++++++++++++++++
>>  arch/arm/mach-socfpga/include/mach/misc.h |  4 ---
>>  arch/arm/mach-socfpga/misc.c              | 31 +---------------
>>  arch/arm/mach-socfpga/misc_s10.c          |  2 ++
>>  drivers/fpga/altera.c                     |  6 ++++
>>  include/altera.h                          |  4 +++
>>  7 files changed, 76 insertions(+), 34 deletions(-)
>>  create mode 100644 arch/arm/mach-socfpga/fpga_device.c
>>
>> diff --git a/arch/arm/mach-socfpga/Makefile b/arch/arm/mach-socfpga/Makefile
>> index e667204..2ff1b3f 100644
>> --- a/arch/arm/mach-socfpga/Makefile
>> +++ b/arch/arm/mach-socfpga/Makefile
>> @@ -10,6 +10,10 @@ obj-y        += clock_manager.o
>>  obj-y  += misc.o
>>  obj-y  += reset_manager.o
>>
>> +ifdef CONFIG_FPGA
>> +obj-y  += fpga_device.o
>> +endif
>> +
>>  ifdef CONFIG_TARGET_SOCFPGA_GEN5
>>  obj-y  += clock_manager_gen5.o
>>  obj-y  += misc_gen5.o
>> diff --git a/arch/arm/mach-socfpga/fpga_device.c b/arch/arm/mach-socfpga/fpga_device.c
>> new file mode 100644
>> index 0000000..97b27eb
>> --- /dev/null
>> +++ b/arch/arm/mach-socfpga/fpga_device.c
>> @@ -0,0 +1,59 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (C) 2018 Intel Corporation <www.intel.com>
>> + */
>> +
>> +#include <common.h>
>> +#include <altera.h>
>> +
>> +#ifdef CONFIG_FPGA_STRATIX10
>> +/*
>> + * FPGA programming support for SoC FPGA Stratix 10
>> + */
>> +static Altera_desc altera_fpga[] = {
>> +       {
>> +               /* Family */
>> +               Intel_FPGA_Stratix10,
>> +               /* Interface type */
>> +               secure_device_manager_mailbox,
>> +               /* No limitation as additional data will be ignored */
>> +               -1,
>> +               /* No device function table */
>> +               NULL,
>> +               /* Base interface address specified in driver */
>> +               NULL,
>> +               /* No cookie implementation */
>> +               0
>> +       },
>> +};
>> +#else
>> +/*
>> + * FPGA programming support for SoC FPGA Cyclone V
>> + */
>> +static Altera_desc altera_fpga[] = {
>> +       {
>> +               /* Family */
>> +               Altera_SoCFPGA,
>> +               /* Interface type */
>> +               fast_passive_parallel,
>> +               /* No limitation as additional data will be ignored */
>> +               -1,
>> +               /* No device function table */
>> +               NULL,
>> +               /* Base interface address specified in driver */
>> +               NULL,
>> +               /* No cookie implementation */
>> +               0
>> +       },
>> +};
>> +#endif
>> +
>> +/* add device descriptor to FPGA device table */
>> +void socfpga_fpga_add(void)
>> +{
>> +       int i;
>> +
>> +       fpga_init();
>> +       for (i = 0; i < ARRAY_SIZE(altera_fpga); i++)
>> +               fpga_add(fpga_altera, &altera_fpga[i]);
>> +}
>> diff --git a/arch/arm/mach-socfpga/include/mach/misc.h b/arch/arm/mach-socfpga/include/mach/misc.h
>> index 4fc9570..6fa9495 100644
>> --- a/arch/arm/mach-socfpga/include/mach/misc.h
>> +++ b/arch/arm/mach-socfpga/include/mach/misc.h
>> @@ -15,11 +15,7 @@ struct bsel {
>>
>>  extern struct bsel bsel_str[];
>>
>> -#ifdef CONFIG_FPGA
>>  void socfpga_fpga_add(void);
>> -#else
>> -static inline void socfpga_fpga_add(void) {}
>> -#endif
>>
>>  #ifdef CONFIG_TARGET_SOCFPGA_GEN5
>>  void socfpga_sdram_remap_zero(void);
>> diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-socfpga/misc.c
>> index a4f6d5c..55f846a 100644
>> --- a/arch/arm/mach-socfpga/misc.c
>> +++ b/arch/arm/mach-socfpga/misc.c
>> @@ -87,36 +87,7 @@ int overwrite_console(void)
>>  }
>>  #endif
>>
>> -#ifdef CONFIG_FPGA
>> -/*
>> - * FPGA programming support for SoC FPGA Cyclone V
>> - */
>> -static Altera_desc altera_fpga[] = {
>> -       {
>> -               /* Family */
>> -               Altera_SoCFPGA,
>> -               /* Interface type */
>> -               fast_passive_parallel,
>> -               /* No limitation as additional data will be ignored */
>> -               -1,
>> -               /* No device function table */
>> -               NULL,
>> -               /* Base interface address specified in driver */
>> -               NULL,
>> -               /* No cookie implementation */
>> -               0
>> -       },
>> -};
>> -
>> -/* add device descriptor to FPGA device table */
>> -void socfpga_fpga_add(void)
>> -{
>> -       int i;
>> -       fpga_init();
>> -       for (i = 0; i < ARRAY_SIZE(altera_fpga); i++)
>> -               fpga_add(fpga_altera, &altera_fpga[i]);
>> -}
>> -#endif
>> +__weak void socfpga_fpga_add(void) {}
> 
> I'm not sure I completely followed the discussion of previous versions
> of this series, but is this really the coding style U-Boot wants?
> I would have thought weak functions are used for unknown extension
> points (multiple architectures or boards), but this is a config option
> in a defined set of files. In my opinion, using weak here is not the
> right thing to do.
> 
> I'd rather add a header file fpga_device.h that declares this function
> depending on CONFIG_FPGA.

My understanding is that the goal was to deduplicate the function for
Gen5 somehow, but you're right, this looks odd.

This function should always be declared for SoCFPGA, except with
different tables for different FPGAs.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v4 3/4] arm: socfpga: stratix10: Add Stratix10 FPGA into FPGA device table

[Ang, Chee Hong]

On Mon, 2018-11-19 at 14:12 +0100, Marek Vasut wrote:
> On 11/19/2018 10:57 AM, Simon Goldschmidt wrote:
> > 
> > On Mon, Nov 19, 2018 at 10:46 AM <chee.hong.ang@intel.com> wrote:
> > > 
> > > 
> > > From: "Ang, Chee Hong" <chee.hong.ang@intel.com>
> > > 
> > > Enable 'fpga' command in u-boot. User will be able to use the
> > > FPGA
> > > command to program the FPGA on Stratix10 SoC.
> > > 
> > > Signed-off-by: Ang, Chee Hong <chee.hong.ang@intel.com>
> > > ---
> > >  arch/arm/mach-socfpga/Makefile            |  4 +++
> > >  arch/arm/mach-socfpga/fpga_device.c       | 59
> > > +++++++++++++++++++++++++++++++
> > >  arch/arm/mach-socfpga/include/mach/misc.h |  4 ---
> > >  arch/arm/mach-socfpga/misc.c              | 31 +---------------
> > >  arch/arm/mach-socfpga/misc_s10.c          |  2 ++
> > >  drivers/fpga/altera.c                     |  6 ++++
> > >  include/altera.h                          |  4 +++
> > >  7 files changed, 76 insertions(+), 34 deletions(-)
> > >  create mode 100644 arch/arm/mach-socfpga/fpga_device.c
> > > 
> > > diff --git a/arch/arm/mach-socfpga/Makefile b/arch/arm/mach-
> > > socfpga/Makefile
> > > index e667204..2ff1b3f 100644
> > > --- a/arch/arm/mach-socfpga/Makefile
> > > +++ b/arch/arm/mach-socfpga/Makefile
> > > @@ -10,6 +10,10 @@ obj-y        += clock_manager.o
> > >  obj-y  += misc.o
> > >  obj-y  += reset_manager.o
> > > 
> > > +ifdef CONFIG_FPGA
> > > +obj-y  += fpga_device.o
> > > +endif
> > > +
> > >  ifdef CONFIG_TARGET_SOCFPGA_GEN5
> > >  obj-y  += clock_manager_gen5.o
> > >  obj-y  += misc_gen5.o
> > > diff --git a/arch/arm/mach-socfpga/fpga_device.c b/arch/arm/mach-
> > > socfpga/fpga_device.c
> > > new file mode 100644
> > > index 0000000..97b27eb
> > > --- /dev/null
> > > +++ b/arch/arm/mach-socfpga/fpga_device.c
> > > @@ -0,0 +1,59 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright (C) 2018 Intel Corporation <www.intel.com>
> > > + */
> > > +
> > > +#include <common.h>
> > > +#include <altera.h>
> > > +
> > > +#ifdef CONFIG_FPGA_STRATIX10
> > > +/*
> > > + * FPGA programming support for SoC FPGA Stratix 10
> > > + */
> > > +static Altera_desc altera_fpga[] = {
> > > +       {
> > > +               /* Family */
> > > +               Intel_FPGA_Stratix10,
> > > +               /* Interface type */
> > > +               secure_device_manager_mailbox,
> > > +               /* No limitation as additional data will be
> > > ignored */
> > > +               -1,
> > > +               /* No device function table */
> > > +               NULL,
> > > +               /* Base interface address specified in driver */
> > > +               NULL,
> > > +               /* No cookie implementation */
> > > +               0
> > > +       },
> > > +};
> > > +#else
> > > +/*
> > > + * FPGA programming support for SoC FPGA Cyclone V
> > > + */
> > > +static Altera_desc altera_fpga[] = {
> > > +       {
> > > +               /* Family */
> > > +               Altera_SoCFPGA,
> > > +               /* Interface type */
> > > +               fast_passive_parallel,
> > > +               /* No limitation as additional data will be
> > > ignored */
> > > +               -1,
> > > +               /* No device function table */
> > > +               NULL,
> > > +               /* Base interface address specified in driver */
> > > +               NULL,
> > > +               /* No cookie implementation */
> > > +               0
> > > +       },
> > > +};
> > > +#endif
> > > +
> > > +/* add device descriptor to FPGA device table */
> > > +void socfpga_fpga_add(void)
> > > +{
> > > +       int i;
> > > +
> > > +       fpga_init();
> > > +       for (i = 0; i < ARRAY_SIZE(altera_fpga); i++)
> > > +               fpga_add(fpga_altera, &altera_fpga[i]);
> > > +}
> > > diff --git a/arch/arm/mach-socfpga/include/mach/misc.h
> > > b/arch/arm/mach-socfpga/include/mach/misc.h
> > > index 4fc9570..6fa9495 100644
> > > --- a/arch/arm/mach-socfpga/include/mach/misc.h
> > > +++ b/arch/arm/mach-socfpga/include/mach/misc.h
> > > @@ -15,11 +15,7 @@ struct bsel {
> > > 
> > >  extern struct bsel bsel_str[];
> > > 
> > > -#ifdef CONFIG_FPGA
> > >  void socfpga_fpga_add(void);
> > > -#else
> > > -static inline void socfpga_fpga_add(void) {}
> > > -#endif
> > > 
> > >  #ifdef CONFIG_TARGET_SOCFPGA_GEN5
> > >  void socfpga_sdram_remap_zero(void);
> > > diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-
> > > socfpga/misc.c
> > > index a4f6d5c..55f846a 100644
> > > --- a/arch/arm/mach-socfpga/misc.c
> > > +++ b/arch/arm/mach-socfpga/misc.c
> > > @@ -87,36 +87,7 @@ int overwrite_console(void)
> > >  }
> > >  #endif
> > > 
> > > -#ifdef CONFIG_FPGA
> > > -/*
> > > - * FPGA programming support for SoC FPGA Cyclone V
> > > - */
> > > -static Altera_desc altera_fpga[] = {
> > > -       {
> > > -               /* Family */
> > > -               Altera_SoCFPGA,
> > > -               /* Interface type */
> > > -               fast_passive_parallel,
> > > -               /* No limitation as additional data will be
> > > ignored */
> > > -               -1,
> > > -               /* No device function table */
> > > -               NULL,
> > > -               /* Base interface address specified in driver */
> > > -               NULL,
> > > -               /* No cookie implementation */
> > > -               0
> > > -       },
> > > -};
> > > -
> > > -/* add device descriptor to FPGA device table */
> > > -void socfpga_fpga_add(void)
> > > -{
> > > -       int i;
> > > -       fpga_init();
> > > -       for (i = 0; i < ARRAY_SIZE(altera_fpga); i++)
> > > -               fpga_add(fpga_altera, &altera_fpga[i]);
> > > -}
> > > -#endif
> > > +__weak void socfpga_fpga_add(void) {}
> > I'm not sure I completely followed the discussion of previous
> > versions
> > of this series, but is this really the coding style U-Boot wants?
> > I would have thought weak functions are used for unknown extension
> > points (multiple architectures or boards), but this is a config
> > option
> > in a defined set of files. In my opinion, using weak here is not
> > the
> > right thing to do.
> > 
> > I'd rather add a header file fpga_device.h that declares this
> > function
> > depending on CONFIG_FPGA.
> My understanding is that the goal was to deduplicate the function for
> Gen5 somehow, but you're right, this looks odd.
> 
> This function should always be declared for SoCFPGA, except with
> different tables for different FPGAs.
This functions is general to all platforms except it is adding
different table for different platform. But this function is depending
on whether CONFIG_FPGA is defined or not. So if I declare this function
as empty function if CONFIG_FPGA is not defined then it would be
something similar to what I already addressed in previous v3 patchsets.
In v3 patchsets the tables and socfpga_fpga_add() function were already
declared in arch/arm/mach-socfpga/misc.c.
Can you guys take a look at v3 patchsets ?
https://lists.denx.de/pipermail/u-boot/2018-October/343561.html

Thanks.
>