remoteproc: simplify unregister/free interfaces

Simplify the unregister/free interfaces, and make them easier
to understand and use, by moving to a symmetric and consistent
alloc() -> register() -> unregister() -> free() flow.

To create and register an rproc instance, one needed to invoke
rproc_alloc() followed by rproc_register().

To unregister and free an rproc instance, one now needs to invoke
rproc_unregister() followed by rproc_free().

Cc: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>

diff --git a/Documentation/remoteproc.txt b/Documentation/remoteproc.txt
index 70a048c..ad6ded4 100644 (file)
--- a/Documentation/remoteproc.txt
+++ b/Documentation/remoteproc.txt
@@ -120,14 +120,14 @@ int dummy_rproc_example(struct rproc *my_rproc)
       On success, the new rproc is returned, and on failure, NULL.
 
       Note: _never_ directly deallocate @rproc, even if it was not registered
-      yet. Instead, if you just need to unroll rproc_alloc(), use rproc_free().
+      yet. Instead, when you need to unroll rproc_alloc(), use rproc_free().
 
   void rproc_free(struct rproc *rproc)
     - Free an rproc handle that was allocated by rproc_alloc.
-      This function should _only_ be used if @rproc was only allocated,
-      but not registered yet.
-      If @rproc was already successfully registered (by calling
-      rproc_register()), then use rproc_unregister() instead.
+      This function essentially unrolls rproc_alloc(), by decrementing the
+      rproc's refcount. It doesn't directly free rproc; that would happen
+      only if there are no other references to rproc and its refcount now
+      dropped to zero.
 
   int rproc_register(struct rproc *rproc)
     - Register @rproc with the remoteproc framework, after it has been
@@ -143,19 +143,14 @@ int dummy_rproc_example(struct rproc *my_rproc)
       probed.
 
   int rproc_unregister(struct rproc *rproc)
-    - Unregister a remote processor, and decrement its refcount.
-      If its refcount drops to zero, then @rproc will be freed. If not,
-      it will be freed later once the last reference is dropped.
-
+    - Unroll rproc_register().
       This function should be called when the platform specific rproc
       implementation decides to remove the rproc device. it should
       _only_ be called if a previous invocation of rproc_register()
       has completed successfully.
 
-      After rproc_unregister() returns, @rproc is _not_ valid anymore and
-      it shouldn't be used. More specifically, don't call rproc_free()
-      or try to directly free @rproc after rproc_unregister() returns;
-      none of these are needed, and calling them is a bug.
+      After rproc_unregister() returns, @rproc is still valid, and its
+      last refcount should be decremented by calling rproc_free().
 
       Returns 0 on success and -EINVAL if @rproc isn't valid.
 

diff --git a/drivers/remoteproc/omap_remoteproc.c b/drivers/remoteproc/omap_remoteproc.c
index b5e6d29..4f2fe8f 100644 (file)
--- a/drivers/remoteproc/omap_remoteproc.c
+++ b/drivers/remoteproc/omap_remoteproc.c
@@ -214,7 +214,10 @@ static int __devexit omap_rproc_remove(struct platform_device *pdev)
 {
        struct rproc *rproc = platform_get_drvdata(pdev);
 
-       return rproc_unregister(rproc);
+       rproc_unregister(rproc);
+       rproc_free(rproc);
+
+       return 0;
 }
 
 static struct platform_driver omap_rproc_driver = {

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index aa713aa..4a77dc1 100644 (file)
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1472,7 +1472,7 @@ static struct device_type rproc_type = {
  * On success the new rproc is returned, and on failure, NULL.
  *
  * Note: _never_ directly deallocate @rproc, even if it was not registered
- * yet. Instead, if you just need to unroll rproc_alloc(), use rproc_free().
+ * yet. Instead, when you need to unroll rproc_alloc(), use rproc_free().
  */
 struct rproc *rproc_alloc(struct device *dev, const char *name,
                                const struct rproc_ops *ops,
@@ -1526,14 +1526,13 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
 EXPORT_SYMBOL(rproc_alloc);
 
 /**
- * rproc_free() - free an rproc handle that was allocated by rproc_alloc
+ * rproc_free() - unroll rproc_alloc()
  * @rproc: the remote processor handle
  *
- * This function should _only_ be used if @rproc was only allocated,
- * but not registered yet.
+ * This function decrements the rproc dev refcount.
  *
- * If @rproc was already successfully registered (by calling rproc_register()),
- * then use rproc_unregister() instead.
+ * If no one holds any reference to rproc anymore, then its refcount would
+ * now drop to zero, and it would be freed.
  */
 void rproc_free(struct rproc *rproc)
 {
@@ -1545,19 +1544,14 @@ EXPORT_SYMBOL(rproc_free);
  * rproc_unregister() - unregister a remote processor
  * @rproc: rproc handle to unregister
  *
- * Unregisters a remote processor, and decrements its refcount.
- * If its refcount drops to zero, then @rproc will be freed. If not,
- * it will be freed later once the last reference is dropped.
- *
  * This function should be called when the platform specific rproc
  * implementation decides to remove the rproc device. it should
  * _only_ be called if a previous invocation of rproc_register()
  * has completed successfully.
  *
- * After rproc_unregister() returns, @rproc is _not_ valid anymore and
- * it shouldn't be used. More specifically, don't call rproc_free()
- * or try to directly free @rproc after rproc_unregister() returns;
- * none of these are needed, and calling them is a bug.
+ * After rproc_unregister() returns, @rproc isn't freed yet, because
+ * of the outstanding reference created by rproc_alloc. To decrement that
+ * one last refcount, one still needs to call rproc_free().
  *
  * Returns 0 on success and -EINVAL if @rproc isn't valid.
  */
@@ -1580,9 +1574,6 @@ int rproc_unregister(struct rproc *rproc)
 
        device_del(&rproc->dev);
 
-       /* unroll rproc_alloc. TODO: we may want to let the users do that */
-       put_device(&rproc->dev);
-
        return 0;
 }
 EXPORT_SYMBOL(rproc_unregister);