[PATCH v9 0/3] Microchip Polarfire FPGA manager

[PATCH v9 0/3] Microchip Polarfire FPGA manager

[Ivan Bornyakov]

Add support to the FPGA manager for programming Microchip Polarfire
FPGAs over slave SPI interface with .dat formatted bitsream image.

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.
  v4 -> v5:
   * prefix defines with MPF_
   * mdelay() -> usleep_range()
   * formatting fixes
   * add DT bindings doc
   * rework fpga_manager_ops.write() to fpga_manager_ops.write_sg()
     We can't parse image header in write_init() because image header
     size is not known beforehand. Thus parsing need to be done in
     fpga_manager_ops.write() callback, but fpga_manager_ops.write()
     also need to be reenterable. On the other hand,
     fpga_manager_ops.write_sg() is called once. Thus, rework usage of
     write() callback to write_sg().
  v5 -> v6: fix patch applying
     I forgot to clean up unrelated local changes which lead to error on
     patch 0001-fpga-microchip-spi-add-Microchip-MPF-FPGA-manager.patch
     applying on vanilla kernel.
  v6 -> v7: fix binding doc to pass dt_binding_check
  v7 -> v8: another fix for dt_binding_check warning
  v8 -> v9:
   * add another patch to support bitstream offset in FPGA image buffer
   * rework fpga_manager_ops.write_sg() back to fpga_manager_ops.write()
   * move image header parsing from write() to write_init()

Ivan Bornyakov (3):
  fpga: fpga-mgr: support bitstream offset in image buffer
  fpga: microchip-spi: add Microchip MPF FPGA manager
  dt-bindings: fpga: add binding doc for microchip-spi fpga mgr

 .../fpga/microchip,mpf-spi-fpga-mgr.yaml      |  44 ++
 drivers/fpga/Kconfig                          |  10 +
 drivers/fpga/Makefile                         |   1 +
 drivers/fpga/fpga-mgr.c                       |  48 ++-
 drivers/fpga/microchip-spi.c                  | 379 ++++++++++++++++++
 include/linux/fpga/fpga-mgr.h                 |   5 +
 6 files changed, 479 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml
 create mode 100644 drivers/fpga/microchip-spi.c

-- 
2.25.1

[PATCH v9 1/3] fpga: fpga-mgr: support bitstream offset in image buffer

[Ivan Bornyakov]

It is not always whole FPGA image buffer meant to be written to the
device.

Add bitstream_start and bitstream_size to the fpga_image_info struct and
adjust fpga_mgr_write() callers with respect to them.

If initial_header_size is not known beforehand, pass whole buffer to low
level driver's write_init() so it could setup info->bitstream_start and
info->bitstream_size regardless.

Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
---
 drivers/fpga/fpga-mgr.c       | 48 +++++++++++++++++++++++++++++------
 include/linux/fpga/fpga-mgr.h |  5 ++++
 2 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index d49a9ce34568..c64e60e23a71 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -139,7 +139,8 @@ EXPORT_SYMBOL_GPL(fpga_image_info_free);
  * Call the low level driver's write_init function.  This will do the
  * device-specific things to get the FPGA into the state where it is ready to
  * receive an FPGA image. The low level driver only gets to see the first
- * initial_header_size bytes in the buffer.
+ * initial_header_size bytes in the buffer, if initial_header_size is set.
+ * Otherwise, the whole buffer will be passed.
  */
 static int fpga_mgr_write_init_buf(struct fpga_manager *mgr,
 				   struct fpga_image_info *info,
@@ -148,12 +149,10 @@ static int fpga_mgr_write_init_buf(struct fpga_manager *mgr,
 	int ret;
 
 	mgr->state = FPGA_MGR_STATE_WRITE_INIT;
-	if (!mgr->mops->initial_header_size)
-		ret = fpga_mgr_write_init(mgr, info, NULL, 0);
-	else
-		ret = fpga_mgr_write_init(
-		    mgr, info, buf, min(mgr->mops->initial_header_size, count));
+	if (mgr->mops->initial_header_size)
+		count = min(mgr->mops->initial_header_size, count);
 
+	ret = fpga_mgr_write_init(mgr, info, buf, count);
 	if (ret) {
 		dev_err(&mgr->dev, "Error preparing FPGA for writing\n");
 		mgr->state = FPGA_MGR_STATE_WRITE_INIT_ERR;
@@ -235,13 +234,33 @@ static int fpga_mgr_buf_load_sg(struct fpga_manager *mgr,
 	if (mgr->mops->write_sg) {
 		ret = fpga_mgr_write_sg(mgr, sgt);
 	} else {
+		size_t offset, count, length, bitstream_size;
 		struct sg_mapping_iter miter;
 
+		offset = info->bitstream_start;
+		bitstream_size = info->bitstream_size;
+		count = 0;
+
 		sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
 		while (sg_miter_next(&miter)) {
-			ret = fpga_mgr_write(mgr, miter.addr, miter.length);
-			if (ret)
+			if (offset >= miter.length) {
+				offset -= miter.length;
+				continue;
+			}
+
+			if (bitstream_size)
+				length = min(miter.length - offset,
+					     bitstream_size - count);
+			else
+				length = miter.length - offset;
+
+			count += length;
+
+			ret = fpga_mgr_write(mgr, miter.addr + offset, length);
+			if (ret || count == bitstream_size)
 				break;
+
+			offset = 0;
 		}
 		sg_miter_stop(&miter);
 	}
@@ -265,6 +284,19 @@ static int fpga_mgr_buf_load_mapped(struct fpga_manager *mgr,
 	if (ret)
 		return ret;
 
+	if (info->bitstream_start > count) {
+		dev_err(&mgr->dev,
+			"Bitstream start %zd outruns firmware image %zd\n",
+			info->bitstream_start, count);
+		return -EINVAL;
+	}
+
+	if (info->bitstream_size)
+		count = min(info->bitstream_start + info->bitstream_size, count);
+
+	buf += info->bitstream_start;
+	count -= info->bitstream_start;
+
 	/*
 	 * Write the FPGA image to the FPGA.
 	 */
diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
index 0f9468771bb9..32464fd10cca 100644
--- a/include/linux/fpga/fpga-mgr.h
+++ b/include/linux/fpga/fpga-mgr.h
@@ -85,6 +85,9 @@ enum fpga_mgr_states {
  * @sgt: scatter/gather table containing FPGA image
  * @buf: contiguous buffer containing FPGA image
  * @count: size of buf
+ * @bitstream_start: offset in image buffer where bitstream data starts
+ * @bitstream_size: size of bitstream.
+ *	If 0, (count - bitstream_start) will be used.
  * @region_id: id of target region
  * @dev: device that owns this
  * @overlay: Device Tree overlay
@@ -98,6 +101,8 @@ struct fpga_image_info {
 	struct sg_table *sgt;
 	const char *buf;
 	size_t count;
+	size_t bitstream_start;
+	size_t bitstream_size;
 	int region_id;
 	struct device *dev;
 #ifdef CONFIG_OF
-- 
2.25.1

[PATCH v9 2/3] fpga: microchip-spi: add Microchip MPF FPGA manager

[Ivan Bornyakov]

Add support to the FPGA manager for programming Microchip Polarfire
FPGAs over slave SPI interface with .dat formatted bitsream image.

Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
---
 drivers/fpga/Kconfig         |  10 +
 drivers/fpga/Makefile        |   1 +
 drivers/fpga/microchip-spi.c | 379 +++++++++++++++++++++++++++++++++++
 3 files changed, 390 insertions(+)
 create mode 100644 drivers/fpga/microchip-spi.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 26025dbab353..791ecf48503a 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -248,4 +248,14 @@ 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 with .dat formatted
+	  bitstream image.
+
 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..1bdd525d25d9
--- /dev/null
+++ b/drivers/fpga/microchip-spi.c
@@ -0,0 +1,379 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Microchip Polarfire FPGA programming over slave SPI interface.
+ */
+
+#include <linux/crc-ccitt.h>
+#include <linux/delay.h>
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/spi/spi.h>
+
+#define	MPF_SPI_ISC_ENABLE	0x0B
+#define	MPF_SPI_ISC_DISABLE	0x0C
+#define	MPF_SPI_READ_STATUS	0x00
+#define	MPF_SPI_READ_DATA	0x01
+#define	MPF_SPI_FRAME_INIT	0xAE
+#define	MPF_SPI_FRAME		0xEE
+#define	MPF_SPI_PRG_MODE	0x01
+#define	MPF_SPI_RELEASE		0x23
+
+#define	MPF_SPI_FRAME_SIZE	16
+
+#define	MPF_HEADER_SIZE_OFFSET	24
+#define	MPF_DATA_SIZE_OFFSET	55
+
+#define	MPF_LOOKUP_TABLE_RECORD_SIZE		9
+#define	MPF_LOOKUP_TABLE_BLOCK_ID_OFFSET	0
+#define	MPF_LOOKUP_TABLE_BLOCK_START_OFFSET	1
+
+#define	MPF_COMPONENTS_SIZE_ID	5
+#define	MPF_BITSTREAM_ID	8
+
+#define	MPF_BITS_PER_COMPONENT_SIZE	22
+
+#define	MPF_STATUS_POLL_TIMEOUT		1000
+#define	MPF_STATUS_BUSY			BIT(0)
+#define	MPF_STATUS_READY		BIT(1)
+#define	MPF_STATUS_SPI_VIOLATION	BIT(2)
+#define	MPF_STATUS_SPI_ERROR		BIT(3)
+
+struct mpf_priv {
+	struct spi_device *spi;
+	bool program_mode;
+};
+
+static int mpf_read_status(struct spi_device *spi)
+{
+	u8 status, status_command = MPF_SPI_READ_STATUS;
+	struct spi_transfer xfer = {
+		.tx_buf = &status_command,
+		.rx_buf = &status,
+		.len = 1,
+	};
+	int ret = spi_sync_transfer(spi, &xfer, 1);
+
+	if ((status & MPF_STATUS_SPI_VIOLATION) ||
+	    (status & MPF_STATUS_SPI_ERROR))
+		ret = -EIO;
+
+	return ret ? : status;
+}
+
+static enum fpga_mgr_states mpf_ops_state(struct fpga_manager *mgr)
+{
+	struct mpf_priv *priv = mgr->priv;
+	struct spi_device *spi;
+	bool program_mode;
+	int status;
+
+	spi = priv->spi;
+	program_mode = priv->program_mode;
+	status = mpf_read_status(spi);
+
+	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)
+{
+	int status, timeout = MPF_STATUS_POLL_TIMEOUT;
+
+	while (timeout--) {
+		status = mpf_read_status(spi);
+		if (status < 0 ||
+		    (!(status & MPF_STATUS_BUSY) && (!mask || (status & mask))))
+			return status;
+
+		usleep_range(1000, 2000);
+	}
+
+	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[] = { MPF_SPI_READ_DATA };
+	int ret;
+
+	ret = mpf_spi_write(spi, txbuf, txbuf_size);
+	if (ret)
+		return ret;
+
+	ret = poll_status_not_busy(spi, MPF_STATUS_READY);
+	if (ret < 0)
+		return ret;
+
+	return spi_write_then_read(spi, read_command, sizeof(read_command),
+				   rxbuf, rxbuf_size);
+}
+
+static ssize_t lookup_block_start(int id, const char *buf, size_t buf_size)
+{
+	size_t block_id_offset, block_start_offset;
+	u8 header_size, blocks_num, block_id;
+
+	header_size = *(buf + MPF_HEADER_SIZE_OFFSET);
+	if (header_size > buf_size)
+		return -EFAULT;
+
+	blocks_num = *(buf + header_size - 1);
+	if (header_size + blocks_num * MPF_LOOKUP_TABLE_RECORD_SIZE > buf_size)
+		return -EFAULT;
+
+	block_id_offset = header_size + MPF_LOOKUP_TABLE_BLOCK_ID_OFFSET;
+	block_start_offset = header_size + MPF_LOOKUP_TABLE_BLOCK_START_OFFSET;
+
+	while (blocks_num--) {
+		block_id = *(buf + block_id_offset);
+		if (block_id == id)
+			return get_unaligned_le32(buf + block_start_offset);
+
+		block_id_offset += MPF_LOOKUP_TABLE_RECORD_SIZE;
+		block_start_offset += MPF_LOOKUP_TABLE_RECORD_SIZE;
+	}
+
+	return -EFAULT;
+}
+
+static ssize_t parse_bitstream_size(const char *buf, size_t buf_size)
+{
+	ssize_t	bitstream_size = 0, components_size_start,
+		component_size_byte_num, component_size_byte_off, i;
+	u16 components_num;
+	u32 component_size;
+
+	if (buf_size < MPF_DATA_SIZE_OFFSET)
+		return -EFAULT;
+
+	components_num = get_unaligned_le16(buf + MPF_DATA_SIZE_OFFSET);
+	components_size_start = lookup_block_start(MPF_COMPONENTS_SIZE_ID, buf,
+						   buf_size);
+	if (components_size_start < 0)
+		return components_size_start;
+
+	if (components_size_start +
+	    DIV_ROUND_UP(components_num * MPF_BITS_PER_COMPONENT_SIZE,
+			 BITS_PER_BYTE) > buf_size)
+		return -EFAULT;
+
+	for (i = 0; i < components_num; i++) {
+		component_size_byte_num =
+			(i * MPF_BITS_PER_COMPONENT_SIZE) / BITS_PER_BYTE;
+		component_size_byte_off =
+			(i * MPF_BITS_PER_COMPONENT_SIZE) % BITS_PER_BYTE;
+
+		component_size = get_unaligned_le32(buf +
+						    components_size_start +
+						    component_size_byte_num);
+		component_size >>= component_size_byte_off;
+		component_size &= GENMASK(MPF_BITS_PER_COMPONENT_SIZE - 1, 0);
+
+		bitstream_size += component_size;
+	}
+
+	return bitstream_size;
+}
+
+static int mpf_ops_write_init(struct fpga_manager *mgr,
+			      struct fpga_image_info *info, const char *buf,
+			      size_t count)
+{
+	const u8 program_mode[] = { MPF_SPI_FRAME_INIT, MPF_SPI_PRG_MODE };
+	const u8 isc_en_command[] = { MPF_SPI_ISC_ENABLE };
+	ssize_t bitstream_start, bitstream_size;
+	struct mpf_priv *priv = mgr->priv;
+	struct device *dev = &mgr->dev;
+	struct spi_device *spi;
+	u32 isc_ret;
+	int ret;
+
+	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
+		dev_err(dev, "Partial reconfiguration is not supported\n");
+		return -EOPNOTSUPP;
+	}
+
+	if (!buf) {
+		dev_err(dev, "FPGA image is not provided\n");
+		return -EINVAL;
+	}
+
+	if (crc_ccitt(0, buf, count)) {
+		dev_err(dev, "CRC error\n");
+		return -EINVAL;
+	}
+
+	bitstream_start = lookup_block_start(MPF_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 * MPF_SPI_FRAME_SIZE > count) {
+		dev_err(dev,
+			"Bitstream outruns firmware. Bitstream start %zd, bitstream size %zd, firmware size %zu\n",
+			bitstream_start, bitstream_size * MPF_SPI_FRAME_SIZE,
+			count);
+		return -EFAULT;
+	}
+
+	info->bitstream_start = bitstream_start;
+	info->bitstream_size = bitstream_size * MPF_SPI_FRAME_SIZE;
+
+	spi = priv->spi;
+
+	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 ? : 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 int mpf_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
+{
+	u8 tmp_buf[MPF_SPI_FRAME_SIZE + 1] = { MPF_SPI_FRAME, };
+	struct mpf_priv *priv = mgr->priv;
+	struct device *dev = &mgr->dev;
+	struct spi_device *spi;
+	int ret, i;
+
+	if (count % MPF_SPI_FRAME_SIZE) {
+		dev_err(dev, "Bitstream size is not a multiple of %d\n",
+			MPF_SPI_FRAME_SIZE);
+		return -EINVAL;
+	}
+
+	spi = priv->spi;
+
+	for (i = 0; i < count / MPF_SPI_FRAME_SIZE; i++) {
+		memcpy(tmp_buf + 1, buf + i * MPF_SPI_FRAME_SIZE,
+		       MPF_SPI_FRAME_SIZE);
+
+		ret = mpf_spi_write(spi, tmp_buf, sizeof(tmp_buf));
+		if (ret) {
+			dev_err(dev, "Failed to write bitstream frame %d/%zd\n",
+				i, count / MPF_SPI_FRAME_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[] = { MPF_SPI_ISC_DISABLE };
+	const u8 release_command[] = { MPF_SPI_RELEASE };
+	struct mpf_priv *priv = mgr->priv;
+	struct device *dev = &mgr->dev;
+	struct spi_device *spi;
+	int ret;
+
+	spi = priv->spi;
+
+	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;
+	}
+
+	usleep_range(1000, 2000);
+
+	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 device *dev = &spi->dev;
+	struct fpga_manager *mgr;
+	struct mpf_priv *priv;
+
+	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.25.1

[PATCH v9 3/3] dt-bindings: fpga: add binding doc for microchip-spi fpga mgr

[Ivan Bornyakov]

Add Device Tree Binding doc for Microchip Polarfire FPGA Manager using
slave SPI to load .dat formatted bitstream image.

Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../fpga/microchip,mpf-spi-fpga-mgr.yaml      | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml

diff --git a/Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml b/Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml
new file mode 100644
index 000000000000..aee45cb15592
--- /dev/null
+++ b/Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/fpga/microchip,mpf-spi-fpga-mgr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip Polarfire FPGA manager.
+
+maintainers:
+  - Ivan Bornyakov <i.bornyakov@metrotek.ru>
+
+description:
+  Device Tree Bindings for Microchip Polarfire FPGA Manager using slave SPI to
+  load the bitstream in .dat format.
+
+properties:
+  compatible:
+    enum:
+      - microchip,mpf-spi-fpga-mgr
+
+  reg:
+    description: SPI chip select
+    maxItems: 1
+
+  spi-max-frequency: true
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    spi {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            fpga_mgr@0 {
+                    compatible = "microchip,mpf-spi-fpga-mgr";
+                    spi-max-frequency = <20000000>;
+                    reg = <0>;
+            };
+    };
-- 
2.25.1

Re: [PATCH v9 1/3] fpga: fpga-mgr: support bitstream offset in image buffer

[Xu Yilun]

On Thu, Apr 07, 2022 at 04:36:56PM +0300, Ivan Bornyakov wrote:
> It is not always whole FPGA image buffer meant to be written to the
> device.

Thanks for improving the fpga core. Please see my comments inline.

Maybe more description about the issue, i.e. in which case we don't
write the whole buffer, what's the problem in current implementation.

> 
> Add bitstream_start and bitstream_size to the fpga_image_info struct and
> adjust fpga_mgr_write() callers with respect to them.
> 
> If initial_header_size is not known beforehand, pass whole buffer to low
> level driver's write_init() so it could setup info->bitstream_start and
> info->bitstream_size regardless.
> 
> Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
> ---
>  drivers/fpga/fpga-mgr.c       | 48 +++++++++++++++++++++++++++++------
>  include/linux/fpga/fpga-mgr.h |  5 ++++
>  2 files changed, 45 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> index d49a9ce34568..c64e60e23a71 100644
> --- a/drivers/fpga/fpga-mgr.c
> +++ b/drivers/fpga/fpga-mgr.c
> @@ -139,7 +139,8 @@ EXPORT_SYMBOL_GPL(fpga_image_info_free);
>   * Call the low level driver's write_init function.  This will do the
>   * device-specific things to get the FPGA into the state where it is ready to
>   * receive an FPGA image. The low level driver only gets to see the first
> - * initial_header_size bytes in the buffer.
> + * initial_header_size bytes in the buffer, if initial_header_size is set.
> + * Otherwise, the whole buffer will be passed.
>   */
>  static int fpga_mgr_write_init_buf(struct fpga_manager *mgr,
>  				   struct fpga_image_info *info,
> @@ -148,12 +149,10 @@ static int fpga_mgr_write_init_buf(struct fpga_manager *mgr,
>  	int ret;
>  
>  	mgr->state = FPGA_MGR_STATE_WRITE_INIT;
> -	if (!mgr->mops->initial_header_size)
> -		ret = fpga_mgr_write_init(mgr, info, NULL, 0);
> -	else
> -		ret = fpga_mgr_write_init(
> -		    mgr, info, buf, min(mgr->mops->initial_header_size, count));
> +	if (mgr->mops->initial_header_size)
> +		count = min(mgr->mops->initial_header_size, count);
>  
> +	ret = fpga_mgr_write_init(mgr, info, buf, count);

Here we pass the whole buffer for write_init(). Maybe it works for mapped buf,
but still doesn't work for sg buf.

It is also inefficient if we change to map and copy all sg buffers just for
write_init().

We could discuss on the solution.

My quick mind is we introduce an optional fpga_manager_ops.parse_header()
callback, and a header_size (dynamic header size) field in
fpga_image_info. FPGA core starts with mapping a buf of initial_header_size
for parse_header(), let the drivers decide the dynamic header_size.

The parse_header() could be called several times with updated dynamic
header_size, if drivers doesn't get enough buffer for final decision and
return -EAGAIN.

Then write_init() be called with the final dynamic header size.

For mapped buffer, just passing the whole buffer for write_init() is
fine.

>  	if (ret) {
>  		dev_err(&mgr->dev, "Error preparing FPGA for writing\n");
>  		mgr->state = FPGA_MGR_STATE_WRITE_INIT_ERR;
> @@ -235,13 +234,33 @@ static int fpga_mgr_buf_load_sg(struct fpga_manager *mgr,
>  	if (mgr->mops->write_sg) {
>  		ret = fpga_mgr_write_sg(mgr, sgt);
>  	} else {
> +		size_t offset, count, length, bitstream_size;
>  		struct sg_mapping_iter miter;
>  
> +		offset = info->bitstream_start;
> +		bitstream_size = info->bitstream_size;
> +		count = 0;
> +
>  		sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
>  		while (sg_miter_next(&miter)) {
> -			ret = fpga_mgr_write(mgr, miter.addr, miter.length);
> -			if (ret)
> +			if (offset >= miter.length) {
> +				offset -= miter.length;
> +				continue;
> +			}
> +
> +			if (bitstream_size)
> +				length = min(miter.length - offset,
> +					     bitstream_size - count);
> +			else
> +				length = miter.length - offset;
> +
> +			count += length;
> +
> +			ret = fpga_mgr_write(mgr, miter.addr + offset, length);
> +			if (ret || count == bitstream_size)
>  				break;
> +
> +			offset = 0;
>  		}
>  		sg_miter_stop(&miter);
>  	}
> @@ -265,6 +284,19 @@ static int fpga_mgr_buf_load_mapped(struct fpga_manager *mgr,
>  	if (ret)
>  		return ret;
>  
> +	if (info->bitstream_start > count) {
> +		dev_err(&mgr->dev,
> +			"Bitstream start %zd outruns firmware image %zd\n",
> +			info->bitstream_start, count);
> +		return -EINVAL;
> +	}
> +
> +	if (info->bitstream_size)
> +		count = min(info->bitstream_start + info->bitstream_size, count);
> +
> +	buf += info->bitstream_start;
> +	count -= info->bitstream_start;
> +
>  	/*
>  	 * Write the FPGA image to the FPGA.
>  	 */
> diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
> index 0f9468771bb9..32464fd10cca 100644
> --- a/include/linux/fpga/fpga-mgr.h
> +++ b/include/linux/fpga/fpga-mgr.h
> @@ -85,6 +85,9 @@ enum fpga_mgr_states {
>   * @sgt: scatter/gather table containing FPGA image
>   * @buf: contiguous buffer containing FPGA image
>   * @count: size of buf
> + * @bitstream_start: offset in image buffer where bitstream data starts
> + * @bitstream_size: size of bitstream.
> + *	If 0, (count - bitstream_start) will be used.
>   * @region_id: id of target region
>   * @dev: device that owns this
>   * @overlay: Device Tree overlay
> @@ -98,6 +101,8 @@ struct fpga_image_info {
>  	struct sg_table *sgt;
>  	const char *buf;
>  	size_t count;
> +	size_t bitstream_start;

How about we name it header_size?

> +	size_t bitstream_size;

And how about data_size?

Thanks,
Yilun

>  	int region_id;
>  	struct device *dev;
>  #ifdef CONFIG_OF
> -- 
> 2.25.1
> 

Re: [PATCH v9 1/3] fpga: fpga-mgr: support bitstream offset in image buffer

[Ivan Bornyakov]

On Sat, Apr 09, 2022 at 01:04:23PM +0800, Xu Yilun wrote:
> On Thu, Apr 07, 2022 at 04:36:56PM +0300, Ivan Bornyakov wrote:
> > 
> > ... snip ...
> > 
> > @@ -148,12 +149,10 @@ static int fpga_mgr_write_init_buf(struct fpga_manager *mgr,
> >  	int ret;
> >  
> >  	mgr->state = FPGA_MGR_STATE_WRITE_INIT;
> > -	if (!mgr->mops->initial_header_size)
> > -		ret = fpga_mgr_write_init(mgr, info, NULL, 0);
> > -	else
> > -		ret = fpga_mgr_write_init(
> > -		    mgr, info, buf, min(mgr->mops->initial_header_size, count));
> > +	if (mgr->mops->initial_header_size)
> > +		count = min(mgr->mops->initial_header_size, count);
> >  
> > +	ret = fpga_mgr_write_init(mgr, info, buf, count);
> 
> Here we pass the whole buffer for write_init(). Maybe it works for mapped buf,
> but still doesn't work for sg buf.
> 
> It is also inefficient if we change to map and copy all sg buffers just for
> write_init().
> 
> We could discuss on the solution.
> 
> My quick mind is we introduce an optional fpga_manager_ops.parse_header()
> callback, and a header_size (dynamic header size) field in
> fpga_image_info. FPGA core starts with mapping a buf of initial_header_size
> for parse_header(), let the drivers decide the dynamic header_size.
> 
> The parse_header() could be called several times with updated dynamic
> header_size, if drivers doesn't get enough buffer for final decision and
> return -EAGAIN.
> 
> Then write_init() be called with the final dynamic header size.
> 
> For mapped buffer, just passing the whole buffer for write_init() is
> fine.
> 

Ok, that's sounds reasonable. Should we pass PAGE_SIZE of a buffer to
the parse_header() if initial_header_size is not set? The other option
would be to make initial_header_size to be mandatory for usage of
parse_header().

> > 
> > ... snip ...
> > 
> > @@ -98,6 +101,8 @@ struct fpga_image_info {
> >  	struct sg_table *sgt;
> >  	const char *buf;
> >  	size_t count;
> > +	size_t bitstream_start;
> 
> How about we name it header_size?
> 
> > +	size_t bitstream_size;
> 
> And how about data_size?
> 

Sure, I don't mind.

Re: [PATCH v9 1/3] fpga: fpga-mgr: support bitstream offset in image buffer

[Xu Yilun]

On Sat, Apr 09, 2022 at 03:38:51PM +0300, Ivan Bornyakov wrote:
> On Sat, Apr 09, 2022 at 01:04:23PM +0800, Xu Yilun wrote:
> > On Thu, Apr 07, 2022 at 04:36:56PM +0300, Ivan Bornyakov wrote:
> > > 
> > > ... snip ...
> > > 
> > > @@ -148,12 +149,10 @@ static int fpga_mgr_write_init_buf(struct fpga_manager *mgr,
> > >  	int ret;
> > >  
> > >  	mgr->state = FPGA_MGR_STATE_WRITE_INIT;
> > > -	if (!mgr->mops->initial_header_size)
> > > -		ret = fpga_mgr_write_init(mgr, info, NULL, 0);
> > > -	else
> > > -		ret = fpga_mgr_write_init(
> > > -		    mgr, info, buf, min(mgr->mops->initial_header_size, count));
> > > +	if (mgr->mops->initial_header_size)
> > > +		count = min(mgr->mops->initial_header_size, count);
> > >  
> > > +	ret = fpga_mgr_write_init(mgr, info, buf, count);
> > 
> > Here we pass the whole buffer for write_init(). Maybe it works for mapped buf,
> > but still doesn't work for sg buf.
> > 
> > It is also inefficient if we change to map and copy all sg buffers just for
> > write_init().
> > 
> > We could discuss on the solution.
> > 
> > My quick mind is we introduce an optional fpga_manager_ops.parse_header()
> > callback, and a header_size (dynamic header size) field in
> > fpga_image_info. FPGA core starts with mapping a buf of initial_header_size
> > for parse_header(), let the drivers decide the dynamic header_size.
> > 
> > The parse_header() could be called several times with updated dynamic
> > header_size, if drivers doesn't get enough buffer for final decision and
> > return -EAGAIN.
> > 
> > Then write_init() be called with the final dynamic header size.
> > 
> > For mapped buffer, just passing the whole buffer for write_init() is
> > fine.
> > 
> 
> Ok, that's sounds reasonable. Should we pass PAGE_SIZE of a buffer to
> the parse_header() if initial_header_size is not set? The other option

I think it is not necessary, no initial_header_size means no need to
parse image header.

> would be to make initial_header_size to be mandatory for usage of
> parse_header().

That's a good idea. If parse_header() is provided, initial_header_size
is mandatory.

Thanks,
Yilun

> 
> > > 
> > > ... snip ...
> > > 
> > > @@ -98,6 +101,8 @@ struct fpga_image_info {
> > >  	struct sg_table *sgt;
> > >  	const char *buf;
> > >  	size_t count;
> > > +	size_t bitstream_start;
> > 
> > How about we name it header_size?
> > 
> > > +	size_t bitstream_size;
> > 
> > And how about data_size?
> > 
> 
> Sure, I don't mind.