[2/4] clocksource: stm32: use prescaler to adjust the resolution

Message ID 20171215085247.14946-3-benjamin.gaignard@st.com
State New
Headers show
Series
  • stm32 clocksource driver rework
Related show

Commit Message

Benjamin Gaignard Dec. 15, 2017, 8:52 a.m.
Rather than use fixed prescaler values compute it to get a clock
as close as possible of 10KHz and a resolution of 0.1ms.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>

---
 drivers/clocksource/timer-stm32.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

-- 
2.15.0

Comments

Daniel Lezcano Dec. 18, 2017, 9:26 a.m. | #1
On 15/12/2017 09:52, Benjamin Gaignard wrote:
> Rather than use fixed prescaler values compute it to get a clock

> as close as possible of 10KHz and a resolution of 0.1ms.

> 

> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>

> ---

>  drivers/clocksource/timer-stm32.c | 23 ++++++++++++++++-------

>  1 file changed, 16 insertions(+), 7 deletions(-)

> 

> diff --git a/drivers/clocksource/timer-stm32.c b/drivers/clocksource/timer-stm32.c

> index 23a321cca45b..de721d318065 100644

> --- a/drivers/clocksource/timer-stm32.c

> +++ b/drivers/clocksource/timer-stm32.c

> @@ -37,6 +37,11 @@

>  

>  #define TIM_EGR_UG	BIT(0)

>  

> +#define MAX_TIM_PSC	0xFFFF

> +

> +/* Target a 10KHz clock to get a resolution of 0.1 ms */

> +#define TARGETED_CLK_RATE 10000

> +

>  static int stm32_clock_event_shutdown(struct clock_event_device *evt)

>  {

>  	struct timer_of *to = to_timer_of(evt);

> @@ -83,7 +88,7 @@ static irqreturn_t stm32_clock_event_handler(int irq, void *dev_id)

>  static void __init stm32_clockevent_init(struct timer_of *to)

>  {

>  	unsigned long max_delta;

> -	int prescaler;

> +	unsigned long prescaler;

>  

>  	to->clkevt.name = "stm32_clockevent";

>  	to->clkevt.features = CLOCK_EVT_FEAT_PERIODIC;

> @@ -96,13 +101,17 @@ static void __init stm32_clockevent_init(struct timer_of *to)

>  	/* Detect whether the timer is 16 or 32 bits */

>  	writel_relaxed(~0U, timer_of_base(to) + TIM_ARR);

>  	max_delta = readl_relaxed(timer_of_base(to) + TIM_ARR);

> -	if (max_delta == ~0U) {

> -		prescaler = 1;

> +	to->clkevt.rating = 50;

> +	if (max_delta == ~0U)

>  		to->clkevt.rating = 250;

> -	} else {

> -		prescaler = 1024;

> -		to->clkevt.rating = 50;

> -	}

> +

> +	/*

> +	 * Get the highest possible prescaler value to be as close

> +	 * as possible of TARGETED_CLK_RATE

> +	 */

> +	prescaler = DIV_ROUND_CLOSEST(timer_of_rate(to), TARGETED_CLK_RATE);


With a 90MHz or 125MHz, the prescaler will be 9000 or 12500, so much
more than the 1024 we have today for 16b, and 1 for 32b.

Shouldn't the computation be weighted with the bits width ?

Otherwise the timer will wrap like:

32bits:

before:	(2^32 / 90e6) x 1 = 47.72 seconds
after:	(2^32 / 90e6) x 9000 = 119.3 *hours* ~= 5days

16bits:

before:	(2^16 / 90e6) x 1024 = 0.745 seconds
after:	(2^16 / 90e6) x 9000 = 6.55 seconds

The patch is ok to target the 10KHz timer rate for 16b with a 1ms
resolution wrapping up after 6.55 seconds. But not for the 32bits timer.
Furthermore, we can't tell anymore the 32bits timers have a rating of
250 after this patch.

Leave the 32bits part as it is and compute the prescaler only in case of
16bits with the target rate, which sounds a reasonable approach.

> +	if (prescaler > MAX_TIM_PSC)

> +		prescaler = MAX_TIM_PSC;


That can happen only if the clock rate is greater than ~655MHz, that
could not happen today as far as I can tell regarding the DT. So if we
hit this condition, we should speak up in the log (pr_warn).

>  	writel_relaxed(0, timer_of_base(to) + TIM_ARR);

>  	writel_relaxed(prescaler - 1, timer_of_base(to) + TIM_PSC);


Can you fix this prescaler - 1 in order to be consistent with the
computation with 16b ? (32b prescaler = 0, 16b prescaler = clk_rate /
target ).

Thanks.

  -- Daniel

-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
Benjamin Gaignard Dec. 18, 2017, 9:44 a.m. | #2
2017-12-18 10:26 GMT+01:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
> On 15/12/2017 09:52, Benjamin Gaignard wrote:

>> Rather than use fixed prescaler values compute it to get a clock

>> as close as possible of 10KHz and a resolution of 0.1ms.

>>

>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>

>> ---

>>  drivers/clocksource/timer-stm32.c | 23 ++++++++++++++++-------

>>  1 file changed, 16 insertions(+), 7 deletions(-)

>>

>> diff --git a/drivers/clocksource/timer-stm32.c b/drivers/clocksource/timer-stm32.c

>> index 23a321cca45b..de721d318065 100644

>> --- a/drivers/clocksource/timer-stm32.c

>> +++ b/drivers/clocksource/timer-stm32.c

>> @@ -37,6 +37,11 @@

>>

>>  #define TIM_EGR_UG   BIT(0)

>>

>> +#define MAX_TIM_PSC  0xFFFF

>> +

>> +/* Target a 10KHz clock to get a resolution of 0.1 ms */

>> +#define TARGETED_CLK_RATE 10000

>> +

>>  static int stm32_clock_event_shutdown(struct clock_event_device *evt)

>>  {

>>       struct timer_of *to = to_timer_of(evt);

>> @@ -83,7 +88,7 @@ static irqreturn_t stm32_clock_event_handler(int irq, void *dev_id)

>>  static void __init stm32_clockevent_init(struct timer_of *to)

>>  {

>>       unsigned long max_delta;

>> -     int prescaler;

>> +     unsigned long prescaler;

>>

>>       to->clkevt.name = "stm32_clockevent";

>>       to->clkevt.features = CLOCK_EVT_FEAT_PERIODIC;

>> @@ -96,13 +101,17 @@ static void __init stm32_clockevent_init(struct timer_of *to)

>>       /* Detect whether the timer is 16 or 32 bits */

>>       writel_relaxed(~0U, timer_of_base(to) + TIM_ARR);

>>       max_delta = readl_relaxed(timer_of_base(to) + TIM_ARR);

>> -     if (max_delta == ~0U) {

>> -             prescaler = 1;

>> +     to->clkevt.rating = 50;

>> +     if (max_delta == ~0U)

>>               to->clkevt.rating = 250;

>> -     } else {

>> -             prescaler = 1024;

>> -             to->clkevt.rating = 50;

>> -     }

>> +

>> +     /*

>> +      * Get the highest possible prescaler value to be as close

>> +      * as possible of TARGETED_CLK_RATE

>> +      */

>> +     prescaler = DIV_ROUND_CLOSEST(timer_of_rate(to), TARGETED_CLK_RATE);

>

> With a 90MHz or 125MHz, the prescaler will be 9000 or 12500, so much

> more than the 1024 we have today for 16b, and 1 for 32b.

>

> Shouldn't the computation be weighted with the bits width ?


My goal was to get the same resolution (0.1ms) for all the timers so
the wrap will depend of the number of bits like you describe below.

>

> Otherwise the timer will wrap like:

>

> 32bits:

>

> before: (2^32 / 90e6) x 1 = 47.72 seconds

> after:  (2^32 / 90e6) x 9000 = 119.3 *hours* ~= 5days

>

> 16bits:

>

> before: (2^16 / 90e6) x 1024 = 0.745 seconds

> after:  (2^16 / 90e6) x 9000 = 6.55 seconds

>

> The patch is ok to target the 10KHz timer rate for 16b with a 1ms

> resolution wrapping up after 6.55 seconds. But not for the 32bits timer.

> Furthermore, we can't tell anymore the 32bits timers have a rating of

> 250 after this patch.


What is the link between rating and resolution (or wrap) ?
Is it a problem to get a long wrap ?

>

> Leave the 32bits part as it is and compute the prescaler only in case of

> 16bits with the target rate, which sounds a reasonable approach.

>

>> +     if (prescaler > MAX_TIM_PSC)

>> +             prescaler = MAX_TIM_PSC;

>

> That can happen only if the clock rate is greater than ~655MHz, that

> could not happen today as far as I can tell regarding the DT. So if we

> hit this condition, we should speak up in the log (pr_warn).


It is to be futur proof for next possible SoC but even if prescaler
reach this limit
it is not a problem the only consequence would be that resolution and
wrap change.

>

>>       writel_relaxed(0, timer_of_base(to) + TIM_ARR);

>>       writel_relaxed(prescaler - 1, timer_of_base(to) + TIM_PSC);

>

> Can you fix this prescaler - 1 in order to be consistent with the

> computation with 16b ? (32b prescaler = 0, 16b prescaler = clk_rate /

> target ).


In the hardware the clock is divise by " TIM_PSC value  1" so to be coherent
with that I need to do prescaler -1.

Benjamin

>

> Thanks.

>

>   -- Daniel

>

> --

>  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

>

> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |

> <http://twitter.com/#!/linaroorg> Twitter |

> <http://www.linaro.org/linaro-blog/> Blog

>
Daniel Lezcano Dec. 18, 2017, 10:54 a.m. | #3
On 18/12/2017 10:44, Benjamin Gaignard wrote:
> 2017-12-18 10:26 GMT+01:00 Daniel Lezcano <daniel.lezcano@linaro.org>:

>> On 15/12/2017 09:52, Benjamin Gaignard wrote:

>>> Rather than use fixed prescaler values compute it to get a clock

>>> as close as possible of 10KHz and a resolution of 0.1ms.

>>>

>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>

>>> ---

>>>  drivers/clocksource/timer-stm32.c | 23 ++++++++++++++++-------

>>>  1 file changed, 16 insertions(+), 7 deletions(-)

>>>

>>> diff --git a/drivers/clocksource/timer-stm32.c b/drivers/clocksource/timer-stm32.c

>>> index 23a321cca45b..de721d318065 100644

>>> --- a/drivers/clocksource/timer-stm32.c

>>> +++ b/drivers/clocksource/timer-stm32.c

>>> @@ -37,6 +37,11 @@

>>>

>>>  #define TIM_EGR_UG   BIT(0)

>>>

>>> +#define MAX_TIM_PSC  0xFFFF

>>> +

>>> +/* Target a 10KHz clock to get a resolution of 0.1 ms */

>>> +#define TARGETED_CLK_RATE 10000

>>> +

>>>  static int stm32_clock_event_shutdown(struct clock_event_device *evt)

>>>  {

>>>       struct timer_of *to = to_timer_of(evt);

>>> @@ -83,7 +88,7 @@ static irqreturn_t stm32_clock_event_handler(int irq, void *dev_id)

>>>  static void __init stm32_clockevent_init(struct timer_of *to)

>>>  {

>>>       unsigned long max_delta;

>>> -     int prescaler;

>>> +     unsigned long prescaler;

>>>

>>>       to->clkevt.name = "stm32_clockevent";

>>>       to->clkevt.features = CLOCK_EVT_FEAT_PERIODIC;

>>> @@ -96,13 +101,17 @@ static void __init stm32_clockevent_init(struct timer_of *to)

>>>       /* Detect whether the timer is 16 or 32 bits */

>>>       writel_relaxed(~0U, timer_of_base(to) + TIM_ARR);

>>>       max_delta = readl_relaxed(timer_of_base(to) + TIM_ARR);

>>> -     if (max_delta == ~0U) {

>>> -             prescaler = 1;

>>> +     to->clkevt.rating = 50;

>>> +     if (max_delta == ~0U)

>>>               to->clkevt.rating = 250;

>>> -     } else {

>>> -             prescaler = 1024;

>>> -             to->clkevt.rating = 50;

>>> -     }

>>> +

>>> +     /*

>>> +      * Get the highest possible prescaler value to be as close

>>> +      * as possible of TARGETED_CLK_RATE

>>> +      */

>>> +     prescaler = DIV_ROUND_CLOSEST(timer_of_rate(to), TARGETED_CLK_RATE);

>>

>> With a 90MHz or 125MHz, the prescaler will be 9000 or 12500, so much

>> more than the 1024 we have today for 16b, and 1 for 32b.

>>

>> Shouldn't the computation be weighted with the bits width ?

> 

> My goal was to get the same resolution (0.1ms) for all the timers so

> the wrap will depend of the number of bits like you describe below.


Do you really want 1ms resolution with a 32bits timer ?

>> Otherwise the timer will wrap like:

>>

>> 32bits:

>>

>> before: (2^32 / 90e6) x 1 = 47.72 seconds

>> after:  (2^32 / 90e6) x 9000 = 119.3 *hours* ~= 5days

>>

>> 16bits:

>>

>> before: (2^16 / 90e6) x 1024 = 0.745 seconds

>> after:  (2^16 / 90e6) x 9000 = 6.55 seconds

>>

>> The patch is ok to target the 10KHz timer rate for 16b with a 1ms

>> resolution wrapping up after 6.55 seconds. But not for the 32bits timer.

>> Furthermore, we can't tell anymore the 32bits timers have a rating of

>> 250 after this patch.

> 

> What is the link between rating and resolution (or wrap) ?


Low resolution => hardly suitable for real use case => bad rating.

From include/linux/clocksource.h

[ ... ]

 *                      100-199: Base level usability.
 *                              Functional for real use, but not desired.
 *                      200-299: Good.
 *                              A correct and usable clocksource.

[ ... ]

If you want to set a timer with a delta of 12.345ms and the resolution
is 1ms. Then you end up with a timer expiring after 13ms.

> Is it a problem to get a long wrap ?


It is not a problem to go for a long wrap, it is usually interesting
when the CPU has deep idle states. But it is not worth to sacrifice the
resolution with the 32bits timer in order to have 5 days before wrap.

Keeping 47secs is fine for the moment. If you want a coarser grain, that
could be acceptable because the resolution is very high but we can
postpone that for later after solving this 16b / 32b thing.

>> Leave the 32bits part as it is and compute the prescaler only in case of

>> 16bits with the target rate, which sounds a reasonable approach.

>>

>>> +     if (prescaler > MAX_TIM_PSC)

>>> +             prescaler = MAX_TIM_PSC;

>>

>> That can happen only if the clock rate is greater than ~655MHz, that

>> could not happen today as far as I can tell regarding the DT. So if we

>> hit this condition, we should speak up in the log (pr_warn).

> 

> It is to be futur proof for next possible SoC but even if prescaler

> reach this limit

> it is not a problem the only consequence would be that resolution and

> wrap change.


Got that, but that needs to be logged with a pr_warn or pr_info.

>>>       writel_relaxed(0, timer_of_base(to) + TIM_ARR);

>>>       writel_relaxed(prescaler - 1, timer_of_base(to) + TIM_PSC);

>>

>> Can you fix this prescaler - 1 in order to be consistent with the

>> computation with 16b ? (32b prescaler = 0, 16b prescaler = clk_rate /

>> target ).

> 

> In the hardware the clock is divise by " TIM_PSC value  1" so to be coherent

> with that I need to do prescaler -1.


Ah, ok.


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
Benjamin Gaignard Dec. 18, 2017, 11:10 a.m. | #4
2017-12-18 11:54 GMT+01:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
> On 18/12/2017 10:44, Benjamin Gaignard wrote:

>> 2017-12-18 10:26 GMT+01:00 Daniel Lezcano <daniel.lezcano@linaro.org>:

>>> On 15/12/2017 09:52, Benjamin Gaignard wrote:

>>>> Rather than use fixed prescaler values compute it to get a clock

>>>> as close as possible of 10KHz and a resolution of 0.1ms.

>>>>

>>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>

>>>> ---

>>>>  drivers/clocksource/timer-stm32.c | 23 ++++++++++++++++-------

>>>>  1 file changed, 16 insertions(+), 7 deletions(-)

>>>>

>>>> diff --git a/drivers/clocksource/timer-stm32.c b/drivers/clocksource/timer-stm32.c

>>>> index 23a321cca45b..de721d318065 100644

>>>> --- a/drivers/clocksource/timer-stm32.c

>>>> +++ b/drivers/clocksource/timer-stm32.c

>>>> @@ -37,6 +37,11 @@

>>>>

>>>>  #define TIM_EGR_UG   BIT(0)

>>>>

>>>> +#define MAX_TIM_PSC  0xFFFF

>>>> +

>>>> +/* Target a 10KHz clock to get a resolution of 0.1 ms */

>>>> +#define TARGETED_CLK_RATE 10000

>>>> +

>>>>  static int stm32_clock_event_shutdown(struct clock_event_device *evt)

>>>>  {

>>>>       struct timer_of *to = to_timer_of(evt);

>>>> @@ -83,7 +88,7 @@ static irqreturn_t stm32_clock_event_handler(int irq, void *dev_id)

>>>>  static void __init stm32_clockevent_init(struct timer_of *to)

>>>>  {

>>>>       unsigned long max_delta;

>>>> -     int prescaler;

>>>> +     unsigned long prescaler;

>>>>

>>>>       to->clkevt.name = "stm32_clockevent";

>>>>       to->clkevt.features = CLOCK_EVT_FEAT_PERIODIC;

>>>> @@ -96,13 +101,17 @@ static void __init stm32_clockevent_init(struct timer_of *to)

>>>>       /* Detect whether the timer is 16 or 32 bits */

>>>>       writel_relaxed(~0U, timer_of_base(to) + TIM_ARR);

>>>>       max_delta = readl_relaxed(timer_of_base(to) + TIM_ARR);

>>>> -     if (max_delta == ~0U) {

>>>> -             prescaler = 1;

>>>> +     to->clkevt.rating = 50;

>>>> +     if (max_delta == ~0U)

>>>>               to->clkevt.rating = 250;

>>>> -     } else {

>>>> -             prescaler = 1024;

>>>> -             to->clkevt.rating = 50;

>>>> -     }

>>>> +

>>>> +     /*

>>>> +      * Get the highest possible prescaler value to be as close

>>>> +      * as possible of TARGETED_CLK_RATE

>>>> +      */

>>>> +     prescaler = DIV_ROUND_CLOSEST(timer_of_rate(to), TARGETED_CLK_RATE);

>>>

>>> With a 90MHz or 125MHz, the prescaler will be 9000 or 12500, so much

>>> more than the 1024 we have today for 16b, and 1 for 32b.

>>>

>>> Shouldn't the computation be weighted with the bits width ?

>>

>> My goal was to get the same resolution (0.1ms) for all the timers so

>> the wrap will depend of the number of bits like you describe below.

>

> Do you really want 1ms resolution with a 32bits timer ?


I want a resolution of 0.1 ms (TARGETED_CLK_RATE = 10.000)
for all the timers or 0.01ms if you think is better.

>

>>> Otherwise the timer will wrap like:

>>>

>>> 32bits:

>>>

>>> before: (2^32 / 90e6) x 1 = 47.72 seconds

>>> after:  (2^32 / 90e6) x 9000 = 119.3 *hours* ~= 5days

>>>

>>> 16bits:

>>>

>>> before: (2^16 / 90e6) x 1024 = 0.745 seconds

>>> after:  (2^16 / 90e6) x 9000 = 6.55 seconds

>>>

>>> The patch is ok to target the 10KHz timer rate for 16b with a 1ms

>>> resolution wrapping up after 6.55 seconds. But not for the 32bits timer.

>>> Furthermore, we can't tell anymore the 32bits timers have a rating of

>>> 250 after this patch.

>>

>> What is the link between rating and resolution (or wrap) ?

>

> Low resolution => hardly suitable for real use case => bad rating.

>

> From include/linux/clocksource.h

>

> [ ... ]

>

>  *                      100-199: Base level usability.

>  *                              Functional for real use, but not desired.

>  *                      200-299: Good.

>  *                              A correct and usable clocksource.

>

> [ ... ]

>

> If you want to set a timer with a delta of 12.345ms and the resolution

> is 1ms. Then you end up with a timer expiring after 13ms.

>

>> Is it a problem to get a long wrap ?

>

> It is not a problem to go for a long wrap, it is usually interesting

> when the CPU has deep idle states. But it is not worth to sacrifice the

> resolution with the 32bits timer in order to have 5 days before wrap.

>

> Keeping 47secs is fine for the moment. If you want a coarser grain, that

> could be acceptable because the resolution is very high but we can

> postpone that for later after solving this 16b / 32b thing.


When the resolution is too high I have issues with min delta value because
CPU can handle interrupt each 11ns.

>

>>> Leave the 32bits part as it is and compute the prescaler only in case of

>>> 16bits with the target rate, which sounds a reasonable approach.

>>>

>>>> +     if (prescaler > MAX_TIM_PSC)

>>>> +             prescaler = MAX_TIM_PSC;

>>>

>>> That can happen only if the clock rate is greater than ~655MHz, that

>>> could not happen today as far as I can tell regarding the DT. So if we

>>> hit this condition, we should speak up in the log (pr_warn).

>>

>> It is to be futur proof for next possible SoC but even if prescaler

>> reach this limit

>> it is not a problem the only consequence would be that resolution and

>> wrap change.

>

> Got that, but that needs to be logged with a pr_warn or pr_info.


OK

>

>>>>       writel_relaxed(0, timer_of_base(to) + TIM_ARR);

>>>>       writel_relaxed(prescaler - 1, timer_of_base(to) + TIM_PSC);

>>>

>>> Can you fix this prescaler - 1 in order to be consistent with the

>>> computation with 16b ? (32b prescaler = 0, 16b prescaler = clk_rate /

>>> target ).

>>

>> In the hardware the clock is divise by " TIM_PSC value  1" so to be coherent

>> with that I need to do prescaler -1.

>

> Ah, ok.

>

>

> --

>  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

>

> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |

> <http://twitter.com/#!/linaroorg> Twitter |

> <http://www.linaro.org/linaro-blog/> Blog

>




-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

Patch

diff --git a/drivers/clocksource/timer-stm32.c b/drivers/clocksource/timer-stm32.c
index 23a321cca45b..de721d318065 100644
--- a/drivers/clocksource/timer-stm32.c
+++ b/drivers/clocksource/timer-stm32.c
@@ -37,6 +37,11 @@ 
 
 #define TIM_EGR_UG	BIT(0)
 
+#define MAX_TIM_PSC	0xFFFF
+
+/* Target a 10KHz clock to get a resolution of 0.1 ms */
+#define TARGETED_CLK_RATE 10000
+
 static int stm32_clock_event_shutdown(struct clock_event_device *evt)
 {
 	struct timer_of *to = to_timer_of(evt);
@@ -83,7 +88,7 @@  static irqreturn_t stm32_clock_event_handler(int irq, void *dev_id)
 static void __init stm32_clockevent_init(struct timer_of *to)
 {
 	unsigned long max_delta;
-	int prescaler;
+	unsigned long prescaler;
 
 	to->clkevt.name = "stm32_clockevent";
 	to->clkevt.features = CLOCK_EVT_FEAT_PERIODIC;
@@ -96,13 +101,17 @@  static void __init stm32_clockevent_init(struct timer_of *to)
 	/* Detect whether the timer is 16 or 32 bits */
 	writel_relaxed(~0U, timer_of_base(to) + TIM_ARR);
 	max_delta = readl_relaxed(timer_of_base(to) + TIM_ARR);
-	if (max_delta == ~0U) {
-		prescaler = 1;
+	to->clkevt.rating = 50;
+	if (max_delta == ~0U)
 		to->clkevt.rating = 250;
-	} else {
-		prescaler = 1024;
-		to->clkevt.rating = 50;
-	}
+
+	/*
+	 * Get the highest possible prescaler value to be as close
+	 * as possible of TARGETED_CLK_RATE
+	 */
+	prescaler = DIV_ROUND_CLOSEST(timer_of_rate(to), TARGETED_CLK_RATE);
+	if (prescaler > MAX_TIM_PSC)
+		prescaler = MAX_TIM_PSC;
 
 	writel_relaxed(0, timer_of_base(to) + TIM_ARR);
 	writel_relaxed(prescaler - 1, timer_of_base(to) + TIM_PSC);