linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] clk: factor: calculate rate by do_div
@ 2012-12-03  8:14 Haojian Zhuang
  2012-12-04  1:32 ` Haojian Zhuang
  0 siblings, 1 reply; 6+ messages in thread
From: Haojian Zhuang @ 2012-12-03  8:14 UTC (permalink / raw)
  To: mturquette, linux-arm-kernel, linux-kernel; +Cc: 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.

Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>
---
 drivers/clk/clk-fixed-factor.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
index a489985..1ef271e 100644
--- a/drivers/clk/clk-fixed-factor.c
+++ b/drivers/clk/clk-fixed-factor.c
@@ -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,
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] clk: factor: calculate rate by do_div
  2012-12-03  8:14 [PATCH] clk: factor: calculate rate by do_div Haojian Zhuang
@ 2012-12-04  1:32 ` Haojian Zhuang
  2012-12-15 16:41   ` Haojian Zhuang
  2012-12-15 16:42   ` Haojian Zhuang
  0 siblings, 2 replies; 6+ messages in thread
From: Haojian Zhuang @ 2012-12-04  1:32 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, mike.turquette; +Cc: Haojian Zhuang

On Mon, Dec 3, 2012 at 4:14 PM, Haojian Zhuang <haojian.zhuang@gmail.com> wrote:
> 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.
>
> Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>
> ---
>  drivers/clk/clk-fixed-factor.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
> index a489985..1ef271e 100644
> --- a/drivers/clk/clk-fixed-factor.c
> +++ b/drivers/clk/clk-fixed-factor.c
> @@ -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,
> --
> 1.7.10.4
>

Correct Mike's email address.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] clk: factor: calculate rate by do_div
  2012-12-04  1:32 ` Haojian Zhuang
@ 2012-12-15 16:41   ` Haojian Zhuang
  2012-12-15 20:54     ` Mike Turquette
  2012-12-15 16:42   ` Haojian Zhuang
  1 sibling, 1 reply; 6+ messages in thread
From: Haojian Zhuang @ 2012-12-15 16:41 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, Mike Turquette, Bergmann Arnd
  Cc: Haojian Zhuang

On Tue, Dec 4, 2012 at 9:32 AM, Haojian Zhuang <haojian.zhuang@gmail.com> wrote:
> On Mon, Dec 3, 2012 at 4:14 PM, Haojian Zhuang <haojian.zhuang@gmail.com> wrote:
>> 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.
>>
>> Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>
>> ---
>>  drivers/clk/clk-fixed-factor.c |    5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
>> index a489985..1ef271e 100644
>> --- a/drivers/clk/clk-fixed-factor.c
>> +++ b/drivers/clk/clk-fixed-factor.c
>> @@ -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,
>> --
>> 1.7.10.4
>>
>
> Correct Mike's email address.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] clk: factor: calculate rate by do_div
  2012-12-04  1:32 ` Haojian Zhuang
  2012-12-15 16:41   ` Haojian Zhuang
@ 2012-12-15 16:42   ` Haojian Zhuang
  1 sibling, 0 replies; 6+ messages in thread
From: Haojian Zhuang @ 2012-12-15 16:42 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, Mike Turquette, Bergmann Arnd
  Cc: Haojian Zhuang

On Tue, Dec 4, 2012 at 9:32 AM, Haojian Zhuang <haojian.zhuang@gmail.com> wrote:
> On Mon, Dec 3, 2012 at 4:14 PM, Haojian Zhuang <haojian.zhuang@gmail.com> wrote:
>> 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.
>>
>> Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>
>> ---
>>  drivers/clk/clk-fixed-factor.c |    5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
>> index a489985..1ef271e 100644
>> --- a/drivers/clk/clk-fixed-factor.c
>> +++ b/drivers/clk/clk-fixed-factor.c
>> @@ -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,
>> --
>> 1.7.10.4
>>
>
> Correct Mike's email address.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] clk: factor: calculate rate by do_div
  2012-12-15 16:41   ` Haojian Zhuang
@ 2012-12-15 20:54     ` Mike Turquette
  2012-12-16  6:27       ` Haojian Zhuang
  0 siblings, 1 reply; 6+ messages in thread
From: Mike Turquette @ 2012-12-15 20:54 UTC (permalink / raw)
  To: Haojian Zhuang; +Cc: linux-arm-kernel, linux-kernel, Bergmann Arnd

On Sat, Dec 15, 2012 at 8:41 AM, Haojian Zhuang
<haojian.zhuang@gmail.com> wrote:
> On Tue, Dec 4, 2012 at 9:32 AM, Haojian Zhuang <haojian.zhuang@gmail.com> wrote:
>> On Mon, Dec 3, 2012 at 4:14 PM, Haojian Zhuang <haojian.zhuang@gmail.com> wrote:
>>> 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.
>>>
>>> Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>
>>> ---
>>>  drivers/clk/clk-fixed-factor.c |    5 ++++-
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
>>> index a489985..1ef271e 100644
>>> --- a/drivers/clk/clk-fixed-factor.c
>>> +++ b/drivers/clk/clk-fixed-factor.c
>>> @@ -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,
>>> --
>>> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] clk: factor: calculate rate by do_div
  2012-12-15 20:54     ` Mike Turquette
@ 2012-12-16  6:27       ` Haojian Zhuang
  0 siblings, 0 replies; 6+ messages in thread
From: Haojian Zhuang @ 2012-12-16  6:27 UTC (permalink / raw)
  To: Mike Turquette; +Cc: linux-arm-kernel, linux-kernel, Bergmann Arnd

On Sun, Dec 16, 2012 at 4:54 AM, Mike Turquette <mturquette@linaro.org> wrote:
> On Sat, Dec 15, 2012 at 8:41 AM, Haojian Zhuang
> <haojian.zhuang@gmail.com> wrote:
>> On Tue, Dec 4, 2012 at 9:32 AM, Haojian Zhuang <haojian.zhuang@gmail.com> wrote:
>>> On Mon, Dec 3, 2012 at 4:14 PM, Haojian Zhuang <haojian.zhuang@gmail.com> wrote:
>>>> 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.
>>>>
>>>> Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>
>>>> ---
>>>>  drivers/clk/clk-fixed-factor.c |    5 ++++-
>>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
>>>> index a489985..1ef271e 100644
>>>> --- a/drivers/clk/clk-fixed-factor.c
>>>> +++ b/drivers/clk/clk-fixed-factor.c
>>>> @@ -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,
>>>> --
>>>> 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

It's nice. Thank you.

Best Regards
Haojian

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-12-16  6:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-03  8:14 [PATCH] clk: factor: calculate rate by do_div Haojian Zhuang
2012-12-04  1:32 ` Haojian Zhuang
2012-12-15 16:41   ` Haojian Zhuang
2012-12-15 20:54     ` Mike Turquette
2012-12-16  6:27       ` Haojian Zhuang
2012-12-15 16:42   ` Haojian Zhuang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).