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

Ang, Chee Hong chee.hong.ang at intel.com
Thu Apr 26 06:12:55 UTC 2018


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 at intel.com>
> > 
> > Enable FPGA reconfiguration support on Stratix10 SoC.
> > 
> > Signed-off-by: Chee Hong Ang <chee.hong.ang at 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;
> > +}
> > 
> 


More information about the U-Boot mailing list