* [PATCH 1/1] ARM iMX28: incorrect frac/div calculation
@ 2012-03-22 23:05 Matt Burtch
2012-03-23 2:52 ` Shawn Guo
2012-03-23 7:13 ` Lothar Waßmann
0 siblings, 2 replies; 5+ messages in thread
From: Matt Burtch @ 2012-03-22 23:05 UTC (permalink / raw)
To: linux-arm-kernel
The frac and div values in _CLK_SET_RATE are calculated incorrectly
for some clock rates ie. CPU clk of 454.73MHz, 392.72MHz, 261.81Mhz,
etc.
Now the frac and div values which result in the closest actual clock
rate to that requested is selected. Also there are no limitations
on what frac values can be used, as long as they fall in the valid
(18 < frac < 35) range.
Tested on custom iMX28 board.
Signed-off-by: Matt Burtch <matt@grid-net.com>
---
arch/arm/mach-mxs/clock-mx28.c | 8 ++------
1 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c
index 5d68e41..a058b99 100644
--- a/arch/arm/mach-mxs/clock-mx28.c
+++ b/arch/arm/mach-mxs/clock-mx28.c
@@ -354,19 +354,15 @@ static int name##_set_rate(struct clk *clk, unsigned long rate) \
} \
for (d = 1; d <= div_max; d++) { \
f = parent_rate * 18 / d / rate; \
- if ((parent_rate * 18 / d) % rate) \
- f++; \
if (f < 18 || f > 35) \
continue; \
\
calc_rate = parent_rate * 18 / f / d; \
- if (calc_rate > rate) \
- continue; \
\
- if (rate - calc_rate < diff) { \
+ if (abs(rate - calc_rate) < diff) { \
frac = f; \
div = d; \
- diff = rate - calc_rate; \
+ diff = abs(rate - calc_rate); \
} \
\
if (diff == 0) \
--
1.7.5.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 1/1] ARM iMX28: incorrect frac/div calculation
2012-03-22 23:05 [PATCH 1/1] ARM iMX28: incorrect frac/div calculation Matt Burtch
@ 2012-03-23 2:52 ` Shawn Guo
2012-03-23 18:38 ` Matt Burtch
2012-03-23 7:13 ` Lothar Waßmann
1 sibling, 1 reply; 5+ messages in thread
From: Shawn Guo @ 2012-03-23 2:52 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Mar 22, 2012 at 04:05:01PM -0700, Matt Burtch wrote:
> The frac and div values in _CLK_SET_RATE are calculated incorrectly
> for some clock rates ie. CPU clk of 454.73MHz, 392.72MHz, 261.81Mhz,
> etc.
>
> Now the frac and div values which result in the closest actual clock
> rate to that requested is selected.
My understanding on clk_set_rate is we need to set the clk to a rate
closest to the requested one but never exceeding it. Does your change
ensure that?
Regards,
Shawn
> Also there are no limitations
> on what frac values can be used, as long as they fall in the valid
> (18 < frac < 35) range.
>
> Tested on custom iMX28 board.
>
> Signed-off-by: Matt Burtch <matt@grid-net.com>
> ---
> arch/arm/mach-mxs/clock-mx28.c | 8 ++------
> 1 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c
> index 5d68e41..a058b99 100644
> --- a/arch/arm/mach-mxs/clock-mx28.c
> +++ b/arch/arm/mach-mxs/clock-mx28.c
> @@ -354,19 +354,15 @@ static int name##_set_rate(struct clk *clk, unsigned long rate) \
> } \
> for (d = 1; d <= div_max; d++) { \
> f = parent_rate * 18 / d / rate; \
> - if ((parent_rate * 18 / d) % rate) \
> - f++; \
> if (f < 18 || f > 35) \
> continue; \
> \
> calc_rate = parent_rate * 18 / f / d; \
> - if (calc_rate > rate) \
> - continue; \
> \
> - if (rate - calc_rate < diff) { \
> + if (abs(rate - calc_rate) < diff) { \
> frac = f; \
> div = d; \
> - diff = rate - calc_rate; \
> + diff = abs(rate - calc_rate); \
> } \
> \
> if (diff == 0) \
> --
> 1.7.5.4
>
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 1/1] ARM iMX28: incorrect frac/div calculation
2012-03-23 2:52 ` Shawn Guo
@ 2012-03-23 18:38 ` Matt Burtch
0 siblings, 0 replies; 5+ messages in thread
From: Matt Burtch @ 2012-03-23 18:38 UTC (permalink / raw)
To: linux-arm-kernel
On 03/22/2012 07:52 PM, Shawn Guo wrote:
> On Thu, Mar 22, 2012 at 04:05:01PM -0700, Matt Burtch wrote:
>> The frac and div values in _CLK_SET_RATE are calculated incorrectly
>> for some clock rates ie. CPU clk of 454.73MHz, 392.72MHz, 261.81Mhz,
>> etc.
>>
>> Now the frac and div values which result in the closest actual clock
>> rate to that requested is selected.
> My understanding on clk_set_rate is we need to set the clk to a rate
> closest to the requested one but never exceeding it. Does your change
> ensure that?
>
> Regards,
> Shawn
It currently doesn't check that the resultant clock is lower than the
requested. I can certainly make that change and resubmit. I wasn't
aware of this requirement.
So with the patch as it stands, if you request a CPU clock of 391Mhz,
it'll give you 392.71MHz. Would it be correct for the clock to be set at
375.65MHz (the next closest valid clock rate without going over)?
Thanks,
Matt
>> Also there are no limitations
>> on what frac values can be used, as long as they fall in the valid
>> (18< frac< 35) range.
>>
>> Tested on custom iMX28 board.
>>
>> Signed-off-by: Matt Burtch<matt@grid-net.com>
>> ---
>> arch/arm/mach-mxs/clock-mx28.c | 8 ++------
>> 1 files changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c
>> index 5d68e41..a058b99 100644
>> --- a/arch/arm/mach-mxs/clock-mx28.c
>> +++ b/arch/arm/mach-mxs/clock-mx28.c
>> @@ -354,19 +354,15 @@ static int name##_set_rate(struct clk *clk, unsigned long rate) \
>> } \
>> for (d = 1; d<= div_max; d++) { \
>> f = parent_rate * 18 / d / rate; \
>> - if ((parent_rate * 18 / d) % rate) \
>> - f++; \
>> if (f< 18 || f> 35) \
>> continue; \
>> \
>> calc_rate = parent_rate * 18 / f / d; \
>> - if (calc_rate> rate) \
>> - continue; \
>> \
>> - if (rate - calc_rate< diff) { \
>> + if (abs(rate - calc_rate)< diff) { \
>> frac = f; \
>> div = d; \
>> - diff = rate - calc_rate; \
>> + diff = abs(rate - calc_rate); \
>> } \
>> \
>> if (diff == 0) \
>> --
>> 1.7.5.4
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1] ARM iMX28: incorrect frac/div calculation
2012-03-22 23:05 [PATCH 1/1] ARM iMX28: incorrect frac/div calculation Matt Burtch
2012-03-23 2:52 ` Shawn Guo
@ 2012-03-23 7:13 ` Lothar Waßmann
2012-03-23 18:38 ` Matt Burtch
1 sibling, 1 reply; 5+ messages in thread
From: Lothar Waßmann @ 2012-03-23 7:13 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Matt Burtch writes:
> The frac and div values in _CLK_SET_RATE are calculated incorrectly
> for some clock rates ie. CPU clk of 454.73MHz, 392.72MHz, 261.81Mhz,
> etc.
>
> Now the frac and div values which result in the closest actual clock
> rate to that requested is selected. Also there are no limitations
> on what frac values can be used, as long as they fall in the valid
> (18 < frac < 35) range.
>
> Tested on custom iMX28 board.
>
> Signed-off-by: Matt Burtch <matt@grid-net.com>
> ---
> arch/arm/mach-mxs/clock-mx28.c | 8 ++------
> 1 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c
> index 5d68e41..a058b99 100644
> --- a/arch/arm/mach-mxs/clock-mx28.c
> +++ b/arch/arm/mach-mxs/clock-mx28.c
> @@ -354,19 +354,15 @@ static int name##_set_rate(struct clk *clk, unsigned long rate) \
> } \
> for (d = 1; d <= div_max; d++) { \
> f = parent_rate * 18 / d / rate; \
> - if ((parent_rate * 18 / d) % rate) \
> - f++; \
> if (f < 18 || f > 35) \
> continue; \
> \
> calc_rate = parent_rate * 18 / f / d; \
> - if (calc_rate > rate) \
> - continue; \
> \
> - if (rate - calc_rate < diff) { \
> + if (abs(rate - calc_rate) < diff) { \
Shouldn't this actually be '<=' (also in the original code)?
Lothar Wa?mann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 1/1] ARM iMX28: incorrect frac/div calculation
2012-03-23 7:13 ` Lothar Waßmann
@ 2012-03-23 18:38 ` Matt Burtch
0 siblings, 0 replies; 5+ messages in thread
From: Matt Burtch @ 2012-03-23 18:38 UTC (permalink / raw)
To: linux-arm-kernel
On 03/23/2012 12:13 AM, Lothar Wa?mann wrote:
> Hi,
>
> Matt Burtch writes:
>> The frac and div values in _CLK_SET_RATE are calculated incorrectly
>> for some clock rates ie. CPU clk of 454.73MHz, 392.72MHz, 261.81Mhz,
>> etc.
>>
>> Now the frac and div values which result in the closest actual clock
>> rate to that requested is selected. Also there are no limitations
>> on what frac values can be used, as long as they fall in the valid
>> (18< frac< 35) range.
>>
>> Tested on custom iMX28 board.
>>
>> Signed-off-by: Matt Burtch<matt@grid-net.com>
>> ---
>> arch/arm/mach-mxs/clock-mx28.c | 8 ++------
>> 1 files changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c
>> index 5d68e41..a058b99 100644
>> --- a/arch/arm/mach-mxs/clock-mx28.c
>> +++ b/arch/arm/mach-mxs/clock-mx28.c
>> @@ -354,19 +354,15 @@ static int name##_set_rate(struct clk *clk, unsigned long rate) \
>> } \
>> for (d = 1; d<= div_max; d++) { \
>> f = parent_rate * 18 / d / rate; \
>> - if ((parent_rate * 18 / d) % rate) \
>> - f++; \
>> if (f< 18 || f> 35) \
>> continue; \
>> \
>> calc_rate = parent_rate * 18 / f / d; \
>> - if (calc_rate> rate) \
>> - continue; \
>> \
>> - if (rate - calc_rate< diff) { \
>> + if (abs(rate - calc_rate)< diff) { \
> Shouldn't this actually be '<=' (also in the original code)?
Hi Lothar,
It shouldn't make a difference. If it was '<=' it would result in a
different frac and higher div value but the difference between the
requested rate and actual would be the same.
Thanks,
Matt Burtch
> Lothar Wa?mann
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-03-23 18:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-22 23:05 [PATCH 1/1] ARM iMX28: incorrect frac/div calculation Matt Burtch
2012-03-23 2:52 ` Shawn Guo
2012-03-23 18:38 ` Matt Burtch
2012-03-23 7:13 ` Lothar Waßmann
2012-03-23 18:38 ` Matt Burtch
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).