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: 3524151 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.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id E60C99F2D6 for ; Wed, 22 Jan 2014 16:49:32 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id CC42020171 for ; Wed, 22 Jan 2014 16:49:31 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D856420170 for ; Wed, 22 Jan 2014 16:49:26 +0000 (UTC) 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.1 (2010-03-16) on mail.kernel.org X-Spam-Level: X-Spam-Status: No, score=-4.8 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 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 (-) 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-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP 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);