All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v21 0/5] Microchip Polarfire FPGA manager
@ 2022-06-21 15:38 Ivan Bornyakov
  2022-06-21 15:38 ` [PATCH v21 1/5] fpga: fpga-mgr: support bitstream offset in image buffer Ivan Bornyakov
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Ivan Bornyakov @ 2022-06-21 15:38 UTC (permalink / raw)
  To: mdf, hao.wu, yilun.xu, trix, corbet
  Cc: Ivan Bornyakov, conor.dooley, robh+dt, krzysztof.kozlowski+dt,
	linux-fpga, linux-doc, devicetree, linux-kernel, system

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()
  v9 -> v10:
   * add parse_header() callback to fpga_manager_ops
   * adjust fpga_mgr_write_init[_buf|_sg]() for parse_header() usage
   * implement parse_header() in microchip-spi driver
  v10 -> v11: include missing unaligned.h to microchip-spi
     fix error: implicit declaration of function 'get_unaligned_le[16|32]'
  v11 -> v12:
   * microchip-spi: double read hw status, ignore first read, because it
     can be unreliable.
   * microchip-spi: remove sleep between status readings in
     poll_status_not_busy() to save a few seconds. Status is polled on
     every 16 byte writes - that is quite often, therefore
     usleep_range() accumulate to a considerable number of seconds.
  v12 -> v13:
   * fpga-mgr: separate fpga_mgr_parse_header_buf() from
     fpga_mgr_write_init_buf()
   * fpga-mgr: introduce FPGA_MGR_STATE_PARSE_HEADER and
     FPGA_MGR_STATE_PARSE_HEADER_ERR fpga_mgr_states
   * fpga-mgr: rename fpga_mgr_write_init_sg() to fpga_mgr_prepare_sg()
     and rework with respect to a new fpga_mgr_parse_header_buf()
   * fpga-mgr: rework write accounting in fpga_mgr_buf_load_sg() for
     better clarity
   * microchip-spi: rename MPF_STATUS_POLL_TIMEOUT to
     MPF_STATUS_POLL_RETRIES
   * microchip-spi: add comment about status reading quirk to
     mpf_read_status()
   * microchip-spi: rename poll_status_not_busy() to mpf_poll_status()
     and add comment.
   * microchip-spi: make if statement in mpf_poll_status() easier to
     read.
  v13 -> v14:
   * fpga-mgr: improvements from Xu Yilun in
      - fpga_mgr_parse_header_buf()
      - fpga_mgr_write_init_buf()
      - fpga_mgr_prepare_sg()
      - fpga_mgr_buf_load_sg()
   * fpga-mgr: add check for -EAGAIN from fpga_mgr_parse_header_buf()
     when called from fpga_mgr_buf_load_mapped()
   * microchip-spi: remove excessive cs_change from second spi_transfer
     in mpf_read_status()
   * microchip-spi: change type of components_size_start,
     bitstream_start, i from size_t to u32 in mpf_ops_parse_header()
  v14 -> v15: eliminate memcpy() in mpf_ops_write()
    Eliminate excessive memcpy() in mpf_ops_write() by using
    spi_sync_transfer() instead of spi_write().
  v15 -> v16:
   * microchip-spi: change back components_size_start and
     bitstream_start variables types to size_t, i - to u16 in
     mpf_ops_parse_header()
   * fpga-mgr: rename fpga_parse_header_buf() to
     fpga_parse_header_mapped(). It serves only mapped FPGA image now,
     adjust it accordingly.
   * fpga-mgr: separate fpga_mgr_parse_header_sg_first() and
     fpga_mgr_parse_header_sg() from fpga_mgr_prepare_sg()
  v16 -> v17:
   * fpga-mgr: return size of allocated header from
     fpga_mgr_parse_header_sg(), add `char **ret_buf` to function args
     to save pointer to allocated header. This allow us to call
     fpga_mgr_write_init_buf() with exact size of allocated header.
   * document parse_header() callback in fpga-mgr.rst
  v17 -> v18:
   * fpga-mgr: change back fpga_mgr_parse_header_sg() to return
     allocated buffer but set buffer size into output parameter
   * fpga-mgr: check returned pointer from krealloc for ZERO_OR_NULL_PTR
     in fpga_mgr_paese_header_sg() as krealloc may return ZERO_SIZE_PTR.
   * fpga-mgr: in fpga_mgr_prepare_sg() return fpga_mgr_write_init() on
     fast path only when both initial_header_size and parse_header() are
     not defined.
   * docs: fpga-mgr: a few rewords from Xu Yilun
  v18 -> v19:
   * microchip-spi: split multiple assignments on a single line in
     functions mpf_read_status() and mpf_ops_parse_header()
   * fpga-mgr: add braces {} around "else if" arm in
     fpga_mgr_prepare_sg()
   * fpga-mgr: don't reuse krealloc() arg in fpga_mgr_parse_header_sg().
     If krealloc() returns NULL, it doesn't free the original.
  v19 -> v20:
   * fpga-mgr: initialize info->header_size with
     mops->initial_header_size at fpga_mgr_load().
   * fpga-mgr: add mops->skip_header boolean flag. Adjust skipping
     header before write to check against skip_header flag instead of
     mere presence of info->header_size.
   * fpga-mgr: split check for ZERO_OR_NULL_PTR() after realloc() in
     fpga_mgr_parse_header_sg() function to check against zero header +
     check against NULL returned from realloc().
   * docs: fpga-mgr: adjust for skip_header flag.
   * microchip-spi: add skip_header to mpf_ops.
  v21 -> v21:
   * fpga-mgr: in function fpga_mgr_parse_header_sg(), after
     reallocation of a buffer for a new header, copy only exceeding
     part, since realloc preserve old buffer content.
   * fpga-mgr: in function fpga_mgr_buf_load_sg() instead of manually
     counting fragments with header that should be skipped, use
     sg_miter_skip().
   * fpga-mgr: add tag "Suggested-by: Xu Yilun <yilun.xu@intel.com>"
     since all changes in fpga-mgr was his ideas.
   * MAINTAINERS: add entry for Microchip PolarFire FPGA drivers as
     Conor Dooley suggested.

Ivan Bornyakov (5):
  fpga: fpga-mgr: support bitstream offset in image buffer
  docs: fpga: mgr: document parse_header() callback
  fpga: microchip-spi: add Microchip MPF FPGA manager
  dt-bindings: fpga: add binding doc for microchip-spi fpga mgr
  MAINTAINERS: add Microchip PolarFire FPGA drivers entry

 .../fpga/microchip,mpf-spi-fpga-mgr.yaml      |  44 ++
 Documentation/driver-api/fpga/fpga-mgr.rst    |  27 +-
 MAINTAINERS                                   |   8 +
 drivers/fpga/Kconfig                          |   8 +
 drivers/fpga/Makefile                         |   1 +
 drivers/fpga/fpga-mgr.c                       | 225 ++++++++--
 drivers/fpga/microchip-spi.c                  | 398 ++++++++++++++++++
 include/linux/fpga/fpga-mgr.h                 |  24 +-
 8 files changed, 702 insertions(+), 33 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



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

* [PATCH v21 1/5] fpga: fpga-mgr: support bitstream offset in image buffer
  2022-06-21 15:38 [PATCH v21 0/5] Microchip Polarfire FPGA manager Ivan Bornyakov
@ 2022-06-21 15:38 ` Ivan Bornyakov
  2022-06-23  9:28   ` Xu Yilun
  2022-06-21 15:38 ` [PATCH v21 2/5] docs: fpga: mgr: document parse_header() callback Ivan Bornyakov
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Ivan Bornyakov @ 2022-06-21 15:38 UTC (permalink / raw)
  To: mdf, hao.wu, yilun.xu, trix, corbet
  Cc: Ivan Bornyakov, conor.dooley, robh+dt, krzysztof.kozlowski+dt,
	linux-fpga, linux-doc, devicetree, linux-kernel, system

At the moment FPGA manager core loads to the device entire image
provided to fpga_mgr_load(). But it is not always whole FPGA image
buffer meant to be written to the device. In particular, .dat formatted
image for Microchip MPF contains meta info in the header that is not
meant to be written to the device. This is issue for those low level
drivers that loads data to the device with write() fpga_manager_ops
callback, since write() can be called in iterator over scatter-gather
table, not only linear image buffer. On the other hand, write_sg()
callback is provided with whole image in scatter-gather form and can
decide itself which part should be sent to the device.

Add header_size and data_size to the fpga_image_info struct, add
skip_header to the fpga_manager_ops struct and adjust fpga_mgr_write()
callers with respect to them.

  * info->header_size indicates part at the beginning of image buffer
    that contains some meta info. It is optional and can be 0,
    initialized with mops->initial_header_size.

  * mops->skip_header tells fpga-mgr core whether write should start
    from the beginning of image buffer or at the offset of header_size.

  * info->data_size is the size of bitstream data that is meant to be
    written to the device. It is also optional and can be 0, which
    means bitstream data is up to the end of image buffer.

Also add parse_header() callback to fpga_manager_ops, which purpose is
to set info->header_size and info->data_size. At least
initial_header_size bytes of image buffer will be passed into
parse_header() first time. If it is not enough, parse_header() should
set desired size into info->header_size and return -EAGAIN, then it will
be called again with greater part of image buffer on the input.

Suggested-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
---
 drivers/fpga/fpga-mgr.c       | 225 ++++++++++++++++++++++++++++++----
 include/linux/fpga/fpga-mgr.h |  24 +++-
 2 files changed, 222 insertions(+), 27 deletions(-)

diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index 08dc85fcd511..d97df0bdc72b 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -74,6 +74,15 @@ static inline int fpga_mgr_write_complete(struct fpga_manager *mgr,
 	return 0;
 }
 
+static inline int fpga_mgr_parse_header(struct fpga_manager *mgr,
+					struct fpga_image_info *info,
+					const char *buf, size_t count)
+{
+	if (mgr->mops->parse_header)
+		return mgr->mops->parse_header(mgr, info, buf, count);
+	return 0;
+}
+
 static inline int fpga_mgr_write_init(struct fpga_manager *mgr,
 				      struct fpga_image_info *info,
 				      const char *buf, size_t count)
@@ -136,24 +145,141 @@ void fpga_image_info_free(struct fpga_image_info *info)
 EXPORT_SYMBOL_GPL(fpga_image_info_free);
 
 /*
- * Call the low level driver's write_init function.  This will do the
+ * Call the low level driver's parse_header function with entire FPGA image
+ * buffer on the input. This will set info->header_size and info->data_size.
+ */
+static int fpga_mgr_parse_header_mapped(struct fpga_manager *mgr,
+					struct fpga_image_info *info,
+					const char *buf, size_t count)
+{
+	int ret;
+
+	mgr->state = FPGA_MGR_STATE_PARSE_HEADER;
+	ret = fpga_mgr_parse_header(mgr, info, buf, count);
+
+	if (info->header_size + info->data_size > count) {
+		dev_err(&mgr->dev, "Bitsream data outruns FPGA image\n");
+		ret = -EINVAL;
+	}
+
+	if (ret) {
+		dev_err(&mgr->dev, "Error while parsing FPGA image header\n");
+		mgr->state = FPGA_MGR_STATE_PARSE_HEADER_ERR;
+	}
+
+	return ret;
+}
+
+/*
+ * Call the low level driver's parse_header function with first fragment of
+ * scattered FPGA image on the input. If header fits first fragment,
+ * parse_header will set info->header_size and info->data_size. If it is not,
+ * parse_header will set desired size to info->header_size and -EAGAIN will be
+ * returned.
+ */
+static int fpga_mgr_parse_header_sg_first(struct fpga_manager *mgr,
+					  struct fpga_image_info *info,
+					  struct sg_table *sgt)
+{
+	struct sg_mapping_iter miter;
+	int ret;
+
+	mgr->state = FPGA_MGR_STATE_PARSE_HEADER;
+
+	sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
+	if (sg_miter_next(&miter) &&
+	    miter.length >= info->header_size)
+		ret = fpga_mgr_parse_header(mgr, info, miter.addr, miter.length);
+	else
+		ret = -EAGAIN;
+	sg_miter_stop(&miter);
+
+	if (ret && ret != -EAGAIN) {
+		dev_err(&mgr->dev, "Error while parsing FPGA image header\n");
+		mgr->state = FPGA_MGR_STATE_PARSE_HEADER_ERR;
+	}
+
+	return ret;
+}
+
+/*
+ * Copy scattered FPGA image fragments to temporary buffer and call the
+ * low level driver's parse_header function. This should be called after
+ * fpga_mgr_parse_header_sg_first() returned -EAGAIN. In case of success,
+ * pointer to the newly allocated image header copy will be returned and
+ * its size will be set into *ret_size. Returned buffer needs to be freed.
+ */
+static void *fpga_mgr_parse_header_sg(struct fpga_manager *mgr,
+				      struct fpga_image_info *info,
+				      struct sg_table *sgt, size_t *ret_size)
+{
+	size_t len, new_header_size, header_size = 0;
+	char *new_buf, *buf = NULL;
+	int ret;
+
+	do {
+		new_header_size = info->header_size;
+		if (new_header_size <= header_size) {
+			dev_err(&mgr->dev, "Requested invalid header size\n");
+			ret = -EFAULT;
+			break;
+		}
+
+		new_buf = krealloc(buf, new_header_size, GFP_KERNEL);
+		if (!new_buf) {
+			ret = -ENOMEM;
+			break;
+		}
+
+		buf = new_buf;
+
+		len = sg_pcopy_to_buffer(sgt->sgl, sgt->nents,
+					 buf + header_size,
+					 new_header_size - header_size,
+					 header_size);
+		if (len != new_header_size - header_size) {
+			ret = -EFAULT;
+			break;
+		}
+
+		header_size = new_header_size;
+		ret = fpga_mgr_parse_header(mgr, info, buf, header_size);
+	} while (ret == -EAGAIN);
+
+	if (ret) {
+		dev_err(&mgr->dev, "Error while parsing FPGA image header\n");
+		mgr->state = FPGA_MGR_STATE_PARSE_HEADER_ERR;
+		kfree(buf);
+		buf = ERR_PTR(ret);
+	}
+
+	*ret_size = header_size;
+
+	return buf;
+}
+
+/*
+ * 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.
+ * receive an FPGA image. The low level driver gets to see at least first
+ * info->header_size bytes in the buffer. If info->header_size is 0,
+ * write_init will not get any bytes of image buffer.
  */
 static int fpga_mgr_write_init_buf(struct fpga_manager *mgr,
 				   struct fpga_image_info *info,
 				   const char *buf, size_t count)
 {
+	size_t header_size = info->header_size;
 	int ret;
 
 	mgr->state = FPGA_MGR_STATE_WRITE_INIT;
-	if (!mgr->mops->initial_header_size) {
+
+	if (header_size > count)
+		ret = -EINVAL;
+	else if (!header_size)
 		ret = fpga_mgr_write_init(mgr, info, NULL, 0);
-	} else {
-		count = min(mgr->mops->initial_header_size, count);
+	else
 		ret = fpga_mgr_write_init(mgr, info, buf, count);
-	}
 
 	if (ret) {
 		dev_err(&mgr->dev, "Error preparing FPGA for writing\n");
@@ -164,39 +290,50 @@ static int fpga_mgr_write_init_buf(struct fpga_manager *mgr,
 	return 0;
 }
 
-static int fpga_mgr_write_init_sg(struct fpga_manager *mgr,
-				  struct fpga_image_info *info,
-				  struct sg_table *sgt)
+static int fpga_mgr_prepare_sg(struct fpga_manager *mgr,
+			       struct fpga_image_info *info,
+			       struct sg_table *sgt)
 {
 	struct sg_mapping_iter miter;
 	size_t len;
 	char *buf;
 	int ret;
 
-	if (!mgr->mops->initial_header_size)
+	/* Short path. Low level driver don't care about image header. */
+	if (!mgr->mops->initial_header_size && !mgr->mops->parse_header)
 		return fpga_mgr_write_init_buf(mgr, info, NULL, 0);
 
 	/*
 	 * First try to use miter to map the first fragment to access the
 	 * header, this is the typical path.
 	 */
-	sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
-	if (sg_miter_next(&miter) &&
-	    miter.length >= mgr->mops->initial_header_size) {
-		ret = fpga_mgr_write_init_buf(mgr, info, miter.addr,
-					      miter.length);
+	ret = fpga_mgr_parse_header_sg_first(mgr, info, sgt);
+	/* If 0, header fits first fragment, call write_init on it */
+	if (!ret) {
+		sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
+		if (sg_miter_next(&miter)) {
+			ret = fpga_mgr_write_init_buf(mgr, info, miter.addr,
+						      miter.length);
+			sg_miter_stop(&miter);
+			return ret;
+		}
 		sg_miter_stop(&miter);
+	/*
+	 * If -EAGAIN, more sg buffer is needed,
+	 * otherwise an error has occurred.
+	 */
+	} else if (ret != -EAGAIN) {
 		return ret;
 	}
-	sg_miter_stop(&miter);
 
-	/* Otherwise copy the fragments into temporary memory. */
-	buf = kmalloc(mgr->mops->initial_header_size, GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
+	/*
+	 * Copy the fragments into temporary memory.
+	 * Copying is done inside fpga_mgr_parse_header_sg().
+	 */
+	buf = fpga_mgr_parse_header_sg(mgr, info, sgt, &len);
+	if (IS_ERR(buf))
+		return PTR_ERR(buf);
 
-	len = sg_copy_to_buffer(sgt->sgl, sgt->nents, buf,
-				mgr->mops->initial_header_size);
 	ret = fpga_mgr_write_init_buf(mgr, info, buf, len);
 
 	kfree(buf);
@@ -227,7 +364,7 @@ static int fpga_mgr_buf_load_sg(struct fpga_manager *mgr,
 {
 	int ret;
 
-	ret = fpga_mgr_write_init_sg(mgr, info, sgt);
+	ret = fpga_mgr_prepare_sg(mgr, info, sgt);
 	if (ret)
 		return ret;
 
@@ -236,17 +373,37 @@ 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 length, data_size = info->data_size;
 		struct sg_mapping_iter miter;
+		ssize_t count = 0;
 
 		sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
+
+		if (mgr->mops->skip_header &&
+		    !sg_miter_skip(&miter, info->header_size)) {
+			ret = -EINVAL;
+			sg_miter_stop(&miter);
+			goto out;
+		}
+
 		while (sg_miter_next(&miter)) {
-			ret = fpga_mgr_write(mgr, miter.addr, miter.length);
+			if (data_size)
+				length = min(miter.length, data_size - count);
+			else
+				length = miter.length;
+
+			ret = fpga_mgr_write(mgr, miter.addr, length);
 			if (ret)
 				break;
+
+			count += length;
+			if (data_size && count >= data_size)
+				break;
 		}
 		sg_miter_stop(&miter);
 	}
 
+out:
 	if (ret) {
 		dev_err(&mgr->dev, "Error while writing image data to FPGA\n");
 		mgr->state = FPGA_MGR_STATE_WRITE_ERR;
@@ -262,10 +419,22 @@ static int fpga_mgr_buf_load_mapped(struct fpga_manager *mgr,
 {
 	int ret;
 
+	ret = fpga_mgr_parse_header_mapped(mgr, info, buf, count);
+	if (ret)
+		return ret;
+
 	ret = fpga_mgr_write_init_buf(mgr, info, buf, count);
 	if (ret)
 		return ret;
 
+	if (mgr->mops->skip_header) {
+		buf += info->header_size;
+		count -= info->header_size;
+	}
+
+	if (info->data_size)
+		count = info->data_size;
+
 	/*
 	 * Write the FPGA image to the FPGA.
 	 */
@@ -404,6 +573,8 @@ static int fpga_mgr_firmware_load(struct fpga_manager *mgr,
  */
 int fpga_mgr_load(struct fpga_manager *mgr, struct fpga_image_info *info)
 {
+	info->header_size = mgr->mops->initial_header_size;
+
 	if (info->sgt)
 		return fpga_mgr_buf_load_sg(mgr, info, info->sgt);
 	if (info->buf && info->count)
@@ -424,6 +595,10 @@ static const char * const state_str[] = {
 	[FPGA_MGR_STATE_FIRMWARE_REQ] =		"firmware request",
 	[FPGA_MGR_STATE_FIRMWARE_REQ_ERR] =	"firmware request error",
 
+	/* Parse FPGA image header */
+	[FPGA_MGR_STATE_PARSE_HEADER] =		"parse header",
+	[FPGA_MGR_STATE_PARSE_HEADER_ERR] =	"parse header error",
+
 	/* Preparing FPGA to receive image */
 	[FPGA_MGR_STATE_WRITE_INIT] =		"write init",
 	[FPGA_MGR_STATE_WRITE_INIT_ERR] =	"write init error",
diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
index 0f9468771bb9..54f63459efd6 100644
--- a/include/linux/fpga/fpga-mgr.h
+++ b/include/linux/fpga/fpga-mgr.h
@@ -22,6 +22,8 @@ struct sg_table;
  * @FPGA_MGR_STATE_RESET: FPGA in reset state
  * @FPGA_MGR_STATE_FIRMWARE_REQ: firmware request in progress
  * @FPGA_MGR_STATE_FIRMWARE_REQ_ERR: firmware request failed
+ * @FPGA_MGR_STATE_PARSE_HEADER: parse FPGA image header
+ * @FPGA_MGR_STATE_PARSE_HEADER_ERR: Error during PARSE_HEADER stage
  * @FPGA_MGR_STATE_WRITE_INIT: preparing FPGA for programming
  * @FPGA_MGR_STATE_WRITE_INIT_ERR: Error during WRITE_INIT stage
  * @FPGA_MGR_STATE_WRITE: writing image to FPGA
@@ -41,7 +43,9 @@ enum fpga_mgr_states {
 	FPGA_MGR_STATE_FIRMWARE_REQ,
 	FPGA_MGR_STATE_FIRMWARE_REQ_ERR,
 
-	/* write sequence: init, write, complete */
+	/* write sequence: parse header, init, write, complete */
+	FPGA_MGR_STATE_PARSE_HEADER,
+	FPGA_MGR_STATE_PARSE_HEADER_ERR,
 	FPGA_MGR_STATE_WRITE_INIT,
 	FPGA_MGR_STATE_WRITE_INIT_ERR,
 	FPGA_MGR_STATE_WRITE,
@@ -85,6 +89,9 @@ enum fpga_mgr_states {
  * @sgt: scatter/gather table containing FPGA image
  * @buf: contiguous buffer containing FPGA image
  * @count: size of buf
+ * @header_size: size of image header.
+ * @data_size: size of image data to be sent to the device. If not specified,
+ *	whole image will be used. Header may be skipped in either case.
  * @region_id: id of target region
  * @dev: device that owns this
  * @overlay: Device Tree overlay
@@ -98,6 +105,8 @@ struct fpga_image_info {
 	struct sg_table *sgt;
 	const char *buf;
 	size_t count;
+	size_t header_size;
+	size_t data_size;
 	int region_id;
 	struct device *dev;
 #ifdef CONFIG_OF
@@ -137,9 +146,16 @@ struct fpga_manager_info {
 
 /**
  * struct fpga_manager_ops - ops for low level fpga manager drivers
- * @initial_header_size: Maximum number of bytes that should be passed into write_init
+ * @initial_header_size: minimum number of bytes that should be passed into
+ *	parse_header and write_init.
+ * @skip_header: bool flag to tell fpga-mgr core whether it should skip
+ *	info->header_size part at the beginning of the image when invoking
+ *	write callback.
  * @state: returns an enum value of the FPGA's state
  * @status: returns status of the FPGA, including reconfiguration error code
+ * @parse_header: parse FPGA image header to set info->header_size and
+ *	info->data_size. In case the input buffer is not large enough, set
+ *	required size to info->header_size and return -EAGAIN.
  * @write_init: prepare the FPGA to receive configuration data
  * @write: write count bytes of configuration data to the FPGA
  * @write_sg: write the scatter list of configuration data to the FPGA
@@ -153,8 +169,12 @@ struct fpga_manager_info {
  */
 struct fpga_manager_ops {
 	size_t initial_header_size;
+	bool skip_header;
 	enum fpga_mgr_states (*state)(struct fpga_manager *mgr);
 	u64 (*status)(struct fpga_manager *mgr);
+	int (*parse_header)(struct fpga_manager *mgr,
+			    struct fpga_image_info *info,
+			    const char *buf, size_t count);
 	int (*write_init)(struct fpga_manager *mgr,
 			  struct fpga_image_info *info,
 			  const char *buf, size_t count);
-- 
2.25.1



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

* [PATCH v21 2/5] docs: fpga: mgr: document parse_header() callback
  2022-06-21 15:38 [PATCH v21 0/5] Microchip Polarfire FPGA manager Ivan Bornyakov
  2022-06-21 15:38 ` [PATCH v21 1/5] fpga: fpga-mgr: support bitstream offset in image buffer Ivan Bornyakov
@ 2022-06-21 15:38 ` Ivan Bornyakov
  2022-06-21 15:38 ` [PATCH v21 3/5] fpga: microchip-spi: add Microchip MPF FPGA manager Ivan Bornyakov
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Ivan Bornyakov @ 2022-06-21 15:38 UTC (permalink / raw)
  To: mdf, hao.wu, yilun.xu, trix, corbet
  Cc: Ivan Bornyakov, conor.dooley, robh+dt, krzysztof.kozlowski+dt,
	linux-fpga, linux-doc, devicetree, linux-kernel, system

Document newly introduced fpga_manager_ops callback parse_header() and
flag skip_header along with header_size and data_size fields of struct
fpga_image_info.

Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
Acked-by: Xu Yilun <yilun.xu@intel.com>
---
 Documentation/driver-api/fpga/fpga-mgr.rst | 27 +++++++++++++++++-----
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/Documentation/driver-api/fpga/fpga-mgr.rst b/Documentation/driver-api/fpga/fpga-mgr.rst
index 42c01f396dce..49c0a9512653 100644
--- a/Documentation/driver-api/fpga/fpga-mgr.rst
+++ b/Documentation/driver-api/fpga/fpga-mgr.rst
@@ -79,12 +79,27 @@ do the programming sequence for this particular FPGA.  These ops return 0 for
 success or negative error codes otherwise.
 
 The programming sequence is::
- 1. .write_init
- 2. .write or .write_sg (may be called once or multiple times)
- 3. .write_complete
-
-The .write_init function will prepare the FPGA to receive the image data.  The
-buffer passed into .write_init will be at most .initial_header_size bytes long;
+ 1. .parse_header (optional, may be called once or multiple times)
+ 2. .write_init
+ 3. .write or .write_sg (may be called once or multiple times)
+ 4. .write_complete
+
+The .parse_header function will set header_size and data_size to
+struct fpga_image_info. Before parse_header call, header_size is initialized
+with initial_header_size. If flag skip_header of fpga_manager_ops is true,
+.write function will get image buffer starting at header_size offset from the
+beginning. If data_size is set, .write function will get data_size bytes of
+the image buffer, otherwise .write will get data up to the end of image buffer.
+This will not affect .write_sg, .write_sg will still get whole image in
+sg_table form. If FPGA image is already mapped as a single contiguous buffer,
+whole buffer will be passed into .parse_header. If image is in scatter-gather
+form, core code will buffer up at least .initial_header_size before the first
+call of .parse_header, if it is not enough, .parse_header should set desired
+size into info->header_size and return -EAGAIN, then it will be called again
+with greater part of image buffer on the input.
+
+The .write_init function will prepare the FPGA to receive the image data. The
+buffer passed into .write_init will be at least info->header_size bytes long;
 if the whole bitstream is not immediately available then the core code will
 buffer up at least this much before starting.
 
-- 
2.25.1



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

* [PATCH v21 3/5] fpga: microchip-spi: add Microchip MPF FPGA manager
  2022-06-21 15:38 [PATCH v21 0/5] Microchip Polarfire FPGA manager Ivan Bornyakov
  2022-06-21 15:38 ` [PATCH v21 1/5] fpga: fpga-mgr: support bitstream offset in image buffer Ivan Bornyakov
  2022-06-21 15:38 ` [PATCH v21 2/5] docs: fpga: mgr: document parse_header() callback Ivan Bornyakov
@ 2022-06-21 15:38 ` Ivan Bornyakov
  2022-06-23  9:29   ` Xu Yilun
  2022-06-21 15:38 ` [PATCH v21 4/5] dt-bindings: fpga: add binding doc for microchip-spi fpga mgr Ivan Bornyakov
  2022-06-21 15:38 ` [PATCH v21 5/5] MAINTAINERS: add Microchip PolarFire FPGA drivers entry Ivan Bornyakov
  4 siblings, 1 reply; 12+ messages in thread
From: Ivan Bornyakov @ 2022-06-21 15:38 UTC (permalink / raw)
  To: mdf, hao.wu, yilun.xu, trix, corbet
  Cc: Ivan Bornyakov, conor.dooley, robh+dt, krzysztof.kozlowski+dt,
	linux-fpga, linux-doc, devicetree, linux-kernel, system

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>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Tested-by: Conor Dooley <conor.dooley@microchip.com>
Acked-by: Xu Yilun <yilun.xu@intel.com>
---
 drivers/fpga/Kconfig         |   8 +
 drivers/fpga/Makefile        |   1 +
 drivers/fpga/microchip-spi.c | 398 +++++++++++++++++++++++++++++++++++
 3 files changed, 407 insertions(+)
 create mode 100644 drivers/fpga/microchip-spi.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 0831eecc9a09..6c416955da53 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -255,4 +255,12 @@ config FPGA_M10_BMC_SEC_UPDATE
 	  (BMC) and provides support for secure updates for the BMC image,
 	  the FPGA image, the Root Entry Hashes, etc.
 
+config FPGA_MGR_MICROCHIP_SPI
+	tristate "Microchip Polarfire SPI FPGA manager"
+	depends on SPI
+	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 139ac1b573d3..42ae8b58abce 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..bd284c7b8dc9
--- /dev/null
+++ b/drivers/fpga/microchip-spi.c
@@ -0,0 +1,398 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Microchip Polarfire FPGA programming over slave SPI interface.
+ */
+
+#include <asm/unaligned.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_RETRIES		10000
+#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 = 0, status_command = MPF_SPI_READ_STATUS;
+	struct spi_transfer xfers[2] = { 0 };
+	int ret;
+
+	/*
+	 * HW status is returned on MISO in the first byte after CS went
+	 * active. However, first reading can be inadequate, so we submit
+	 * two identical SPI transfers and use result of the later one.
+	 */
+	xfers[0].tx_buf = &status_command;
+	xfers[1].tx_buf = &status_command;
+	xfers[0].rx_buf = &status;
+	xfers[1].rx_buf = &status;
+	xfers[0].len = 1;
+	xfers[1].len = 1;
+	xfers[0].cs_change = 1;
+
+	ret = spi_sync_transfer(spi, xfers, 2);
+
+	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 mpf_ops_parse_header(struct fpga_manager *mgr,
+				struct fpga_image_info *info,
+				const char *buf, size_t count)
+{
+	size_t component_size_byte_num, component_size_byte_off,
+	       components_size_start, bitstream_start,
+	       block_id_offset, block_start_offset;
+	u8 header_size, blocks_num, block_id;
+	u32 block_start, component_size;
+	u16 components_num, i;
+
+	if (!buf) {
+		dev_err(&mgr->dev, "Image buffer is not provided\n");
+		return -EINVAL;
+	}
+
+	header_size = *(buf + MPF_HEADER_SIZE_OFFSET);
+	if (header_size > count) {
+		info->header_size = header_size;
+		return -EAGAIN;
+	}
+
+	/*
+	 * Go through look-up table to find out where actual bitstream starts
+	 * and where sizes of components of the bitstream lies.
+	 */
+	blocks_num = *(buf + header_size - 1);
+	block_id_offset = header_size + MPF_LOOKUP_TABLE_BLOCK_ID_OFFSET;
+	block_start_offset = header_size + MPF_LOOKUP_TABLE_BLOCK_START_OFFSET;
+
+	header_size += blocks_num * MPF_LOOKUP_TABLE_RECORD_SIZE;
+	if (header_size > count) {
+		info->header_size = header_size;
+		return -EAGAIN;
+	}
+
+	components_size_start = 0;
+	bitstream_start = 0;
+
+	while (blocks_num--) {
+		block_id = *(buf + block_id_offset);
+		block_start = get_unaligned_le32(buf + block_start_offset);
+
+		switch (block_id) {
+		case MPF_BITSTREAM_ID:
+			bitstream_start = block_start;
+			info->header_size = block_start;
+			if (block_start > count)
+				return -EAGAIN;
+
+			break;
+		case MPF_COMPONENTS_SIZE_ID:
+			components_size_start = block_start;
+			break;
+		default:
+			break;
+		}
+
+		if (bitstream_start && components_size_start)
+			break;
+
+		block_id_offset += MPF_LOOKUP_TABLE_RECORD_SIZE;
+		block_start_offset += MPF_LOOKUP_TABLE_RECORD_SIZE;
+	}
+
+	if (!bitstream_start || !components_size_start) {
+		dev_err(&mgr->dev, "Failed to parse header look-up table\n");
+		return -EFAULT;
+	}
+
+	/*
+	 * Parse bitstream size.
+	 * Sizes of components of the bitstream are 22-bits long placed next
+	 * to each other. Image header should be extended by now up to where
+	 * actual bitstream starts, so no need for overflow check anymore.
+	 */
+	components_num = get_unaligned_le16(buf + MPF_DATA_SIZE_OFFSET);
+
+	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);
+
+		info->data_size += component_size * MPF_SPI_FRAME_SIZE;
+	}
+
+	return 0;
+}
+
+/* Poll HW status until busy bit is cleared and mask bits are set. */
+static int mpf_poll_status(struct spi_device *spi, u8 mask)
+{
+	int status, retries = MPF_STATUS_POLL_RETRIES;
+
+	while (retries--) {
+		status = mpf_read_status(spi);
+		if (status < 0)
+			return status;
+
+		if (status & MPF_STATUS_BUSY)
+			continue;
+
+		if (!mask || (status & mask))
+			return status;
+	}
+
+	return -EBUSY;
+}
+
+static int mpf_spi_write(struct spi_device *spi, const void *buf, size_t buf_size)
+{
+	int status = mpf_poll_status(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 = mpf_poll_status(spi, MPF_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 program_mode[] = { MPF_SPI_FRAME_INIT, MPF_SPI_PRG_MODE };
+	const u8 isc_en_command[] = { MPF_SPI_ISC_ENABLE };
+	struct mpf_priv *priv = mgr->priv;
+	struct device *dev = &mgr->dev;
+	struct spi_device *spi;
+	u32 isc_ret = 0;
+	int ret;
+
+	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
+		dev_err(dev, "Partial reconfiguration is not supported\n");
+		return -EOPNOTSUPP;
+	}
+
+	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: spi_ret %d, isc_ret %u\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 spi_frame_command[] = { MPF_SPI_FRAME };
+	struct spi_transfer xfers[2] = { 0 };
+	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;
+
+	xfers[0].tx_buf = spi_frame_command;
+	xfers[0].len = sizeof(spi_frame_command);
+
+	for (i = 0; i < count / MPF_SPI_FRAME_SIZE; i++) {
+		xfers[1].tx_buf = buf + i * MPF_SPI_FRAME_SIZE;
+		xfers[1].len = MPF_SPI_FRAME_SIZE;
+
+		ret = mpf_poll_status(spi, 0);
+		if (ret >= 0)
+			ret = spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));
+
+		if (ret) {
+			dev_err(dev, "Failed to write bitstream frame %d/%zu\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,
+	.initial_header_size = 71,
+	.skip_header = true,
+	.parse_header = mpf_ops_parse_header,
+	.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



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

* [PATCH v21 4/5] dt-bindings: fpga: add binding doc for microchip-spi fpga mgr
  2022-06-21 15:38 [PATCH v21 0/5] Microchip Polarfire FPGA manager Ivan Bornyakov
                   ` (2 preceding siblings ...)
  2022-06-21 15:38 ` [PATCH v21 3/5] fpga: microchip-spi: add Microchip MPF FPGA manager Ivan Bornyakov
@ 2022-06-21 15:38 ` Ivan Bornyakov
  2022-06-21 15:38 ` [PATCH v21 5/5] MAINTAINERS: add Microchip PolarFire FPGA drivers entry Ivan Bornyakov
  4 siblings, 0 replies; 12+ messages in thread
From: Ivan Bornyakov @ 2022-06-21 15:38 UTC (permalink / raw)
  To: mdf, hao.wu, yilun.xu, trix, corbet
  Cc: Ivan Bornyakov, conor.dooley, robh+dt, krzysztof.kozlowski+dt,
	linux-fpga, linux-doc, devicetree, linux-kernel, system,
	Rob Herring

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>
Acked-by: Xu Yilun <yilun.xu@intel.com>
---
 .../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



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

* [PATCH v21 5/5] MAINTAINERS: add Microchip PolarFire FPGA drivers entry
  2022-06-21 15:38 [PATCH v21 0/5] Microchip Polarfire FPGA manager Ivan Bornyakov
                   ` (3 preceding siblings ...)
  2022-06-21 15:38 ` [PATCH v21 4/5] dt-bindings: fpga: add binding doc for microchip-spi fpga mgr Ivan Bornyakov
@ 2022-06-21 15:38 ` Ivan Bornyakov
  2022-06-21 15:48   ` Conor.Dooley
  2022-06-23  9:32   ` Xu Yilun
  4 siblings, 2 replies; 12+ messages in thread
From: Ivan Bornyakov @ 2022-06-21 15:38 UTC (permalink / raw)
  To: mdf, hao.wu, yilun.xu, trix, corbet
  Cc: Ivan Bornyakov, conor.dooley, robh+dt, krzysztof.kozlowski+dt,
	linux-fpga, linux-doc, devicetree, linux-kernel, system

Add MAINTAINERS entry for the newly added Microchip PolarFire (MPF) FPGA
manager. Add myself as a reviewer and Conor Dooley as a maintainer.

Suggested-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b774f21828f7..ed2517574804 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7884,6 +7884,14 @@ S:	Maintained
 F:	Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-sec-update
 F:	drivers/fpga/intel-m10-bmc-sec-update.c
 
+MICROCHIP POLARFIRE FPGA DRIVERS
+M:	Conor Dooley <conor.dooley@microchip.com>
+R:	Ivan Bornyakov <i.bornyakov@metrotek.ru>
+L:	linux-fpga@vger.kernel.org
+S:	Supported
+F:	Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml
+F:	drivers/fpga/microchip-spi.c
+
 FPU EMULATOR
 M:	Bill Metzenthen <billm@melbpc.org.au>
 S:	Maintained
-- 
2.25.1



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

* Re: [PATCH v21 5/5] MAINTAINERS: add Microchip PolarFire FPGA drivers entry
  2022-06-21 15:38 ` [PATCH v21 5/5] MAINTAINERS: add Microchip PolarFire FPGA drivers entry Ivan Bornyakov
@ 2022-06-21 15:48   ` Conor.Dooley
  2022-06-23  9:32   ` Xu Yilun
  1 sibling, 0 replies; 12+ messages in thread
From: Conor.Dooley @ 2022-06-21 15:48 UTC (permalink / raw)
  To: i.bornyakov, mdf, hao.wu, yilun.xu, trix, corbet
  Cc: Conor.Dooley, robh+dt, krzysztof.kozlowski+dt, linux-fpga,
	linux-doc, devicetree, linux-kernel, system



On 21/06/2022 16:38, Ivan Bornyakov wrote:
> Add MAINTAINERS entry for the newly added Microchip PolarFire (MPF) FPGA
> manager. Add myself as a reviewer and Conor Dooley as a maintainer.
> 
> Suggested-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
> ---
>  MAINTAINERS | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b774f21828f7..ed2517574804 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7884,6 +7884,14 @@ S:	Maintained
>  F:	Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-sec-update
>  F:	drivers/fpga/intel-m10-bmc-sec-update.c
>  
> +MICROCHIP POLARFIRE FPGA DRIVERS
> +M:	Conor Dooley <conor.dooley@microchip.com>
> +R:	Ivan Bornyakov <i.bornyakov@metrotek.ru>
> +L:	linux-fpga@vger.kernel.org
> +S:	Supported
> +F:	Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml
> +F:	drivers/fpga/microchip-spi.c
> +
>  FPU EMULATOR
>  M:	Bill Metzenthen <billm@melbpc.org.au>
>  S:	Maintained

Acked-by: Conor Dooley <conor.dooley@microchip.com>

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

* Re: [PATCH v21 1/5] fpga: fpga-mgr: support bitstream offset in image buffer
  2022-06-21 15:38 ` [PATCH v21 1/5] fpga: fpga-mgr: support bitstream offset in image buffer Ivan Bornyakov
@ 2022-06-23  9:28   ` Xu Yilun
  0 siblings, 0 replies; 12+ messages in thread
From: Xu Yilun @ 2022-06-23  9:28 UTC (permalink / raw)
  To: Ivan Bornyakov
  Cc: mdf, hao.wu, trix, corbet, conor.dooley, robh+dt,
	krzysztof.kozlowski+dt, linux-fpga, linux-doc, devicetree,
	linux-kernel, system

On Tue, Jun 21, 2022 at 06:38:43PM +0300, Ivan Bornyakov wrote:
> At the moment FPGA manager core loads to the device entire image
> provided to fpga_mgr_load(). But it is not always whole FPGA image
> buffer meant to be written to the device. In particular, .dat formatted
> image for Microchip MPF contains meta info in the header that is not
> meant to be written to the device. This is issue for those low level
> drivers that loads data to the device with write() fpga_manager_ops
> callback, since write() can be called in iterator over scatter-gather
> table, not only linear image buffer. On the other hand, write_sg()
> callback is provided with whole image in scatter-gather form and can
> decide itself which part should be sent to the device.
> 
> Add header_size and data_size to the fpga_image_info struct, add
> skip_header to the fpga_manager_ops struct and adjust fpga_mgr_write()
> callers with respect to them.
> 
>   * info->header_size indicates part at the beginning of image buffer
>     that contains some meta info. It is optional and can be 0,
>     initialized with mops->initial_header_size.
> 
>   * mops->skip_header tells fpga-mgr core whether write should start
>     from the beginning of image buffer or at the offset of header_size.
> 
>   * info->data_size is the size of bitstream data that is meant to be
>     written to the device. It is also optional and can be 0, which
>     means bitstream data is up to the end of image buffer.
> 
> Also add parse_header() callback to fpga_manager_ops, which purpose is
> to set info->header_size and info->data_size. At least
> initial_header_size bytes of image buffer will be passed into
> parse_header() first time. If it is not enough, parse_header() should
> set desired size into info->header_size and return -EAGAIN, then it will
> be called again with greater part of image buffer on the input.
> 
> Suggested-by: Xu Yilun <yilun.xu@intel.com>
> Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
> ---
>  drivers/fpga/fpga-mgr.c       | 225 ++++++++++++++++++++++++++++++----
>  include/linux/fpga/fpga-mgr.h |  24 +++-
>  2 files changed, 222 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> index 08dc85fcd511..d97df0bdc72b 100644
> --- a/drivers/fpga/fpga-mgr.c
> +++ b/drivers/fpga/fpga-mgr.c
> @@ -74,6 +74,15 @@ static inline int fpga_mgr_write_complete(struct fpga_manager *mgr,
>  	return 0;
>  }
>  
> +static inline int fpga_mgr_parse_header(struct fpga_manager *mgr,
> +					struct fpga_image_info *info,
> +					const char *buf, size_t count)
> +{
> +	if (mgr->mops->parse_header)
> +		return mgr->mops->parse_header(mgr, info, buf, count);
> +	return 0;
> +}
> +
>  static inline int fpga_mgr_write_init(struct fpga_manager *mgr,
>  				      struct fpga_image_info *info,
>  				      const char *buf, size_t count)
> @@ -136,24 +145,141 @@ void fpga_image_info_free(struct fpga_image_info *info)
>  EXPORT_SYMBOL_GPL(fpga_image_info_free);
>  
>  /*
> - * Call the low level driver's write_init function.  This will do the
> + * Call the low level driver's parse_header function with entire FPGA image
> + * buffer on the input. This will set info->header_size and info->data_size.
> + */
> +static int fpga_mgr_parse_header_mapped(struct fpga_manager *mgr,
> +					struct fpga_image_info *info,
> +					const char *buf, size_t count)
> +{
> +	int ret;
> +
> +	mgr->state = FPGA_MGR_STATE_PARSE_HEADER;
> +	ret = fpga_mgr_parse_header(mgr, info, buf, count);
> +
> +	if (info->header_size + info->data_size > count) {
> +		dev_err(&mgr->dev, "Bitsream data outruns FPGA image\n");
> +		ret = -EINVAL;
> +	}
> +
> +	if (ret) {
> +		dev_err(&mgr->dev, "Error while parsing FPGA image header\n");
> +		mgr->state = FPGA_MGR_STATE_PARSE_HEADER_ERR;
> +	}
> +
> +	return ret;
> +}
> +
> +/*
> + * Call the low level driver's parse_header function with first fragment of
> + * scattered FPGA image on the input. If header fits first fragment,
> + * parse_header will set info->header_size and info->data_size. If it is not,
> + * parse_header will set desired size to info->header_size and -EAGAIN will be
> + * returned.
> + */
> +static int fpga_mgr_parse_header_sg_first(struct fpga_manager *mgr,
> +					  struct fpga_image_info *info,
> +					  struct sg_table *sgt)
> +{
> +	struct sg_mapping_iter miter;
> +	int ret;
> +
> +	mgr->state = FPGA_MGR_STATE_PARSE_HEADER;
> +
> +	sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
> +	if (sg_miter_next(&miter) &&
> +	    miter.length >= info->header_size)
> +		ret = fpga_mgr_parse_header(mgr, info, miter.addr, miter.length);
> +	else
> +		ret = -EAGAIN;
> +	sg_miter_stop(&miter);
> +
> +	if (ret && ret != -EAGAIN) {
> +		dev_err(&mgr->dev, "Error while parsing FPGA image header\n");
> +		mgr->state = FPGA_MGR_STATE_PARSE_HEADER_ERR;
> +	}
> +
> +	return ret;
> +}
> +
> +/*
> + * Copy scattered FPGA image fragments to temporary buffer and call the
> + * low level driver's parse_header function. This should be called after
> + * fpga_mgr_parse_header_sg_first() returned -EAGAIN. In case of success,
> + * pointer to the newly allocated image header copy will be returned and
> + * its size will be set into *ret_size. Returned buffer needs to be freed.
> + */
> +static void *fpga_mgr_parse_header_sg(struct fpga_manager *mgr,
> +				      struct fpga_image_info *info,
> +				      struct sg_table *sgt, size_t *ret_size)
> +{
> +	size_t len, new_header_size, header_size = 0;
> +	char *new_buf, *buf = NULL;
> +	int ret;
> +
> +	do {
> +		new_header_size = info->header_size;
> +		if (new_header_size <= header_size) {
> +			dev_err(&mgr->dev, "Requested invalid header size\n");
> +			ret = -EFAULT;
> +			break;
> +		}
> +
> +		new_buf = krealloc(buf, new_header_size, GFP_KERNEL);
> +		if (!new_buf) {
> +			ret = -ENOMEM;
> +			break;
> +		}
> +
> +		buf = new_buf;
> +
> +		len = sg_pcopy_to_buffer(sgt->sgl, sgt->nents,
> +					 buf + header_size,
> +					 new_header_size - header_size,
> +					 header_size);
> +		if (len != new_header_size - header_size) {
> +			ret = -EFAULT;
> +			break;
> +		}
> +
> +		header_size = new_header_size;
> +		ret = fpga_mgr_parse_header(mgr, info, buf, header_size);
> +	} while (ret == -EAGAIN);
> +
> +	if (ret) {
> +		dev_err(&mgr->dev, "Error while parsing FPGA image header\n");
> +		mgr->state = FPGA_MGR_STATE_PARSE_HEADER_ERR;
> +		kfree(buf);
> +		buf = ERR_PTR(ret);
> +	}
> +
> +	*ret_size = header_size;
> +
> +	return buf;
> +}
> +
> +/*
> + * 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.
> + * receive an FPGA image. The low level driver gets to see at least first
> + * info->header_size bytes in the buffer. If info->header_size is 0,
> + * write_init will not get any bytes of image buffer.
>   */
>  static int fpga_mgr_write_init_buf(struct fpga_manager *mgr,
>  				   struct fpga_image_info *info,
>  				   const char *buf, size_t count)
>  {
> +	size_t header_size = info->header_size;
>  	int ret;
>  
>  	mgr->state = FPGA_MGR_STATE_WRITE_INIT;
> -	if (!mgr->mops->initial_header_size) {
> +
> +	if (header_size > count)
> +		ret = -EINVAL;
> +	else if (!header_size)
>  		ret = fpga_mgr_write_init(mgr, info, NULL, 0);
> -	} else {
> -		count = min(mgr->mops->initial_header_size, count);
> +	else
>  		ret = fpga_mgr_write_init(mgr, info, buf, count);
> -	}
>  
>  	if (ret) {
>  		dev_err(&mgr->dev, "Error preparing FPGA for writing\n");
> @@ -164,39 +290,50 @@ static int fpga_mgr_write_init_buf(struct fpga_manager *mgr,
>  	return 0;
>  }
>  
> -static int fpga_mgr_write_init_sg(struct fpga_manager *mgr,
> -				  struct fpga_image_info *info,
> -				  struct sg_table *sgt)
> +static int fpga_mgr_prepare_sg(struct fpga_manager *mgr,
> +			       struct fpga_image_info *info,
> +			       struct sg_table *sgt)
>  {
>  	struct sg_mapping_iter miter;
>  	size_t len;
>  	char *buf;
>  	int ret;
>  
> -	if (!mgr->mops->initial_header_size)
> +	/* Short path. Low level driver don't care about image header. */
> +	if (!mgr->mops->initial_header_size && !mgr->mops->parse_header)
>  		return fpga_mgr_write_init_buf(mgr, info, NULL, 0);
>  
>  	/*
>  	 * First try to use miter to map the first fragment to access the
>  	 * header, this is the typical path.
>  	 */
> -	sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
> -	if (sg_miter_next(&miter) &&
> -	    miter.length >= mgr->mops->initial_header_size) {
> -		ret = fpga_mgr_write_init_buf(mgr, info, miter.addr,
> -					      miter.length);
> +	ret = fpga_mgr_parse_header_sg_first(mgr, info, sgt);
> +	/* If 0, header fits first fragment, call write_init on it */
> +	if (!ret) {
> +		sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
> +		if (sg_miter_next(&miter)) {
> +			ret = fpga_mgr_write_init_buf(mgr, info, miter.addr,
> +						      miter.length);
> +			sg_miter_stop(&miter);
> +			return ret;
> +		}
>  		sg_miter_stop(&miter);
> +	/*
> +	 * If -EAGAIN, more sg buffer is needed,
> +	 * otherwise an error has occurred.
> +	 */
> +	} else if (ret != -EAGAIN) {
>  		return ret;
>  	}
> -	sg_miter_stop(&miter);
>  
> -	/* Otherwise copy the fragments into temporary memory. */
> -	buf = kmalloc(mgr->mops->initial_header_size, GFP_KERNEL);
> -	if (!buf)
> -		return -ENOMEM;
> +	/*
> +	 * Copy the fragments into temporary memory.
> +	 * Copying is done inside fpga_mgr_parse_header_sg().
> +	 */
> +	buf = fpga_mgr_parse_header_sg(mgr, info, sgt, &len);
> +	if (IS_ERR(buf))
> +		return PTR_ERR(buf);
>  
> -	len = sg_copy_to_buffer(sgt->sgl, sgt->nents, buf,
> -				mgr->mops->initial_header_size);
>  	ret = fpga_mgr_write_init_buf(mgr, info, buf, len);
>  
>  	kfree(buf);
> @@ -227,7 +364,7 @@ static int fpga_mgr_buf_load_sg(struct fpga_manager *mgr,
>  {
>  	int ret;
>  
> -	ret = fpga_mgr_write_init_sg(mgr, info, sgt);
> +	ret = fpga_mgr_prepare_sg(mgr, info, sgt);
>  	if (ret)
>  		return ret;
>  
> @@ -236,17 +373,37 @@ 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 length, data_size = info->data_size;
>  		struct sg_mapping_iter miter;
> +		ssize_t count = 0;
>  
>  		sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
> +
> +		if (mgr->mops->skip_header &&
> +		    !sg_miter_skip(&miter, info->header_size)) {
> +			ret = -EINVAL;
> +			sg_miter_stop(&miter);

Seems sg_miter_stop is not need if sg_miter_skip reaches end, is it?

Thanks,
Yilun

> +			goto out;
> +		}
> +
>  		while (sg_miter_next(&miter)) {
> -			ret = fpga_mgr_write(mgr, miter.addr, miter.length);
> +			if (data_size)
> +				length = min(miter.length, data_size - count);
> +			else
> +				length = miter.length;
> +
> +			ret = fpga_mgr_write(mgr, miter.addr, length);
>  			if (ret)
>  				break;
> +
> +			count += length;
> +			if (data_size && count >= data_size)
> +				break;
>  		}
>  		sg_miter_stop(&miter);
>  	}
>  
> +out:
>  	if (ret) {
>  		dev_err(&mgr->dev, "Error while writing image data to FPGA\n");
>  		mgr->state = FPGA_MGR_STATE_WRITE_ERR;
> @@ -262,10 +419,22 @@ static int fpga_mgr_buf_load_mapped(struct fpga_manager *mgr,
>  {
>  	int ret;
>  
> +	ret = fpga_mgr_parse_header_mapped(mgr, info, buf, count);
> +	if (ret)
> +		return ret;
> +
>  	ret = fpga_mgr_write_init_buf(mgr, info, buf, count);
>  	if (ret)
>  		return ret;
>  
> +	if (mgr->mops->skip_header) {
> +		buf += info->header_size;
> +		count -= info->header_size;
> +	}
> +
> +	if (info->data_size)
> +		count = info->data_size;
> +
>  	/*
>  	 * Write the FPGA image to the FPGA.
>  	 */
> @@ -404,6 +573,8 @@ static int fpga_mgr_firmware_load(struct fpga_manager *mgr,
>   */
>  int fpga_mgr_load(struct fpga_manager *mgr, struct fpga_image_info *info)
>  {
> +	info->header_size = mgr->mops->initial_header_size;
> +
>  	if (info->sgt)
>  		return fpga_mgr_buf_load_sg(mgr, info, info->sgt);
>  	if (info->buf && info->count)
> @@ -424,6 +595,10 @@ static const char * const state_str[] = {
>  	[FPGA_MGR_STATE_FIRMWARE_REQ] =		"firmware request",
>  	[FPGA_MGR_STATE_FIRMWARE_REQ_ERR] =	"firmware request error",
>  
> +	/* Parse FPGA image header */
> +	[FPGA_MGR_STATE_PARSE_HEADER] =		"parse header",
> +	[FPGA_MGR_STATE_PARSE_HEADER_ERR] =	"parse header error",
> +
>  	/* Preparing FPGA to receive image */
>  	[FPGA_MGR_STATE_WRITE_INIT] =		"write init",
>  	[FPGA_MGR_STATE_WRITE_INIT_ERR] =	"write init error",
> diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
> index 0f9468771bb9..54f63459efd6 100644
> --- a/include/linux/fpga/fpga-mgr.h
> +++ b/include/linux/fpga/fpga-mgr.h
> @@ -22,6 +22,8 @@ struct sg_table;
>   * @FPGA_MGR_STATE_RESET: FPGA in reset state
>   * @FPGA_MGR_STATE_FIRMWARE_REQ: firmware request in progress
>   * @FPGA_MGR_STATE_FIRMWARE_REQ_ERR: firmware request failed
> + * @FPGA_MGR_STATE_PARSE_HEADER: parse FPGA image header
> + * @FPGA_MGR_STATE_PARSE_HEADER_ERR: Error during PARSE_HEADER stage
>   * @FPGA_MGR_STATE_WRITE_INIT: preparing FPGA for programming
>   * @FPGA_MGR_STATE_WRITE_INIT_ERR: Error during WRITE_INIT stage
>   * @FPGA_MGR_STATE_WRITE: writing image to FPGA
> @@ -41,7 +43,9 @@ enum fpga_mgr_states {
>  	FPGA_MGR_STATE_FIRMWARE_REQ,
>  	FPGA_MGR_STATE_FIRMWARE_REQ_ERR,
>  
> -	/* write sequence: init, write, complete */
> +	/* write sequence: parse header, init, write, complete */
> +	FPGA_MGR_STATE_PARSE_HEADER,
> +	FPGA_MGR_STATE_PARSE_HEADER_ERR,
>  	FPGA_MGR_STATE_WRITE_INIT,
>  	FPGA_MGR_STATE_WRITE_INIT_ERR,
>  	FPGA_MGR_STATE_WRITE,
> @@ -85,6 +89,9 @@ enum fpga_mgr_states {
>   * @sgt: scatter/gather table containing FPGA image
>   * @buf: contiguous buffer containing FPGA image
>   * @count: size of buf
> + * @header_size: size of image header.
> + * @data_size: size of image data to be sent to the device. If not specified,
> + *	whole image will be used. Header may be skipped in either case.
>   * @region_id: id of target region
>   * @dev: device that owns this
>   * @overlay: Device Tree overlay
> @@ -98,6 +105,8 @@ struct fpga_image_info {
>  	struct sg_table *sgt;
>  	const char *buf;
>  	size_t count;
> +	size_t header_size;
> +	size_t data_size;
>  	int region_id;
>  	struct device *dev;
>  #ifdef CONFIG_OF
> @@ -137,9 +146,16 @@ struct fpga_manager_info {
>  
>  /**
>   * struct fpga_manager_ops - ops for low level fpga manager drivers
> - * @initial_header_size: Maximum number of bytes that should be passed into write_init
> + * @initial_header_size: minimum number of bytes that should be passed into
> + *	parse_header and write_init.
> + * @skip_header: bool flag to tell fpga-mgr core whether it should skip
> + *	info->header_size part at the beginning of the image when invoking
> + *	write callback.
>   * @state: returns an enum value of the FPGA's state
>   * @status: returns status of the FPGA, including reconfiguration error code
> + * @parse_header: parse FPGA image header to set info->header_size and
> + *	info->data_size. In case the input buffer is not large enough, set
> + *	required size to info->header_size and return -EAGAIN.
>   * @write_init: prepare the FPGA to receive configuration data
>   * @write: write count bytes of configuration data to the FPGA
>   * @write_sg: write the scatter list of configuration data to the FPGA
> @@ -153,8 +169,12 @@ struct fpga_manager_info {
>   */
>  struct fpga_manager_ops {
>  	size_t initial_header_size;
> +	bool skip_header;
>  	enum fpga_mgr_states (*state)(struct fpga_manager *mgr);
>  	u64 (*status)(struct fpga_manager *mgr);
> +	int (*parse_header)(struct fpga_manager *mgr,
> +			    struct fpga_image_info *info,
> +			    const char *buf, size_t count);
>  	int (*write_init)(struct fpga_manager *mgr,
>  			  struct fpga_image_info *info,
>  			  const char *buf, size_t count);
> -- 
> 2.25.1
> 

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

* Re: [PATCH v21 3/5] fpga: microchip-spi: add Microchip MPF FPGA manager
  2022-06-21 15:38 ` [PATCH v21 3/5] fpga: microchip-spi: add Microchip MPF FPGA manager Ivan Bornyakov
@ 2022-06-23  9:29   ` Xu Yilun
  2022-06-23  9:48     ` Ivan Bornyakov
  0 siblings, 1 reply; 12+ messages in thread
From: Xu Yilun @ 2022-06-23  9:29 UTC (permalink / raw)
  To: Ivan Bornyakov
  Cc: mdf, hao.wu, trix, corbet, conor.dooley, robh+dt,
	krzysztof.kozlowski+dt, linux-fpga, linux-doc, devicetree,
	linux-kernel, system

On Tue, Jun 21, 2022 at 06:38:45PM +0300, Ivan Bornyakov wrote:
> 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>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Tested-by: Conor Dooley <conor.dooley@microchip.com>
> Acked-by: Xu Yilun <yilun.xu@intel.com>
> ---
>  drivers/fpga/Kconfig         |   8 +
>  drivers/fpga/Makefile        |   1 +
>  drivers/fpga/microchip-spi.c | 398 +++++++++++++++++++++++++++++++++++
>  3 files changed, 407 insertions(+)
>  create mode 100644 drivers/fpga/microchip-spi.c
> 
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index 0831eecc9a09..6c416955da53 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -255,4 +255,12 @@ config FPGA_M10_BMC_SEC_UPDATE
>  	  (BMC) and provides support for secure updates for the BMC image,
>  	  the FPGA image, the Root Entry Hashes, etc.
>  
> +config FPGA_MGR_MICROCHIP_SPI
> +	tristate "Microchip Polarfire SPI FPGA manager"
> +	depends on SPI
> +	help
> +	  FPGA manager driver support for Microchip Polarfire FPGAs
> +	  programming over slave SPI interface with .dat formatted
> +	  bitstream image.

I still see the checkpatch warning here, please help fix it.

WARNING: please write a help paragraph that fully describes the config symbol

Thanks,
Yilun

> +
>  endif # FPGA
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 139ac1b573d3..42ae8b58abce 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..bd284c7b8dc9
> --- /dev/null
> +++ b/drivers/fpga/microchip-spi.c
> @@ -0,0 +1,398 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Microchip Polarfire FPGA programming over slave SPI interface.
> + */
> +
> +#include <asm/unaligned.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_RETRIES		10000
> +#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 = 0, status_command = MPF_SPI_READ_STATUS;
> +	struct spi_transfer xfers[2] = { 0 };
> +	int ret;
> +
> +	/*
> +	 * HW status is returned on MISO in the first byte after CS went
> +	 * active. However, first reading can be inadequate, so we submit
> +	 * two identical SPI transfers and use result of the later one.
> +	 */
> +	xfers[0].tx_buf = &status_command;
> +	xfers[1].tx_buf = &status_command;
> +	xfers[0].rx_buf = &status;
> +	xfers[1].rx_buf = &status;
> +	xfers[0].len = 1;
> +	xfers[1].len = 1;
> +	xfers[0].cs_change = 1;
> +
> +	ret = spi_sync_transfer(spi, xfers, 2);
> +
> +	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 mpf_ops_parse_header(struct fpga_manager *mgr,
> +				struct fpga_image_info *info,
> +				const char *buf, size_t count)
> +{
> +	size_t component_size_byte_num, component_size_byte_off,
> +	       components_size_start, bitstream_start,
> +	       block_id_offset, block_start_offset;
> +	u8 header_size, blocks_num, block_id;
> +	u32 block_start, component_size;
> +	u16 components_num, i;
> +
> +	if (!buf) {
> +		dev_err(&mgr->dev, "Image buffer is not provided\n");
> +		return -EINVAL;
> +	}
> +
> +	header_size = *(buf + MPF_HEADER_SIZE_OFFSET);
> +	if (header_size > count) {
> +		info->header_size = header_size;
> +		return -EAGAIN;
> +	}
> +
> +	/*
> +	 * Go through look-up table to find out where actual bitstream starts
> +	 * and where sizes of components of the bitstream lies.
> +	 */
> +	blocks_num = *(buf + header_size - 1);
> +	block_id_offset = header_size + MPF_LOOKUP_TABLE_BLOCK_ID_OFFSET;
> +	block_start_offset = header_size + MPF_LOOKUP_TABLE_BLOCK_START_OFFSET;
> +
> +	header_size += blocks_num * MPF_LOOKUP_TABLE_RECORD_SIZE;
> +	if (header_size > count) {
> +		info->header_size = header_size;
> +		return -EAGAIN;
> +	}
> +
> +	components_size_start = 0;
> +	bitstream_start = 0;
> +
> +	while (blocks_num--) {
> +		block_id = *(buf + block_id_offset);
> +		block_start = get_unaligned_le32(buf + block_start_offset);
> +
> +		switch (block_id) {
> +		case MPF_BITSTREAM_ID:
> +			bitstream_start = block_start;
> +			info->header_size = block_start;
> +			if (block_start > count)
> +				return -EAGAIN;
> +
> +			break;
> +		case MPF_COMPONENTS_SIZE_ID:
> +			components_size_start = block_start;
> +			break;
> +		default:
> +			break;
> +		}
> +
> +		if (bitstream_start && components_size_start)
> +			break;
> +
> +		block_id_offset += MPF_LOOKUP_TABLE_RECORD_SIZE;
> +		block_start_offset += MPF_LOOKUP_TABLE_RECORD_SIZE;
> +	}
> +
> +	if (!bitstream_start || !components_size_start) {
> +		dev_err(&mgr->dev, "Failed to parse header look-up table\n");
> +		return -EFAULT;
> +	}
> +
> +	/*
> +	 * Parse bitstream size.
> +	 * Sizes of components of the bitstream are 22-bits long placed next
> +	 * to each other. Image header should be extended by now up to where
> +	 * actual bitstream starts, so no need for overflow check anymore.
> +	 */
> +	components_num = get_unaligned_le16(buf + MPF_DATA_SIZE_OFFSET);
> +
> +	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);
> +
> +		info->data_size += component_size * MPF_SPI_FRAME_SIZE;
> +	}
> +
> +	return 0;
> +}
> +
> +/* Poll HW status until busy bit is cleared and mask bits are set. */
> +static int mpf_poll_status(struct spi_device *spi, u8 mask)
> +{
> +	int status, retries = MPF_STATUS_POLL_RETRIES;
> +
> +	while (retries--) {
> +		status = mpf_read_status(spi);
> +		if (status < 0)
> +			return status;
> +
> +		if (status & MPF_STATUS_BUSY)
> +			continue;
> +
> +		if (!mask || (status & mask))
> +			return status;
> +	}
> +
> +	return -EBUSY;
> +}
> +
> +static int mpf_spi_write(struct spi_device *spi, const void *buf, size_t buf_size)
> +{
> +	int status = mpf_poll_status(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 = mpf_poll_status(spi, MPF_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 program_mode[] = { MPF_SPI_FRAME_INIT, MPF_SPI_PRG_MODE };
> +	const u8 isc_en_command[] = { MPF_SPI_ISC_ENABLE };
> +	struct mpf_priv *priv = mgr->priv;
> +	struct device *dev = &mgr->dev;
> +	struct spi_device *spi;
> +	u32 isc_ret = 0;
> +	int ret;
> +
> +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> +		dev_err(dev, "Partial reconfiguration is not supported\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	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: spi_ret %d, isc_ret %u\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 spi_frame_command[] = { MPF_SPI_FRAME };
> +	struct spi_transfer xfers[2] = { 0 };
> +	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;
> +
> +	xfers[0].tx_buf = spi_frame_command;
> +	xfers[0].len = sizeof(spi_frame_command);
> +
> +	for (i = 0; i < count / MPF_SPI_FRAME_SIZE; i++) {
> +		xfers[1].tx_buf = buf + i * MPF_SPI_FRAME_SIZE;
> +		xfers[1].len = MPF_SPI_FRAME_SIZE;
> +
> +		ret = mpf_poll_status(spi, 0);
> +		if (ret >= 0)
> +			ret = spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));
> +
> +		if (ret) {
> +			dev_err(dev, "Failed to write bitstream frame %d/%zu\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,
> +	.initial_header_size = 71,
> +	.skip_header = true,
> +	.parse_header = mpf_ops_parse_header,
> +	.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
> 

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

* Re: [PATCH v21 5/5] MAINTAINERS: add Microchip PolarFire FPGA drivers entry
  2022-06-21 15:38 ` [PATCH v21 5/5] MAINTAINERS: add Microchip PolarFire FPGA drivers entry Ivan Bornyakov
  2022-06-21 15:48   ` Conor.Dooley
@ 2022-06-23  9:32   ` Xu Yilun
  1 sibling, 0 replies; 12+ messages in thread
From: Xu Yilun @ 2022-06-23  9:32 UTC (permalink / raw)
  To: Ivan Bornyakov
  Cc: mdf, hao.wu, trix, corbet, conor.dooley, robh+dt,
	krzysztof.kozlowski+dt, linux-fpga, linux-doc, devicetree,
	linux-kernel, system

On Tue, Jun 21, 2022 at 06:38:47PM +0300, Ivan Bornyakov wrote:
> Add MAINTAINERS entry for the newly added Microchip PolarFire (MPF) FPGA
> manager. Add myself as a reviewer and Conor Dooley as a maintainer.
> 
> Suggested-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>

Acked-by: Xu Yilun <yilun.xu@intel.com>

> ---
>  MAINTAINERS | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b774f21828f7..ed2517574804 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7884,6 +7884,14 @@ S:	Maintained
>  F:	Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-sec-update
>  F:	drivers/fpga/intel-m10-bmc-sec-update.c
>  
> +MICROCHIP POLARFIRE FPGA DRIVERS
> +M:	Conor Dooley <conor.dooley@microchip.com>
> +R:	Ivan Bornyakov <i.bornyakov@metrotek.ru>
> +L:	linux-fpga@vger.kernel.org
> +S:	Supported
> +F:	Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml
> +F:	drivers/fpga/microchip-spi.c
> +
>  FPU EMULATOR
>  M:	Bill Metzenthen <billm@melbpc.org.au>
>  S:	Maintained
> -- 
> 2.25.1
> 

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

* Re: [PATCH v21 3/5] fpga: microchip-spi: add Microchip MPF FPGA manager
  2022-06-23  9:29   ` Xu Yilun
@ 2022-06-23  9:48     ` Ivan Bornyakov
  2022-06-23  9:50       ` Xu Yilun
  0 siblings, 1 reply; 12+ messages in thread
From: Ivan Bornyakov @ 2022-06-23  9:48 UTC (permalink / raw)
  To: Xu Yilun
  Cc: mdf, hao.wu, trix, corbet, conor.dooley, robh+dt,
	krzysztof.kozlowski+dt, linux-fpga, linux-doc, devicetree,
	linux-kernel, system

On Thu, Jun 23, 2022 at 05:29:39PM +0800, Xu Yilun wrote:
> On Tue, Jun 21, 2022 at 06:38:45PM +0300, Ivan Bornyakov wrote:
> > 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>
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > Tested-by: Conor Dooley <conor.dooley@microchip.com>
> > Acked-by: Xu Yilun <yilun.xu@intel.com>
> > ---
> >  drivers/fpga/Kconfig         |   8 +
> >  drivers/fpga/Makefile        |   1 +
> >  drivers/fpga/microchip-spi.c | 398 +++++++++++++++++++++++++++++++++++
> >  3 files changed, 407 insertions(+)
> >  create mode 100644 drivers/fpga/microchip-spi.c
> > 
> > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > index 0831eecc9a09..6c416955da53 100644
> > --- a/drivers/fpga/Kconfig
> > +++ b/drivers/fpga/Kconfig
> > @@ -255,4 +255,12 @@ config FPGA_M10_BMC_SEC_UPDATE
> >  	  (BMC) and provides support for secure updates for the BMC image,
> >  	  the FPGA image, the Root Entry Hashes, etc.
> >  
> > +config FPGA_MGR_MICROCHIP_SPI
> > +	tristate "Microchip Polarfire SPI FPGA manager"
> > +	depends on SPI
> > +	help
> > +	  FPGA manager driver support for Microchip Polarfire FPGAs
> > +	  programming over slave SPI interface with .dat formatted
> > +	  bitstream image.
> 
> I still see the checkpatch warning here, please help fix it.
> 
> WARNING: please write a help paragraph that fully describes the config symbol
> 

Clearly there is a help paragraph. The reason why checkpatch warns here
is that help paragrap is of 3 lines long while minimum requirement is 4
lines. But does it really matter?

> Thanks,
> Yilun
> 
> > +
> >  endif # FPGA
> > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> > index 139ac1b573d3..42ae8b58abce 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..bd284c7b8dc9
> > --- /dev/null
> > +++ b/drivers/fpga/microchip-spi.c
> > @@ -0,0 +1,398 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Microchip Polarfire FPGA programming over slave SPI interface.
> > + */
> > +
> > +#include <asm/unaligned.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_RETRIES		10000
> > +#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 = 0, status_command = MPF_SPI_READ_STATUS;
> > +	struct spi_transfer xfers[2] = { 0 };
> > +	int ret;
> > +
> > +	/*
> > +	 * HW status is returned on MISO in the first byte after CS went
> > +	 * active. However, first reading can be inadequate, so we submit
> > +	 * two identical SPI transfers and use result of the later one.
> > +	 */
> > +	xfers[0].tx_buf = &status_command;
> > +	xfers[1].tx_buf = &status_command;
> > +	xfers[0].rx_buf = &status;
> > +	xfers[1].rx_buf = &status;
> > +	xfers[0].len = 1;
> > +	xfers[1].len = 1;
> > +	xfers[0].cs_change = 1;
> > +
> > +	ret = spi_sync_transfer(spi, xfers, 2);
> > +
> > +	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 mpf_ops_parse_header(struct fpga_manager *mgr,
> > +				struct fpga_image_info *info,
> > +				const char *buf, size_t count)
> > +{
> > +	size_t component_size_byte_num, component_size_byte_off,
> > +	       components_size_start, bitstream_start,
> > +	       block_id_offset, block_start_offset;
> > +	u8 header_size, blocks_num, block_id;
> > +	u32 block_start, component_size;
> > +	u16 components_num, i;
> > +
> > +	if (!buf) {
> > +		dev_err(&mgr->dev, "Image buffer is not provided\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	header_size = *(buf + MPF_HEADER_SIZE_OFFSET);
> > +	if (header_size > count) {
> > +		info->header_size = header_size;
> > +		return -EAGAIN;
> > +	}
> > +
> > +	/*
> > +	 * Go through look-up table to find out where actual bitstream starts
> > +	 * and where sizes of components of the bitstream lies.
> > +	 */
> > +	blocks_num = *(buf + header_size - 1);
> > +	block_id_offset = header_size + MPF_LOOKUP_TABLE_BLOCK_ID_OFFSET;
> > +	block_start_offset = header_size + MPF_LOOKUP_TABLE_BLOCK_START_OFFSET;
> > +
> > +	header_size += blocks_num * MPF_LOOKUP_TABLE_RECORD_SIZE;
> > +	if (header_size > count) {
> > +		info->header_size = header_size;
> > +		return -EAGAIN;
> > +	}
> > +
> > +	components_size_start = 0;
> > +	bitstream_start = 0;
> > +
> > +	while (blocks_num--) {
> > +		block_id = *(buf + block_id_offset);
> > +		block_start = get_unaligned_le32(buf + block_start_offset);
> > +
> > +		switch (block_id) {
> > +		case MPF_BITSTREAM_ID:
> > +			bitstream_start = block_start;
> > +			info->header_size = block_start;
> > +			if (block_start > count)
> > +				return -EAGAIN;
> > +
> > +			break;
> > +		case MPF_COMPONENTS_SIZE_ID:
> > +			components_size_start = block_start;
> > +			break;
> > +		default:
> > +			break;
> > +		}
> > +
> > +		if (bitstream_start && components_size_start)
> > +			break;
> > +
> > +		block_id_offset += MPF_LOOKUP_TABLE_RECORD_SIZE;
> > +		block_start_offset += MPF_LOOKUP_TABLE_RECORD_SIZE;
> > +	}
> > +
> > +	if (!bitstream_start || !components_size_start) {
> > +		dev_err(&mgr->dev, "Failed to parse header look-up table\n");
> > +		return -EFAULT;
> > +	}
> > +
> > +	/*
> > +	 * Parse bitstream size.
> > +	 * Sizes of components of the bitstream are 22-bits long placed next
> > +	 * to each other. Image header should be extended by now up to where
> > +	 * actual bitstream starts, so no need for overflow check anymore.
> > +	 */
> > +	components_num = get_unaligned_le16(buf + MPF_DATA_SIZE_OFFSET);
> > +
> > +	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);
> > +
> > +		info->data_size += component_size * MPF_SPI_FRAME_SIZE;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/* Poll HW status until busy bit is cleared and mask bits are set. */
> > +static int mpf_poll_status(struct spi_device *spi, u8 mask)
> > +{
> > +	int status, retries = MPF_STATUS_POLL_RETRIES;
> > +
> > +	while (retries--) {
> > +		status = mpf_read_status(spi);
> > +		if (status < 0)
> > +			return status;
> > +
> > +		if (status & MPF_STATUS_BUSY)
> > +			continue;
> > +
> > +		if (!mask || (status & mask))
> > +			return status;
> > +	}
> > +
> > +	return -EBUSY;
> > +}
> > +
> > +static int mpf_spi_write(struct spi_device *spi, const void *buf, size_t buf_size)
> > +{
> > +	int status = mpf_poll_status(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 = mpf_poll_status(spi, MPF_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 program_mode[] = { MPF_SPI_FRAME_INIT, MPF_SPI_PRG_MODE };
> > +	const u8 isc_en_command[] = { MPF_SPI_ISC_ENABLE };
> > +	struct mpf_priv *priv = mgr->priv;
> > +	struct device *dev = &mgr->dev;
> > +	struct spi_device *spi;
> > +	u32 isc_ret = 0;
> > +	int ret;
> > +
> > +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> > +		dev_err(dev, "Partial reconfiguration is not supported\n");
> > +		return -EOPNOTSUPP;
> > +	}
> > +
> > +	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: spi_ret %d, isc_ret %u\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 spi_frame_command[] = { MPF_SPI_FRAME };
> > +	struct spi_transfer xfers[2] = { 0 };
> > +	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;
> > +
> > +	xfers[0].tx_buf = spi_frame_command;
> > +	xfers[0].len = sizeof(spi_frame_command);
> > +
> > +	for (i = 0; i < count / MPF_SPI_FRAME_SIZE; i++) {
> > +		xfers[1].tx_buf = buf + i * MPF_SPI_FRAME_SIZE;
> > +		xfers[1].len = MPF_SPI_FRAME_SIZE;
> > +
> > +		ret = mpf_poll_status(spi, 0);
> > +		if (ret >= 0)
> > +			ret = spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));
> > +
> > +		if (ret) {
> > +			dev_err(dev, "Failed to write bitstream frame %d/%zu\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,
> > +	.initial_header_size = 71,
> > +	.skip_header = true,
> > +	.parse_header = mpf_ops_parse_header,
> > +	.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
> > 


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

* Re: [PATCH v21 3/5] fpga: microchip-spi: add Microchip MPF FPGA manager
  2022-06-23  9:48     ` Ivan Bornyakov
@ 2022-06-23  9:50       ` Xu Yilun
  0 siblings, 0 replies; 12+ messages in thread
From: Xu Yilun @ 2022-06-23  9:50 UTC (permalink / raw)
  To: Ivan Bornyakov
  Cc: mdf, hao.wu, trix, corbet, conor.dooley, robh+dt,
	krzysztof.kozlowski+dt, linux-fpga, linux-doc, devicetree,
	linux-kernel, system

On Thu, Jun 23, 2022 at 12:48:02PM +0300, Ivan Bornyakov wrote:
> On Thu, Jun 23, 2022 at 05:29:39PM +0800, Xu Yilun wrote:
> > On Tue, Jun 21, 2022 at 06:38:45PM +0300, Ivan Bornyakov wrote:
> > > 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>
> > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > > Tested-by: Conor Dooley <conor.dooley@microchip.com>
> > > Acked-by: Xu Yilun <yilun.xu@intel.com>
> > > ---
> > >  drivers/fpga/Kconfig         |   8 +
> > >  drivers/fpga/Makefile        |   1 +
> > >  drivers/fpga/microchip-spi.c | 398 +++++++++++++++++++++++++++++++++++
> > >  3 files changed, 407 insertions(+)
> > >  create mode 100644 drivers/fpga/microchip-spi.c
> > > 
> > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > > index 0831eecc9a09..6c416955da53 100644
> > > --- a/drivers/fpga/Kconfig
> > > +++ b/drivers/fpga/Kconfig
> > > @@ -255,4 +255,12 @@ config FPGA_M10_BMC_SEC_UPDATE
> > >  	  (BMC) and provides support for secure updates for the BMC image,
> > >  	  the FPGA image, the Root Entry Hashes, etc.
> > >  
> > > +config FPGA_MGR_MICROCHIP_SPI
> > > +	tristate "Microchip Polarfire SPI FPGA manager"
> > > +	depends on SPI
> > > +	help
> > > +	  FPGA manager driver support for Microchip Polarfire FPGAs
> > > +	  programming over slave SPI interface with .dat formatted
> > > +	  bitstream image.
> > 
> > I still see the checkpatch warning here, please help fix it.
> > 
> > WARNING: please write a help paragraph that fully describes the config symbol
> > 
> 
> Clearly there is a help paragraph. The reason why checkpatch warns here
> is that help paragrap is of 3 lines long while minimum requirement is 4
> lines. But does it really matter?

Thanks for the info. It's good to me now.

> 
> > Thanks,
> > Yilun
> > 
> > > +
> > >  endif # FPGA
> > > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> > > index 139ac1b573d3..42ae8b58abce 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..bd284c7b8dc9
> > > --- /dev/null
> > > +++ b/drivers/fpga/microchip-spi.c
> > > @@ -0,0 +1,398 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Microchip Polarfire FPGA programming over slave SPI interface.
> > > + */
> > > +
> > > +#include <asm/unaligned.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_RETRIES		10000
> > > +#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 = 0, status_command = MPF_SPI_READ_STATUS;
> > > +	struct spi_transfer xfers[2] = { 0 };
> > > +	int ret;
> > > +
> > > +	/*
> > > +	 * HW status is returned on MISO in the first byte after CS went
> > > +	 * active. However, first reading can be inadequate, so we submit
> > > +	 * two identical SPI transfers and use result of the later one.
> > > +	 */
> > > +	xfers[0].tx_buf = &status_command;
> > > +	xfers[1].tx_buf = &status_command;
> > > +	xfers[0].rx_buf = &status;
> > > +	xfers[1].rx_buf = &status;
> > > +	xfers[0].len = 1;
> > > +	xfers[1].len = 1;
> > > +	xfers[0].cs_change = 1;
> > > +
> > > +	ret = spi_sync_transfer(spi, xfers, 2);
> > > +
> > > +	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 mpf_ops_parse_header(struct fpga_manager *mgr,
> > > +				struct fpga_image_info *info,
> > > +				const char *buf, size_t count)
> > > +{
> > > +	size_t component_size_byte_num, component_size_byte_off,
> > > +	       components_size_start, bitstream_start,
> > > +	       block_id_offset, block_start_offset;
> > > +	u8 header_size, blocks_num, block_id;
> > > +	u32 block_start, component_size;
> > > +	u16 components_num, i;
> > > +
> > > +	if (!buf) {
> > > +		dev_err(&mgr->dev, "Image buffer is not provided\n");
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	header_size = *(buf + MPF_HEADER_SIZE_OFFSET);
> > > +	if (header_size > count) {
> > > +		info->header_size = header_size;
> > > +		return -EAGAIN;
> > > +	}
> > > +
> > > +	/*
> > > +	 * Go through look-up table to find out where actual bitstream starts
> > > +	 * and where sizes of components of the bitstream lies.
> > > +	 */
> > > +	blocks_num = *(buf + header_size - 1);
> > > +	block_id_offset = header_size + MPF_LOOKUP_TABLE_BLOCK_ID_OFFSET;
> > > +	block_start_offset = header_size + MPF_LOOKUP_TABLE_BLOCK_START_OFFSET;
> > > +
> > > +	header_size += blocks_num * MPF_LOOKUP_TABLE_RECORD_SIZE;
> > > +	if (header_size > count) {
> > > +		info->header_size = header_size;
> > > +		return -EAGAIN;
> > > +	}
> > > +
> > > +	components_size_start = 0;
> > > +	bitstream_start = 0;
> > > +
> > > +	while (blocks_num--) {
> > > +		block_id = *(buf + block_id_offset);
> > > +		block_start = get_unaligned_le32(buf + block_start_offset);
> > > +
> > > +		switch (block_id) {
> > > +		case MPF_BITSTREAM_ID:
> > > +			bitstream_start = block_start;
> > > +			info->header_size = block_start;
> > > +			if (block_start > count)
> > > +				return -EAGAIN;
> > > +
> > > +			break;
> > > +		case MPF_COMPONENTS_SIZE_ID:
> > > +			components_size_start = block_start;
> > > +			break;
> > > +		default:
> > > +			break;
> > > +		}
> > > +
> > > +		if (bitstream_start && components_size_start)
> > > +			break;
> > > +
> > > +		block_id_offset += MPF_LOOKUP_TABLE_RECORD_SIZE;
> > > +		block_start_offset += MPF_LOOKUP_TABLE_RECORD_SIZE;
> > > +	}
> > > +
> > > +	if (!bitstream_start || !components_size_start) {
> > > +		dev_err(&mgr->dev, "Failed to parse header look-up table\n");
> > > +		return -EFAULT;
> > > +	}
> > > +
> > > +	/*
> > > +	 * Parse bitstream size.
> > > +	 * Sizes of components of the bitstream are 22-bits long placed next
> > > +	 * to each other. Image header should be extended by now up to where
> > > +	 * actual bitstream starts, so no need for overflow check anymore.
> > > +	 */
> > > +	components_num = get_unaligned_le16(buf + MPF_DATA_SIZE_OFFSET);
> > > +
> > > +	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);
> > > +
> > > +		info->data_size += component_size * MPF_SPI_FRAME_SIZE;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +/* Poll HW status until busy bit is cleared and mask bits are set. */
> > > +static int mpf_poll_status(struct spi_device *spi, u8 mask)
> > > +{
> > > +	int status, retries = MPF_STATUS_POLL_RETRIES;
> > > +
> > > +	while (retries--) {
> > > +		status = mpf_read_status(spi);
> > > +		if (status < 0)
> > > +			return status;
> > > +
> > > +		if (status & MPF_STATUS_BUSY)
> > > +			continue;
> > > +
> > > +		if (!mask || (status & mask))
> > > +			return status;
> > > +	}
> > > +
> > > +	return -EBUSY;
> > > +}
> > > +
> > > +static int mpf_spi_write(struct spi_device *spi, const void *buf, size_t buf_size)
> > > +{
> > > +	int status = mpf_poll_status(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 = mpf_poll_status(spi, MPF_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 program_mode[] = { MPF_SPI_FRAME_INIT, MPF_SPI_PRG_MODE };
> > > +	const u8 isc_en_command[] = { MPF_SPI_ISC_ENABLE };
> > > +	struct mpf_priv *priv = mgr->priv;
> > > +	struct device *dev = &mgr->dev;
> > > +	struct spi_device *spi;
> > > +	u32 isc_ret = 0;
> > > +	int ret;
> > > +
> > > +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> > > +		dev_err(dev, "Partial reconfiguration is not supported\n");
> > > +		return -EOPNOTSUPP;
> > > +	}
> > > +
> > > +	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: spi_ret %d, isc_ret %u\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 spi_frame_command[] = { MPF_SPI_FRAME };
> > > +	struct spi_transfer xfers[2] = { 0 };
> > > +	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;
> > > +
> > > +	xfers[0].tx_buf = spi_frame_command;
> > > +	xfers[0].len = sizeof(spi_frame_command);
> > > +
> > > +	for (i = 0; i < count / MPF_SPI_FRAME_SIZE; i++) {
> > > +		xfers[1].tx_buf = buf + i * MPF_SPI_FRAME_SIZE;
> > > +		xfers[1].len = MPF_SPI_FRAME_SIZE;
> > > +
> > > +		ret = mpf_poll_status(spi, 0);
> > > +		if (ret >= 0)
> > > +			ret = spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));
> > > +
> > > +		if (ret) {
> > > +			dev_err(dev, "Failed to write bitstream frame %d/%zu\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,
> > > +	.initial_header_size = 71,
> > > +	.skip_header = true,
> > > +	.parse_header = mpf_ops_parse_header,
> > > +	.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
> > > 

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

end of thread, other threads:[~2022-06-23  9:58 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-21 15:38 [PATCH v21 0/5] Microchip Polarfire FPGA manager Ivan Bornyakov
2022-06-21 15:38 ` [PATCH v21 1/5] fpga: fpga-mgr: support bitstream offset in image buffer Ivan Bornyakov
2022-06-23  9:28   ` Xu Yilun
2022-06-21 15:38 ` [PATCH v21 2/5] docs: fpga: mgr: document parse_header() callback Ivan Bornyakov
2022-06-21 15:38 ` [PATCH v21 3/5] fpga: microchip-spi: add Microchip MPF FPGA manager Ivan Bornyakov
2022-06-23  9:29   ` Xu Yilun
2022-06-23  9:48     ` Ivan Bornyakov
2022-06-23  9:50       ` Xu Yilun
2022-06-21 15:38 ` [PATCH v21 4/5] dt-bindings: fpga: add binding doc for microchip-spi fpga mgr Ivan Bornyakov
2022-06-21 15:38 ` [PATCH v21 5/5] MAINTAINERS: add Microchip PolarFire FPGA drivers entry Ivan Bornyakov
2022-06-21 15:48   ` Conor.Dooley
2022-06-23  9:32   ` Xu Yilun

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.