Re: [PATCH v2 01/10] ASoC: Change the PCM runtime array to a list

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1374 bytes --]

On Mon, Aug 10, 2015 at 10:45:28PM +0800, mengdong.lin@intel.com wrote:

> +	rtd = snd_soc_get_pcm_runtime(card, card->dai_link[1].name);
> +	codec_dai = rtd->codec_dai;
> +

For ease of review please split the addition of this new interface for
getting the runtime from the change to the implementation - the same
function can be written in terms of a list.  As ever please make one
change per commit :(

> +static struct snd_soc_pcm_runtime *soc_new_pcm_runtime(
> +	struct snd_soc_card *card, struct snd_soc_dai_link *dai_link)
> +{
> +	struct snd_soc_pcm_runtime *rtd;
> +
> +	rtd = devm_kzalloc(card->dev,
> +			sizeof(struct snd_soc_pcm_runtime), GFP_KERNEL);
> +	if (!rtd)
> +		return NULL;

Why are we allocating something with devm_kzalloc()...

> +static void soc_free_pcm_runtime(struct snd_soc_card *card,
> +	struct snd_soc_pcm_runtime *rtd)
> +{
> +	if (rtd->codec_dai)
> +		devm_kfree(card->dev, rtd->codec_dais);
> +	devm_kfree(card->dev, rtd);
> +}

...things that we later explicitly free in (hopefully?) all cases.

> +static void soc_remove_pcm_runtimes(struct snd_soc_card *card)
> +{
> +	struct snd_soc_pcm_runtime *rtd, *_rtd;
> +
> +	list_for_each_entry_safe(rtd, _rtd, &card->rtd_list, list) {
> +		list_del(&rtd->list);
> +		soc_free_pcm_runtime(card, rtd);
> +	}
> +
> +	card->num_rtd = 0;
> +}

Why doesn't the free function do the list removal?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]