2022-02-14 20:41:33

by Ivan Bornyakov

[permalink] [raw]
Subject: [PATCH] fpga: microsemi-spi: add Microsemi FPGA manager

Add support to the FPGA manager for programming Microsemi Polarfire
FPGAs over slave SPI interface.

Signed-off-by: Ivan Bornyakov <[email protected]>
---
drivers/fpga/Kconfig | 9 +
drivers/fpga/Makefile | 1 +
drivers/fpga/microsemi-spi.c | 366 +++++++++++++++++++++++++++++++++++
3 files changed, 376 insertions(+)
create mode 100644 drivers/fpga/microsemi-spi.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 991b3f361ec9..25c2631a387c 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -243,4 +243,13 @@ config FPGA_MGR_VERSAL_FPGA
configure the programmable logic(PL).

To compile this as a module, choose M here.
+
+config FPGA_MGR_MICROSEMI_SPI
+ tristate "Microsemi FPGA manager"
+ depends on SPI
+ select CRC_CCITT
+ help
+ FPGA manager driver support for Microsemi Polarfire FPGAs
+ programming over slave SPI interface.
+
endif # FPGA
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 0bff783d1b61..8b3d818546a6 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_MICROSEMI_SPI) += microsemi-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/microsemi-spi.c b/drivers/fpga/microsemi-spi.c
new file mode 100644
index 000000000000..02c3dc6d6b0d
--- /dev/null
+++ b/drivers/fpga/microsemi-spi.c
@@ -0,0 +1,366 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Microsemi 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)
+#define STATUS_SPI_VIOLATION BIT(2)
+#define STATUS_SPI_ERROR BIT(3)
+
+struct microsemi_fpga_priv {
+ struct spi_device *spi;
+ bool program_mode;
+};
+
+static enum fpga_mgr_states microsemi_fpga_ops_state(struct fpga_manager *mgr)
+{
+ struct microsemi_fpga_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 microsemi_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 microsemi_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 = microsemi_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 microsemi_fpga_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 microsemi_fpga_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 = microsemi_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 = microsemi_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 microsemi_fpga_ops_write(struct fpga_manager *mgr, const char *buf,
+ size_t count)
+{
+ ssize_t bitstream_start = 0, bitstream_size;
+ struct microsemi_fpga_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 %d\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 %d\n",
+ bitstream_size);
+
+ return bitstream_size;
+ }
+
+ if (bitstream_start + bitstream_size * SPI_FRAME_SIZE > count) {
+ dev_err(dev,
+ "Bitstram outruns firmware. Bitstream start %d, bitstream size %d, firmware size %d\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 = microsemi_spi_write(spi, tmp_buf, sizeof(tmp_buf));
+ if (ret) {
+ dev_err(dev,
+ "Failed to write bitstream frame number %d of %d\n",
+ i, bitstream_size);
+
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static int microsemi_fpga_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 microsemi_fpga_priv *priv = mgr->priv;
+ struct spi_device *spi = priv->spi;
+ struct device *dev = &mgr->dev;
+ int ret;
+
+ ret = microsemi_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 = microsemi_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 microsemi_fpga_ops = {
+ .state = microsemi_fpga_ops_state,
+ .write_init = microsemi_fpga_ops_write_init,
+ .write = microsemi_fpga_ops_write,
+ .write_complete = microsemi_fpga_ops_write_complete,
+};
+
+static int microsemi_fpga_probe(struct spi_device *spi)
+{
+ struct microsemi_fpga_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, "Microsemi FPGA Manager",
+ &microsemi_fpga_ops, priv);
+
+ return PTR_ERR_OR_ZERO(mgr);
+}
+
+static const struct spi_device_id microsemi_fpga_spi_ids[] = {
+ { .name = "polarfire-fpga-mgr", },
+ {},
+};
+MODULE_DEVICE_TABLE(spi, microsemi_fpga_spi_ids);
+
+static const struct of_device_id microsemi_fpga_of_ids[] = {
+ { .compatible = "mscc,polarfire-fpga-mgr" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, microsemi_fpga_of_ids);
+
+static struct spi_driver microsemi_fpga_driver = {
+ .probe = microsemi_fpga_probe,
+ .id_table = microsemi_fpga_spi_ids,
+ .driver = {
+ .name = "microsemi_fpga_manager",
+ .of_match_table = of_match_ptr(microsemi_fpga_of_ids),
+ },
+};
+
+module_spi_driver(microsemi_fpga_driver);
+
+MODULE_DESCRIPTION("Microsemi FPGA Manager");
+MODULE_LICENSE("GPL");
--
2.34.1



2022-02-14 21:35:43

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] fpga: microsemi-spi: add Microsemi FPGA manager

Hi Ivan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.17-rc4 next-20220214]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Ivan-Bornyakov/fpga-microsemi-spi-add-Microsemi-FPGA-manager/20220214-222923
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 754e0b0e35608ed5206d6a67a791563c631cec07
config: h8300-allyesconfig (https://download.01.org/0day-ci/archive/20220215/[email protected]/config)
compiler: h8300-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/19d9c174f03a9b8387ba654d558351cac9d63d24
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Ivan-Bornyakov/fpga-microsemi-spi-add-Microsemi-FPGA-manager/20220214-222923
git checkout 19d9c174f03a9b8387ba654d558351cac9d63d24
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=h8300 SHELL=/bin/bash drivers/fpga/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

In file included from include/linux/build_bug.h:5,
from include/linux/container_of.h:5,
from include/linux/list.h:5,
from include/linux/module.h:12,
from drivers/fpga/microsemi-spi.c:6:
include/linux/scatterlist.h: In function 'sg_set_buf':
include/asm-generic/page.h:89:51: warning: ordered comparison of pointer with null pointer [-Wextra]
89 | #define virt_addr_valid(kaddr) (((void *)(kaddr) >= (void *)PAGE_OFFSET) && \
| ^~
include/linux/compiler.h:78:45: note: in definition of macro 'unlikely'
78 | # define unlikely(x) __builtin_expect(!!(x), 0)
| ^
include/linux/scatterlist.h:160:9: note: in expansion of macro 'BUG_ON'
160 | BUG_ON(!virt_addr_valid(buf));
| ^~~~~~
include/linux/scatterlist.h:160:17: note: in expansion of macro 'virt_addr_valid'
160 | BUG_ON(!virt_addr_valid(buf));
| ^~~~~~~~~~~~~~~
In file included from include/linux/device.h:15,
from include/linux/spi/spi.h:10,
from drivers/fpga/microsemi-spi.c:7:
drivers/fpga/microsemi-spi.c: In function 'microsemi_fpga_ops_write':
drivers/fpga/microsemi-spi.c:244:30: warning: format '%d' expects argument of type 'int', but argument 3 has type 'ssize_t' {aka 'long int'} [-Wformat=]
244 | dev_err(dev, "Failed to find bitstream start %d\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/fpga/microsemi-spi.c:244:17: note: in expansion of macro 'dev_err'
244 | dev_err(dev, "Failed to find bitstream start %d\n",
| ^~~~~~~
drivers/fpga/microsemi-spi.c:244:63: note: format string is defined here
244 | dev_err(dev, "Failed to find bitstream start %d\n",
| ~^
| |
| int
| %ld
In file included from include/linux/device.h:15,
from include/linux/spi/spi.h:10,
from drivers/fpga/microsemi-spi.c:7:
drivers/fpga/microsemi-spi.c:252:30: warning: format '%d' expects argument of type 'int', but argument 3 has type 'ssize_t' {aka 'long int'} [-Wformat=]
252 | dev_err(dev, "Failed to parse bitstream size %d\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/fpga/microsemi-spi.c:252:17: note: in expansion of macro 'dev_err'
252 | dev_err(dev, "Failed to parse bitstream size %d\n",
| ^~~~~~~
drivers/fpga/microsemi-spi.c:252:63: note: format string is defined here
252 | dev_err(dev, "Failed to parse bitstream size %d\n",
| ~^
| |
| int
| %ld
In file included from include/linux/device.h:15,
from include/linux/spi/spi.h:10,
from drivers/fpga/microsemi-spi.c:7:
drivers/fpga/microsemi-spi.c:260:25: warning: format '%d' expects argument of type 'int', but argument 3 has type 'ssize_t' {aka 'long int'} [-Wformat=]
260 | "Bitstram outruns firmware. Bitstream start %d, bitstream size %d, firmware size %d\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/fpga/microsemi-spi.c:259:17: note: in expansion of macro 'dev_err'
259 | dev_err(dev,
| ^~~~~~~
drivers/fpga/microsemi-spi.c:260:70: note: format string is defined here
260 | "Bitstram outruns firmware. Bitstream start %d, bitstream size %d, firmware size %d\n",
| ~^
| |
| int
| %ld
In file included from include/linux/device.h:15,
from include/linux/spi/spi.h:10,
from drivers/fpga/microsemi-spi.c:7:
>> drivers/fpga/microsemi-spi.c:260:25: warning: format '%d' expects argument of type 'int', but argument 4 has type 'long int' [-Wformat=]
260 | "Bitstram outruns firmware. Bitstream start %d, bitstream size %d, firmware size %d\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/fpga/microsemi-spi.c:259:17: note: in expansion of macro 'dev_err'
259 | dev_err(dev,
| ^~~~~~~
drivers/fpga/microsemi-spi.c:260:89: note: format string is defined here
260 | "Bitstram outruns firmware. Bitstream start %d, bitstream size %d, firmware size %d\n",
| ~^
| |
| int
| %ld
In file included from include/linux/device.h:15,
from include/linux/spi/spi.h:10,
from drivers/fpga/microsemi-spi.c:7:
drivers/fpga/microsemi-spi.c:260:25: warning: format '%d' expects argument of type 'int', but argument 5 has type 'size_t' {aka 'long unsigned int'} [-Wformat=]
260 | "Bitstram outruns firmware. Bitstream start %d, bitstream size %d, firmware size %d\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/fpga/microsemi-spi.c:259:17: note: in expansion of macro 'dev_err'
259 | dev_err(dev,
| ^~~~~~~
drivers/fpga/microsemi-spi.c:260:107: note: format string is defined here
260 | "Bitstram outruns firmware. Bitstream start %d, bitstream size %d, firmware size %d\n",
| ~^
| |
| int
| %ld
In file included from include/linux/device.h:15,
from include/linux/spi/spi.h:10,
from drivers/fpga/microsemi-spi.c:7:
drivers/fpga/microsemi-spi.c:274:33: warning: format '%d' expects argument of type 'int', but argument 4 has type 'ssize_t' {aka 'long int'} [-Wformat=]
274 | "Failed to write bitstream frame number %d of %d\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/fpga/microsemi-spi.c:273:25: note: in expansion of macro 'dev_err'
273 | dev_err(dev,
| ^~~~~~~
drivers/fpga/microsemi-spi.c:274:80: note: format string is defined here
274 | "Failed to write bitstream frame number %d of %d\n",
| ~^
| |
| int
| %ld


vim +260 drivers/fpga/microsemi-spi.c

225
226 static int microsemi_fpga_ops_write(struct fpga_manager *mgr, const char *buf,
227 size_t count)
228 {
229 ssize_t bitstream_start = 0, bitstream_size;
230 struct microsemi_fpga_priv *priv = mgr->priv;
231 struct spi_device *spi = priv->spi;
232 struct device *dev = &mgr->dev;
233 u8 tmp_buf[SPI_FRAME_SIZE + 1];
234 int ret, i;
235
236 if (crc_ccitt(0, buf, count)) {
237 dev_err(dev, "CRC error\n");
238
239 return -EINVAL;
240 }
241
242 bitstream_start = lookup_block_start(BITSTREAM_ID, buf, count);
243 if (bitstream_start < 0) {
244 dev_err(dev, "Failed to find bitstream start %d\n",
245 bitstream_start);
246
247 return bitstream_start;
248 }
249
250 bitstream_size = parse_bitstream_size(buf, count);
251 if (bitstream_size < 0) {
252 dev_err(dev, "Failed to parse bitstream size %d\n",
253 bitstream_size);
254
255 return bitstream_size;
256 }
257
258 if (bitstream_start + bitstream_size * SPI_FRAME_SIZE > count) {
259 dev_err(dev,
> 260 "Bitstram outruns firmware. Bitstream start %d, bitstream size %d, firmware size %d\n",
261 bitstream_start, bitstream_size * SPI_FRAME_SIZE, count);
262
263 return -EFAULT;
264 }
265
266 for (i = 0; i < bitstream_size; i++) {
267 tmp_buf[0] = SPI_FRAME;
268 memcpy(tmp_buf + 1, buf + bitstream_start + i * SPI_FRAME_SIZE,
269 SPI_FRAME_SIZE);
270
271 ret = microsemi_spi_write(spi, tmp_buf, sizeof(tmp_buf));
272 if (ret) {
273 dev_err(dev,
274 "Failed to write bitstream frame number %d of %d\n",
275 i, bitstream_size);
276
277 return ret;
278 }
279 }
280
281 return 0;
282 }
283

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

2022-02-14 21:36:13

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] fpga: microsemi-spi: add Microsemi FPGA manager

Hi Ivan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.17-rc4 next-20220214]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Ivan-Bornyakov/fpga-microsemi-spi-add-Microsemi-FPGA-manager/20220214-222923
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 754e0b0e35608ed5206d6a67a791563c631cec07
config: s390-allyesconfig (https://download.01.org/0day-ci/archive/20220215/[email protected]/config)
compiler: s390-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/19d9c174f03a9b8387ba654d558351cac9d63d24
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Ivan-Bornyakov/fpga-microsemi-spi-add-Microsemi-FPGA-manager/20220214-222923
git checkout 19d9c174f03a9b8387ba654d558351cac9d63d24
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=s390 SHELL=/bin/bash drivers/fpga/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

In file included from include/linux/device.h:15,
from include/linux/spi/spi.h:10,
from drivers/fpga/microsemi-spi.c:7:
drivers/fpga/microsemi-spi.c: In function 'microsemi_fpga_ops_write':
>> drivers/fpga/microsemi-spi.c:244:30: warning: format '%d' expects argument of type 'int', but argument 3 has type 'ssize_t' {aka 'long int'} [-Wformat=]
244 | dev_err(dev, "Failed to find bitstream start %d\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/fpga/microsemi-spi.c:244:17: note: in expansion of macro 'dev_err'
244 | dev_err(dev, "Failed to find bitstream start %d\n",
| ^~~~~~~
drivers/fpga/microsemi-spi.c:244:63: note: format string is defined here
244 | dev_err(dev, "Failed to find bitstream start %d\n",
| ~^
| |
| int
| %ld
In file included from include/linux/device.h:15,
from include/linux/spi/spi.h:10,
from drivers/fpga/microsemi-spi.c:7:
drivers/fpga/microsemi-spi.c:252:30: warning: format '%d' expects argument of type 'int', but argument 3 has type 'ssize_t' {aka 'long int'} [-Wformat=]
252 | dev_err(dev, "Failed to parse bitstream size %d\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/fpga/microsemi-spi.c:252:17: note: in expansion of macro 'dev_err'
252 | dev_err(dev, "Failed to parse bitstream size %d\n",
| ^~~~~~~
drivers/fpga/microsemi-spi.c:252:63: note: format string is defined here
252 | dev_err(dev, "Failed to parse bitstream size %d\n",
| ~^
| |
| int
| %ld
In file included from include/linux/device.h:15,
from include/linux/spi/spi.h:10,
from drivers/fpga/microsemi-spi.c:7:
drivers/fpga/microsemi-spi.c:260:25: warning: format '%d' expects argument of type 'int', but argument 3 has type 'ssize_t' {aka 'long int'} [-Wformat=]
260 | "Bitstram outruns firmware. Bitstream start %d, bitstream size %d, firmware size %d\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/fpga/microsemi-spi.c:259:17: note: in expansion of macro 'dev_err'
259 | dev_err(dev,
| ^~~~~~~
drivers/fpga/microsemi-spi.c:260:70: note: format string is defined here
260 | "Bitstram outruns firmware. Bitstream start %d, bitstream size %d, firmware size %d\n",
| ~^
| |
| int
| %ld
In file included from include/linux/device.h:15,
from include/linux/spi/spi.h:10,
from drivers/fpga/microsemi-spi.c:7:
drivers/fpga/microsemi-spi.c:260:25: warning: format '%d' expects argument of type 'int', but argument 4 has type 'ssize_t' {aka 'long int'} [-Wformat=]
260 | "Bitstram outruns firmware. Bitstream start %d, bitstream size %d, firmware size %d\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/fpga/microsemi-spi.c:259:17: note: in expansion of macro 'dev_err'
259 | dev_err(dev,
| ^~~~~~~
drivers/fpga/microsemi-spi.c:260:89: note: format string is defined here
260 | "Bitstram outruns firmware. Bitstream start %d, bitstream size %d, firmware size %d\n",
| ~^
| |
| int
| %ld
In file included from include/linux/device.h:15,
from include/linux/spi/spi.h:10,
from drivers/fpga/microsemi-spi.c:7:
>> drivers/fpga/microsemi-spi.c:260:25: warning: format '%d' expects argument of type 'int', but argument 5 has type 'size_t' {aka 'long unsigned int'} [-Wformat=]
260 | "Bitstram outruns firmware. Bitstream start %d, bitstream size %d, firmware size %d\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/fpga/microsemi-spi.c:259:17: note: in expansion of macro 'dev_err'
259 | dev_err(dev,
| ^~~~~~~
drivers/fpga/microsemi-spi.c:260:107: note: format string is defined here
260 | "Bitstram outruns firmware. Bitstream start %d, bitstream size %d, firmware size %d\n",
| ~^
| |
| int
| %ld
In file included from include/linux/device.h:15,
from include/linux/spi/spi.h:10,
from drivers/fpga/microsemi-spi.c:7:
drivers/fpga/microsemi-spi.c:274:33: warning: format '%d' expects argument of type 'int', but argument 4 has type 'ssize_t' {aka 'long int'} [-Wformat=]
274 | "Failed to write bitstream frame number %d of %d\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/fpga/microsemi-spi.c:273:25: note: in expansion of macro 'dev_err'
273 | dev_err(dev,
| ^~~~~~~
drivers/fpga/microsemi-spi.c:274:80: note: format string is defined here
274 | "Failed to write bitstream frame number %d of %d\n",
| ~^
| |
| int
| %ld


vim +244 drivers/fpga/microsemi-spi.c

225
226 static int microsemi_fpga_ops_write(struct fpga_manager *mgr, const char *buf,
227 size_t count)
228 {
229 ssize_t bitstream_start = 0, bitstream_size;
230 struct microsemi_fpga_priv *priv = mgr->priv;
231 struct spi_device *spi = priv->spi;
232 struct device *dev = &mgr->dev;
233 u8 tmp_buf[SPI_FRAME_SIZE + 1];
234 int ret, i;
235
236 if (crc_ccitt(0, buf, count)) {
237 dev_err(dev, "CRC error\n");
238
239 return -EINVAL;
240 }
241
242 bitstream_start = lookup_block_start(BITSTREAM_ID, buf, count);
243 if (bitstream_start < 0) {
> 244 dev_err(dev, "Failed to find bitstream start %d\n",
245 bitstream_start);
246
247 return bitstream_start;
248 }
249
250 bitstream_size = parse_bitstream_size(buf, count);
251 if (bitstream_size < 0) {
252 dev_err(dev, "Failed to parse bitstream size %d\n",
253 bitstream_size);
254
255 return bitstream_size;
256 }
257
258 if (bitstream_start + bitstream_size * SPI_FRAME_SIZE > count) {
259 dev_err(dev,
> 260 "Bitstram outruns firmware. Bitstream start %d, bitstream size %d, firmware size %d\n",
261 bitstream_start, bitstream_size * SPI_FRAME_SIZE, count);
262
263 return -EFAULT;
264 }
265
266 for (i = 0; i < bitstream_size; i++) {
267 tmp_buf[0] = SPI_FRAME;
268 memcpy(tmp_buf + 1, buf + bitstream_start + i * SPI_FRAME_SIZE,
269 SPI_FRAME_SIZE);
270
271 ret = microsemi_spi_write(spi, tmp_buf, sizeof(tmp_buf));
272 if (ret) {
273 dev_err(dev,
274 "Failed to write bitstream frame number %d of %d\n",
275 i, bitstream_size);
276
277 return ret;
278 }
279 }
280
281 return 0;
282 }
283

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

2022-02-15 16:55:35

by Ivan Bornyakov

[permalink] [raw]
Subject: [PATCH v2] fpga: microsemi-spi: add Microsemi FPGA manager

Add support to the FPGA manager for programming Microsemi Polarfire
FPGAs over slave SPI interface.

Signed-off-by: Ivan Bornyakov <[email protected]>
---
Changelog:
v1 -> v2: fix printk formating

drivers/fpga/Kconfig | 9 +
drivers/fpga/Makefile | 1 +
drivers/fpga/microsemi-spi.c | 366 +++++++++++++++++++++++++++++++++++
3 files changed, 376 insertions(+)
create mode 100644 drivers/fpga/microsemi-spi.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 991b3f361ec9..25c2631a387c 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -243,4 +243,13 @@ config FPGA_MGR_VERSAL_FPGA
configure the programmable logic(PL).

To compile this as a module, choose M here.
+
+config FPGA_MGR_MICROSEMI_SPI
+ tristate "Microsemi FPGA manager"
+ depends on SPI
+ select CRC_CCITT
+ help
+ FPGA manager driver support for Microsemi Polarfire FPGAs
+ programming over slave SPI interface.
+
endif # FPGA
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 0bff783d1b61..8b3d818546a6 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_MICROSEMI_SPI) += microsemi-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/microsemi-spi.c b/drivers/fpga/microsemi-spi.c
new file mode 100644
index 000000000000..facbc8f600be
--- /dev/null
+++ b/drivers/fpga/microsemi-spi.c
@@ -0,0 +1,366 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Microsemi 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)
+#define STATUS_SPI_VIOLATION BIT(2)
+#define STATUS_SPI_ERROR BIT(3)
+
+struct microsemi_fpga_priv {
+ struct spi_device *spi;
+ bool program_mode;
+};
+
+static enum fpga_mgr_states microsemi_fpga_ops_state(struct fpga_manager *mgr)
+{
+ struct microsemi_fpga_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 microsemi_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 microsemi_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 = microsemi_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 microsemi_fpga_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 microsemi_fpga_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 = microsemi_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 = microsemi_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 microsemi_fpga_ops_write(struct fpga_manager *mgr, const char *buf,
+ size_t count)
+{
+ ssize_t bitstream_start = 0, bitstream_size;
+ struct microsemi_fpga_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 = microsemi_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 microsemi_fpga_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 microsemi_fpga_priv *priv = mgr->priv;
+ struct spi_device *spi = priv->spi;
+ struct device *dev = &mgr->dev;
+ int ret;
+
+ ret = microsemi_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 = microsemi_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 microsemi_fpga_ops = {
+ .state = microsemi_fpga_ops_state,
+ .write_init = microsemi_fpga_ops_write_init,
+ .write = microsemi_fpga_ops_write,
+ .write_complete = microsemi_fpga_ops_write_complete,
+};
+
+static int microsemi_fpga_probe(struct spi_device *spi)
+{
+ struct microsemi_fpga_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, "Microsemi FPGA Manager",
+ &microsemi_fpga_ops, priv);
+
+ return PTR_ERR_OR_ZERO(mgr);
+}
+
+static const struct spi_device_id microsemi_fpga_spi_ids[] = {
+ { .name = "polarfire-fpga-mgr", },
+ {},
+};
+MODULE_DEVICE_TABLE(spi, microsemi_fpga_spi_ids);
+
+static const struct of_device_id microsemi_fpga_of_ids[] = {
+ { .compatible = "mscc,polarfire-fpga-mgr" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, microsemi_fpga_of_ids);
+
+static struct spi_driver microsemi_fpga_driver = {
+ .probe = microsemi_fpga_probe,
+ .id_table = microsemi_fpga_spi_ids,
+ .driver = {
+ .name = "microsemi_fpga_manager",
+ .of_match_table = of_match_ptr(microsemi_fpga_of_ids),
+ },
+};
+
+module_spi_driver(microsemi_fpga_driver);
+
+MODULE_DESCRIPTION("Microsemi FPGA Manager");
+MODULE_LICENSE("GPL");
--
2.34.1


2022-02-15 21:55:18

by Ivan Bornyakov

[permalink] [raw]
Subject: Re: [PATCH v2] fpga: microsemi-spi: add Microsemi FPGA manager

Hello Conor, Moritz

On Tue, Feb 15, 2022 at 09:52:30AM -0800, Moritz Fischer wrote:
> Hi Conor, Ivan,
>
> On Tue, Feb 15, 2022 at 05:37:04PM +0000, Conor Dooley wrote:
> > Hey Ivan,
> > Firstly thanks for the patch(es), stumbled across them today.
> > As you may know Microsemi has been acquired by Microchip, so
> > s/microsemi/microchip/ please. This would make the correct vendor
> > prefix for compatible strings "microchip". While you've said this is
> > for the PolarFire FPGA, there is prescendent for using "mpfs" for the
> > PolarFire SoC FPGA in the kernel - so if you could change the uses of
> > "polarfire" to "mpf" that'd be great.
>
> I personally don't have a strong opinion on hte microchip vs microsemi
> here. We have precedent with intel/altera.
>

Me neither, so I'll do what Conor asked.

> >
> > The current item on my own todo list is the opposite side of this,
> > reprogramming the FPGA via the system controller acting as a SPI
> > master for PolarFire SoC.
> > I will get back to you when I have a better idea of what (if any) code
> > can be made generic between both modes. In the meantime, I will get
> > together a setup to test SPI slave reprogramming of the PolarFire (SoC)
> >
> > Thanks,
> > Conor <[email protected]>
>
> Thanks for chiming in. Always nice to have vendors help out reviewing.
>

Yeah, that's great, thanks in advance Conor.

> > > Add support to the FPGA manager for programming Microsemi Polarfire
> > > FPGAs over slave SPI interface.
> > >
> > > Signed-off-by: Ivan Bornyakov <[email protected]>
> > > ---
> > > Changelog:
> > > v1 -> v2: fix printk formating
> > >
> > > drivers/fpga/Kconfig | 9 +
> > > drivers/fpga/Makefile | 1 +
> > > drivers/fpga/microsemi-spi.c | 366 +++++++++++++++++++++++++++++++++++
> > > 3 files changed, 376 insertions(+)
> > > create mode 100644 drivers/fpga/microsemi-spi.c
> > >
> > --<snip>--
>
> I'll take a closer look once the bot's complaints are addressed.
>
> Thanks,
> Moritz

Thanks in advance Moritz.

2022-02-16 02:54:00

by Moritz Fischer

[permalink] [raw]
Subject: Re: [PATCH v2] fpga: microsemi-spi: add Microsemi FPGA manager

Hi Conor, Ivan,

On Tue, Feb 15, 2022 at 05:37:04PM +0000, Conor Dooley wrote:
> Hey Ivan,
> Firstly thanks for the patch(es), stumbled across them today.
> As you may know Microsemi has been acquired by Microchip, so
> s/microsemi/microchip/ please. This would make the correct vendor
> prefix for compatible strings "microchip". While you've said this is
> for the PolarFire FPGA, there is prescendent for using "mpfs" for the
> PolarFire SoC FPGA in the kernel - so if you could change the uses of
> "polarfire" to "mpf" that'd be great.

I personally don't have a strong opinion on hte microchip vs microsemi
here. We have precedent with intel/altera.

>
> The current item on my own todo list is the opposite side of this,
> reprogramming the FPGA via the system controller acting as a SPI
> master for PolarFire SoC.
> I will get back to you when I have a better idea of what (if any) code
> can be made generic between both modes. In the meantime, I will get
> together a setup to test SPI slave reprogramming of the PolarFire (SoC)
>
> Thanks,
> Conor <[email protected]>

Thanks for chiming in. Always nice to have vendors help out reviewing.
>
> > Add support to the FPGA manager for programming Microsemi Polarfire
> > FPGAs over slave SPI interface.
> >
> > Signed-off-by: Ivan Bornyakov <[email protected]>
> > ---
> > Changelog:
> > v1 -> v2: fix printk formating
> >
> > drivers/fpga/Kconfig | 9 +
> > drivers/fpga/Makefile | 1 +
> > drivers/fpga/microsemi-spi.c | 366 +++++++++++++++++++++++++++++++++++
> > 3 files changed, 376 insertions(+)
> > create mode 100644 drivers/fpga/microsemi-spi.c
> >
> --<snip>--

I'll take a closer look once the bot's complaints are addressed.

Thanks,
Moritz

2022-02-16 06:58:41

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2] fpga: microsemi-spi: add Microsemi FPGA manager

Hey Ivan,
Firstly thanks for the patch(es), stumbled across them today.
As you may know Microsemi has been acquired by Microchip, so
s/microsemi/microchip/ please. This would make the correct vendor
prefix for compatible strings "microchip". While you've said this is
for the PolarFire FPGA, there is prescendent for using "mpfs" for the
PolarFire SoC FPGA in the kernel - so if you could change the uses of
"polarfire" to "mpf" that'd be great.

The current item on my own todo list is the opposite side of this,
reprogramming the FPGA via the system controller acting as a SPI
master for PolarFire SoC.
I will get back to you when I have a better idea of what (if any) code
can be made generic between both modes. In the meantime, I will get
together a setup to test SPI slave reprogramming of the PolarFire (SoC)

Thanks,
Conor <[email protected]>

> Add support to the FPGA manager for programming Microsemi Polarfire
> FPGAs over slave SPI interface.
>
> Signed-off-by: Ivan Bornyakov <[email protected]>
> ---
> Changelog:
> v1 -> v2: fix printk formating
>
> drivers/fpga/Kconfig | 9 +
> drivers/fpga/Makefile | 1 +
> drivers/fpga/microsemi-spi.c | 366 +++++++++++++++++++++++++++++++++++
> 3 files changed, 376 insertions(+)
> create mode 100644 drivers/fpga/microsemi-spi.c
>
--<snip>--

2022-02-16 13:22:05

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2] fpga: microsemi-spi: add Microsemi FPGA manager



On 15/02/2022 11:58, Ivan Bornyakov wrote:
> Add support to the FPGA manager for programming Microsemi Polarfire
> FPGAs over slave SPI interface.
>
> Signed-off-by: Ivan Bornyakov <[email protected]>
> ---
> Changelog:
> v1 -> v2: fix printk formating
>
> drivers/fpga/Kconfig | 9 +
> drivers/fpga/Makefile | 1 +
> drivers/fpga/microsemi-spi.c | 366 +++++++++++++++++++++++++++++++++++
> 3 files changed, 376 insertions(+)
> create mode 100644 drivers/fpga/microsemi-spi.c
>
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index 991b3f361ec9..25c2631a387c 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -243,4 +243,13 @@ config FPGA_MGR_VERSAL_FPGA
> configure the programmable logic(PL).
>
> To compile this as a module, choose M here.
> +
> +config FPGA_MGR_MICROSEMI_SPI
> + tristate "Microsemi FPGA manager"
> + depends on SPI
> + select CRC_CCITT
> + help
> + FPGA manager driver support for Microsemi Polarfire FPGAs
> + programming over slave SPI interface.
> +
> endif # FPGA
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 0bff783d1b61..8b3d818546a6 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_MICROSEMI_SPI) += microsemi-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/microsemi-spi.c b/drivers/fpga/microsemi-spi.c
> new file mode 100644
> index 000000000000..facbc8f600be
> --- /dev/null
> +++ b/drivers/fpga/microsemi-spi.c
> @@ -0,0 +1,366 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Microsemi 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)
> +#define STATUS_SPI_VIOLATION BIT(2)
> +#define STATUS_SPI_ERROR BIT(3)
> +
> +struct microsemi_fpga_priv {
> + struct spi_device *spi;
> + bool program_mode;
> +};
> +
> +static enum fpga_mgr_states microsemi_fpga_ops_state(struct fpga_manager *mgr)
> +{
> + struct microsemi_fpga_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 microsemi_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 microsemi_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 = microsemi_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 microsemi_fpga_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 microsemi_fpga_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 = microsemi_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 = microsemi_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 microsemi_fpga_ops_write(struct fpga_manager *mgr, const char *buf,
> + size_t count)
> +{
> + ssize_t bitstream_start = 0, bitstream_size;
> + struct microsemi_fpga_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 = microsemi_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 microsemi_fpga_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 microsemi_fpga_priv *priv = mgr->priv;
> + struct spi_device *spi = priv->spi;
> + struct device *dev = &mgr->dev;
> + int ret;
> +
> + ret = microsemi_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 = microsemi_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 microsemi_fpga_ops = {
> + .state = microsemi_fpga_ops_state,
> + .write_init = microsemi_fpga_ops_write_init,
> + .write = microsemi_fpga_ops_write,
> + .write_complete = microsemi_fpga_ops_write_complete,
> +};
> +
> +static int microsemi_fpga_probe(struct spi_device *spi)
> +{
> + struct microsemi_fpga_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, "Microsemi FPGA Manager",
> + &microsemi_fpga_ops, priv);
Something else quick that I noticed today is that this string, plus the
.name strings, compatible string etc should indicate that this is spi
slave programming mode as opposed to the other possible ones (jtag/iap)
> +
> + return PTR_ERR_OR_ZERO(mgr);
> +}
> +
> +static const struct spi_device_id microsemi_fpga_spi_ids[] = {
> + { .name = "polarfire-fpga-mgr", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(spi, microsemi_fpga_spi_ids);
> +
> +static const struct of_device_id microsemi_fpga_of_ids[] = {
> + { .compatible = "mscc,polarfire-fpga-mgr" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, microsemi_fpga_of_ids);
> +
> +static struct spi_driver microsemi_fpga_driver = {
> + .probe = microsemi_fpga_probe,
> + .id_table = microsemi_fpga_spi_ids,
> + .driver = {
> + .name = "microsemi_fpga_manager",
> + .of_match_table = of_match_ptr(microsemi_fpga_of_ids),
> + },
> +};
> +
> +module_spi_driver(microsemi_fpga_driver);
> +
> +MODULE_DESCRIPTION("Microsemi FPGA Manager");
> +MODULE_LICENSE("GPL");

2022-02-16 18:16:41

by Ivan Bornyakov

[permalink] [raw]
Subject: [PATCH v3] fpga: microchip-spi: add Microchip FPGA manager

Add support to the FPGA manager for programming Microchip Polarfire
FPGAs over slave SPI interface.

Signed-off-by: Ivan Bornyakov <[email protected]>
---
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

drivers/fpga/Kconfig | 9 +
drivers/fpga/Makefile | 1 +
drivers/fpga/microchip-spi.c | 359 +++++++++++++++++++++++++++++++++++
3 files changed, 369 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..c0bf7a8f136f
--- /dev/null
+++ b/drivers/fpga/microchip-spi.c
@@ -0,0 +1,359 @@
+// 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);
+
+static const struct of_device_id mpf_of_ids[] = {
+ { .compatible = "microchip,mpf-spi-fpga-mgr" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mpf_of_ids);
+
+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


2022-02-17 23:57:33

by Ivan Bornyakov

[permalink] [raw]
Subject: [PATCH v4] fpga: microchip-spi: add Microchip FPGA manager

Add support to the FPGA manager for programming Microchip Polarfire
FPGAs over slave SPI interface.

Signed-off-by: Ivan Bornyakov <[email protected]>
---
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");
--
2.34.1


2022-02-18 15:59:38

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v4] fpga: microchip-spi: add Microchip FPGA manager

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 <[email protected]>
> ---
> 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");

2022-02-18 21:25:47

by Xu Yilun

[permalink] [raw]
Subject: Re: [PATCH v4] fpga: microchip-spi: add Microchip FPGA manager

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 <[email protected]>
> ---
> 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
>

2022-02-21 05:18:11

by Ivan Bornyakov

[permalink] [raw]
Subject: Re: [PATCH v4] fpga: microchip-spi: add Microchip FPGA manager

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.

2022-02-21 06:13:03

by Ivan Bornyakov

[permalink] [raw]
Subject: Re: [PATCH v4] fpga: microchip-spi: add Microchip FPGA manager

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

2022-02-21 09:09:13

by Ivan Bornyakov

[permalink] [raw]
Subject: Re: [PATCH v4] fpga: microchip-spi: add Microchip FPGA manager

Hi, Conor

On Fri, Feb 18, 2022 at 03:22:57PM +0000, [email protected] 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.