ASoC: dapm: Avoid creating kcontrol for params

From: anish kumar <[email protected]>

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.

Quoted 7 lines · On Fri, Sep 22, 2017 at 12:55:40AM -0700, [email protected] wrote:

From: anish kumar <[email protected]>

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.

You've sent me patch 3/4 and only patch 3/4.

What's going on with the
other three patches? Please remember that the entire purpose of numbering patches is to order them in a series, if you're not sending a series there's no need to number things and if

you're sending a series
the numbering should reflect what you're sending now, not any previous versions of the patches.

From: anish kumar <[email protected]>

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 <[email protected]>

---

sound/soc/soc-dapm.c +23 -0

+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;

sound/soc/soc-dapm.c +13 -0

+
+	/* 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);

sound/soc/soc-dapm.c +3 -2

-outfree_w_param:
+outfree_link_name:
 	devm_kfree(card->dev, w_param_text);
-
+param_fail:
+	devm_kfree(card->dev, link_name);
 	return ret;
 }
 

Signature · 1 line

2.5.4 (Apple Git-61)

Quoted 18 lines · On Fri, Sep 22, 2017 at 12:55:40AM -0700, [email protected] wrote:

From: anish kumar <[email protected]>

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 <[email protected]>

---

<snip>

Quoted 29 lines

sound/soc/soc-dapm.c +29 -0

+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);

sound/soc/soc-dapm.c int snd_soc_dapm_new_pcm(struct snd_soc_card *card, · line 3885

Quoted 4 lines

sound/soc/soc-dapm.c +4 -0

+     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.

Quoted 16 lines

sound/soc/soc-dapm.c +13 -0

+
+     /* 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);

sound/soc/soc-dapm.c int snd_soc_dapm_new_pcm(struct snd_soc_card *card, · line 3932

Quoted 11 lines

      w = snd_soc_dapm_new_control_unlocked(&card->dapm, &template);

sound/soc/soc-dapm.c +0 -3

@@ -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.

Quoted 11 lines

sound/soc/soc-dapm.c +3 -3

-outfree_w_param:
+outfree_link_name:
      devm_kfree(card->dev, w_param_text);
-
+param_fail:
+     devm_kfree(card->dev, link_name);
      return ret;
 }
 
--

Quoted 1 line

2.5.4 (Apple Git-61)

Thanks,
Charles