Commit 5061e488 authored by Mark Brown's avatar Mark Brown

Merge series "ASoC: merge soc_pcm_open() rollback and soc_pcm_close()" from...

Merge series "ASoC: merge soc_pcm_open() rollback and soc_pcm_close()" from Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>:

Hi Mark

1 month past and nothing happened.
This is resend of v2 patch-set.

soc_pcm_open() does rollback when failed (A),
but, it is almost same as soc_pcm_close().

	static int soc_pcm_open(xxx)
	{
		...
		if (ret < 0)
			goto xxx_err;
		...
		return 0;

 ^	config_err:
 |		...
 |	rtd_startup_err:
(A)		...
 |	component_err:
 |		...
 v		return ret;
	}

This kind of duplicated code can be a hotbed of bugs,
thus, this patch-set share soc_pcm_close() and rollback.

v1 -> v2
	- indicate more detail background/logic on git-log

Link: https://lore.kernel.org/r/87wo2oku0m.wl-kuninori.morimoto.gx@renesas.com
Link: https://lore.kernel.org/r/87wo1kvozz.wl-kuninori.morimoto.gx@renesas.com

Kuninori Morimoto (7):
  ASoC: soc-dai: add mark for snd_soc_dai_startup/shutdown()
  ASoC: soc-link: add mark for snd_soc_link_startup/shutdown()
  ASoC: soc-component: add mark for soc_pcm_components_open/close()
  ASoC: soc-component: add mark for
    snd_soc_pcm_component_pm_runtime_get/put()
  ASoC: soc-pcm: add soc_pcm_clean() and call it from
    soc_pcm_open/close()
  ASoC: soc-pcm: remove unneeded dev_err() for snd_soc_dai_startup()
  ASoC: soc-pcm: remove unneeded dev_err() for
    snd_soc_component_module/open()

 include/sound/soc-component.h |  28 +++++---
 include/sound/soc-dai.h       |   5 +-
 include/sound/soc-link.h      |   3 +-
 include/sound/soc.h           |   3 +
 sound/soc/soc-component.c     |  73 ++++++++++++++++++++-
 sound/soc/soc-compress.c      |  30 +++------
 sound/soc/soc-dai.c           |  21 +++++-
 sound/soc/soc-dapm.c          |   4 +-
 sound/soc/soc-link.c          |  21 +++++-
 sound/soc/soc-pcm.c           | 120 ++++++++++++----------------------
 10 files changed, 190 insertions(+), 118 deletions(-)

--
2.25.1
parents 19895e92 bcae1631
......@@ -217,6 +217,11 @@ struct snd_soc_component {
/* machine specific init */
int (*init)(struct snd_soc_component *component);
/* function mark */
struct snd_pcm_substream *mark_module;
struct snd_pcm_substream *mark_open;
void *mark_pm;
#ifdef CONFIG_DEBUG_FS
struct dentry *debugfs_root;
const char *debugfs_prefix;
......@@ -370,17 +375,19 @@ void snd_soc_component_exit_regmap(struct snd_soc_component *component);
#endif
#define snd_soc_component_module_get_when_probe(component)\
snd_soc_component_module_get(component, 0)
#define snd_soc_component_module_get_when_open(component) \
snd_soc_component_module_get(component, 1)
snd_soc_component_module_get(component, NULL, 0)
#define snd_soc_component_module_get_when_open(component, substream) \
snd_soc_component_module_get(component, substream, 1)
int snd_soc_component_module_get(struct snd_soc_component *component,
struct snd_pcm_substream *substream,
int upon_open);
#define snd_soc_component_module_put_when_remove(component) \
snd_soc_component_module_put(component, 0)
#define snd_soc_component_module_put_when_close(component) \
snd_soc_component_module_put(component, 1)
snd_soc_component_module_put(component, NULL, 0, 0)
#define snd_soc_component_module_put_when_close(component, substream, rollback) \
snd_soc_component_module_put(component, substream, 1, rollback)
void snd_soc_component_module_put(struct snd_soc_component *component,
int upon_open);
struct snd_pcm_substream *substream,
int upon_open, int rollback);
static inline void snd_soc_component_set_drvdata(struct snd_soc_component *c,
void *data)
......@@ -424,7 +431,8 @@ int snd_soc_component_force_enable_pin_unlocked(
int snd_soc_component_open(struct snd_soc_component *component,
struct snd_pcm_substream *substream);
int snd_soc_component_close(struct snd_soc_component *component,
struct snd_pcm_substream *substream);
struct snd_pcm_substream *substream,
int rollback);
void snd_soc_component_suspend(struct snd_soc_component *component);
void snd_soc_component_resume(struct snd_soc_component *component);
int snd_soc_component_is_suspended(struct snd_soc_component *component);
......@@ -457,5 +465,9 @@ void snd_soc_pcm_component_hw_free(struct snd_pcm_substream *substream,
struct snd_soc_component *last);
int snd_soc_pcm_component_trigger(struct snd_pcm_substream *substream,
int cmd);
int snd_soc_pcm_component_pm_runtime_get(struct snd_soc_pcm_runtime *rtd,
void *stream);
void snd_soc_pcm_component_pm_runtime_put(struct snd_soc_pcm_runtime *rtd,
void *stream, int rollback);
#endif /* __SOC_COMPONENT_H */
......@@ -153,7 +153,7 @@ void snd_soc_dai_hw_free(struct snd_soc_dai *dai,
int snd_soc_dai_startup(struct snd_soc_dai *dai,
struct snd_pcm_substream *substream);
void snd_soc_dai_shutdown(struct snd_soc_dai *dai,
struct snd_pcm_substream *substream);
struct snd_pcm_substream *substream, int rollback);
snd_pcm_sframes_t snd_soc_dai_delay(struct snd_soc_dai *dai,
struct snd_pcm_substream *substream);
void snd_soc_dai_suspend(struct snd_soc_dai *dai);
......@@ -388,6 +388,9 @@ struct snd_soc_dai {
struct list_head list;
/* function mark */
struct snd_pcm_substream *mark_startup;
/* bit field */
unsigned int probed:1;
};
......
......@@ -14,7 +14,8 @@ int snd_soc_link_be_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
struct snd_pcm_hw_params *params);
int snd_soc_link_startup(struct snd_pcm_substream *substream);
void snd_soc_link_shutdown(struct snd_pcm_substream *substream);
void snd_soc_link_shutdown(struct snd_pcm_substream *substream,
int rollback);
int snd_soc_link_prepare(struct snd_pcm_substream *substream);
int snd_soc_link_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params);
......
......@@ -1159,6 +1159,9 @@ struct snd_soc_pcm_runtime {
unsigned int num; /* 0-based and monotonic increasing */
struct list_head list; /* rtd list of the soc card */
/* function mark */
struct snd_pcm_substream *mark_startup;
/* bit field */
unsigned int pop_wait:1;
unsigned int fe_compr:1; /* for Dynamic PCM */
......
......@@ -9,6 +9,7 @@
// Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
//
#include <linux/module.h>
#include <linux/pm_runtime.h>
#include <sound/soc.h>
#define soc_component_ret(dai, ret) _soc_component_ret(dai, __func__, ret)
......@@ -33,6 +34,14 @@ static inline int _soc_component_ret(struct snd_soc_component *component,
return ret;
}
/*
* We might want to check substream by using list.
* In such case, we can update these macros.
*/
#define soc_component_mark_push(component, substream, tgt) ((component)->mark_##tgt = substream)
#define soc_component_mark_pop(component, substream, tgt) ((component)->mark_##tgt = NULL)
#define soc_component_mark_match(component, substream, tgt) ((component)->mark_##tgt == substream)
void snd_soc_component_set_aux(struct snd_soc_component *component,
struct snd_soc_aux_dev *aux)
{
......@@ -238,6 +247,7 @@ int snd_soc_component_set_jack(struct snd_soc_component *component,
EXPORT_SYMBOL_GPL(snd_soc_component_set_jack);
int snd_soc_component_module_get(struct snd_soc_component *component,
struct snd_pcm_substream *substream,
int upon_open)
{
int ret = 0;
......@@ -246,14 +256,25 @@ int snd_soc_component_module_get(struct snd_soc_component *component,
!try_module_get(component->dev->driver->owner))
ret = -ENODEV;
/* mark substream if succeeded */
if (ret == 0)
soc_component_mark_push(component, substream, module);
return soc_component_ret(component, ret);
}
void snd_soc_component_module_put(struct snd_soc_component *component,
int upon_open)
struct snd_pcm_substream *substream,
int upon_open, int rollback)
{
if (rollback && !soc_component_mark_match(component, substream, module))
return;
if (component->driver->module_get_upon_open == !!upon_open)
module_put(component->dev->driver->owner);
/* remove marked substream */
soc_component_mark_pop(component, substream, module);
}
int snd_soc_component_open(struct snd_soc_component *component,
......@@ -264,17 +285,28 @@ int snd_soc_component_open(struct snd_soc_component *component,
if (component->driver->open)
ret = component->driver->open(component, substream);
/* mark substream if succeeded */
if (ret == 0)
soc_component_mark_push(component, substream, open);
return soc_component_ret(component, ret);
}
int snd_soc_component_close(struct snd_soc_component *component,
struct snd_pcm_substream *substream)
struct snd_pcm_substream *substream,
int rollback)
{
int ret = 0;
if (rollback && !soc_component_mark_match(component, substream, open))
return 0;
if (component->driver->close)
ret = component->driver->close(component, substream);
/* remove marked substream */
soc_component_mark_pop(component, substream, open);
return soc_component_ret(component, ret);
}
......@@ -805,3 +837,40 @@ int snd_soc_pcm_component_trigger(struct snd_pcm_substream *substream,
return 0;
}
int snd_soc_pcm_component_pm_runtime_get(struct snd_soc_pcm_runtime *rtd,
void *stream)
{
struct snd_soc_component *component;
int i, ret;
for_each_rtd_components(rtd, i, component) {
ret = pm_runtime_get_sync(component->dev);
if (ret < 0 && ret != -EACCES) {
pm_runtime_put_noidle(component->dev);
return soc_component_ret(component, ret);
}
/* mark stream if succeeded */
soc_component_mark_push(component, stream, pm);
}
return 0;
}
void snd_soc_pcm_component_pm_runtime_put(struct snd_soc_pcm_runtime *rtd,
void *stream, int rollback)
{
struct snd_soc_component *component;
int i;
for_each_rtd_components(rtd, i, component) {
if (rollback && !soc_component_mark_match(component, stream, pm))
continue;
pm_runtime_mark_last_busy(component->dev);
pm_runtime_put_autosuspend(component->dev);
/* remove marked stream */
soc_component_mark_pop(component, stream, pm);
}
}
......@@ -73,18 +73,13 @@ static int soc_compr_components_free(struct snd_compr_stream *cstream,
static int soc_compr_open(struct snd_compr_stream *cstream)
{
struct snd_soc_pcm_runtime *rtd = cstream->private_data;
struct snd_soc_component *component = NULL, *save = NULL;
struct snd_soc_component *component = NULL;
struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
int ret, i;
int ret;
for_each_rtd_components(rtd, i, component) {
ret = pm_runtime_get_sync(component->dev);
if (ret < 0 && ret != -EACCES) {
pm_runtime_put_noidle(component->dev);
save = component;
ret = snd_soc_pcm_component_pm_runtime_get(rtd, cstream);
if (ret < 0)
goto pm_err;
}
}
mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
......@@ -113,12 +108,7 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
out:
mutex_unlock(&rtd->card->pcm_mutex);
pm_err:
for_each_rtd_components(rtd, i, component) {
if (component == save)
break;
pm_runtime_mark_last_busy(component->dev);
pm_runtime_put_autosuspend(component->dev);
}
snd_soc_pcm_component_pm_runtime_put(rtd, cstream, 1);
return ret;
}
......@@ -205,10 +195,9 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
static int soc_compr_free(struct snd_compr_stream *cstream)
{
struct snd_soc_pcm_runtime *rtd = cstream->private_data;
struct snd_soc_component *component;
struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
int stream, i;
int stream;
mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
......@@ -237,10 +226,7 @@ static int soc_compr_free(struct snd_compr_stream *cstream)
mutex_unlock(&rtd->card->pcm_mutex);
for_each_rtd_components(rtd, i, component) {
pm_runtime_mark_last_busy(component->dev);
pm_runtime_put_autosuspend(component->dev);
}
snd_soc_pcm_component_pm_runtime_put(rtd, cstream, 0);
return 0;
}
......
......@@ -32,6 +32,14 @@ static inline int _soc_dai_ret(struct snd_soc_dai *dai,
return ret;
}
/*
* We might want to check substream by using list.
* In such case, we can update these macros.
*/
#define soc_dai_mark_push(dai, substream, tgt) ((dai)->mark_##tgt = substream)
#define soc_dai_mark_pop(dai, substream, tgt) ((dai)->mark_##tgt = NULL)
#define soc_dai_mark_match(dai, substream, tgt) ((dai)->mark_##tgt == substream)
/**
* snd_soc_dai_set_sysclk - configure DAI system or master clock.
* @dai: DAI
......@@ -348,15 +356,26 @@ int snd_soc_dai_startup(struct snd_soc_dai *dai,
dai->driver->ops->startup)
ret = dai->driver->ops->startup(substream, dai);
/* mark substream if succeeded */
if (ret == 0)
soc_dai_mark_push(dai, substream, startup);
return soc_dai_ret(dai, ret);
}
void snd_soc_dai_shutdown(struct snd_soc_dai *dai,
struct snd_pcm_substream *substream)
struct snd_pcm_substream *substream,
int rollback)
{
if (rollback && !soc_dai_mark_match(dai, substream, startup))
return;
if (dai->driver->ops &&
dai->driver->ops->shutdown)
dai->driver->ops->shutdown(substream, dai);
/* remove marked substream */
soc_dai_mark_pop(dai, substream, startup);
}
snd_pcm_sframes_t snd_soc_dai_delay(struct snd_soc_dai *dai,
......
......@@ -3968,14 +3968,14 @@ static int snd_soc_dai_link_event(struct snd_soc_dapm_widget *w,
snd_soc_dapm_widget_for_each_source_path(w, path) {
source = path->source->priv;
snd_soc_dai_deactivate(source, substream->stream);
snd_soc_dai_shutdown(source, substream);
snd_soc_dai_shutdown(source, substream, 0);
}
substream->stream = SNDRV_PCM_STREAM_PLAYBACK;
snd_soc_dapm_widget_for_each_sink_path(w, path) {
sink = path->sink->priv;
snd_soc_dai_deactivate(sink, substream->stream);
snd_soc_dai_shutdown(sink, substream);
snd_soc_dai_shutdown(sink, substream, 0);
}
break;
......
......@@ -30,6 +30,14 @@ static inline int _soc_link_ret(struct snd_soc_pcm_runtime *rtd,
return ret;
}
/*
* We might want to check substream by using list.
* In such case, we can update these macros.
*/
#define soc_link_mark_push(rtd, substream, tgt) ((rtd)->mark_##tgt = substream)
#define soc_link_mark_pop(rtd, substream, tgt) ((rtd)->mark_##tgt = NULL)
#define soc_link_mark_match(rtd, substream, tgt) ((rtd)->mark_##tgt == substream)
int snd_soc_link_init(struct snd_soc_pcm_runtime *rtd)
{
int ret = 0;
......@@ -66,16 +74,27 @@ int snd_soc_link_startup(struct snd_pcm_substream *substream)
rtd->dai_link->ops->startup)
ret = rtd->dai_link->ops->startup(substream);
/* mark substream if succeeded */
if (ret == 0)
soc_link_mark_push(rtd, substream, startup);
return soc_link_ret(rtd, ret);
}
void snd_soc_link_shutdown(struct snd_pcm_substream *substream)
void snd_soc_link_shutdown(struct snd_pcm_substream *substream,
int rollback)
{
struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
if (rollback && !soc_link_mark_match(rtd, substream, startup))
return;
if (rtd->dai_link->ops &&
rtd->dai_link->ops->shutdown)
rtd->dai_link->ops->shutdown(substream);
/* remove marked substream */
soc_link_mark_pop(rtd, substream, startup);
}
int snd_soc_link_prepare(struct snd_pcm_substream *substream)
......
......@@ -609,68 +609,41 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
static int soc_pcm_components_open(struct snd_pcm_substream *substream)
{
struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
struct snd_soc_component *last = NULL;
struct snd_soc_component *component;
int i, ret = 0;
for_each_rtd_components(rtd, i, component) {
last = component;
ret = snd_soc_component_module_get_when_open(component);
if (ret < 0) {
dev_err(component->dev,
"ASoC: can't get module %s\n",
component->name);
ret = snd_soc_component_module_get_when_open(component, substream);
if (ret < 0)
break;
}
ret = snd_soc_component_open(component, substream);
if (ret < 0) {
snd_soc_component_module_put_when_close(component);
dev_err(component->dev,
"ASoC: can't open component %s: %d\n",
component->name, ret);
break;
}
}
if (ret < 0) {
/* rollback on error */
for_each_rtd_components(rtd, i, component) {
if (component == last)
if (ret < 0)
break;
snd_soc_component_close(component, substream);
snd_soc_component_module_put_when_close(component);
}
}
return ret;
}
static int soc_pcm_components_close(struct snd_pcm_substream *substream)
static int soc_pcm_components_close(struct snd_pcm_substream *substream,
int rollback)
{
struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
struct snd_soc_component *component;
int i, r, ret = 0;
for_each_rtd_components(rtd, i, component) {
r = snd_soc_component_close(component, substream);
r = snd_soc_component_close(component, substream, rollback);
if (r < 0)
ret = r; /* use last ret */
snd_soc_component_module_put_when_close(component);
snd_soc_component_module_put_when_close(component, substream, rollback);
}
return ret;
}
/*
* Called by ALSA when a PCM substream is closed. Private data can be
* freed here. The cpu DAI, codec DAI, machine and components are also
* shutdown.
*/
static int soc_pcm_close(struct snd_pcm_substream *substream)
static int soc_pcm_clean(struct snd_pcm_substream *substream, int rollback)
{
struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
struct snd_soc_component *component;
......@@ -679,23 +652,22 @@ static int soc_pcm_close(struct snd_pcm_substream *substream)
mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
if (!rollback)
snd_soc_runtime_deactivate(rtd, substream->stream);
for_each_rtd_dais(rtd, i, dai)
snd_soc_dai_shutdown(dai, substream);
snd_soc_dai_shutdown(dai, substream, rollback);
snd_soc_link_shutdown(substream);
snd_soc_link_shutdown(substream, rollback);
soc_pcm_components_close(substream);
soc_pcm_components_close(substream, rollback);
if (!rollback)
snd_soc_dapm_stream_stop(rtd, substream->stream);
mutex_unlock(&rtd->card->pcm_mutex);
for_each_rtd_components(rtd, i, component) {
pm_runtime_mark_last_busy(component->dev);
pm_runtime_put_autosuspend(component->dev);
}
snd_soc_pcm_component_pm_runtime_put(rtd, substream, rollback);
for_each_rtd_components(rtd, i, component)
if (!snd_soc_component_active(component))
......@@ -704,6 +676,16 @@ static int soc_pcm_close(struct snd_pcm_substream *substream)
return 0;
}
/*
* Called by ALSA when a PCM substream is closed. Private data can be
* freed here. The cpu DAI, codec DAI, machine and components are also
* shutdown.
*/
static int soc_pcm_close(struct snd_pcm_substream *substream)
{
return soc_pcm_clean(substream, 0);
}
/*
* Called by ALSA when a PCM substream is opened, the runtime->hw record is
* then initialized and any private data can be allocated. This also calls
......@@ -722,28 +704,25 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
for_each_rtd_components(rtd, i, component)
pinctrl_pm_select_default_state(component->dev);
for_each_rtd_components(rtd, i, component)
pm_runtime_get_sync(component->dev);
ret = snd_soc_pcm_component_pm_runtime_get(rtd, substream);
if (ret < 0)
goto err;
mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
ret = soc_pcm_components_open(substream);
if (ret < 0)
goto component_err;
goto err;
ret = snd_soc_link_startup(substream);
if (ret < 0)
goto rtd_startup_err;
goto err;
/* startup the audio subsystem */
for_each_rtd_dais(rtd, i, dai) {
ret = snd_soc_dai_startup(dai, substream);
if (ret < 0) {
dev_err(dai->dev,
"ASoC: can't open DAI %s: %d\n",
dai->name, ret);
goto config_err;
}
if (ret < 0)
goto err;
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
dai->tx_mask = 0;
......@@ -771,18 +750,18 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
if (!runtime->hw.rates) {
printk(KERN_ERR "ASoC: %s <-> %s No matching rates\n",
codec_dai_name, cpu_dai_name);
goto config_err;
goto err;
}
if (!runtime->hw.formats) {
printk(KERN_ERR "ASoC: %s <-> %s No matching formats\n",
codec_dai_name, cpu_dai_name);
goto config_err;
goto err;
}
if (!runtime->hw.channels_min || !runtime->hw.channels_max ||
runtime->hw.channels_min > runtime->hw.channels_max) {
printk(KERN_ERR "ASoC: %s <-> %s No matching channels\n",
codec_dai_name, cpu_dai_name);
goto config_err;
goto err;
}
soc_pcm_apply_msb(substream);
......@@ -792,7 +771,7 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
if (snd_soc_dai_active(dai)) {
ret = soc_pcm_apply_symmetry(substream, dai);
if (ret != 0)
goto config_err;
goto err;
}
}
......@@ -803,32 +782,13 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
runtime->hw.channels_max);
pr_debug("ASoC: min rate %d max rate %d\n", runtime->hw.rate_min,
runtime->hw.rate_max);
dynamic:
snd_soc_runtime_activate(rtd, substream->stream);
err:
mutex_unlock(&rtd->card->pcm_mutex);
return 0;
config_err:
for_each_rtd_dais_rollback(rtd, i, dai)
snd_soc_dai_shutdown(dai, substream);
snd_soc_link_shutdown(substream);
rtd_startup_err:
soc_pcm_components_close(substream);
component_err:
mutex_unlock(&rtd->card->pcm_mutex);
for_each_rtd_components(rtd, i, component) {
pm_runtime_mark_last_busy(component->dev);
pm_runtime_put_autosuspend(component->dev);
}
for_each_rtd_components(rtd, i, component)
if (!snd_soc_component_active(component))
pinctrl_pm_select_sleep_state(component->dev);
if (ret < 0)
soc_pcm_clean(substream, 1);
return ret;
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment