linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] fpga: microsemi-spi: add Microsemi FPGA manager
@ 2022-02-14 13:38 Ivan Bornyakov
  2022-02-14 19:22 ` kernel test robot
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Ivan Bornyakov @ 2022-02-14 13:38 UTC (permalink / raw)
  Cc: mdf, hao.wu, yilun.xu, trix, linux-kernel, linux-fpga, system,
	Ivan Bornyakov

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

Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
---
 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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] fpga: microsemi-spi: add Microsemi FPGA manager
  2022-02-14 13:38 [PATCH] fpga: microsemi-spi: add Microsemi FPGA manager Ivan Bornyakov
@ 2022-02-14 19:22 ` kernel test robot
  2022-02-14 20:34 ` kernel test robot
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2022-02-14 19:22 UTC (permalink / raw)
  To: Ivan Bornyakov
  Cc: kbuild-all, mdf, hao.wu, yilun.xu, trix, linux-kernel,
	linux-fpga, system, Ivan Bornyakov

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/202202150337.lVXXx685-lkp@intel.com/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 <lkp@intel.com>

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/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] fpga: microsemi-spi: add Microsemi FPGA manager
  2022-02-14 13:38 [PATCH] fpga: microsemi-spi: add Microsemi FPGA manager Ivan Bornyakov
  2022-02-14 19:22 ` kernel test robot
@ 2022-02-14 20:34 ` kernel test robot
  2022-02-15 11:58 ` [PATCH v2] " Ivan Bornyakov
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2022-02-14 20:34 UTC (permalink / raw)
  To: Ivan Bornyakov
  Cc: kbuild-all, mdf, hao.wu, yilun.xu, trix, linux-kernel,
	linux-fpga, system, Ivan Bornyakov

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/202202150434.i0eTuy1u-lkp@intel.com/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 <lkp@intel.com>

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/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2] fpga: microsemi-spi: add Microsemi FPGA manager
  2022-02-14 13:38 [PATCH] fpga: microsemi-spi: add Microsemi FPGA manager Ivan Bornyakov
  2022-02-14 19:22 ` kernel test robot
  2022-02-14 20:34 ` kernel test robot
@ 2022-02-15 11:58 ` Ivan Bornyakov
  2022-02-15 17:37   ` Conor Dooley
  2022-02-16 12:55   ` Conor.Dooley
  2022-02-16 15:56 ` [PATCH v3] fpga: microchip-spi: add Microchip " Ivan Bornyakov
  2022-02-17 19:18 ` [PATCH v4] " Ivan Bornyakov
  4 siblings, 2 replies; 15+ messages in thread
From: Ivan Bornyakov @ 2022-02-15 11:58 UTC (permalink / raw)
  Cc: mdf, hao.wu, yilun.xu, trix, linux-kernel, linux-fpga, system,
	Ivan Bornyakov

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

Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
---
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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] fpga: microsemi-spi: add Microsemi FPGA manager
  2022-02-15 11:58 ` [PATCH v2] " Ivan Bornyakov
@ 2022-02-15 17:37   ` Conor Dooley
  2022-02-15 17:52     ` Moritz Fischer
  2022-02-16 12:55   ` Conor.Dooley
  1 sibling, 1 reply; 15+ messages in thread
From: Conor Dooley @ 2022-02-15 17:37 UTC (permalink / raw)
  To: Ivan Bornyakov
  Cc: mdf, hao.wu, yilun.xu, trix, linux-kernel, linux-fpga, system

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 <conor.dooley@microchip.com>

 > Add support to the FPGA manager for programming Microsemi Polarfire
 > FPGAs over slave SPI interface.
 >
 > Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
 > ---
 > 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>--

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] fpga: microsemi-spi: add Microsemi FPGA manager
  2022-02-15 17:37   ` Conor Dooley
@ 2022-02-15 17:52     ` Moritz Fischer
  2022-02-15 18:05       ` Ivan Bornyakov
  0 siblings, 1 reply; 15+ messages in thread
From: Moritz Fischer @ 2022-02-15 17:52 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Ivan Bornyakov, mdf, hao.wu, yilun.xu, trix, linux-kernel,
	linux-fpga, system

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 <conor.dooley@microchip.com>

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 <i.bornyakov@metrotek.ru>
> > ---
> > 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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] fpga: microsemi-spi: add Microsemi FPGA manager
  2022-02-15 17:52     ` Moritz Fischer
@ 2022-02-15 18:05       ` Ivan Bornyakov
  0 siblings, 0 replies; 15+ messages in thread
From: Ivan Bornyakov @ 2022-02-15 18:05 UTC (permalink / raw)
  To: Moritz Fischer
  Cc: Conor Dooley, hao.wu, yilun.xu, trix, linux-kernel, linux-fpga, system

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 <conor.dooley@microchip.com>
> 
> 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 <i.bornyakov@metrotek.ru>
> > > ---
> > > 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.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] fpga: microsemi-spi: add Microsemi FPGA manager
  2022-02-15 11:58 ` [PATCH v2] " Ivan Bornyakov
  2022-02-15 17:37   ` Conor Dooley
@ 2022-02-16 12:55   ` Conor.Dooley
  1 sibling, 0 replies; 15+ messages in thread
From: Conor.Dooley @ 2022-02-16 12:55 UTC (permalink / raw)
  To: i.bornyakov; +Cc: mdf, hao.wu, yilun.xu, trix, linux-kernel, linux-fpga, system



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 <i.bornyakov@metrotek.ru>
> ---
> 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");

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3] fpga: microchip-spi: add Microchip FPGA manager
  2022-02-14 13:38 [PATCH] fpga: microsemi-spi: add Microsemi FPGA manager Ivan Bornyakov
                   ` (2 preceding siblings ...)
  2022-02-15 11:58 ` [PATCH v2] " Ivan Bornyakov
@ 2022-02-16 15:56 ` Ivan Bornyakov
  2022-02-17 19:18 ` [PATCH v4] " Ivan Bornyakov
  4 siblings, 0 replies; 15+ messages in thread
From: Ivan Bornyakov @ 2022-02-16 15:56 UTC (permalink / raw)
  Cc: mdf, hao.wu, yilun.xu, trix, conor.dooley, linux-kernel,
	linux-fpga, system, Ivan Bornyakov

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

Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
---
Changelog:
  v1 -> v2: fix printk formating
  v2 -> v3:
   * replace "microsemi" with "microchip"
   * replace prefix "microsemi_fpga_" with "mpf_"
   * more sensible .compatible and .name strings
   * remove unused defines STATUS_SPI_VIOLATION and STATUS_SPI_ERROR

 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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4] fpga: microchip-spi: add Microchip FPGA manager
  2022-02-14 13:38 [PATCH] fpga: microsemi-spi: add Microsemi FPGA manager Ivan Bornyakov
                   ` (3 preceding siblings ...)
  2022-02-16 15:56 ` [PATCH v3] fpga: microchip-spi: add Microchip " Ivan Bornyakov
@ 2022-02-17 19:18 ` Ivan Bornyakov
  2022-02-18 15:22   ` Conor.Dooley
  2022-02-18 16:05   ` Xu Yilun
  4 siblings, 2 replies; 15+ messages in thread
From: Ivan Bornyakov @ 2022-02-17 19:18 UTC (permalink / raw)
  Cc: mdf, hao.wu, yilun.xu, trix, conor.dooley, linux-kernel,
	linux-fpga, system, Ivan Bornyakov

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

Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
---
Changelog:
  v1 -> v2: fix printk formating
  v2 -> v3:
   * replace "microsemi" with "microchip"
   * replace prefix "microsemi_fpga_" with "mpf_"
   * more sensible .compatible and .name strings
   * remove unused defines STATUS_SPI_VIOLATION and STATUS_SPI_ERROR
  v3 -> v4: fix unused variable warning
    Put 'mpf_of_ids' definition under conditional compilation, so it
    would not hang unused if CONFIG_OF is not enabled.

 drivers/fpga/Kconfig         |   9 +
 drivers/fpga/Makefile        |   1 +
 drivers/fpga/microchip-spi.c | 361 +++++++++++++++++++++++++++++++++++
 3 files changed, 371 insertions(+)
 create mode 100644 drivers/fpga/microchip-spi.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 26025dbab353..4240c641b100 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -248,4 +248,13 @@ config FPGA_MGR_VERSAL_FPGA
 	  configure the programmable logic(PL).
 
 	  To compile this as a module, choose M here.
+
+config FPGA_MGR_MICROCHIP_SPI
+	tristate "Microchip Polarfire SPI FPGA manager"
+	depends on SPI
+	select CRC_CCITT
+	help
+	  FPGA manager driver support for Microchip Polarfire FPGAs
+	  programming over slave SPI interface.
+
 endif # FPGA
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 4da5273948df..fcb389ca4873 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_FPGA_MGR_XILINX_SPI)	+= xilinx-spi.o
 obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
 obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA)	+= zynqmp-fpga.o
 obj-$(CONFIG_FPGA_MGR_VERSAL_FPGA)      += versal-fpga.o
+obj-$(CONFIG_FPGA_MGR_MICROCHIP_SPI)	+= microchip-spi.o
 obj-$(CONFIG_ALTERA_PR_IP_CORE)         += altera-pr-ip-core.o
 obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT)    += altera-pr-ip-core-plat.o

diff --git a/drivers/fpga/microchip-spi.c b/drivers/fpga/microchip-spi.c
new file mode 100644
index 000000000000..5db25734a27a
--- /dev/null
+++ b/drivers/fpga/microchip-spi.c
@@ -0,0 +1,361 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Microchip Polarfire FPGA programming over slave SPI interface.
+ */
+
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+#include <linux/of_device.h>
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/delay.h>
+#include <linux/crc-ccitt.h>
+
+#define	SPI_ISC_ENABLE		0x0B
+#define	SPI_ISC_DISABLE		0x0C
+#define	SPI_READ_STATUS		0x00
+#define	SPI_READ_DATA		0x01
+#define	SPI_FRAME_INIT		0xAE
+#define	SPI_FRAME		0xEE
+#define	SPI_PRG_MODE		0x01
+#define	SPI_RELEASE		0x23
+
+#define	SPI_FRAME_SIZE		16
+
+#define	HEADER_SIZE_OFFSET	24
+#define	DATA_SIZE_OFFSET	55
+
+#define	LOOKUP_TABLE_RECORD_SIZE	9
+#define	LOOKUP_TABLE_BLOCK_ID_OFFSET	0
+#define	LOOKUP_TABLE_BLOCK_START_OFFSET	1
+
+#define	COMPONENTS_SIZE_ID	5
+#define	BITSTREAM_ID		8
+
+#define	BITS_PER_COMPONENT_SIZE	22
+
+#define	STATUS_POLL_TIMEOUT_MS	1000
+#define	STATUS_BUSY		BIT(0)
+#define	STATUS_READY		BIT(1)
+
+struct mpf_priv {
+	struct spi_device *spi;
+	bool program_mode;
+};
+
+static enum fpga_mgr_states mpf_ops_state(struct fpga_manager *mgr)
+{
+	struct mpf_priv *priv = mgr->priv;
+	struct spi_device *spi = priv->spi;
+	bool program_mode = priv->program_mode;
+	ssize_t status;
+
+	status = spi_w8r8(spi, SPI_READ_STATUS);
+
+	if (!program_mode && !status)
+		return FPGA_MGR_STATE_OPERATING;
+
+	return FPGA_MGR_STATE_UNKNOWN;
+}
+
+static int poll_status_not_busy(struct spi_device *spi, u8 mask)
+{
+	ssize_t status, timeout = STATUS_POLL_TIMEOUT_MS;
+
+	while (timeout--) {
+		status = spi_w8r8(spi, SPI_READ_STATUS);
+		if (status < 0)
+			return status;
+
+		if (mask) {
+			if (!(status & STATUS_BUSY) && (status & mask))
+				return status;
+		} else {
+			if (!(status & STATUS_BUSY))
+				return status;
+		}
+
+		mdelay(1);
+	}
+
+	return -EBUSY;
+}
+
+static int mpf_spi_write(struct spi_device *spi, const void *buf, size_t buf_size)
+{
+	int status = poll_status_not_busy(spi, 0);
+
+	if (status < 0)
+		return status;
+
+	return spi_write(spi, buf, buf_size);
+}
+
+static int mpf_spi_write_then_read(struct spi_device *spi,
+				   const void *txbuf, size_t txbuf_size,
+				   void *rxbuf, size_t rxbuf_size)
+{
+	const u8 read_command[] = { SPI_READ_DATA };
+	int ret;
+
+	ret = mpf_spi_write(spi, txbuf, txbuf_size);
+	if (ret)
+		return ret;
+
+	ret = poll_status_not_busy(spi, STATUS_READY);
+	if (ret < 0)
+		return ret;
+
+	return spi_write_then_read(spi, read_command, sizeof(read_command),
+				   rxbuf, rxbuf_size);
+}
+
+static int mpf_ops_write_init(struct fpga_manager *mgr,
+			      struct fpga_image_info *info, const char *buf,
+			      size_t count)
+{
+	const u8 isc_en_command[] = { SPI_ISC_ENABLE };
+	const u8 program_mode[] = { SPI_FRAME_INIT, SPI_PRG_MODE };
+	struct mpf_priv *priv = mgr->priv;
+	struct spi_device *spi = priv->spi;
+	struct device *dev = &mgr->dev;
+	u32 isc_ret;
+	int ret;
+
+	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
+		dev_err(dev, "Partial reconfiguration is not supported\n");
+
+		return -EOPNOTSUPP;
+	}
+
+	ret = mpf_spi_write_then_read(spi, isc_en_command, sizeof(isc_en_command),
+				      &isc_ret, sizeof(isc_ret));
+	if (ret || isc_ret) {
+		dev_err(dev, "Failed to enable ISC: %d\n", ret ? ret : isc_ret);
+
+		return -EFAULT;
+	}
+
+	ret = mpf_spi_write(spi, program_mode, sizeof(program_mode));
+	if (ret) {
+		dev_err(dev, "Failed to enter program mode: %d\n", ret);
+
+		return ret;
+	}
+
+	priv->program_mode = true;
+
+	return 0;
+}
+
+static ssize_t lookup_block_start(int id, const char *buf, size_t buf_size)
+{
+	u8 header_size, blocks_num, block_id;
+	u32 block_start, i;
+
+	header_size = *(buf + HEADER_SIZE_OFFSET);
+
+	if (header_size > buf_size)
+		return -EFAULT;
+
+	blocks_num = *(buf + header_size - 1);
+
+	if (header_size + blocks_num * LOOKUP_TABLE_RECORD_SIZE > buf_size)
+		return -EFAULT;
+
+	for (i = 0; i < blocks_num; i++) {
+		block_id = *(buf + header_size + LOOKUP_TABLE_RECORD_SIZE * i +
+			     LOOKUP_TABLE_BLOCK_ID_OFFSET);
+
+		if (block_id == id) {
+			memcpy(&block_start,
+			       buf + header_size +
+			       LOOKUP_TABLE_RECORD_SIZE * i +
+			       LOOKUP_TABLE_BLOCK_START_OFFSET,
+			       sizeof(block_start));
+
+			return le32_to_cpu(block_start);
+		}
+	}
+
+	return -EFAULT;
+}
+
+static ssize_t parse_bitstream_size(const char *buf, size_t buf_size)
+{
+	ssize_t	bitstream_size = 0, components_size_start = 0,
+		component_size_byte_num, component_size_byte_off, i;
+	u16 components_num;
+	u32 component_size;
+
+	memcpy(&components_num, buf + DATA_SIZE_OFFSET, sizeof(components_num));
+	components_num = le16_to_cpu(components_num);
+
+	components_size_start = lookup_block_start(COMPONENTS_SIZE_ID, buf,
+						   buf_size);
+	if (components_size_start < 0)
+		return components_size_start;
+
+	if (components_size_start +
+	    DIV_ROUND_UP(components_num * BITS_PER_COMPONENT_SIZE,
+			 BITS_PER_BYTE) > buf_size)
+		return -EFAULT;
+
+	for (i = 0; i < components_num; i++) {
+		component_size_byte_num =
+			(i * BITS_PER_COMPONENT_SIZE) / BITS_PER_BYTE;
+		component_size_byte_off =
+			(i * BITS_PER_COMPONENT_SIZE) % BITS_PER_BYTE;
+
+		memcpy(&component_size,
+		       buf + components_size_start + component_size_byte_num,
+		       sizeof(component_size));
+		component_size = le32_to_cpu(component_size);
+		component_size >>= component_size_byte_off;
+		component_size &= GENMASK(BITS_PER_COMPONENT_SIZE - 1, 0);
+
+		bitstream_size += component_size;
+	}
+
+	return bitstream_size;
+}
+
+static int mpf_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
+{
+	ssize_t bitstream_start = 0, bitstream_size;
+	struct mpf_priv *priv = mgr->priv;
+	struct spi_device *spi = priv->spi;
+	struct device *dev = &mgr->dev;
+	u8 tmp_buf[SPI_FRAME_SIZE + 1];
+	int ret, i;
+
+	if (crc_ccitt(0, buf, count)) {
+		dev_err(dev, "CRC error\n");
+
+		return -EINVAL;
+	}
+
+	bitstream_start = lookup_block_start(BITSTREAM_ID, buf, count);
+	if (bitstream_start < 0) {
+		dev_err(dev, "Failed to find bitstream start %zd\n",
+			bitstream_start);
+
+		return bitstream_start;
+	}
+
+	bitstream_size = parse_bitstream_size(buf, count);
+	if (bitstream_size < 0) {
+		dev_err(dev, "Failed to parse bitstream size %zd\n",
+			bitstream_size);
+
+		return bitstream_size;
+	}
+
+	if (bitstream_start + bitstream_size * SPI_FRAME_SIZE > count) {
+		dev_err(dev,
+			"Bitstram outruns firmware. Bitstream start %zd, bitstream size %zd, firmware size %zu\n",
+			bitstream_start, bitstream_size * SPI_FRAME_SIZE, count);
+
+		return -EFAULT;
+	}
+
+	for (i = 0; i < bitstream_size; i++) {
+		tmp_buf[0] = SPI_FRAME;
+		memcpy(tmp_buf + 1, buf + bitstream_start + i * SPI_FRAME_SIZE,
+		       SPI_FRAME_SIZE);
+
+		ret = mpf_spi_write(spi, tmp_buf, sizeof(tmp_buf));
+		if (ret) {
+			dev_err(dev,
+				"Failed to write bitstream frame number %d of %zd\n",
+				i, bitstream_size);
+
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int mpf_ops_write_complete(struct fpga_manager *mgr,
+					     struct fpga_image_info *info)
+{
+	const u8 isc_dis_command[] = { SPI_ISC_DISABLE };
+	const u8 release_command[] = { SPI_RELEASE };
+	struct mpf_priv *priv = mgr->priv;
+	struct spi_device *spi = priv->spi;
+	struct device *dev = &mgr->dev;
+	int ret;
+
+	ret = mpf_spi_write(spi, isc_dis_command, sizeof(isc_dis_command));
+	if (ret) {
+		dev_err(dev, "Failed to disable ISC: %d\n", ret);
+
+		return ret;
+	}
+
+	mdelay(1);
+
+	ret = mpf_spi_write(spi, release_command, sizeof(release_command));
+	if (ret) {
+		dev_err(dev, "Failed to exit program mode: %d\n", ret);
+
+		return ret;
+	}
+
+	priv->program_mode = false;
+
+	return 0;
+}
+
+static const struct fpga_manager_ops mpf_ops = {
+	.state = mpf_ops_state,
+	.write_init = mpf_ops_write_init,
+	.write = mpf_ops_write,
+	.write_complete = mpf_ops_write_complete,
+};
+
+static int mpf_probe(struct spi_device *spi)
+{
+	struct mpf_priv *priv;
+	struct device *dev = &spi->dev;
+	struct fpga_manager *mgr;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->spi = spi;
+
+	mgr = devm_fpga_mgr_register(dev, "Microchip Polarfire SPI FPGA Manager",
+				     &mpf_ops, priv);
+
+	return PTR_ERR_OR_ZERO(mgr);
+}
+
+static const struct spi_device_id mpf_spi_ids[] = {
+	{ .name = "mpf-spi-fpga-mgr", },
+	{},
+};
+MODULE_DEVICE_TABLE(spi, mpf_spi_ids);
+
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id mpf_of_ids[] = {
+	{ .compatible = "microchip,mpf-spi-fpga-mgr" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mpf_of_ids);
+#endif /* IS_ENABLED(CONFIG_OF) */
+
+static struct spi_driver mpf_driver = {
+	.probe = mpf_probe,
+	.id_table = mpf_spi_ids,
+	.driver = {
+		.name = "microchip_mpf_spi_fpga_mgr",
+		.of_match_table = of_match_ptr(mpf_of_ids),
+	},
+};
+
+module_spi_driver(mpf_driver);
+
+MODULE_DESCRIPTION("Microchip Polarfire SPI FPGA Manager");
+MODULE_LICENSE("GPL");
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v4] fpga: microchip-spi: add Microchip FPGA manager
  2022-02-17 19:18 ` [PATCH v4] " Ivan Bornyakov
@ 2022-02-18 15:22   ` Conor.Dooley
  2022-02-19  5:45     ` Ivan Bornyakov
  2022-02-18 16:05   ` Xu Yilun
  1 sibling, 1 reply; 15+ messages in thread
From: Conor.Dooley @ 2022-02-18 15:22 UTC (permalink / raw)
  To: i.bornyakov
  Cc: mdf, hao.wu, yilun.xu, trix, Conor.Dooley, linux-kernel,
	linux-fpga, system

Hey Ivan,
Finally got my hands on a board with a non SoC PolarFire today & started
trying to test. Ran into problems with my SPI setup - would be nice to
know if youre currently doing the reprogramming on one of our devkits
etc or on a custom board of your own?
Will be Monday before I can have look at it again, will have another
board I can try then in the odd chance this one isnt actually capable of
reprogramming.
Thanks,
Conor.


On 17/02/2022 19:18, Ivan Bornyakov wrote:
> Add support to the FPGA manager for programming Microchip Polarfire
> FPGAs over slave SPI interface.
> 
> Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
> ---
> Changelog:
>    v1 -> v2: fix printk formating
>    v2 -> v3:
>     * replace "microsemi" with "microchip"
>     * replace prefix "microsemi_fpga_" with "mpf_"
>     * more sensible .compatible and .name strings
>     * remove unused defines STATUS_SPI_VIOLATION and STATUS_SPI_ERROR
>    v3 -> v4: fix unused variable warning
>      Put 'mpf_of_ids' definition under conditional compilation, so it
>      would not hang unused if CONFIG_OF is not enabled.
> 
>   drivers/fpga/Kconfig         |   9 +
>   drivers/fpga/Makefile        |   1 +
>   drivers/fpga/microchip-spi.c | 361 +++++++++++++++++++++++++++++++++++
>   3 files changed, 371 insertions(+)
>   create mode 100644 drivers/fpga/microchip-spi.c
> 
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index 26025dbab353..4240c641b100 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -248,4 +248,13 @@ config FPGA_MGR_VERSAL_FPGA
>   	  configure the programmable logic(PL).
>   
>   	  To compile this as a module, choose M here.
> +
> +config FPGA_MGR_MICROCHIP_SPI
> +	tristate "Microchip Polarfire SPI FPGA manager"
> +	depends on SPI
> +	select CRC_CCITT
> +	help
> +	  FPGA manager driver support for Microchip Polarfire FPGAs
> +	  programming over slave SPI interface.
> +
>   endif # FPGA
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 4da5273948df..fcb389ca4873 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_FPGA_MGR_XILINX_SPI)	+= xilinx-spi.o
>   obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
>   obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA)	+= zynqmp-fpga.o
>   obj-$(CONFIG_FPGA_MGR_VERSAL_FPGA)      += versal-fpga.o
> +obj-$(CONFIG_FPGA_MGR_MICROCHIP_SPI)	+= microchip-spi.o
>   obj-$(CONFIG_ALTERA_PR_IP_CORE)         += altera-pr-ip-core.o
>   obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT)    += altera-pr-ip-core-plat.o
> 
> diff --git a/drivers/fpga/microchip-spi.c b/drivers/fpga/microchip-spi.c
> new file mode 100644
> index 000000000000..5db25734a27a
> --- /dev/null
> +++ b/drivers/fpga/microchip-spi.c
> @@ -0,0 +1,361 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Microchip Polarfire FPGA programming over slave SPI interface.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/of_device.h>
> +#include <linux/fpga/fpga-mgr.h>
> +#include <linux/delay.h>
> +#include <linux/crc-ccitt.h>
> +
> +#define	SPI_ISC_ENABLE		0x0B
> +#define	SPI_ISC_DISABLE		0x0C
> +#define	SPI_READ_STATUS		0x00
> +#define	SPI_READ_DATA		0x01
> +#define	SPI_FRAME_INIT		0xAE
> +#define	SPI_FRAME		0xEE
> +#define	SPI_PRG_MODE		0x01
> +#define	SPI_RELEASE		0x23
> +
> +#define	SPI_FRAME_SIZE		16
> +
> +#define	HEADER_SIZE_OFFSET	24
> +#define	DATA_SIZE_OFFSET	55
> +
> +#define	LOOKUP_TABLE_RECORD_SIZE	9
> +#define	LOOKUP_TABLE_BLOCK_ID_OFFSET	0
> +#define	LOOKUP_TABLE_BLOCK_START_OFFSET	1
> +
> +#define	COMPONENTS_SIZE_ID	5
> +#define	BITSTREAM_ID		8
> +
> +#define	BITS_PER_COMPONENT_SIZE	22
> +
> +#define	STATUS_POLL_TIMEOUT_MS	1000
> +#define	STATUS_BUSY		BIT(0)
> +#define	STATUS_READY		BIT(1)
> +
> +struct mpf_priv {
> +	struct spi_device *spi;
> +	bool program_mode;
> +};
> +
> +static enum fpga_mgr_states mpf_ops_state(struct fpga_manager *mgr)
> +{
> +	struct mpf_priv *priv = mgr->priv;
> +	struct spi_device *spi = priv->spi;
> +	bool program_mode = priv->program_mode;
> +	ssize_t status;
> +
> +	status = spi_w8r8(spi, SPI_READ_STATUS);
> +
> +	if (!program_mode && !status)
> +		return FPGA_MGR_STATE_OPERATING;
> +
> +	return FPGA_MGR_STATE_UNKNOWN;
> +}
> +
> +static int poll_status_not_busy(struct spi_device *spi, u8 mask)
> +{
> +	ssize_t status, timeout = STATUS_POLL_TIMEOUT_MS;
> +
> +	while (timeout--) {
> +		status = spi_w8r8(spi, SPI_READ_STATUS);
> +		if (status < 0)
> +			return status;
> +
> +		if (mask) {
> +			if (!(status & STATUS_BUSY) && (status & mask))
> +				return status;
> +		} else {
> +			if (!(status & STATUS_BUSY))
> +				return status;
> +		}
> +
> +		mdelay(1);
> +	}
> +
> +	return -EBUSY;
> +}
> +
> +static int mpf_spi_write(struct spi_device *spi, const void *buf, size_t buf_size)
> +{
> +	int status = poll_status_not_busy(spi, 0);
> +
> +	if (status < 0)
> +		return status;
> +
> +	return spi_write(spi, buf, buf_size);
> +}
> +
> +static int mpf_spi_write_then_read(struct spi_device *spi,
> +				   const void *txbuf, size_t txbuf_size,
> +				   void *rxbuf, size_t rxbuf_size)
> +{
> +	const u8 read_command[] = { SPI_READ_DATA };
> +	int ret;
> +
> +	ret = mpf_spi_write(spi, txbuf, txbuf_size);
> +	if (ret)
> +		return ret;
> +
> +	ret = poll_status_not_busy(spi, STATUS_READY);
> +	if (ret < 0)
> +		return ret;
> +
> +	return spi_write_then_read(spi, read_command, sizeof(read_command),
> +				   rxbuf, rxbuf_size);
> +}
> +
> +static int mpf_ops_write_init(struct fpga_manager *mgr,
> +			      struct fpga_image_info *info, const char *buf,
> +			      size_t count)
> +{
> +	const u8 isc_en_command[] = { SPI_ISC_ENABLE };
> +	const u8 program_mode[] = { SPI_FRAME_INIT, SPI_PRG_MODE };
> +	struct mpf_priv *priv = mgr->priv;
> +	struct spi_device *spi = priv->spi;
> +	struct device *dev = &mgr->dev;
> +	u32 isc_ret;
> +	int ret;
> +
> +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> +		dev_err(dev, "Partial reconfiguration is not supported\n");
> +
> +		return -EOPNOTSUPP;
> +	}
> +
> +	ret = mpf_spi_write_then_read(spi, isc_en_command, sizeof(isc_en_command),
> +				      &isc_ret, sizeof(isc_ret));
> +	if (ret || isc_ret) {
> +		dev_err(dev, "Failed to enable ISC: %d\n", ret ? ret : isc_ret);
> +
> +		return -EFAULT;
> +	}
> +
> +	ret = mpf_spi_write(spi, program_mode, sizeof(program_mode));
> +	if (ret) {
> +		dev_err(dev, "Failed to enter program mode: %d\n", ret);
> +
> +		return ret;
> +	}
> +
> +	priv->program_mode = true;
> +
> +	return 0;
> +}
> +
> +static ssize_t lookup_block_start(int id, const char *buf, size_t buf_size)
> +{
> +	u8 header_size, blocks_num, block_id;
> +	u32 block_start, i;
> +
> +	header_size = *(buf + HEADER_SIZE_OFFSET);
> +
> +	if (header_size > buf_size)
> +		return -EFAULT;
> +
> +	blocks_num = *(buf + header_size - 1);
> +
> +	if (header_size + blocks_num * LOOKUP_TABLE_RECORD_SIZE > buf_size)
> +		return -EFAULT;
> +
> +	for (i = 0; i < blocks_num; i++) {
> +		block_id = *(buf + header_size + LOOKUP_TABLE_RECORD_SIZE * i +
> +			     LOOKUP_TABLE_BLOCK_ID_OFFSET);
> +
> +		if (block_id == id) {
> +			memcpy(&block_start,
> +			       buf + header_size +
> +			       LOOKUP_TABLE_RECORD_SIZE * i +
> +			       LOOKUP_TABLE_BLOCK_START_OFFSET,
> +			       sizeof(block_start));
> +
> +			return le32_to_cpu(block_start);
> +		}
> +	}
> +
> +	return -EFAULT;
> +}
> +
> +static ssize_t parse_bitstream_size(const char *buf, size_t buf_size)
> +{
> +	ssize_t	bitstream_size = 0, components_size_start = 0,
> +		component_size_byte_num, component_size_byte_off, i;
> +	u16 components_num;
> +	u32 component_size;
> +
> +	memcpy(&components_num, buf + DATA_SIZE_OFFSET, sizeof(components_num));
> +	components_num = le16_to_cpu(components_num);
> +
> +	components_size_start = lookup_block_start(COMPONENTS_SIZE_ID, buf,
> +						   buf_size);
> +	if (components_size_start < 0)
> +		return components_size_start;
> +
> +	if (components_size_start +
> +	    DIV_ROUND_UP(components_num * BITS_PER_COMPONENT_SIZE,
> +			 BITS_PER_BYTE) > buf_size)
> +		return -EFAULT;
> +
> +	for (i = 0; i < components_num; i++) {
> +		component_size_byte_num =
> +			(i * BITS_PER_COMPONENT_SIZE) / BITS_PER_BYTE;
> +		component_size_byte_off =
> +			(i * BITS_PER_COMPONENT_SIZE) % BITS_PER_BYTE;
> +
> +		memcpy(&component_size,
> +		       buf + components_size_start + component_size_byte_num,
> +		       sizeof(component_size));
> +		component_size = le32_to_cpu(component_size);
> +		component_size >>= component_size_byte_off;
> +		component_size &= GENMASK(BITS_PER_COMPONENT_SIZE - 1, 0);
> +
> +		bitstream_size += component_size;
> +	}
> +
> +	return bitstream_size;
> +}
> +
> +static int mpf_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
> +{
> +	ssize_t bitstream_start = 0, bitstream_size;
> +	struct mpf_priv *priv = mgr->priv;
> +	struct spi_device *spi = priv->spi;
> +	struct device *dev = &mgr->dev;
> +	u8 tmp_buf[SPI_FRAME_SIZE + 1];
> +	int ret, i;
> +
> +	if (crc_ccitt(0, buf, count)) {
> +		dev_err(dev, "CRC error\n");
> +
> +		return -EINVAL;
> +	}
> +
> +	bitstream_start = lookup_block_start(BITSTREAM_ID, buf, count);
> +	if (bitstream_start < 0) {
> +		dev_err(dev, "Failed to find bitstream start %zd\n",
> +			bitstream_start);
> +
> +		return bitstream_start;
> +	}
> +
> +	bitstream_size = parse_bitstream_size(buf, count);
> +	if (bitstream_size < 0) {
> +		dev_err(dev, "Failed to parse bitstream size %zd\n",
> +			bitstream_size);
> +
> +		return bitstream_size;
> +	}
> +
> +	if (bitstream_start + bitstream_size * SPI_FRAME_SIZE > count) {
> +		dev_err(dev,
> +			"Bitstram outruns firmware. Bitstream start %zd, bitstream size %zd, firmware size %zu\n",
> +			bitstream_start, bitstream_size * SPI_FRAME_SIZE, count);
> +
> +		return -EFAULT;
> +	}
> +
> +	for (i = 0; i < bitstream_size; i++) {
> +		tmp_buf[0] = SPI_FRAME;
> +		memcpy(tmp_buf + 1, buf + bitstream_start + i * SPI_FRAME_SIZE,
> +		       SPI_FRAME_SIZE);
> +
> +		ret = mpf_spi_write(spi, tmp_buf, sizeof(tmp_buf));
> +		if (ret) {
> +			dev_err(dev,
> +				"Failed to write bitstream frame number %d of %zd\n",
> +				i, bitstream_size);
> +
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int mpf_ops_write_complete(struct fpga_manager *mgr,
> +					     struct fpga_image_info *info)
> +{
> +	const u8 isc_dis_command[] = { SPI_ISC_DISABLE };
> +	const u8 release_command[] = { SPI_RELEASE };
> +	struct mpf_priv *priv = mgr->priv;
> +	struct spi_device *spi = priv->spi;
> +	struct device *dev = &mgr->dev;
> +	int ret;
> +
> +	ret = mpf_spi_write(spi, isc_dis_command, sizeof(isc_dis_command));
> +	if (ret) {
> +		dev_err(dev, "Failed to disable ISC: %d\n", ret);
> +
> +		return ret;
> +	}
> +
> +	mdelay(1);
> +
> +	ret = mpf_spi_write(spi, release_command, sizeof(release_command));
> +	if (ret) {
> +		dev_err(dev, "Failed to exit program mode: %d\n", ret);
> +
> +		return ret;
> +	}
> +
> +	priv->program_mode = false;
> +
> +	return 0;
> +}
> +
> +static const struct fpga_manager_ops mpf_ops = {
> +	.state = mpf_ops_state,
> +	.write_init = mpf_ops_write_init,
> +	.write = mpf_ops_write,
> +	.write_complete = mpf_ops_write_complete,
> +};
> +
> +static int mpf_probe(struct spi_device *spi)
> +{
> +	struct mpf_priv *priv;
> +	struct device *dev = &spi->dev;
> +	struct fpga_manager *mgr;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->spi = spi;
> +
> +	mgr = devm_fpga_mgr_register(dev, "Microchip Polarfire SPI FPGA Manager",
> +				     &mpf_ops, priv);
> +
> +	return PTR_ERR_OR_ZERO(mgr);
> +}
> +
> +static const struct spi_device_id mpf_spi_ids[] = {
> +	{ .name = "mpf-spi-fpga-mgr", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(spi, mpf_spi_ids);
> +
> +#if IS_ENABLED(CONFIG_OF)
> +static const struct of_device_id mpf_of_ids[] = {
> +	{ .compatible = "microchip,mpf-spi-fpga-mgr" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, mpf_of_ids);
> +#endif /* IS_ENABLED(CONFIG_OF) */
> +
> +static struct spi_driver mpf_driver = {
> +	.probe = mpf_probe,
> +	.id_table = mpf_spi_ids,
> +	.driver = {
> +		.name = "microchip_mpf_spi_fpga_mgr",
> +		.of_match_table = of_match_ptr(mpf_of_ids),
> +	},
> +};
> +
> +module_spi_driver(mpf_driver);
> +
> +MODULE_DESCRIPTION("Microchip Polarfire SPI FPGA Manager");
> +MODULE_LICENSE("GPL");

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4] fpga: microchip-spi: add Microchip FPGA manager
  2022-02-17 19:18 ` [PATCH v4] " Ivan Bornyakov
  2022-02-18 15:22   ` Conor.Dooley
@ 2022-02-18 16:05   ` Xu Yilun
  2022-02-19  6:16     ` Ivan Bornyakov
  1 sibling, 1 reply; 15+ messages in thread
From: Xu Yilun @ 2022-02-18 16:05 UTC (permalink / raw)
  To: Ivan Bornyakov
  Cc: mdf, hao.wu, trix, conor.dooley, linux-kernel, linux-fpga, system

On Thu, Feb 17, 2022 at 10:18:51PM +0300, Ivan Bornyakov wrote:
> Add support to the FPGA manager for programming Microchip Polarfire
> FPGAs over slave SPI interface.
> 
> Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
> ---
> Changelog:
>   v1 -> v2: fix printk formating
>   v2 -> v3:
>    * replace "microsemi" with "microchip"
>    * replace prefix "microsemi_fpga_" with "mpf_"
>    * more sensible .compatible and .name strings
>    * remove unused defines STATUS_SPI_VIOLATION and STATUS_SPI_ERROR
>   v3 -> v4: fix unused variable warning
>     Put 'mpf_of_ids' definition under conditional compilation, so it
>     would not hang unused if CONFIG_OF is not enabled.
> 
>  drivers/fpga/Kconfig         |   9 +
>  drivers/fpga/Makefile        |   1 +
>  drivers/fpga/microchip-spi.c | 361 +++++++++++++++++++++++++++++++++++
>  3 files changed, 371 insertions(+)
>  create mode 100644 drivers/fpga/microchip-spi.c
> 
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index 26025dbab353..4240c641b100 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -248,4 +248,13 @@ config FPGA_MGR_VERSAL_FPGA
>  	  configure the programmable logic(PL).
>  
>  	  To compile this as a module, choose M here.
> +
> +config FPGA_MGR_MICROCHIP_SPI
> +	tristate "Microchip Polarfire SPI FPGA manager"
> +	depends on SPI
> +	select CRC_CCITT
> +	help
> +	  FPGA manager driver support for Microchip Polarfire FPGAs
> +	  programming over slave SPI interface.
> +
>  endif # FPGA
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 4da5273948df..fcb389ca4873 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_FPGA_MGR_XILINX_SPI)	+= xilinx-spi.o
>  obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
>  obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA)	+= zynqmp-fpga.o
>  obj-$(CONFIG_FPGA_MGR_VERSAL_FPGA)      += versal-fpga.o
> +obj-$(CONFIG_FPGA_MGR_MICROCHIP_SPI)	+= microchip-spi.o
>  obj-$(CONFIG_ALTERA_PR_IP_CORE)         += altera-pr-ip-core.o
>  obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT)    += altera-pr-ip-core-plat.o
> 
> diff --git a/drivers/fpga/microchip-spi.c b/drivers/fpga/microchip-spi.c
> new file mode 100644
> index 000000000000..5db25734a27a
> --- /dev/null
> +++ b/drivers/fpga/microchip-spi.c
> @@ -0,0 +1,361 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Microchip Polarfire FPGA programming over slave SPI interface.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/of_device.h>
> +#include <linux/fpga/fpga-mgr.h>
> +#include <linux/delay.h>
> +#include <linux/crc-ccitt.h>

Please list them in alphabetical order.

> +
> +#define	SPI_ISC_ENABLE		0x0B
> +#define	SPI_ISC_DISABLE		0x0C
> +#define	SPI_READ_STATUS		0x00
> +#define	SPI_READ_DATA		0x01
> +#define	SPI_FRAME_INIT		0xAE
> +#define	SPI_FRAME		0xEE
> +#define	SPI_PRG_MODE		0x01
> +#define	SPI_RELEASE		0x23
> +
> +#define	SPI_FRAME_SIZE		16
> +
> +#define	HEADER_SIZE_OFFSET	24
> +#define	DATA_SIZE_OFFSET	55
> +
> +#define	LOOKUP_TABLE_RECORD_SIZE	9
> +#define	LOOKUP_TABLE_BLOCK_ID_OFFSET	0
> +#define	LOOKUP_TABLE_BLOCK_START_OFFSET	1
> +
> +#define	COMPONENTS_SIZE_ID	5
> +#define	BITSTREAM_ID		8
> +
> +#define	BITS_PER_COMPONENT_SIZE	22
> +
> +#define	STATUS_POLL_TIMEOUT_MS	1000
> +#define	STATUS_BUSY		BIT(0)
> +#define	STATUS_READY		BIT(1)

Maybe add some prefix for these vendor specific definitions, like
MFP_SPI_ISC_ENABLE.

> +
> +struct mpf_priv {
> +	struct spi_device *spi;
> +	bool program_mode;
> +};
> +
> +static enum fpga_mgr_states mpf_ops_state(struct fpga_manager *mgr)
> +{
> +	struct mpf_priv *priv = mgr->priv;
> +	struct spi_device *spi = priv->spi;
> +	bool program_mode = priv->program_mode;
> +	ssize_t status;
> +
> +	status = spi_w8r8(spi, SPI_READ_STATUS);
> +
> +	if (!program_mode && !status)
> +		return FPGA_MGR_STATE_OPERATING;
> +
> +	return FPGA_MGR_STATE_UNKNOWN;
> +}
> +
> +static int poll_status_not_busy(struct spi_device *spi, u8 mask)
> +{
> +	ssize_t status, timeout = STATUS_POLL_TIMEOUT_MS;
> +
> +	while (timeout--) {
> +		status = spi_w8r8(spi, SPI_READ_STATUS);
> +		if (status < 0)
> +			return status;
> +
> +		if (mask) {
> +			if (!(status & STATUS_BUSY) && (status & mask))
> +				return status;
> +		} else {
> +			if (!(status & STATUS_BUSY))
> +				return status;
> +		}

!(status & STATUS_BUSY) is always checked regardless of mask, so may
move the check out of the if...else statement.

> +
> +		mdelay(1);

busy wait for 1ms is discouraged, maybe usleep_range(). See
Documentation/timers/timers-howto.rst

> +	}

The actual timeout may be much longer than what is defined. To be more
accurate, you may use:

	time_after(jiffies, timeout_jiffies)

> +
> +	return -EBUSY;
> +}
> +
> +static int mpf_spi_write(struct spi_device *spi, const void *buf, size_t buf_size)
> +{
> +	int status = poll_status_not_busy(spi, 0);
> +
> +	if (status < 0)
> +		return status;
> +
> +	return spi_write(spi, buf, buf_size);
> +}
> +
> +static int mpf_spi_write_then_read(struct spi_device *spi,
> +				   const void *txbuf, size_t txbuf_size,
> +				   void *rxbuf, size_t rxbuf_size)
> +{
> +	const u8 read_command[] = { SPI_READ_DATA };
> +	int ret;
> +
> +	ret = mpf_spi_write(spi, txbuf, txbuf_size);
> +	if (ret)
> +		return ret;
> +
> +	ret = poll_status_not_busy(spi, STATUS_READY);
> +	if (ret < 0)
> +		return ret;
> +
> +	return spi_write_then_read(spi, read_command, sizeof(read_command),
> +				   rxbuf, rxbuf_size);
> +}
> +
> +static int mpf_ops_write_init(struct fpga_manager *mgr,
> +			      struct fpga_image_info *info, const char *buf,
> +			      size_t count)
> +{
> +	const u8 isc_en_command[] = { SPI_ISC_ENABLE };
> +	const u8 program_mode[] = { SPI_FRAME_INIT, SPI_PRG_MODE };

Better we follow the reverse xmas tree declaration, so reverse the 2
lines please.

> +	struct mpf_priv *priv = mgr->priv;
> +	struct spi_device *spi = priv->spi;
> +	struct device *dev = &mgr->dev;
> +	u32 isc_ret;
> +	int ret;
> +
> +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> +		dev_err(dev, "Partial reconfiguration is not supported\n");
> +

remove the blank line please

> +		return -EOPNOTSUPP;
> +	}
> +
> +	ret = mpf_spi_write_then_read(spi, isc_en_command, sizeof(isc_en_command),
> +				      &isc_ret, sizeof(isc_ret));
> +	if (ret || isc_ret) {
> +		dev_err(dev, "Failed to enable ISC: %d\n", ret ? ret : isc_ret);

							   ret ? : isc_ret

> +

remove the blank line please

> +		return -EFAULT;
> +	}
> +
> +	ret = mpf_spi_write(spi, program_mode, sizeof(program_mode));
> +	if (ret) {
> +		dev_err(dev, "Failed to enter program mode: %d\n", ret);
> +

the same

> +		return ret;
> +	}
> +
> +	priv->program_mode = true;
> +
> +	return 0;
> +}
> +
> +static ssize_t lookup_block_start(int id, const char *buf, size_t buf_size)
> +{
> +	u8 header_size, blocks_num, block_id;
> +	u32 block_start, i;
> +
> +	header_size = *(buf + HEADER_SIZE_OFFSET);
> +
> +	if (header_size > buf_size)
> +		return -EFAULT;
> +
> +	blocks_num = *(buf + header_size - 1);
> +
> +	if (header_size + blocks_num * LOOKUP_TABLE_RECORD_SIZE > buf_size)
> +		return -EFAULT;
> +
> +	for (i = 0; i < blocks_num; i++) {
> +		block_id = *(buf + header_size + LOOKUP_TABLE_RECORD_SIZE * i +
> +			     LOOKUP_TABLE_BLOCK_ID_OFFSET);
> +
> +		if (block_id == id) {
> +			memcpy(&block_start,
> +			       buf + header_size +
> +			       LOOKUP_TABLE_RECORD_SIZE * i +
> +			       LOOKUP_TABLE_BLOCK_START_OFFSET,
> +			       sizeof(block_start));

why a memcpy here, could we just read from the offset?

> +
> +			return le32_to_cpu(block_start);
> +		}
> +	}
> +
> +	return -EFAULT;
> +}
> +
> +static ssize_t parse_bitstream_size(const char *buf, size_t buf_size)
> +{
> +	ssize_t	bitstream_size = 0, components_size_start = 0,
> +		component_size_byte_num, component_size_byte_off, i;
> +	u16 components_num;
> +	u32 component_size;
> +
> +	memcpy(&components_num, buf + DATA_SIZE_OFFSET, sizeof(components_num));

the same, why a memcpy?

> +	components_num = le16_to_cpu(components_num);
> +
> +	components_size_start = lookup_block_start(COMPONENTS_SIZE_ID, buf,
> +						   buf_size);
> +	if (components_size_start < 0)
> +		return components_size_start;
> +
> +	if (components_size_start +
> +	    DIV_ROUND_UP(components_num * BITS_PER_COMPONENT_SIZE,
> +			 BITS_PER_BYTE) > buf_size)
> +		return -EFAULT;
> +
> +	for (i = 0; i < components_num; i++) {
> +		component_size_byte_num =
> +			(i * BITS_PER_COMPONENT_SIZE) / BITS_PER_BYTE;
> +		component_size_byte_off =
> +			(i * BITS_PER_COMPONENT_SIZE) % BITS_PER_BYTE;
> +
> +		memcpy(&component_size,
> +		       buf + components_size_start + component_size_byte_num,
> +		       sizeof(component_size));
> +		component_size = le32_to_cpu(component_size);
> +		component_size >>= component_size_byte_off;
> +		component_size &= GENMASK(BITS_PER_COMPONENT_SIZE - 1, 0);
> +
> +		bitstream_size += component_size;
> +	}
> +
> +	return bitstream_size;
> +}
> +
> +static int mpf_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
> +{
> +	ssize_t bitstream_start = 0, bitstream_size;
> +	struct mpf_priv *priv = mgr->priv;
> +	struct spi_device *spi = priv->spi;
> +	struct device *dev = &mgr->dev;
> +	u8 tmp_buf[SPI_FRAME_SIZE + 1];
> +	int ret, i;
> +
> +	if (crc_ccitt(0, buf, count)) {
> +		dev_err(dev, "CRC error\n");
> +
> +		return -EINVAL;
> +	}
> +
> +	bitstream_start = lookup_block_start(BITSTREAM_ID, buf, count);
> +	if (bitstream_start < 0) {
> +		dev_err(dev, "Failed to find bitstream start %zd\n",
> +			bitstream_start);
> +
> +		return bitstream_start;
> +	}
> +
> +	bitstream_size = parse_bitstream_size(buf, count);
> +	if (bitstream_size < 0) {
> +		dev_err(dev, "Failed to parse bitstream size %zd\n",
> +			bitstream_size);
> +
> +		return bitstream_size;
> +	}
> +
> +	if (bitstream_start + bitstream_size * SPI_FRAME_SIZE > count) {
> +		dev_err(dev,
> +			"Bitstram outruns firmware. Bitstream start %zd, bitstream size %zd, firmware size %zu\n",

			 Bitstream

> +			bitstream_start, bitstream_size * SPI_FRAME_SIZE, count);
> +
> +		return -EFAULT;
> +	}
> +

If I understand right, this function assumes the users provide the
entire image buffer. But it is possible the image buffer is from a
scatter list and the callback would be called several times.

Maybe the bitstream info at the head of the image could be parsed in
write_init(), and this requires the driver fill the
fpga_manager_ops.initial_header_size

> +	for (i = 0; i < bitstream_size; i++) {
> +		tmp_buf[0] = SPI_FRAME;
> +		memcpy(tmp_buf + 1, buf + bitstream_start + i * SPI_FRAME_SIZE,
> +		       SPI_FRAME_SIZE);
> +
> +		ret = mpf_spi_write(spi, tmp_buf, sizeof(tmp_buf));

Is it possible we use spi_sync_transfer to avoid memcpy the whole
bitstream?

> +		if (ret) {
> +			dev_err(dev,
> +				"Failed to write bitstream frame number %d of %zd\n",
> +				i, bitstream_size);
> +
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int mpf_ops_write_complete(struct fpga_manager *mgr,
> +					     struct fpga_image_info *info)

"CHECK: Alignment should match open parenthesis" from checkpatch.pl --strict

> +{
> +	const u8 isc_dis_command[] = { SPI_ISC_DISABLE };
> +	const u8 release_command[] = { SPI_RELEASE };
> +	struct mpf_priv *priv = mgr->priv;
> +	struct spi_device *spi = priv->spi;
> +	struct device *dev = &mgr->dev;
> +	int ret;
> +
> +	ret = mpf_spi_write(spi, isc_dis_command, sizeof(isc_dis_command));
> +	if (ret) {
> +		dev_err(dev, "Failed to disable ISC: %d\n", ret);
> +
> +		return ret;
> +	}
> +
> +	mdelay(1);

Same concern

> +
> +	ret = mpf_spi_write(spi, release_command, sizeof(release_command));
> +	if (ret) {
> +		dev_err(dev, "Failed to exit program mode: %d\n", ret);
> +
> +		return ret;
> +	}
> +
> +	priv->program_mode = false;
> +
> +	return 0;
> +}
> +
> +static const struct fpga_manager_ops mpf_ops = {
> +	.state = mpf_ops_state,
> +	.write_init = mpf_ops_write_init,
> +	.write = mpf_ops_write,
> +	.write_complete = mpf_ops_write_complete,
> +};
> +
> +static int mpf_probe(struct spi_device *spi)
> +{
> +	struct mpf_priv *priv;
> +	struct device *dev = &spi->dev;
> +	struct fpga_manager *mgr;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->spi = spi;
> +
> +	mgr = devm_fpga_mgr_register(dev, "Microchip Polarfire SPI FPGA Manager",
> +				     &mpf_ops, priv);
> +
> +	return PTR_ERR_OR_ZERO(mgr);
> +}
> +
> +static const struct spi_device_id mpf_spi_ids[] = {
> +	{ .name = "mpf-spi-fpga-mgr", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(spi, mpf_spi_ids);
> +
> +#if IS_ENABLED(CONFIG_OF)
> +static const struct of_device_id mpf_of_ids[] = {
> +	{ .compatible = "microchip,mpf-spi-fpga-mgr" },

From checkpatch.pl

"WARNING: DT compatible string "microchip,mpf-spi-fpga-mgr" appears un-documented -- check ./Documentation/devicetree/bindings/"

Thanks,
Yilun

> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, mpf_of_ids);
> +#endif /* IS_ENABLED(CONFIG_OF) */
> +
> +static struct spi_driver mpf_driver = {
> +	.probe = mpf_probe,
> +	.id_table = mpf_spi_ids,
> +	.driver = {
> +		.name = "microchip_mpf_spi_fpga_mgr",
> +		.of_match_table = of_match_ptr(mpf_of_ids),
> +	},
> +};
> +
> +module_spi_driver(mpf_driver);
> +
> +MODULE_DESCRIPTION("Microchip Polarfire SPI FPGA Manager");
> +MODULE_LICENSE("GPL");
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4] fpga: microchip-spi: add Microchip FPGA manager
  2022-02-18 15:22   ` Conor.Dooley
@ 2022-02-19  5:45     ` Ivan Bornyakov
  0 siblings, 0 replies; 15+ messages in thread
From: Ivan Bornyakov @ 2022-02-19  5:45 UTC (permalink / raw)
  To: Conor.Dooley
  Cc: mdf, hao.wu, yilun.xu, trix, linux-kernel, linux-fpga, system

Hi, Conor

On Fri, Feb 18, 2022 at 03:22:57PM +0000, Conor.Dooley@microchip.com wrote:
> Hey Ivan,
> Finally got my hands on a board with a non SoC PolarFire today & started
> trying to test. Ran into problems with my SPI setup - would be nice to
> know if youre currently doing the reprogramming on one of our devkits
> etc or on a custom board of your own?
> Will be Monday before I can have look at it again, will have another
> board I can try then in the odd chance this one isnt actually capable of
> reprogramming.
> Thanks,
> Conor.
> 

I'm working with a custom board.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4] fpga: microchip-spi: add Microchip FPGA manager
  2022-02-18 16:05   ` Xu Yilun
@ 2022-02-19  6:16     ` Ivan Bornyakov
  2022-02-19 10:42       ` Ivan Bornyakov
  0 siblings, 1 reply; 15+ messages in thread
From: Ivan Bornyakov @ 2022-02-19  6:16 UTC (permalink / raw)
  To: Xu Yilun
  Cc: mdf, hao.wu, trix, conor.dooley, linux-kernel, linux-fpga, system

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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4] fpga: microchip-spi: add Microchip FPGA manager
  2022-02-19  6:16     ` Ivan Bornyakov
@ 2022-02-19 10:42       ` Ivan Bornyakov
  0 siblings, 0 replies; 15+ messages in thread
From: Ivan Bornyakov @ 2022-02-19 10:42 UTC (permalink / raw)
  To: Xu Yilun
  Cc: mdf, hao.wu, trix, conor.dooley, linux-kernel, linux-fpga, system

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.


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-02-19 11:03 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-14 13:38 [PATCH] fpga: microsemi-spi: add Microsemi FPGA manager Ivan Bornyakov
2022-02-14 19:22 ` kernel test robot
2022-02-14 20:34 ` kernel test robot
2022-02-15 11:58 ` [PATCH v2] " Ivan Bornyakov
2022-02-15 17:37   ` Conor Dooley
2022-02-15 17:52     ` Moritz Fischer
2022-02-15 18:05       ` Ivan Bornyakov
2022-02-16 12:55   ` Conor.Dooley
2022-02-16 15:56 ` [PATCH v3] fpga: microchip-spi: add Microchip " Ivan Bornyakov
2022-02-17 19:18 ` [PATCH v4] " Ivan Bornyakov
2022-02-18 15:22   ` Conor.Dooley
2022-02-19  5:45     ` Ivan Bornyakov
2022-02-18 16:05   ` Xu Yilun
2022-02-19  6:16     ` Ivan Bornyakov
2022-02-19 10:42       ` Ivan Bornyakov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).