On Mon, Sep 25, 2017 at 6:09 AM, Charles Keepax
<ckeepax@xxxxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, Sep 22, 2017 at 12:55:40AM -0700, yesanishhere@xxxxxxxxx wrote:
>> From: anish kumar <yesanishhere@xxxxxxxxx>
>>
>> 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@xxxxxxxxx>
>> ---
> <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.
Nice catch.
>
>> +
>> + /* 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.
Sure. Let me re-spin it.
>
>> -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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
