linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "irqchip-bot for Douglas Anderson" <tip-bot2@linutronix.de>
To: linux-kernel@vger.kernel.org
Cc: Douglas Anderson <dianders@chromium.org>,
	Marc Zyngier <maz@kernel.org>,
	Maulik Shah <mkshah@codeaurora.org>,
	Stephen Boyd <swboyd@chromium.org>,
	tglx@linutronix.de
Subject: [irqchip: irq/irqchip-next] irqchip/qcom-pdc: Fix phantom irq when changing between rising/falling
Date: Sat, 12 Dec 2020 10:52:44 -0000	[thread overview]
Message-ID: <160777036408.3364.8611167035514304941.tip-bot2@tip-bot2> (raw)
In-Reply-To: <20201211141514.v4.1.I2702919afc253e2a451bebc3b701b462b2d22344@changeid>

The following commit has been merged into the irq/irqchip-next branch of irqchip:

Commit-ID:     2f5fbc4305d07725bfebaedb09e57271315691ef
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/2f5fbc4305d07725bfebaedb09e57271315691ef
Author:        Douglas Anderson <dianders@chromium.org>
AuthorDate:    Fri, 11 Dec 2020 14:15:35 -08:00
Committer:     Marc Zyngier <maz@kernel.org>
CommitterDate: Sat, 12 Dec 2020 10:46:02 

irqchip/qcom-pdc: Fix phantom irq when changing between rising/falling

We have a problem if we use gpio-keys and configure wakeups such that
we only want one edge to wake us up.  AKA:
  wakeup-event-action = <EV_ACT_DEASSERTED>;
  wakeup-source;

Specifically we end up with a phantom interrupt that blocks suspend if
the line was already high and we want wakeups on rising edges (AKA we
want the GPIO to go low and then high again before we wake up).  The
opposite is also problematic.

Specifically, here's what's happening today:
1. Normally, gpio-keys configures to look for both edges.  Due to the
   current workaround introduced in commit c3c0c2e18d94 ("pinctrl:
   qcom: Handle broken/missing PDC dual edge IRQs on sc7180"), if the
   line was high we'd configure for falling edges.
2. At suspend time, we change to look for rising edges.
3. After qcom_pdc_gic_set_type() runs, we get a phantom interrupt.

We can solve this by just clearing the phantom interrupt.

NOTE: it is possible that this could cause problems for a client with
very specific needs, but there's not much we can do with this
hardware.  As an example, let's say the interrupt signal is currently
high and the client is looking for falling edges.  The client now
changes to look for rising edges.  The client could possibly expect
that if the line has a short pulse low (and back high) that it would
always be detected.  Specifically no matter when the pulse happened,
it should either have tripped the (old) falling edge trigger or the
(new) rising edge trigger.  We will simply not trip it.  We could
narrow down the race a bit by polling our parent before changing
types, but no matter what we do there will still be a period of time
where we can't tell the difference between a real transition (or more
than one transition) and the phantom.

Fixes: f55c73aef890 ("irqchip/pdc: Add PDC interrupt controller for QCOM SoCs")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Tested-by: Maulik Shah <mkshah@codeaurora.org>
Reviewed-by: Maulik Shah <mkshah@codeaurora.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Link: https://lore.kernel.org/r/20201211141514.v4.1.I2702919afc253e2a451bebc3b701b462b2d22344@changeid
---
 drivers/irqchip/qcom-pdc.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index bd39e9d..5dc63c2 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -159,6 +159,8 @@ static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type)
 {
 	int pin_out = d->hwirq;
 	enum pdc_irq_config_bits pdc_type;
+	enum pdc_irq_config_bits old_pdc_type;
+	int ret;
 
 	if (pin_out == GPIO_NO_WAKE_IRQ)
 		return 0;
@@ -187,9 +189,26 @@ static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type)
 		return -EINVAL;
 	}
 
+	old_pdc_type = pdc_reg_read(IRQ_i_CFG, pin_out);
 	pdc_reg_write(IRQ_i_CFG, pin_out, pdc_type);
 
-	return irq_chip_set_type_parent(d, type);
+	ret = irq_chip_set_type_parent(d, type);
+	if (ret)
+		return ret;
+
+	/*
+	 * When we change types the PDC can give a phantom interrupt.
+	 * Clear it.  Specifically the phantom shows up when reconfiguring
+	 * polarity of interrupt without changing the state of the signal
+	 * but let's be consistent and clear it always.
+	 *
+	 * Doing this works because we have IRQCHIP_SET_TYPE_MASKED so the
+	 * interrupt will be cleared before the rest of the system sees it.
+	 */
+	if (old_pdc_type != pdc_type)
+		irq_chip_set_parent_state(d, IRQCHIP_STATE_PENDING, false);
+
+	return 0;
 }
 
 static struct irq_chip qcom_pdc_gic_chip = {

      parent reply	other threads:[~2020-12-12 10:53 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-11 22:15 [PATCH v4 1/4] irqchip: qcom-pdc: Fix phantom irq when changing between rising/falling Douglas Anderson
2020-12-11 22:15 ` [PATCH v4 2/4] pinctrl: qcom: Allow SoCs to specify a GPIO function that's not 0 Douglas Anderson
2020-12-11 22:15 ` [PATCH v4 3/4] pinctrl: qcom: Don't clear pending interrupts when enabling Douglas Anderson
2020-12-17  4:54   ` Stephen Boyd
2020-12-18  5:35   ` Rajendra Nayak
2020-12-21 16:00   ` Maulik Shah
2021-01-08 17:37     ` Doug Anderson
2020-12-11 22:15 ` [PATCH v4 4/4] pinctrl: qcom: Clear possible pending parent irq when remuxing GPIOs Douglas Anderson
2020-12-12 10:48 ` (subset) [PATCH v4 1/4] irqchip: qcom-pdc: Fix phantom irq when changing between rising/falling Marc Zyngier
2020-12-12 10:52 ` irqchip-bot for Douglas Anderson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=160777036408.3364.8611167035514304941.tip-bot2@tip-bot2 \
    --to=tip-bot2@linutronix.de \
    --cc=dianders@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=mkshah@codeaurora.org \
    --cc=swboyd@chromium.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).