On Thu, 12 Oct 2017, Thomas Gleixner wrote:

> On Thu, 12 Oct 2017, Daniel Lezcano wrote:
>
>> On 11/10/2017 22:48, David Kozub wrote:
>>
>> [ ... ]
>>
>>>>
>>>> +       disable_timer(timer);
>>>> +       cs5535_mfgpt_write(timer, MFGPT_REG_COUNTER, 0);
>>>> +
>>>>        /* Set up the IRQ on the MFGPT side */
>>>>        if (cs5535_mfgpt_setup_irq(timer, MFGPT_CMP2, &timer_irq)) {
>>>>                printk(KERN_ERR DRV_NAME ": Could not set up IRQ %d\n",
>>>
>>> I tried that and the handler is still called. So I did some more random
>>> experiments and I found out that if I call disable_timer(timer) twice,
>>> then the issue is resolved (the handler is not called before the
>>> registration is finished.) And I don't have to set MFGPT_REG_COUNTER to 0.
>>
>> Aha! we are close to a fix.
>>
>>> I have no idea why do I have to call disable_timer twice.
>>
>> For testing purpose, can you try by adding mmiowb() and/or wmb() after
>> disable_timer()?

I tried one, the other, both. Still, the unexpected handler call happens. 
I (again) added a msleep(1000) after the disable (and the barriers). If 
the trouble was a missing barrier, the sleep would most likely also make 
the isaue go away (even if in a hacky way).

> The real question is why
>
>        /* Set the clock scale and enable the event mode for CMP2 */
>        val = MFGPT_SCALE | (3 << 8);
>
>        cs5535_mfgpt_write(cs5535_event_clock, MFGPT_REG_SETUP, val);
>
> is in the setup code at all.
>
> The obvious place for this is in mfgpt_set_periodic() which gets called
> when the clock event and the handler is set up in the core code. Up to that
> point the interrupt handler is protected against shared interrupts via the
> is_shutdown() check.

It is, but only after clockevents_config_and_register. But mfgpt_tick is 
called before that (just after setup_irq).

Best regards,
David