Skip to contentSkip to conversation

CCF fixlets

PATCH In review Stephen Boyd 2 participants 6 messages Mar 26, 2014 – Apr 29, 2014 Common clk

lore
Share & tools
lore
#0 lore raw

cover CCF fixlets

Two small changes to clean up the code plus a change to make driver code simpler.

Stephen Boyd (3):
clk: Don't check for missing ops in clk_set_parent()
clk: Consolidate recalc rate logic
clk: Ignore error and NULL pointers passed to
clk_{unprepare,disable}()

 drivers/clk/clk.c | 39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#1 lore raw

PATCH 1/3 clk: Don't check for missing ops in clk_set_parent()

We dereference clk->ops during clock registration so this check for NULL ops can't possibly ever be true.

---
 drivers/clk/clk.c | 3 ---
 1 file changed, 3 deletions(-)
drivers/clk/clk.c +0 -3
index fb3c40b4fbe2..10e0afaefe2f 100644
@@ -1712,9 +1712,6 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
 	if (!clk)
 		return 0;
 
-	if (!clk->ops)
-		return -EINVAL;
-
 	/* verify ops for for multi-parent clks */
 	if ((clk->num_parents > 1) && (!clk->ops->set_parent))
 		return -ENOSYS;
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#2 lore raw

PATCH 2/3 clk: Consolidate recalc rate logic

In reply to Stephen Boyd

The same if-else statement exists four times to recalculate the rate of a clock. Consolidate this logic into a single function to save some lines.

---
 drivers/clk/clk.c | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)
drivers/clk/clk.c +11 -16
index 10e0afaefe2f..e924401c0030 100644
@@ -1111,6 +1111,13 @@ long clk_get_accuracy(struct clk *clk)
 }
 EXPORT_SYMBOL_GPL(clk_get_accuracy);
 
+static unsigned long clk_recalc(struct clk *clk, unsigned long parent_rate)
+{
+	if (clk->ops->recalc_rate)
+		return clk->ops->recalc_rate(clk->hw, parent_rate);
+	return parent_rate;
+}
+
 /**
  * __clk_recalc_rates
  * @clk: first clk in the subtree
@@ -1136,10 +1143,7 @@ static void __clk_recalc_rates(struct clk *clk, unsigned long msg)
 	if (clk->parent)
 		parent_rate = clk->parent->rate;
 
-	if (clk->ops->recalc_rate)
-		clk->rate = clk->ops->recalc_rate(clk->hw, parent_rate);
-	else
-		clk->rate = parent_rate;
+	clk->rate = clk_recalc(clk, parent_rate);
 
 	/*
 	 * ignore NOTIFY_STOP and NOTIFY_BAD return values for POST_RATE_CHANGE
@@ -1330,10 +1334,7 @@ static int __clk_speculate_rates(struct clk *clk, unsigned long parent_rate)
 	unsigned long new_rate;
 	int ret = NOTIFY_DONE;
 
-	if (clk->ops->recalc_rate)
-		new_rate = clk->ops->recalc_rate(clk->hw, parent_rate);
-	else
-		new_rate = parent_rate;
+	new_rate = clk_recalc(clk, parent_rate);
 
 	/* abort rate change if a driver returns NOTIFY_BAD or NOTIFY_STOP */
 	if (clk->notifier_count)
@@ -1369,10 +1370,7 @@ static void clk_calc_subtree(struct clk *clk, unsigned long new_rate,
 		new_parent->new_child = clk;
 
 	hlist_for_each_entry(child, &clk->children, child_node) {
-		if (child->ops->recalc_rate)
-			child->new_rate = child->ops->recalc_rate(child->hw, new_rate);
-		else
-			child->new_rate = new_rate;
+		child->new_rate = clk_recalc(child, new_rate);
 		clk_calc_subtree(child, child->new_rate, NULL, 0);
 	}
 }
@@ -1520,10 +1518,7 @@ static void clk_change_rate(struct clk *clk)
 	if (!skip_set_rate && clk->ops->set_rate)
 		clk->ops->set_rate(clk->hw, clk->new_rate, best_parent_rate);
 
-	if (clk->ops->recalc_rate)
-		clk->rate = clk->ops->recalc_rate(clk->hw, best_parent_rate);
-	else
-		clk->rate = best_parent_rate;
+	clk->rate = clk_recalc(clk, best_parent_rate);
 
 	if (clk->notifier_count && old_rate != clk->rate)
 		__clk_notify(clk, POST_RATE_CHANGE, old_rate, clk->rate);
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#3 lore raw

PATCH 3/3 clk: Ignore error and NULL pointers passed to clk_{unprepare,disable}()

In reply to Stephen Boyd

This simplifies error paths in drivers that use optional clocks by allowing the NULL or error pointer to be passed unconditionally.

---
 drivers/clk/clk.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/clk/clk.c +6 -3
index e924401c0030..d7d359b0f76d 100644
@@ -818,6 +818,9 @@ void __clk_unprepare(struct clk *clk)
  */
 void clk_unprepare(struct clk *clk)
 {
+	if (IS_ERR_OR_NULL(clk))
+		return;
+
 	clk_prepare_lock();
 	__clk_unprepare(clk);
 	clk_prepare_unlock();
@@ -879,9 +882,6 @@ static void __clk_disable(struct clk *clk)
 	if (!clk)
 		return;
 
-	if (WARN_ON(IS_ERR(clk)))
-		return;
-
 	if (WARN_ON(clk->enable_count == 0))
 		return;
 
@@ -910,6 +910,9 @@ void clk_disable(struct clk *clk)
 {
 	unsigned long flags;
 
+	if (IS_ERR_OR_NULL(clk))
+		return;
+
 	flags = clk_enable_lock();
 	__clk_disable(clk);
 	clk_enable_unlock(flags);
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#4 lore raw
In reply to Stephen Boyd
Quoted 4 lines · Stephen Boyd

Two small changes to clean up the code plus a change to make driver code simpler.

ping?

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#5 lore raw

cover CCF fixlets

In reply to Stephen Boyd

Quoting Stephen Boyd (2014-03-26 16:06:34)

Quoted 2 lines

Two small changes to clean up the code plus a change to make driver code simpler.

Take into clk-next. Thanks for the fixlets.

Regards,
Mike

Quoted 14 lines

Stephen Boyd (3):
clk: Don't check for missing ops in clk_set_parent()
clk: Consolidate recalc rate logic
clk: Ignore error and NULL pointers passed to
clk_{unprepare,disable}()

 drivers/clk/clk.c | 39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation