From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756266AbcARSmx (ORCPT ); Mon, 18 Jan 2016 13:42:53 -0500 Received: from down.free-electrons.com ([37.187.137.238]:59782 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1755161AbcARSmt (ORCPT ); Mon, 18 Jan 2016 13:42:49 -0500 Date: Mon, 18 Jan 2016 19:42:47 +0100 From: Alexandre Belloni To: Sebastian Andrzej Siewior Cc: Thomas Gleixner , Boris Brezillon , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-rt-users@vger.kernel.org, Nicolas Ferre Subject: Re: [PATCH 4.4-rt2] fix arm-at91-pit-remove-irq-handler-when-clock-is-unused.patch Message-ID: <20160118184247.GQ3367@piout.net> References: <1452997394-8554-1-git-send-email-alexandre.belloni@free-electrons.com> <1452997394-8554-2-git-send-email-alexandre.belloni@free-electrons.com> <20160118172522.GB12309@linutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20160118172522.GB12309@linutronix.de> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 18/01/2016 at 18:25:22 +0100, Sebastian Andrzej Siewior wrote : > * Alexandre Belloni | 2016-01-17 03:23:14 [+0100]: > > >index 80d74c4adcbe..43b50634d640 100644 > >--- a/drivers/clocksource/timer-atmel-pit.c > >+++ b/drivers/clocksource/timer-atmel-pit.c > >@@ -96,11 +96,44 @@ static int pit_clkevt_shutdown(struct clock_event_device *dev) > > > > /* disable irq, leaving the clocksource active */ > > pit_write(data->base, AT91_PIT_MR, (data->cycle - 1) | AT91_PIT_PITEN); > >- free_irq(atmel_pit_irq, data); > >+ if (!clockevent_state_detached(dev)) > >+ free_irq(data->irq, data); > > I did it in the meantime without clockevent_state_detached(). From what > it looks, it first sets the state and then invokes > pit_clkevt_shutdown(). Any particular reason for this? > Yeah, I forgot to mention that. Freeing the irq unconditionally results in: ------------[ cut here ]------------ WARNING: CPU: 0 PID: 0 at kernel/irq/manage.c:1541 __free_irq+0xb4/0x2c8() Trying to free already-free IRQ 16 Modules linked in: CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.4.0-rt2+ #31 Hardware name: Atmel SAMA5 [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0x80/0x94) [] (dump_stack) from [] (warn_slowpath_common+0x80/0xb0) [] (warn_slowpath_common) from [] (warn_slowpath_fmt+0x30/0x40) [] (warn_slowpath_fmt) from [] (__free_irq+0xb4/0x2c8) [] (__free_irq) from [] (free_irq+0x3c/0x70) [] (free_irq) from [] (pit_clkevt_shutdown+0x24/0x2c) [] (pit_clkevt_shutdown) from [] (clockevents_switch_state+0x60/0x130) [] (clockevents_switch_state) from [] (clockevents_exchange_device+0x78/0x8c) [] (clockevents_exchange_device) from [] (tick_check_new_device+0x90/0xd0) [] (tick_check_new_device) from [] (clockevents_register_device+0x54/0x10c) [] (clockevents_register_device) from [] (clocksource_probe+0x4c/0x90) [] (clocksource_probe) from [] (start_kernel+0x278/0x3a4) [] (start_kernel) from [<2000807c>] (0x2000807c) ---[ end trace 0000000000000001 ]--- My understanding is that clockevents_exchange_device() changes the state from detached to shutdown and so at that point the IRQ has never been requested. > > return 0; > > } > > > > /* > >+ * IRQ handler for the timer. > >+ */ > >+static irqreturn_t at91sam926x_pit_interrupt(int irq, void *dev_id) > > this is just here to avoid to forward declaration. > … Indeed. > >diff --git a/drivers/clocksource/timer-atmel-st.c b/drivers/clocksource/timer-atmel-st.c > >index ea37afc26e1b..11ce404d0791 100644 > >--- a/drivers/clocksource/timer-atmel-st.c > >+++ b/drivers/clocksource/timer-atmel-st.c > >@@ -229,15 +229,15 @@ static void __init atmel_st_timer_init(struct device_node *node) > > regmap_read(regmap_st, AT91_ST_SR, &val); > > > > /* Get the interrupts property */ > >- irq = irq_of_parse_and_map(node, 0); > >- if (!irq) > >+ atmel_st_irq = irq_of_parse_and_map(node, 0); > >+ if (!atmel_st_irq) > > panic(pr_fmt("Unable to get IRQ from DT\n")); > > > > sclk = of_clk_get(node, 0); > > if (IS_ERR(sclk)) > > panic(pr_fmt("Unable to get slow clock\n")); > > > >- clk_prepare_enable(sclk); > >+ ret = clk_prepare_enable(sclk); > this piece applies to upstream v4.4. > Yeah, I'll submit it. -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com