Re: [alsa-devel] [PATCH 7/7] ASoC: soc-pcm: tidyup soc_pcm_open() order

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
To: Mark Brown <broonie@kernel.org>
Cc: Liam Girdwood <liam.r.girdwood@linux.intel.com>,
	Linux-ALSA <alsa-devel@alsa-project.org>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Subject: Re: [alsa-devel] [PATCH 7/7] ASoC: soc-pcm: tidyup soc_pcm_open() order
Date: 25 Feb 2020 13:20:01 +0900	[thread overview]
Message-ID: <87zhd7jo3b.wl-kuninori.morimoto.gx@renesas.com> (raw)
In-Reply-To: <20200225014039.GA21366@sirena.org.uk>

Hi Pierre-Louis, Mark

> > you'll see that the intent was to
> > - start the cpu_dai
> > - open the platform driver to e.g handle DMAs
> > - start the codec_dai.
> 
> > The second step is not really needed for Intel but might be needed on others
> > where the DMA is centrally handled (Omap, etc).
> 
> > My concern is that we've modified the order to deal with module
> > dependencies, without necessarily taking into account that DMA setup part
> 
> > That said I don't understand the initial sequence for soc_pcm_close() so I
> > am not advocating for a revert, but more a clarification of what those
> > component open/close steps are supposed to do.
> 
> It's possible we're going to shake some issues out here but the
> ordering has always been a bit arbitrary, especially the CPU vs
> CODEC ordering.  We're basically hoping that the ordering we've
> picked is the one that causes fewest glitches and upsets on the
> broad range of hardware.  My expectation/hope is that for most
> hardware the flow control between the DAI and the DMA controller
> (which has to be there anyway) will mean that it mostly doesn't
> matter, if there's anything that is too sensitive to it we can
> always revert and try something
> else.

I'm sorry to not clarify the detail.
Yes, as you mentioned, the patch exchanged start/stop order,
and some platform *might* get damage (I hope not, of course).

But we can't say that the previous order was the best order for all platform.
I'm thinking that the current order is also not the best,
but is not random.
The *best* solution is that we need to have xxx_order flag like for_each_comp_order().

Thank you for your help !!
Best regards
---
Kuninori Morimoto