[PATCH] ASoC: wm8903: disable mic detect irqs until jack registered

[PATCH] ASoC: wm8903: disable mic detect irqs until jack registered

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

If either mic detection interrupt is unmasked when the IRQ handler is
first registered, they might fire immediately. However, at this point,
wm8903->mic_jack cannot have been set, since wm8903_mic_detect() can
only operate after wm8903_probe() has completed. If the IRQ fires at
this point, wm8903_irq() will use the uninitialized or stale mic_jack,
and crash.

This problem can be triggered by fully initializing an audio card, then
removing and re-inserting the machine driver module. This would leave
mic detection enabled in HW, and mic_jack set to a stale value.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
This is useful in 3.5.

As an aside, I wondered instead about modifying wm8903_remove() to disable
mic detection, and clear any status. However, even if we do that, I think
we still want to make this change too, since I think we want probe() to
work the first time without relying on previous HW state?

 sound/soc/codecs/wm8903.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/sound/soc/codecs/wm8903.c b/sound/soc/codecs/wm8903.c
index 304b5cf..d00a2b3 100644
--- a/sound/soc/codecs/wm8903.c
+++ b/sound/soc/codecs/wm8903.c
@@ -1946,7 +1946,16 @@ static int wm8903_probe(struct snd_soc_codec *codec)
 
 		snd_soc_update_bits(codec, WM8903_INTERRUPT_CONTROL,
 				    WM8903_IRQ_POL, irq_pol);
-		
+
+		/*
+		 * Ensure mic detection interrupts are masked until after
+		 * wm8903_mic_detect() is called, so that wm8903_irq() has
+		 * a valid wm8903->mic_jack() to report on.
+		 */
+		snd_soc_update_bits(codec, WM8903_INTERRUPT_STATUS_1_MASK,
+				    WM8903_MICDET_EINT | WM8903_MICSHRT_EINT,
+				    WM8903_MICDET_EINT | WM8903_MICSHRT_EINT);
+
 		ret = request_threaded_irq(wm8903->irq, NULL, wm8903_irq,
 					   trigger | IRQF_ONESHOT,
 					   "wm8903", codec);
-- 
1.7.0.4

Re: [PATCH] ASoC: wm8903: disable mic detect irqs until jack registered

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1551 bytes --]

On Fri, Jun 08, 2012 at 11:37:54AM -0600, Stephen Warren wrote:

> This problem can be triggered by fully initializing an audio card, then
> removing and re-inserting the machine driver module. This would leave
> mic detection enabled in HW, and mic_jack set to a stale value.

This isn't a good fix for this issue, - the fact that it works is more a
sign that nobody got round to moving the interrupt registration to the
I2C probe() function than anything else.

> As an aside, I wondered instead about modifying wm8903_remove() to disable
> mic detection, and clear any status. However, even if we do that, I think
> we still want to make this change too, since I think we want probe() to
> work the first time without relying on previous HW state?

It does currently work without relying on previous hardware state, the
first thing that we do with the chip as soon as we get control of it is
a hard reset.

In terms of the mic detection itself it's really the responsibility of
the machine driver to say that the jack doesn't exist any more when it's
being removed - it should be calling wm8903_mic_detect() again during
unregistration to disable the interrupt.  Otherwise there's always going
to be an interval where the jack object has been discarded but the CODEC
driver is still active and could get an interrupt.  That fix should be
enough for 3.5.

Normally nobody cares about this because for most systems there's no
realistic use case for removing a driver like this, it's something
you're only going to run into during driver development.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: wm8903: disable mic detect irqs until jack registered

[Stephen Warren]

On 06/08/2012 07:31 PM, Mark Brown wrote:
> On Fri, Jun 08, 2012 at 11:37:54AM -0600, Stephen Warren wrote:
> 
>> This problem can be triggered by fully initializing an audio
>> card, then removing and re-inserting the machine driver module.
>> This would leave mic detection enabled in HW, and mic_jack set to
>> a stale value.
> 
> This isn't a good fix for this issue, - the fact that it works is
> more a sign that nobody got round to moving the interrupt
> registration to the I2C probe() function than anything else.
...
> In terms of the mic detection itself it's really the responsibility
> of the machine driver to say that the jack doesn't exist any more
> when it's being removed - it should be calling wm8903_mic_detect()
> again during unregistration to disable the interrupt. ...

OK, that makes sense. I guess I can do that in the card's .remove()
function. I'd shied away from doing that before since the jacks are
created in the DAI link's init() function, and there wasn't a
symmetrical DAI link remove() function to match that, but the card
remove() should be fine.