[alsa-devel] [PATCH] ASoC: rt5645: Move irq request to rt5645_probe
rt5645_jack_detect_work (the workqueue called by rt5645_irq), needs to access rt5645->codec. However, this field is only initialized in rt5645_probe. This can obviously lead to kernel panics.
Fix that by moving the irq request from rt5645_i2c_probe to rt5645_probe. Also, fail if the IRQ cannot be requested, and move irq_free from rt5645_i2c_remove to rt5645_remove.
Signed-off-by: Nicolas Boichat drinkcat@chromium.org --- sound/soc/codecs/rt5645.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/sound/soc/codecs/rt5645.c b/sound/soc/codecs/rt5645.c index 9ce311e..7f84f21 100644 --- a/sound/soc/codecs/rt5645.c +++ b/sound/soc/codecs/rt5645.c @@ -3044,6 +3044,7 @@ static int rt5645_irq_detection(struct rt5645_priv *rt5645) static int rt5645_probe(struct snd_soc_codec *codec) { struct rt5645_priv *rt5645 = snd_soc_codec_get_drvdata(codec); + int ret;
rt5645->codec = codec;
@@ -3072,12 +3073,32 @@ static int rt5645_probe(struct snd_soc_codec *codec) snd_soc_dapm_sync(&codec->dapm); }
+ INIT_DELAYED_WORK(&rt5645->jack_detect_work, rt5645_jack_detect_work); + + if (rt5645->i2c->irq) { + ret = request_threaded_irq(rt5645->i2c->irq, NULL, rt5645_irq, + IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING + | IRQF_ONESHOT, "rt5645", rt5645); + if (ret) { + dev_err(codec->dev, "Failed to request IRQ: %d\n", ret); + return ret; + } + } + return 0; }
static int rt5645_remove(struct snd_soc_codec *codec) { + struct rt5645_priv *rt5645 = snd_soc_codec_get_drvdata(codec); + + if (rt5645->i2c->irq) + free_irq(rt5645->i2c->irq, rt5645); + + cancel_delayed_work_sync(&rt5645->jack_detect_work); + rt5645_reset(codec); + return 0; }
@@ -3428,29 +3449,12 @@ static int rt5645_i2c_probe(struct i2c_client *i2c, } }
- INIT_DELAYED_WORK(&rt5645->jack_detect_work, rt5645_jack_detect_work); - - if (rt5645->i2c->irq) { - ret = request_threaded_irq(rt5645->i2c->irq, NULL, rt5645_irq, - IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING - | IRQF_ONESHOT, "rt5645", rt5645); - if (ret) - dev_err(&i2c->dev, "Failed to reguest IRQ: %d\n", ret); - } - return snd_soc_register_codec(&i2c->dev, &soc_codec_dev_rt5645, rt5645_dai, ARRAY_SIZE(rt5645_dai)); }
static int rt5645_i2c_remove(struct i2c_client *i2c) { - struct rt5645_priv *rt5645 = i2c_get_clientdata(i2c); - - if (i2c->irq) - free_irq(i2c->irq, rt5645); - - cancel_delayed_work_sync(&rt5645->jack_detect_work); - snd_soc_unregister_codec(&i2c->dev);
return 0;
On Tue, Jun 30, 2015 at 03:30:41PM +0800, Nicolas Boichat wrote:
rt5645_jack_detect_work (the workqueue called by rt5645_irq), needs to access rt5645->codec. However, this field is only initialized in rt5645_probe. This can obviously lead to kernel panics.
Fix that by moving the irq request from rt5645_i2c_probe to rt5645_probe. Also, fail if the IRQ cannot be requested, and move irq_free from rt5645_i2c_remove to rt5645_remove.
No, this is broken - we want to be requesting resources in the device level probe since it integrates better with things like deferred probing. You need to fix the interrupt handler to cope without the CODEC.
participants (2)
-
Mark Brown
-
Nicolas Boichat