clk: implement clk_unregister

Currently clk_unregister is unimplemented, it is required in case sub modules want actually remove clk device registered by clk_register. This patch adds the implementation of clk_unregister.

Signed-off-by Jiada Wang <[email protected]>

---
 drivers/clk/clk.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 57 insertions(+), 2 deletions(-)

drivers/clk/clk.c +5 -0

index 934cfd1..0b9e13c 100644
@@ -342,6 +342,25 @@ out:
 	return ret;
 }
 
+ /**
+ * clk_debug_unregister - remove a clk node from the debugfs clk tree
+ * @clk: the clk being removed from the debugfs clk tree
+ *
+ * Dynamically removes a clk and all it's children clk nodes from the

drivers/clk/clk.c +9 -2

+
+	debugfs_remove_recursive(clk->dentry);
+}
+
 /**
  * clk_debug_reparent - reparent clk node in the debugfs clk tree
  * @clk: the clk being reparented

@@ -432,6 +451,9 @@ static inline int clk_debug_register(struct clk *clk) { return 0; }
 static inline void clk_debug_reparent(struct clk *clk, struct clk *new_parent)
 {
 }
+static inline void clk_debug_unregister(struct clk *clk)
+{
+}
 #endif
 
 /* caller must hold prepare_lock */

@@ -1790,9 +1812,42 @@ EXPORT_SYMBOL_GPL(clk_register);
  * clk_unregister - unregister a currently registered clock
  * @clk: clock to unregister
  *
- * Currently unimplemented.
  */
-void clk_unregister(struct clk *clk) {}
+void clk_unregister(struct clk *clk)
+{

drivers/clk/clk.c +23 -0

+	}
+
+	if (!hlist_empty(&clk->children)) {
+		pr_debug("%s: clk %s has registered children\n",
+				__func__, clk->name);
+		goto out;
+	}
+
+	clk_debug_unregister(clk);
+
+	hlist_del_init(&clk->child_node);
+
+	kfree(clk->parents);
+	i = clk->num_parents;
+	while (--i >= 0)
+		kfree(clk->parent_names[i]);
+	kfree(clk->parent_names);
+	kfree(clk->name);
+	kfree(clk);
+out:
+	mutex_unlock(&prepare_lock);
+	return;
+}
 EXPORT_SYMBOL_GPL(clk_unregister);
 
 static void devm_clk_release(struct device *dev, void *res)

Signature · 1 line

1.8.1.1

-----Original Message-----
From: Uwe Kleine-K?nig [mailto:u.kleine-koenig at pengutronix.de]
Sent: Monday, June 03, 2013 6:11 PM
To: Wang, Jiada (ESD)
Cc: linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH v3] clk: implement clk_unregister

Hello,

Quoted 18 lines · Jiada Wang

Currently clk_unregister is unimplemented, it is required in case sub modules want actually remove clk device registered by clk_register. This patch adds the implementation of clk_unregister.

Signed-off-by Jiada Wang <[email protected]>

---
  drivers/clk/clk.c | 59
+++++++++++++++++++++++++++++++++++++++++++++++++++++--
  1 file changed, 57 insertions(+), 2 deletions(-)

drivers/clk/clk.c index renamed from drivers/clk/clk.c +0 -0

934cfd1..0b9e13c 100644

drivers/clk/clk.c +0 -0

@@ -342,6 +342,25 @@ out:
  	return ret;
  }
 

Quoted 4 lines

drivers/clk/clk.c +4 -0

+ /**
+ * clk_debug_unregister - remove a clk node from the debugfs clk tree
+ * @clk: the clk being removed from the debugfs clk tree
+ *

drivers/clk/clk.c out: · line 359

Quoted 10 lines

drivers/clk/clk.c +10 -0

+ * Dynamically removes a clk and all it's children clk nodes from the
+ * debugfs clk tree if clk->dentry points to debugfs created by
+ * clk_debug_register in __clk_init.
+ *
+ * Caller must hold prepare_lock.
+ *
+ */
+static void clk_debug_unregister(struct clk *clk) {
+	if (!clk || !clk->dentry)
+		return;

!clk can never trigger because clk_unregister doesn't call it in this case and debugfs_remove_recursive checks for dentry being NULL itself. So you can drop this check. Maybe you can even call debugfs_remove_recursive even in the case where CONFIG_COMMON_CLK_DEBUG is off?

Quoted 23 lines

drivers/clk/clk.c +7 -2

+
+	debugfs_remove_recursive(clk->dentry);
+}
+
  /**
   * clk_debug_reparent - reparent clk node in the debugfs clk tree
   * @clk: the clk being reparented

@@ -432,6 +451,9 @@ static inline int clk_debug_register(struct clk
 *clk) { return 0; }  static inline void clk_debug_reparent(struct clk
 *clk, struct clk *new_parent)  {  }
+static inline void clk_debug_unregister(struct clk *clk) { }
  #endif
 
  /* caller must hold prepare_lock */

@@ -1790,9 +1812,42 @@ EXPORT_SYMBOL_GPL(clk_register);
   * clk_unregister - unregister a currently registered clock
   * @clk: clock to unregister
   *
- * Currently unimplemented.
   */
-void clk_unregister(struct clk *clk) {}
+void clk_unregister(struct clk *clk)
+{

drivers/clk/clk.c EXPORT_SYMBOL_GPL(clk_register); · line 1827

Quoted 10 lines

drivers/clk/clk.c +10 -0

+	int i;
+
+	if (!clk)
+		return;
+
+	mutex_lock(&prepare_lock);
+	if (clk->prepare_count) {
+		pr_debug("%s: can't unregister clk %s, it is prepared\n",
+				__func__, clk->name);
+		goto out;

Hmm, there is no way for the caller to detect this error. Probably it cannot do anything about it most of the time, but still it would be nice to signal the problem by letting clk_unregister return an int instead of void.

Best regards
Uwe

Quoted 30 lines

drivers/clk/clk.c +23 -1

+	}
+
+	if (!hlist_empty(&clk->children)) {
+		pr_debug("%s: clk %s has registered children\n",
+				__func__, clk->name);
+		goto out;
+	}
+
+	clk_debug_unregister(clk);
+
+	hlist_del_init(&clk->child_node);
+
+	kfree(clk->parents);
+	i = clk->num_parents;
+	while (--i >= 0)
+		kfree(clk->parent_names[i]);
+	kfree(clk->parent_names);
+	kfree(clk->name);
+	kfree(clk);
+out:
+	mutex_unlock(&prepare_lock);
+	return;
+}
  EXPORT_SYMBOL_GPL(clk_unregister);
 
  static void devm_clk_release(struct device *dev, void *res)
--

1.8.1.1

Currently clk_unregister is unimplemented, it is required in case sub modules want actually remove clk device registered by clk_register. This patch adds the implementation of clk_unregister.

-----Original Message-----
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: Monday, June 03, 2013 6:46 PM
To: Wang, Jiada (ESD)
Cc: linux-arm-kernel at lists.infradead.org; u.kleine-koenig at pengutronix.de
Subject: Re: [PATCH v3] clk: implement clk_unregister

On Mon, Jun 03, 2013 at 05:37:06PM +0900, Jiada Wang wrote:

Quoted 3 lines

Currently clk_unregister is unimplemented, it is required in case sub modules want actually remove clk device registered by clk_register. This patch adds the implementation of clk_unregister.

So what if someone has clk_get()'d this clock but not prepared it?
No, this is the wrong approach. If we want to do this then clocks must be reference counted and freed when the refcount gets to zero, just like struct device etc.

We have struct kref as the base structure in the kernel for dealing with these problems, please look at using it.

Currently clk_unregister is unimplemented, it is required in case sub modules want actually remove clk device registered by clk_register. This patch adds the implementation of clk_unregister.

Signed-off-by Jiada Wang <[email protected]>

---
 drivers/clk/clk.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 57 insertions(+), 2 deletions(-)

drivers/clk/clk.c +30 -2

index 934cfd1..0b9e13c 100644
@@ -342,6 +342,25 @@ out:
 	return ret;
 }
 
+ /**
+ * clk_debug_unregister - remove a clk node from the debugfs clk tree
+ * @clk: the clk being removed from the debugfs clk tree
+ *
+ * Dynamically removes a clk and all it's children clk nodes from the
+ * debugfs clk tree if clk->dentry points to debugfs created by
+ * clk_debug_register in __clk_init.
+ *
+ * Caller must hold prepare_lock.
+ *
+ */
+static void clk_debug_unregister(struct clk *clk)
+{
+	if (!clk || !clk->dentry)
+		return;
+
+	debugfs_remove_recursive(clk->dentry);
+}
+
 /**
  * clk_debug_reparent - reparent clk node in the debugfs clk tree
  * @clk: the clk being reparented

@@ -432,6 +451,9 @@ static inline int clk_debug_register(struct clk *clk) { return 0; }
 static inline void clk_debug_reparent(struct clk *clk, struct clk *new_parent)
 {
 }
+static inline void clk_debug_unregister(struct clk *clk)
+{
+}
 #endif
 
 /* caller must hold prepare_lock */

@@ -1790,9 +1812,42 @@ EXPORT_SYMBOL_GPL(clk_register);
  * clk_unregister - unregister a currently registered clock
  * @clk: clock to unregister
  *
- * Currently unimplemented.
  */
-void clk_unregister(struct clk *clk) {}
+void clk_unregister(struct clk *clk)
+{
+	int i;
+
+	if (!clk)
+		return;
+
+	mutex_lock(&prepare_lock);

drivers/clk/clk.c +6 -0

+	}
+
+	clk_debug_unregister(clk);
+
+	hlist_del_init(&clk->child_node);
+