| Message ID | 1560872836-21456-1-git-send-email-hancock@sedsystems.ca |
|---|---|
| Headers | show |
| Series | Virtex2 FPGA enhancements | expand |
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