On 11/4/22 09:12, Cezary Rojewski wrote:
> From: Amadeusz Sławiński <amadeuszx.slawinski@xxxxxxxxxxxxxxx>
>
> Commit 6ed2597883b1 ("ALSA: ASoC: Don't block system resume") introduced
> deferred_resume_work for ASoC subsystem. While this allows for potential
> speed up during boot on some slow devices, it doesn't allow to properly
> propagate return values in case something failed during system resume.
Are you suggesting to remove this workqueue that's been there since
2008, which would impact negatively slow devices?
If I follow your logic, we should also remove the workqueue used for
probes for HDaudio devices, on the grounds that probe errors are not
propagated either.
Any time we have deferred processing to avoid blocking the rest of the
system, we incur the risk of not having errors propagated. It's a
compromise between having a system that's usable and a system that's
consistent.
> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@xxxxxxxxxxxxxxx>
> Signed-off-by: Cezary Rojewski <cezary.rojewski@xxxxxxxxx>
> ---
> include/sound/soc.h | 3 ---
> sound/soc/soc-core.c | 48 +++++++++++---------------------------------
> 2 files changed, 12 insertions(+), 39 deletions(-)
>
> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index 37bbfc8b45cb..3465aa075afe 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -1005,9 +1005,6 @@ struct snd_soc_card {
>
> #ifdef CONFIG_DEBUG_FS
> struct dentry *debugfs_card_root;
> -#endif
> -#ifdef CONFIG_PM_SLEEP
> - struct work_struct deferred_resume_work;
> #endif
> u32 pop_time;
>
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index a409fbed8f34..5f7e0735f0c1 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -643,17 +643,21 @@ int snd_soc_suspend(struct device *dev)
> }
> EXPORT_SYMBOL_GPL(snd_soc_suspend);
>
> -/*
> - * deferred resume work, so resume can complete before we finished
> - * setting our codec back up, which can be very slow on I2C
> - */
> -static void soc_resume_deferred(struct work_struct *work)
> +/* powers up audio subsystem after a suspend */
> +int snd_soc_resume(struct device *dev)
> {
> - struct snd_soc_card *card =
> - container_of(work, struct snd_soc_card,
> - deferred_resume_work);
> + struct snd_soc_card *card = dev_get_drvdata(dev);
> struct snd_soc_component *component;
>
> + /* If the card is not initialized yet there is nothing to do */
> + if (!card->instantiated)
> + return 0;
> +
> + /* activate pins from sleep state */
> + for_each_card_components(card, component)
> + if (snd_soc_component_active(component))
> + pinctrl_pm_select_default_state(component->dev);
> +
> /*
> * our power state is still SNDRV_CTL_POWER_D3hot from suspend time,
> * so userspace apps are blocked from touching us
> @@ -686,40 +690,14 @@ static void soc_resume_deferred(struct work_struct *work)
>
> /* userspace can access us now we are back as we were before */
> snd_power_change_state(card->snd_card, SNDRV_CTL_POWER_D0);
> -}
> -
> -/* powers up audio subsystem after a suspend */
> -int snd_soc_resume(struct device *dev)
> -{
> - struct snd_soc_card *card = dev_get_drvdata(dev);
> - struct snd_soc_component *component;
> -
> - /* If the card is not initialized yet there is nothing to do */
> - if (!card->instantiated)
> - return 0;
> -
> - /* activate pins from sleep state */
> - for_each_card_components(card, component)
> - if (snd_soc_component_active(component))
> - pinctrl_pm_select_default_state(component->dev);
> -
> - dev_dbg(dev, "ASoC: Scheduling resume work\n");
> - if (!schedule_work(&card->deferred_resume_work))
> - dev_err(dev, "ASoC: resume work item may be lost\n");
>
> return 0;
> }
> EXPORT_SYMBOL_GPL(snd_soc_resume);
>
> -static void soc_resume_init(struct snd_soc_card *card)
> -{
> - /* deferred resume work */
> - INIT_WORK(&card->deferred_resume_work, soc_resume_deferred);
> -}
> #else
> #define snd_soc_suspend NULL
> #define snd_soc_resume NULL
> -static inline void soc_resume_init(struct snd_soc_card *card) { }
> #endif
>
> static struct device_node
> @@ -1968,8 +1946,6 @@ static int snd_soc_bind_card(struct snd_soc_card *card)
>
> soc_init_card_debugfs(card);
>
> - soc_resume_init(card);
> -
> ret = snd_soc_dapm_new_controls(&card->dapm, card->dapm_widgets,
> card->num_dapm_widgets);
> if (ret < 0)
