Skip to contentSkip to conversation

clk: divider: do not propagate rate change request when unnecessary

PATCH New Shawn Guo 3 participants 3 messages Jun 2, 2013 – Jun 11, 2013 Common clk

lore
Share & tools
lore

Shawn Guo

author
#0 lore raw

PATCH 1/1 clk: divider: do not propagate rate change request when unnecessary

If the current rate of parent clock is sufficient to provide child a requested rate with a proper divider setting, the rate change request should not be propagated. Instead, changing the divider setting is good enough to get child clock run at the requested rate.

On an imx6q clock configuration illustrated below,

ahb --> ipg --> ipg_per
132M 66M 66M

calling clk_set_rate(ipg_per, 22M) with the current clk_divider_bestdiv() implementation will result in the rate change up to ahb level like the following, because of the unnecessary/incorrect rate change propagation.

ahb --> ipg --> ipg_per
66M 22M 22M

Fix the problem by trying to see if the requested rate can be achieved by simply changing the divider value, and in that case return the divider immediately from function clk_divider_bestdiv() as the best one, so that all those unnecessary rate change propagation can be saved.

Reported-by Anson Huang <[email protected]>
Signed-off-by Shawn Guo <[email protected]>
---
 drivers/clk/clk-divider.c |   10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/clk/clk-divider.c +10 -0
index 6d96741..6024e60 100644
@@ -150,6 +150,7 @@ static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
 	struct clk_divider *divider = to_clk_divider(hw);
 	int i, bestdiv = 0;
 	unsigned long parent_rate, best = 0, now, maxdiv;
+	unsigned long parent_rate_saved = *best_parent_rate;
 
 	if (!rate)
 		rate = 1;

@@ -173,6 +174,15 @@ static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
 	for (i = 1; i <= maxdiv; i++) {
 		if (!_is_valid_div(divider, i))
 			continue;
+		if (rate * i == parent_rate_saved) {
+			/*
+			 * It's the most ideal case if the requested rate can be
+			 * divided from parent clock without needing to change
+			 * parent rate, so return the divider immediately.
+			 */
+			*best_parent_rate = parent_rate_saved;
+			return i;
+		}
 		parent_rate = __clk_round_rate(__clk_get_parent(hw->clk),
 				MULT_ROUND_UP(rate, i));
 		now = parent_rate / i;
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH 1/1 clk: divider: do not propagate rate change request when unnecessary

Quoted 48 lines · Shawn Guo

If the current rate of parent clock is sufficient to provide child a requested rate with a proper divider setting, the rate change request should not be propagated. Instead, changing the divider setting is good enough to get child clock run at the requested rate.

On an imx6q clock configuration illustrated below,

   ahb --> ipg --> ipg_per
   132M    66M     66M

calling clk_set_rate(ipg_per, 22M) with the current clk_divider_bestdiv() implementation will result in the rate change up to ahb level like the following, because of the unnecessary/incorrect rate change propagation.

   ahb --> ipg --> ipg_per
   66M     22M     22M

Fix the problem by trying to see if the requested rate can be achieved by simply changing the divider value, and in that case return the divider immediately from function clk_divider_bestdiv() as the best one, so that all those unnecessary rate change propagation can be saved.

Reported-by Anson Huang <[email protected]>
Signed-off-by Shawn Guo <[email protected]>
---
  drivers/clk/clk-divider.c |   10 ++++++++++
  1 file changed, 10 insertions(+)
drivers/clk/clk-divider.c +3 -0
index 6d96741..6024e60 100644
@@ -150,6 +150,7 @@ static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
  	struct clk_divider *divider = to_clk_divider(hw);
  	int i, bestdiv = 0;
  	unsigned long parent_rate, best = 0, now, maxdiv;
+	unsigned long parent_rate_saved = *best_parent_rate;
 
  	if (!rate)
  		rate = 1;

@@ -173,6 +174,15 @@ static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
  	for (i = 1; i <= maxdiv; i++) {
  		if (!_is_valid_div(divider, i))
  			continue;
+		if (rate * i == parent_rate_saved) {
+			/*

drivers/clk/clk-divider.c static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate, · line 188

Quoted 10 lines
drivers/clk/clk-divider.c +7 -0
+			 * It's the most ideal case if the requested rate can be
+			 * divided from parent clock without needing to change
+			 * parent rate, so return the divider immediately.
+			 */
+			*best_parent_rate = parent_rate_saved;
+			return i;
+		}
  		parent_rate = __clk_round_rate(__clk_get_parent(hw->clk),
  				MULT_ROUND_UP(rate, i));
  		now = parent_rate / i;

Is this ready to be applied?

Thanks

Dirk

#2 lore raw

PATCH 1/1 clk: divider: do not propagate rate change request when unnecessary

In reply to Shawn Guo

Quoting Shawn Guo (2013-06-02 07:20:55)

Quoted 18 lines

If the current rate of parent clock is sufficient to provide child a requested rate with a proper divider setting, the rate change request should not be propagated. Instead, changing the divider setting is good enough to get child clock run at the requested rate.

On an imx6q clock configuration illustrated below,

ahb --> ipg --> ipg_per
132M 66M 66M

calling clk_set_rate(ipg_per, 22M) with the current clk_divider_bestdiv() implementation will result in the rate change up to ahb level like the following, because of the unnecessary/incorrect rate change propagation.

ahb --> ipg --> ipg_per
66M 22M 22M

Quoted 7 lines

Fix the problem by trying to see if the requested rate can be achieved by simply changing the divider value, and in that case return the divider immediately from function clk_divider_bestdiv() as the best one, so that all those unnecessary rate change propagation can be saved.

Reported-by Anson Huang <[email protected]>
Signed-off-by Shawn Guo <[email protected]>

I've taken this into clk-next. I'm keeping an eye out for any regressions this change might cause. Any help testing it would be appreciated.

Regards,
Mike

Quoted 34 lines
---
 drivers/clk/clk-divider.c |   10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/clk/clk-divider.c +10 -0
index 6d96741..6024e60 100644
@@ -150,6 +150,7 @@ static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
        struct clk_divider *divider = to_clk_divider(hw);
        int i, bestdiv = 0;
        unsigned long parent_rate, best = 0, now, maxdiv;
+       unsigned long parent_rate_saved = *best_parent_rate;
 
        if (!rate)
                rate = 1;

@@ -173,6 +174,15 @@ static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
        for (i = 1; i <= maxdiv; i++) {
                if (!_is_valid_div(divider, i))
                        continue;
+               if (rate * i == parent_rate_saved) {
+                       /*
+                        * It's the most ideal case if the requested rate can be
+                        * divided from parent clock without needing to change
+                        * parent rate, so return the divider immediately.
+                        */
+                       *best_parent_rate = parent_rate_saved;
+                       return i;
+               }
                parent_rate = __clk_round_rate(__clk_get_parent(hw->clk),
                                MULT_ROUND_UP(rate, i));
                now = parent_rate / i;
Signature · 1 line
1.7.9.5