fpga-mgr has three methods for programming FPGAs, depending on
whether the image is in a scatter gather list, a contiguous
buffer, or a firmware file. This makes it difficult to write
upper layers as the caller has to assume whether the FPGA image
is in a sg table, as a single buffer, or a firmware file.
This commit moves these parameters to struct fpga_image_info
and adds a single function for programming fpgas.

New functions:
* fpga_mgr_load - given fpga manager and struct fpga_image_info,
   program the fpga.

* fpga_image_info_alloc - alloc a struct fpga_image_info.

* fpga_image_info_free - free a struct fpga_image_info.

These three functions are unexported:
* fpga_mgr_buf_load_sg
* fpga_mgr_buf_load
* fpga_mgr_firmware_load

Also use devm_kstrdup to copy firmware_name so we aren't making
assumptions about where it comes from when allocing/freeing the
struct fpga_image_info.

Signed-off-by: Alan Tull <at...@kernel.org>
---
 drivers/fpga/fpga-mgr.c       | 59 +++++++++++++++++++++++++++++++++++--------
 drivers/fpga/fpga-region.c    | 43 ++++++++++++++++++++-----------
 include/linux/fpga/fpga-mgr.h | 22 ++++++++++------
 3 files changed, 90 insertions(+), 34 deletions(-)

diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index 188ffef..3478aab 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -2,6 +2,7 @@
  * FPGA Manager Core
  *
  *  Copyright (C) 2013-2015 Altera Corporation
+ *  Copyright (C) 2017 Intel Corporation
  *
  * With code from the mailing list:
  * Copyright (C) 2013 Xilinx, Inc.
@@ -31,6 +32,31 @@
 static DEFINE_IDA(fpga_mgr_ida);
 static struct class *fpga_mgr_class;
 
+struct fpga_image_info *fpga_image_info_alloc(struct device *dev)
+{
+       struct fpga_image_info *info;
+
+       info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
+       if (!info)
+               return ERR_PTR(-ENOMEM);
+
+       return info;
+}
+EXPORT_SYMBOL_GPL(fpga_image_info_alloc);
+
+void fpga_image_info_free(struct device *dev,
+                         struct fpga_image_info *info)
+{
+       if (!info)
+               return;
+
+       if (info->firmware_name)
+               devm_kfree(dev, info->firmware_name);
+
+       devm_kfree(dev, info);
+}
+EXPORT_SYMBOL_GPL(fpga_image_info_free);
+
 /*
  * Call the low level driver's write_init function.  This will do the
  * device-specific things to get the FPGA into the state where it is ready to
@@ -137,8 +163,9 @@ static int fpga_mgr_write_complete(struct fpga_manager *mgr,
  *
  * Return: 0 on success, negative error code otherwise.
  */
-int fpga_mgr_buf_load_sg(struct fpga_manager *mgr, struct fpga_image_info 
*info,
-                        struct sg_table *sgt)
+static int fpga_mgr_buf_load_sg(struct fpga_manager *mgr,
+                               struct fpga_image_info *info,
+                               struct sg_table *sgt)
 {
        int ret;
 
@@ -170,7 +197,6 @@ int fpga_mgr_buf_load_sg(struct fpga_manager *mgr, struct 
fpga_image_info *info,
 
        return fpga_mgr_write_complete(mgr, info);
 }
-EXPORT_SYMBOL_GPL(fpga_mgr_buf_load_sg);
 
 static int fpga_mgr_buf_load_mapped(struct fpga_manager *mgr,
                                    struct fpga_image_info *info,
@@ -210,8 +236,9 @@ static int fpga_mgr_buf_load_mapped(struct fpga_manager 
*mgr,
  *
  * Return: 0 on success, negative error code otherwise.
  */
-int fpga_mgr_buf_load(struct fpga_manager *mgr, struct fpga_image_info *info,
-                     const char *buf, size_t count)
+static int fpga_mgr_buf_load(struct fpga_manager *mgr,
+                            struct fpga_image_info *info,
+                            const char *buf, size_t count)
 {
        struct page **pages;
        struct sg_table sgt;
@@ -266,7 +293,6 @@ int fpga_mgr_buf_load(struct fpga_manager *mgr, struct 
fpga_image_info *info,
 
        return rc;
 }
-EXPORT_SYMBOL_GPL(fpga_mgr_buf_load);
 
 /**
  * fpga_mgr_firmware_load - request firmware and load to fpga
@@ -282,9 +308,9 @@ EXPORT_SYMBOL_GPL(fpga_mgr_buf_load);
  *
  * Return: 0 on success, negative error code otherwise.
  */
-int fpga_mgr_firmware_load(struct fpga_manager *mgr,
-                          struct fpga_image_info *info,
-                          const char *image_name)
+static int fpga_mgr_firmware_load(struct fpga_manager *mgr,
+                                 struct fpga_image_info *info,
+                                 const char *image_name)
 {
        struct device *dev = &mgr->dev;
        const struct firmware *fw;
@@ -307,7 +333,18 @@ int fpga_mgr_firmware_load(struct fpga_manager *mgr,
 
        return ret;
 }
-EXPORT_SYMBOL_GPL(fpga_mgr_firmware_load);
+
+int fpga_mgr_load(struct fpga_manager *mgr, struct fpga_image_info *info)
+{
+       if (info->sgt)
+               return fpga_mgr_buf_load_sg(mgr, info, info->sgt);
+       if (info->buf && info->count)
+               return fpga_mgr_buf_load(mgr, info, info->buf, info->count);
+       if (info->firmware_name)
+               return fpga_mgr_firmware_load(mgr, info, info->firmware_name);
+       return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(fpga_mgr_load);
 
 static const char * const state_str[] = {
        [FPGA_MGR_STATE_UNKNOWN] =              "unknown",
@@ -578,7 +615,7 @@ static void __exit fpga_mgr_class_exit(void)
        ida_destroy(&fpga_mgr_ida);
 }
 
-MODULE_AUTHOR("Alan Tull <at...@opensource.altera.com>");
+MODULE_AUTHOR("Alan Tull <at...@kernel.org>");
 MODULE_DESCRIPTION("FPGA manager framework");
 MODULE_LICENSE("GPL v2");
 
diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
index 655940f..c71c9cb 100644
--- a/drivers/fpga/fpga-region.c
+++ b/drivers/fpga/fpga-region.c
@@ -226,14 +226,11 @@ static int fpga_region_get_bridges(struct fpga_region 
*region,
 /**
  * fpga_region_program_fpga - program FPGA
  * @region: FPGA region
- * @firmware_name: name of FPGA image firmware file
  * @overlay: device node of the overlay
- * Program an FPGA using information in the device tree.
- * Function assumes that there is a firmware-name property.
+ * Program an FPGA using information in the region's fpga image info.
  * Return 0 for success or negative error code.
  */
 static int fpga_region_program_fpga(struct fpga_region *region,
-                                   const char *firmware_name,
                                    struct device_node *overlay)
 {
        struct fpga_manager *mgr;
@@ -264,7 +261,7 @@ static int fpga_region_program_fpga(struct fpga_region 
*region,
                goto err_put_br;
        }
 
-       ret = fpga_mgr_firmware_load(mgr, region->info, firmware_name);
+       ret = fpga_mgr_load(mgr, region->info);
        if (ret) {
                pr_err("failed to load fpga image\n");
                goto err_put_br;
@@ -357,16 +354,15 @@ static int child_regions_with_firmware(struct device_node 
*overlay)
 static int fpga_region_notify_pre_apply(struct fpga_region *region,
                                        struct of_overlay_notify_data *nd)
 {
-       const char *firmware_name = NULL;
+       struct device *dev = &region->dev;
        struct fpga_image_info *info;
+       const char *firmware_name;
        int ret;
 
-       info = devm_kzalloc(&region->dev, sizeof(*info), GFP_KERNEL);
+       info = fpga_image_info_alloc(dev);
        if (!info)
                return -ENOMEM;
 
-       region->info = info;
-
        /* Reject overlay if child FPGA Regions have firmware-name property */
        ret = child_regions_with_firmware(nd->overlay);
        if (ret)
@@ -382,7 +378,13 @@ static int fpga_region_notify_pre_apply(struct fpga_region 
*region,
        if (of_property_read_bool(nd->overlay, "encrypted-fpga-config"))
                info->flags |= FPGA_MGR_ENCRYPTED_BITSTREAM;
 
-       of_property_read_string(nd->overlay, "firmware-name", &firmware_name);
+       if (!of_property_read_string(nd->overlay, "firmware-name",
+                                    &firmware_name)) {
+               info->firmware_name = devm_kstrdup(dev, firmware_name,
+                                                  GFP_KERNEL);
+               if (!info->firmware_name)
+                       return -ENOMEM;
+       }
 
        of_property_read_u32(nd->overlay, "region-unfreeze-timeout-us",
                             &info->enable_timeout_us);
@@ -394,22 +396,33 @@ static int fpga_region_notify_pre_apply(struct 
fpga_region *region,
                             &info->config_complete_timeout_us);
 
        /* If FPGA was externally programmed, don't specify firmware */
-       if ((info->flags & FPGA_MGR_EXTERNAL_CONFIG) && firmware_name) {
+       if ((info->flags & FPGA_MGR_EXTERNAL_CONFIG) && info->firmware_name) {
                pr_err("error: specified firmware and external-fpga-config");
+               fpga_image_info_free(dev, info);
                return -EINVAL;
        }
 
        /* FPGA is already configured externally.  We're done. */
-       if (info->flags & FPGA_MGR_EXTERNAL_CONFIG)
+       if (info->flags & FPGA_MGR_EXTERNAL_CONFIG) {
+               fpga_image_info_free(dev, info);
                return 0;
+       }
 
        /* If we got this far, we should be programming the FPGA */
-       if (!firmware_name) {
+       if (!info->firmware_name) {
                pr_err("should specify firmware-name or 
external-fpga-config\n");
+               fpga_image_info_free(dev, info);
                return -EINVAL;
        }
 
-       return fpga_region_program_fpga(region, firmware_name, nd->overlay);
+       region->info = info;
+       ret = fpga_region_program_fpga(region, nd->overlay);
+       if (ret) {
+               fpga_image_info_free(dev, info);
+               region->info = NULL;
+       }
+
+       return ret;
 }
 
 /**
@@ -426,7 +439,7 @@ static void fpga_region_notify_post_remove(struct 
fpga_region *region,
 {
        fpga_bridges_disable(&region->bridge_list);
        fpga_bridges_put(&region->bridge_list);
-       devm_kfree(&region->dev, region->info);
+       fpga_image_info_free(&region->dev, region->info);
        region->info = NULL;
 }
 
diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
index b4ac24c..1c53aa3 100644
--- a/include/linux/fpga/fpga-mgr.h
+++ b/include/linux/fpga/fpga-mgr.h
@@ -1,7 +1,8 @@
 /*
  * FPGA Framework
  *
- *  Copyright (C) 2013-2015 Altera Corporation
+ *  Copyright (C) 2013-2016 Altera Corporation
+ *  Copyright (C) 2017 Intel Corporation
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -79,12 +80,20 @@ enum fpga_mgr_states {
  * @disable_timeout_us: maximum time to disable traffic through bridge (uSec)
  * @config_complete_timeout_us: maximum time for FPGA to switch to operating
  *        status in the write_complete op.
+ * @firmware_name: name of FPGA image firmware file
+ * @sgt: scatter/gather table containing FPGA image
+ * @buf: contiguous buffer containing FPGA image
+ * @count: size of buf
  */
 struct fpga_image_info {
        u32 flags;
        u32 enable_timeout_us;
        u32 disable_timeout_us;
        u32 config_complete_timeout_us;
+       char *firmware_name;
+       struct sg_table *sgt;
+       const char *buf;
+       size_t count;
 };
 
 /**
@@ -134,14 +143,11 @@ struct fpga_manager {
 
 #define to_fpga_manager(d) container_of(d, struct fpga_manager, dev)
 
-int fpga_mgr_buf_load(struct fpga_manager *mgr, struct fpga_image_info *info,
-                     const char *buf, size_t count);
-int fpga_mgr_buf_load_sg(struct fpga_manager *mgr, struct fpga_image_info 
*info,
-                        struct sg_table *sgt);
+struct fpga_image_info *fpga_image_info_alloc(struct device *dev);
 
-int fpga_mgr_firmware_load(struct fpga_manager *mgr,
-                          struct fpga_image_info *info,
-                          const char *image_name);
+void fpga_image_info_free(struct device *dev, struct fpga_image_info *info);
+
+int fpga_mgr_load(struct fpga_manager *mgr, struct fpga_image_info *info);
 
 struct fpga_manager *of_fpga_mgr_get(struct device_node *node);
 
-- 
2.7.4

Reply via email to