[alsa-devel] [PATCH 0/5] ASoC: twl6040: Updates for 3.9
Hi Mark, Liam,
Small fixes from Misael for twl6040 codec driver and devm_* conversion, switch to system workqueue and dead code removal.
Regards, Peter --- Misael Lopez Cruz (2): ASoC: twl6040: Convert PLUGINT to no-suspend irq ASoC: twl6040: Prevent extra power transitions during resume
Peter Ujfalusi (3): ASoC: twl6040: Convert to use devm_* when possible ASoC: twl6040: Switch to use system workqueue for jack reporting ASoC: twl6040: Remove leftover code from hs/hf ramp implementation
sound/soc/codecs/twl6040.c | 67 +++++++++------------------------------------- 1 file changed, 13 insertions(+), 54 deletions(-)
From: Misael Lopez Cruz misael.lopez@ti.com
Convert headset PLUGINT interrupt to NO_SUSPEND type in order to allow handling of insertion/removal events while device is suspended.
Signed-off-by: Misael Lopez Cruz misael.lopez@ti.com Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/codecs/twl6040.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/twl6040.c b/sound/soc/codecs/twl6040.c index 3fc3fc6..ef31ace 100644 --- a/sound/soc/codecs/twl6040.c +++ b/sound/soc/codecs/twl6040.c @@ -1174,7 +1174,7 @@ static int twl6040_probe(struct snd_soc_codec *codec) mutex_init(&priv->mutex);
ret = request_threaded_irq(priv->plug_irq, NULL, twl6040_audio_handler, - 0, "twl6040_irq_plug", codec); + IRQF_NO_SUSPEND, "twl6040_irq_plug", codec); if (ret) { dev_err(codec->dev, "PLUG IRQ request failed: %d\n", ret); goto plugirq_err;
From: Misael Lopez Cruz misael.lopez@ti.com
Prevent unnecessary power state transitions that might occur while CODEC is resuming if already in target state.
Signed-off-by: Misael Lopez Cruz misael.lopez@ti.com Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/codecs/twl6040.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/twl6040.c b/sound/soc/codecs/twl6040.c index ef31ace..b40830c 100644 --- a/sound/soc/codecs/twl6040.c +++ b/sound/soc/codecs/twl6040.c @@ -1114,8 +1114,10 @@ static int twl6040_suspend(struct snd_soc_codec *codec)
static int twl6040_resume(struct snd_soc_codec *codec) { - twl6040_set_bias_level(codec, SND_SOC_BIAS_STANDBY); - twl6040_set_bias_level(codec, codec->dapm.suspend_bias_level); + if (codec->dapm.bias_level != codec->dapm.suspend_bias_level) { + twl6040_set_bias_level(codec, SND_SOC_BIAS_STANDBY); + twl6040_set_bias_level(codec, codec->dapm.suspend_bias_level); + }
return 0; }
On Fri, Jan 11, 2013 at 11:32:32AM +0100, Peter Ujfalusi wrote:
- twl6040_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
- twl6040_set_bias_level(codec, codec->dapm.suspend_bias_level);
- if (codec->dapm.bias_level != codec->dapm.suspend_bias_level) {
twl6040_set_bias_level(codec, SND_SOC_BIAS_STANDBY);twl6040_set_bias_level(codec, codec->dapm.suspend_bias_level);- }
The device should always be in either _STANDBY or _OFF (depending on if it supports idle_bias_off) before it tries to suspend so suspend_bias_level ought to be redundant. We should really get round to killing it...
On 01/11/2013 12:52 PM, Mark Brown wrote:
On Fri, Jan 11, 2013 at 11:32:32AM +0100, Peter Ujfalusi wrote:
- twl6040_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
- twl6040_set_bias_level(codec, codec->dapm.suspend_bias_level);
- if (codec->dapm.bias_level != codec->dapm.suspend_bias_level) {
twl6040_set_bias_level(codec, SND_SOC_BIAS_STANDBY);twl6040_set_bias_level(codec, codec->dapm.suspend_bias_level);- }
The device should always be in either _STANDBY or _OFF (depending on if it supports idle_bias_off) before it tries to suspend so suspend_bias_level ought to be redundant. We should really get round to killing it...
Yes, that's true. At the moment I can just send the codec to _STANDBY in resume since I don't have idle_bias_off support ready (due to external dependencies, but I'm working on it). But when we have idle_bias_off support we want to avoid sending the codec to _STANDBY here just that ASoC core will put us back to _OFF later on. On/Off is a bit expensive on twl6040. Since the core saves the previous bias_level in before suspend I can just do this here:
twl6040_set_bias_level(codec, codec->dapm.suspend_bias_level);
If we were in _OFF before it is going to do nothing, but if we were in _STANDBY it brings the codec back.
On Fri, Jan 11, 2013 at 02:40:20PM +0100, Peter Ujfalusi wrote:
Since the core saves the previous bias_level in before suspend I can just do this here:
twl6040_set_bias_level(codec, codec->dapm.suspend_bias_level);
If we were in _OFF before it is going to do nothing, but if we were in _STANDBY it brings the codec back.
That'll work, yes (though removing the function when idle_bias_off is implemented would obviously be better).
On 01/11/2013 02:46 PM, Mark Brown wrote:
On Fri, Jan 11, 2013 at 02:40:20PM +0100, Peter Ujfalusi wrote:
Since the core saves the previous bias_level in before suspend I can just do this here:
twl6040_set_bias_level(codec, codec->dapm.suspend_bias_level);
If we were in _OFF before it is going to do nothing, but if we were in _STANDBY it brings the codec back.
That'll work, yes (though removing the function when idle_bias_off is implemented would obviously be better).
True. Along with the suspend function since I'm not doing anything fancy there and if idle_bias_off is set the core will not call suspend of the driver if it is not already in _OFF. I'll keep this in mind for the future and resend with only with
twl6040_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
in the resume. But I wait if you have other comments for the rest for the series.
On Fri, Jan 11, 2013 at 02:55:53PM +0100, Peter Ujfalusi wrote:
But I wait if you have other comments for the rest for the series.
The rest of it was fine, I was going to apply them next time I go through and applied things.
On 01/11/2013 03:24 PM, Mark Brown wrote:
On Fri, Jan 11, 2013 at 02:55:53PM +0100, Peter Ujfalusi wrote:
But I wait if you have other comments for the rest for the series.
The rest of it was fine, I was going to apply them next time I go through and applied things.
If it's OK I'll resend the series and replace this patch with a different one to remove just the second twl6040_set_bias_level(codec, codec->dapm.suspend_bias_level); call from resume.
On Fri, Jan 11, 2013 at 03:44:07PM +0100, Peter Ujfalusi wrote:
If it's OK I'll resend the series and replace this patch with a different one to remove just the second twl6040_set_bias_level(codec, codec->dapm.suspend_bias_level); call from resume.
Sure, no problem.
In this way we can clean up the probe and remove paths
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/codecs/twl6040.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/sound/soc/codecs/twl6040.c b/sound/soc/codecs/twl6040.c index b40830c..29fa457 100644 --- a/sound/soc/codecs/twl6040.c +++ b/sound/soc/codecs/twl6040.c @@ -1134,9 +1134,10 @@ static int twl6040_probe(struct snd_soc_codec *codec) struct platform_device, dev); int ret = 0;
- priv = kzalloc(sizeof(struct twl6040_data), GFP_KERNEL); + priv = devm_kzalloc(codec->dev, sizeof(*priv), GFP_KERNEL); if (priv == NULL) return -ENOMEM; + snd_soc_codec_set_drvdata(codec, priv);
priv->codec = codec; @@ -1161,25 +1162,23 @@ static int twl6040_probe(struct snd_soc_codec *codec) priv->plug_irq = platform_get_irq(pdev, 0); if (priv->plug_irq < 0) { dev_err(codec->dev, "invalid irq\n"); - ret = -EINVAL; - goto work_err; + return -EINVAL; }
priv->workqueue = alloc_workqueue("twl6040-codec", 0, 0); - if (!priv->workqueue) { - ret = -ENOMEM; - goto work_err; - } + if (!priv->workqueue) + return -ENOMEM;
INIT_DELAYED_WORK(&priv->hs_jack.work, twl6040_accessory_work);
mutex_init(&priv->mutex);
- ret = request_threaded_irq(priv->plug_irq, NULL, twl6040_audio_handler, - IRQF_NO_SUSPEND, "twl6040_irq_plug", codec); + ret = devm_request_threaded_irq(codec->dev, priv->plug_irq, NULL, + twl6040_audio_handler, IRQF_NO_SUSPEND, + "twl6040_irq_plug", codec); if (ret) { dev_err(codec->dev, "PLUG IRQ request failed: %d\n", ret); - goto plugirq_err; + goto err; }
twl6040_init_chip(codec); @@ -1189,12 +1188,8 @@ static int twl6040_probe(struct snd_soc_codec *codec) if (!ret) return 0;
- /* Error path */ - free_irq(priv->plug_irq, codec); -plugirq_err: +err: destroy_workqueue(priv->workqueue); -work_err: - kfree(priv); return ret; }
@@ -1203,9 +1198,7 @@ static int twl6040_remove(struct snd_soc_codec *codec) struct twl6040_data *priv = snd_soc_codec_get_drvdata(codec);
twl6040_set_bias_level(codec, SND_SOC_BIAS_OFF); - free_irq(priv->plug_irq, codec); destroy_workqueue(priv->workqueue); - kfree(priv);
return 0; }
There's no need to create a queue for this anymore
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/codecs/twl6040.c | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-)
diff --git a/sound/soc/codecs/twl6040.c b/sound/soc/codecs/twl6040.c index 29fa457..6c2aa93 100644 --- a/sound/soc/codecs/twl6040.c +++ b/sound/soc/codecs/twl6040.c @@ -75,7 +75,6 @@ struct twl6040_data { u16 hf_right_step; struct twl6040_jack_data hs_jack; struct snd_soc_codec *codec; - struct workqueue_struct *workqueue; struct mutex mutex; };
@@ -404,8 +403,7 @@ static irqreturn_t twl6040_audio_handler(int irq, void *data) struct snd_soc_codec *codec = data; struct twl6040_data *priv = snd_soc_codec_get_drvdata(codec);
- queue_delayed_work(priv->workqueue, &priv->hs_jack.work, - msecs_to_jiffies(200)); + schedule_delayed_work(&priv->hs_jack.work, msecs_to_jiffies(200));
return IRQ_HANDLED; } @@ -1165,10 +1163,6 @@ static int twl6040_probe(struct snd_soc_codec *codec) return -EINVAL; }
- priv->workqueue = alloc_workqueue("twl6040-codec", 0, 0); - if (!priv->workqueue) - return -ENOMEM; - INIT_DELAYED_WORK(&priv->hs_jack.work, twl6040_accessory_work);
mutex_init(&priv->mutex); @@ -1178,27 +1172,18 @@ static int twl6040_probe(struct snd_soc_codec *codec) "twl6040_irq_plug", codec); if (ret) { dev_err(codec->dev, "PLUG IRQ request failed: %d\n", ret); - goto err; + return ret; }
twl6040_init_chip(codec);
/* power on device */ - ret = twl6040_set_bias_level(codec, SND_SOC_BIAS_STANDBY); - if (!ret) - return 0; - -err: - destroy_workqueue(priv->workqueue); - return ret; + return twl6040_set_bias_level(codec, SND_SOC_BIAS_STANDBY); }
static int twl6040_remove(struct snd_soc_codec *codec) { - struct twl6040_data *priv = snd_soc_codec_get_drvdata(codec); - twl6040_set_bias_level(codec, SND_SOC_BIAS_OFF); - destroy_workqueue(priv->workqueue);
return 0; }
The code to do the ramp has been removed a long time ago. Remove the remaining code as well since this is not needed.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/codecs/twl6040.c | 21 --------------------- 1 file changed, 21 deletions(-)
diff --git a/sound/soc/codecs/twl6040.c b/sound/soc/codecs/twl6040.c index 6c2aa93..6ad2909 100644 --- a/sound/soc/codecs/twl6040.c +++ b/sound/soc/codecs/twl6040.c @@ -69,10 +69,6 @@ struct twl6040_data { int hs_power_mode_locked; unsigned int clk_in; unsigned int sysclk; - u16 hs_left_step; - u16 hs_right_step; - u16 hf_left_step; - u16 hf_right_step; struct twl6040_jack_data hs_jack; struct snd_soc_codec *codec; struct mutex mutex; @@ -1127,7 +1123,6 @@ static int twl6040_resume(struct snd_soc_codec *codec) static int twl6040_probe(struct snd_soc_codec *codec) { struct twl6040_data *priv; - struct twl6040_codec_data *pdata = dev_get_platdata(codec->dev); struct platform_device *pdev = container_of(codec->dev, struct platform_device, dev); int ret = 0; @@ -1141,22 +1136,6 @@ static int twl6040_probe(struct snd_soc_codec *codec) priv->codec = codec; codec->control_data = dev_get_drvdata(codec->dev->parent);
- if (pdata && pdata->hs_left_step && pdata->hs_right_step) { - priv->hs_left_step = pdata->hs_left_step; - priv->hs_right_step = pdata->hs_right_step; - } else { - priv->hs_left_step = 1; - priv->hs_right_step = 1; - } - - if (pdata && pdata->hf_left_step && pdata->hf_right_step) { - priv->hf_left_step = pdata->hf_left_step; - priv->hf_right_step = pdata->hf_right_step; - } else { - priv->hf_left_step = 1; - priv->hf_right_step = 1; - } - priv->plug_irq = platform_get_irq(pdev, 0); if (priv->plug_irq < 0) { dev_err(codec->dev, "invalid irq\n");
participants (2)
-
Mark Brown -
Peter Ujfalusi