Skip to contentSkip to conversation

clk: Enforce error checking

RFC New Thierry Reding 1 participant 1 message May 22, 2014

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

RFC 1/1 clk: Enforce error checking

From: Thierry Reding <[email protected]>

This topic comes up every so often, so I figured that perhaps instead of repeatedly telling people that they should check for errors returned by clk_prepare_enable() and friends we should simply mark them __must_check to settle this matter once and for all.

Unfortunately not even the clock core seems to get this right, though. A few quick builds of a few platforms yield a massive amount of warnings. It looks pretty grim.

---
 drivers/clk/clk.c   | 22 ++++++++++++++++++----
 include/linux/clk.h | 16 ++++++++--------
 2 files changed, 26 insertions(+), 12 deletions(-)
drivers/clk/clk.c +18 -4
index 4d562206d62f..e5dceabef92a 100644
@@ -1240,6 +1240,7 @@ static struct clk *__clk_set_parent_before(struct clk *clk, struct clk *parent)
 {
 	unsigned long flags;
 	struct clk *old_parent = clk->parent;
+	int err;
 
 	/*
 	 * Migrate prepare state between parents and prevent race with
@@ -1260,8 +1261,16 @@ static struct clk *__clk_set_parent_before(struct clk *clk, struct clk *parent)
 	 */
 	if (clk->prepare_count) {
 		__clk_prepare(parent);
-		clk_enable(parent);
-		clk_enable(clk);
+
+		err = clk_enable(parent);
+		if (err < 0)
+			pr_err("%s: failed to enable parent clock: %d\n",
+			       __func__, err);
+
+		err = clk_enable(clk);
+		if (err < 0)
+			pr_err("%s: failed to enable clock: %d\n", __func__,
+			       err);
 	}
 
 	/* update the clk tree topology */
@@ -2136,10 +2145,15 @@ void clk_unregister(struct clk *clk)
 	if (!hlist_empty(&clk->children)) {
 		struct clk *child;
 		struct hlist_node *t;
+		int err;
 
 		/* Reparent all children to the orphan list. */
-		hlist_for_each_entry_safe(child, t, &clk->children, child_node)
-			clk_set_parent(child, NULL);
+		hlist_for_each_entry_safe(child, t, &clk->children, child_node) {
+			err = clk_set_parent(child, NULL);
+			if (err < 0)
+				pr_err("%s: failed to orphan clock %s: %d\n",
+				       __func__, clk->name, err);
+		}
 	}
 
 	clk_debug_unregister(clk);
include/linux/clk.h +8 -8
index fb5e097d8f72..721b554a296f 100644
@@ -124,9 +124,9 @@ static inline long clk_get_accuracy(struct clk *clk)
  * Must not be called from within atomic context.
  */
 #ifdef CONFIG_HAVE_CLK_PREPARE
-int clk_prepare(struct clk *clk);
+int __must_check clk_prepare(struct clk *clk);
 #else
-static inline int clk_prepare(struct clk *clk)
+static inline int __must_check clk_prepare(struct clk *clk)
 {
 	might_sleep();
 	return 0;
@@ -199,7 +199,7 @@ struct clk *devm_clk_get(struct device *dev, const char *id);
  *
  * Returns success (0) or negative errno.
  */
-int clk_enable(struct clk *clk);
+int __must_check clk_enable(struct clk *clk);
 
 /**
  * clk_disable - inform the system when the clock source is no longer required.
@@ -270,7 +270,7 @@ long clk_round_rate(struct clk *clk, unsigned long rate);
  *
  * Returns success (0) or negative errno.
  */
-int clk_set_rate(struct clk *clk, unsigned long rate);
+int __must_check clk_set_rate(struct clk *clk, unsigned long rate);
 
 /**
  * clk_set_parent - set the parent clock source for this clock
@@ -279,7 +279,7 @@ int clk_set_rate(struct clk *clk, unsigned long rate);
  *
  * Returns success (0) or negative errno.
  */
-int clk_set_parent(struct clk *clk, struct clk *parent);
+int __must_check clk_set_parent(struct clk *clk, struct clk *parent);
 
 /**
  * clk_get_parent - get the parent clock source for this clock
@@ -335,7 +335,7 @@ static inline unsigned long clk_get_rate(struct clk *clk)
 	return 0;
 }
 
-static inline int clk_set_rate(struct clk *clk, unsigned long rate)
+static inline int __must_check clk_set_rate(struct clk *clk, unsigned long rate)
 {
 	return 0;
 }
@@ -345,7 +345,7 @@ static inline long clk_round_rate(struct clk *clk, unsigned long rate)
 	return 0;
 }
 
-static inline int clk_set_parent(struct clk *clk, struct clk *parent)
+static inline int __must_check clk_set_parent(struct clk *clk, struct clk *parent)
 {
 	return 0;
 }
@@ -358,7 +358,7 @@ static inline struct clk *clk_get_parent(struct clk *clk)
 #endif
 
 /* clk_prepare_enable helps cases using clk_enable in non-atomic context. */
-static inline int clk_prepare_enable(struct clk *clk)
+static inline int __must_check clk_prepare_enable(struct clk *clk)
 {
 	int ret;
 
Signature · 1 line
1.9.2