From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com

Current snd_soc_dai_link_event_pre_pmu() is using kzalloc() / kfree() for "params", but it is fixed size, and not used on private data. It is used for setup rtd at end of this function, just a local variable. We don't need to use kzalloc() / kfree() for it. This patch replace it as local variable.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-dapm.c | 38 +++++++++++++++----------------------- 1 file changed, 15 insertions(+), 23 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 73b8bd452ca7..7d4e4068f870 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3837,15 +3837,13 @@ snd_soc_dai_link_event_pre_pmu(struct snd_soc_dapm_widget *w, struct snd_soc_dapm_path *path; struct snd_soc_dai *source, *sink; struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); - struct snd_pcm_hw_params *params = NULL; + struct snd_pcm_hw_params params; const struct snd_soc_pcm_stream *config = NULL; struct snd_pcm_runtime *runtime = NULL; unsigned int fmt; int ret = 0;

- params = kzalloc(sizeof(*params), GFP_KERNEL); - if (!params) - return -ENOMEM; + memset(&params, 0, sizeof(params));

runtime = kzalloc(sizeof(*runtime), GFP_KERNEL); if (!runtime) { @@ -3902,45 +3900,39 @@ snd_soc_dai_link_event_pre_pmu(struct snd_soc_dapm_widget *w, goto out; }

- snd_mask_set(hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT), fmt); - hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE)->min = - config->rate_min; - hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE)->max = - config->rate_max; - hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS)->min - = config->channels_min; - hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS)->max - = config->channels_max; + snd_mask_set(hw_param_mask(&params, SNDRV_PCM_HW_PARAM_FORMAT), fmt); + hw_param_interval(&params, SNDRV_PCM_HW_PARAM_RATE)->min = config->rate_min; + hw_param_interval(&params, SNDRV_PCM_HW_PARAM_RATE)->max = config->rate_max; + hw_param_interval(&params, SNDRV_PCM_HW_PARAM_CHANNELS)->min = config->channels_min; + hw_param_interval(&params, SNDRV_PCM_HW_PARAM_CHANNELS)->max = config->channels_max;

substream->stream = SNDRV_PCM_STREAM_CAPTURE; snd_soc_dapm_widget_for_each_source_path(w, path) { source = path->source->priv;

- ret = snd_soc_dai_hw_params(source, substream, params); + ret = snd_soc_dai_hw_params(source, substream, &params); if (ret < 0) goto out;

- dapm_update_dai_unlocked(substream, params, source); + dapm_update_dai_unlocked(substream, &params, source); }

substream->stream = SNDRV_PCM_STREAM_PLAYBACK; snd_soc_dapm_widget_for_each_sink_path(w, path) { sink = path->sink->priv;

- ret = snd_soc_dai_hw_params(sink, substream, params); + ret = snd_soc_dai_hw_params(sink, substream, &params); if (ret < 0) goto out;

- dapm_update_dai_unlocked(substream, params, sink); + dapm_update_dai_unlocked(substream, &params, sink); }

- runtime->format = params_format(params); - runtime->subformat = params_subformat(params); - runtime->channels = params_channels(params); - runtime->rate = params_rate(params); - + runtime->format = params_format(&params); + runtime->subformat = params_subformat(&params); + runtime->channels = params_channels(&params); + runtime->rate = params_rate(&params); out: - kfree(params); return ret; }

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com

snd_soc_dai_link_event_pre_pmu() is using if/else for config->formats check, but "else" case is for just error. Unnecessary if/else is not good for readable code. this patch checks if config->formats was zero and call "goto out" in such case.

[not readable] if (config->formats) { (A) fmt = ... } else { ... (B) goto err; }

[readable] if (!config->formats) { ... (B) goto err; }

(A) fmt = ...

Moreover, we don't need to indicate config->formats value in error message, because it is zero.

=> if (config->formats) { ... } else { dev_warn(w->dapm->dev, "ASoC: Invalid format %llx specified\n", => config->formats); ... }

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-dapm.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index bc7d64b570b4..e8c813586b53 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3890,16 +3890,14 @@ snd_soc_dai_link_event_pre_pmu(struct snd_soc_dapm_widget *w, }

/* Be a little careful as we don't want to overflow the mask array */ - if (config->formats) { - fmt = ffs(config->formats) - 1; - } else { - dev_warn(w->dapm->dev, "ASoC: Invalid format %llx specified\n", - config->formats); - + if (!config->formats) { + dev_warn(w->dapm->dev, "ASoC: Invalid format was specified\n"); ret = -EINVAL; goto out; }

+ fmt = ffs(config->formats) - 1; + snd_mask_set(hw_param_mask(&params, SNDRV_PCM_HW_PARAM_FORMAT), fmt); hw_param_interval(&params, SNDRV_PCM_HW_PARAM_RATE)->min = config->rate_min; hw_param_interval(&params, SNDRV_PCM_HW_PARAM_RATE)->max = config->rate_max;

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com

Current snd_soc_dapm_new_control_unlocked() error handling is wrong. It is using "goto request_failed" (A), but error message is using "w->name" (B) which is not yet created in such timing.

snd_soc_dapm_new_control_unlocked(xxx) { ... switch (w->id) { case xxx: ... if (IS_ERR(...)) { ret = PTR_ERR(...); (A) goto request_failed; } ... }

prefix = soc_dapm_prefix(...); if (prefix) (B) w->name = kasprintf(...); else (B) w->name = kstrdup_const(...); ...

(A) request_failed: if (ret != -EPROBE_DEFER) (B) dev_err(..., w->name, ...);

return ...; }

we can create "w->name" at beginning of this function. In such case, we need to call kfree_const(w->name) at error case. This patch do these.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-dapm.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index e8c813586b53..928e3bfe7457 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3630,10 +3630,18 @@ snd_soc_dapm_new_control_unlocked(struct snd_soc_dapm_context *dapm, enum snd_soc_dapm_direction dir; struct snd_soc_dapm_widget *w; const char *prefix; - int ret; + int ret = -ENOMEM;

if ((w = dapm_cnew_widget(widget)) == NULL) - return ERR_PTR(-ENOMEM); + goto cnew_failed; + + prefix = soc_dapm_prefix(dapm); + if (prefix) + w->name = kasprintf(GFP_KERNEL, "%s %s", prefix, widget->name); + else + w->name = kstrdup_const(widget->name, GFP_KERNEL); + if (!w->name) + goto name_failed;

switch (w->id) { case snd_soc_dapm_regulator_supply: @@ -3672,17 +3680,6 @@ snd_soc_dapm_new_control_unlocked(struct snd_soc_dapm_context *dapm, break; }

- prefix = soc_dapm_prefix(dapm); - if (prefix) - w->name = kasprintf(GFP_KERNEL, "%s %s", prefix, widget->name); - else - w->name = kstrdup_const(widget->name, GFP_KERNEL); - if (w->name == NULL) { - kfree_const(w->sname); - kfree(w); - return ERR_PTR(-ENOMEM); - } - switch (w->id) { case snd_soc_dapm_mic: w->is_ep = SND_SOC_DAPM_EP_SOURCE; @@ -3770,9 +3767,11 @@ snd_soc_dapm_new_control_unlocked(struct snd_soc_dapm_context *dapm, if (ret != -EPROBE_DEFER) dev_err(dapm->dev, "ASoC: Failed to request %s: %d\n", w->name, ret); - + kfree_const(w->name); +name_failed: kfree_const(w->sname); kfree(w); +cnew_failed: return ERR_PTR(ret); }