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: aab1ad11d69f ("ASoC: nau8821: new driver")
Fixes: 2551b6e89936 ("ASoC: nau8821: Add headset button detection")
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
Link: https://patch.msgid.link/20251003-nau8821-jdet-fixes-v1-3-f7b0e2543f09@collabora.com
Signed-off-by: Mark Brown <broonie@kernel.org>

diff --git a/sound/soc/codecs/nau8821.c b/sound/soc/codecs/nau8821.c
index cefce97893123462198c2e66d1de9d06e4d3dd28..56e5a0d80782b85d8c256745911297f8edf5dd98 100644 (file)
--- a/sound/soc/codecs/nau8821.c
+++ b/sound/soc/codecs/nau8821.c
@@ -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,17 +1181,18 @@ 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);
+                          NAU8821_IRQ_EJECT_EN, 0);
        regmap_update_bits(regmap, NAU8821_R12_INTERRUPT_DIS_CTRL,
-               NAU8821_IRQ_EJECT_DIS, 0);
+                          NAU8821_IRQ_EJECT_DIS, 0);
 }
 
 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);