The commit
77cc982 clocksource: use clockevents_config_and_register() where possible
switches from manually calculating min_delta_ns (and others) and
clockevents_register_device() to automatic calculation via
clockevents_config_and_register(). During this conversation the "+ 1" in
min_delta_ns = clockevent_delta2ns(1, &clkevt.clkevt) + 1;
was lost. This leads to problems when programming clock events, resuling in
e.g. a sleep(2) sleeping more than 3 seconds. The "+ 1" was added in the
original code to fix a rounding problem in clockevent_delta2ns(), see
http://permalink.gmane.org/gmane.linux.kernel/549744 for background
information.
This patch fixes the problem by increasing the min_delta to "2" ticks.
Cc: Marc Pignat <[email protected]>
Cc: Ronald Wahl <[email protected]>
Acked-by: Shawn Guo <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
---
Changes since v1:
- Improved description. Thanks to Ronald Wahl.
drivers/clocksource/tcb_clksrc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clocksource/tcb_clksrc.c b/drivers/clocksource/tcb_clksrc.c
index 8a61872..7cf6dc7 100644
--- a/drivers/clocksource/tcb_clksrc.c
+++ b/drivers/clocksource/tcb_clksrc.c
@@ -197,7 +197,7 @@ static void __init setup_clkevents(struct atmel_tc *tc, int clk32k_divisor_idx)
clkevt.clkevt.cpumask = cpumask_of(0);
- clockevents_config_and_register(&clkevt.clkevt, 32768, 1, 0xffff);
+ clockevents_config_and_register(&clkevt.clkevt, 32768, 2, 0xffff);
setup_irq(irq, &tc_irqaction);
}
--
1.8.4.rc3
On Tue, Sep 17, 2013 at 11:56:00AM +0200, Ludovic Desroches wrote:
> Any reason to not do this:
>
> --- a/drivers/clocksource/tcb_clksrc.c
> +++ b/drivers/clocksource/tcb_clksrc.c
> @@ -144,6 +144,9 @@ static void tc_mode(enum clock_event_mode m, struct
> clock_event_device *d)
>
> static int tc_next_event(unsigned long delta, struct clock_event_device
> *d)
> {
> + if (delta < d->min_delta_ticks)
> + delta = d->min_delta_ticks;
> +
> __raw_writel(delta, tcaddr + ATMEL_TC_REG(2, RC));
>
> /* go go gadget! */
>
> Then we can keep the same min_delta.
You really should not play such games in your set_next_event() code - if
the interval is not supported, you should return -ETIME so that the core
code knows about it and can adjust things to suit. If you're getting
deltas which are too small for the hardware, that'll either be because
the bounds are wrong, or there's a bug in the core code.
On Tue, 17 Sep 2013, Russell King - ARM Linux wrote:
> On Tue, Sep 17, 2013 at 11:56:00AM +0200, Ludovic Desroches wrote:
> > Any reason to not do this:
> >
> > --- a/drivers/clocksource/tcb_clksrc.c
> > +++ b/drivers/clocksource/tcb_clksrc.c
> > @@ -144,6 +144,9 @@ static void tc_mode(enum clock_event_mode m, struct
> > clock_event_device *d)
> >
> > static int tc_next_event(unsigned long delta, struct clock_event_device
> > *d)
> > {
> > + if (delta < d->min_delta_ticks)
> > + delta = d->min_delta_ticks;
> > +
> > __raw_writel(delta, tcaddr + ATMEL_TC_REG(2, RC));
> >
> > /* go go gadget! */
> >
> > Then we can keep the same min_delta.
>
> You really should not play such games in your set_next_event() code - if
> the interval is not supported, you should return -ETIME so that the core
> code knows about it and can adjust things to suit. If you're getting
> deltas which are too small for the hardware, that'll either be because
> the bounds are wrong, or there's a bug in the core code.
The core code does:
int clockevents_program_event(struct clock_event_device *dev, ktime_t expires,
bool force)
{
....
delta = min(delta, (int64_t) dev->max_delta_ns);
delta = max(delta, (int64_t) dev->min_delta_ns);
clc = ((unsigned long long) delta * dev->mult) >> dev->shift;
rc = dev->set_next_event((unsigned long) clc, dev);
}
So, the min_delta in timer ticks is set to 1 and converted by the core
to nsecs for various reasons. This is where the rounding problem comes
into play.
The patch below should fix that.
Thanks,
tglx
----
diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 38959c8..41b7a30 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -42,7 +42,7 @@ struct ce_unbind {
*/
u64 clockevent_delta2ns(unsigned long latch, struct clock_event_device *evt)
{
- u64 clc = (u64) latch << evt->shift;
+ u64 tmp, clc = (u64) latch << evt->shift;
if (unlikely(!evt->mult)) {
evt->mult = 1;
@@ -52,6 +52,14 @@ u64 clockevent_delta2ns(unsigned long latch, struct clock_event_device *evt)
do_div(clc, evt->mult);
if (clc < 1000)
clc = 1000;
+
+ /* Avoid rounding artifacts */
+ tmp = (clc * dev->mult) >> dev->shift;
+ while (tmp < (u64) dev->min_delta_ticks) {
+ clc += 1000;
+ tmp = (clc * dev->mult) >> dev->shift;
+ }
+
if (clc > KTIME_MAX)
clc = KTIME_MAX;