[PATCH 2/4] clocksource: stm32: use prescaler to adjust the resolution
From: Daniel Lezcano <hidden>
Date: 2017-12-18 09:26:19
Also in:
lkml
On 15/12/2017 09:52, Benjamin Gaignard wrote:
quoted hunk ↗ jump to hunk
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 <redacted> --- 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