[alsa-devel] [PATCH] ASoC: wm8903: disable mic detect irqs until jack registered
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);
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.
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.
participants (2)
-
Mark Brown -
Stephen Warren