Re: [PATCH 1/7] ASoC: soc-dai: add mark for snd_soc_dai_startup/shutdown()

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

Hi Morimoto-san,

> +#define soc_dai_mark_push(dai, substream, tgt)	((dai)->mark_##tgt = substream)

we may want a check that detects if the pointer is NULL before assigning 
it, otherwise we won't be able to detect bad configuration where a 
pointer is overwritten by 2 mark_push() calls on the same object?

> +#define soc_dai_mark_pop(dai, substream, tgt)	((dai)->mark_##tgt = NULL)
> +#define soc_dai_mark_match(dai, substream, tgt)	((dai)->mark_##tgt == substream)
> +
>   /**
>    * snd_soc_dai_set_sysclk - configure DAI system or master clock.
>    * @dai: DAI
> @@ -348,15 +356,26 @@ int snd_soc_dai_startup(struct snd_soc_dai *dai,
>   	    dai->driver->ops->startup)
>   		ret = dai->driver->ops->startup(substream, dai);
>   
> +	/* mark substream if succeeded */
> +	if (ret == 0)
> +		soc_dai_mark_push(dai, substream, startup);
> +

I am a bit concerned here about the case of a bi-directional DAI, it's 
my understanding that the .startup() callback could be called for each 
direction?

soc-dapm.c:		ret = snd_soc_dai_startup(source, substream);
soc-dapm.c:		ret = snd_soc_dai_startup(sink, substream);

To convince myself of this, I added a dummy startup routine and I do see 
it called when I do playback and capture at the same time:

[  179.057494] plb: ssp2 startup stream 0
[  183.976963] plb: ssp2 startup stream 1

That makes me nervous about having a single pointer and unbalanced calls 
between startup and shutdown.

We had such issues in the past so I may be on the paranoid side here...

Thanks
-Pierre