Message-Id: <20170913204841.2730-4-atull@kernel.org>
Date:   Wed, 13 Sep 2017 15:48:26 -0500
From:   Alan Tull <atull@...nel.org>
To:     Moritz Fischer <mdf@...nel.org>
Cc:     Alan Tull <atull@...nel.org>, linux-kernel@...r.kernel.org,
        linux-fpga@...r.kernel.org
Subject: [PATCH v4 03/18] fpga: mgr: separate getting/locking FPGA manager

Previously when the user gets a FPGA manager, it was locked
and nobody else could use it for programming.

This commit makes it straightforward to save a reference to an
FPGA manager and only lock it when programming the FPGA.

Add functions that get an FPGA manager's mutex for exclusive use:
* fpga_mgr_lock
* fpga_mgr_unlock

The following functions no longer lock an FPGA manager's mutex:
* of_fpga_mgr_get
* fpga_mgr_get
* fpga_mgr_put

Signed-off-by: Alan Tull <atull@...nel.org>
---
v2: add static for __fpga_mgr_get
    function documentation corrections
    use dev_err
v3: bisectibility fix
v4: add API doc updates that were originally in a separate patch
---
 Documentation/fpga/fpga-mgr.txt | 35 ++++++++++++++++++++-------
 drivers/fpga/fpga-mgr.c         | 52 ++++++++++++++++++++++++++++-------------
 drivers/fpga/fpga-region.c      | 14 +++++++++--
 include/linux/fpga/fpga-mgr.h   |  3 +++
 4 files changed, 77 insertions(+), 27 deletions(-)

diff --git a/Documentation/fpga/fpga-mgr.txt b/Documentation/fpga/fpga-mgr.txt
index 6ebc714..cc6413e 100644
--- a/Documentation/fpga/fpga-mgr.txt
+++ b/Documentation/fpga/fpga-mgr.txt
@@ -48,8 +48,20 @@ To get/put a reference to a FPGA manager:
 	struct fpga_manager *fpga_mgr_get(struct device *dev);
 	void fpga_mgr_put(struct fpga_manager *mgr);
 
-Given a DT node or device, get an exclusive reference to a FPGA manager.
-fpga_mgr_put releases the reference.
+Given a DT node or device, get a reference to a FPGA manager.  This pointer
+can be saved until you are ready to program the FPGA.  fpga_mgr_put releases
+the reference.
+
+
+To get exclusive control of a FPGA manager:
+-------------------------------------------
+
+	int fpga_mgr_lock(struct fpga_manager *mgr);
+	void fpga_mgr_unlock(struct fpga_manager *mgr);
+
+The user should call fpga_mgr_lock and verify that it returns 0 before
+attempting to program the FPGA.  Likewise, the user should call
+fpga_mgr_unlock when done programming the FPGA.
 
 
 To register or unregister the low level FPGA-specific driver:
@@ -67,13 +79,21 @@ device."
 
 How to write an image buffer to a supported FPGA
 ================================================
-/* Include to get the API */
 #include <linux/fpga/fpga-mgr.h>
 
 struct fpga_manager *mgr;
 struct fpga_image_info *info;
 int ret;
 
+/*
+ * Get a reference to FPGA manager.  The manager is not locked, so you can
+ * hold onto this reference without it preventing programming.
+ *
+ * This example uses the device node of the manager.  Alternatively, use
+ * fpga_mgr_get(dev) instead if you have the device.
+ */
+mgr = of_fpga_mgr_get(mgr_node);
+
 /* struct with information about the FPGA image to program. */
 info = fpga_image_info_alloc(dev);
 
@@ -99,17 +119,14 @@ if (image is in a scatter gather table) {
 
 }
 
-/*
- * Get a reference to FPGA manager.  This example uses the  device node of the
- * manager.  You could use fpga_mgr_get() instead if you have the device instead
- * of the device node.
- */
-mgr = of_fpga_mgr_get(mgr_node);
+/* Get exclusive control of FPGA manager */
+ret = fpga_mgr_lock(mgr);
 
 /* Load the buffer to the FPGA */
 ret = fpga_mgr_buf_load(mgr, &info, buf, count);
 
 /* Release the FPGA manager */
+fpga_mgr_unlock(mgr);
 fpga_mgr_put(mgr);
 
 /* Deallocate the image info if you're done with it */
diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index a8dd549..d27e8d2 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -410,28 +410,19 @@ ATTRIBUTE_GROUPS(fpga_mgr);
 static struct fpga_manager *__fpga_mgr_get(struct device *dev)
 {
 	struct fpga_manager *mgr;
-	int ret = -ENODEV;
 
 	mgr = to_fpga_manager(dev);
 	if (!mgr)
 		goto err_dev;
 
-	/* Get exclusive use of fpga manager */
-	if (!mutex_trylock(&mgr->ref_mutex)) {
-		ret = -EBUSY;
-		goto err_dev;
-	}
-
 	if (!try_module_get(dev->parent->driver->owner))
-		goto err_ll_mod;
+		goto err_dev;
 
 	return mgr;
 
-err_ll_mod:
-	mutex_unlock(&mgr->ref_mutex);
 err_dev:
 	put_device(dev);
-	return ERR_PTR(ret);
+	return ERR_PTR(-ENODEV);
 }
 
 static int fpga_mgr_dev_match(struct device *dev, const void *data)
@@ -440,10 +431,10 @@ static int fpga_mgr_dev_match(struct device *dev, const void *data)
 }
 
 /**
- * fpga_mgr_get - get an exclusive reference to a fpga mgr
+ * fpga_mgr_get - get a reference to a fpga mgr
  * @dev:	parent device that fpga mgr was registered with
  *
- * Given a device, get an exclusive reference to a fpga mgr.
+ * Given a device, get a reference to a fpga mgr.
  *
  * Return: fpga manager struct or IS_ERR() condition containing error code.
  */
@@ -464,10 +455,10 @@ static int fpga_mgr_of_node_match(struct device *dev, const void *data)
 }
 
 /**
- * of_fpga_mgr_get - get an exclusive reference to a fpga mgr
+ * of_fpga_mgr_get - get a reference to a fpga mgr
  * @node:	device node
  *
- * Given a device node, get an exclusive reference to a fpga mgr.
+ * Given a device node, get a reference to a fpga mgr.
  *
  * Return: fpga manager struct or IS_ERR() condition containing error code.
  */
@@ -491,12 +482,41 @@ EXPORT_SYMBOL_GPL(of_fpga_mgr_get);
 void fpga_mgr_put(struct fpga_manager *mgr)
 {
 	module_put(mgr->dev.parent->driver->owner);
-	mutex_unlock(&mgr->ref_mutex);
 	put_device(&mgr->dev);
 }
 EXPORT_SYMBOL_GPL(fpga_mgr_put);
 
 /**
+ * fpga_mgr_lock - Lock FPGA manager for exclusive use
+ * @mgr:	fpga manager
+ *
+ * Given a pointer to FPGA Manager (from fpga_mgr_get() or
+ * of_fpga_mgr_put()) attempt to get the mutex.
+ *
+ * Return: 0 for success or -EBUSY
+ */
+int fpga_mgr_lock(struct fpga_manager *mgr)
+{
+	if (!mutex_trylock(&mgr->ref_mutex)) {
+		dev_err(&mgr->dev, "FPGA manager is in use.\n");
+		return -EBUSY;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(fpga_mgr_lock);
+
+/**
+ * fpga_mgr_unlock - Unlock FPGA manager
+ * @mgr:	fpga manager
+ */
+void fpga_mgr_unlock(struct fpga_manager *mgr)
+{
+	mutex_unlock(&mgr->ref_mutex);
+}
+EXPORT_SYMBOL_GPL(fpga_mgr_unlock);
+
+/**
  * fpga_mgr_register - register a low level fpga manager driver
  * @dev:	fpga manager device from pdev
  * @name:	fpga manager name
diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
index 120c496..1e1640a 100644
--- a/drivers/fpga/fpga-region.c
+++ b/drivers/fpga/fpga-region.c
@@ -125,7 +125,7 @@ static void fpga_region_put(struct fpga_region *region)
 }
 
 /**
- * fpga_region_get_manager - get exclusive reference for FPGA manager
+ * fpga_region_get_manager - get reference for FPGA manager
  * @region: FPGA region
  *
  * Get FPGA Manager from "fpga-mgr" property or from ancestor region.
@@ -233,6 +233,7 @@ static int fpga_region_get_bridges(struct fpga_region *region,
 static int fpga_region_program_fpga(struct fpga_region *region,
 				    struct device_node *overlay)
 {
+	struct device *dev = &region->dev;
 	struct fpga_manager *mgr;
 	int ret;
 
@@ -249,10 +250,16 @@ static int fpga_region_program_fpga(struct fpga_region *region,
 		goto err_put_region;
 	}
 
+	ret = fpga_mgr_lock(mgr);
+	if (ret) {
+		dev_err(dev, "FPGA manager is busy\n");
+		goto err_put_mgr;
+	}
+
 	ret = fpga_region_get_bridges(region, overlay);
 	if (ret) {
 		pr_err("failed to get fpga region bridges\n");
-		goto err_put_mgr;
+		goto err_unlock_mgr;
 	}
 
 	ret = fpga_bridges_disable(&region->bridge_list);
@@ -273,6 +280,7 @@ static int fpga_region_program_fpga(struct fpga_region *region,
 		goto err_put_br;
 	}
 
+	fpga_mgr_unlock(mgr);
 	fpga_mgr_put(mgr);
 	fpga_region_put(region);
 
@@ -280,6 +288,8 @@ static int fpga_region_program_fpga(struct fpga_region *region,
 
 err_put_br:
 	fpga_bridges_put(&region->bridge_list);
+err_unlock_mgr:
+	fpga_mgr_unlock(mgr);
 err_put_mgr:
 	fpga_mgr_put(mgr);
 err_put_region:
diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
index be371e6..5e2b36e 100644
--- a/include/linux/fpga/fpga-mgr.h
+++ b/include/linux/fpga/fpga-mgr.h
@@ -155,6 +155,9 @@ void fpga_image_info_free(struct fpga_image_info *info);
 
 int fpga_mgr_load(struct fpga_manager *mgr, struct fpga_image_info *info);
 
+int fpga_mgr_lock(struct fpga_manager *mgr);
+void fpga_mgr_unlock(struct fpga_manager *mgr);
+
 struct fpga_manager *of_fpga_mgr_get(struct device_node *node);
 
 struct fpga_manager *fpga_mgr_get(struct device *dev);
-- 
2.7.4

Powered by blists - more mailing lists