[alsa-devel] [RFC PATCH] ASoC: max98357a: use mdelay for sdmode-delay
max98357a_daiops_trigger() is possible to be called in atomic context if the .nonatomic flag is equal to 0 in the DAI links.
When cancel_delayed_work_sync() in max98357a_daiops_trigger() is called in atomic context, kernel emits the following message: "BUG: sleeping function called from invalid context".
According to the DT binding document, value less than or equal to 5ms of sdmod-delay should be sufficient to avoid the pop noise. Use mdelay (i.e. busy loop) for such low delay should be acceptable.
Fixes: cec5b01f8f1c ("ASoC: max98357a: avoid speaker pop when playback startup")
Signed-off-by: Tzung-Bi Shih tzungbi@google.com --- Hi,
Not sure whether this fix is acceptable or not. Because in the document "timers-howto.txt", it states "In general, use of mdelay is discouraged and code should be refactored to allow for the use of msleep."
sound/soc/codecs/max98357a.c | 25 ++++--------------------- 1 file changed, 4 insertions(+), 21 deletions(-)
diff --git a/sound/soc/codecs/max98357a.c b/sound/soc/codecs/max98357a.c index 6f0e28f903bf..16313b973eaa 100644 --- a/sound/soc/codecs/max98357a.c +++ b/sound/soc/codecs/max98357a.c @@ -20,20 +20,10 @@ #include <sound/soc-dapm.h>
struct max98357a_priv { - struct delayed_work enable_sdmode_work; struct gpio_desc *sdmode; unsigned int sdmode_delay; };
-static void max98357a_enable_sdmode_work(struct work_struct *work) -{ - struct max98357a_priv *max98357a = - container_of(work, struct max98357a_priv, - enable_sdmode_work.work); - - gpiod_set_value(max98357a->sdmode, 1); -} - static int max98357a_daiops_trigger(struct snd_pcm_substream *substream, int cmd, struct snd_soc_dai *dai) { @@ -46,14 +36,12 @@ static int max98357a_daiops_trigger(struct snd_pcm_substream *substream, case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: - queue_delayed_work(system_power_efficient_wq, - &max98357a->enable_sdmode_work, - msecs_to_jiffies(max98357a->sdmode_delay)); + mdelay(max98357a->sdmode_delay); + gpiod_set_value(max98357a->sdmode, 1); break; case SNDRV_PCM_TRIGGER_STOP: case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_PAUSE_PUSH: - cancel_delayed_work_sync(&max98357a->enable_sdmode_work); gpiod_set_value(max98357a->sdmode, 0); break; } @@ -112,30 +100,25 @@ static int max98357a_platform_probe(struct platform_device *pdev) int ret;
max98357a = devm_kzalloc(&pdev->dev, sizeof(*max98357a), GFP_KERNEL); - if (!max98357a) return -ENOMEM;
max98357a->sdmode = devm_gpiod_get_optional(&pdev->dev, "sdmode", GPIOD_OUT_LOW); - if (IS_ERR(max98357a->sdmode)) return PTR_ERR(max98357a->sdmode);
ret = device_property_read_u32(&pdev->dev, "sdmode-delay", &max98357a->sdmode_delay); - if (ret) { max98357a->sdmode_delay = 0; dev_dbg(&pdev->dev, - "no optional property 'sdmode-delay' found, default: no delay\n"); + "no optional property 'sdmode-delay' found, " + "default: no delay\n"); }
dev_set_drvdata(&pdev->dev, max98357a);
- INIT_DELAYED_WORK(&max98357a->enable_sdmode_work, - max98357a_enable_sdmode_work); - return devm_snd_soc_register_component(&pdev->dev, &max98357a_component_driver, &max98357a_dai_driver, 1);
The patch
ASoC: max98357a: use mdelay for sdmode-delay
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.3
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 6cd249cfad68a231336983e2216d75b3ddfde1d6 Mon Sep 17 00:00:00 2001
From: Tzung-Bi Shih tzungbi@google.com Date: Mon, 8 Jul 2019 22:19:01 +0800 Subject: [PATCH] ASoC: max98357a: use mdelay for sdmode-delay
max98357a_daiops_trigger() is possible to be called in atomic context if the .nonatomic flag is equal to 0 in the DAI links.
When cancel_delayed_work_sync() in max98357a_daiops_trigger() is called in atomic context, kernel emits the following message: "BUG: sleeping function called from invalid context".
According to the DT binding document, value less than or equal to 5ms of sdmod-delay should be sufficient to avoid the pop noise. Use mdelay (i.e. busy loop) for such low delay should be acceptable.
Fixes: cec5b01f8f1c ("ASoC: max98357a: avoid speaker pop when playback startup")
Signed-off-by: Tzung-Bi Shih tzungbi@google.com Link: https://lore.kernel.org/r/20190708141901.68797-1-tzungbi@google.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/codecs/max98357a.c | 25 ++++--------------------- 1 file changed, 4 insertions(+), 21 deletions(-)
diff --git a/sound/soc/codecs/max98357a.c b/sound/soc/codecs/max98357a.c index 6f0e28f903bf..16313b973eaa 100644 --- a/sound/soc/codecs/max98357a.c +++ b/sound/soc/codecs/max98357a.c @@ -20,20 +20,10 @@ #include <sound/soc-dapm.h>
struct max98357a_priv { - struct delayed_work enable_sdmode_work; struct gpio_desc *sdmode; unsigned int sdmode_delay; };
-static void max98357a_enable_sdmode_work(struct work_struct *work) -{ - struct max98357a_priv *max98357a = - container_of(work, struct max98357a_priv, - enable_sdmode_work.work); - - gpiod_set_value(max98357a->sdmode, 1); -} - static int max98357a_daiops_trigger(struct snd_pcm_substream *substream, int cmd, struct snd_soc_dai *dai) { @@ -46,14 +36,12 @@ static int max98357a_daiops_trigger(struct snd_pcm_substream *substream, case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: - queue_delayed_work(system_power_efficient_wq, - &max98357a->enable_sdmode_work, - msecs_to_jiffies(max98357a->sdmode_delay)); + mdelay(max98357a->sdmode_delay); + gpiod_set_value(max98357a->sdmode, 1); break; case SNDRV_PCM_TRIGGER_STOP: case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_PAUSE_PUSH: - cancel_delayed_work_sync(&max98357a->enable_sdmode_work); gpiod_set_value(max98357a->sdmode, 0); break; } @@ -112,30 +100,25 @@ static int max98357a_platform_probe(struct platform_device *pdev) int ret;
max98357a = devm_kzalloc(&pdev->dev, sizeof(*max98357a), GFP_KERNEL); - if (!max98357a) return -ENOMEM;
max98357a->sdmode = devm_gpiod_get_optional(&pdev->dev, "sdmode", GPIOD_OUT_LOW); - if (IS_ERR(max98357a->sdmode)) return PTR_ERR(max98357a->sdmode);
ret = device_property_read_u32(&pdev->dev, "sdmode-delay", &max98357a->sdmode_delay); - if (ret) { max98357a->sdmode_delay = 0; dev_dbg(&pdev->dev, - "no optional property 'sdmode-delay' found, default: no delay\n"); + "no optional property 'sdmode-delay' found, " + "default: no delay\n"); }
dev_set_drvdata(&pdev->dev, max98357a);
- INIT_DELAYED_WORK(&max98357a->enable_sdmode_work, - max98357a_enable_sdmode_work); - return devm_snd_soc_register_component(&pdev->dev, &max98357a_component_driver, &max98357a_dai_driver, 1);
participants (2)
-
Mark Brown -
Tzung-Bi Shih