[U-Boot] [PATCH v1 0/3] Stratix10 FPGA reconfiguration support

[U-Boot] [PATCH v1 0/3] Stratix10 FPGA reconfiguration support

[chee.hong.ang at intel.com]

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

This patch enable FPGA reconfiguration for Stratix10 SoC.
This patch works on top of:
https://lists.denx.de/pipermail/u-boot/2018-April/325900.html

Chee Hong Ang (3):
  arm: socfpga: stratix10: Add Stratix10 FPGA Reconfiguration Driver
  arm: socfpga: stratix10: Add Stratix10 FPGA into FPGA device table
  arm: socfpga: stratix10: Enable Stratix10 FPGA Reconfiguration

 arch/arm/mach-socfpga/misc.c        |  20 ++-
 arch/arm/mach-socfpga/misc_s10.c    |   4 +
 configs/socfpga_stratix10_defconfig |   2 +
 drivers/fpga/Kconfig                |  10 ++
 drivers/fpga/Makefile               |   1 +
 drivers/fpga/altera.c               |   6 +
 drivers/fpga/stratix10.c            | 298 ++++++++++++++++++++++++++++++++++++
 include/altera.h                    |   8 +
 8 files changed, 346 insertions(+), 3 deletions(-)
 create mode 100644 drivers/fpga/stratix10.c

-- 
2.2.0

[U-Boot] [PATCH v1 1/3] arm: socfpga: stratix10: Add Stratix10 FPGA Reconfiguration Driver

[chee.hong.ang at intel.com]

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

Enable FPGA reconfiguration support on Stratix10 SoC.

Signed-off-by: Chee Hong Ang <chee.hong.ang@intel.com>
---
 drivers/fpga/Kconfig     |  10 ++
 drivers/fpga/Makefile    |   1 +
 drivers/fpga/stratix10.c | 298 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 309 insertions(+)
 create mode 100644 drivers/fpga/stratix10.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index d36c4c5..255683d 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -31,6 +31,16 @@ 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 FPGA_ALTERA && TARGET_SOCFPGA_STRATIX10
+	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 08c9ff8..3c906ec 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -19,6 +19,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..f0c5ace
--- /dev/null
+++ b/drivers/fpga/stratix10.c
@@ -0,0 +1,298 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2016-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, "Acknowledgement 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!"},
+	{-ETIMEDOUT, "I/O timeout error"},
+	{-1, "Unknown error!"}
+};
+
+#define MBOX_CFGSTAT_MAX \
+	  (sizeof(mbox_cfgstat_state) / sizeof(struct 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;
+}
+
+static void inc_cmd_id(u8 *id)
+{
+	u8 val = *id + 1;
+
+	if (val > 15)
+		val = 1;
+
+	*id = val;
+}
+
+/*
+ * Polling the FPGA configuration status.
+ * Return 0 for success, non-zero for error.
+ */
+static int reconfig_status_polling_resp(void)
+{
+	u32 reconfig_status_resp_len;
+	u32 reconfig_status_resp[RECONFIG_STATUS_RESPONSE_LEN];
+	int ret;
+	unsigned long start = get_timer(0);
+
+	while (1) {
+		reconfig_status_resp_len = RECONFIG_STATUS_RESPONSE_LEN;
+		ret = mbox_send_cmd(MBOX_ID_UBOOT, MBOX_RECONFIG_STATUS,
+				    MBOX_CMD_DIRECT, 0, NULL, 0,
+				    &reconfig_status_resp_len,
+				    reconfig_status_resp);
+
+		if (ret) {
+			puts("Failure in RECONFIG_STATUS mailbox command!\n");
+			return ret;
+		}
+
+		/* Check for any error */
+		ret = reconfig_status_resp[RECONFIG_STATUS_STATE];
+		if (ret && ret != MBOX_CFGSTAT_STATE_CONFIG)
+			return ret;
+
+		/* Make sure nStatus is not 0 */
+		ret = reconfig_status_resp[RECONFIG_STATUS_PIN_STATUS];
+		if (!(ret & RCF_PIN_STATUS_NSTATUS))
+			return MBOX_CFGSTAT_STATE_ERROR_HARDWARE;
+
+		ret = reconfig_status_resp[RECONFIG_STATUS_SOFTFUNC_STATUS];
+		if (ret & RCF_SOFTFUNC_STATUS_SEU_ERROR)
+			return MBOX_CFGSTAT_STATE_ERROR_HARDWARE;
+
+		if ((ret & RCF_SOFTFUNC_STATUS_CONF_DONE) &&
+		    (ret & RCF_SOFTFUNC_STATUS_INIT_DONE) &&
+		    !reconfig_status_resp[RECONFIG_STATUS_STATE])
+			return 0;	/* configuration success */
+
+		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 i;
+	u32 mbox_hdr;
+	u32 resp_len;
+	u32 hdr_len;
+	u32 buf[MBOX_RESP_BUFFER_SIZE];
+
+	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;
+}
+
+static int send_reconfig_data(const void *rbf_data, size_t rbf_size,
+			      u32 xfer_max, u32 buf_size_max)
+{
+	u32 resp_rindex = 0;
+	u32 resp_windex = 0;
+	u32 resp_count = 0;
+	u32 response_buffer[MBOX_RESP_BUFFER_SIZE];
+	u8 resp_err = 0;
+	u8 cmd_id = 1;
+	u32 xfer_count = 0;
+	u32 xfer_pending[MBOX_RESP_BUFFER_SIZE];
+	u32 args[3];
+	int ret = 0;
+	u32 i;
+
+	debug("SDM xfer_max = %d\n", xfer_max);
+	debug("SDM buf_size_max = %x\n\n", buf_size_max);
+
+	for (i = 0; i < MBOX_RESP_BUFFER_SIZE; i++)
+		xfer_pending[i] = 0;
+
+	while (rbf_size || xfer_count) {
+		if (!resp_err && rbf_size && xfer_count < xfer_max) {
+			args[0] = (1 << 8);
+			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;
+			}
+
+			debug("Sending MBOX_RECONFIG_DATA...\n");
+
+			ret = mbox_send_cmd_only(cmd_id, MBOX_RECONFIG_DATA,
+						 MBOX_CMD_INDIRECT, 3, args);
+			if (ret) {
+				resp_err = 1;
+			} else {
+				xfer_count++;
+				for (i = 0; i < MBOX_RESP_BUFFER_SIZE; i++) {
+					if (!xfer_pending[i]) {
+						xfer_pending[i] = cmd_id;
+						inc_cmd_id(&cmd_id);
+						break;
+					}
+				}
+				debug("+xfer_count = %d\n", xfer_count);
+				debug("xfer ID = %d\n", xfer_pending[i]);
+				debug("data offset = %08x\n", args[1]);
+				debug("data size = %08x\n", args[2]);
+			}
+#ifndef DEBUG
+			puts(".");
+#endif
+		} else {
+			u32 r_id = 0;
+			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 */
+			if (!resp_hdr)
+				continue;
+
+			/* Expect 0 length from RECONFIG_DATA */
+			if (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;
+			}
+
+			/* Read the response header's ID */
+			r_id = MBOX_RESP_ID_GET(resp_hdr);
+			for (i = 0; i < MBOX_RESP_BUFFER_SIZE; i++) {
+				if (r_id == xfer_pending[i]) {
+					/* Reclaim ID */
+					cmd_id = (u32)xfer_pending[i];
+					xfer_pending[i] = 0;
+					xfer_count--;
+					break;
+				}
+			}
+
+			debug("Reclaimed xfer ID = %d\n", cmd_id);
+			debug("-xfer_count = %d\n", 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("\nPolling with MBOX_RECONFIG_STATUS...\n");
+	ret = reconfig_status_polling_resp();
+	if (ret) {
+		printf("Error: %s\n", mbox_cfgstat_to_str(ret));
+		return ret;
+	}
+
+	puts("\nFPGA reconfiguration OK!\n");
+
+	return ret;
+}
-- 
2.2.0

[U-Boot] [PATCH v1 1/3] arm: socfpga: stratix10: Add Stratix10 FPGA Reconfiguration Driver

[Marek Vasut]

On 04/20/2018 05:26 AM, chee.hong.ang at intel.com wrote:
> From: Chee Hong Ang <chee.hong.ang@intel.com>
> 
> Enable FPGA reconfiguration support on Stratix10 SoC.
> 
> Signed-off-by: Chee Hong Ang <chee.hong.ang@intel.com>
> ---
>  drivers/fpga/Kconfig     |  10 ++
>  drivers/fpga/Makefile    |   1 +
>  drivers/fpga/stratix10.c | 298 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 309 insertions(+)
>  create mode 100644 drivers/fpga/stratix10.c
> 
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index d36c4c5..255683d 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -31,6 +31,16 @@ 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 FPGA_ALTERA && TARGET_SOCFPGA_STRATIX10
> +	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 08c9ff8..3c906ec 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -19,6 +19,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..f0c5ace
> --- /dev/null
> +++ b/drivers/fpga/stratix10.c
> @@ -0,0 +1,298 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2016-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, "Acknowledgement 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!"},
> +	{-ETIMEDOUT, "I/O timeout error"},
> +	{-1, "Unknown error!"}
> +};
> +
> +#define MBOX_CFGSTAT_MAX \
> +	  (sizeof(mbox_cfgstat_state) / sizeof(struct 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;
> +}
> +
> +static void inc_cmd_id(u8 *id)
> +{
> +	u8 val = *id + 1;
> +
> +	if (val > 15)
> +		val = 1;

What's this function about, elaborate implementation of (i % 15) + 1 ?

> +	*id = val;
> +}
> +
> +/*
> + * Polling the FPGA configuration status.
> + * Return 0 for success, non-zero for error.
> + */
> +static int reconfig_status_polling_resp(void)
> +{
> +	u32 reconfig_status_resp_len;
> +	u32 reconfig_status_resp[RECONFIG_STATUS_RESPONSE_LEN];
> +	int ret;
> +	unsigned long start = get_timer(0);
> +
> +	while (1) {
> +		reconfig_status_resp_len = RECONFIG_STATUS_RESPONSE_LEN;
> +		ret = mbox_send_cmd(MBOX_ID_UBOOT, MBOX_RECONFIG_STATUS,
> +				    MBOX_CMD_DIRECT, 0, NULL, 0,
> +				    &reconfig_status_resp_len,
> +				    reconfig_status_resp);
> +
> +		if (ret) {
> +			puts("Failure in RECONFIG_STATUS mailbox command!\n");
> +			return ret;
> +		}
> +
> +		/* Check for any error */
> +		ret = reconfig_status_resp[RECONFIG_STATUS_STATE];
> +		if (ret && ret != MBOX_CFGSTAT_STATE_CONFIG)
> +			return ret;
> +
> +		/* Make sure nStatus is not 0 */
> +		ret = reconfig_status_resp[RECONFIG_STATUS_PIN_STATUS];
> +		if (!(ret & RCF_PIN_STATUS_NSTATUS))
> +			return MBOX_CFGSTAT_STATE_ERROR_HARDWARE;

wait_for_bit_le32() or somesuch ?

> +		ret = reconfig_status_resp[RECONFIG_STATUS_SOFTFUNC_STATUS];
> +		if (ret & RCF_SOFTFUNC_STATUS_SEU_ERROR)
> +			return MBOX_CFGSTAT_STATE_ERROR_HARDWARE;
> +
> +		if ((ret & RCF_SOFTFUNC_STATUS_CONF_DONE) &&
> +		    (ret & RCF_SOFTFUNC_STATUS_INIT_DONE) &&
> +		    !reconfig_status_resp[RECONFIG_STATUS_STATE])
> +			return 0;	/* configuration success */
> +
> +		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 i;
> +	u32 mbox_hdr;
> +	u32 resp_len;
> +	u32 hdr_len;
> +	u32 buf[MBOX_RESP_BUFFER_SIZE];
> +
> +	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];

Is this like a memcpy() ?

> +			*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;
> +}
> +
> +static int send_reconfig_data(const void *rbf_data, size_t rbf_size,
> +			      u32 xfer_max, u32 buf_size_max)
> +{
> +	u32 resp_rindex = 0;
> +	u32 resp_windex = 0;
> +	u32 resp_count = 0;
> +	u32 response_buffer[MBOX_RESP_BUFFER_SIZE];
> +	u8 resp_err = 0;
> +	u8 cmd_id = 1;
> +	u32 xfer_count = 0;
> +	u32 xfer_pending[MBOX_RESP_BUFFER_SIZE];
> +	u32 args[3];
> +	int ret = 0;
> +	u32 i;
> +
> +	debug("SDM xfer_max = %d\n", xfer_max);
> +	debug("SDM buf_size_max = %x\n\n", buf_size_max);
> +
> +	for (i = 0; i < MBOX_RESP_BUFFER_SIZE; i++)
> +		xfer_pending[i] = 0;
> +
> +	while (rbf_size || xfer_count) {
> +		if (!resp_err && rbf_size && xfer_count < xfer_max) {

This function is clearly too long, split it into separate functions

> +			args[0] = (1 << 8);

That's BIT(8) .

> +			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;
> +			}
> +
> +			debug("Sending MBOX_RECONFIG_DATA...\n");
> +
> +			ret = mbox_send_cmd_only(cmd_id, MBOX_RECONFIG_DATA,
> +						 MBOX_CMD_INDIRECT, 3, args);
> +			if (ret) {
> +				resp_err = 1;
> +			} else {
> +				xfer_count++;
> +				for (i = 0; i < MBOX_RESP_BUFFER_SIZE; i++) {
> +					if (!xfer_pending[i]) {
> +						xfer_pending[i] = cmd_id;
> +						inc_cmd_id(&cmd_id);
> +						break;
> +					}
> +				}
> +				debug("+xfer_count = %d\n", xfer_count);
> +				debug("xfer ID = %d\n", xfer_pending[i]);
> +				debug("data offset = %08x\n", args[1]);
> +				debug("data size = %08x\n", args[2]);
> +			}
> +#ifndef DEBUG
> +			puts(".");

debug(".") is the same, except without the ifdef

> +#endif
> +		} else {
> +			u32 r_id = 0;
> +			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 */
> +			if (!resp_hdr)
> +				continue;
> +
> +			/* Expect 0 length from RECONFIG_DATA */
> +			if (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;
> +			}
> +
> +			/* Read the response header's ID */
> +			r_id = MBOX_RESP_ID_GET(resp_hdr);
> +			for (i = 0; i < MBOX_RESP_BUFFER_SIZE; i++) {
> +				if (r_id == xfer_pending[i]) {
> +					/* Reclaim ID */
> +					cmd_id = (u32)xfer_pending[i];
> +					xfer_pending[i] = 0;
> +					xfer_count--;
> +					break;
> +				}
> +			}
> +
> +			debug("Reclaimed xfer ID = %d\n", cmd_id);
> +			debug("-xfer_count = %d\n", 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);

Hum, this is iffy, is that a hardware bug ?

> +	debug("\nPolling with MBOX_RECONFIG_STATUS...\n");
> +	ret = reconfig_status_polling_resp();
> +	if (ret) {
> +		printf("Error: %s\n", mbox_cfgstat_to_str(ret));
> +		return ret;
> +	}
> +
> +	puts("\nFPGA reconfiguration OK!\n");
> +
> +	return ret;
> +}
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v1 1/3] arm: socfpga: stratix10: Add Stratix10 FPGA Reconfiguration Driver

[Ang, Chee Hong]

On Fri, 2018-04-20 at 05:41 +0200, Marek Vasut wrote:
> On 04/20/2018 05:26 AM, chee.hong.ang at intel.com wrote:
> > 
> > From: Chee Hong Ang <chee.hong.ang@intel.com>
> > 
> > Enable FPGA reconfiguration support on Stratix10 SoC.
> > 
> > Signed-off-by: Chee Hong Ang <chee.hong.ang@intel.com>
> > ---
> >  drivers/fpga/Kconfig     |  10 ++
> >  drivers/fpga/Makefile    |   1 +
> >  drivers/fpga/stratix10.c | 298
> > +++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 309 insertions(+)
> >  create mode 100644 drivers/fpga/stratix10.c
> > 
> > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > index d36c4c5..255683d 100644
> > --- a/drivers/fpga/Kconfig
> > +++ b/drivers/fpga/Kconfig
> > @@ -31,6 +31,16 @@ 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 FPGA_ALTERA && TARGET_SOCFPGA_STRATIX10
> > +	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 08c9ff8..3c906ec 100644
> > --- a/drivers/fpga/Makefile
> > +++ b/drivers/fpga/Makefile
> > @@ -19,6 +19,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..f0c5ace
> > --- /dev/null
> > +++ b/drivers/fpga/stratix10.c
> > @@ -0,0 +1,298 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2016-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, "Acknowledgement 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!"},
> > +	{-ETIMEDOUT, "I/O timeout error"},
> > +	{-1, "Unknown error!"}
> > +};
> > +
> > +#define MBOX_CFGSTAT_MAX \
> > +	  (sizeof(mbox_cfgstat_state) / sizeof(struct
> > 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;
> > +}
> > +
> > +static void inc_cmd_id(u8 *id)
> > +{
> > +	u8 val = *id + 1;
> > +
> > +	if (val > 15)
> > +		val = 1;
> What's this function about, elaborate implementation of (i % 15) + 1
> ?
This function increment the 4 bits transaction ID (1-15, 0 is unused)
used for sending mailbox command to device.
> 
> > 
> > +	*id = val;
> > +}
> > +
> > +/*
> > + * Polling the FPGA configuration status.
> > + * Return 0 for success, non-zero for error.
> > + */
> > +static int reconfig_status_polling_resp(void)
> > +{
> > +	u32 reconfig_status_resp_len;
> > +	u32 reconfig_status_resp[RECONFIG_STATUS_RESPONSE_LEN];
> > +	int ret;
> > +	unsigned long start = get_timer(0);
> > +
> > +	while (1) {
> > +		reconfig_status_resp_len =
> > RECONFIG_STATUS_RESPONSE_LEN;
> > +		ret = mbox_send_cmd(MBOX_ID_UBOOT,
> > MBOX_RECONFIG_STATUS,
> > +				    MBOX_CMD_DIRECT, 0, NULL, 0,
> > +				    &reconfig_status_resp_len,
> > +				    reconfig_status_resp);
> > +
> > +		if (ret) {
> > +			puts("Failure in RECONFIG_STATUS mailbox
> > command!\n");
> > +			return ret;
> > +		}
> > +
> > +		/* Check for any error */
> > +		ret = reconfig_status_resp[RECONFIG_STATUS_STATE];
> > +		if (ret && ret != MBOX_CFGSTAT_STATE_CONFIG)
> > +			return ret;
> > +
> > +		/* Make sure nStatus is not 0 */
> > +		ret =
> > reconfig_status_resp[RECONFIG_STATUS_PIN_STATUS];
> > +		if (!(ret & RCF_PIN_STATUS_NSTATUS))
> > +			return MBOX_CFGSTAT_STATE_ERROR_HARDWARE;
> wait_for_bit_le32() or somesuch ?
No, we don't read the bit status directly from register. We only need
to test the bit of the pin status stored in memory.
> 
> > 
> > +		ret =
> > reconfig_status_resp[RECONFIG_STATUS_SOFTFUNC_STATUS];
> > +		if (ret & RCF_SOFTFUNC_STATUS_SEU_ERROR)
> > +			return MBOX_CFGSTAT_STATE_ERROR_HARDWARE;
> > +
> > +		if ((ret & RCF_SOFTFUNC_STATUS_CONF_DONE) &&
> > +		    (ret & RCF_SOFTFUNC_STATUS_INIT_DONE) &&
> > +		    !reconfig_status_resp[RECONFIG_STATUS_STATE])
> > +			return 0;	/* configuration success
> > */
> > +
> > +		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 i;
> > +	u32 mbox_hdr;
> > +	u32 resp_len;
> > +	u32 hdr_len;
> > +	u32 buf[MBOX_RESP_BUFFER_SIZE];
> > +
> > +	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];
> Is this like a memcpy() ?
No. This is a circular buffer, index to the memory location may wrap
around.
> 
> > 
> > +			*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;
> > +}
> > +
> > +static int send_reconfig_data(const void *rbf_data, size_t
> > rbf_size,
> > +			      u32 xfer_max, u32 buf_size_max)
> > +{
> > +	u32 resp_rindex = 0;
> > +	u32 resp_windex = 0;
> > +	u32 resp_count = 0;
> > +	u32 response_buffer[MBOX_RESP_BUFFER_SIZE];
> > +	u8 resp_err = 0;
> > +	u8 cmd_id = 1;
> > +	u32 xfer_count = 0;
> > +	u32 xfer_pending[MBOX_RESP_BUFFER_SIZE];
> > +	u32 args[3];
> > +	int ret = 0;
> > +	u32 i;
> > +
> > +	debug("SDM xfer_max = %d\n", xfer_max);
> > +	debug("SDM buf_size_max = %x\n\n", buf_size_max);
> > +
> > +	for (i = 0; i < MBOX_RESP_BUFFER_SIZE; i++)
> > +		xfer_pending[i] = 0;
> > +
> > +	while (rbf_size || xfer_count) {
> > +		if (!resp_err && rbf_size && xfer_count <
> > xfer_max) {
> This function is clearly too long, split it into separate functions
This function is not short but not too long. But let me see what sort
of refactoring can be done to make it shorter abit.
> 
> > 
> > +			args[0] = (1 << 8);
> That's BIT(8) .
Will be addressed in v2 patch.
> 
> > 
> > +			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;
> > +			}
> > +
> > +			debug("Sending MBOX_RECONFIG_DATA...\n");
> > +
> > +			ret = mbox_send_cmd_only(cmd_id,
> > MBOX_RECONFIG_DATA,
> > +						 MBOX_CMD_INDIRECT
> > , 3, args);
> > +			if (ret) {
> > +				resp_err = 1;
> > +			} else {
> > +				xfer_count++;
> > +				for (i = 0; i <
> > MBOX_RESP_BUFFER_SIZE; i++) {
> > +					if (!xfer_pending[i]) {
> > +						xfer_pending[i] =
> > cmd_id;
> > +						inc_cmd_id(&cmd_id
> > );
> > +						break;
> > +					}
> > +				}
> > +				debug("+xfer_count = %d\n",
> > xfer_count);
> > +				debug("xfer ID = %d\n",
> > xfer_pending[i]);
> > +				debug("data offset = %08x\n",
> > args[1]);
> > +				debug("data size = %08x\n",
> > args[2]);
> > +			}
> > +#ifndef DEBUG
> > +			puts(".");
> debug(".") is the same, except without the ifdef
This is #ifndef DEBUG, we only print '.' in non-debug environment.
> 
> > 
> > +#endif
> > +		} else {
> > +			u32 r_id = 0;
> > +			u32 resp_hdr = get_resp_hdr(&resp_rindex,
> > &resp_windex,
> > +						    &resp_count,
> > +						    response_buffe
> > r,
> > +						    MBOX_RESP_BUFF
> > ER_SIZE,
> > +						    MBOX_CLIENT_ID
> > _UBOOT);
> > +
> > +			/* If no valid response header found */
> > +			if (!resp_hdr)
> > +				continue;
> > +
> > +			/* Expect 0 length from RECONFIG_DATA */
> > +			if (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;
> > +			}
> > +
> > +			/* Read the response header's ID */
> > +			r_id = MBOX_RESP_ID_GET(resp_hdr);
> > +			for (i = 0; i < MBOX_RESP_BUFFER_SIZE;
> > i++) {
> > +				if (r_id == xfer_pending[i]) {
> > +					/* Reclaim ID */
> > +					cmd_id =
> > (u32)xfer_pending[i];
> > +					xfer_pending[i] = 0;
> > +					xfer_count--;
> > +					break;
> > +				}
> > +			}
> > +
> > +			debug("Reclaimed xfer ID = %d\n", cmd_id);
> > +			debug("-xfer_count = %d\n", 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);
> Hum, this is iffy, is that a hardware bug ?
Yes. The firmware running in that device is not able to response
quickly.
> 
> > 
> > +	debug("\nPolling with MBOX_RECONFIG_STATUS...\n");
> > +	ret = reconfig_status_polling_resp();
> > +	if (ret) {
> > +		printf("Error: %s\n", mbox_cfgstat_to_str(ret));
> > +		return ret;
> > +	}
> > +
> > +	puts("\nFPGA reconfiguration OK!\n");
> > +
> > +	return ret;
> > +}
> > 
> 

[U-Boot] [PATCH v1 1/3] arm: socfpga: stratix10: Add Stratix10 FPGA Reconfiguration Driver

[Marek Vasut]

On 04/26/2018 08:12 AM, Ang, Chee Hong wrote:
> On Fri, 2018-04-20 at 05:41 +0200, Marek Vasut wrote:
>> On 04/20/2018 05:26 AM, chee.hong.ang at intel.com wrote:
>>>
>>> From: Chee Hong Ang <chee.hong.ang@intel.com>
>>>
>>> Enable FPGA reconfiguration support on Stratix10 SoC.
>>>
>>> Signed-off-by: Chee Hong Ang <chee.hong.ang@intel.com>
>>> ---
>>>  drivers/fpga/Kconfig     |  10 ++
>>>  drivers/fpga/Makefile    |   1 +
>>>  drivers/fpga/stratix10.c | 298
>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 309 insertions(+)
>>>  create mode 100644 drivers/fpga/stratix10.c
>>>
>>> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
>>> index d36c4c5..255683d 100644
>>> --- a/drivers/fpga/Kconfig
>>> +++ b/drivers/fpga/Kconfig
>>> @@ -31,6 +31,16 @@ 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 FPGA_ALTERA && TARGET_SOCFPGA_STRATIX10
>>> +	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 08c9ff8..3c906ec 100644
>>> --- a/drivers/fpga/Makefile
>>> +++ b/drivers/fpga/Makefile
>>> @@ -19,6 +19,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..f0c5ace
>>> --- /dev/null
>>> +++ b/drivers/fpga/stratix10.c
>>> @@ -0,0 +1,298 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (C) 2016-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, "Acknowledgement 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!"},
>>> +	{-ETIMEDOUT, "I/O timeout error"},
>>> +	{-1, "Unknown error!"}
>>> +};
>>> +
>>> +#define MBOX_CFGSTAT_MAX \
>>> +	  (sizeof(mbox_cfgstat_state) / sizeof(struct
>>> 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;
>>> +}
>>> +
>>> +static void inc_cmd_id(u8 *id)
>>> +{
>>> +	u8 val = *id + 1;
>>> +
>>> +	if (val > 15)
>>> +		val = 1;
>> What's this function about, elaborate implementation of (i % 15) + 1
>> ?
> This function increment the 4 bits transaction ID (1-15, 0 is unused)
> used for sending mailbox command to device.
>>
>>>
>>> +	*id = val;
>>> +}
>>> +
>>> +/*
>>> + * Polling the FPGA configuration status.
>>> + * Return 0 for success, non-zero for error.
>>> + */
>>> +static int reconfig_status_polling_resp(void)
>>> +{
>>> +	u32 reconfig_status_resp_len;
>>> +	u32 reconfig_status_resp[RECONFIG_STATUS_RESPONSE_LEN];
>>> +	int ret;
>>> +	unsigned long start = get_timer(0);
>>> +
>>> +	while (1) {
>>> +		reconfig_status_resp_len =
>>> RECONFIG_STATUS_RESPONSE_LEN;
>>> +		ret = mbox_send_cmd(MBOX_ID_UBOOT,
>>> MBOX_RECONFIG_STATUS,
>>> +				    MBOX_CMD_DIRECT, 0, NULL, 0,
>>> +				    &reconfig_status_resp_len,
>>> +				    reconfig_status_resp);
>>> +
>>> +		if (ret) {
>>> +			puts("Failure in RECONFIG_STATUS mailbox
>>> command!\n");
>>> +			return ret;
>>> +		}
>>> +
>>> +		/* Check for any error */
>>> +		ret = reconfig_status_resp[RECONFIG_STATUS_STATE];
>>> +		if (ret && ret != MBOX_CFGSTAT_STATE_CONFIG)
>>> +			return ret;
>>> +
>>> +		/* Make sure nStatus is not 0 */
>>> +		ret =
>>> reconfig_status_resp[RECONFIG_STATUS_PIN_STATUS];
>>> +		if (!(ret & RCF_PIN_STATUS_NSTATUS))
>>> +			return MBOX_CFGSTAT_STATE_ERROR_HARDWARE;
>> wait_for_bit_le32() or somesuch ?
> No, we don't read the bit status directly from register. We only need
> to test the bit of the pin status stored in memory.

Who's populating the memory ?

>>
>>>
>>> +		ret =
>>> reconfig_status_resp[RECONFIG_STATUS_SOFTFUNC_STATUS];
>>> +		if (ret & RCF_SOFTFUNC_STATUS_SEU_ERROR)
>>> +			return MBOX_CFGSTAT_STATE_ERROR_HARDWARE;
>>> +
>>> +		if ((ret & RCF_SOFTFUNC_STATUS_CONF_DONE) &&
>>> +		    (ret & RCF_SOFTFUNC_STATUS_INIT_DONE) &&
>>> +		    !reconfig_status_resp[RECONFIG_STATUS_STATE])
>>> +			return 0;	/* configuration success
>>> */
>>> +
>>> +		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 i;
>>> +	u32 mbox_hdr;
>>> +	u32 resp_len;
>>> +	u32 hdr_len;
>>> +	u32 buf[MBOX_RESP_BUFFER_SIZE];
>>> +
>>> +	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];
>> Is this like a memcpy() ?
> No. This is a circular buffer, index to the memory location may wrap
> around.

Two memcpys then ?

>>>
>>> +			*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;
>>> +}
>>> +
>>> +static int send_reconfig_data(const void *rbf_data, size_t
>>> rbf_size,
>>> +			      u32 xfer_max, u32 buf_size_max)
>>> +{
>>> +	u32 resp_rindex = 0;
>>> +	u32 resp_windex = 0;
>>> +	u32 resp_count = 0;
>>> +	u32 response_buffer[MBOX_RESP_BUFFER_SIZE];
>>> +	u8 resp_err = 0;
>>> +	u8 cmd_id = 1;
>>> +	u32 xfer_count = 0;
>>> +	u32 xfer_pending[MBOX_RESP_BUFFER_SIZE];
>>> +	u32 args[3];
>>> +	int ret = 0;
>>> +	u32 i;
>>> +
>>> +	debug("SDM xfer_max = %d\n", xfer_max);
>>> +	debug("SDM buf_size_max = %x\n\n", buf_size_max);
>>> +
>>> +	for (i = 0; i < MBOX_RESP_BUFFER_SIZE; i++)
>>> +		xfer_pending[i] = 0;
>>> +
>>> +	while (rbf_size || xfer_count) {
>>> +		if (!resp_err && rbf_size && xfer_count <
>>> xfer_max) {
>> This function is clearly too long, split it into separate functions
> This function is not short but not too long. But let me see what sort
> of refactoring can be done to make it shorter abit.

Split it, it is too long.

>>>
>>> +			args[0] = (1 << 8);
>> That's BIT(8) .
> Will be addressed in v2 patch.
>>
>>>
>>> +			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;
>>> +			}
>>> +
>>> +			debug("Sending MBOX_RECONFIG_DATA...\n");
>>> +
>>> +			ret = mbox_send_cmd_only(cmd_id,
>>> MBOX_RECONFIG_DATA,
>>> +						 MBOX_CMD_INDIRECT
>>> , 3, args);
>>> +			if (ret) {
>>> +				resp_err = 1;
>>> +			} else {
>>> +				xfer_count++;
>>> +				for (i = 0; i <
>>> MBOX_RESP_BUFFER_SIZE; i++) {
>>> +					if (!xfer_pending[i]) {
>>> +						xfer_pending[i] =
>>> cmd_id;
>>> +						inc_cmd_id(&cmd_id
>>> );
>>> +						break;
>>> +					}
>>> +				}
>>> +				debug("+xfer_count = %d\n",
>>> xfer_count);
>>> +				debug("xfer ID = %d\n",
>>> xfer_pending[i]);
>>> +				debug("data offset = %08x\n",
>>> args[1]);
>>> +				debug("data size = %08x\n",
>>> args[2]);
>>> +			}
>>> +#ifndef DEBUG
>>> +			puts(".");
>> debug(".") is the same, except without the ifdef
> This is #ifndef DEBUG, we only print '.' in non-debug environment.

Oh, OK

>>>
>>> +#endif
>>> +		} else {
>>> +			u32 r_id = 0;
>>> +			u32 resp_hdr = get_resp_hdr(&resp_rindex,
>>> &resp_windex,
>>> +						    &resp_count,
>>> +						    response_buffe
>>> r,
>>> +						    MBOX_RESP_BUFF
>>> ER_SIZE,
>>> +						    MBOX_CLIENT_ID
>>> _UBOOT);
>>> +
>>> +			/* If no valid response header found */
>>> +			if (!resp_hdr)
>>> +				continue;
>>> +
>>> +			/* Expect 0 length from RECONFIG_DATA */
>>> +			if (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;
>>> +			}
>>> +
>>> +			/* Read the response header's ID */
>>> +			r_id = MBOX_RESP_ID_GET(resp_hdr);
>>> +			for (i = 0; i < MBOX_RESP_BUFFER_SIZE;
>>> i++) {
>>> +				if (r_id == xfer_pending[i]) {
>>> +					/* Reclaim ID */
>>> +					cmd_id =
>>> (u32)xfer_pending[i];
>>> +					xfer_pending[i] = 0;
>>> +					xfer_count--;
>>> +					break;
>>> +				}
>>> +			}
>>> +
>>> +			debug("Reclaimed xfer ID = %d\n", cmd_id);
>>> +			debug("-xfer_count = %d\n", 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);
>> Hum, this is iffy, is that a hardware bug ?
> Yes. The firmware running in that device is not able to response
> quickly.

And you cannot poll it in some way ?

>>>
>>> +	debug("\nPolling with MBOX_RECONFIG_STATUS...\n");
>>> +	ret = reconfig_status_polling_resp();
>>> +	if (ret) {
>>> +		printf("Error: %s\n", mbox_cfgstat_to_str(ret));
>>> +		return ret;
>>> +	}
>>> +
>>> +	puts("\nFPGA reconfiguration OK!\n");
>>> +
>>> +	return ret;
>>> +}
>>>


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v1 1/3] arm: socfpga: stratix10: Add Stratix10 FPGA Reconfiguration Driver

[Ang, Chee Hong]

On Thu, 2018-04-26 at 14:37 +0200, Marek Vasut wrote:
> On 04/26/2018 08:12 AM, Ang, Chee Hong wrote:
> > 
> > On Fri, 2018-04-20 at 05:41 +0200, Marek Vasut wrote:
> > > 
> > > On 04/20/2018 05:26 AM, chee.hong.ang at intel.com wrote:
> > > > 
> > > > 
> > > > From: Chee Hong Ang <chee.hong.ang@intel.com>
> > > > 
> > > > Enable FPGA reconfiguration support on Stratix10 SoC.
> > > > 
> > > > Signed-off-by: Chee Hong Ang <chee.hong.ang@intel.com>
> > > > ---
> > > >  drivers/fpga/Kconfig     |  10 ++
> > > >  drivers/fpga/Makefile    |   1 +
> > > >  drivers/fpga/stratix10.c | 298
> > > > +++++++++++++++++++++++++++++++++++++++++++++++
> > > >  3 files changed, 309 insertions(+)
> > > >  create mode 100644 drivers/fpga/stratix10.c
> > > > 
> > > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > > > index d36c4c5..255683d 100644
> > > > --- a/drivers/fpga/Kconfig
> > > > +++ b/drivers/fpga/Kconfig
> > > > @@ -31,6 +31,16 @@ 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 FPGA_ALTERA && TARGET_SOCFPGA_STRATIX10
> > > > +	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 08c9ff8..3c906ec 100644
> > > > --- a/drivers/fpga/Makefile
> > > > +++ b/drivers/fpga/Makefile
> > > > @@ -19,6 +19,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..f0c5ace
> > > > --- /dev/null
> > > > +++ b/drivers/fpga/stratix10.c
> > > > @@ -0,0 +1,298 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Copyright (C) 2016-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		60
> > > > 000
> > > > +#define RECONFIG_STATUS_INTERVAL_DELAY_US		10000
> > > > 00
> > > > +
> > > > +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, "Acknowledgement
> > > > 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!"},
> > > > +	{-ETIMEDOUT, "I/O timeout error"},
> > > > +	{-1, "Unknown error!"}
> > > > +};
> > > > +
> > > > +#define MBOX_CFGSTAT_MAX \
> > > > +	  (sizeof(mbox_cfgstat_state) / sizeof(struct
> > > > 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;
> > > > +}
> > > > +
> > > > +static void inc_cmd_id(u8 *id)
> > > > +{
> > > > +	u8 val = *id + 1;
> > > > +
> > > > +	if (val > 15)
> > > > +		val = 1;
> > > What's this function about, elaborate implementation of (i % 15)
> > > + 1
> > > ?
> > This function increment the 4 bits transaction ID (1-15, 0 is
> > unused)
> > used for sending mailbox command to device.
> > > 
> > > 
> > > > 
> > > > 
> > > > +	*id = val;
> > > > +}
> > > > +
> > > > +/*
> > > > + * Polling the FPGA configuration status.
> > > > + * Return 0 for success, non-zero for error.
> > > > + */
> > > > +static int reconfig_status_polling_resp(void)
> > > > +{
> > > > +	u32 reconfig_status_resp_len;
> > > > +	u32
> > > > reconfig_status_resp[RECONFIG_STATUS_RESPONSE_LEN];
> > > > +	int ret;
> > > > +	unsigned long start = get_timer(0);
> > > > +
> > > > +	while (1) {
> > > > +		reconfig_status_resp_len =
> > > > RECONFIG_STATUS_RESPONSE_LEN;
> > > > +		ret = mbox_send_cmd(MBOX_ID_UBOOT,
> > > > MBOX_RECONFIG_STATUS,
> > > > +				    MBOX_CMD_DIRECT, 0, NULL,
> > > > 0,
> > > > +				    &reconfig_status_resp_len,
> > > > +				    reconfig_status_resp);
> > > > +
> > > > +		if (ret) {
> > > > +			puts("Failure in RECONFIG_STATUS
> > > > mailbox
> > > > command!\n");
> > > > +			return ret;
> > > > +		}
> > > > +
> > > > +		/* Check for any error */
> > > > +		ret =
> > > > reconfig_status_resp[RECONFIG_STATUS_STATE];
> > > > +		if (ret && ret != MBOX_CFGSTAT_STATE_CONFIG)
> > > > +			return ret;
> > > > +
> > > > +		/* Make sure nStatus is not 0 */
> > > > +		ret =
> > > > reconfig_status_resp[RECONFIG_STATUS_PIN_STATUS];
> > > > +		if (!(ret & RCF_PIN_STATUS_NSTATUS))
> > > > +			return
> > > > MBOX_CFGSTAT_STATE_ERROR_HARDWARE;
> > > wait_for_bit_le32() or somesuch ?
> > No, we don't read the bit status directly from register. We only
> > need
> > to test the bit of the pin status stored in memory.
> Who's populating the memory ?
ret = mbox_send_cmd(MBOX_ID_UBOOT, MBOX_RECONFIG_STATUS,
MBOX_CMD_DIRECT, 0, NULL, 0, &reconfig_status_resp_len,
reconfig_status_resp);

We send mailbox command to the device and the device will respond by
filling the 'reconfig_status_resp' with the device status.

> 
> > 
> > > 
> > > 
> > > > 
> > > > 
> > > > +		ret =
> > > > reconfig_status_resp[RECONFIG_STATUS_SOFTFUNC_STATUS];
> > > > +		if (ret & RCF_SOFTFUNC_STATUS_SEU_ERROR)
> > > > +			return
> > > > MBOX_CFGSTAT_STATE_ERROR_HARDWARE;
> > > > +
> > > > +		if ((ret & RCF_SOFTFUNC_STATUS_CONF_DONE) &&
> > > > +		    (ret & RCF_SOFTFUNC_STATUS_INIT_DONE) &&
> > > > +		    !reconfig_status_resp[RECONFIG_STATUS_STAT
> > > > E])
> > > > +			return 0;	/* configuration
> > > > success
> > > > */
> > > > +
> > > > +		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 i;
> > > > +	u32 mbox_hdr;
> > > > +	u32 resp_len;
> > > > +	u32 hdr_len;
> > > > +	u32 buf[MBOX_RESP_BUFFER_SIZE];
> > > > +
> > > > +	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];
> > > Is this like a memcpy() ?
> > No. This is a circular buffer, index to the memory location may
> > wrap
> > around.
> Two memcpys then ?
Will refactor this part in v2.
> 
> > 
> > > 
> > > > 
> > > > 
> > > > +			*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;
> > > > +}
> > > > +
> > > > +static int send_reconfig_data(const void *rbf_data, size_t
> > > > rbf_size,
> > > > +			      u32 xfer_max, u32 buf_size_max)
> > > > +{
> > > > +	u32 resp_rindex = 0;
> > > > +	u32 resp_windex = 0;
> > > > +	u32 resp_count = 0;
> > > > +	u32 response_buffer[MBOX_RESP_BUFFER_SIZE];
> > > > +	u8 resp_err = 0;
> > > > +	u8 cmd_id = 1;
> > > > +	u32 xfer_count = 0;
> > > > +	u32 xfer_pending[MBOX_RESP_BUFFER_SIZE];
> > > > +	u32 args[3];
> > > > +	int ret = 0;
> > > > +	u32 i;
> > > > +
> > > > +	debug("SDM xfer_max = %d\n", xfer_max);
> > > > +	debug("SDM buf_size_max = %x\n\n", buf_size_max);
> > > > +
> > > > +	for (i = 0; i < MBOX_RESP_BUFFER_SIZE; i++)
> > > > +		xfer_pending[i] = 0;
> > > > +
> > > > +	while (rbf_size || xfer_count) {
> > > > +		if (!resp_err && rbf_size && xfer_count <
> > > > xfer_max) {
> > > This function is clearly too long, split it into separate
> > > functions
> > This function is not short but not too long. But let me see what
> > sort
> > of refactoring can be done to make it shorter abit.
> Split it, it is too long.
> 
> > 
> > > 
> > > > 
> > > > 
> > > > +			args[0] = (1 << 8);
> > > That's BIT(8) .
> > Will be addressed in v2 patch.
> > > 
> > > 
> > > > 
> > > > 
> > > > +			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;
> > > > +			}
> > > > +
> > > > +			debug("Sending
> > > > MBOX_RECONFIG_DATA...\n");
> > > > +
> > > > +			ret = mbox_send_cmd_only(cmd_id,
> > > > MBOX_RECONFIG_DATA,
> > > > +						 MBOX_CMD_INDI
> > > > RECT
> > > > , 3, args);
> > > > +			if (ret) {
> > > > +				resp_err = 1;
> > > > +			} else {
> > > > +				xfer_count++;
> > > > +				for (i = 0; i <
> > > > MBOX_RESP_BUFFER_SIZE; i++) {
> > > > +					if (!xfer_pending[i])
> > > > {
> > > > +						xfer_pending[i
> > > > ] =
> > > > cmd_id;
> > > > +						inc_cmd_id(&cm
> > > > d_id
> > > > );
> > > > +						break;
> > > > +					}
> > > > +				}
> > > > +				debug("+xfer_count = %d\n",
> > > > xfer_count);
> > > > +				debug("xfer ID = %d\n",
> > > > xfer_pending[i]);
> > > > +				debug("data offset = %08x\n",
> > > > args[1]);
> > > > +				debug("data size = %08x\n",
> > > > args[2]);
> > > > +			}
> > > > +#ifndef DEBUG
> > > > +			puts(".");
> > > debug(".") is the same, except without the ifdef
> > This is #ifndef DEBUG, we only print '.' in non-debug environment.
> Oh, OK
> 
> > 
> > > 
> > > > 
> > > > 
> > > > +#endif
> > > > +		} else {
> > > > +			u32 r_id = 0;
> > > > +			u32 resp_hdr =
> > > > get_resp_hdr(&resp_rindex,
> > > > &resp_windex,
> > > > +						    &resp_coun
> > > > t,
> > > > +						    response_b
> > > > uffe
> > > > r,
> > > > +						    MBOX_RESP_
> > > > BUFF
> > > > ER_SIZE,
> > > > +						    MBOX_CLIEN
> > > > T_ID
> > > > _UBOOT);
> > > > +
> > > > +			/* If no valid response header found
> > > > */
> > > > +			if (!resp_hdr)
> > > > +				continue;
> > > > +
> > > > +			/* Expect 0 length from RECONFIG_DATA
> > > > */
> > > > +			if (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;
> > > > +			}
> > > > +
> > > > +			/* Read the response header's ID */
> > > > +			r_id = MBOX_RESP_ID_GET(resp_hdr);
> > > > +			for (i = 0; i < MBOX_RESP_BUFFER_SIZE;
> > > > i++) {
> > > > +				if (r_id == xfer_pending[i]) {
> > > > +					/* Reclaim ID */
> > > > +					cmd_id =
> > > > (u32)xfer_pending[i];
> > > > +					xfer_pending[i] = 0;
> > > > +					xfer_count--;
> > > > +					break;
> > > > +				}
> > > > +			}
> > > > +
> > > > +			debug("Reclaimed xfer ID = %d\n",
> > > > cmd_id);
> > > > +			debug("-xfer_count = %d\n",
> > > > 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);
> > > Hum, this is iffy, is that a hardware bug ?
> > Yes. The firmware running in that device is not able to response
> > quickly.
> And you cannot poll it in some way ?
I know this is ugly and looks buggy. But there are no mechanism to poll
whether the device is ready to accept any mailbox command or not. So
for now, slowing down the mailbox exchange is the only way to go.
> 
> > 
> > > 
> > > > 
> > > > 
> > > > +	debug("\nPolling with MBOX_RECONFIG_STATUS...\n");
> > > > +	ret = reconfig_status_polling_resp();
> > > > +	if (ret) {
> > > > +		printf("Error: %s\n",
> > > > mbox_cfgstat_to_str(ret));
> > > > +		return ret;
> > > > +	}
> > > > +
> > > > +	puts("\nFPGA reconfiguration OK!\n");
> > > > +
> > > > +	return ret;
> > > > +}
> > > > 
> 

[U-Boot] [PATCH v1 1/3] arm: socfpga: stratix10: Add Stratix10 FPGA Reconfiguration Driver

[Marek Vasut]

On 04/27/2018 07:51 AM, Ang, Chee Hong wrote:
[...]

>>>>> +		/* Check for any error */
>>>>> +		ret =
>>>>> reconfig_status_resp[RECONFIG_STATUS_STATE];
>>>>> +		if (ret && ret != MBOX_CFGSTAT_STATE_CONFIG)
>>>>> +			return ret;
>>>>> +
>>>>> +		/* Make sure nStatus is not 0 */
>>>>> +		ret =
>>>>> reconfig_status_resp[RECONFIG_STATUS_PIN_STATUS];
>>>>> +		if (!(ret & RCF_PIN_STATUS_NSTATUS))
>>>>> +			return
>>>>> MBOX_CFGSTAT_STATE_ERROR_HARDWARE;
>>>> wait_for_bit_le32() or somesuch ?
>>> No, we don't read the bit status directly from register. We only
>>> need
>>> to test the bit of the pin status stored in memory.
>> Who's populating the memory ?
> ret = mbox_send_cmd(MBOX_ID_UBOOT, MBOX_RECONFIG_STATUS,
> MBOX_CMD_DIRECT, 0, NULL, 0, &reconfig_status_resp_len,
> reconfig_status_resp);
> 
> We send mailbox command to the device and the device will respond by
> filling the 'reconfig_status_resp' with the device status.

Ah, I see. Would it make sense to implement a generic "poll for bit" for
this mailbox communication ? Also, we have drivers/mailbox/ , would it
make sense to move the mailbox stuff there ?

[...]

>>>>> +	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];
>>>> Is this like a memcpy() ?
>>> No. This is a circular buffer, index to the memory location may
>>> wrap
>>> around.
>> Two memcpys then ?
> Will refactor this part in v2.

Does it even have to be a circular buffer in the first place ?

[...]

>>>>> +	/* Make sure we don't send MBOX_RECONFIG_STATUS too
>>>>> fast
>>>>> */
>>>>> +	udelay(RECONFIG_STATUS_INTERVAL_DELAY_US);
>>>> Hum, this is iffy, is that a hardware bug ?
>>> Yes. The firmware running in that device is not able to response
>>> quickly.
>> And you cannot poll it in some way ?
> I know this is ugly and looks buggy. But there are no mechanism to poll
> whether the device is ready to accept any mailbox command or not. So
> for now, slowing down the mailbox exchange is the only way to go.

If it works reliably, so be it.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v1 1/3] arm: socfpga: stratix10: Add Stratix10 FPGA Reconfiguration Driver

[Ang, Chee Hong]

On Fri, 2018-04-27 at 09:08 +0200, Marek Vasut wrote:
> On 04/27/2018 07:51 AM, Ang, Chee Hong wrote:
> [...]
> 
> > 
> > > 
> > > > 
> > > > > 
> > > > > > 
> > > > > > +		/* Check for any error */
> > > > > > +		ret =
> > > > > > reconfig_status_resp[RECONFIG_STATUS_STATE];
> > > > > > +		if (ret && ret !=
> > > > > > MBOX_CFGSTAT_STATE_CONFIG)
> > > > > > +			return ret;
> > > > > > +
> > > > > > +		/* Make sure nStatus is not 0 */
> > > > > > +		ret =
> > > > > > reconfig_status_resp[RECONFIG_STATUS_PIN_STATUS];
> > > > > > +		if (!(ret & RCF_PIN_STATUS_NSTATUS))
> > > > > > +			return
> > > > > > MBOX_CFGSTAT_STATE_ERROR_HARDWARE;
> > > > > wait_for_bit_le32() or somesuch ?
> > > > No, we don't read the bit status directly from register. We
> > > > only
> > > > need
> > > > to test the bit of the pin status stored in memory.
> > > Who's populating the memory ?
> > ret = mbox_send_cmd(MBOX_ID_UBOOT, MBOX_RECONFIG_STATUS,
> > MBOX_CMD_DIRECT, 0, NULL, 0, &reconfig_status_resp_len,
> > reconfig_status_resp);
> > 
> > We send mailbox command to the device and the device will respond
> > by
> > filling the 'reconfig_status_resp' with the device status.
> Ah, I see. Would it make sense to implement a generic "poll for bit"
> for
> this mailbox communication ? Also, we have drivers/mailbox/ , would
> it
> make sense to move the mailbox stuff there ?
There is a separate patch for mailbox communication stuffs in the
process of upstreaming by my colleague Tan, Ley Foon. But currently, I
don't think the mailbox code is placed under drivers/mailbox. I can
refactor this code into a generic function like 'get fpga device
status' via mailbox and place it under drivers/mailbox. Will work with
her to address this.
> 
> [...]
> 
> > 
> > > 
> > > > 
> > > > > 
> > > > > > 
> > > > > > +	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];
> > > > > Is this like a memcpy() ?
> > > > No. This is a circular buffer, index to the memory location may
> > > > wrap
> > > > around.
> > > Two memcpys then ?
> > Will refactor this part in v2.
> Does it even have to be a circular buffer in the first place ?
Yes, these mailbox responses are stored in buffer and will be retrieved
in sequence at later stage.
> 
> [...]
> 
> > 
> > > 
> > > > 
> > > > > 
> > > > > > 
> > > > > > +	/* Make sure we don't send MBOX_RECONFIG_STATUS
> > > > > > too
> > > > > > fast
> > > > > > */
> > > > > > +	udelay(RECONFIG_STATUS_INTERVAL_DELAY_US);
> > > > > Hum, this is iffy, is that a hardware bug ?
> > > > Yes. The firmware running in that device is not able to
> > > > response
> > > > quickly.
> > > And you cannot poll it in some way ?
> > I know this is ugly and looks buggy. But there are no mechanism to
> > poll
> > whether the device is ready to accept any mailbox command or not.
> > So
> > for now, slowing down the mailbox exchange is the only way to go.
> If it works reliably, so be it.
Yes. It works reliably.
> 

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

[chee.hong.ang at intel.com]

From: Chee Hong Ang <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: Chee Hong Ang <chee.hong.ang@intel.com>
---
 arch/arm/mach-socfpga/misc.c     | 20 +++++++++++++++++---
 arch/arm/mach-socfpga/misc_s10.c |  4 ++++
 drivers/fpga/altera.c            |  6 ++++++
 include/altera.h                 |  8 ++++++++
 4 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-socfpga/misc.c
index d15cbc7..e36c686 100644
--- a/arch/arm/mach-socfpga/misc.c
+++ b/arch/arm/mach-socfpga/misc.c
@@ -87,11 +87,24 @@ int overwrite_console(void)
 #endif
 
 #ifdef CONFIG_FPGA
-/*
- * FPGA programming support for SoC FPGA Cyclone V
- */
 static Altera_desc altera_fpga[] = {
 	{
+#ifdef CONFIG_FPGA_STRATIX10
+		/* FPGA programming support for SoC FPGA Stratix 10 */
+		/* 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 */
 		/* Family */
 		Altera_SoCFPGA,
 		/* Interface type */
@@ -104,6 +117,7 @@ static Altera_desc altera_fpga[] = {
 		NULL,
 		/* No cookie implementation */
 		0
+#endif
 	},
 };
 
diff --git a/arch/arm/mach-socfpga/misc_s10.c b/arch/arm/mach-socfpga/misc_s10.c
index b1cc6ca..012d8f6 100644
--- a/arch/arm/mach-socfpga/misc_s10.c
+++ b/arch/arm/mach-socfpga/misc_s10.c
@@ -71,6 +71,10 @@ int arch_misc_init(void)
 
 int arch_early_init_r(void)
 {
+#ifdef CONFIG_FPGA
+	socfpga_fpga_add();
+#endif
+
 	return 0;
 }
 
diff --git a/drivers/fpga/altera.c b/drivers/fpga/altera.c
index 135a357..b662ff5 100644
--- a/drivers/fpga/altera.c
+++ b/drivers/fpga/altera.c
@@ -40,6 +40,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
@@ -155,6 +158,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 48d3eb7..e9ba47a 100644
--- a/include/altera.h
+++ b/include/altera.h
@@ -40,6 +40,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,
 };
@@ -55,6 +57,8 @@ enum altera_family {
 	Altera_StratixII,
 	/* StratixV Family */
 	Altera_StratixV,
+	/* Stratix10 Family */
+	Intel_FPGA_Stratix10,
 	/* SoCFPGA Family */
 	Altera_SoCFPGA,
 
@@ -117,4 +121,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.2.0

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

[Marek Vasut]

On 04/20/2018 05:26 AM, chee.hong.ang at intel.com wrote:
> From: Chee Hong Ang <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: Chee Hong Ang <chee.hong.ang@intel.com>
> ---
>  arch/arm/mach-socfpga/misc.c     | 20 +++++++++++++++++---
>  arch/arm/mach-socfpga/misc_s10.c |  4 ++++
>  drivers/fpga/altera.c            |  6 ++++++
>  include/altera.h                 |  8 ++++++++
>  4 files changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-socfpga/misc.c
> index d15cbc7..e36c686 100644
> --- a/arch/arm/mach-socfpga/misc.c
> +++ b/arch/arm/mach-socfpga/misc.c
> @@ -87,11 +87,24 @@ int overwrite_console(void)
>  #endif
>  
>  #ifdef CONFIG_FPGA
> -/*
> - * FPGA programming support for SoC FPGA Cyclone V
> - */
>  static Altera_desc altera_fpga[] = {
>  	{
> +#ifdef CONFIG_FPGA_STRATIX10

Create a separate structure for each FPGA family.

> +		/* FPGA programming support for SoC FPGA Stratix 10 */
> +		/* 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 */
>  		/* Family */
>  		Altera_SoCFPGA,
>  		/* Interface type */
> @@ -104,6 +117,7 @@ static Altera_desc altera_fpga[] = {
>  		NULL,
>  		/* No cookie implementation */
>  		0
> +#endif
>  	},
>  };
>  
> diff --git a/arch/arm/mach-socfpga/misc_s10.c b/arch/arm/mach-socfpga/misc_s10.c
> index b1cc6ca..012d8f6 100644
> --- a/arch/arm/mach-socfpga/misc_s10.c
> +++ b/arch/arm/mach-socfpga/misc_s10.c
> @@ -71,6 +71,10 @@ int arch_misc_init(void)
>  
>  int arch_early_init_r(void)
>  {
> +#ifdef CONFIG_FPGA
> +	socfpga_fpga_add();
> +#endif
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/fpga/altera.c b/drivers/fpga/altera.c
> index 135a357..b662ff5 100644
> --- a/drivers/fpga/altera.c
> +++ b/drivers/fpga/altera.c
> @@ -40,6 +40,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
> @@ -155,6 +158,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 48d3eb7..e9ba47a 100644
> --- a/include/altera.h
> +++ b/include/altera.h
> @@ -40,6 +40,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,
>  };
> @@ -55,6 +57,8 @@ enum altera_family {
>  	Altera_StratixII,
>  	/* StratixV Family */
>  	Altera_StratixV,
> +	/* Stratix10 Family */
> +	Intel_FPGA_Stratix10,
>  	/* SoCFPGA Family */
>  	Altera_SoCFPGA,
>  
> @@ -117,4 +121,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);

What is this doing here ?

> +#endif
> +
>  #endif /* _ALTERA_H_ */
> 


-- 
Best regards,
Marek Vasut

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

[Ang, Chee Hong]

On Fri, 2018-04-20 at 05:42 +0200, Marek Vasut wrote:
> On 04/20/2018 05:26 AM, chee.hong.ang at intel.com wrote:
> > 
> > From: Chee Hong Ang <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: Chee Hong Ang <chee.hong.ang@intel.com>
> > ---
> >  arch/arm/mach-socfpga/misc.c     | 20 +++++++++++++++++---
> >  arch/arm/mach-socfpga/misc_s10.c |  4 ++++
> >  drivers/fpga/altera.c            |  6 ++++++
> >  include/altera.h                 |  8 ++++++++
> >  4 files changed, 35 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-
> > socfpga/misc.c
> > index d15cbc7..e36c686 100644
> > --- a/arch/arm/mach-socfpga/misc.c
> > +++ b/arch/arm/mach-socfpga/misc.c
> > @@ -87,11 +87,24 @@ int overwrite_console(void)
> >  #endif
> >  
> >  #ifdef CONFIG_FPGA
> > -/*
> > - * FPGA programming support for SoC FPGA Cyclone V
> > - */
> >  static Altera_desc altera_fpga[] = {
> >  	{
> > +#ifdef CONFIG_FPGA_STRATIX10
> Create a separate structure for each FPGA family.
Will be addressed in v2 patch.
> 
> > 
> > +		/* FPGA programming support for SoC FPGA Stratix
> > 10 */
> > +		/* 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
> > */
> >  		/* Family */
> >  		Altera_SoCFPGA,
> >  		/* Interface type */
> > @@ -104,6 +117,7 @@ static Altera_desc altera_fpga[] = {
> >  		NULL,
> >  		/* No cookie implementation */
> >  		0
> > +#endif
> >  	},
> >  };
> >  
> > diff --git a/arch/arm/mach-socfpga/misc_s10.c b/arch/arm/mach-
> > socfpga/misc_s10.c
> > index b1cc6ca..012d8f6 100644
> > --- a/arch/arm/mach-socfpga/misc_s10.c
> > +++ b/arch/arm/mach-socfpga/misc_s10.c
> > @@ -71,6 +71,10 @@ int arch_misc_init(void)
> >  
> >  int arch_early_init_r(void)
> >  {
> > +#ifdef CONFIG_FPGA
> > +	socfpga_fpga_add();
> > +#endif
> > +
> >  	return 0;
> >  }
> >  
> > diff --git a/drivers/fpga/altera.c b/drivers/fpga/altera.c
> > index 135a357..b662ff5 100644
> > --- a/drivers/fpga/altera.c
> > +++ b/drivers/fpga/altera.c
> > @@ -40,6 +40,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
> > @@ -155,6 +158,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 48d3eb7..e9ba47a 100644
> > --- a/include/altera.h
> > +++ b/include/altera.h
> > @@ -40,6 +40,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,
> >  };
> > @@ -55,6 +57,8 @@ enum altera_family {
> >  	Altera_StratixII,
> >  	/* StratixV Family */
> >  	Altera_StratixV,
> > +	/* Stratix10 Family */
> > +	Intel_FPGA_Stratix10,
> >  	/* SoCFPGA Family */
> >  	Altera_SoCFPGA,
> >  
> > @@ -117,4 +121,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);
> What is this doing here ?
When user issue 'fpga load' command at uboot command prompt to do FPGA
programming, this call back function will be invoked.
> 
> > 
> > +#endif
> > +
> >  #endif /* _ALTERA_H_ */
> > 
> 

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

[Marek Vasut]

On 04/26/2018 08:15 AM, Ang, Chee Hong wrote:
> On Fri, 2018-04-20 at 05:42 +0200, Marek Vasut wrote:
>> On 04/20/2018 05:26 AM, chee.hong.ang at intel.com wrote:
>>>
>>> From: Chee Hong Ang <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: Chee Hong Ang <chee.hong.ang@intel.com>
>>> ---
>>>  arch/arm/mach-socfpga/misc.c     | 20 +++++++++++++++++---
>>>  arch/arm/mach-socfpga/misc_s10.c |  4 ++++
>>>  drivers/fpga/altera.c            |  6 ++++++
>>>  include/altera.h                 |  8 ++++++++
>>>  4 files changed, 35 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-
>>> socfpga/misc.c
>>> index d15cbc7..e36c686 100644
>>> --- a/arch/arm/mach-socfpga/misc.c
>>> +++ b/arch/arm/mach-socfpga/misc.c
>>> @@ -87,11 +87,24 @@ int overwrite_console(void)
>>>  #endif
>>>  
>>>  #ifdef CONFIG_FPGA
>>> -/*
>>> - * FPGA programming support for SoC FPGA Cyclone V
>>> - */
>>>  static Altera_desc altera_fpga[] = {
>>>  	{
>>> +#ifdef CONFIG_FPGA_STRATIX10
>> Create a separate structure for each FPGA family.
> Will be addressed in v2 patch.
>>
>>>
>>> +		/* FPGA programming support for SoC FPGA Stratix
>>> 10 */
>>> +		/* 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
>>> */
>>>  		/* Family */
>>>  		Altera_SoCFPGA,
>>>  		/* Interface type */
>>> @@ -104,6 +117,7 @@ static Altera_desc altera_fpga[] = {
>>>  		NULL,
>>>  		/* No cookie implementation */
>>>  		0
>>> +#endif
>>>  	},
>>>  };
>>>  
>>> diff --git a/arch/arm/mach-socfpga/misc_s10.c b/arch/arm/mach-
>>> socfpga/misc_s10.c
>>> index b1cc6ca..012d8f6 100644
>>> --- a/arch/arm/mach-socfpga/misc_s10.c
>>> +++ b/arch/arm/mach-socfpga/misc_s10.c
>>> @@ -71,6 +71,10 @@ int arch_misc_init(void)
>>>  
>>>  int arch_early_init_r(void)
>>>  {
>>> +#ifdef CONFIG_FPGA
>>> +	socfpga_fpga_add();
>>> +#endif
>>> +
>>>  	return 0;
>>>  }
>>>  
>>> diff --git a/drivers/fpga/altera.c b/drivers/fpga/altera.c
>>> index 135a357..b662ff5 100644
>>> --- a/drivers/fpga/altera.c
>>> +++ b/drivers/fpga/altera.c
>>> @@ -40,6 +40,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
>>> @@ -155,6 +158,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 48d3eb7..e9ba47a 100644
>>> --- a/include/altera.h
>>> +++ b/include/altera.h
>>> @@ -40,6 +40,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,
>>>  };
>>> @@ -55,6 +57,8 @@ enum altera_family {
>>>  	Altera_StratixII,
>>>  	/* StratixV Family */
>>>  	Altera_StratixV,
>>> +	/* Stratix10 Family */
>>> +	Intel_FPGA_Stratix10,
>>>  	/* SoCFPGA Family */
>>>  	Altera_SoCFPGA,
>>>  
>>> @@ -117,4 +121,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);
>> What is this doing here ?
> When user issue 'fpga load' command at uboot command prompt to do FPGA
> programming, this call back function will be invoked.

I mean, why is it part of this patch ? It's not implemented by this patch.

>>
>>>
>>> +#endif
>>> +
>>>  #endif /* _ALTERA_H_ */
>>>


-- 
Best regards,
Marek Vasut

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

[Ang, Chee Hong]

On Thu, 2018-04-26 at 14:38 +0200, Marek Vasut wrote:
> On 04/26/2018 08:15 AM, Ang, Chee Hong wrote:
> > 
> > On Fri, 2018-04-20 at 05:42 +0200, Marek Vasut wrote:
> > > 
> > > On 04/20/2018 05:26 AM, chee.hong.ang at intel.com wrote:
> > > > 
> > > > 
> > > > From: Chee Hong Ang <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: Chee Hong Ang <chee.hong.ang@intel.com>
> > > > ---
> > > >  arch/arm/mach-socfpga/misc.c     | 20 +++++++++++++++++---
> > > >  arch/arm/mach-socfpga/misc_s10.c |  4 ++++
> > > >  drivers/fpga/altera.c            |  6 ++++++
> > > >  include/altera.h                 |  8 ++++++++
> > > >  4 files changed, 35 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-
> > > > socfpga/misc.c
> > > > index d15cbc7..e36c686 100644
> > > > --- a/arch/arm/mach-socfpga/misc.c
> > > > +++ b/arch/arm/mach-socfpga/misc.c
> > > > @@ -87,11 +87,24 @@ int overwrite_console(void)
> > > >  #endif
> > > >  
> > > >  #ifdef CONFIG_FPGA
> > > > -/*
> > > > - * FPGA programming support for SoC FPGA Cyclone V
> > > > - */
> > > >  static Altera_desc altera_fpga[] = {
> > > >  	{
> > > > +#ifdef CONFIG_FPGA_STRATIX10
> > > Create a separate structure for each FPGA family.
> > Will be addressed in v2 patch.
> > > 
> > > 
> > > > 
> > > > 
> > > > +		/* FPGA programming support for SoC FPGA
> > > > Stratix
> > > > 10 */
> > > > +		/* 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
> > > > */
> > > >  		/* Family */
> > > >  		Altera_SoCFPGA,
> > > >  		/* Interface type */
> > > > @@ -104,6 +117,7 @@ static Altera_desc altera_fpga[] = {
> > > >  		NULL,
> > > >  		/* No cookie implementation */
> > > >  		0
> > > > +#endif
> > > >  	},
> > > >  };
> > > >  
> > > > diff --git a/arch/arm/mach-socfpga/misc_s10.c b/arch/arm/mach-
> > > > socfpga/misc_s10.c
> > > > index b1cc6ca..012d8f6 100644
> > > > --- a/arch/arm/mach-socfpga/misc_s10.c
> > > > +++ b/arch/arm/mach-socfpga/misc_s10.c
> > > > @@ -71,6 +71,10 @@ int arch_misc_init(void)
> > > >  
> > > >  int arch_early_init_r(void)
> > > >  {
> > > > +#ifdef CONFIG_FPGA
> > > > +	socfpga_fpga_add();
> > > > +#endif
> > > > +
> > > >  	return 0;
> > > >  }
> > > >  
> > > > diff --git a/drivers/fpga/altera.c b/drivers/fpga/altera.c
> > > > index 135a357..b662ff5 100644
> > > > --- a/drivers/fpga/altera.c
> > > > +++ b/drivers/fpga/altera.c
> > > > @@ -40,6 +40,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
> > > > @@ -155,6 +158,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 48d3eb7..e9ba47a 100644
> > > > --- a/include/altera.h
> > > > +++ b/include/altera.h
> > > > @@ -40,6 +40,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,
> > > >  };
> > > > @@ -55,6 +57,8 @@ enum altera_family {
> > > >  	Altera_StratixII,
> > > >  	/* StratixV Family */
> > > >  	Altera_StratixV,
> > > > +	/* Stratix10 Family */
> > > > +	Intel_FPGA_Stratix10,
> > > >  	/* SoCFPGA Family */
> > > >  	Altera_SoCFPGA,
> > > >  
> > > > @@ -117,4 +121,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);
> > > What is this doing here ?
> > When user issue 'fpga load' command at uboot command prompt to do
> > FPGA
> > programming, this call back function will be invoked.
> I mean, why is it part of this patch ? It's not implemented by this
> patch.
It's implemented in 1/3 patch. This callback function is needed in
drivers/fpga/altera.c to perform device specific FPGA programming. I
can can move this function declaration into another header file.
> 
> > 
> > > 
> > > 
> > > > 
> > > > 
> > > > +#endif
> > > > +
> > > >  #endif /* _ALTERA_H_ */
> > > > 
> 

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

[Marek Vasut]

On 04/27/2018 07:31 AM, Ang, Chee Hong wrote:
> On Thu, 2018-04-26 at 14:38 +0200, Marek Vasut wrote:
>> On 04/26/2018 08:15 AM, Ang, Chee Hong wrote:
>>>
>>> On Fri, 2018-04-20 at 05:42 +0200, Marek Vasut wrote:
>>>>
>>>> On 04/20/2018 05:26 AM, chee.hong.ang at intel.com wrote:
>>>>>
>>>>>
>>>>> From: Chee Hong Ang <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: Chee Hong Ang <chee.hong.ang@intel.com>
>>>>> ---
>>>>>  arch/arm/mach-socfpga/misc.c     | 20 +++++++++++++++++---
>>>>>  arch/arm/mach-socfpga/misc_s10.c |  4 ++++
>>>>>  drivers/fpga/altera.c            |  6 ++++++
>>>>>  include/altera.h                 |  8 ++++++++
>>>>>  4 files changed, 35 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-
>>>>> socfpga/misc.c
>>>>> index d15cbc7..e36c686 100644
>>>>> --- a/arch/arm/mach-socfpga/misc.c
>>>>> +++ b/arch/arm/mach-socfpga/misc.c
>>>>> @@ -87,11 +87,24 @@ int overwrite_console(void)
>>>>>  #endif
>>>>>  
>>>>>  #ifdef CONFIG_FPGA
>>>>> -/*
>>>>> - * FPGA programming support for SoC FPGA Cyclone V
>>>>> - */
>>>>>  static Altera_desc altera_fpga[] = {
>>>>>  	{
>>>>> +#ifdef CONFIG_FPGA_STRATIX10
>>>> Create a separate structure for each FPGA family.
>>> Will be addressed in v2 patch.
>>>>
>>>>
>>>>>
>>>>>
>>>>> +		/* FPGA programming support for SoC FPGA
>>>>> Stratix
>>>>> 10 */
>>>>> +		/* 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
>>>>> */
>>>>>  		/* Family */
>>>>>  		Altera_SoCFPGA,
>>>>>  		/* Interface type */
>>>>> @@ -104,6 +117,7 @@ static Altera_desc altera_fpga[] = {
>>>>>  		NULL,
>>>>>  		/* No cookie implementation */
>>>>>  		0
>>>>> +#endif
>>>>>  	},
>>>>>  };
>>>>>  
>>>>> diff --git a/arch/arm/mach-socfpga/misc_s10.c b/arch/arm/mach-
>>>>> socfpga/misc_s10.c
>>>>> index b1cc6ca..012d8f6 100644
>>>>> --- a/arch/arm/mach-socfpga/misc_s10.c
>>>>> +++ b/arch/arm/mach-socfpga/misc_s10.c
>>>>> @@ -71,6 +71,10 @@ int arch_misc_init(void)
>>>>>  
>>>>>  int arch_early_init_r(void)
>>>>>  {
>>>>> +#ifdef CONFIG_FPGA
>>>>> +	socfpga_fpga_add();
>>>>> +#endif
>>>>> +
>>>>>  	return 0;
>>>>>  }
>>>>>  
>>>>> diff --git a/drivers/fpga/altera.c b/drivers/fpga/altera.c
>>>>> index 135a357..b662ff5 100644
>>>>> --- a/drivers/fpga/altera.c
>>>>> +++ b/drivers/fpga/altera.c
>>>>> @@ -40,6 +40,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
>>>>> @@ -155,6 +158,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 48d3eb7..e9ba47a 100644
>>>>> --- a/include/altera.h
>>>>> +++ b/include/altera.h
>>>>> @@ -40,6 +40,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,
>>>>>  };
>>>>> @@ -55,6 +57,8 @@ enum altera_family {
>>>>>  	Altera_StratixII,
>>>>>  	/* StratixV Family */
>>>>>  	Altera_StratixV,
>>>>> +	/* Stratix10 Family */
>>>>> +	Intel_FPGA_Stratix10,
>>>>>  	/* SoCFPGA Family */
>>>>>  	Altera_SoCFPGA,
>>>>>  
>>>>> @@ -117,4 +121,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);
>>>> What is this doing here ?
>>> When user issue 'fpga load' command at uboot command prompt to do
>>> FPGA
>>> programming, this call back function will be invoked.
>> I mean, why is it part of this patch ? It's not implemented by this
>> patch.
> It's implemented in 1/3 patch. This callback function is needed in
> drivers/fpga/altera.c to perform device specific FPGA programming. I
> can can move this function declaration into another header file.

If it's added in 1/3 , then this should also be in 1/3 .

-- 
Best regards,
Marek Vasut

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

[Ang, Chee Hong]

On Fri, 2018-04-27 at 09:08 +0200, Marek Vasut wrote:
> On 04/27/2018 07:31 AM, Ang, Chee Hong wrote:
> > 
> > On Thu, 2018-04-26 at 14:38 +0200, Marek Vasut wrote:
> > > 
> > > On 04/26/2018 08:15 AM, Ang, Chee Hong wrote:
> > > > 
> > > > 
> > > > On Fri, 2018-04-20 at 05:42 +0200, Marek Vasut wrote:
> > > > > 
> > > > > 
> > > > > On 04/20/2018 05:26 AM, chee.hong.ang at intel.com wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > From: Chee Hong Ang <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: Chee Hong Ang <chee.hong.ang@intel.com>
> > > > > > ---
> > > > > >  arch/arm/mach-socfpga/misc.c     | 20 +++++++++++++++++---
> > > > > >  arch/arm/mach-socfpga/misc_s10.c |  4 ++++
> > > > > >  drivers/fpga/altera.c            |  6 ++++++
> > > > > >  include/altera.h                 |  8 ++++++++
> > > > > >  4 files changed, 35 insertions(+), 3 deletions(-)
> > > > > > 
> > > > > > diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-
> > > > > > socfpga/misc.c
> > > > > > index d15cbc7..e36c686 100644
> > > > > > --- a/arch/arm/mach-socfpga/misc.c
> > > > > > +++ b/arch/arm/mach-socfpga/misc.c
> > > > > > @@ -87,11 +87,24 @@ int overwrite_console(void)
> > > > > >  #endif
> > > > > >  
> > > > > >  #ifdef CONFIG_FPGA
> > > > > > -/*
> > > > > > - * FPGA programming support for SoC FPGA Cyclone V
> > > > > > - */
> > > > > >  static Altera_desc altera_fpga[] = {
> > > > > >  	{
> > > > > > +#ifdef CONFIG_FPGA_STRATIX10
> > > > > Create a separate structure for each FPGA family.
> > > > Will be addressed in v2 patch.
> > > > > 
> > > > > 
> > > > > 
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > +		/* FPGA programming support for SoC FPGA
> > > > > > Stratix
> > > > > > 10 */
> > > > > > +		/* 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
> > > > > > */
> > > > > >  		/* Family */
> > > > > >  		Altera_SoCFPGA,
> > > > > >  		/* Interface type */
> > > > > > @@ -104,6 +117,7 @@ static Altera_desc altera_fpga[] = {
> > > > > >  		NULL,
> > > > > >  		/* No cookie implementation */
> > > > > >  		0
> > > > > > +#endif
> > > > > >  	},
> > > > > >  };
> > > > > >  
> > > > > > diff --git a/arch/arm/mach-socfpga/misc_s10.c
> > > > > > b/arch/arm/mach-
> > > > > > socfpga/misc_s10.c
> > > > > > index b1cc6ca..012d8f6 100644
> > > > > > --- a/arch/arm/mach-socfpga/misc_s10.c
> > > > > > +++ b/arch/arm/mach-socfpga/misc_s10.c
> > > > > > @@ -71,6 +71,10 @@ int arch_misc_init(void)
> > > > > >  
> > > > > >  int arch_early_init_r(void)
> > > > > >  {
> > > > > > +#ifdef CONFIG_FPGA
> > > > > > +	socfpga_fpga_add();
> > > > > > +#endif
> > > > > > +
> > > > > >  	return 0;
> > > > > >  }
> > > > > >  
> > > > > > diff --git a/drivers/fpga/altera.c b/drivers/fpga/altera.c
> > > > > > index 135a357..b662ff5 100644
> > > > > > --- a/drivers/fpga/altera.c
> > > > > > +++ b/drivers/fpga/altera.c
> > > > > > @@ -40,6 +40,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
> > > > > > @@ -155,6 +158,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 48d3eb7..e9ba47a 100644
> > > > > > --- a/include/altera.h
> > > > > > +++ b/include/altera.h
> > > > > > @@ -40,6 +40,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,
> > > > > >  };
> > > > > > @@ -55,6 +57,8 @@ enum altera_family {
> > > > > >  	Altera_StratixII,
> > > > > >  	/* StratixV Family */
> > > > > >  	Altera_StratixV,
> > > > > > +	/* Stratix10 Family */
> > > > > > +	Intel_FPGA_Stratix10,
> > > > > >  	/* SoCFPGA Family */
> > > > > >  	Altera_SoCFPGA,
> > > > > >  
> > > > > > @@ -117,4 +121,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);
> > > > > What is this doing here ?
> > > > When user issue 'fpga load' command at uboot command prompt to
> > > > do
> > > > FPGA
> > > > programming, this call back function will be invoked.
> > > I mean, why is it part of this patch ? It's not implemented by
> > > this
> > > patch.
> > It's implemented in 1/3 patch. This callback function is needed in
> > drivers/fpga/altera.c to perform device specific FPGA programming.
> > I
> > can can move this function declaration into another header file.
> If it's added in 1/3 , then this should also be in 1/3 .
OK. I will move this function declaration to 1/3 in v2 patch.
> 

[U-Boot] [PATCH v1 3/3] arm: socfpga: stratix10: Enable Stratix10 FPGA Reconfiguration

[chee.hong.ang at intel.com]

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

Enable Stratix10 FPGA reconfiguration support in defconfig.

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

diff --git a/configs/socfpga_stratix10_defconfig b/configs/socfpga_stratix10_defconfig
index 46b7999..2414f4e 100644
--- a/configs/socfpga_stratix10_defconfig
+++ b/configs/socfpga_stratix10_defconfig
@@ -9,6 +9,8 @@ CONFIG_SPL_FAT_SUPPORT=y
 CONFIG_DEFAULT_DEVICE_TREE="socfpga_stratix10_socdk"
 CONFIG_BOOTDELAY=5
 CONFIG_HUSH_PARSER=y
+CONFIG_FPGA_ALTERA=y
+CONFIG_FPGA_STRATIX10=y
 CONFIG_SYS_PROMPT="SOCFPGA_STRATIX10 # "
 CONFIG_CMD_MEMTEST=y
 # CONFIG_CMD_FLASH is not set
-- 
2.2.0

[U-Boot] [PATCH v1 3/3] arm: socfpga: stratix10: Enable Stratix10 FPGA Reconfiguration

[Marek Vasut]

On 04/20/2018 05:26 AM, chee.hong.ang at intel.com wrote:
> From: Chee Hong Ang <chee.hong.ang@intel.com>
> 
> Enable Stratix10 FPGA reconfiguration support in defconfig.
> 
> Signed-off-by: Chee Hong Ang <chee.hong.ang@intel.com>
> ---
>  configs/socfpga_stratix10_defconfig | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/configs/socfpga_stratix10_defconfig b/configs/socfpga_stratix10_defconfig
> index 46b7999..2414f4e 100644
> --- a/configs/socfpga_stratix10_defconfig
> +++ b/configs/socfpga_stratix10_defconfig
> @@ -9,6 +9,8 @@ CONFIG_SPL_FAT_SUPPORT=y
>  CONFIG_DEFAULT_DEVICE_TREE="socfpga_stratix10_socdk"
>  CONFIG_BOOTDELAY=5
>  CONFIG_HUSH_PARSER=y
> +CONFIG_FPGA_ALTERA=y
> +CONFIG_FPGA_STRATIX10=y

CONFIG_FPGA_STRATIX10 should select FPGA_ALTERA in Kconfig

>  CONFIG_SYS_PROMPT="SOCFPGA_STRATIX10 # "
>  CONFIG_CMD_MEMTEST=y
>  # CONFIG_CMD_FLASH is not set
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v1 3/3] arm: socfpga: stratix10: Enable Stratix10 FPGA Reconfiguration

[Ang, Chee Hong]

On Fri, 2018-04-20 at 05:43 +0200, Marek Vasut wrote:
> On 04/20/2018 05:26 AM, chee.hong.ang at intel.com wrote:
> > 
> > From: Chee Hong Ang <chee.hong.ang@intel.com>
> > 
> > Enable Stratix10 FPGA reconfiguration support in defconfig.
> > 
> > Signed-off-by: Chee Hong Ang <chee.hong.ang@intel.com>
> > ---
> >  configs/socfpga_stratix10_defconfig | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/configs/socfpga_stratix10_defconfig
> > b/configs/socfpga_stratix10_defconfig
> > index 46b7999..2414f4e 100644
> > --- a/configs/socfpga_stratix10_defconfig
> > +++ b/configs/socfpga_stratix10_defconfig
> > @@ -9,6 +9,8 @@ CONFIG_SPL_FAT_SUPPORT=y
> >  CONFIG_DEFAULT_DEVICE_TREE="socfpga_stratix10_socdk"
> >  CONFIG_BOOTDELAY=5
> >  CONFIG_HUSH_PARSER=y
> > +CONFIG_FPGA_ALTERA=y
> > +CONFIG_FPGA_STRATIX10=y
> CONFIG_FPGA_STRATIX10 should select FPGA_ALTERA in Kconfig
Will be addressed in v2 patch.
> 
> > 
> >  CONFIG_SYS_PROMPT="SOCFPGA_STRATIX10 # "
> >  CONFIG_CMD_MEMTEST=y
> >  # CONFIG_CMD_FLASH is not set
> > 
>