Skip to contentSkip to conversation

clk: factor: calculate rate by do_div

PATCH In review Haojian Zhuang 2 participants 7 messages Dec 3, 2012 – Jan 12, 2013 Common clk

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 clk: factor: calculate rate by do_div

clk->rate = parent->rate / div * mult

The formula is OK. But it may overflow while we do operate with unsigned long. So use do_div instead.

---
 drivers/clk/clk-fixed-factor.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/clk/clk-fixed-factor.c +4 -1
index a489985..1ef271e 100644
@@ -28,8 +28,11 @@ static unsigned long clk_factor_recalc_rate(struct clk_hw *hw,
 		unsigned long parent_rate)
 {
 	struct clk_fixed_factor *fix = to_clk_fixed_factor(hw);
+	unsigned long long int rate;
 
-	return parent_rate * fix->mult / fix->div;
+	rate = (unsigned long long int)parent_rate * fix->mult;
+	do_div(rate, fix->div);
+	return (unsigned long)rate;
 }
 
 static long clk_factor_round_rate(struct clk_hw *hw, unsigned long rate,
Signature · 1 line
1.7.10.4
#1 lore raw
Quoted 28 lines · Haojian Zhuang

clk->rate = parent->rate / div * mult

The formula is OK. But it may overflow while we do operate with unsigned long. So use do_div instead.

---
 drivers/clk/clk-fixed-factor.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/clk/clk-fixed-factor.c +4 -2
index a489985..1ef271e 100644
@@ -28,8 +28,11 @@ static unsigned long clk_factor_recalc_rate(struct clk_hw *hw,
                unsigned long parent_rate)
 {
        struct clk_fixed_factor *fix = to_clk_fixed_factor(hw);
+       unsigned long long int rate;
 
-       return parent_rate * fix->mult / fix->div;
+       rate = (unsigned long long int)parent_rate * fix->mult;
+       do_div(rate, fix->div);
+       return (unsigned long)rate;
 }
 
 static long clk_factor_round_rate(struct clk_hw *hw, unsigned long rate,
--
Quoted 2 lines

1.7.10.4

Correct Mike's email address.

#2 lore raw
Quoted 27 lines · Haojian Zhuang
Quoted 26 lines · Haojian Zhuang

clk->rate = parent->rate / div * mult

The formula is OK. But it may overflow while we do operate with unsigned long. So use do_div instead.

---
 drivers/clk/clk-fixed-factor.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/clk/clk-fixed-factor.c +4 -1
index a489985..1ef271e 100644
@@ -28,8 +28,11 @@ static unsigned long clk_factor_recalc_rate(struct clk_hw *hw,
                unsigned long parent_rate)
 {
        struct clk_fixed_factor *fix = to_clk_fixed_factor(hw);
+       unsigned long long int rate;
 
-       return parent_rate * fix->mult / fix->div;
+       rate = (unsigned long long int)parent_rate * fix->mult;
+       do_div(rate, fix->div);
+       return (unsigned long)rate;
 }
 
Quoted 6 lines
Quoted 4 lines
drivers/clk/clk-fixed-factor.c +0 -1
 static long clk_factor_round_rate(struct clk_hw *hw, unsigned long rate,
--

1.7.10.4

Correct Mike's email address.

Any comments? Does it mean that nobody want to fix the bug?

#3 lore raw
Quoted 28 lines · Haojian Zhuang
Quoted 27 lines · Haojian Zhuang
Quoted 26 lines · Haojian Zhuang

clk->rate = parent->rate / div * mult

The formula is OK. But it may overflow while we do operate with unsigned long. So use do_div instead.

---
 drivers/clk/clk-fixed-factor.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/clk/clk-fixed-factor.c +4 -1
index a489985..1ef271e 100644
@@ -28,8 +28,11 @@ static unsigned long clk_factor_recalc_rate(struct clk_hw *hw,
                unsigned long parent_rate)
 {
        struct clk_fixed_factor *fix = to_clk_fixed_factor(hw);
+       unsigned long long int rate;
 
-       return parent_rate * fix->mult / fix->div;
+       rate = (unsigned long long int)parent_rate * fix->mult;
+       do_div(rate, fix->div);
+       return (unsigned long)rate;
 }
 
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
drivers/clk/clk-fixed-factor.c +0 -1
 static long clk_factor_round_rate(struct clk_hw *hw, unsigned long rate,
--

1.7.10.4

Correct Mike's email address.

Any comments? Does it mean that nobody want to fix the bug?

Thanks for the patch. My apologies for letting this one slip through the cracks but my normal email workflow was unavoidably disrupted and I find myself playing catch-up with pending patches.

The patch looks good to me but I'll change the $SUBJECT to "clk:
fixed-factor: round_rate should use do_div" and do some testing before taking it in.

Regards,
Mike

#4 lore raw
Quoted 43 lines · Mike Turquette
Quoted 28 lines · Haojian Zhuang
Quoted 27 lines · Haojian Zhuang
Quoted 26 lines · Haojian Zhuang

clk->rate = parent->rate / div * mult

The formula is OK. But it may overflow while we do operate with unsigned long. So use do_div instead.

---
 drivers/clk/clk-fixed-factor.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/clk/clk-fixed-factor.c +4 -1
index a489985..1ef271e 100644
@@ -28,8 +28,11 @@ static unsigned long clk_factor_recalc_rate(struct clk_hw *hw,
                unsigned long parent_rate)
 {
        struct clk_fixed_factor *fix = to_clk_fixed_factor(hw);
+       unsigned long long int rate;
 
-       return parent_rate * fix->mult / fix->div;
+       rate = (unsigned long long int)parent_rate * fix->mult;
+       do_div(rate, fix->div);
+       return (unsigned long)rate;
 }
 
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
drivers/clk/clk-fixed-factor.c +0 -1
 static long clk_factor_round_rate(struct clk_hw *hw, unsigned long rate,
--

1.7.10.4

Correct Mike's email address.

Any comments? Does it mean that nobody want to fix the bug?

Thanks for the patch. My apologies for letting this one slip through the cracks but my normal email workflow was unavoidably disrupted and I find myself playing catch-up with pending patches.

Quoted 6 lines

The patch looks good to me but I'll change the $SUBJECT to "clk:
fixed-factor: round_rate should use do_div" and do some testing before taking it in.

Regards,
Mike

It's nice. Thank you.

Best Regards
Haojian

#5 lore raw

PATCH 1/1 clk: factor: calculate rate by do_div

Quoting Haojian Zhuang (2012-12-15 22:27:58)

Quoted 44 lines · Mike Turquette
Quoted 43 lines
Quoted 28 lines · Haojian Zhuang
Quoted 27 lines · Haojian Zhuang
Quoted 26 lines · Haojian Zhuang

clk->rate = parent->rate / div * mult

The formula is OK. But it may overflow while we do operate with unsigned long. So use do_div instead.

---
 drivers/clk/clk-fixed-factor.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/clk/clk-fixed-factor.c +4 -1
index a489985..1ef271e 100644
@@ -28,8 +28,11 @@ static unsigned long clk_factor_recalc_rate(struct clk_hw *hw,
                unsigned long parent_rate)
 {
        struct clk_fixed_factor *fix = to_clk_fixed_factor(hw);
+       unsigned long long int rate;
 
-       return parent_rate * fix->mult / fix->div;
+       rate = (unsigned long long int)parent_rate * fix->mult;
+       do_div(rate, fix->div);
+       return (unsigned long)rate;
 }
 
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
drivers/clk/clk-fixed-factor.c +0 -1
 static long clk_factor_round_rate(struct clk_hw *hw, unsigned long rate,
--

1.7.10.4

Correct Mike's email address.

Any comments? Does it mean that nobody want to fix the bug?

Thanks for the patch. My apologies for letting this one slip through the cracks but my normal email workflow was unavoidably disrupted and I find myself playing catch-up with pending patches.

Quoted 9 lines
Quoted 6 lines

The patch looks good to me but I'll change the $SUBJECT to "clk:
fixed-factor: round_rate should use do_div" and do some testing before taking it in.

Regards,
Mike

It's nice. Thank you.

Pulled the fix into clk-next.

Thanks for the patch,
Mike

Quoted 2 lines

Best Regards
Haojian

#6 lore raw
In reply to Haojian Zhuang
Quoted 27 lines · Haojian Zhuang
Quoted 26 lines · Haojian Zhuang

clk->rate = parent->rate / div * mult

The formula is OK. But it may overflow while we do operate with unsigned long. So use do_div instead.

---
 drivers/clk/clk-fixed-factor.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/clk/clk-fixed-factor.c +4 -1
index a489985..1ef271e 100644
@@ -28,8 +28,11 @@ static unsigned long clk_factor_recalc_rate(struct clk_hw *hw,
                unsigned long parent_rate)
 {
        struct clk_fixed_factor *fix = to_clk_fixed_factor(hw);
+       unsigned long long int rate;
 
-       return parent_rate * fix->mult / fix->div;
+       rate = (unsigned long long int)parent_rate * fix->mult;
+       do_div(rate, fix->div);
+       return (unsigned long)rate;
 }
 
Quoted 6 lines
Quoted 4 lines
drivers/clk/clk-fixed-factor.c +0 -1
 static long clk_factor_round_rate(struct clk_hw *hw, unsigned long rate,
--

1.7.10.4

Correct Mike's email address.

Any comments? It's the bug what needs to be fixed.