Re: 4.4.1 regression from 4.1.x: Soekris net5501 crash in IRQ after mfgpt timer initialization

[Nix <nix@esperi.org.uk>]

On 2 Feb 2016, Thomas Gleixner said:

> On Tue, 2 Feb 2016, Nix wrote:
>
>> [Cc:ed Thomas on the vague hope that maybe this is osmething to do with
>>  the IRQ subsystem in general, though I doubt it, since only the one
>>  machine is crashing for me: it's probably the CS5531's interactions
>>  with said subsystem at fault.]
>
> Kinda. That driver does the following:
>
>        setup the irq in CS5531
>
>        request the interrupt to install the handler
>
>        register the clockevents device

It seems like it should do those in the opposite order, really, or at
the very least do the IRQ setup last!

> So the interrupt hits before the clockevent device is registered and the event
> handler is installed. So mfgpt_tick() will happily call a null pointer.
>
> The patch below should fix^Wwork around the issue.

The fairly trivial code motion below also seems to work, and may be more
like an actual fix, though I'm a bit horrified that it's this simple. I
may well have moved too much and unknowingly violated some invariant.

(Note: the actual code motion was of course to move the IRQ registration
down, but git chose to depict it as the opposite, somewhat unclearly.)

Done under my work address because all this firewall rebooting is
stopping me from getting work done:

>From 4ba04a48573c8a2136533556a3fbef7de288913f Mon Sep 17 00:00:00 2001
From: Nick Alcock <nick.alcock@oracle.com>
Date: Tue, 2 Feb 2016 14:57:56 +0000
Subject: [PATCH] cs5535-clockevt: set up the MFGPT only after registering the
 IRQ

This prevents a race whereby the IRQ arrives before the clockevent
handler is installed.

Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
Inspired-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/clocksource/cs5535-clockevt.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/clocksource/cs5535-clockevt.c b/drivers/clocksource/cs5535-clockevt.c
index 9a7e37c..5737e17 100644
--- a/drivers/clocksource/cs5535-clockevt.c
+++ b/drivers/clocksource/cs5535-clockevt.c
@@ -152,6 +152,18 @@ static int __init cs5535_mfgpt_init(void)
 	}
 	cs5535_event_clock = timer;
 
+	/* 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);
+
+	/* Set up the clock event */
+	printk(KERN_INFO DRV_NAME
+		": Registering MFGPT timer as a clock event, using IRQ %d\n",
+		timer_irq);
+	clockevents_config_and_register(&cs5535_clockevent, MFGPT_HZ,
+					0xF, 0xFFFE);
+
 	/* 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",
@@ -166,18 +178,6 @@ static int __init cs5535_mfgpt_init(void)
 		goto err_irq;
 	}
 
-	/* 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);
-
-	/* Set up the clock event */
-	printk(KERN_INFO DRV_NAME
-		": Registering MFGPT timer as a clock event, using IRQ %d\n",
-		timer_irq);
-	clockevents_config_and_register(&cs5535_clockevent, MFGPT_HZ,
-					0xF, 0xFFFE);
-
 	return 0;
 
 err_irq:
-- 
2.7.0.198.g6dd47b6