From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932847AbcBBPEF (ORCPT ); Tue, 2 Feb 2016 10:04:05 -0500 Received: from aserp1040.oracle.com ([141.146.126.69]:46970 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932304AbcBBPED (ORCPT ); Tue, 2 Feb 2016 10:04:03 -0500 From: Nix To: Thomas Gleixner Cc: Andres Salomon , arnd@arndb.de, linux-kernel@vger.kernel.org Subject: Re: 4.4.1 regression from 4.1.x: Soekris net5501 crash in IRQ after mfgpt timer initialization References: <87a8njchvj.fsf@esperi.org.uk> Emacs: ... it's not just a way of life, it's a text editor! Date: Tue, 02 Feb 2016 15:03:45 +0000 In-Reply-To: (Thomas Gleixner's message of "Tue, 2 Feb 2016 15:40:08 +0100 (CET)") Message-ID: <8760y7cfxq.fsf@esperi.org.uk> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Source-IP: userv0021.oracle.com [156.151.31.71] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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 Inspired-by: Thomas Gleixner --- 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