[PATCHv1 0/2] Add streaming support for base firmware and fpga-mgr

[PATCHv1 0/2] Add streaming support for base firmware and fpga-mgr

[yi1.li]

From: Yi Li <yi1.li@linux.intel.com>

As the FPGA hardware advances, the bitstream image size grows. It could be an issue for kernel to allocate big buffers to load the whole firmware file. Here is an attempt to load the bitstream file into 4KB buffer and program the FPGA in a streaming mode. The streaming support is added in the driver_data_request_sync API on Luis R. Rodriguez's v6 branch. https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20170329-driver-data-v2-try3, which is sent to this list for review on March/29 [PATCH v6 0/5] firmware: add driver data API.
It should not be a performance hit for FPGA programing since the majority of time spend is with the FPGA CvP/PR-IP interface.

Yi Li (2):
  firmware: Add streaming support
  fpga-mgr: add streaming support

 drivers/base/firmware_class.c | 119 +++++++++++++++++++++++++++++++++++++++++-
 drivers/fpga/fpga-mgr.c       |  96 ++++++++++++++++++++++++++++++++++
 include/linux/driver_data.h   |  13 +++++
 include/linux/fpga/fpga-mgr.h |   4 ++
 4 files changed, 231 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCHv1 1/2] firmware: Add streaming support

[yi1.li]

From: Yi Li <yi1.li@linux.intel.com>

Load firmware in multiple chucks(4KB) instead of the whole big firmware file
at once. The streaming support is added in the driver_data_request_sync API
based on Luis R. Rodriguez's 20170329-driver-data-v2-try3 branch.

Signed-off-by: Yi Li <yi1.li@linux.intel.com>
---
 drivers/base/firmware_class.c | 119 +++++++++++++++++++++++++++++++++++++++++-
 include/linux/driver_data.h   |  13 +++++
 2 files changed, 131 insertions(+), 1 deletion(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index cc3c224..beecbf5 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -569,6 +569,73 @@ fw_get_filesystem_firmware(struct device *device, struct firmware_buf *buf)
 	return rc;
 }
 
+static int
+fw_stream_filesystem_firmware(struct device *device, struct firmware_buf *buf,
+			      size_t offset, size_t length, char **path)
+{
+	int i, len;
+	int rc = 0;
+	struct file *file;
+
+	buf->size = 0;
+	if (!buf->data) {
+		buf->data = vmalloc(length);
+		if (!buf->data) {
+			rc = -ENOMEM;
+			return rc;
+		}
+	}
+
+	/* skip the repeating folder searching */
+	if (*path) {
+		file = filp_open(*path, O_RDONLY, 0);
+		if (IS_ERR(file)) {
+			rc = -ENOENT;
+			return rc;
+		}
+
+		buf->size = kernel_read(file, offset, (char *)buf->data,
+					length);
+		fput(file);
+		return rc;
+	}
+
+	dev_info(device, "search %s in the folder\n", buf->fw_id);
+	*path = __getname();
+	if (!*path)
+		return -ENOMEM;
+
+	for (i = 0; i < ARRAY_SIZE(fw_path); i++) {
+		/* skip the unset customized path */
+		if (!fw_path[i][0])
+			continue;
+
+		len = snprintf(*path, PATH_MAX, "%s/%s",
+			       fw_path[i], buf->fw_id);
+		if (len >= PATH_MAX) {
+			rc = -ENAMETOOLONG;
+			break;
+		}
+
+		file = filp_open(*path, O_RDONLY, 0);
+		if (IS_ERR(file)) {
+			rc = -ENOENT;
+			continue;
+		}
+
+		buf->size = kernel_read(file, offset, (char *)buf->data,
+					length);
+		rc = 0;
+		fput(file);
+		break;
+	}
+
+	if (rc)
+		dev_err(device, "loading %s failed with error %d\n",
+			*path, rc);
+	return rc;
+}
+
 /* firmware holds the ownership of pages */
 static void firmware_free_data(const struct firmware *fw)
 {
@@ -1310,6 +1377,46 @@ _request_firmware_prepare(struct firmware **firmware_p, const char *name,
 	return 1; /* need to load */
 }
 
+static int
+_stream_firmware(const struct firmware **firmware_p, const char *name,
+		 struct driver_data_params *data_params,
+		 struct device *device)
+{
+	int ret;
+	struct firmware *fw = NULL;
+	size_t offset = data_params->req_params.streaming_reqs.offset;
+	size_t buf_size = data_params->req_params.streaming_reqs.buf_size;
+
+	if ((!firmware_p) || (!name || name[0] == '\0')) {
+		dev_err(device, "invalid firmware pointer or file name\n");
+		return -EINVAL;
+	}
+
+	if (!*firmware_p) {
+		ret = _request_firmware_prepare(&fw, name, device, data_params);
+		if (ret <= 0) {
+			dev_err(device, "%s: failed %d\n",
+				__func__, ret);
+		}
+	} else {
+		fw = (struct firmware *)*firmware_p;
+	}
+
+	ret = fw_stream_filesystem_firmware(device, fw->priv, offset, buf_size,
+				data_params->req_params.streaming_reqs.path);
+	if (ret) {
+		if (!driver_data_param_optional(&data_params->req_params))
+			dev_warn(device,
+				 "streaming %s failed with error %d\n",
+				 name, ret);
+	} else {
+		ret = assign_firmware_buf(fw, device, data_params);
+	}
+	*firmware_p = fw;
+
+	return ret;
+}
+
 /* called from request_firmware() and request_firmware_work_func() */
 static int
 _request_firmware(const struct firmware **firmware_p, const char *name,
@@ -1577,7 +1684,17 @@ int driver_data_request_sync(const char *name,
 	__module_get(sync_reqs->module);
 	get_device(device);
 
-	ret = _request_firmware(&driver_data, name, &params, device);
+	if (req_params->streaming_reqs.streaming) {
+		/* For streaming, driver_data is recycled and kept */
+		if (!req_params->keep)
+			return -EINVAL;
+
+		ret = _stream_firmware(req_params->streaming_reqs.driver_data,
+				       name, &params, device);
+		driver_data = *req_params->streaming_reqs.driver_data;
+	} else {
+		ret = _request_firmware(&driver_data, name, &params, device);
+	}
 	if (ret && driver_data_param_optional(req_params))
 		ret = driver_data_sync_opt_call_cb(req_params);
 	else
diff --git a/include/linux/driver_data.h b/include/linux/driver_data.h
index fda1e71..ee485a5 100644
--- a/include/linux/driver_data.h
+++ b/include/linux/driver_data.h
@@ -62,6 +62,16 @@ struct driver_data_reqs {
 	gfp_t gfp;
 };
 
+struct driver_data_streaming_reqs {
+	bool streaming;
+	char **path;
+	size_t offset;
+	size_t buf_size;
+	const struct firmware **driver_data;
+	void *opt_ctx1;
+	void *opt_ctx2;
+};
+
 /**
  * struct driver_data_req_params - driver data request parameters
  * @optional: if true it is not a hard requirement by the caller that this
@@ -93,6 +103,8 @@ struct driver_data_reqs {
  *	digit will be placed in the middle, followed by the @api_name_postfix.
  * @sync_reqs: synchronization requirements
  *
+ * @streaming_reqs: streaming requirements
+ *
  * This data structure is intended to carry all requirements and specifications
  * required to complete the task to get the requested driver date file to the
  * caller.
@@ -107,6 +119,7 @@ struct driver_data_req_params {
 	const char *api_name_postfix;
 	struct driver_data_reqs sync_reqs;
 	const union driver_data_cbs cbs;
+	struct driver_data_streaming_reqs streaming_reqs;
 };
 
 /*
-- 
2.7.4

[PATCHv1 2/2] fpga-mgr: add streaming support

[yi1.li]

From: Yi Li <yi1.li@linux.intel.com>

Add fpga_mgr_firmware_stream API to enable streaming/programing
FPGA bitstream file from file system to FPGA manager. This code is
on top of Luis R. Rodriguez's new driver_data_request_sync API.

Signed-off-by: Yi Li <yi1.li@linux.intel.com>
---
 drivers/fpga/fpga-mgr.c       | 96 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/fpga/fpga-mgr.h |  4 ++
 2 files changed, 100 insertions(+)

diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index 188ffef..975194c 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -27,6 +27,8 @@
 #include <linux/slab.h>
 #include <linux/scatterlist.h>
 #include <linux/highmem.h>
+#include <linux/sizes.h>
+#include <linux/driver_data.h>
 
 static DEFINE_IDA(fpga_mgr_ida);
 static struct class *fpga_mgr_class;
@@ -196,6 +198,100 @@ static int fpga_mgr_buf_load_mapped(struct fpga_manager *mgr,
 	return fpga_mgr_write_complete(mgr, info);
 }
 
+static int fpga_mgr_streaming_fw_cb(void *context, const struct firmware *fw)
+{
+	int ret = -EINVAL;
+	struct driver_data_req_params *params =
+		(struct driver_data_req_params *)context;
+	struct fpga_image_info *info = (struct fpga_image_info *)
+		params->streaming_reqs.opt_ctx1;
+	struct fpga_manager *mgr = (struct fpga_manager *)
+		params->streaming_reqs.opt_ctx2;
+	struct device *dev = &mgr->dev;
+
+	/*
+	 * init.
+	 */
+	if (params->streaming_reqs.offset == 0) {
+		ret = fpga_mgr_write_init_buf(mgr, info, fw->data, fw->size);
+		if (ret)
+			return ret;
+	}
+
+	/*
+	 * Write the FPGA image to the FPGA.
+	 */
+	mgr->state = FPGA_MGR_STATE_WRITE;
+	ret = mgr->mops->write(mgr, fw->data, fw->size);
+	if (ret) {
+		dev_err(dev, "Error while writing image data to FPGA\n");
+		mgr->state = FPGA_MGR_STATE_WRITE_ERR;
+		return ret;
+	}
+
+	params->streaming_reqs.offset += fw->size;
+	if (fw->size < SZ_4K)
+		ret = fpga_mgr_write_complete(mgr, info);
+
+	return ret;
+}
+
+/**
+ * fpga_mgr_firmware_stream - streaming firmware and load to fpga
+ * @mgr:	fpga manager
+ * @info:	fpga image specific information
+ * @image_name:	name of image file on the firmware search path
+ *
+ * Streaming an FPGA image using the firmware class, then write out to the FPGA.
+ * Update the state before each step to provide info on what step failed if
+ * there is a failure.  This code assumes the caller got the mgr pointer
+ * from of_fpga_mgr_get() or fpga_mgr_get() and checked that it is not an error
+ * code.
+ *
+ * Return: 0 on success, negative error code otherwise.
+ */
+int fpga_mgr_firmware_stream(struct fpga_manager *mgr,
+			     struct fpga_image_info *info,
+			     const char *image_name)
+{
+	int ret;
+	const struct firmware *fw = NULL;
+	char *path = NULL;
+	size_t length = INT_MAX;
+	struct device *dev = &mgr->dev;
+	const struct driver_data_req_params req_params = {
+		.streaming_reqs.streaming = true,
+		.streaming_reqs.offset = 0,
+		.streaming_reqs.driver_data = &fw,
+		.streaming_reqs.path = &path,
+		.streaming_reqs.buf_size = SZ_4K,
+		.streaming_reqs.opt_ctx1 = info,
+		.streaming_reqs.opt_ctx2 = mgr,
+		DRIVER_DATA_KEEP_SYNC(fpga_mgr_streaming_fw_cb,
+				      &req_params),
+	};
+
+	mgr->state = FPGA_MGR_STATE_FIRMWARE_REQ;
+	while (length > 0) {
+		ret = driver_data_request_sync(image_name, &req_params, dev);
+		if (ret) {
+			dev_err(dev, "Error reading firmware %d\n", ret);
+			mgr->state = FPGA_MGR_STATE_FIRMWARE_REQ_ERR;
+			return ret;
+		}
+
+		length -= fw->size;
+		if (fw->size < SZ_4K)
+			break;
+	}
+
+	__putname(*req_params.streaming_reqs.path);
+	release_firmware(fw);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(fpga_mgr_firmware_stream);
+
 /**
  * fpga_mgr_buf_load - load fpga from image in buffer
  * @mgr:	fpga manager
diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
index e2ef94fd..97907f5 100644
--- a/include/linux/fpga/fpga-mgr.h
+++ b/include/linux/fpga/fpga-mgr.h
@@ -140,6 +140,10 @@ int fpga_mgr_firmware_load(struct fpga_manager *mgr,
 			   struct fpga_image_info *info,
 			   const char *image_name);
 
+int fpga_mgr_firmware_stream(struct fpga_manager *mgr,
+			     struct fpga_image_info *info,
+			     const char *image_name);
+
 struct fpga_manager *of_fpga_mgr_get(struct device_node *node);
 
 struct fpga_manager *fpga_mgr_get(struct device *dev);
-- 
2.7.4