From patchwork Mon Jan 26 16:24:03 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 5711181 Return-Path: X-Original-To: patchwork-linux-rockchip@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 84028C058D for ; Mon, 26 Jan 2015 16:26:18 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 6A4A820115 for ; Mon, 26 Jan 2015 16:26:17 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (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 7E6D9200FF for ; Mon, 26 Jan 2015 16:26:16 +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 1YFmUR-0004wI-NC; Mon, 26 Jan 2015 16:26:15 +0000 Received: from mail-ie0-x22b.google.com ([2607:f8b0:4001:c03::22b]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1YFmSx-0001dz-Dy for linux-rockchip@lists.infradead.org; Mon, 26 Jan 2015 16:24:44 +0000 Received: by mail-ie0-f171.google.com with SMTP id tr6so9617427ieb.2 for ; Mon, 26 Jan 2015 08:24:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id; bh=q2yGE6Wr+NFrxo6n54p/XDyJHXcLQdwNIUCrUJCdCS0=; b=Fdmh4zs4X5VHEYg535+/dKZ9QUexChxauFqNisAis83zR9fc+ZXlUL24gKaFJLPo8O Etb10bsjZIG6/rPTM3p1qrm2fgzowCtorPb+fBniag+a+Xf+1XiGYFMSWZtC22zq5dSK uhSMFuC3yE0g54DSvNYnw+N8SPrGaDYQGZP6k= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=q2yGE6Wr+NFrxo6n54p/XDyJHXcLQdwNIUCrUJCdCS0=; b=MzTNvOFf+B4IQUdFc9KfQOnzTfxodVSqnaGKbfWFdvoLUz3fhGsma4xeNVQl0G9LoB Hdylwa3MsDgjImvg8ntYEpo8KQDJsIyQ62ICaco5dje7Uj7wZHe4dWN+qwc/hEnCTEq6 Hmq1B+FKX3oZFFUA89+s1t2/dty+U9q7EZMaKQp1HPv6+u2WcNTy8ieUUPaEM3Aqd2HI BFYRnpejI0TSTmyDFsszy5VNHpeyLYDalxc8EbdeLWtZ9Q+4tIV30k5JN5khmkxzU0G1 LW6BZv5EgQOMcMgziUKwcSHocrkCuyUHikLEClwkrQPEp3BUVk3PnO34wtYmWPfKnz0/ ksyg== X-Gm-Message-State: ALoCoQnP+Oa48DVyv5trfPCSWHuwq+pNXxM0OCPZao6urZFNEvAk9wn3FoaMNRCKLoZA1oFXmcHL X-Received: by 10.50.102.4 with SMTP id fk4mr17078246igb.28.1422289460675; Mon, 26 Jan 2015 08:24:20 -0800 (PST) Received: from tictac.mtv.corp.google.com ([172.22.65.76]) by mx.google.com with ESMTPSA id s94sm6132834ioe.40.2015.01.26.08.24.19 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 26 Jan 2015 08:24:19 -0800 (PST) From: Doug Anderson To: Linus Walleij , Heiko Stuebner Subject: [PATCH] pinctrl: rockchip: Only mask interrupts; never disable Date: Mon, 26 Jan 2015 08:24:03 -0800 Message-Id: <1422289444-15142-1-git-send-email-dianders@chromium.org> X-Mailer: git-send-email 2.2.0.rc0.207.ga3a616c X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150126_082443_566127_9D6AC877 X-CRM114-Status: GOOD ( 12.42 ) X-Spam-Score: -0.8 (/) Cc: linux-gpio@vger.kernel.org, Jeffy Chen , linux-kernel@vger.kernel.org, Doug Anderson , linux-rockchip@lists.infradead.org, linux-arm-kernel@lists.infradead.org X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+patchwork-linux-rockchip=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID, T_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 The Rockchip GPIO interrupt controller totally throws away all status about an interrupt when you "disable" the interrupt. That has unfortunate consequences in the following situation: 1. An edge-triggered interrupt is enabled and should wake the system. 2. System suspend happens: interrupt is disabled and marked for wake. 3. rockchip_irq_suspend() reenables the interrupt so we can wake. 4. Interrupt happens when asleep. 5. rockchip_irq_resume() redisables the interrupt. 6. Disabling the interrupt throws away all status about it. 7. Normal system resume happens and we enable the interrupt again, since we threw away status about the interrupt we don't know it fired while suspended. Even worse: if we need both edges of the interrupt the logic to swap edges never runs. Note: even if we somehow can post the status about wakeup interrupts in rockchip_irq_resume() we would still have a window of losing any edges that came in while interrupts were disabled. If we use mask only then we don't need to worry. The GPIO Interrupt controller keeps track of pending interrupts that are enabled and just masked. There was no real strong reason to support the enable/disable functionality (other than that it seemed right), so let's go back to just supporting mask/unmask but actually map it to the real mask/unmask. This ends up with slightly different (and more correct) behavior than before (f2dd028 pinctrl: rockchip: Fix enable/disable/mask/unmask). Signed-off-by: Doug Anderson Reviewed-by: Heiko Stuebner --- drivers/pinctrl/pinctrl-rockchip.c | 48 +++++++++++--------------------------- 1 file changed, 13 insertions(+), 35 deletions(-) diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c index d144330..dee7d5f 100644 --- a/drivers/pinctrl/pinctrl-rockchip.c +++ b/drivers/pinctrl/pinctrl-rockchip.c @@ -89,7 +89,7 @@ struct rockchip_iomux { * @reg_pull: optional separate register for additional pull settings * @clk: clock of the gpio bank * @irq: interrupt of the gpio bank - * @saved_enables: Saved content of GPIO_INTEN at suspend time. + * @saved_masks: Saved content of GPIO_INTEN at suspend time. * @pin_base: first pin number * @nr_pins: number of pins in this bank * @name: name of the bank @@ -108,7 +108,7 @@ struct rockchip_pin_bank { struct regmap *regmap_pull; struct clk *clk; int irq; - u32 saved_enables; + u32 saved_masks; u32 pin_base; u8 nr_pins; char *name; @@ -1545,8 +1545,8 @@ static void rockchip_irq_suspend(struct irq_data *d) struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); struct rockchip_pin_bank *bank = gc->private; - bank->saved_enables = irq_reg_readl(gc, GPIO_INTEN); - irq_reg_writel(gc, gc->wake_active, GPIO_INTEN); + bank->saved_masks = irq_reg_readl(gc, GPIO_INTMASK); + irq_reg_writel(gc, ~gc->wake_active, GPIO_INTMASK); } static void rockchip_irq_resume(struct irq_data *d) @@ -1554,35 +1554,7 @@ static void rockchip_irq_resume(struct irq_data *d) struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); struct rockchip_pin_bank *bank = gc->private; - irq_reg_writel(gc, bank->saved_enables, GPIO_INTEN); -} - -static void rockchip_irq_disable(struct irq_data *d) -{ - struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); - u32 val; - - irq_gc_lock(gc); - - val = irq_reg_readl(gc, GPIO_INTEN); - val &= ~d->mask; - irq_reg_writel(gc, val, GPIO_INTEN); - - irq_gc_unlock(gc); -} - -static void rockchip_irq_enable(struct irq_data *d) -{ - struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); - u32 val; - - irq_gc_lock(gc); - - val = irq_reg_readl(gc, GPIO_INTEN); - val |= d->mask; - irq_reg_writel(gc, val, GPIO_INTEN); - - irq_gc_unlock(gc); + irq_reg_writel(gc, bank->saved_masks, GPIO_INTMASK); } static int rockchip_interrupts_register(struct platform_device *pdev, @@ -1620,6 +1592,14 @@ static int rockchip_interrupts_register(struct platform_device *pdev, continue; } + /* + * Linux assumes that all interrupts start out disabled/masked. + * Our driver only uses the concept of masked and always keeps + * things enabled, so for us that's all masked and all enabled. + */ + writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTMASK); + writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTEN); + gc = irq_get_domain_generic_chip(bank->domain, 0); gc->reg_base = bank->reg_base; gc->private = bank; @@ -1628,8 +1608,6 @@ static int rockchip_interrupts_register(struct platform_device *pdev, gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit; gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit; gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit; - gc->chip_types[0].chip.irq_enable = rockchip_irq_enable; - gc->chip_types[0].chip.irq_disable = rockchip_irq_disable; gc->chip_types[0].chip.irq_set_wake = irq_gc_set_wake; gc->chip_types[0].chip.irq_suspend = rockchip_irq_suspend; gc->chip_types[0].chip.irq_resume = rockchip_irq_resume;