From patchwork Wed Jan 22 16:49:05 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ezequiel Garcia X-Patchwork-Id: 313325 Return-Path: X-Original-To: incoming-imx@patchwork.ozlabs.org Delivered-To: patchwork-incoming-imx@bilbo.ozlabs.org Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:770:15f::2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 988B42C0082 for ; Thu, 23 Jan 2014 03:49:33 +1100 (EST) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1W60zM-0001Cy-Vj; Wed, 22 Jan 2014 16:49:17 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1W60zK-0004fF-K8; Wed, 22 Jan 2014 16:49:14 +0000 Received: from top.free-electrons.com ([176.31.233.9] helo=mail.free-electrons.com) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1W60zH-0004er-P0 for linux-arm-kernel@lists.infradead.org; Wed, 22 Jan 2014 16:49:12 +0000 Received: by mail.free-electrons.com (Postfix, from userid 106) id CC2987FA; Wed, 22 Jan 2014 17:48:49 +0100 (CET) X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on mail.free-electrons.com X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,SHORTCIRCUIT shortcircuit=ham autolearn=disabled version=3.3.2 Received: from localhost (unknown [190.2.98.212]) by mail.free-electrons.com (Postfix) with ESMTPSA id F35C57AE; Wed, 22 Jan 2014 17:48:45 +0100 (CET) Date: Wed, 22 Jan 2014 13:49:05 -0300 From: Ezequiel Garcia To: Jason Gunthorpe , Sebastian Hesselbarth Subject: Re: [PATCH v2 06/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear Message-ID: <20140122164904.GB27273@localhost> References: <1390295561-3466-1-git-send-email-ezequiel.garcia@free-electrons.com> <1390295561-3466-7-git-send-email-ezequiel.garcia@free-electrons.com> <20140121233537.GS18269@obsidianresearch.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20140121233537.GS18269@obsidianresearch.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140122_114911_987024_5B517DAB X-CRM114-Status: GOOD ( 22.47 ) X-Spam-Score: -1.8 (-) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-1.8 points) pts rule name description ---- ---------------------- -------------------------------------------------- 0.7 SPF_SOFTFAIL SPF: sender does not match SPF record (softfail) -0.6 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Lior Amsalem , devicetree@vger.kernel.org, linux-watchdog@vger.kernel.org, Tawfik Bayouk , Andrew Lunn , Wim Van Sebroeck , Gregory Clement , Thomas Petazzoni , linux-arm-kernel@lists.infradead.org, Jason Cooper X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org List-Id: linux-imx-kernel.lists.patchwork.ozlabs.org On Tue, Jan 21, 2014 at 04:35:37PM -0700, Jason Gunthorpe wrote: > On Tue, Jan 21, 2014 at 06:12:32AM -0300, Ezequiel Garcia wrote: > > After adding the IRQ request, the BRIDGE_CAUSE bit should be cleared by the > > bridge interrupt controller. There's no longer a need to do it in the watchdog > > driver, so we can simply remove it. > > When we talked about this before I pointed out that sequence here is > important: > > - Disable WDT > - Clear bridge > - Enable WDT > Sure, I wrote this series with that in mind and made some tests to ensure that no spurious trigger was possible. > Looking at this patch in isolation it looks to me like the clear > bridge lines should be replaced with a request_irq (as that does the > clear) - is the request_irq in the wrong spot? > First of all, it seems to me that the first item "Disable WDT" is not currently true on this driver. orion_wdt_start() seem to reset the counter, but doesn't clear the WDT_EN bit. Do you think we should enforce a "true" disable? As an example, s3c2410wdt calls stop() from start(). Maybe we should do something like it? Regarding the sequence. Let me see if I got this problem right. The concern here is about the bootloader leaving the registers in a weird-state, right? In that case, I thought that requesting the IRQ at probe time was enough to ensure the BRIDGE_CAUSE would be cleared by the time the watchdog is started. However, after reading through the irqchip code again, I'm no longer sure this is the case. It looks like the BRIDGE_CAUSE register is cleared when the interruption is acked (which happens in the handler if I understood the code right). So requesting the IRQ is useless... I'll trace the code to confirm this. Sebastian: If the above is correct, do you think we can add a cause clear to the orion irqchip? (supposing it's harmful) Something like this: diff --git a/drivers/irqchip/irq-orion.c b/drivers/irqchip/irq-orion.c index e51d400..fef9171 100644 --- a/drivers/irqchip/irq-orion.c +++ b/drivers/irqchip/irq-orion.c @@ -180,6 +180,9 @@ static int __init orion_bridge_irq_init(struct device_node *np, gc->chip_types[0].chip.irq_mask = irq_gc_mask_clr_bit; gc->chip_types[0].chip.irq_unmask = irq_gc_mask_set_bit; + /* clear pending interrupts */ + writel(0, gc->reg_base + ORION_BRIDGE_IRQ_CAUSE); + /* mask all interrupts */ writel(0, gc->reg_base + ORION_BRIDGE_IRQ_MASK);