On Fri, Sep 22, 2017 at 12:55:40AM -0700, yesanishhere@gmail.com wrote:
From: anish kumar yesanishhere@gmail.com
Currently in codec to codec dai link if there are multiple params defined then dapm can use created kcontrol to decide which param to apply at runtime.
However, in case there is only single param configuration then there is no point in creating the kcontrol and also there is no point in allocating memory for kcontrol.
In the snd_soc_dapm_new_pcm function, there is memory allocation happening for kcontrol which is later used or not used based on num_param. It is better to not allocate memory when there is only a single configuration. This change is to remedy that anomaly.
Signed-off-by: anish kumar yesanishhere@gmail.com
<snip>
+int snd_soc_dapm_new_pcm(struct snd_soc_card *card,
const struct snd_soc_pcm_stream *params,unsigned int num_params,struct snd_soc_dapm_widget *source,struct snd_soc_dapm_widget *sink)+{
- struct snd_soc_dapm_widget template;
- struct snd_soc_dapm_widget *w;
- const char **w_param_text;
- unsigned long private_value;
- char *link_name;
- int ret, count;
- link_name = devm_kasprintf(card->dev, GFP_KERNEL, "%s-%s",
source->name, sink->name);- if (!link_name)
return -ENOMEM;- memset(&template, 0, sizeof(template));
- template.reg = SND_SOC_NOPM;
- template.id = snd_soc_dapm_dai_link;
- template.name = link_name;
- template.event = snd_soc_dai_link_event;
- template.event_flags = SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU |
SND_SOC_DAPM_PRE_PMD;- template.kcontrol_news = NULL;
- w_param_text = devm_kcalloc(card->dev, num_params,
sizeof(char *), GFP_KERNEL);- if (!w_param_text) {
ret = -ENOMEM;goto param_fail;- }
Should this not also be done conditionally based on num_params? Seems odd to do all this to avoid allocations but then alloc this regardless of if it is used.
/* allocate memory for control, only in case of multiple configs */
if (num_params > 1) {
template.num_kcontrols = 1;template.kcontrol_news =snd_soc_dapm_alloc_kcontrol(card,link_name, params, num_params,w_param_text, &private_value);if (!template.kcontrol_news) {ret = -ENOMEM;goto outfree_link_name;}} dev_dbg(card->dev, "ASoC: adding %s widget\n", link_name);
w = snd_soc_dapm_new_control_unlocked(&card->dapm, &template);
@@ -3899,15 +3927,13 @@ int snd_soc_dapm_new_pcm(struct snd_soc_card *card, devm_kfree(card->dev, w); outfree_kcontrol_news: devm_kfree(card->dev, (void *)template.kcontrol_news); -outfree_private_value: devm_kfree(card->dev, (void *)private_value); -outfree_link_name:
- devm_kfree(card->dev, link_name); for (count = 0 ; count < num_params; count++) devm_kfree(card->dev, (void *)w_param_text[count]);
Do we maybe just want to add a snd_soc_dapm_free_kcontrol or some such and call that from both places rather than having basically the same error path in snd_soc_dapm_alloc_kcontrol and here.
-outfree_w_param: +outfree_link_name: devm_kfree(card->dev, w_param_text);
+param_fail:
- devm_kfree(card->dev, link_name); return ret;
}
-- 2.5.4 (Apple Git-61)
Thanks, Charles