From patchwork Wed Jan 20 11:07:30 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Gleixner X-Patchwork-Id: 8071151 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id D7FE39F6FA for ; Wed, 20 Jan 2016 11:11:12 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id E5AF920443 for ; Wed, 20 Jan 2016 11:11:11 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id BFE0F20437 for ; Wed, 20 Jan 2016 11:11:10 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1aLqdF-00016y-AX; Wed, 20 Jan 2016 11:08:57 +0000 Received: from linutronix.de ([2001:470:1f0b:db:abcd:42:0:1] helo=Galois.linutronix.de) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1aLqdA-0000mU-Nc for linux-arm-kernel@lists.infradead.org; Wed, 20 Jan 2016 11:08:54 +0000 Received: from localhost ([127.0.0.1]) by Galois.linutronix.de with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.80) (envelope-from ) id 1aLqcm-0007Cw-P9; Wed, 20 Jan 2016 12:08:28 +0100 Date: Wed, 20 Jan 2016 12:07:30 +0100 (CET) From: Thomas Gleixner To: Alexandre Belloni Subject: Re: [PATCH 4.4-rt2] fix arm-at91-pit-remove-irq-handler-when-clock-is-unused.patch In-Reply-To: <20160118184247.GQ3367@piout.net> Message-ID: 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> <20160118184247.GQ3367@piout.net> User-Agent: Alpine 2.11 (DEB 23 2013-08-11) MIME-Version: 1.0 X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1, SHORTCIRCUIT=-0.0001 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160120_030852_959717_E5CF47E2 X-CRM114-Status: GOOD ( 26.69 ) X-Spam-Score: -1.7 (-) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Boris Brezillon , linux-rt-users@vger.kernel.org, Sebastian Andrzej Siewior , Nicolas Ferre , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, 18 Jan 2016, Alexandre Belloni wrote: > 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: Well freeing the irq from that context in RT only works because its called before SYSTEM_STATE=RUNNING. So no, this was wrong forever. The issue we are dealing with is that the timer interrupt is shared with the uart. So the timer has IRQ_NO_THREAD set and the uart interrupt gets force threaded. So that results in a failure to request the interrupt for the UART. That's not RT specific, that already happens in mainline if you add 'threadirqs' to the command line. So until the DT folks come to senses and we get that dummy demux chip done, I came up with the following - completely untested - solution. The downside of this is, that the timer will be delayed until the uart thread returns, but with the replacement clockevent in place on RT that's a non issue. For mainline it's obviously better than what we have now. Thanks, tglx 8<--------------- --- a/include/linux/interrupt.h +++ b/include/linux/interrupt.h @@ -61,6 +61,10 @@ * interrupt handler after suspending interrupts. For system * wakeup devices users need to implement wakeup detection in * their interrupt handlers. + * IRQF_COND_ONESHOT - If the IRQ is shared between a NO_THREAD user and a + * regular interrupt, force the ONESHOT flag on the NO_THREAD user + * when threaded irqs are enforced. Workaround for silly ATMEL + * SoCs which share the timer and the UART interrupt * IRQF_NO_SOFTIRQ_CALL - Do not process softirqs in the irq thread context (RT) */ #define IRQF_SHARED 0x00000080 @@ -75,7 +79,8 @@ #define IRQF_NO_THREAD 0x00010000 #define IRQF_EARLY_RESUME 0x00020000 #define IRQF_COND_SUSPEND 0x00040000 -#define IRQF_NO_SOFTIRQ_CALL 0x00080000 +#define IRQF_COND_ONESHOT 0x00080000 +#define IRQF_NO_SOFTIRQ_CALL 0x00100000 #define IRQF_TIMER (__IRQF_TIMER | IRQF_NO_SUSPEND | IRQF_NO_THREAD) --- a/kernel/irq/manage.c +++ b/kernel/irq/manage.c @@ -1208,6 +1208,14 @@ static int new->irq = irq; /* + * Workaround for silly ATMEL SoCs with shared timer and uart + * interrupt. + */ + if (force_irqthreads && (new->flags & IRQF_COND_ONESHOT) && + (new->flags & IRQF_NO_THREAD)) + new->flags |= IRQF_ONESHOT; + + /* * Check whether the interrupt nests into another interrupt * thread. */ --- a/drivers/clocksource/timer-atmel-pit.c +++ b/drivers/clocksource/timer-atmel-pit.c @@ -208,7 +208,7 @@ static void __init at91sam926x_pit_commo /* Set up irq handler */ ret = request_irq(data->irq, at91sam926x_pit_interrupt, - IRQF_SHARED | IRQF_TIMER | IRQF_IRQPOLL, + IRQF_SHARED | IRQF_TIMER | IRQF_IRQPOLL| IRQF_COND_ONESHOT, "at91_tick", data); if (ret) panic(pr_fmt("Unable to setup IRQ\n")); --- a/drivers/clocksource/timer-atmel-st.c +++ b/drivers/clocksource/timer-atmel-st.c @@ -216,7 +216,7 @@ static void __init atmel_st_timer_init(s /* Make IRQs happen for the system timer */ ret = request_irq(irq, at91rm9200_timer_interrupt, - IRQF_SHARED | IRQF_TIMER | IRQF_IRQPOLL, + IRQF_SHARED | IRQF_TIMER | IRQF_IRQPOLL | IRQF_COND_ONESHOT, "at91_tick", regmap_st); if (ret) panic(pr_fmt("Unable to setup IRQ\n"));