fpga manager fixes
cover fpga manager fixes
From: Alan Tull <[email protected]>
Two patches to fix some issues that were brought up in the review of "add FPGA manager core".
Alan
Alan Tull (2):
fpga manager: ensure lifetime with of_fpga_mgr_get fpga manager: remove unnecessary null pointer checks
drivers/fpga/fpga-mgr.c | 44 +++++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 23 deletions(-)
Signature · 1 line
1.7.9.5
PATCH 1/2 fpga manager: ensure lifetime with of_fpga_mgr_get
From: Alan Tull <[email protected]>
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.
--- drivers/fpga/fpga-mgr.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-)
drivers/fpga/fpga-mgr.c +17 -15
@@ -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);
Signature · 1 line
1.7.9.5
PATCH 2/2 fpga manager: remove unnecessary null pointer checks
In reply to [email protected]From: Alan Tull <[email protected]>
Remove unnecessary null pointer checks. We want the caller of these functions to do their own pointer checks. Add some comments to document this.
--- drivers/fpga/fpga-mgr.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)
drivers/fpga/fpga-mgr.c +4 -8
@@ -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 ;
Signature · 1 line
1.7.9.5
Re: [PATCH 2/2] fpga manager: remove unnecessary null pointer checks
Quoted 7 lines · On Thu, Oct 22, 2015 at 10:38 AM, <[email protected]> wrote:
From: Alan Tull <[email protected]>
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 <[email protected]>
Looks good!