Unverified Commit a698679f authored by Cristian Ciocaltea's avatar Cristian Ciocaltea Committed by Mark Brown
Browse files

ASoC: nau8821: Consistently clear interrupts before unmasking



The interrupt handler attempts to perform some IRQ status clear
operations *after* rather than *before* unmasking and enabling
interrupts.  This is a rather fragile approach since it may generally
lead to missing IRQ requests or causing spurious interrupts.

Make use of the nau8821_irq_status_clear() helper instead of
manipulating the related register directly and ensure any interrupt
clearing is performed *after* the target interrupts are disabled/masked
and *before* proceeding with additional interrupt unmasking/enablement
operations.

This also implicitly drops the redundant clear operation of the ejection
IRQ in the interrupt handler, since nau8821_eject_jack() has been
already responsible for clearing all active interrupts.

Fixes: aab1ad11 ("ASoC: nau8821: new driver")
Fixes: 2551b6e8 ("ASoC: nau8821: Add headset button detection")
Signed-off-by: default avatarCristian Ciocaltea <cristian.ciocaltea@collabora.com>
Link: https://patch.msgid.link/20251003-nau8821-jdet-fixes-v1-3-f7b0e2543f09@collabora.com


Signed-off-by: default avatarMark Brown <broonie@kernel.org>
parent 9273aa85
Loading
Loading
Loading
Loading
+30 −28
Original line number Diff line number Diff line
@@ -1057,20 +1057,24 @@ static void nau8821_eject_jack(struct nau8821 *nau8821)
	snd_soc_component_disable_pin(component, "MICBIAS");
	snd_soc_dapm_sync(dapm);

	/* Disable & mask both insertion & ejection IRQs */
	regmap_update_bits(regmap, NAU8821_R12_INTERRUPT_DIS_CTRL,
			   NAU8821_IRQ_INSERT_DIS | NAU8821_IRQ_EJECT_DIS,
			   NAU8821_IRQ_INSERT_DIS | NAU8821_IRQ_EJECT_DIS);
	regmap_update_bits(regmap, NAU8821_R0F_INTERRUPT_MASK,
			   NAU8821_IRQ_INSERT_EN | NAU8821_IRQ_EJECT_EN,
			   NAU8821_IRQ_INSERT_EN | NAU8821_IRQ_EJECT_EN);

	/* Clear all interruption status */
	nau8821_irq_status_clear(regmap, 0);

	/* Enable the insertion interruption, disable the ejection inter-
	 * ruption, and then bypass de-bounce circuit.
	 */
	/* Enable & unmask the insertion IRQ */
	regmap_update_bits(regmap, NAU8821_R12_INTERRUPT_DIS_CTRL,
		NAU8821_IRQ_EJECT_DIS | NAU8821_IRQ_INSERT_DIS,
		NAU8821_IRQ_EJECT_DIS);
	/* Mask unneeded IRQs: 1 - disable, 0 - enable */
			   NAU8821_IRQ_INSERT_DIS, 0);
	regmap_update_bits(regmap, NAU8821_R0F_INTERRUPT_MASK,
		NAU8821_IRQ_EJECT_EN | NAU8821_IRQ_INSERT_EN,
		NAU8821_IRQ_EJECT_EN);
			   NAU8821_IRQ_INSERT_EN, 0);

	/* Bypass de-bounce circuit */
	regmap_update_bits(regmap, NAU8821_R0D_JACK_DET_CTRL,
		NAU8821_JACK_DET_DB_BYPASS, NAU8821_JACK_DET_DB_BYPASS);

@@ -1094,7 +1098,6 @@ static void nau8821_eject_jack(struct nau8821 *nau8821)
			NAU8821_IRQ_KEY_RELEASE_DIS |
			NAU8821_IRQ_KEY_PRESS_DIS);
	}

}

static void nau8821_jdet_work(struct work_struct *work)
@@ -1151,6 +1154,15 @@ static void nau8821_setup_inserted_irq(struct nau8821 *nau8821)
{
	struct regmap *regmap = nau8821->regmap;

	/* Disable & mask insertion IRQ */
	regmap_update_bits(regmap, NAU8821_R12_INTERRUPT_DIS_CTRL,
			   NAU8821_IRQ_INSERT_DIS, NAU8821_IRQ_INSERT_DIS);
	regmap_update_bits(regmap, NAU8821_R0F_INTERRUPT_MASK,
			   NAU8821_IRQ_INSERT_EN, NAU8821_IRQ_INSERT_EN);

	/* Clear insert IRQ status */
	nau8821_irq_status_clear(regmap, NAU8821_JACK_INSERT_DETECTED);

	/* Enable internal VCO needed for interruptions */
	if (nau8821->dapm->bias_level < SND_SOC_BIAS_PREPARE)
		nau8821_configure_sysclk(nau8821, NAU8821_CLK_INTERNAL, 0);
@@ -1169,6 +1181,7 @@ static void nau8821_setup_inserted_irq(struct nau8821 *nau8821)
	regmap_update_bits(regmap, NAU8821_R0D_JACK_DET_CTRL,
		NAU8821_JACK_DET_DB_BYPASS, 0);

	/* Unmask & enable the ejection IRQs */
	regmap_update_bits(regmap, NAU8821_R0F_INTERRUPT_MASK,
			   NAU8821_IRQ_EJECT_EN, 0);
	regmap_update_bits(regmap, NAU8821_R12_INTERRUPT_DIS_CTRL,
@@ -1179,7 +1192,7 @@ static irqreturn_t nau8821_interrupt(int irq, void *data)
{
	struct nau8821 *nau8821 = (struct nau8821 *)data;
	struct regmap *regmap = nau8821->regmap;
	int active_irq, clear_irq = 0, event = 0, event_mask = 0;
	int active_irq, event = 0, event_mask = 0;

	if (regmap_read(regmap, NAU8821_R10_IRQ_STATUS, &active_irq)) {
		dev_err(nau8821->dev, "failed to read irq status\n");
@@ -1195,14 +1208,13 @@ static irqreturn_t nau8821_interrupt(int irq, void *data)
			NAU8821_MICDET_MASK, NAU8821_MICDET_DIS);
		nau8821_eject_jack(nau8821);
		event_mask |= SND_JACK_HEADSET;
		clear_irq = NAU8821_JACK_EJECT_IRQ_MASK;
	} else if (active_irq & NAU8821_KEY_SHORT_PRESS_IRQ) {
		event |= NAU8821_BUTTON;
		event_mask |= NAU8821_BUTTON;
		clear_irq = NAU8821_KEY_SHORT_PRESS_IRQ;
		nau8821_irq_status_clear(regmap, NAU8821_KEY_SHORT_PRESS_IRQ);
	} else if (active_irq & NAU8821_KEY_RELEASE_IRQ) {
		event_mask = NAU8821_BUTTON;
		clear_irq = NAU8821_KEY_RELEASE_IRQ;
		nau8821_irq_status_clear(regmap, NAU8821_KEY_RELEASE_IRQ);
	} else if ((active_irq & NAU8821_JACK_INSERT_IRQ_MASK) ==
		NAU8821_JACK_INSERT_DETECTED) {
		cancel_work_sync(&nau8821->jdet_work);
@@ -1212,27 +1224,17 @@ static irqreturn_t nau8821_interrupt(int irq, void *data)
			/* detect microphone and jack type */
			schedule_work(&nau8821->jdet_work);
			/* Turn off insertion interruption at manual mode */
			regmap_update_bits(regmap,
				NAU8821_R12_INTERRUPT_DIS_CTRL,
				NAU8821_IRQ_INSERT_DIS,
				NAU8821_IRQ_INSERT_DIS);
			regmap_update_bits(regmap,
				NAU8821_R0F_INTERRUPT_MASK,
				NAU8821_IRQ_INSERT_EN,
				NAU8821_IRQ_INSERT_EN);
			nau8821_setup_inserted_irq(nau8821);
		} else {
			dev_warn(nau8821->dev,
				"Inserted IRQ fired but not connected\n");
			nau8821_eject_jack(nau8821);
		}
	} else {
		/* Clear the rightmost interrupt */
		nau8821_irq_status_clear(regmap, active_irq);
	}

	if (!clear_irq)
		clear_irq = active_irq;
	/* clears the rightmost interruption */
	regmap_write(regmap, NAU8821_R11_INT_CLR_KEY_STATUS, clear_irq);

	if (event_mask)
		snd_soc_jack_report(nau8821->jack, event, event_mask);