[U-Boot] [PATCH 0/5] Virtex2 FPGA enhancements
These changes add support for slave serial mode, in addition to the existing slave SelectMAP mode, for programming Xilinx Virtex2 (and later) FPGAs, as well as fixing up code style and an issue with the programming sequence.
Robert Hancock (5): fpga: virtex2: cosmetic: Cleanup code style fpga: virtex2: added Kconfig option fpga: virtex2: Split out image writing from pre/post operations fpga: virtex2: Add additional clock cycles after DONE assertion fpga: virtex2: Add slave serial programming support
drivers/fpga/Kconfig | 8 + drivers/fpga/virtex2.c | 503 +++++++++++++++++++++++++++++-------------------- include/virtex2.h | 13 +- 3 files changed, 313 insertions(+), 211 deletions(-)
Address Checkpatch warnings in virtex2 code prior to making other changes. No functional change intended.
Signed-off-by: Robert Hancock hancock@sedsystems.ca --- drivers/fpga/virtex2.c | 270 +++++++++++++++++++++++++------------------------ 1 file changed, 136 insertions(+), 134 deletions(-)
diff --git a/drivers/fpga/virtex2.c b/drivers/fpga/virtex2.c index 02773d6..b01a31f 100644 --- a/drivers/fpga/virtex2.c +++ b/drivers/fpga/virtex2.c @@ -19,16 +19,16 @@ #endif
#ifdef FPGA_DEBUG -#define PRINTF(fmt,args...) printf (fmt ,##args) +#define PRINTF(fmt, args...) printf(fmt, ##args) #else -#define PRINTF(fmt,args...) +#define PRINTF(fmt, args...) #endif
/* * If the SelectMap interface can be overrun by the processor, define - * CONFIG_SYS_FPGA_CHECK_BUSY and/or CONFIG_FPGA_DELAY in the board configuration - * file and add board-specific support for checking BUSY status. By default, - * assume that the SelectMap interface cannot be overrun. + * CONFIG_SYS_FPGA_CHECK_BUSY and/or CONFIG_FPGA_DELAY in the board + * configuration file and add board-specific support for checking BUSY status. + * By default, assume that the SelectMap interface cannot be overrun. */ #ifndef CONFIG_SYS_FPGA_CHECK_BUSY #undef CONFIG_SYS_FPGA_CHECK_BUSY @@ -65,7 +65,7 @@ * an XC2V1000, if anyone can ever get ahold of one. */ #ifndef CONFIG_SYS_FPGA_WAIT_INIT -#define CONFIG_SYS_FPGA_WAIT_INIT CONFIG_SYS_HZ/2 /* 500 ms */ +#define CONFIG_SYS_FPGA_WAIT_INIT CONFIG_SYS_HZ / 2 /* 500 ms */ #endif
/* @@ -74,14 +74,14 @@ * clock frequencies (i.e. 66 MHz or less), BUSY monitoring is unnecessary. */ #ifndef CONFIG_SYS_FPGA_WAIT_BUSY -#define CONFIG_SYS_FPGA_WAIT_BUSY CONFIG_SYS_HZ/200 /* 5 ms*/ +#define CONFIG_SYS_FPGA_WAIT_BUSY CONFIG_SYS_HZ / 200 /* 5 ms*/ #endif
/* Default timeout for waiting for FPGA to enter operational mode after * configuration data has been written. */ #ifndef CONFIG_SYS_FPGA_WAIT_CONFIG -#define CONFIG_SYS_FPGA_WAIT_CONFIG CONFIG_SYS_HZ/5 /* 200 ms */ +#define CONFIG_SYS_FPGA_WAIT_CONFIG CONFIG_SYS_HZ / 5 /* 200 ms */ #endif
static int virtex2_ssm_load(xilinx_desc *desc, const void *buf, size_t bsize); @@ -97,18 +97,18 @@ static int virtex2_load(xilinx_desc *desc, const void *buf, size_t bsize,
switch (desc->iface) { case slave_serial: - PRINTF ("%s: Launching Slave Serial Load\n", __FUNCTION__); + PRINTF("%s: Launching Slave Serial Load\n", __func__); ret_val = virtex2_ss_load(desc, buf, bsize); break;
case slave_selectmap: - PRINTF ("%s: Launching Slave Parallel Load\n", __FUNCTION__); + PRINTF("%s: Launching Slave Parallel Load\n", __func__); ret_val = virtex2_ssm_load(desc, buf, bsize); break;
default: - printf ("%s: Unsupported interface type, %d\n", - __FUNCTION__, desc->iface); + printf("%s: Unsupported interface type, %d\n", + __func__, desc->iface); } return ret_val; } @@ -119,18 +119,18 @@ static int virtex2_dump(xilinx_desc *desc, const void *buf, size_t bsize)
switch (desc->iface) { case slave_serial: - PRINTF ("%s: Launching Slave Serial Dump\n", __FUNCTION__); + PRINTF("%s: Launching Slave Serial Dump\n", __func__); ret_val = virtex2_ss_dump(desc, buf, bsize); break;
case slave_parallel: - PRINTF ("%s: Launching Slave Parallel Dump\n", __FUNCTION__); + PRINTF("%s: Launching Slave Parallel Dump\n", __func__); ret_val = virtex2_ssm_dump(desc, buf, bsize); break;
default: - printf ("%s: Unsupported interface type, %d\n", - __FUNCTION__, desc->iface); + printf("%s: Unsupported interface type, %d\n", + __func__, desc->iface); } return ret_val; } @@ -159,135 +159,135 @@ static int virtex2_ssm_load(xilinx_desc *desc, const void *buf, size_t bsize) int ret_val = FPGA_FAIL; xilinx_virtex2_slave_selectmap_fns *fn = desc->iface_fns;
- PRINTF ("%s:%d: Start with interface functions @ 0x%p\n", - __FUNCTION__, __LINE__, fn); + PRINTF("%s:%d: Start with interface functions @ 0x%p\n", + __func__, __LINE__, fn);
if (fn) { size_t bytecount = 0; - unsigned char *data = (unsigned char *) buf; + unsigned char *data = (unsigned char *)buf; int cookie = desc->cookie; unsigned long ts;
/* Gotta split this one up (so the stack won't blow??) */ - PRINTF ("%s:%d: Function Table:\n" - " base 0x%p\n" - " struct 0x%p\n" - " pre 0x%p\n" - " prog 0x%p\n" - " init 0x%p\n" - " error 0x%p\n", - __FUNCTION__, __LINE__, - &fn, fn, fn->pre, fn->pgm, fn->init, fn->err); - PRINTF (" clock 0x%p\n" - " cs 0x%p\n" - " write 0x%p\n" - " rdata 0x%p\n" - " wdata 0x%p\n" - " busy 0x%p\n" - " abort 0x%p\n" - " post 0x%p\n\n", - fn->clk, fn->cs, fn->wr, fn->rdata, fn->wdata, - fn->busy, fn->abort, fn->post); + PRINTF("%s:%d: Function Table:\n" + " base 0x%p\n" + " struct 0x%p\n" + " pre 0x%p\n" + " prog 0x%p\n" + " init 0x%p\n" + " error 0x%p\n", + __func__, __LINE__, + &fn, fn, fn->pre, fn->pgm, fn->init, fn->err); + PRINTF(" clock 0x%p\n" + " cs 0x%p\n" + " write 0x%p\n" + " rdata 0x%p\n" + " wdata 0x%p\n" + " busy 0x%p\n" + " abort 0x%p\n" + " post 0x%p\n\n", + fn->clk, fn->cs, fn->wr, fn->rdata, fn->wdata, + fn->busy, fn->abort, fn->post);
#ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK - printf ("Initializing FPGA Device %d...\n", cookie); + printf("Initializing FPGA Device %d...\n", cookie); #endif /* * Run the pre configuration function if there is one. */ - if (*fn->pre) { - (*fn->pre) (cookie); - } + if (*fn->pre) + (*fn->pre)(cookie);
/* * Assert the program line. The minimum pulse width for - * Virtex II devices is 300 nS (Tprogram parameter in datasheet). - * There is no maximum value for the pulse width. Check to make - * sure that INIT_B goes low after assertion of PROG_B + * Virtex II devices is 300 nS (Tprogram parameter in + * datasheet). There is no maximum value for the pulse width. + * Check to make sure that INIT_B goes low after assertion of + * PROG_B */ - (*fn->pgm) (true, true, cookie); - udelay (10); - ts = get_timer (0); + (*fn->pgm)(true, true, cookie); + udelay(10); + ts = get_timer(0); do { - if (get_timer (ts) > CONFIG_SYS_FPGA_WAIT_INIT) { - printf ("%s:%d: ** Timeout after %d ticks waiting for INIT" - " to assert.\n", __FUNCTION__, __LINE__, - CONFIG_SYS_FPGA_WAIT_INIT); - (*fn->abort) (cookie); + if (get_timer(ts) > CONFIG_SYS_FPGA_WAIT_INIT) { + printf("%s:%d: ** Timeout after %d ticks waiting for INIT to assert.\n", + __func__, __LINE__, + CONFIG_SYS_FPGA_WAIT_INIT); + (*fn->abort)(cookie); return FPGA_FAIL; } - } while (!(*fn->init) (cookie)); + } while (!(*fn->init)(cookie));
- (*fn->pgm) (false, true, cookie); - CONFIG_FPGA_DELAY (); - (*fn->clk) (true, true, cookie); + (*fn->pgm)(false, true, cookie); + CONFIG_FPGA_DELAY(); + (*fn->clk)(true, true, cookie);
/* * Start a timer and wait for INIT_B to go high */ - ts = get_timer (0); + ts = get_timer(0); do { - CONFIG_FPGA_DELAY (); - if (get_timer (ts) > CONFIG_SYS_FPGA_WAIT_INIT) { - printf ("%s:%d: ** Timeout after %d ticks waiting for INIT" - " to deassert.\n", __FUNCTION__, __LINE__, - CONFIG_SYS_FPGA_WAIT_INIT); - (*fn->abort) (cookie); + CONFIG_FPGA_DELAY(); + if (get_timer(ts) > CONFIG_SYS_FPGA_WAIT_INIT) { + printf("%s:%d: ** Timeout after %d ticks waiting for INIT to deassert.\n", + __func__, __LINE__, + CONFIG_SYS_FPGA_WAIT_INIT); + (*fn->abort)(cookie); return FPGA_FAIL; } - } while ((*fn->init) (cookie) && (*fn->busy) (cookie)); + } while ((*fn->init)(cookie) && (*fn->busy)(cookie));
- (*fn->wr) (true, true, cookie); - (*fn->cs) (true, true, cookie); + (*fn->wr)(true, true, cookie); + (*fn->cs)(true, true, cookie);
- udelay (10000); + mdelay(10);
/* * Load the data byte by byte */ while (bytecount < bsize) { #ifdef CONFIG_SYS_FPGA_CHECK_CTRLC - if (ctrlc ()) { - (*fn->abort) (cookie); + if (ctrlc()) { + (*fn->abort)(cookie); return FPGA_FAIL; } #endif
- if ((*fn->done) (cookie) == FPGA_SUCCESS) { - PRINTF ("%s:%d:done went active early, bytecount = %d\n", - __FUNCTION__, __LINE__, bytecount); - break; + if ((*fn->done)(cookie) == FPGA_SUCCESS) { + PRINTF("%s:%d:done went active early, bytecount = %d\n", + __func__, __LINE__, bytecount); + break; }
#ifdef CONFIG_SYS_FPGA_CHECK_ERROR - if ((*fn->init) (cookie)) { - printf ("\n%s:%d: ** Error: INIT asserted during" - " configuration\n", __FUNCTION__, __LINE__); - printf ("%d = buffer offset, %d = buffer size\n", - bytecount, bsize); - (*fn->abort) (cookie); + if ((*fn->init)(cookie)) { + printf("\n%s:%d: ** Error: INIT asserted during configuration\n", + __func__, __LINE__); + printf("%d = buffer offset, %d = buffer size\n", + bytecount, bsize); + (*fn->abort)(cookie); return FPGA_FAIL; } #endif
- (*fn->wdata) (data[bytecount++], true, cookie); - CONFIG_FPGA_DELAY (); + (*fn->wdata)(data[bytecount++], true, cookie); + CONFIG_FPGA_DELAY();
/* * Cycle the clock pin */ - (*fn->clk) (false, true, cookie); - CONFIG_FPGA_DELAY (); - (*fn->clk) (true, true, cookie); + (*fn->clk)(false, true, cookie); + CONFIG_FPGA_DELAY(); + (*fn->clk)(true, true, cookie);
#ifdef CONFIG_SYS_FPGA_CHECK_BUSY - ts = get_timer (0); - while ((*fn->busy) (cookie)) { - if (get_timer (ts) > CONFIG_SYS_FPGA_WAIT_BUSY) { - printf ("%s:%d: ** Timeout after %d ticks waiting for" - " BUSY to deassert\n", - __FUNCTION__, __LINE__, CONFIG_SYS_FPGA_WAIT_BUSY); - (*fn->abort) (cookie); + ts = get_timer(0); + while ((*fn->busy)(cookie)) { + if (get_timer(ts) > CONFIG_SYS_FPGA_WAIT_BUSY) { + printf("%s:%d: ** Timeout after %d ticks waiting for BUSY to deassert\n", + __func__, __LINE__, + CONFIG_SYS_FPGA_WAIT_BUSY); + (*fn->abort)(cookie); return FPGA_FAIL; } } @@ -295,33 +295,35 @@ static int virtex2_ssm_load(xilinx_desc *desc, const void *buf, size_t bsize)
#ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK if (bytecount % (bsize / 40) == 0) - putc ('.'); + putc('.'); #endif }
/* - * Finished writing the data; deassert FPGA CS_B and WRITE_B signals. + * Finished writing the data; deassert FPGA CS_B and WRITE_B + * signals. */ - CONFIG_FPGA_DELAY (); - (*fn->cs) (false, true, cookie); - (*fn->wr) (false, true, cookie); + CONFIG_FPGA_DELAY(); + (*fn->cs)(false, true, cookie); + (*fn->wr)(false, true, cookie);
#ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK - putc ('\n'); + putc('\n'); #endif
/* - * Check for successful configuration. FPGA INIT_B and DONE should - * both be high upon successful configuration. + * Check for successful configuration. FPGA INIT_B and DONE + * should both be high upon successful configuration. */ - ts = get_timer (0); + ts = get_timer(0); ret_val = FPGA_SUCCESS; - while (((*fn->done) (cookie) == FPGA_FAIL) || (*fn->init) (cookie)) { - if (get_timer (ts) > CONFIG_SYS_FPGA_WAIT_CONFIG) { - printf ("%s:%d: ** Timeout after %d ticks waiting for DONE to" - "assert and INIT to deassert\n", - __FUNCTION__, __LINE__, CONFIG_SYS_FPGA_WAIT_CONFIG); - (*fn->abort) (cookie); + while (((*fn->done)(cookie) == FPGA_FAIL) || + (*fn->init)(cookie)) { + if (get_timer(ts) > CONFIG_SYS_FPGA_WAIT_CONFIG) { + printf("%s:%d: ** Timeout after %d ticks waiting for DONE toassert and INIT to deassert\n", + __func__, __LINE__, + CONFIG_SYS_FPGA_WAIT_CONFIG); + (*fn->abort)(cookie); ret_val = FPGA_FAIL; break; } @@ -329,23 +331,23 @@ static int virtex2_ssm_load(xilinx_desc *desc, const void *buf, size_t bsize)
if (ret_val == FPGA_SUCCESS) { #ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK - printf ("Initialization of FPGA device %d complete\n", cookie); + printf("Initialization of FPGA device %d complete\n", + cookie); #endif /* * Run the post configuration function if there is one. */ - if (*fn->post) { - (*fn->post) (cookie); - } + if (*fn->post) + (*fn->post)(cookie); } else { #ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK - printf ("** Initialization of FPGA device %d FAILED\n", - cookie); + printf("** Initialization of FPGA device %d FAILED\n", + cookie); #endif } } else { - printf ("%s:%d: NULL Interface function table!\n", - __FUNCTION__, __LINE__); + printf("%s:%d: NULL Interface function table!\n", + __func__, __LINE__); } return ret_val; } @@ -359,61 +361,61 @@ static int virtex2_ssm_dump(xilinx_desc *desc, const void *buf, size_t bsize) xilinx_virtex2_slave_selectmap_fns *fn = desc->iface_fns;
if (fn) { - unsigned char *data = (unsigned char *) buf; + unsigned char *data = (unsigned char *)buf; size_t bytecount = 0; int cookie = desc->cookie;
- printf ("Starting Dump of FPGA Device %d...\n", cookie); + printf("Starting Dump of FPGA Device %d...\n", cookie);
- (*fn->cs) (true, true, cookie); - (*fn->clk) (true, true, cookie); + (*fn->cs)(true, true, cookie); + (*fn->clk)(true, true, cookie);
while (bytecount < bsize) { #ifdef CONFIG_SYS_FPGA_CHECK_CTRLC - if (ctrlc ()) { - (*fn->abort) (cookie); + if (ctrlc()) { + (*fn->abort)(cookie); return FPGA_FAIL; } #endif /* * Cycle the clock and read the data */ - (*fn->clk) (false, true, cookie); - (*fn->clk) (true, true, cookie); - (*fn->rdata) (&(data[bytecount++]), cookie); + (*fn->clk)(false, true, cookie); + (*fn->clk)(true, true, cookie); + (*fn->rdata)(&data[bytecount++], cookie); #ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK if (bytecount % (bsize / 40) == 0) - putc ('.'); + putc('.'); #endif }
/* * Deassert CS_B and cycle the clock to deselect the device. */ - (*fn->cs) (false, false, cookie); - (*fn->clk) (false, true, cookie); - (*fn->clk) (true, true, cookie); + (*fn->cs)(false, false, cookie); + (*fn->clk)(false, true, cookie); + (*fn->clk)(true, true, cookie);
#ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK - putc ('\n'); + putc('\n'); #endif - puts ("Done.\n"); + puts("Done.\n"); } else { - printf ("%s:%d: NULL Interface function table!\n", - __FUNCTION__, __LINE__); + printf("%s:%d: NULL Interface function table!\n", + __func__, __LINE__); } return ret_val; }
static int virtex2_ss_load(xilinx_desc *desc, const void *buf, size_t bsize) { - printf ("%s: Slave Serial Loading is unsupported\n", __FUNCTION__); + printf("%s: Slave Serial Loading is unsupported\n", __func__); return FPGA_FAIL; }
static int virtex2_ss_dump(xilinx_desc *desc, const void *buf, size_t bsize) { - printf ("%s: Slave Serial Dumping is unsupported\n", __FUNCTION__); + printf("%s: Slave Serial Dumping is unsupported\n", __func__); return FPGA_FAIL; }
Add an option to allow this driver to be selected with Kconfig. As noted in the description, this driver should also work with many newer Xilinx FPGA families as the programming methods are essentially the same.
Also added a missing FPGA_XILINX dependency to the similar Spartan 3 driver.
Signed-off-by: Robert Hancock hancock@sedsystems.ca --- drivers/fpga/Kconfig | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig index 8f59193..105a299 100644 --- a/drivers/fpga/Kconfig +++ b/drivers/fpga/Kconfig @@ -58,9 +58,17 @@ config FPGA_ZYNQMPPL
config FPGA_SPARTAN3 bool "Enable Spartan3 FPGA driver" + depends on FPGA_XILINX help Enable Spartan3 FPGA driver for loading in BIT format.
+config FPGA_VIRTEX2 + bool "Enable Xilinx Virtex-II and later FPGA driver" + depends on FPGA_XILINX + help + Enable Virtex-II FPGA driver for loading in BIT format. This driver + also supports many newer Xilinx FPGA families. + config FPGA_ZYNQPL bool "Enable Xilinx FPGA for Zynq" depends on ARCH_ZYNQ
This is in preparation for adding slave serial programming support, which uses the same pre/post operations as slave SelectMAP, to avoid duplicating code.
Signed-off-by: Robert Hancock hancock@sedsystems.ca --- drivers/fpga/virtex2.c | 331 ++++++++++++++++++++++++++----------------------- 1 file changed, 174 insertions(+), 157 deletions(-)
diff --git a/drivers/fpga/virtex2.c b/drivers/fpga/virtex2.c index b01a31f..2383c69 100644 --- a/drivers/fpga/virtex2.c +++ b/drivers/fpga/virtex2.c @@ -154,202 +154,219 @@ static int virtex2_info(xilinx_desc *desc) * INIT_B and DONE lines. If both are high, configuration has * succeeded. Congratulations! */ -static int virtex2_ssm_load(xilinx_desc *desc, const void *buf, size_t bsize) +static int virtex2_slave_pre(xilinx_virtex2_slave_selectmap_fns *fn, int cookie) { - int ret_val = FPGA_FAIL; - xilinx_virtex2_slave_selectmap_fns *fn = desc->iface_fns; + unsigned long ts;
PRINTF("%s:%d: Start with interface functions @ 0x%p\n", __func__, __LINE__, fn);
- if (fn) { - size_t bytecount = 0; - unsigned char *data = (unsigned char *)buf; - int cookie = desc->cookie; - unsigned long ts; - - /* Gotta split this one up (so the stack won't blow??) */ - PRINTF("%s:%d: Function Table:\n" - " base 0x%p\n" - " struct 0x%p\n" - " pre 0x%p\n" - " prog 0x%p\n" - " init 0x%p\n" - " error 0x%p\n", - __func__, __LINE__, - &fn, fn, fn->pre, fn->pgm, fn->init, fn->err); - PRINTF(" clock 0x%p\n" - " cs 0x%p\n" - " write 0x%p\n" - " rdata 0x%p\n" - " wdata 0x%p\n" - " busy 0x%p\n" - " abort 0x%p\n" - " post 0x%p\n\n", - fn->clk, fn->cs, fn->wr, fn->rdata, fn->wdata, - fn->busy, fn->abort, fn->post); + if (!fn) { + printf("%s:%d: NULL Interface function table!\n", + __func__, __LINE__); + return FPGA_FAIL; + } + + /* Gotta split this one up (so the stack won't blow??) */ + PRINTF("%s:%d: Function Table:\n" + " base 0x%p\n" + " struct 0x%p\n" + " pre 0x%p\n" + " prog 0x%p\n" + " init 0x%p\n" + " error 0x%p\n", + __func__, __LINE__, + &fn, fn, fn->pre, fn->pgm, fn->init, fn->err); + PRINTF(" clock 0x%p\n" + " cs 0x%p\n" + " write 0x%p\n" + " rdata 0x%p\n" + " wdata 0x%p\n" + " busy 0x%p\n" + " abort 0x%p\n" + " post 0x%p\n\n", + fn->clk, fn->cs, fn->wr, fn->rdata, fn->wdata, + fn->busy, fn->abort, fn->post);
#ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK - printf("Initializing FPGA Device %d...\n", cookie); + printf("Initializing FPGA Device %d...\n", cookie); #endif - /* - * Run the pre configuration function if there is one. - */ - if (*fn->pre) - (*fn->pre)(cookie); - - /* - * Assert the program line. The minimum pulse width for - * Virtex II devices is 300 nS (Tprogram parameter in - * datasheet). There is no maximum value for the pulse width. - * Check to make sure that INIT_B goes low after assertion of - * PROG_B - */ - (*fn->pgm)(true, true, cookie); - udelay(10); - ts = get_timer(0); - do { - if (get_timer(ts) > CONFIG_SYS_FPGA_WAIT_INIT) { - printf("%s:%d: ** Timeout after %d ticks waiting for INIT to assert.\n", - __func__, __LINE__, - CONFIG_SYS_FPGA_WAIT_INIT); - (*fn->abort)(cookie); - return FPGA_FAIL; - } - } while (!(*fn->init)(cookie)); + /* + * Run the pre configuration function if there is one. + */ + if (*fn->pre) + (*fn->pre)(cookie); + + /* + * Assert the program line. The minimum pulse width for + * Virtex II devices is 300 nS (Tprogram parameter in datasheet). + * There is no maximum value for the pulse width. Check to make + * sure that INIT_B goes low after assertion of PROG_B + */ + (*fn->pgm)(true, true, cookie); + udelay(10); + ts = get_timer(0); + do { + if (get_timer(ts) > CONFIG_SYS_FPGA_WAIT_INIT) { + printf("%s:%d: ** Timeout after %d ticks waiting for INIT to assert.\n", + __func__, __LINE__, CONFIG_SYS_FPGA_WAIT_INIT); + (*fn->abort)(cookie); + return FPGA_FAIL; + } + } while (!(*fn->init)(cookie));
- (*fn->pgm)(false, true, cookie); - CONFIG_FPGA_DELAY(); + (*fn->pgm)(false, true, cookie); + CONFIG_FPGA_DELAY(); + if (fn->clk) (*fn->clk)(true, true, cookie);
- /* - * Start a timer and wait for INIT_B to go high - */ - ts = get_timer(0); - do { - CONFIG_FPGA_DELAY(); - if (get_timer(ts) > CONFIG_SYS_FPGA_WAIT_INIT) { - printf("%s:%d: ** Timeout after %d ticks waiting for INIT to deassert.\n", - __func__, __LINE__, - CONFIG_SYS_FPGA_WAIT_INIT); - (*fn->abort)(cookie); - return FPGA_FAIL; - } - } while ((*fn->init)(cookie) && (*fn->busy)(cookie)); + /* + * Start a timer and wait for INIT_B to go high + */ + ts = get_timer(0); + do { + CONFIG_FPGA_DELAY(); + if (get_timer(ts) > CONFIG_SYS_FPGA_WAIT_INIT) { + printf("%s:%d: ** Timeout after %d ticks waiting for INIT to deassert.\n", + __func__, __LINE__, CONFIG_SYS_FPGA_WAIT_INIT); + (*fn->abort)(cookie); + return FPGA_FAIL; + } + } while ((*fn->init)(cookie) && (*fn->busy)(cookie));
+ if (fn->wr) (*fn->wr)(true, true, cookie); + if (fn->cs) (*fn->cs)(true, true, cookie);
- mdelay(10); - - /* - * Load the data byte by byte - */ - while (bytecount < bsize) { -#ifdef CONFIG_SYS_FPGA_CHECK_CTRLC - if (ctrlc()) { - (*fn->abort)(cookie); - return FPGA_FAIL; - } -#endif + mdelay(10); + return FPGA_SUCCESS; +}
- if ((*fn->done)(cookie) == FPGA_SUCCESS) { - PRINTF("%s:%d:done went active early, bytecount = %d\n", - __func__, __LINE__, bytecount); - break; - } +static int virtex2_slave_post(xilinx_virtex2_slave_selectmap_fns *fn, + int cookie) +{ + int ret_val = FPGA_SUCCESS; + unsigned long ts; + + /* + * Finished writing the data; deassert FPGA CS_B and WRITE_B signals. + */ + CONFIG_FPGA_DELAY(); + if (fn->cs) + (*fn->cs)(false, true, cookie); + if (fn->wr) + (*fn->wr)(false, true, cookie);
-#ifdef CONFIG_SYS_FPGA_CHECK_ERROR - if ((*fn->init)(cookie)) { - printf("\n%s:%d: ** Error: INIT asserted during configuration\n", - __func__, __LINE__); - printf("%d = buffer offset, %d = buffer size\n", - bytecount, bsize); - (*fn->abort)(cookie); - return FPGA_FAIL; - } +#ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK + putc('\n'); #endif
- (*fn->wdata)(data[bytecount++], true, cookie); - CONFIG_FPGA_DELAY(); - - /* - * Cycle the clock pin - */ - (*fn->clk)(false, true, cookie); - CONFIG_FPGA_DELAY(); - (*fn->clk)(true, true, cookie); + /* + * Check for successful configuration. FPGA INIT_B and DONE + * should both be high upon successful configuration. + */ + ts = get_timer(0); + ret_val = FPGA_SUCCESS; + while (((*fn->done)(cookie) == FPGA_FAIL) || + (*fn->init)(cookie)) { + if (get_timer(ts) > CONFIG_SYS_FPGA_WAIT_CONFIG) { + printf("%s:%d: ** Timeout after %d ticks waiting for DONE to assert and INIT to deassert\n", + __func__, __LINE__, CONFIG_SYS_FPGA_WAIT_CONFIG); + (*fn->abort)(cookie); + ret_val = FPGA_FAIL; + break; + } + }
-#ifdef CONFIG_SYS_FPGA_CHECK_BUSY - ts = get_timer(0); - while ((*fn->busy)(cookie)) { - if (get_timer(ts) > CONFIG_SYS_FPGA_WAIT_BUSY) { - printf("%s:%d: ** Timeout after %d ticks waiting for BUSY to deassert\n", - __func__, __LINE__, - CONFIG_SYS_FPGA_WAIT_BUSY); - (*fn->abort)(cookie); - return FPGA_FAIL; - } - } + if (ret_val == FPGA_SUCCESS) { +#ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK + printf("Initialization of FPGA device %d complete\n", cookie); #endif - + /* + * Run the post configuration function if there is one. + */ + if (*fn->post) + (*fn->post)(cookie); + } else { #ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK - if (bytecount % (bsize / 40) == 0) - putc('.'); + printf("** Initialization of FPGA device %d FAILED\n", + cookie); #endif + } + return ret_val; +} + +static int virtex2_ssm_load(xilinx_desc *desc, const void *buf, size_t bsize) +{ + int ret_val = FPGA_FAIL; + xilinx_virtex2_slave_selectmap_fns *fn = desc->iface_fns; + size_t bytecount = 0; + unsigned char *data = (unsigned char *)buf; + int cookie = desc->cookie; + + ret_val = virtex2_slave_pre(fn, cookie); + if (ret_val != FPGA_SUCCESS) + return ret_val; + + /* + * Load the data byte by byte + */ + while (bytecount < bsize) { +#ifdef CONFIG_SYS_FPGA_CHECK_CTRLC + if (ctrlc()) { + (*fn->abort)(cookie); + return FPGA_FAIL; } +#endif
- /* - * Finished writing the data; deassert FPGA CS_B and WRITE_B - * signals. - */ - CONFIG_FPGA_DELAY(); - (*fn->cs)(false, true, cookie); - (*fn->wr)(false, true, cookie); + if ((*fn->done)(cookie) == FPGA_SUCCESS) { + PRINTF("%s:%d:done went active early, bytecount = %d\n", + __func__, __LINE__, bytecount); + break; + }
-#ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK - putc('\n'); +#ifdef CONFIG_SYS_FPGA_CHECK_ERROR + if ((*fn->init)(cookie)) { + printf("\n%s:%d: ** Error: INIT asserted during configuration\n", + __func__, __LINE__); + printf("%zu = buffer offset, %zu = buffer size\n", + bytecount, bsize); + (*fn->abort)(cookie); + return FPGA_FAIL; + } #endif
+ (*fn->wdata)(data[bytecount++], true, cookie); + CONFIG_FPGA_DELAY(); + /* - * Check for successful configuration. FPGA INIT_B and DONE - * should both be high upon successful configuration. + * Cycle the clock pin */ + (*fn->clk)(false, true, cookie); + CONFIG_FPGA_DELAY(); + (*fn->clk)(true, true, cookie); + +#ifdef CONFIG_SYS_FPGA_CHECK_BUSY ts = get_timer(0); - ret_val = FPGA_SUCCESS; - while (((*fn->done)(cookie) == FPGA_FAIL) || - (*fn->init)(cookie)) { - if (get_timer(ts) > CONFIG_SYS_FPGA_WAIT_CONFIG) { - printf("%s:%d: ** Timeout after %d ticks waiting for DONE toassert and INIT to deassert\n", + while ((*fn->busy)(cookie)) { + if (get_timer(ts) > CONFIG_SYS_FPGA_WAIT_BUSY) { + printf("%s:%d: ** Timeout after %d ticks waiting for BUSY to deassert\n", __func__, __LINE__, - CONFIG_SYS_FPGA_WAIT_CONFIG); + CONFIG_SYS_FPGA_WAIT_BUSY); (*fn->abort)(cookie); - ret_val = FPGA_FAIL; - break; + return FPGA_FAIL; } } - - if (ret_val == FPGA_SUCCESS) { -#ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK - printf("Initialization of FPGA device %d complete\n", - cookie); #endif - /* - * Run the post configuration function if there is one. - */ - if (*fn->post) - (*fn->post)(cookie); - } else { + #ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK - printf("** Initialization of FPGA device %d FAILED\n", - cookie); + if (bytecount % (bsize / 40) == 0) + putc('.'); #endif - } - } else { - printf("%s:%d: NULL Interface function table!\n", - __func__, __LINE__); } - return ret_val; + + return virtex2_slave_post(fn, cookie); }
/*
Some Xilinx FPGA configuration options can result in the startup sequence extending past the end of the FPGA bitstream. Continue applying CCLK clock cycles for 8 cycles after DONE is asserted in order to ensure the startup sequence is complete, as recommended by Xilinx.
Signed-off-by: Robert Hancock hancock@sedsystems.ca --- drivers/fpga/virtex2.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/fpga/virtex2.c b/drivers/fpga/virtex2.c index 2383c69..5028244 100644 --- a/drivers/fpga/virtex2.c +++ b/drivers/fpga/virtex2.c @@ -247,6 +247,7 @@ static int virtex2_slave_post(xilinx_virtex2_slave_selectmap_fns *fn, int cookie) { int ret_val = FPGA_SUCCESS; + int num_done = 0; unsigned long ts;
/* @@ -264,12 +265,18 @@ static int virtex2_slave_post(xilinx_virtex2_slave_selectmap_fns *fn,
/* * Check for successful configuration. FPGA INIT_B and DONE - * should both be high upon successful configuration. + * should both be high upon successful configuration. Continue pulsing + * clock with data set to all ones until DONE is asserted and for 8 + * clock cycles afterwards. */ ts = get_timer(0); - ret_val = FPGA_SUCCESS; - while (((*fn->done)(cookie) == FPGA_FAIL) || - (*fn->init)(cookie)) { + while (true) { + if ((*fn->done)(cookie) == FPGA_SUCCESS && + !((*fn->init)(cookie))) { + if (num_done++ >= 8) + break; + } + if (get_timer(ts) > CONFIG_SYS_FPGA_WAIT_CONFIG) { printf("%s:%d: ** Timeout after %d ticks waiting for DONE to assert and INIT to deassert\n", __func__, __LINE__, CONFIG_SYS_FPGA_WAIT_CONFIG); @@ -277,6 +284,11 @@ static int virtex2_slave_post(xilinx_virtex2_slave_selectmap_fns *fn, ret_val = FPGA_FAIL; break; } + (*fn->wdata) (0xff, true, cookie); + CONFIG_FPGA_DELAY(); + (*fn->clk) (false, true, cookie); + CONFIG_FPGA_DELAY(); + (*fn->clk) (true, true, cookie); }
if (ret_val == FPGA_SUCCESS) {
On 18. 06. 19 17:47, Robert Hancock wrote:
Some Xilinx FPGA configuration options can result in the startup sequence extending past the end of the FPGA bitstream. Continue applying CCLK clock cycles for 8 cycles after DONE is asserted in order to ensure the startup sequence is complete, as recommended by Xilinx.
Signed-off-by: Robert Hancock hancock@sedsystems.ca
drivers/fpga/virtex2.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/fpga/virtex2.c b/drivers/fpga/virtex2.c index 2383c69..5028244 100644 --- a/drivers/fpga/virtex2.c +++ b/drivers/fpga/virtex2.c @@ -247,6 +247,7 @@ static int virtex2_slave_post(xilinx_virtex2_slave_selectmap_fns *fn, int cookie) { int ret_val = FPGA_SUCCESS;
int num_done = 0; unsigned long ts;
/*
@@ -264,12 +265,18 @@ static int virtex2_slave_post(xilinx_virtex2_slave_selectmap_fns *fn,
/* * Check for successful configuration. FPGA INIT_B and DONE
* should both be high upon successful configuration.
* should both be high upon successful configuration. Continue pulsing* clock with data set to all ones until DONE is asserted and for 8* clock cycles afterwards.
- ret_val = FPGA_SUCCESS;
- while (((*fn->done)(cookie) == FPGA_FAIL) ||
(*fn->init)(cookie)) {
- while (true) {
if ((*fn->done)(cookie) == FPGA_SUCCESS &&!((*fn->init)(cookie))) {if (num_done++ >= 8)break;}- if (get_timer(ts) > CONFIG_SYS_FPGA_WAIT_CONFIG) { printf("%s:%d: ** Timeout after %d ticks waiting for DONE to assert and INIT to deassert\n", __func__, __LINE__, CONFIG_SYS_FPGA_WAIT_CONFIG);
@@ -277,6 +284,11 @@ static int virtex2_slave_post(xilinx_virtex2_slave_selectmap_fns *fn, ret_val = FPGA_FAIL; break; }
(*fn->wdata) (0xff, true, cookie);CONFIG_FPGA_DELAY();(*fn->clk) (false, true, cookie);CONFIG_FPGA_DELAY();(*fn->clk) (true, true, cookie);
There are some functions which check if function is present before it is called. I think that the whole driver should be sync in this to make sure that functions are really implemented if code expects them or fail.
But this can be done on the top of this series.
Thanks, Michal
This adds support for slave serial programming, in addition to the previously supported slave SelectMAP mode. There are two ways that this can be used:
-Using the clk and wdata callbacks in order to write image data one bit at a time using pure bit-banging. This works, but is rather painfully slow with typical image sizes.
-By specifying the wbulkdata callback instead, the image loading process can be offloaded to SPI hardware. In this mode the clk and wdata callbacks do not need to be specified. This allows the image to be loaded much faster, taking only a few seconds with even relatively large images.
Slave serial programming has been tested on the Kintex-7 series of FPGAs.
Signed-off-by: Robert Hancock hancock@sedsystems.ca --- drivers/fpga/virtex2.c | 96 +++++++++++++++++++++++++++++++++++++++++++------- include/virtex2.h | 13 ++----- 2 files changed, 86 insertions(+), 23 deletions(-)
diff --git a/drivers/fpga/virtex2.c b/drivers/fpga/virtex2.c index 5028244..3957368 100644 --- a/drivers/fpga/virtex2.c +++ b/drivers/fpga/virtex2.c @@ -3,6 +3,8 @@ * (C) Copyright 2002 * Rich Ireland, Enterasys Networks, rireland@enterasys.com. * Keith Outwater, keith_outwater@mvis.com + * + * Copyright (c) 2019 SED Systems, a division of Calian Ltd. */
/* @@ -141,8 +143,8 @@ static int virtex2_info(xilinx_desc *desc) }
/* - * Virtex-II Slave SelectMap configuration loader. Configuration via - * SelectMap is as follows: + * Virtex-II Slave SelectMap or Serial configuration loader. Configuration + * is as follows: * 1. Set the FPGA's PROG_B line low. * 2. Set the FPGA's PROG_B line high. Wait for INIT_B to go high. * 3. Write data to the SelectMap port. If INIT_B goes low at any time @@ -154,7 +156,7 @@ static int virtex2_info(xilinx_desc *desc) * INIT_B and DONE lines. If both are high, configuration has * succeeded. Congratulations! */ -static int virtex2_slave_pre(xilinx_virtex2_slave_selectmap_fns *fn, int cookie) +static int virtex2_slave_pre(xilinx_virtex2_slave_fns *fn, int cookie) { unsigned long ts;
@@ -243,7 +245,7 @@ static int virtex2_slave_pre(xilinx_virtex2_slave_selectmap_fns *fn, int cookie) return FPGA_SUCCESS; }
-static int virtex2_slave_post(xilinx_virtex2_slave_selectmap_fns *fn, +static int virtex2_slave_post(xilinx_virtex2_slave_fns *fn, int cookie) { int ret_val = FPGA_SUCCESS; @@ -284,11 +286,16 @@ static int virtex2_slave_post(xilinx_virtex2_slave_selectmap_fns *fn, ret_val = FPGA_FAIL; break; } - (*fn->wdata) (0xff, true, cookie); - CONFIG_FPGA_DELAY(); - (*fn->clk) (false, true, cookie); - CONFIG_FPGA_DELAY(); - (*fn->clk) (true, true, cookie); + if (fn->wbulkdata) { + unsigned char dummy = 0xff; + (*fn->wbulkdata)(&dummy, 1, true, cookie); + } else { + (*fn->wdata)(0xff, true, cookie); + CONFIG_FPGA_DELAY(); + (*fn->clk)(false, true, cookie); + CONFIG_FPGA_DELAY(); + (*fn->clk)(true, true, cookie); + } }
if (ret_val == FPGA_SUCCESS) { @@ -312,7 +319,7 @@ static int virtex2_slave_post(xilinx_virtex2_slave_selectmap_fns *fn, static int virtex2_ssm_load(xilinx_desc *desc, const void *buf, size_t bsize) { int ret_val = FPGA_FAIL; - xilinx_virtex2_slave_selectmap_fns *fn = desc->iface_fns; + xilinx_virtex2_slave_fns *fn = desc->iface_fns; size_t bytecount = 0; unsigned char *data = (unsigned char *)buf; int cookie = desc->cookie; @@ -387,7 +394,7 @@ static int virtex2_ssm_load(xilinx_desc *desc, const void *buf, size_t bsize) static int virtex2_ssm_dump(xilinx_desc *desc, const void *buf, size_t bsize) { int ret_val = FPGA_FAIL; - xilinx_virtex2_slave_selectmap_fns *fn = desc->iface_fns; + xilinx_virtex2_slave_fns *fn = desc->iface_fns;
if (fn) { unsigned char *data = (unsigned char *)buf; @@ -438,8 +445,71 @@ static int virtex2_ssm_dump(xilinx_desc *desc, const void *buf, size_t bsize)
static int virtex2_ss_load(xilinx_desc *desc, const void *buf, size_t bsize) { - printf("%s: Slave Serial Loading is unsupported\n", __func__); - return FPGA_FAIL; + int ret_val = FPGA_FAIL; + xilinx_virtex2_slave_fns *fn = desc->iface_fns; + unsigned char *data = (unsigned char *)buf; + int cookie = desc->cookie; + + ret_val = virtex2_slave_pre(fn, cookie); + if (ret_val != FPGA_SUCCESS) + return ret_val; + + if (fn->wbulkdata) { + /* Load the data in a single chunk */ + (*fn->wbulkdata)(data, bsize, true, cookie); + } else { + size_t bytecount = 0; + + /* + * Load the data bit by bit + */ + while (bytecount < bsize) { + unsigned char curr_data = data[bytecount++]; + int bit; + +#ifdef CONFIG_SYS_FPGA_CHECK_CTRLC + if (ctrlc()) { + (*fn->abort) (cookie); + return FPGA_FAIL; + } +#endif + + if ((*fn->done)(cookie) == FPGA_SUCCESS) { + PRINTF("%s:%d:done went active early, bytecount = %d\n", + __func__, __LINE__, bytecount); + break; + } + +#ifdef CONFIG_SYS_FPGA_CHECK_ERROR + if ((*fn->init)(cookie)) { + printf("\n%s:%d: ** Error: INIT asserted during configuration\n", + __func__, __LINE__); + printf("%zu = buffer offset, %zu = buffer size\n", + bytecount, bsize); + (*fn->abort)(cookie); + return FPGA_FAIL; + } +#endif + + for (bit = 7; bit >= 0; --bit) { + unsigned char curr_bit = (curr_data >> bit) & 1; + (*fn->wdata)(curr_bit, true, cookie); + CONFIG_FPGA_DELAY(); + (*fn->clk)(false, true, cookie); + CONFIG_FPGA_DELAY(); + (*fn->clk)(true, true, cookie); + } + + /* Slave serial never uses a busy pin */ + +#ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK + if (bytecount % (bsize / 40) == 0) + putc('.'); +#endif + } + } + + return virtex2_slave_post(fn, cookie); }
static int virtex2_ss_dump(xilinx_desc *desc, const void *buf, size_t bsize) diff --git a/include/virtex2.h b/include/virtex2.h index a481130..7e8d93f 100644 --- a/include/virtex2.h +++ b/include/virtex2.h @@ -11,7 +11,7 @@ #include <xilinx.h>
/* - * Slave SelectMap Implementation function table. + * Slave SelectMap or Serial Implementation function table. */ typedef struct { xilinx_pre_fn pre; @@ -24,18 +24,11 @@ typedef struct { xilinx_wr_fn wr; xilinx_rdata_fn rdata; xilinx_wdata_fn wdata; + xilinx_bwr_fn wbulkdata; xilinx_busy_fn busy; xilinx_abort_fn abort; xilinx_post_fn post; -} xilinx_virtex2_slave_selectmap_fns; - -/* Slave Serial Implementation function table */ -typedef struct { - xilinx_pgm_fn pgm; - xilinx_clk_fn clk; - xilinx_rdata_fn rdata; - xilinx_wdata_fn wdata; -} xilinx_virtex2_slave_serial_fns; +} xilinx_virtex2_slave_fns;
#if defined(CONFIG_FPGA_VIRTEX2) extern struct xilinx_fpga_op virtex2_op;
On 18. 06. 19 17:47, Robert Hancock wrote:
These changes add support for slave serial mode, in addition to the existing slave SelectMAP mode, for programming Xilinx Virtex2 (and later) FPGAs, as well as fixing up code style and an issue with the programming sequence.
Robert Hancock (5): fpga: virtex2: cosmetic: Cleanup code style fpga: virtex2: added Kconfig option fpga: virtex2: Split out image writing from pre/post operations fpga: virtex2: Add additional clock cycles after DONE assertion fpga: virtex2: Add slave serial programming support
drivers/fpga/Kconfig | 8 + drivers/fpga/virtex2.c | 503 +++++++++++++++++++++++++++++-------------------- include/virtex2.h | 13 +- 3 files changed, 313 insertions(+), 211 deletions(-)
I have not a problem with this code but my question is what's your plan about it? Right now none is really calling/building this code. Are you going to push any platform which will enable this driver?
Thanks, Michal
On 2019-06-19 6:10 a.m., Michal Simek wrote:
On 18. 06. 19 17:47, Robert Hancock wrote:
These changes add support for slave serial mode, in addition to the existing slave SelectMAP mode, for programming Xilinx Virtex2 (and later) FPGAs, as well as fixing up code style and an issue with the programming sequence.
Robert Hancock (5): fpga: virtex2: cosmetic: Cleanup code style fpga: virtex2: added Kconfig option fpga: virtex2: Split out image writing from pre/post operations fpga: virtex2: Add additional clock cycles after DONE assertion fpga: virtex2: Add slave serial programming support
drivers/fpga/Kconfig | 8 + drivers/fpga/virtex2.c | 503 +++++++++++++++++++++++++++++-------------------- include/virtex2.h | 13 +- 3 files changed, 313 insertions(+), 211 deletions(-)
I have not a problem with this code but my question is what's your plan about it? Right now none is really calling/building this code. Are you going to push any platform which will enable this driver?
This is being used on an internal platform that hasn't been upstreamed yet. There is some more cleanup that needs to happen in the board code before that can happen but I think we could potentially do that.
However, now that there is a Kconfig option for this it would be a lot easier for those code to be built for other platforms that will use it.
Thanks, Michal
On 19. 06. 19 17:43, Robert Hancock wrote:
On 2019-06-19 6:10 a.m., Michal Simek wrote:
On 18. 06. 19 17:47, Robert Hancock wrote:
These changes add support for slave serial mode, in addition to the existing slave SelectMAP mode, for programming Xilinx Virtex2 (and later) FPGAs, as well as fixing up code style and an issue with the programming sequence.
Robert Hancock (5): fpga: virtex2: cosmetic: Cleanup code style fpga: virtex2: added Kconfig option fpga: virtex2: Split out image writing from pre/post operations fpga: virtex2: Add additional clock cycles after DONE assertion fpga: virtex2: Add slave serial programming support
drivers/fpga/Kconfig | 8 + drivers/fpga/virtex2.c | 503 +++++++++++++++++++++++++++++-------------------- include/virtex2.h | 13 +- 3 files changed, 313 insertions(+), 211 deletions(-)
I have not a problem with this code but my question is what's your plan about it? Right now none is really calling/building this code. Are you going to push any platform which will enable this driver?
This is being used on an internal platform that hasn't been upstreamed yet. There is some more cleanup that needs to happen in the board code before that can happen but I think we could potentially do that.
However, now that there is a Kconfig option for this it would be a lot easier for those code to be built for other platforms that will use it.
It will be useful to have this code available because of fpga functions needs to be implemented based on your connection. Do you have this in any internal repo or also available in your u-boot clone somewhere like github?
Thanks, Michal
On 2019-06-20 5:55 a.m., Michal Simek wrote:
On 19. 06. 19 17:43, Robert Hancock wrote:
On 2019-06-19 6:10 a.m., Michal Simek wrote:
On 18. 06. 19 17:47, Robert Hancock wrote:
These changes add support for slave serial mode, in addition to the existing slave SelectMAP mode, for programming Xilinx Virtex2 (and later) FPGAs, as well as fixing up code style and an issue with the programming sequence.
Robert Hancock (5): fpga: virtex2: cosmetic: Cleanup code style fpga: virtex2: added Kconfig option fpga: virtex2: Split out image writing from pre/post operations fpga: virtex2: Add additional clock cycles after DONE assertion fpga: virtex2: Add slave serial programming support
drivers/fpga/Kconfig | 8 + drivers/fpga/virtex2.c | 503 +++++++++++++++++++++++++++++-------------------- include/virtex2.h | 13 +- 3 files changed, 313 insertions(+), 211 deletions(-)
I have not a problem with this code but my question is what's your plan about it? Right now none is really calling/building this code. Are you going to push any platform which will enable this driver?
This is being used on an internal platform that hasn't been upstreamed yet. There is some more cleanup that needs to happen in the board code before that can happen but I think we could potentially do that.
However, now that there is a Kconfig option for this it would be a lot easier for those code to be built for other platforms that will use it.
It will be useful to have this code available because of fpga functions needs to be implemented based on your connection. Do you have this in any internal repo or also available in your u-boot clone somewhere like github?
If it helps, here is part of our board file that does the FPGA GPIO and SPI initialization and initializes the FPGA callbacks. We are using an ITB image that has the FPGA .bit file as one of the entries and which gets loaded as part of the bootm command.
static struct gpio_desc fpga_done; static struct gpio_desc fpga_prog; static struct gpio_desc fpga_init_in;
static struct spi_slave *fpga_slave;
...
/* * Initialize before download */ static int fpga_pre_fn(int cookie) { /* Initialize GPIO pins */ dm_gpio_set_dir_flags(&fpga_prog, GPIOD_IS_OUT); dm_gpio_set_value(&fpga_prog, 1);
return cookie; }
/* * Set the FPGA's active-low program line to the specified level */ static int fpga_pgm_fn(int assert, int flush, int cookie) { dm_gpio_set_value(&fpga_prog, !assert); return assert; }
/* * Test the state of the active-low FPGA INIT line. Return 1 on INIT * asserted (low). */ static int fpga_init_fn(int cookie) { return !dm_gpio_get_value(&fpga_init_in); }
/* * Test the state of the active-high FPGA DONE pin */ static int fpga_done_fn(int cookie) { return dm_gpio_get_value(&fpga_done) ? FPGA_SUCCESS : FPGA_FAIL; }
static int fpga_wbulkdata_fn(void *buf, size_t len, int flush, int cookie) { int ret = spi_xfer(fpga_slave, len * 8, buf, NULL, SPI_XFER_BEGIN | SPI_XFER_END); if (ret) { printf("Failed to transfer %zu bytes of SPI data: %d\n", len, ret); } return cookie; }
static int fpga_post_fn(int cookie) { return cookie; }
static int fpga_abort_fn(int cookie) { return fpga_post_fn(cookie); }
static int fpga_busy_fn(int cookie) { return 0; }
static xilinx_virtex2_slave_fns fpga_fns = { .pre = fpga_pre_fn, .pgm = fpga_pgm_fn, .init = fpga_init_fn, .done = fpga_done_fn, .wbulkdata = fpga_wbulkdata_fn, .busy = fpga_busy_fn, .abort = fpga_abort_fn, .post = fpga_post_fn, };
static xilinx_desc fpga = { .family = xilinx_virtex2, .iface = slave_serial, .size = 6692572, .iface_fns = &fpga_fns, .cookie = 0, .operations = &virtex2_op, .name = "7k160tffg676" };
int board_late_init(void) { struct udevice *dev; int ret, i;
...
/* Bind SPI generic driver to the device */ ret = spi_get_bus_and_cs(2, 0, 50000000, SPI_MODE_0, "spi_generic_drv", "fpga_spi", &dev, &fpga_slave); if (ret) { printf("Failed to bind SPI generic device (err=%d)\n", ret); return 1; }
/* Claim SPI bus */ ret = spi_claim_bus(fpga_slave); if (ret) { printf("Failed to claim SPI generic device (err=%d)\n", ret); return 1; }
/* Claim FPGA GPIOs */ ret = dm_gpio_lookup_name("GPIO6_5", &fpga_done); if (ret) { printf("Unable to lookup GPIO6_5\n"); return 1; }
ret = dm_gpio_request(&fpga_done, "fpga_done"); if (ret) { printf("Unable to request fpga_done\n"); return 1; }
dm_gpio_set_dir_flags(&fpga_done, GPIOD_IS_IN);
ret = dm_gpio_lookup_name("GPIO6_3", &fpga_prog); if (ret) { printf("Unable to lookup GPIO6_3\n"); return 1; }
ret = dm_gpio_request(&fpga_prog, "fpga_prog"); if (ret) { printf("Unable to request fpga_prog\n"); return 1; }
ret = dm_gpio_lookup_name("GPIO6_2", &fpga_init_in); if (ret) { printf("Unable to lookup GPIO6_2\n"); return 1; }
ret = dm_gpio_request(&fpga_init_in, "fpga_init_in"); if (ret) { printf("Unable to request fpga_init_in\n"); return 1; }
dm_gpio_set_dir_flags(&fpga_init_in, GPIOD_IS_IN);
fpga_init();
fpga_add(fpga_xilinx, &fpga);
...
return 0; }
On 21. 06. 19 0:11, Robert Hancock wrote:
On 2019-06-20 5:55 a.m., Michal Simek wrote:
On 19. 06. 19 17:43, Robert Hancock wrote:
On 2019-06-19 6:10 a.m., Michal Simek wrote:
On 18. 06. 19 17:47, Robert Hancock wrote:
These changes add support for slave serial mode, in addition to the existing slave SelectMAP mode, for programming Xilinx Virtex2 (and later) FPGAs, as well as fixing up code style and an issue with the programming sequence.
Robert Hancock (5): fpga: virtex2: cosmetic: Cleanup code style fpga: virtex2: added Kconfig option fpga: virtex2: Split out image writing from pre/post operations fpga: virtex2: Add additional clock cycles after DONE assertion fpga: virtex2: Add slave serial programming support
drivers/fpga/Kconfig | 8 + drivers/fpga/virtex2.c | 503 +++++++++++++++++++++++++++++-------------------- include/virtex2.h | 13 +- 3 files changed, 313 insertions(+), 211 deletions(-)
I have not a problem with this code but my question is what's your plan about it? Right now none is really calling/building this code. Are you going to push any platform which will enable this driver?
This is being used on an internal platform that hasn't been upstreamed yet. There is some more cleanup that needs to happen in the board code before that can happen but I think we could potentially do that.
However, now that there is a Kconfig option for this it would be a lot easier for those code to be built for other platforms that will use it.
It will be useful to have this code available because of fpga functions needs to be implemented based on your connection. Do you have this in any internal repo or also available in your u-boot clone somewhere like github?
If it helps, here is part of our board file that does the FPGA GPIO and SPI initialization and initializes the FPGA callbacks. We are using an ITB image that has the FPGA .bit file as one of the entries and which gets loaded as part of the bootm command.
thanks for this.
static struct gpio_desc fpga_done; static struct gpio_desc fpga_prog; static struct gpio_desc fpga_init_in;
static struct spi_slave *fpga_slave;
...
/*
- Initialize before download
*/ static int fpga_pre_fn(int cookie) { /* Initialize GPIO pins */ dm_gpio_set_dir_flags(&fpga_prog, GPIOD_IS_OUT); dm_gpio_set_value(&fpga_prog, 1);
return cookie; }
/*
- Set the FPGA's active-low program line to the specified level
*/ static int fpga_pgm_fn(int assert, int flush, int cookie) { dm_gpio_set_value(&fpga_prog, !assert); return assert; }
/*
- Test the state of the active-low FPGA INIT line. Return 1 on INIT
- asserted (low).
*/ static int fpga_init_fn(int cookie) { return !dm_gpio_get_value(&fpga_init_in); }
/*
- Test the state of the active-high FPGA DONE pin
*/ static int fpga_done_fn(int cookie) { return dm_gpio_get_value(&fpga_done) ? FPGA_SUCCESS : FPGA_FAIL; }
static int fpga_wbulkdata_fn(void *buf, size_t len, int flush, int cookie) { int ret = spi_xfer(fpga_slave, len * 8, buf, NULL, SPI_XFER_BEGIN | SPI_XFER_END); if (ret) { printf("Failed to transfer %zu bytes of SPI data: %d\n", len, ret); } return cookie; }
static int fpga_post_fn(int cookie) { return cookie; }
static int fpga_abort_fn(int cookie) { return fpga_post_fn(cookie); }
static int fpga_busy_fn(int cookie) { return 0; }
static xilinx_virtex2_slave_fns fpga_fns = { .pre = fpga_pre_fn, .pgm = fpga_pgm_fn, .init = fpga_init_fn, .done = fpga_done_fn, .wbulkdata = fpga_wbulkdata_fn, .busy = fpga_busy_fn, .abort = fpga_abort_fn, .post = fpga_post_fn, };
static xilinx_desc fpga = { .family = xilinx_virtex2, .iface = slave_serial, .size = 6692572, .iface_fns = &fpga_fns, .cookie = 0, .operations = &virtex2_op, .name = "7k160tffg676"
This doesn't look like virtex2. It looks like kintex-7. What do you really have there?
Thanks, Michal
On 2019-06-21 12:19 a.m., Michal Simek wrote:
If it helps, here is part of our board file that does the FPGA GPIO and SPI initialization and initializes the FPGA callbacks. We are using an ITB image that has the FPGA .bit file as one of the entries and which gets loaded as part of the bootm command.
thanks for this.
static struct gpio_desc fpga_done; static struct gpio_desc fpga_prog; static struct gpio_desc fpga_init_in;
static struct spi_slave *fpga_slave;
...
/*
- Initialize before download
*/ static int fpga_pre_fn(int cookie) { /* Initialize GPIO pins */ dm_gpio_set_dir_flags(&fpga_prog, GPIOD_IS_OUT); dm_gpio_set_value(&fpga_prog, 1);
return cookie; }
/*
- Set the FPGA's active-low program line to the specified level
*/ static int fpga_pgm_fn(int assert, int flush, int cookie) { dm_gpio_set_value(&fpga_prog, !assert); return assert; }
/*
- Test the state of the active-low FPGA INIT line. Return 1 on INIT
- asserted (low).
*/ static int fpga_init_fn(int cookie) { return !dm_gpio_get_value(&fpga_init_in); }
/*
- Test the state of the active-high FPGA DONE pin
*/ static int fpga_done_fn(int cookie) { return dm_gpio_get_value(&fpga_done) ? FPGA_SUCCESS : FPGA_FAIL; }
static int fpga_wbulkdata_fn(void *buf, size_t len, int flush, int cookie) { int ret = spi_xfer(fpga_slave, len * 8, buf, NULL, SPI_XFER_BEGIN | SPI_XFER_END); if (ret) { printf("Failed to transfer %zu bytes of SPI data: %d\n", len, ret); } return cookie; }
static int fpga_post_fn(int cookie) { return cookie; }
static int fpga_abort_fn(int cookie) { return fpga_post_fn(cookie); }
static int fpga_busy_fn(int cookie) { return 0; }
static xilinx_virtex2_slave_fns fpga_fns = { .pre = fpga_pre_fn, .pgm = fpga_pgm_fn, .init = fpga_init_fn, .done = fpga_done_fn, .wbulkdata = fpga_wbulkdata_fn, .busy = fpga_busy_fn, .abort = fpga_abort_fn, .post = fpga_post_fn, };
static xilinx_desc fpga = { .family = xilinx_virtex2, .iface = slave_serial, .size = 6692572, .iface_fns = &fpga_fns, .cookie = 0, .operations = &virtex2_op, .name = "7k160tffg676"
This doesn't look like virtex2. It looks like kintex-7. What do you really have there?
Correct, it's a Kintex 7 part. The slave serial protocol doesn't appear to have changed much from Virtex2 onward, so the same driver can be used.
On 21. 06. 19 18:10, Robert Hancock wrote:
On 2019-06-21 12:19 a.m., Michal Simek wrote:
If it helps, here is part of our board file that does the FPGA GPIO and SPI initialization and initializes the FPGA callbacks. We are using an ITB image that has the FPGA .bit file as one of the entries and which gets loaded as part of the bootm command.
thanks for this.
static struct gpio_desc fpga_done; static struct gpio_desc fpga_prog; static struct gpio_desc fpga_init_in;
static struct spi_slave *fpga_slave;
...
/*
- Initialize before download
*/ static int fpga_pre_fn(int cookie) { /* Initialize GPIO pins */ dm_gpio_set_dir_flags(&fpga_prog, GPIOD_IS_OUT); dm_gpio_set_value(&fpga_prog, 1);
return cookie; }
/*
- Set the FPGA's active-low program line to the specified level
*/ static int fpga_pgm_fn(int assert, int flush, int cookie) { dm_gpio_set_value(&fpga_prog, !assert); return assert; }
/*
- Test the state of the active-low FPGA INIT line. Return 1 on INIT
- asserted (low).
*/ static int fpga_init_fn(int cookie) { return !dm_gpio_get_value(&fpga_init_in); }
/*
- Test the state of the active-high FPGA DONE pin
*/ static int fpga_done_fn(int cookie) { return dm_gpio_get_value(&fpga_done) ? FPGA_SUCCESS : FPGA_FAIL; }
static int fpga_wbulkdata_fn(void *buf, size_t len, int flush, int cookie) { int ret = spi_xfer(fpga_slave, len * 8, buf, NULL, SPI_XFER_BEGIN | SPI_XFER_END); if (ret) { printf("Failed to transfer %zu bytes of SPI data: %d\n", len, ret); } return cookie; }
static int fpga_post_fn(int cookie) { return cookie; }
static int fpga_abort_fn(int cookie) { return fpga_post_fn(cookie); }
static int fpga_busy_fn(int cookie) { return 0; }
static xilinx_virtex2_slave_fns fpga_fns = { .pre = fpga_pre_fn, .pgm = fpga_pgm_fn, .init = fpga_init_fn, .done = fpga_done_fn, .wbulkdata = fpga_wbulkdata_fn, .busy = fpga_busy_fn, .abort = fpga_abort_fn, .post = fpga_post_fn, };
static xilinx_desc fpga = { .family = xilinx_virtex2, .iface = slave_serial, .size = 6692572, .iface_fns = &fpga_fns, .cookie = 0, .operations = &virtex2_op, .name = "7k160tffg676"
This doesn't look like virtex2. It looks like kintex-7. What do you really have there?
Correct, it's a Kintex 7 part. The slave serial protocol doesn't appear to have changed much from Virtex2 onward, so the same driver can be used.
Question here is if we should be really using virtex2 file and not any generic one if this can be used for multiple fpgas. But enabling virtex2 driver for programming kintex7 is weird for sure.
Thanks, Michal
On 2019-06-25 7:01 a.m., Michal Simek wrote:
On 21. 06. 19 18:10, Robert Hancock wrote:
On 2019-06-21 12:19 a.m., Michal Simek wrote:
If it helps, here is part of our board file that does the FPGA GPIO and SPI initialization and initializes the FPGA callbacks. We are using an ITB image that has the FPGA .bit file as one of the entries and which gets loaded as part of the bootm command.
thanks for this.
static struct gpio_desc fpga_done; static struct gpio_desc fpga_prog; static struct gpio_desc fpga_init_in;
static struct spi_slave *fpga_slave;
...
/*
- Initialize before download
*/ static int fpga_pre_fn(int cookie) { /* Initialize GPIO pins */ dm_gpio_set_dir_flags(&fpga_prog, GPIOD_IS_OUT); dm_gpio_set_value(&fpga_prog, 1);
return cookie; }
/*
- Set the FPGA's active-low program line to the specified level
*/ static int fpga_pgm_fn(int assert, int flush, int cookie) { dm_gpio_set_value(&fpga_prog, !assert); return assert; }
/*
- Test the state of the active-low FPGA INIT line. Return 1 on INIT
- asserted (low).
*/ static int fpga_init_fn(int cookie) { return !dm_gpio_get_value(&fpga_init_in); }
/*
- Test the state of the active-high FPGA DONE pin
*/ static int fpga_done_fn(int cookie) { return dm_gpio_get_value(&fpga_done) ? FPGA_SUCCESS : FPGA_FAIL; }
static int fpga_wbulkdata_fn(void *buf, size_t len, int flush, int cookie) { int ret = spi_xfer(fpga_slave, len * 8, buf, NULL, SPI_XFER_BEGIN | SPI_XFER_END); if (ret) { printf("Failed to transfer %zu bytes of SPI data: %d\n", len, ret); } return cookie; }
static int fpga_post_fn(int cookie) { return cookie; }
static int fpga_abort_fn(int cookie) { return fpga_post_fn(cookie); }
static int fpga_busy_fn(int cookie) { return 0; }
static xilinx_virtex2_slave_fns fpga_fns = { .pre = fpga_pre_fn, .pgm = fpga_pgm_fn, .init = fpga_init_fn, .done = fpga_done_fn, .wbulkdata = fpga_wbulkdata_fn, .busy = fpga_busy_fn, .abort = fpga_abort_fn, .post = fpga_post_fn, };
static xilinx_desc fpga = { .family = xilinx_virtex2, .iface = slave_serial, .size = 6692572, .iface_fns = &fpga_fns, .cookie = 0, .operations = &virtex2_op, .name = "7k160tffg676"
This doesn't look like virtex2. It looks like kintex-7. What do you really have there?
Correct, it's a Kintex 7 part. The slave serial protocol doesn't appear to have changed much from Virtex2 onward, so the same driver can be used.
Question here is if we should be really using virtex2 file and not any generic one if this can be used for multiple fpgas. But enabling virtex2 driver for programming kintex7 is weird for sure.
I did note in the Kconfig help text that the driver can be used for a bunch of newer FPGA families as well. However we could rename the driver to be more generic - I'm just not sure what to call it, since there are some older FPGAs like Spartan3 that still have specific support elsewhere.
On 25. 06. 19 17:53, Robert Hancock wrote:
On 2019-06-25 7:01 a.m., Michal Simek wrote:
On 21. 06. 19 18:10, Robert Hancock wrote:
On 2019-06-21 12:19 a.m., Michal Simek wrote:
If it helps, here is part of our board file that does the FPGA GPIO and SPI initialization and initializes the FPGA callbacks. We are using an ITB image that has the FPGA .bit file as one of the entries and which gets loaded as part of the bootm command.
thanks for this.
static struct gpio_desc fpga_done; static struct gpio_desc fpga_prog; static struct gpio_desc fpga_init_in;
static struct spi_slave *fpga_slave;
...
/*
- Initialize before download
*/ static int fpga_pre_fn(int cookie) { /* Initialize GPIO pins */ dm_gpio_set_dir_flags(&fpga_prog, GPIOD_IS_OUT); dm_gpio_set_value(&fpga_prog, 1);
return cookie; }
/*
- Set the FPGA's active-low program line to the specified level
*/ static int fpga_pgm_fn(int assert, int flush, int cookie) { dm_gpio_set_value(&fpga_prog, !assert); return assert; }
/*
- Test the state of the active-low FPGA INIT line. Return 1 on INIT
- asserted (low).
*/ static int fpga_init_fn(int cookie) { return !dm_gpio_get_value(&fpga_init_in); }
/*
- Test the state of the active-high FPGA DONE pin
*/ static int fpga_done_fn(int cookie) { return dm_gpio_get_value(&fpga_done) ? FPGA_SUCCESS : FPGA_FAIL; }
static int fpga_wbulkdata_fn(void *buf, size_t len, int flush, int cookie) { int ret = spi_xfer(fpga_slave, len * 8, buf, NULL, SPI_XFER_BEGIN | SPI_XFER_END); if (ret) { printf("Failed to transfer %zu bytes of SPI data: %d\n", len, ret); } return cookie; }
static int fpga_post_fn(int cookie) { return cookie; }
static int fpga_abort_fn(int cookie) { return fpga_post_fn(cookie); }
static int fpga_busy_fn(int cookie) { return 0; }
static xilinx_virtex2_slave_fns fpga_fns = { .pre = fpga_pre_fn, .pgm = fpga_pgm_fn, .init = fpga_init_fn, .done = fpga_done_fn, .wbulkdata = fpga_wbulkdata_fn, .busy = fpga_busy_fn, .abort = fpga_abort_fn, .post = fpga_post_fn, };
static xilinx_desc fpga = { .family = xilinx_virtex2, .iface = slave_serial, .size = 6692572, .iface_fns = &fpga_fns, .cookie = 0, .operations = &virtex2_op, .name = "7k160tffg676"
This doesn't look like virtex2. It looks like kintex-7. What do you really have there?
Correct, it's a Kintex 7 part. The slave serial protocol doesn't appear to have changed much from Virtex2 onward, so the same driver can be used.
Question here is if we should be really using virtex2 file and not any generic one if this can be used for multiple fpgas. But enabling virtex2 driver for programming kintex7 is weird for sure.
I did note in the Kconfig help text that the driver can be used for a bunch of newer FPGA families as well. However we could rename the driver to be more generic - I'm just not sure what to call it, since there are some older FPGAs like Spartan3 that still have specific support elsewhere.
ok. Fair enough. I have applied this series and I think will be good to take out generic stuff to one file. I don't have any HW for this that's why I have no way to test this and do closer look. Anyway if you want to do I am happy to review it or someone else can do it.
Thanks, Michal
participants (2)
-
Michal Simek -
Robert Hancock