* [PATCH] clk:mxs: Fix bug on frequency divider
@ 2015-07-31 16:24 Victorien Vedrine
2015-07-31 17:34 ` Stephen Boyd
0 siblings, 1 reply; 12+ messages in thread
From: Victorien Vedrine @ 2015-07-31 16:24 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-clk, sboyd, mturquette, Victorien Vedrine
On drivers/clk/mxs/clk-frac.c, the function clk_frac_round_rate returned a bad
result. The division before multiplication computes a wrong value ; the
calculation is inverted to fix the problem. The second issue is that the exact
rate have decimals and they are truncate. The consequence is that the function
clk_frac_set_rate (which use the result of clk_frac_round_rate) computes a
wrong value for the register (the rate generated can be closer to the desired
rate). The correction is : if there is decimal to the result, it is rounded to
the next larger integer.
On drivers/clk/mxs/clk-frac.c, the function clk_frac_recalc_rate returned
a bad result. The multiplication is made before the division to compute a
correct value.
The issue is reproducible by this way : Set the SAIF frequency to 22579200Hz
(it's the appropriate frequency for 44100hz sample rate for SGTL5000 codec).
With the divider register (HW_CLKCTRL_SAIF0), the closest lower value is
22573242.1875Hz (0xC0A on register). The original clk-frac functions give 0xC09
on the divider register.
Signed-off-by: Victorien Vedrine <victorien.vedrine@ophrys.net>
---
drivers/clk/mxs/clk-frac.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/mxs/clk-frac.c b/drivers/clk/mxs/clk-frac.c
index e6aa6b5..d51cf03 100644
--- a/drivers/clk/mxs/clk-frac.c
+++ b/drivers/clk/mxs/clk-frac.c
@@ -42,11 +42,13 @@ static unsigned long clk_frac_recalc_rate(struct clk_hw *hw,
{
struct clk_frac *frac = to_clk_frac(hw);
u32 div;
+ u64 tmp_rate;
div = readl_relaxed(frac->reg) >> frac->shift;
div &= (1 << frac->width) - 1;
- return (parent_rate >> frac->width) * div;
+ tmp_rate = (u64)parent_rate * (u64)div;
+ return (unsigned long)(tmp_rate >> frac->width);
}
static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
@@ -55,7 +57,7 @@ static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
struct clk_frac *frac = to_clk_frac(hw);
unsigned long parent_rate = *prate;
u32 div;
- u64 tmp;
+ u64 tmp, tmp_rate, result;
if (rate > parent_rate)
return -EINVAL;
@@ -68,7 +70,12 @@ static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
if (!div)
return -EINVAL;
- return (parent_rate >> frac->width) * div;
+ tmp_rate = (u64)parent_rate * (u64)div;
+ result = (u64)(tmp_rate >> frac->width);
+ if ((result << frac->width) < tmp_rate)
+ return result + 1;
+ else
+ return result;
}
static int clk_frac_set_rate(struct clk_hw *hw, unsigned long rate,
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] clk:mxs: Fix bug on frequency divider
2015-07-31 16:24 [PATCH] clk:mxs: Fix bug on frequency divider Victorien Vedrine
@ 2015-07-31 17:34 ` Stephen Boyd
2015-08-19 10:29 ` victorien.vedrine
2015-08-31 8:45 ` [PATCH v2] " Victorien Vedrine
0 siblings, 2 replies; 12+ messages in thread
From: Stephen Boyd @ 2015-07-31 17:34 UTC (permalink / raw)
To: Victorien Vedrine; +Cc: linux-kernel, linux-clk, mturquette, Shawn Guo
+Shawn
On 07/31/2015 09:24 AM, Victorien Vedrine wrote:
> On drivers/clk/mxs/clk-frac.c, the function clk_frac_round_rate returned a bad
> result. The division before multiplication computes a wrong value ; the
> calculation is inverted to fix the problem. The second issue is that the exact
> rate have decimals and they are truncate. The consequence is that the function
> clk_frac_set_rate (which use the result of clk_frac_round_rate) computes a
> wrong value for the register (the rate generated can be closer to the desired
> rate). The correction is : if there is decimal to the result, it is rounded to
> the next larger integer.
>
> On drivers/clk/mxs/clk-frac.c, the function clk_frac_recalc_rate returned
> a bad result. The multiplication is made before the division to compute a
> correct value.
>
> The issue is reproducible by this way : Set the SAIF frequency to 22579200Hz
> (it's the appropriate frequency for 44100hz sample rate for SGTL5000 codec).
> With the divider register (HW_CLKCTRL_SAIF0), the closest lower value is
> 22573242.1875Hz (0xC0A on register). The original clk-frac functions give 0xC09
> on the divider register.
>
> Signed-off-by: Victorien Vedrine <victorien.vedrine@ophrys.net>
> ---
> drivers/clk/mxs/clk-frac.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/mxs/clk-frac.c b/drivers/clk/mxs/clk-frac.c
> index e6aa6b5..d51cf03 100644
> --- a/drivers/clk/mxs/clk-frac.c
> +++ b/drivers/clk/mxs/clk-frac.c
> @@ -42,11 +42,13 @@ static unsigned long clk_frac_recalc_rate(struct clk_hw *hw,
> {
> struct clk_frac *frac = to_clk_frac(hw);
> u32 div;
> + u64 tmp_rate;
>
> div = readl_relaxed(frac->reg) >> frac->shift;
> div &= (1 << frac->width) - 1;
>
> - return (parent_rate >> frac->width) * div;
> + tmp_rate = (u64)parent_rate * (u64)div;
We shouldn't need to cast both to 64 bits...
> + return (unsigned long)(tmp_rate >> frac->width);
Isn't the cast implicit by means of the return type of this function?
> }
>
> static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
> @@ -55,7 +57,7 @@ static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
> struct clk_frac *frac = to_clk_frac(hw);
> unsigned long parent_rate = *prate;
> u32 div;
> - u64 tmp;
> + u64 tmp, tmp_rate, result;
>
> if (rate > parent_rate)
> return -EINVAL;
> @@ -68,7 +70,12 @@ static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
> if (!div)
> return -EINVAL;
>
> - return (parent_rate >> frac->width) * div;
> + tmp_rate = (u64)parent_rate * (u64)div;
> + result = (u64)(tmp_rate >> frac->width);
Cast looks useless here too.
> + if ((result << frac->width) < tmp_rate)
> + return result + 1;
> + else
> + return result;
The else could be dropped and just be return result. Also, have you see
mult_frac()? I suppose we're doing shifts with width because it's a
power-of-2 denominator?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clk:mxs: Fix bug on frequency divider
2015-07-31 17:34 ` Stephen Boyd
@ 2015-08-19 10:29 ` victorien.vedrine
2015-08-31 8:45 ` [PATCH v2] " Victorien Vedrine
1 sibling, 0 replies; 12+ messages in thread
From: victorien.vedrine @ 2015-08-19 10:29 UTC (permalink / raw)
To: Stephen Boyd; +Cc: linux-kernel, linux-clk, mturquette, Shawn Guo
I'll modify the cast and propose a new version of patch.
This patch is specific for imx28 processor and for this one there is no
mult_frac()
function. I checked the other functions of imx28 clock part and I didn't
see a
similar problem.
Le 2015-07-31 19:34, Stephen Boyd a écrit :
> +Shawn
>
> On 07/31/2015 09:24 AM, Victorien Vedrine wrote:
>> On drivers/clk/mxs/clk-frac.c, the function clk_frac_round_rate
>> returned a bad
>> result. The division before multiplication computes a wrong value ;
>> the
>> calculation is inverted to fix the problem. The second issue is that
>> the exact
>> rate have decimals and they are truncate. The consequence is that the
>> function
>> clk_frac_set_rate (which use the result of clk_frac_round_rate)
>> computes a
>> wrong value for the register (the rate generated can be closer to the
>> desired
>> rate). The correction is : if there is decimal to the result, it is
>> rounded to
>> the next larger integer.
>>
>> On drivers/clk/mxs/clk-frac.c, the function clk_frac_recalc_rate
>> returned
>> a bad result. The multiplication is made before the division to
>> compute a
>> correct value.
>>
>> The issue is reproducible by this way : Set the SAIF frequency to
>> 22579200Hz
>> (it's the appropriate frequency for 44100hz sample rate for SGTL5000
>> codec).
>> With the divider register (HW_CLKCTRL_SAIF0), the closest lower value
>> is
>> 22573242.1875Hz (0xC0A on register). The original clk-frac functions
>> give 0xC09
>> on the divider register.
>>
>> Signed-off-by: Victorien Vedrine <victorien.vedrine@ophrys.net>
>> ---
>> drivers/clk/mxs/clk-frac.c | 13 ++++++++++---
>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clk/mxs/clk-frac.c b/drivers/clk/mxs/clk-frac.c
>> index e6aa6b5..d51cf03 100644
>> --- a/drivers/clk/mxs/clk-frac.c
>> +++ b/drivers/clk/mxs/clk-frac.c
>> @@ -42,11 +42,13 @@ static unsigned long clk_frac_recalc_rate(struct
>> clk_hw *hw,
>> {
>> struct clk_frac *frac = to_clk_frac(hw);
>> u32 div;
>> + u64 tmp_rate;
>> div = readl_relaxed(frac->reg) >> frac->shift;
>> div &= (1 << frac->width) - 1;
>> - return (parent_rate >> frac->width) * div;
>> + tmp_rate = (u64)parent_rate * (u64)div;
>
> We shouldn't need to cast both to 64 bits...
>
>> + return (unsigned long)(tmp_rate >> frac->width);
>
> Isn't the cast implicit by means of the return type of this function?
>
>> }
>> static long clk_frac_round_rate(struct clk_hw *hw, unsigned long
>> rate,
>> @@ -55,7 +57,7 @@ static long clk_frac_round_rate(struct clk_hw *hw,
>> unsigned long rate,
>> struct clk_frac *frac = to_clk_frac(hw);
>> unsigned long parent_rate = *prate;
>> u32 div;
>> - u64 tmp;
>> + u64 tmp, tmp_rate, result;
>> if (rate > parent_rate)
>> return -EINVAL;
>> @@ -68,7 +70,12 @@ static long clk_frac_round_rate(struct clk_hw *hw,
>> unsigned long rate,
>> if (!div)
>> return -EINVAL;
>> - return (parent_rate >> frac->width) * div;
>> + tmp_rate = (u64)parent_rate * (u64)div;
>> + result = (u64)(tmp_rate >> frac->width);
>
> Cast looks useless here too.
>
>> + if ((result << frac->width) < tmp_rate)
>> + return result + 1;
>> + else
>> + return result;
>
> The else could be dropped and just be return result. Also, have you
> see mult_frac()? I suppose we're doing shifts with width because it's
> a power-of-2 denominator?
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] clk:mxs: Fix bug on frequency divider
2015-07-31 17:34 ` Stephen Boyd
2015-08-19 10:29 ` victorien.vedrine
@ 2015-08-31 8:45 ` Victorien Vedrine
2015-09-16 14:16 ` Stefan Wahren
` (2 more replies)
1 sibling, 3 replies; 12+ messages in thread
From: Victorien Vedrine @ 2015-08-31 8:45 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-clk, sboyd, mturquette, shawn.guo, Victorien Vedrine
On drivers/clk/mxs/clk-frac.c, the function clk_frac_round_rate returned a bad
result. The division before multiplication computes a wrong value ; the
calculation is inverted to fix the problem. The second issue is that the exact
rate have decimals and they are truncate. The consequence is that the function
clk_frac_set_rate (which use the result of clk_frac_round_rate) computes a
wrong value for the register (the rate generated can be closer to the desired
rate). The correction is : if there is decimal to the result, it is rounded to
the next larger integer.
On drivers/clk/mxs/clk-frac.c, the function clk_frac_recalc_rate returned
a bad result. The multiplication is made before the division to compute a
correct value.
Signed-off-by: Victorien Vedrine <victorien.vedrine@ophrys.net>
---
drivers/clk/mxs/clk-frac.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/mxs/clk-frac.c b/drivers/clk/mxs/clk-frac.c
index e6aa6b5..b3fa7fa 100644
--- a/drivers/clk/mxs/clk-frac.c
+++ b/drivers/clk/mxs/clk-frac.c
@@ -42,11 +42,13 @@ static unsigned long clk_frac_recalc_rate(struct clk_hw *hw,
{
struct clk_frac *frac = to_clk_frac(hw);
u32 div;
+ u64 tmp_rate;
div = readl_relaxed(frac->reg) >> frac->shift;
div &= (1 << frac->width) - 1;
- return (parent_rate >> frac->width) * div;
+ tmp_rate = (u64)parent_rate * div;
+ return tmp_rate >> frac->width;
}
static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
@@ -55,7 +57,7 @@ static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
struct clk_frac *frac = to_clk_frac(hw);
unsigned long parent_rate = *prate;
u32 div;
- u64 tmp;
+ u64 tmp, tmp_rate, result;
if (rate > parent_rate)
return -EINVAL;
@@ -68,7 +70,11 @@ static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
if (!div)
return -EINVAL;
- return (parent_rate >> frac->width) * div;
+ tmp_rate = (u64)parent_rate * div;
+ result = tmp_rate >> frac->width;
+ if ((result << frac->width) < tmp_rate)
+ result += 1;
+ return result;
}
static int clk_frac_set_rate(struct clk_hw *hw, unsigned long rate,
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] clk:mxs: Fix bug on frequency divider
2015-08-31 8:45 ` [PATCH v2] " Victorien Vedrine
@ 2015-09-16 14:16 ` Stefan Wahren
2015-09-18 7:21 ` Stefan Wahren
2015-10-01 22:25 ` Stephen Boyd
2 siblings, 0 replies; 12+ messages in thread
From: Stefan Wahren @ 2015-09-16 14:16 UTC (permalink / raw)
To: Victorien Vedrine, linux-kernel
Cc: linux-clk, sboyd, mturquette, shawn.guo, Fabio.Estevam, Marek Vasut
+Fabio
+Marek
> On drivers/clk/mxs/clk-frac.c, the function clk_frac_round_rate returned a bad
> result. The division before multiplication computes a wrong value ; the
> calculation is inverted to fix the problem. The second issue is that the exact
> rate have decimals and they are truncate. The consequence is that the function
> clk_frac_set_rate (which use the result of clk_frac_round_rate) computes a
> wrong value for the register (the rate generated can be closer to the desired
> rate). The correction is : if there is decimal to the result, it is rounded to
> the next larger integer.
> On drivers/clk/mxs/clk-frac.c, the function clk_frac_recalc_rate returned
> a bad result. The multiplication is made before the division to compute a
> correct value.
>
> Signed-off-by: Victorien Vedrine <victorien.vedrine@ophrys.net>
> ---
> drivers/clk/mxs/clk-frac.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/mxs/clk-frac.c b/drivers/clk/mxs/clk-frac.c
> index e6aa6b5..b3fa7fa 100644
> --- a/drivers/clk/mxs/clk-frac.c
> +++ b/drivers/clk/mxs/clk-frac.c
> @@ -42,11 +42,13 @@ static unsigned long clk_frac_recalc_rate(struct clk_hw *hw,
> {
> struct clk_frac *frac = to_clk_frac(hw);
> u32 div;
> + u64 tmp_rate;
>
> div = readl_relaxed(frac->reg) >> frac->shift;
> div &= (1 << frac->width) - 1;
>
> - return (parent_rate >> frac->width) * div;
> + tmp_rate = (u64)parent_rate * div;
> + return tmp_rate >> frac->width;
> }
>
> static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
> @@ -55,7 +57,7 @@ static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
> struct clk_frac *frac = to_clk_frac(hw);
> unsigned long parent_rate = *prate;
> u32 div;
> - u64 tmp;
> + u64 tmp, tmp_rate, result;
>
> if (rate > parent_rate)
> return -EINVAL;
> @@ -68,7 +70,11 @@ static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
> if (!div)
> return -EINVAL;
>
> - return (parent_rate >> frac->width) * div;
> + tmp_rate = (u64)parent_rate * div;
> + result = tmp_rate >> frac->width;
> + if ((result << frac->width) < tmp_rate)
> + result += 1;
> + return result;
> }
>
> static int clk_frac_set_rate(struct clk_hw *hw, unsigned long rate,
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] clk:mxs: Fix bug on frequency divider
2015-08-31 8:45 ` [PATCH v2] " Victorien Vedrine
2015-09-16 14:16 ` Stefan Wahren
@ 2015-09-18 7:21 ` Stefan Wahren
2015-10-01 22:25 ` Stephen Boyd
2 siblings, 0 replies; 12+ messages in thread
From: Stefan Wahren @ 2015-09-18 7:21 UTC (permalink / raw)
To: Victorien Vedrine, linux-kernel
Cc: linux-clk, sboyd, mturquette, shawn.guo, Fabio.Estevam,
Marek Vasut, Shawn Guo, linux-arm-kernel
+Shawn's new address
+linux-arm-kernel
> On drivers/clk/mxs/clk-frac.c, the function clk_frac_round_rate returned a bad
> result. The division before multiplication computes a wrong value ; the
> calculation is inverted to fix the problem. The second issue is that the exact
> rate have decimals and they are truncate. The consequence is that the function
> clk_frac_set_rate (which use the result of clk_frac_round_rate) computes a
> wrong value for the register (the rate generated can be closer to the desired
> rate). The correction is : if there is decimal to the result, it is rounded to
> the next larger integer.
> On drivers/clk/mxs/clk-frac.c, the function clk_frac_recalc_rate returned
> a bad result. The multiplication is made before the division to compute a
> correct value.
>
> Signed-off-by: Victorien Vedrine <victorien.vedrine@ophrys.net>
> ---
> drivers/clk/mxs/clk-frac.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/mxs/clk-frac.c b/drivers/clk/mxs/clk-frac.c
> index e6aa6b5..b3fa7fa 100644
> --- a/drivers/clk/mxs/clk-frac.c
> +++ b/drivers/clk/mxs/clk-frac.c
> @@ -42,11 +42,13 @@ static unsigned long clk_frac_recalc_rate(struct clk_hw *hw,
> {
> struct clk_frac *frac = to_clk_frac(hw);
> u32 div;
> + u64 tmp_rate;
>
> div = readl_relaxed(frac->reg) >> frac->shift;
> div &= (1 << frac->width) - 1;
>
> - return (parent_rate >> frac->width) * div;
> + tmp_rate = (u64)parent_rate * div;
> + return tmp_rate >> frac->width;
> }
>
> static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
> @@ -55,7 +57,7 @@ static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
> struct clk_frac *frac = to_clk_frac(hw);
> unsigned long parent_rate = *prate;
> u32 div;
> - u64 tmp;
> + u64 tmp, tmp_rate, result;
>
> if (rate > parent_rate)
> return -EINVAL;
> @@ -68,7 +70,11 @@ static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
> if (!div)
> return -EINVAL;
>
> - return (parent_rate >> frac->width) * div;
> + tmp_rate = (u64)parent_rate * div;
> + result = tmp_rate >> frac->width;
> + if ((result << frac->width) < tmp_rate)
> + result += 1;
> + return result;
> }
>
> static int clk_frac_set_rate(struct clk_hw *hw, unsigned long rate,
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] clk:mxs: Fix bug on frequency divider
@ 2015-09-18 7:21 ` Stefan Wahren
0 siblings, 0 replies; 12+ messages in thread
From: Stefan Wahren @ 2015-09-18 7:21 UTC (permalink / raw)
To: Victorien Vedrine, linux-kernel
Cc: linux-clk, sboyd, mturquette, shawn.guo, Fabio.Estevam,
Marek Vasut, Shawn Guo, linux-arm-kernel
+Shawn's new address
+linux-arm-kernel
> On drivers/clk/mxs/clk-frac.c, the function clk_frac_round_rate returned a bad
> result. The division before multiplication computes a wrong value ; the
> calculation is inverted to fix the problem. The second issue is that the exact
> rate have decimals and they are truncate. The consequence is that the function
> clk_frac_set_rate (which use the result of clk_frac_round_rate) computes a
> wrong value for the register (the rate generated can be closer to the desired
> rate). The correction is : if there is decimal to the result, it is rounded to
> the next larger integer.
> On drivers/clk/mxs/clk-frac.c, the function clk_frac_recalc_rate returned
> a bad result. The multiplication is made before the division to compute a
> correct value.
>
> Signed-off-by: Victorien Vedrine <victorien.vedrine@ophrys.net>
> ---
> drivers/clk/mxs/clk-frac.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/mxs/clk-frac.c b/drivers/clk/mxs/clk-frac.c
> index e6aa6b5..b3fa7fa 100644
> --- a/drivers/clk/mxs/clk-frac.c
> +++ b/drivers/clk/mxs/clk-frac.c
> @@ -42,11 +42,13 @@ static unsigned long clk_frac_recalc_rate(struct clk_hw *hw,
> {
> struct clk_frac *frac = to_clk_frac(hw);
> u32 div;
> + u64 tmp_rate;
>
> div = readl_relaxed(frac->reg) >> frac->shift;
> div &= (1 << frac->width) - 1;
>
> - return (parent_rate >> frac->width) * div;
> + tmp_rate = (u64)parent_rate * div;
> + return tmp_rate >> frac->width;
> }
>
> static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
> @@ -55,7 +57,7 @@ static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
> struct clk_frac *frac = to_clk_frac(hw);
> unsigned long parent_rate = *prate;
> u32 div;
> - u64 tmp;
> + u64 tmp, tmp_rate, result;
>
> if (rate > parent_rate)
> return -EINVAL;
> @@ -68,7 +70,11 @@ static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
> if (!div)
> return -EINVAL;
>
> - return (parent_rate >> frac->width) * div;
> + tmp_rate = (u64)parent_rate * div;
> + result = tmp_rate >> frac->width;
> + if ((result << frac->width) < tmp_rate)
> + result += 1;
> + return result;
> }
>
> static int clk_frac_set_rate(struct clk_hw *hw, unsigned long rate,
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] clk:mxs: Fix bug on frequency divider
@ 2015-09-18 7:21 ` Stefan Wahren
0 siblings, 0 replies; 12+ messages in thread
From: Stefan Wahren @ 2015-09-18 7:21 UTC (permalink / raw)
To: linux-arm-kernel
+Shawn's new address
+linux-arm-kernel
> On drivers/clk/mxs/clk-frac.c, the function clk_frac_round_rate returned a bad
> result. The division before multiplication computes a wrong value ; the
> calculation is inverted to fix the problem. The second issue is that the exact
> rate have decimals and they are truncate. The consequence is that the function
> clk_frac_set_rate (which use the result of clk_frac_round_rate) computes a
> wrong value for the register (the rate generated can be closer to the desired
> rate). The correction is : if there is decimal to the result, it is rounded to
> the next larger integer.
> On drivers/clk/mxs/clk-frac.c, the function clk_frac_recalc_rate returned
> a bad result. The multiplication is made before the division to compute a
> correct value.
>
> Signed-off-by: Victorien Vedrine <victorien.vedrine@ophrys.net>
> ---
> drivers/clk/mxs/clk-frac.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/mxs/clk-frac.c b/drivers/clk/mxs/clk-frac.c
> index e6aa6b5..b3fa7fa 100644
> --- a/drivers/clk/mxs/clk-frac.c
> +++ b/drivers/clk/mxs/clk-frac.c
> @@ -42,11 +42,13 @@ static unsigned long clk_frac_recalc_rate(struct clk_hw *hw,
> {
> struct clk_frac *frac = to_clk_frac(hw);
> u32 div;
> + u64 tmp_rate;
>
> div = readl_relaxed(frac->reg) >> frac->shift;
> div &= (1 << frac->width) - 1;
>
> - return (parent_rate >> frac->width) * div;
> + tmp_rate = (u64)parent_rate * div;
> + return tmp_rate >> frac->width;
> }
>
> static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
> @@ -55,7 +57,7 @@ static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
> struct clk_frac *frac = to_clk_frac(hw);
> unsigned long parent_rate = *prate;
> u32 div;
> - u64 tmp;
> + u64 tmp, tmp_rate, result;
>
> if (rate > parent_rate)
> return -EINVAL;
> @@ -68,7 +70,11 @@ static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
> if (!div)
> return -EINVAL;
>
> - return (parent_rate >> frac->width) * div;
> + tmp_rate = (u64)parent_rate * div;
> + result = tmp_rate >> frac->width;
> + if ((result << frac->width) < tmp_rate)
> + result += 1;
> + return result;
> }
>
> static int clk_frac_set_rate(struct clk_hw *hw, unsigned long rate,
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] clk:mxs: Fix bug on frequency divider
2015-09-18 7:21 ` Stefan Wahren
(?)
@ 2015-09-24 12:17 ` Shawn Guo
-1 siblings, 0 replies; 12+ messages in thread
From: Shawn Guo @ 2015-09-24 12:17 UTC (permalink / raw)
To: Stefan Wahren
Cc: Victorien Vedrine, linux-kernel, linux-clk, sboyd, mturquette,
shawn.guo, Fabio.Estevam, Marek Vasut, linux-arm-kernel
On Fri, Sep 18, 2015 at 09:21:17AM +0200, Stefan Wahren wrote:
> +Shawn's new address
> +linux-arm-kernel
>
> > On drivers/clk/mxs/clk-frac.c, the function clk_frac_round_rate returned a bad
> > result. The division before multiplication computes a wrong value ; the
> > calculation is inverted to fix the problem. The second issue is that the exact
> > rate have decimals and they are truncate. The consequence is that the function
> > clk_frac_set_rate (which use the result of clk_frac_round_rate) computes a
> > wrong value for the register (the rate generated can be closer to the desired
> > rate). The correction is : if there is decimal to the result, it is rounded to
> > the next larger integer.
> > On drivers/clk/mxs/clk-frac.c, the function clk_frac_recalc_rate returned
> > a bad result. The multiplication is made before the division to compute a
> > correct value.
> >
> > Signed-off-by: Victorien Vedrine <victorien.vedrine@ophrys.net>
Acked-by: Shawn Guo <shawnguo@kernel.org>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] clk:mxs: Fix bug on frequency divider
@ 2015-09-24 12:17 ` Shawn Guo
0 siblings, 0 replies; 12+ messages in thread
From: Shawn Guo @ 2015-09-24 12:17 UTC (permalink / raw)
To: Stefan Wahren
Cc: Victorien Vedrine, linux-kernel, linux-clk, sboyd, mturquette,
shawn.guo, Fabio.Estevam, Marek Vasut, linux-arm-kernel
On Fri, Sep 18, 2015 at 09:21:17AM +0200, Stefan Wahren wrote:
> +Shawn's new address
> +linux-arm-kernel
>
> > On drivers/clk/mxs/clk-frac.c, the function clk_frac_round_rate returned a bad
> > result. The division before multiplication computes a wrong value ; the
> > calculation is inverted to fix the problem. The second issue is that the exact
> > rate have decimals and they are truncate. The consequence is that the function
> > clk_frac_set_rate (which use the result of clk_frac_round_rate) computes a
> > wrong value for the register (the rate generated can be closer to the desired
> > rate). The correction is : if there is decimal to the result, it is rounded to
> > the next larger integer.
> > On drivers/clk/mxs/clk-frac.c, the function clk_frac_recalc_rate returned
> > a bad result. The multiplication is made before the division to compute a
> > correct value.
> >
> > Signed-off-by: Victorien Vedrine <victorien.vedrine@ophrys.net>
Acked-by: Shawn Guo <shawnguo@kernel.org>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] clk:mxs: Fix bug on frequency divider
@ 2015-09-24 12:17 ` Shawn Guo
0 siblings, 0 replies; 12+ messages in thread
From: Shawn Guo @ 2015-09-24 12:17 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Sep 18, 2015 at 09:21:17AM +0200, Stefan Wahren wrote:
> +Shawn's new address
> +linux-arm-kernel
>
> > On drivers/clk/mxs/clk-frac.c, the function clk_frac_round_rate returned a bad
> > result. The division before multiplication computes a wrong value ; the
> > calculation is inverted to fix the problem. The second issue is that the exact
> > rate have decimals and they are truncate. The consequence is that the function
> > clk_frac_set_rate (which use the result of clk_frac_round_rate) computes a
> > wrong value for the register (the rate generated can be closer to the desired
> > rate). The correction is : if there is decimal to the result, it is rounded to
> > the next larger integer.
> > On drivers/clk/mxs/clk-frac.c, the function clk_frac_recalc_rate returned
> > a bad result. The multiplication is made before the division to compute a
> > correct value.
> >
> > Signed-off-by: Victorien Vedrine <victorien.vedrine@ophrys.net>
Acked-by: Shawn Guo <shawnguo@kernel.org>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] clk:mxs: Fix bug on frequency divider
2015-08-31 8:45 ` [PATCH v2] " Victorien Vedrine
2015-09-16 14:16 ` Stefan Wahren
2015-09-18 7:21 ` Stefan Wahren
@ 2015-10-01 22:25 ` Stephen Boyd
2 siblings, 0 replies; 12+ messages in thread
From: Stephen Boyd @ 2015-10-01 22:25 UTC (permalink / raw)
To: Victorien Vedrine; +Cc: linux-kernel, linux-clk, mturquette, shawn.guo
On 08/31, Victorien Vedrine wrote:
> On drivers/clk/mxs/clk-frac.c, the function clk_frac_round_rate returned a bad
> result. The division before multiplication computes a wrong value ; the
> calculation is inverted to fix the problem. The second issue is that the exact
> rate have decimals and they are truncate. The consequence is that the function
> clk_frac_set_rate (which use the result of clk_frac_round_rate) computes a
> wrong value for the register (the rate generated can be closer to the desired
> rate). The correction is : if there is decimal to the result, it is rounded to
> the next larger integer.
> On drivers/clk/mxs/clk-frac.c, the function clk_frac_recalc_rate returned
> a bad result. The multiplication is made before the division to compute a
> correct value.
>
> Signed-off-by: Victorien Vedrine <victorien.vedrine@ophrys.net>
> ---
Applied to clk-next
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-10-01 22:25 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-31 16:24 [PATCH] clk:mxs: Fix bug on frequency divider Victorien Vedrine
2015-07-31 17:34 ` Stephen Boyd
2015-08-19 10:29 ` victorien.vedrine
2015-08-31 8:45 ` [PATCH v2] " Victorien Vedrine
2015-09-16 14:16 ` Stefan Wahren
2015-09-18 7:21 ` Stefan Wahren
2015-09-18 7:21 ` Stefan Wahren
2015-09-18 7:21 ` Stefan Wahren
2015-09-24 12:17 ` Shawn Guo
2015-09-24 12:17 ` Shawn Guo
2015-09-24 12:17 ` Shawn Guo
2015-10-01 22:25 ` Stephen Boyd
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.