LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Fletcher Woodruff <fletcherw@chromium.org>
To: linux-kernel@vger.kernel.org
Cc: Ben Zhang <benzh@chromium.org>, Jaroslav Kysela <perex@perex.cz>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	Oder Chiou <oder_chiou@realtek.com>,
	Takashi Iwai <tiwai@suse.com>,
	Curtis Malainey <cujomalainey@chromium.org>,
	Ross Zwisler <zwisler@chromium.org>,
	alsa-devel@alsa-project.org,
	Fletcher Woodruff <fletcherw@chromium.org>
Subject: [PATCH v6 4/4] ASoC: rt5677: handle concurrent interrupts
Date: Wed,  5 Jun 2019 16:24:19 -0600	[thread overview]
Message-ID: <20190605222419.54479-5-fletcherw@chromium.org> (raw)
In-Reply-To: <20190605222419.54479-1-fletcherw@chromium.org>

From: Ben Zhang <benzh@chromium.org>

The rt5677 driver writes to the IRQ control register within the IRQ
handler in order to flip the polarity of the interrupts that have been
signalled.  If an interrupt fires in the interval between the
regmap_read and the regmap_write, it will not trigger a new call to
rt5677_irq.

Add a bounded loop to rt5677_irq that keeps checking interrupts until
none are seen, so that any interrupts that are signalled in that
interval are correctly handled.

Signed-off-by: Ben Zhang <benzh@chromium.org>
Signed-off-by: Fletcher Woodruff <fletcherw@chromium.org>
---
 sound/soc/codecs/rt5677.c | 67 ++++++++++++++++++++++++---------------
 1 file changed, 42 insertions(+), 25 deletions(-)

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index 86555d7ec9ea8d..7f7e60aceb49d3 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -5071,38 +5071,55 @@ static const struct rt5677_irq_desc rt5677_irq_descs[] = {
 static irqreturn_t rt5677_irq(int unused, void *data)
 {
 	struct rt5677_priv *rt5677 = data;
-	int ret = 0, i, reg_irq, virq;
+	int ret = 0, loop, i, reg_irq, virq;
 	bool irq_fired = false;
 
 	mutex_lock(&rt5677->irq_lock);
-	/* Read interrupt status */
-	ret = regmap_read(rt5677->regmap, RT5677_IRQ_CTRL1, &reg_irq);
-	if (ret) {
-		pr_err("rt5677: failed reading IRQ status: %d\n", ret);
-		goto exit;
-	}
 
-	for (i = 0; i < RT5677_IRQ_NUM; i++) {
-		if (reg_irq & rt5677_irq_descs[i].status_mask) {
-			irq_fired = true;
-			virq = irq_find_mapping(rt5677->domain, i);
-			if (virq)
-				handle_nested_irq(virq);
-
-			/* Clear the interrupt by flipping the polarity of the
-			 * interrupt source line that fired
-			 */
-			reg_irq ^= rt5677_irq_descs[i].polarity_mask;
+	/*
+	 * Loop to handle interrupts until the last i2c read shows no pending
+	 * irqs. The interrupt line is shared by multiple interrupt sources.
+	 * After the regmap_read() below, a new interrupt source line may
+	 * become high before the regmap_write() finishes, so there isn't a
+	 * rising edge on the shared interrupt line for the new interrupt. Thus,
+	 * the loop is needed to avoid missing irqs.
+	 *
+	 * A safeguard of 20 loops is used to avoid hanging in the irq handler
+	 * if there is something wrong with the interrupt status update. The
+	 * interrupt sources here are audio jack plug/unplug events which
+	 * shouldn't happen at a high frequency for a long period of time.
+	 * Empirically, more than 3 loops have never been seen.
+	 */
+	for (loop = 0; loop < 20; loop++) {
+		/* Read interrupt status */
+		ret = regmap_read(rt5677->regmap, RT5677_IRQ_CTRL1, &reg_irq);
+		if (ret) {
+			pr_err("rt5677: failed reading IRQ status: %d\n", ret);
+			goto exit;
 		}
-	}
 
-	if (!irq_fired)
-		goto exit;
+		irq_fired = false;
+		for (i = 0; i < RT5677_IRQ_NUM; i++) {
+			if (reg_irq & rt5677_irq_descs[i].status_mask) {
+				irq_fired = true;
+				virq = irq_find_mapping(rt5677->domain, i);
+				if (virq)
+					handle_nested_irq(virq);
+
+				/* Clear the interrupt by flipping the polarity
+				 * of the interrupt source line that fired
+				 */
+				reg_irq ^= rt5677_irq_descs[i].polarity_mask;
+			}
+		}
+		if (!irq_fired)
+			goto exit;
 
-	ret = regmap_write(rt5677->regmap, RT5677_IRQ_CTRL1, reg_irq);
-	if (ret) {
-		pr_err("rt5677: failed updating IRQ status: %d\n", ret);
-		goto exit;
+		ret = regmap_write(rt5677->regmap, RT5677_IRQ_CTRL1, reg_irq);
+		if (ret) {
+			pr_err("rt5677: failed updating IRQ status: %d\n", ret);
+			goto exit;
+		}
 	}
 exit:
 	mutex_unlock(&rt5677->irq_lock);
-- 
2.22.0.rc1.311.g5d7573a151-goog


  parent reply	other threads:[~2019-06-05 22:25 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-07 22:01 [PATCH v5 0/3] Fix jack detection for Chromebook Pixel Fletcher Woodruff
2019-05-07 22:01 ` [PATCH v5 1/3] ASoC: rt5677: allow multiple interrupt sources Fletcher Woodruff
2019-05-08  7:36   ` Mark Brown
2019-05-08 21:39     ` Curtis Malainey
2019-05-09  2:32       ` Mark Brown
2019-05-09 21:25         ` Curtis Malainey
2019-05-07 22:01 ` [PATCH v5 2/3] ASoC: rt5677: handle concurrent interrupts Fletcher Woodruff
2019-05-07 22:01 ` [PATCH v5 3/3] ASoC: rt5677: fall back to DT prop names on error Fletcher Woodruff
2019-05-08  9:04   ` Mark Brown
2019-06-05 22:24 ` [PATCH v6 0/4] Fix jack detection for Chromebook Pixel Fletcher Woodruff
2019-06-05 22:24   ` [PATCH v6 1/4] ASoC: rt5677: fall back to DT prop names on error Fletcher Woodruff
2019-06-05 22:24   ` [PATCH v6 2/4] ASoC: rt5677: move jack-detect init to i2c probe Fletcher Woodruff
2019-06-05 22:24   ` [PATCH v6 3/4] ASoC: rt5677: clear interrupts by polarity flip Fletcher Woodruff
2019-06-05 22:24   ` Fletcher Woodruff [this message]
2019-06-14 18:43 [PATCH v6 0/4] Fix jack detection for Chromebook Pixel Fletcher Woodruff
2019-06-14 18:43 ` [PATCH v6 4/4] ASoC: rt5677: handle concurrent interrupts Fletcher Woodruff

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=20190605222419.54479-5-fletcherw@chromium.org \
    --to=fletcherw@chromium.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=benzh@chromium.org \
    --cc=broonie@kernel.org \
    --cc=cujomalainey@chromium.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oder_chiou@realtek.com \
    --cc=perex@perex.cz \
    --cc=tiwai@suse.com \
    --cc=zwisler@chromium.org \
    --subject='Re: [PATCH v6 4/4] ASoC: rt5677: handle concurrent interrupts' \
    /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

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).