* [PATCH 1/2] fpga manager: ensure lifetime with of_fpga_mgr_get
2015-10-22 17:38 [PATCH 0/2] fpga manager fixes atull
@ 2015-10-22 17:38 ` atull
2015-10-22 17:38 ` [PATCH 2/2] fpga manager: remove unnecessary null pointer checks atull
1 sibling, 0 replies; 4+ messages in thread
From: atull @ 2015-10-22 17:38 UTC (permalink / raw)
To: gregkh
Cc: Josh Cartwright, Moritz Fischer, linux-kernel, delicious.quinoa,
dinguyen, Alan Tull
From: Alan Tull <atull@opensource.altera.com>
Ensure device and driver lifetime from of_fpga_mgr_get() to
fpga_mgr_put().
* Don't put_device() in of_fpga_mgr_get, do it in fpga_mgr_put().
(still do put_device if there is an error).
* Do module_get on the low level driver.
* Don't need to module_get(THIS_MODULE) since we won't be allowed
to unload the fpga manager core without unloading low level
driver first.
* Remove unnedessary null check for node pointer.
Signed-off-by: Alan Tull <atull@opensource.altera.com>
---
drivers/fpga/fpga-mgr.c | 32 +++++++++++++++++---------------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index 2526163..68d7b41 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -204,9 +204,7 @@ struct fpga_manager *of_fpga_mgr_get(struct device_node *node)
{
struct fpga_manager *mgr;
struct device *dev;
-
- if (!node)
- return ERR_PTR(-EINVAL);
+ int ret = -ENODEV;
dev = class_find_device(fpga_mgr_class, NULL, node,
fpga_mgr_of_node_match);
@@ -214,20 +212,25 @@ struct fpga_manager *of_fpga_mgr_get(struct device_node *node)
return ERR_PTR(-ENODEV);
mgr = to_fpga_manager(dev);
- put_device(dev);
if (!mgr)
- return ERR_PTR(-ENODEV);
+ goto err_dev;
/* Get exclusive use of fpga manager */
- if (!mutex_trylock(&mgr->ref_mutex))
- return ERR_PTR(-EBUSY);
-
- if (!try_module_get(THIS_MODULE)) {
- mutex_unlock(&mgr->ref_mutex);
- return ERR_PTR(-ENODEV);
+ if (!mutex_trylock(&mgr->ref_mutex)) {
+ ret = -EBUSY;
+ goto err_dev;
}
+ if (!try_module_get(dev->parent->driver->owner))
+ goto err_ll_mod;
+
return mgr;
+
+err_ll_mod:
+ mutex_unlock(&mgr->ref_mutex);
+err_dev:
+ put_device(dev);
+ return ERR_PTR(ret);
}
EXPORT_SYMBOL_GPL(of_fpga_mgr_get);
@@ -237,10 +240,9 @@ EXPORT_SYMBOL_GPL(of_fpga_mgr_get);
*/
void fpga_mgr_put(struct fpga_manager *mgr)
{
- if (mgr) {
- module_put(THIS_MODULE);
- mutex_unlock(&mgr->ref_mutex);
- }
+ module_put(mgr->dev.parent->driver->owner);
+ mutex_unlock(&mgr->ref_mutex);
+ put_device(&mgr->dev);
}
EXPORT_SYMBOL_GPL(fpga_mgr_put);
--
1.7.9.5
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 2/2] fpga manager: remove unnecessary null pointer checks
2015-10-22 17:38 [PATCH 0/2] fpga manager fixes atull
2015-10-22 17:38 ` [PATCH 1/2] fpga manager: ensure lifetime with of_fpga_mgr_get atull
@ 2015-10-22 17:38 ` atull
2015-10-22 17:50 ` Moritz Fischer
1 sibling, 1 reply; 4+ messages in thread
From: atull @ 2015-10-22 17:38 UTC (permalink / raw)
To: gregkh
Cc: Josh Cartwright, Moritz Fischer, linux-kernel, delicious.quinoa,
dinguyen, Alan Tull
From: Alan Tull <atull@opensource.altera.com>
Remove unnecessary null pointer checks. We want the caller of
these functions to do their own pointer checks. Add some
comments to document this.
Signed-off-by: Alan Tull <atull@opensource.altera.com>
---
drivers/fpga/fpga-mgr.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index 68d7b41..a24f5cb 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -38,7 +38,8 @@ static struct class *fpga_mgr_class;
*
* Step the low level fpga manager through the device-specific steps of getting
* an FPGA ready to be configured, writing the image to it, then doing whatever
- * post-configuration steps necessary.
+ * post-configuration steps necessary. This code assumes the caller got the
+ * mgr pointer from of_fpga_mgr_get() and checked that it is not an error code.
*
* Return: 0 on success, negative error code otherwise.
*/
@@ -48,9 +49,6 @@ int fpga_mgr_buf_load(struct fpga_manager *mgr, u32 flags, const char *buf,
struct device *dev = &mgr->dev;
int ret;
- if (!mgr)
- return -ENODEV;
-
/*
* 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
@@ -100,7 +98,8 @@ EXPORT_SYMBOL_GPL(fpga_mgr_buf_load);
*
* Request 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.
+ * there is a failure. This code assumes the caller got the mgr pointer
+ * from of_fpga_mgr_get() and checked that it is not an error code.
*
* Return: 0 on success, negative error code otherwise.
*/
@@ -111,9 +110,6 @@ int fpga_mgr_firmware_load(struct fpga_manager *mgr, u32 flags,
const struct firmware *fw;
int ret;
- if (!mgr)
- return -ENODEV;
-
dev_info(dev, "writing %s to %s\n", image_name, mgr->name);
mgr->state = FPGA_MGR_STATE_FIRMWARE_REQ;
--
1.7.9.5
^ permalink raw reply [flat|nested] 4+ messages in thread