Message ID | 20220217191851.11730-1-i.bornyakov@metrotek.ru (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | [v4] fpga: microchip-spi: add Microchip FPGA manager | expand |
Hey Ivan, Finally got my hands on a board with a non SoC PolarFire today & started trying to test. Ran into problems with my SPI setup - would be nice to know if youre currently doing the reprogramming on one of our devkits etc or on a custom board of your own? Will be Monday before I can have look at it again, will have another board I can try then in the odd chance this one isnt actually capable of reprogramming. Thanks, Conor. On 17/02/2022 19:18, Ivan Bornyakov wrote: > Add support to the FPGA manager for programming Microchip Polarfire > FPGAs over slave SPI interface. > > Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru> > --- > Changelog: > v1 -> v2: fix printk formating > v2 -> v3: > * replace "microsemi" with "microchip" > * replace prefix "microsemi_fpga_" with "mpf_" > * more sensible .compatible and .name strings > * remove unused defines STATUS_SPI_VIOLATION and STATUS_SPI_ERROR > v3 -> v4: fix unused variable warning > Put 'mpf_of_ids' definition under conditional compilation, so it > would not hang unused if CONFIG_OF is not enabled. > > drivers/fpga/Kconfig | 9 + > drivers/fpga/Makefile | 1 + > drivers/fpga/microchip-spi.c | 361 +++++++++++++++++++++++++++++++++++ > 3 files changed, 371 insertions(+) > create mode 100644 drivers/fpga/microchip-spi.c > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig > index 26025dbab353..4240c641b100 100644 > --- a/drivers/fpga/Kconfig > +++ b/drivers/fpga/Kconfig > @@ -248,4 +248,13 @@ config FPGA_MGR_VERSAL_FPGA > configure the programmable logic(PL). > > To compile this as a module, choose M here. > + > +config FPGA_MGR_MICROCHIP_SPI > + tristate "Microchip Polarfire SPI FPGA manager" > + depends on SPI > + select CRC_CCITT > + help > + FPGA manager driver support for Microchip Polarfire FPGAs > + programming over slave SPI interface. > + > endif # FPGA > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile > index 4da5273948df..fcb389ca4873 100644 > --- a/drivers/fpga/Makefile > +++ b/drivers/fpga/Makefile > @@ -19,6 +19,7 @@ obj-$(CONFIG_FPGA_MGR_XILINX_SPI) += xilinx-spi.o > obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA) += zynq-fpga.o > obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA) += zynqmp-fpga.o > obj-$(CONFIG_FPGA_MGR_VERSAL_FPGA) += versal-fpga.o > +obj-$(CONFIG_FPGA_MGR_MICROCHIP_SPI) += microchip-spi.o > obj-$(CONFIG_ALTERA_PR_IP_CORE) += altera-pr-ip-core.o > obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT) += altera-pr-ip-core-plat.o > > diff --git a/drivers/fpga/microchip-spi.c b/drivers/fpga/microchip-spi.c > new file mode 100644 > index 000000000000..5db25734a27a > --- /dev/null > +++ b/drivers/fpga/microchip-spi.c > @@ -0,0 +1,361 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Microchip Polarfire FPGA programming over slave SPI interface. > + */ > + > +#include <linux/module.h> > +#include <linux/spi/spi.h> > +#include <linux/of_device.h> > +#include <linux/fpga/fpga-mgr.h> > +#include <linux/delay.h> > +#include <linux/crc-ccitt.h> > + > +#define SPI_ISC_ENABLE 0x0B > +#define SPI_ISC_DISABLE 0x0C > +#define SPI_READ_STATUS 0x00 > +#define SPI_READ_DATA 0x01 > +#define SPI_FRAME_INIT 0xAE > +#define SPI_FRAME 0xEE > +#define SPI_PRG_MODE 0x01 > +#define SPI_RELEASE 0x23 > + > +#define SPI_FRAME_SIZE 16 > + > +#define HEADER_SIZE_OFFSET 24 > +#define DATA_SIZE_OFFSET 55 > + > +#define LOOKUP_TABLE_RECORD_SIZE 9 > +#define LOOKUP_TABLE_BLOCK_ID_OFFSET 0 > +#define LOOKUP_TABLE_BLOCK_START_OFFSET 1 > + > +#define COMPONENTS_SIZE_ID 5 > +#define BITSTREAM_ID 8 > + > +#define BITS_PER_COMPONENT_SIZE 22 > + > +#define STATUS_POLL_TIMEOUT_MS 1000 > +#define STATUS_BUSY BIT(0) > +#define STATUS_READY BIT(1) > + > +struct mpf_priv { > + struct spi_device *spi; > + bool program_mode; > +}; > + > +static enum fpga_mgr_states mpf_ops_state(struct fpga_manager *mgr) > +{ > + struct mpf_priv *priv = mgr->priv; > + struct spi_device *spi = priv->spi; > + bool program_mode = priv->program_mode; > + ssize_t status; > + > + status = spi_w8r8(spi, SPI_READ_STATUS); > + > + if (!program_mode && !status) > + return FPGA_MGR_STATE_OPERATING; > + > + return FPGA_MGR_STATE_UNKNOWN; > +} > + > +static int poll_status_not_busy(struct spi_device *spi, u8 mask) > +{ > + ssize_t status, timeout = STATUS_POLL_TIMEOUT_MS; > + > + while (timeout--) { > + status = spi_w8r8(spi, SPI_READ_STATUS); > + if (status < 0) > + return status; > + > + if (mask) { > + if (!(status & STATUS_BUSY) && (status & mask)) > + return status; > + } else { > + if (!(status & STATUS_BUSY)) > + return status; > + } > + > + mdelay(1); > + } > + > + return -EBUSY; > +} > + > +static int mpf_spi_write(struct spi_device *spi, const void *buf, size_t buf_size) > +{ > + int status = poll_status_not_busy(spi, 0); > + > + if (status < 0) > + return status; > + > + return spi_write(spi, buf, buf_size); > +} > + > +static int mpf_spi_write_then_read(struct spi_device *spi, > + const void *txbuf, size_t txbuf_size, > + void *rxbuf, size_t rxbuf_size) > +{ > + const u8 read_command[] = { SPI_READ_DATA }; > + int ret; > + > + ret = mpf_spi_write(spi, txbuf, txbuf_size); > + if (ret) > + return ret; > + > + ret = poll_status_not_busy(spi, STATUS_READY); > + if (ret < 0) > + return ret; > + > + return spi_write_then_read(spi, read_command, sizeof(read_command), > + rxbuf, rxbuf_size); > +} > + > +static int mpf_ops_write_init(struct fpga_manager *mgr, > + struct fpga_image_info *info, const char *buf, > + size_t count) > +{ > + const u8 isc_en_command[] = { SPI_ISC_ENABLE }; > + const u8 program_mode[] = { SPI_FRAME_INIT, SPI_PRG_MODE }; > + struct mpf_priv *priv = mgr->priv; > + struct spi_device *spi = priv->spi; > + struct device *dev = &mgr->dev; > + u32 isc_ret; > + int ret; > + > + if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) { > + dev_err(dev, "Partial reconfiguration is not supported\n"); > + > + return -EOPNOTSUPP; > + } > + > + ret = mpf_spi_write_then_read(spi, isc_en_command, sizeof(isc_en_command), > + &isc_ret, sizeof(isc_ret)); > + if (ret || isc_ret) { > + dev_err(dev, "Failed to enable ISC: %d\n", ret ? ret : isc_ret); > + > + return -EFAULT; > + } > + > + ret = mpf_spi_write(spi, program_mode, sizeof(program_mode)); > + if (ret) { > + dev_err(dev, "Failed to enter program mode: %d\n", ret); > + > + return ret; > + } > + > + priv->program_mode = true; > + > + return 0; > +} > + > +static ssize_t lookup_block_start(int id, const char *buf, size_t buf_size) > +{ > + u8 header_size, blocks_num, block_id; > + u32 block_start, i; > + > + header_size = *(buf + HEADER_SIZE_OFFSET); > + > + if (header_size > buf_size) > + return -EFAULT; > + > + blocks_num = *(buf + header_size - 1); > + > + if (header_size + blocks_num * LOOKUP_TABLE_RECORD_SIZE > buf_size) > + return -EFAULT; > + > + for (i = 0; i < blocks_num; i++) { > + block_id = *(buf + header_size + LOOKUP_TABLE_RECORD_SIZE * i + > + LOOKUP_TABLE_BLOCK_ID_OFFSET); > + > + if (block_id == id) { > + memcpy(&block_start, > + buf + header_size + > + LOOKUP_TABLE_RECORD_SIZE * i + > + LOOKUP_TABLE_BLOCK_START_OFFSET, > + sizeof(block_start)); > + > + return le32_to_cpu(block_start); > + } > + } > + > + return -EFAULT; > +} > + > +static ssize_t parse_bitstream_size(const char *buf, size_t buf_size) > +{ > + ssize_t bitstream_size = 0, components_size_start = 0, > + component_size_byte_num, component_size_byte_off, i; > + u16 components_num; > + u32 component_size; > + > + memcpy(&components_num, buf + DATA_SIZE_OFFSET, sizeof(components_num)); > + components_num = le16_to_cpu(components_num); > + > + components_size_start = lookup_block_start(COMPONENTS_SIZE_ID, buf, > + buf_size); > + if (components_size_start < 0) > + return components_size_start; > + > + if (components_size_start + > + DIV_ROUND_UP(components_num * BITS_PER_COMPONENT_SIZE, > + BITS_PER_BYTE) > buf_size) > + return -EFAULT; > + > + for (i = 0; i < components_num; i++) { > + component_size_byte_num = > + (i * BITS_PER_COMPONENT_SIZE) / BITS_PER_BYTE; > + component_size_byte_off = > + (i * BITS_PER_COMPONENT_SIZE) % BITS_PER_BYTE; > + > + memcpy(&component_size, > + buf + components_size_start + component_size_byte_num, > + sizeof(component_size)); > + component_size = le32_to_cpu(component_size); > + component_size >>= component_size_byte_off; > + component_size &= GENMASK(BITS_PER_COMPONENT_SIZE - 1, 0); > + > + bitstream_size += component_size; > + } > + > + return bitstream_size; > +} > + > +static int mpf_ops_write(struct fpga_manager *mgr, const char *buf, size_t count) > +{ > + ssize_t bitstream_start = 0, bitstream_size; > + struct mpf_priv *priv = mgr->priv; > + struct spi_device *spi = priv->spi; > + struct device *dev = &mgr->dev; > + u8 tmp_buf[SPI_FRAME_SIZE + 1]; > + int ret, i; > + > + if (crc_ccitt(0, buf, count)) { > + dev_err(dev, "CRC error\n"); > + > + return -EINVAL; > + } > + > + bitstream_start = lookup_block_start(BITSTREAM_ID, buf, count); > + if (bitstream_start < 0) { > + dev_err(dev, "Failed to find bitstream start %zd\n", > + bitstream_start); > + > + return bitstream_start; > + } > + > + bitstream_size = parse_bitstream_size(buf, count); > + if (bitstream_size < 0) { > + dev_err(dev, "Failed to parse bitstream size %zd\n", > + bitstream_size); > + > + return bitstream_size; > + } > + > + if (bitstream_start + bitstream_size * SPI_FRAME_SIZE > count) { > + dev_err(dev, > + "Bitstram outruns firmware. Bitstream start %zd, bitstream size %zd, firmware size %zu\n", > + bitstream_start, bitstream_size * SPI_FRAME_SIZE, count); > + > + return -EFAULT; > + } > + > + for (i = 0; i < bitstream_size; i++) { > + tmp_buf[0] = SPI_FRAME; > + memcpy(tmp_buf + 1, buf + bitstream_start + i * SPI_FRAME_SIZE, > + SPI_FRAME_SIZE); > + > + ret = mpf_spi_write(spi, tmp_buf, sizeof(tmp_buf)); > + if (ret) { > + dev_err(dev, > + "Failed to write bitstream frame number %d of %zd\n", > + i, bitstream_size); > + > + return ret; > + } > + } > + > + return 0; > +} > + > +static int mpf_ops_write_complete(struct fpga_manager *mgr, > + struct fpga_image_info *info) > +{ > + const u8 isc_dis_command[] = { SPI_ISC_DISABLE }; > + const u8 release_command[] = { SPI_RELEASE }; > + struct mpf_priv *priv = mgr->priv; > + struct spi_device *spi = priv->spi; > + struct device *dev = &mgr->dev; > + int ret; > + > + ret = mpf_spi_write(spi, isc_dis_command, sizeof(isc_dis_command)); > + if (ret) { > + dev_err(dev, "Failed to disable ISC: %d\n", ret); > + > + return ret; > + } > + > + mdelay(1); > + > + ret = mpf_spi_write(spi, release_command, sizeof(release_command)); > + if (ret) { > + dev_err(dev, "Failed to exit program mode: %d\n", ret); > + > + return ret; > + } > + > + priv->program_mode = false; > + > + return 0; > +} > + > +static const struct fpga_manager_ops mpf_ops = { > + .state = mpf_ops_state, > + .write_init = mpf_ops_write_init, > + .write = mpf_ops_write, > + .write_complete = mpf_ops_write_complete, > +}; > + > +static int mpf_probe(struct spi_device *spi) > +{ > + struct mpf_priv *priv; > + struct device *dev = &spi->dev; > + struct fpga_manager *mgr; > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->spi = spi; > + > + mgr = devm_fpga_mgr_register(dev, "Microchip Polarfire SPI FPGA Manager", > + &mpf_ops, priv); > + > + return PTR_ERR_OR_ZERO(mgr); > +} > + > +static const struct spi_device_id mpf_spi_ids[] = { > + { .name = "mpf-spi-fpga-mgr", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(spi, mpf_spi_ids); > + > +#if IS_ENABLED(CONFIG_OF) > +static const struct of_device_id mpf_of_ids[] = { > + { .compatible = "microchip,mpf-spi-fpga-mgr" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, mpf_of_ids); > +#endif /* IS_ENABLED(CONFIG_OF) */ > + > +static struct spi_driver mpf_driver = { > + .probe = mpf_probe, > + .id_table = mpf_spi_ids, > + .driver = { > + .name = "microchip_mpf_spi_fpga_mgr", > + .of_match_table = of_match_ptr(mpf_of_ids), > + }, > +}; > + > +module_spi_driver(mpf_driver); > + > +MODULE_DESCRIPTION("Microchip Polarfire SPI FPGA Manager"); > +MODULE_LICENSE("GPL");
On Thu, Feb 17, 2022 at 10:18:51PM +0300, Ivan Bornyakov wrote: > Add support to the FPGA manager for programming Microchip Polarfire > FPGAs over slave SPI interface. > > Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru> > --- > Changelog: > v1 -> v2: fix printk formating > v2 -> v3: > * replace "microsemi" with "microchip" > * replace prefix "microsemi_fpga_" with "mpf_" > * more sensible .compatible and .name strings > * remove unused defines STATUS_SPI_VIOLATION and STATUS_SPI_ERROR > v3 -> v4: fix unused variable warning > Put 'mpf_of_ids' definition under conditional compilation, so it > would not hang unused if CONFIG_OF is not enabled. > > drivers/fpga/Kconfig | 9 + > drivers/fpga/Makefile | 1 + > drivers/fpga/microchip-spi.c | 361 +++++++++++++++++++++++++++++++++++ > 3 files changed, 371 insertions(+) > create mode 100644 drivers/fpga/microchip-spi.c > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig > index 26025dbab353..4240c641b100 100644 > --- a/drivers/fpga/Kconfig > +++ b/drivers/fpga/Kconfig > @@ -248,4 +248,13 @@ config FPGA_MGR_VERSAL_FPGA > configure the programmable logic(PL). > > To compile this as a module, choose M here. > + > +config FPGA_MGR_MICROCHIP_SPI > + tristate "Microchip Polarfire SPI FPGA manager" > + depends on SPI > + select CRC_CCITT > + help > + FPGA manager driver support for Microchip Polarfire FPGAs > + programming over slave SPI interface. > + > endif # FPGA > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile > index 4da5273948df..fcb389ca4873 100644 > --- a/drivers/fpga/Makefile > +++ b/drivers/fpga/Makefile > @@ -19,6 +19,7 @@ obj-$(CONFIG_FPGA_MGR_XILINX_SPI) += xilinx-spi.o > obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA) += zynq-fpga.o > obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA) += zynqmp-fpga.o > obj-$(CONFIG_FPGA_MGR_VERSAL_FPGA) += versal-fpga.o > +obj-$(CONFIG_FPGA_MGR_MICROCHIP_SPI) += microchip-spi.o > obj-$(CONFIG_ALTERA_PR_IP_CORE) += altera-pr-ip-core.o > obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT) += altera-pr-ip-core-plat.o > > diff --git a/drivers/fpga/microchip-spi.c b/drivers/fpga/microchip-spi.c > new file mode 100644 > index 000000000000..5db25734a27a > --- /dev/null > +++ b/drivers/fpga/microchip-spi.c > @@ -0,0 +1,361 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Microchip Polarfire FPGA programming over slave SPI interface. > + */ > + > +#include <linux/module.h> > +#include <linux/spi/spi.h> > +#include <linux/of_device.h> > +#include <linux/fpga/fpga-mgr.h> > +#include <linux/delay.h> > +#include <linux/crc-ccitt.h> Please list them in alphabetical order. > + > +#define SPI_ISC_ENABLE 0x0B > +#define SPI_ISC_DISABLE 0x0C > +#define SPI_READ_STATUS 0x00 > +#define SPI_READ_DATA 0x01 > +#define SPI_FRAME_INIT 0xAE > +#define SPI_FRAME 0xEE > +#define SPI_PRG_MODE 0x01 > +#define SPI_RELEASE 0x23 > + > +#define SPI_FRAME_SIZE 16 > + > +#define HEADER_SIZE_OFFSET 24 > +#define DATA_SIZE_OFFSET 55 > + > +#define LOOKUP_TABLE_RECORD_SIZE 9 > +#define LOOKUP_TABLE_BLOCK_ID_OFFSET 0 > +#define LOOKUP_TABLE_BLOCK_START_OFFSET 1 > + > +#define COMPONENTS_SIZE_ID 5 > +#define BITSTREAM_ID 8 > + > +#define BITS_PER_COMPONENT_SIZE 22 > + > +#define STATUS_POLL_TIMEOUT_MS 1000 > +#define STATUS_BUSY BIT(0) > +#define STATUS_READY BIT(1) Maybe add some prefix for these vendor specific definitions, like MFP_SPI_ISC_ENABLE. > + > +struct mpf_priv { > + struct spi_device *spi; > + bool program_mode; > +}; > + > +static enum fpga_mgr_states mpf_ops_state(struct fpga_manager *mgr) > +{ > + struct mpf_priv *priv = mgr->priv; > + struct spi_device *spi = priv->spi; > + bool program_mode = priv->program_mode; > + ssize_t status; > + > + status = spi_w8r8(spi, SPI_READ_STATUS); > + > + if (!program_mode && !status) > + return FPGA_MGR_STATE_OPERATING; > + > + return FPGA_MGR_STATE_UNKNOWN; > +} > + > +static int poll_status_not_busy(struct spi_device *spi, u8 mask) > +{ > + ssize_t status, timeout = STATUS_POLL_TIMEOUT_MS; > + > + while (timeout--) { > + status = spi_w8r8(spi, SPI_READ_STATUS); > + if (status < 0) > + return status; > + > + if (mask) { > + if (!(status & STATUS_BUSY) && (status & mask)) > + return status; > + } else { > + if (!(status & STATUS_BUSY)) > + return status; > + } !(status & STATUS_BUSY) is always checked regardless of mask, so may move the check out of the if...else statement. > + > + mdelay(1); busy wait for 1ms is discouraged, maybe usleep_range(). See Documentation/timers/timers-howto.rst > + } The actual timeout may be much longer than what is defined. To be more accurate, you may use: time_after(jiffies, timeout_jiffies) > + > + return -EBUSY; > +} > + > +static int mpf_spi_write(struct spi_device *spi, const void *buf, size_t buf_size) > +{ > + int status = poll_status_not_busy(spi, 0); > + > + if (status < 0) > + return status; > + > + return spi_write(spi, buf, buf_size); > +} > + > +static int mpf_spi_write_then_read(struct spi_device *spi, > + const void *txbuf, size_t txbuf_size, > + void *rxbuf, size_t rxbuf_size) > +{ > + const u8 read_command[] = { SPI_READ_DATA }; > + int ret; > + > + ret = mpf_spi_write(spi, txbuf, txbuf_size); > + if (ret) > + return ret; > + > + ret = poll_status_not_busy(spi, STATUS_READY); > + if (ret < 0) > + return ret; > + > + return spi_write_then_read(spi, read_command, sizeof(read_command), > + rxbuf, rxbuf_size); > +} > + > +static int mpf_ops_write_init(struct fpga_manager *mgr, > + struct fpga_image_info *info, const char *buf, > + size_t count) > +{ > + const u8 isc_en_command[] = { SPI_ISC_ENABLE }; > + const u8 program_mode[] = { SPI_FRAME_INIT, SPI_PRG_MODE }; Better we follow the reverse xmas tree declaration, so reverse the 2 lines please. > + struct mpf_priv *priv = mgr->priv; > + struct spi_device *spi = priv->spi; > + struct device *dev = &mgr->dev; > + u32 isc_ret; > + int ret; > + > + if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) { > + dev_err(dev, "Partial reconfiguration is not supported\n"); > + remove the blank line please > + return -EOPNOTSUPP; > + } > + > + ret = mpf_spi_write_then_read(spi, isc_en_command, sizeof(isc_en_command), > + &isc_ret, sizeof(isc_ret)); > + if (ret || isc_ret) { > + dev_err(dev, "Failed to enable ISC: %d\n", ret ? ret : isc_ret); ret ? : isc_ret > + remove the blank line please > + return -EFAULT; > + } > + > + ret = mpf_spi_write(spi, program_mode, sizeof(program_mode)); > + if (ret) { > + dev_err(dev, "Failed to enter program mode: %d\n", ret); > + the same > + return ret; > + } > + > + priv->program_mode = true; > + > + return 0; > +} > + > +static ssize_t lookup_block_start(int id, const char *buf, size_t buf_size) > +{ > + u8 header_size, blocks_num, block_id; > + u32 block_start, i; > + > + header_size = *(buf + HEADER_SIZE_OFFSET); > + > + if (header_size > buf_size) > + return -EFAULT; > + > + blocks_num = *(buf + header_size - 1); > + > + if (header_size + blocks_num * LOOKUP_TABLE_RECORD_SIZE > buf_size) > + return -EFAULT; > + > + for (i = 0; i < blocks_num; i++) { > + block_id = *(buf + header_size + LOOKUP_TABLE_RECORD_SIZE * i + > + LOOKUP_TABLE_BLOCK_ID_OFFSET); > + > + if (block_id == id) { > + memcpy(&block_start, > + buf + header_size + > + LOOKUP_TABLE_RECORD_SIZE * i + > + LOOKUP_TABLE_BLOCK_START_OFFSET, > + sizeof(block_start)); why a memcpy here, could we just read from the offset? > + > + return le32_to_cpu(block_start); > + } > + } > + > + return -EFAULT; > +} > + > +static ssize_t parse_bitstream_size(const char *buf, size_t buf_size) > +{ > + ssize_t bitstream_size = 0, components_size_start = 0, > + component_size_byte_num, component_size_byte_off, i; > + u16 components_num; > + u32 component_size; > + > + memcpy(&components_num, buf + DATA_SIZE_OFFSET, sizeof(components_num)); the same, why a memcpy? > + components_num = le16_to_cpu(components_num); > + > + components_size_start = lookup_block_start(COMPONENTS_SIZE_ID, buf, > + buf_size); > + if (components_size_start < 0) > + return components_size_start; > + > + if (components_size_start + > + DIV_ROUND_UP(components_num * BITS_PER_COMPONENT_SIZE, > + BITS_PER_BYTE) > buf_size) > + return -EFAULT; > + > + for (i = 0; i < components_num; i++) { > + component_size_byte_num = > + (i * BITS_PER_COMPONENT_SIZE) / BITS_PER_BYTE; > + component_size_byte_off = > + (i * BITS_PER_COMPONENT_SIZE) % BITS_PER_BYTE; > + > + memcpy(&component_size, > + buf + components_size_start + component_size_byte_num, > + sizeof(component_size)); > + component_size = le32_to_cpu(component_size); > + component_size >>= component_size_byte_off; > + component_size &= GENMASK(BITS_PER_COMPONENT_SIZE - 1, 0); > + > + bitstream_size += component_size; > + } > + > + return bitstream_size; > +} > + > +static int mpf_ops_write(struct fpga_manager *mgr, const char *buf, size_t count) > +{ > + ssize_t bitstream_start = 0, bitstream_size; > + struct mpf_priv *priv = mgr->priv; > + struct spi_device *spi = priv->spi; > + struct device *dev = &mgr->dev; > + u8 tmp_buf[SPI_FRAME_SIZE + 1]; > + int ret, i; > + > + if (crc_ccitt(0, buf, count)) { > + dev_err(dev, "CRC error\n"); > + > + return -EINVAL; > + } > + > + bitstream_start = lookup_block_start(BITSTREAM_ID, buf, count); > + if (bitstream_start < 0) { > + dev_err(dev, "Failed to find bitstream start %zd\n", > + bitstream_start); > + > + return bitstream_start; > + } > + > + bitstream_size = parse_bitstream_size(buf, count); > + if (bitstream_size < 0) { > + dev_err(dev, "Failed to parse bitstream size %zd\n", > + bitstream_size); > + > + return bitstream_size; > + } > + > + if (bitstream_start + bitstream_size * SPI_FRAME_SIZE > count) { > + dev_err(dev, > + "Bitstram outruns firmware. Bitstream start %zd, bitstream size %zd, firmware size %zu\n", Bitstream > + bitstream_start, bitstream_size * SPI_FRAME_SIZE, count); > + > + return -EFAULT; > + } > + If I understand right, this function assumes the users provide the entire image buffer. But it is possible the image buffer is from a scatter list and the callback would be called several times. Maybe the bitstream info at the head of the image could be parsed in write_init(), and this requires the driver fill the fpga_manager_ops.initial_header_size > + for (i = 0; i < bitstream_size; i++) { > + tmp_buf[0] = SPI_FRAME; > + memcpy(tmp_buf + 1, buf + bitstream_start + i * SPI_FRAME_SIZE, > + SPI_FRAME_SIZE); > + > + ret = mpf_spi_write(spi, tmp_buf, sizeof(tmp_buf)); Is it possible we use spi_sync_transfer to avoid memcpy the whole bitstream? > + if (ret) { > + dev_err(dev, > + "Failed to write bitstream frame number %d of %zd\n", > + i, bitstream_size); > + > + return ret; > + } > + } > + > + return 0; > +} > + > +static int mpf_ops_write_complete(struct fpga_manager *mgr, > + struct fpga_image_info *info) "CHECK: Alignment should match open parenthesis" from checkpatch.pl --strict > +{ > + const u8 isc_dis_command[] = { SPI_ISC_DISABLE }; > + const u8 release_command[] = { SPI_RELEASE }; > + struct mpf_priv *priv = mgr->priv; > + struct spi_device *spi = priv->spi; > + struct device *dev = &mgr->dev; > + int ret; > + > + ret = mpf_spi_write(spi, isc_dis_command, sizeof(isc_dis_command)); > + if (ret) { > + dev_err(dev, "Failed to disable ISC: %d\n", ret); > + > + return ret; > + } > + > + mdelay(1); Same concern > + > + ret = mpf_spi_write(spi, release_command, sizeof(release_command)); > + if (ret) { > + dev_err(dev, "Failed to exit program mode: %d\n", ret); > + > + return ret; > + } > + > + priv->program_mode = false; > + > + return 0; > +} > + > +static const struct fpga_manager_ops mpf_ops = { > + .state = mpf_ops_state, > + .write_init = mpf_ops_write_init, > + .write = mpf_ops_write, > + .write_complete = mpf_ops_write_complete, > +}; > + > +static int mpf_probe(struct spi_device *spi) > +{ > + struct mpf_priv *priv; > + struct device *dev = &spi->dev; > + struct fpga_manager *mgr; > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->spi = spi; > + > + mgr = devm_fpga_mgr_register(dev, "Microchip Polarfire SPI FPGA Manager", > + &mpf_ops, priv); > + > + return PTR_ERR_OR_ZERO(mgr); > +} > + > +static const struct spi_device_id mpf_spi_ids[] = { > + { .name = "mpf-spi-fpga-mgr", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(spi, mpf_spi_ids); > + > +#if IS_ENABLED(CONFIG_OF) > +static const struct of_device_id mpf_of_ids[] = { > + { .compatible = "microchip,mpf-spi-fpga-mgr" }, From checkpatch.pl "WARNING: DT compatible string "microchip,mpf-spi-fpga-mgr" appears un-documented -- check ./Documentation/devicetree/bindings/" Thanks, Yilun > + {}, > +}; > +MODULE_DEVICE_TABLE(of, mpf_of_ids); > +#endif /* IS_ENABLED(CONFIG_OF) */ > + > +static struct spi_driver mpf_driver = { > + .probe = mpf_probe, > + .id_table = mpf_spi_ids, > + .driver = { > + .name = "microchip_mpf_spi_fpga_mgr", > + .of_match_table = of_match_ptr(mpf_of_ids), > + }, > +}; > + > +module_spi_driver(mpf_driver); > + > +MODULE_DESCRIPTION("Microchip Polarfire SPI FPGA Manager"); > +MODULE_LICENSE("GPL"); > -- > 2.34.1 >
Hi, Conor On Fri, Feb 18, 2022 at 03:22:57PM +0000, Conor.Dooley@microchip.com wrote: > Hey Ivan, > Finally got my hands on a board with a non SoC PolarFire today & started > trying to test. Ran into problems with my SPI setup - would be nice to > know if youre currently doing the reprogramming on one of our devkits > etc or on a custom board of your own? > Will be Monday before I can have look at it again, will have another > board I can try then in the odd chance this one isnt actually capable of > reprogramming. > Thanks, > Conor. > I'm working with a custom board.
Hi, Yilun. On Sat, Feb 19, 2022 at 12:05:55AM +0800, Xu Yilun wrote: > On Thu, Feb 17, 2022 at 10:18:51PM +0300, Ivan Bornyakov wrote: > > +static int mpf_ops_write(struct fpga_manager *mgr, const char *buf, size_t count) > > +{ > > + ssize_t bitstream_start = 0, bitstream_size; > > + struct mpf_priv *priv = mgr->priv; > > + struct spi_device *spi = priv->spi; > > + struct device *dev = &mgr->dev; > > + u8 tmp_buf[SPI_FRAME_SIZE + 1]; > > + int ret, i; > > + > > + if (crc_ccitt(0, buf, count)) { > > + dev_err(dev, "CRC error\n"); > > + > > + return -EINVAL; > > + } > > + > > + bitstream_start = lookup_block_start(BITSTREAM_ID, buf, count); > > + if (bitstream_start < 0) { > > + dev_err(dev, "Failed to find bitstream start %zd\n", > > + bitstream_start); > > + > > + return bitstream_start; > > + } > > + > > + bitstream_size = parse_bitstream_size(buf, count); > > + if (bitstream_size < 0) { > > + dev_err(dev, "Failed to parse bitstream size %zd\n", > > + bitstream_size); > > + > > + return bitstream_size; > > + } > > + > > + if (bitstream_start + bitstream_size * SPI_FRAME_SIZE > count) { > > + dev_err(dev, > > + "Bitstram outruns firmware. Bitstream start %zd, bitstream size %zd, firmware size %zu\n", > > Bitstream > > > + bitstream_start, bitstream_size * SPI_FRAME_SIZE, count); > > + > > + return -EFAULT; > > + } > > + > > If I understand right, this function assumes the users provide the > entire image buffer. But it is possible the image buffer is from a > scatter list and the callback would be called several times. > That is unfortunate. I thought fpga_manager_ops->write_sg() is here for that purpose. > > Maybe the bitstream info at the head of the image could be parsed in > write_init(), and this requires the driver fill the > fpga_manager_ops.initial_header_size > Header size is not known beforehand and is stored in 25th byte of the image. Overall, thanks for detailed review
On Sat, Feb 19, 2022 at 09:16:27AM +0300, Ivan Bornyakov wrote: > On Sat, Feb 19, 2022 at 12:05:55AM +0800, Xu Yilun wrote: > > > > Maybe the bitstream info at the head of the image could be parsed in > > write_init(), and this requires the driver fill the > > fpga_manager_ops.initial_header_size > > > > Header size is not known beforehand and is stored in 25th byte of the > image. > Actually I was not quite accurate here. The header itself seems to be of constant size according to https://coredocs.s3.amazonaws.com/DirectC/2021_2/spi_directc.pdf (4. Data File Format) But the lookup table is definitely of a variable size. Moreover bitstream start and size both are somewhere in the image and needed to be located through lookup table.
diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig index 26025dbab353..4240c641b100 100644 --- a/drivers/fpga/Kconfig +++ b/drivers/fpga/Kconfig @@ -248,4 +248,13 @@ config FPGA_MGR_VERSAL_FPGA configure the programmable logic(PL). To compile this as a module, choose M here. + +config FPGA_MGR_MICROCHIP_SPI + tristate "Microchip Polarfire SPI FPGA manager" + depends on SPI + select CRC_CCITT + help + FPGA manager driver support for Microchip Polarfire FPGAs + programming over slave SPI interface. + endif # FPGA diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile index 4da5273948df..fcb389ca4873 100644 --- a/drivers/fpga/Makefile +++ b/drivers/fpga/Makefile @@ -19,6 +19,7 @@ obj-$(CONFIG_FPGA_MGR_XILINX_SPI) += xilinx-spi.o obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA) += zynq-fpga.o obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA) += zynqmp-fpga.o obj-$(CONFIG_FPGA_MGR_VERSAL_FPGA) += versal-fpga.o +obj-$(CONFIG_FPGA_MGR_MICROCHIP_SPI) += microchip-spi.o obj-$(CONFIG_ALTERA_PR_IP_CORE) += altera-pr-ip-core.o obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT) += altera-pr-ip-core-plat.o diff --git a/drivers/fpga/microchip-spi.c b/drivers/fpga/microchip-spi.c new file mode 100644 index 000000000000..5db25734a27a --- /dev/null +++ b/drivers/fpga/microchip-spi.c @@ -0,0 +1,361 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Microchip Polarfire FPGA programming over slave SPI interface. + */ + +#include <linux/module.h> +#include <linux/spi/spi.h> +#include <linux/of_device.h> +#include <linux/fpga/fpga-mgr.h> +#include <linux/delay.h> +#include <linux/crc-ccitt.h> + +#define SPI_ISC_ENABLE 0x0B +#define SPI_ISC_DISABLE 0x0C +#define SPI_READ_STATUS 0x00 +#define SPI_READ_DATA 0x01 +#define SPI_FRAME_INIT 0xAE +#define SPI_FRAME 0xEE +#define SPI_PRG_MODE 0x01 +#define SPI_RELEASE 0x23 + +#define SPI_FRAME_SIZE 16 + +#define HEADER_SIZE_OFFSET 24 +#define DATA_SIZE_OFFSET 55 + +#define LOOKUP_TABLE_RECORD_SIZE 9 +#define LOOKUP_TABLE_BLOCK_ID_OFFSET 0 +#define LOOKUP_TABLE_BLOCK_START_OFFSET 1 + +#define COMPONENTS_SIZE_ID 5 +#define BITSTREAM_ID 8 + +#define BITS_PER_COMPONENT_SIZE 22 + +#define STATUS_POLL_TIMEOUT_MS 1000 +#define STATUS_BUSY BIT(0) +#define STATUS_READY BIT(1) + +struct mpf_priv { + struct spi_device *spi; + bool program_mode; +}; + +static enum fpga_mgr_states mpf_ops_state(struct fpga_manager *mgr) +{ + struct mpf_priv *priv = mgr->priv; + struct spi_device *spi = priv->spi; + bool program_mode = priv->program_mode; + ssize_t status; + + status = spi_w8r8(spi, SPI_READ_STATUS); + + if (!program_mode && !status) + return FPGA_MGR_STATE_OPERATING; + + return FPGA_MGR_STATE_UNKNOWN; +} + +static int poll_status_not_busy(struct spi_device *spi, u8 mask) +{ + ssize_t status, timeout = STATUS_POLL_TIMEOUT_MS; + + while (timeout--) { + status = spi_w8r8(spi, SPI_READ_STATUS); + if (status < 0) + return status; + + if (mask) { + if (!(status & STATUS_BUSY) && (status & mask)) + return status; + } else { + if (!(status & STATUS_BUSY)) + return status; + } + + mdelay(1); + } + + return -EBUSY; +} + +static int mpf_spi_write(struct spi_device *spi, const void *buf, size_t buf_size) +{ + int status = poll_status_not_busy(spi, 0); + + if (status < 0) + return status; + + return spi_write(spi, buf, buf_size); +} + +static int mpf_spi_write_then_read(struct spi_device *spi, + const void *txbuf, size_t txbuf_size, + void *rxbuf, size_t rxbuf_size) +{ + const u8 read_command[] = { SPI_READ_DATA }; + int ret; + + ret = mpf_spi_write(spi, txbuf, txbuf_size); + if (ret) + return ret; + + ret = poll_status_not_busy(spi, STATUS_READY); + if (ret < 0) + return ret; + + return spi_write_then_read(spi, read_command, sizeof(read_command), + rxbuf, rxbuf_size); +} + +static int mpf_ops_write_init(struct fpga_manager *mgr, + struct fpga_image_info *info, const char *buf, + size_t count) +{ + const u8 isc_en_command[] = { SPI_ISC_ENABLE }; + const u8 program_mode[] = { SPI_FRAME_INIT, SPI_PRG_MODE }; + struct mpf_priv *priv = mgr->priv; + struct spi_device *spi = priv->spi; + struct device *dev = &mgr->dev; + u32 isc_ret; + int ret; + + if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) { + dev_err(dev, "Partial reconfiguration is not supported\n"); + + return -EOPNOTSUPP; + } + + ret = mpf_spi_write_then_read(spi, isc_en_command, sizeof(isc_en_command), + &isc_ret, sizeof(isc_ret)); + if (ret || isc_ret) { + dev_err(dev, "Failed to enable ISC: %d\n", ret ? ret : isc_ret); + + return -EFAULT; + } + + ret = mpf_spi_write(spi, program_mode, sizeof(program_mode)); + if (ret) { + dev_err(dev, "Failed to enter program mode: %d\n", ret); + + return ret; + } + + priv->program_mode = true; + + return 0; +} + +static ssize_t lookup_block_start(int id, const char *buf, size_t buf_size) +{ + u8 header_size, blocks_num, block_id; + u32 block_start, i; + + header_size = *(buf + HEADER_SIZE_OFFSET); + + if (header_size > buf_size) + return -EFAULT; + + blocks_num = *(buf + header_size - 1); + + if (header_size + blocks_num * LOOKUP_TABLE_RECORD_SIZE > buf_size) + return -EFAULT; + + for (i = 0; i < blocks_num; i++) { + block_id = *(buf + header_size + LOOKUP_TABLE_RECORD_SIZE * i + + LOOKUP_TABLE_BLOCK_ID_OFFSET); + + if (block_id == id) { + memcpy(&block_start, + buf + header_size + + LOOKUP_TABLE_RECORD_SIZE * i + + LOOKUP_TABLE_BLOCK_START_OFFSET, + sizeof(block_start)); + + return le32_to_cpu(block_start); + } + } + + return -EFAULT; +} + +static ssize_t parse_bitstream_size(const char *buf, size_t buf_size) +{ + ssize_t bitstream_size = 0, components_size_start = 0, + component_size_byte_num, component_size_byte_off, i; + u16 components_num; + u32 component_size; + + memcpy(&components_num, buf + DATA_SIZE_OFFSET, sizeof(components_num)); + components_num = le16_to_cpu(components_num); + + components_size_start = lookup_block_start(COMPONENTS_SIZE_ID, buf, + buf_size); + if (components_size_start < 0) + return components_size_start; + + if (components_size_start + + DIV_ROUND_UP(components_num * BITS_PER_COMPONENT_SIZE, + BITS_PER_BYTE) > buf_size) + return -EFAULT; + + for (i = 0; i < components_num; i++) { + component_size_byte_num = + (i * BITS_PER_COMPONENT_SIZE) / BITS_PER_BYTE; + component_size_byte_off = + (i * BITS_PER_COMPONENT_SIZE) % BITS_PER_BYTE; + + memcpy(&component_size, + buf + components_size_start + component_size_byte_num, + sizeof(component_size)); + component_size = le32_to_cpu(component_size); + component_size >>= component_size_byte_off; + component_size &= GENMASK(BITS_PER_COMPONENT_SIZE - 1, 0); + + bitstream_size += component_size; + } + + return bitstream_size; +} + +static int mpf_ops_write(struct fpga_manager *mgr, const char *buf, size_t count) +{ + ssize_t bitstream_start = 0, bitstream_size; + struct mpf_priv *priv = mgr->priv; + struct spi_device *spi = priv->spi; + struct device *dev = &mgr->dev; + u8 tmp_buf[SPI_FRAME_SIZE + 1]; + int ret, i; + + if (crc_ccitt(0, buf, count)) { + dev_err(dev, "CRC error\n"); + + return -EINVAL; + } + + bitstream_start = lookup_block_start(BITSTREAM_ID, buf, count); + if (bitstream_start < 0) { + dev_err(dev, "Failed to find bitstream start %zd\n", + bitstream_start); + + return bitstream_start; + } + + bitstream_size = parse_bitstream_size(buf, count); + if (bitstream_size < 0) { + dev_err(dev, "Failed to parse bitstream size %zd\n", + bitstream_size); + + return bitstream_size; + } + + if (bitstream_start + bitstream_size * SPI_FRAME_SIZE > count) { + dev_err(dev, + "Bitstram outruns firmware. Bitstream start %zd, bitstream size %zd, firmware size %zu\n", + bitstream_start, bitstream_size * SPI_FRAME_SIZE, count); + + return -EFAULT; + } + + for (i = 0; i < bitstream_size; i++) { + tmp_buf[0] = SPI_FRAME; + memcpy(tmp_buf + 1, buf + bitstream_start + i * SPI_FRAME_SIZE, + SPI_FRAME_SIZE); + + ret = mpf_spi_write(spi, tmp_buf, sizeof(tmp_buf)); + if (ret) { + dev_err(dev, + "Failed to write bitstream frame number %d of %zd\n", + i, bitstream_size); + + return ret; + } + } + + return 0; +} + +static int mpf_ops_write_complete(struct fpga_manager *mgr, + struct fpga_image_info *info) +{ + const u8 isc_dis_command[] = { SPI_ISC_DISABLE }; + const u8 release_command[] = { SPI_RELEASE }; + struct mpf_priv *priv = mgr->priv; + struct spi_device *spi = priv->spi; + struct device *dev = &mgr->dev; + int ret; + + ret = mpf_spi_write(spi, isc_dis_command, sizeof(isc_dis_command)); + if (ret) { + dev_err(dev, "Failed to disable ISC: %d\n", ret); + + return ret; + } + + mdelay(1); + + ret = mpf_spi_write(spi, release_command, sizeof(release_command)); + if (ret) { + dev_err(dev, "Failed to exit program mode: %d\n", ret); + + return ret; + } + + priv->program_mode = false; + + return 0; +} + +static const struct fpga_manager_ops mpf_ops = { + .state = mpf_ops_state, + .write_init = mpf_ops_write_init, + .write = mpf_ops_write, + .write_complete = mpf_ops_write_complete, +}; + +static int mpf_probe(struct spi_device *spi) +{ + struct mpf_priv *priv; + struct device *dev = &spi->dev; + struct fpga_manager *mgr; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->spi = spi; + + mgr = devm_fpga_mgr_register(dev, "Microchip Polarfire SPI FPGA Manager", + &mpf_ops, priv); + + return PTR_ERR_OR_ZERO(mgr); +} + +static const struct spi_device_id mpf_spi_ids[] = { + { .name = "mpf-spi-fpga-mgr", }, + {}, +}; +MODULE_DEVICE_TABLE(spi, mpf_spi_ids); + +#if IS_ENABLED(CONFIG_OF) +static const struct of_device_id mpf_of_ids[] = { + { .compatible = "microchip,mpf-spi-fpga-mgr" }, + {}, +}; +MODULE_DEVICE_TABLE(of, mpf_of_ids); +#endif /* IS_ENABLED(CONFIG_OF) */ + +static struct spi_driver mpf_driver = { + .probe = mpf_probe, + .id_table = mpf_spi_ids, + .driver = { + .name = "microchip_mpf_spi_fpga_mgr", + .of_match_table = of_match_ptr(mpf_of_ids), + }, +}; + +module_spi_driver(mpf_driver); + +MODULE_DESCRIPTION("Microchip Polarfire SPI FPGA Manager"); +MODULE_LICENSE("GPL");
Add support to the FPGA manager for programming Microchip Polarfire FPGAs over slave SPI interface. Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru> --- Changelog: v1 -> v2: fix printk formating v2 -> v3: * replace "microsemi" with "microchip" * replace prefix "microsemi_fpga_" with "mpf_" * more sensible .compatible and .name strings * remove unused defines STATUS_SPI_VIOLATION and STATUS_SPI_ERROR v3 -> v4: fix unused variable warning Put 'mpf_of_ids' definition under conditional compilation, so it would not hang unused if CONFIG_OF is not enabled. drivers/fpga/Kconfig | 9 + drivers/fpga/Makefile | 1 + drivers/fpga/microchip-spi.c | 361 +++++++++++++++++++++++++++++++++++ 3 files changed, 371 insertions(+) create mode 100644 drivers/fpga/microchip-spi.c