On Fri, May 8, 2009 at 10:07 PM, Karl Beldan karl.beldan@gmail.com wrote:
On Fri, May 8, 2009 at 12:57 PM, Mark Brown broonie@sirena.org.uk wrote:
On Fri, May 08, 2009 at 01:53:55AM +0200, Karl Beldan wrote:
The FIFO logic and the registers are reset at stream startup with SACR0_RST => the sibling function might suffer and the FIFOs might unpleasantly fill up whence the calls to pxa_i2s_wait all over the place. This gets rid of it.
The change to stream startup makes sense but...
@@ -256,7 +242,6 @@ static void pxa2xx_i2s_shutdown(struct snd_pcm_substream *substream,
if ((SACR1 & (SACR1_DREC | SACR1_DRPL)) == (SACR1_DREC | SACR1_DRPL)) { SACR0 &= ~SACR0_ENB;
- pxa_i2s_wait();
clk_disable(clk_i2s); } } @@ -275,7 +260,6 @@ static int pxa2xx_i2s_suspend(struct snd_soc_dai *dai)
/* deactivate link */ SACR0 &= ~SACR0_ENB;
- pxa_i2s_wait();
return 0; }
...there's also changes to the suspend and resume paths here which seem like they'd be as well not to do simply for robustness. Is there any great reason for doing this or are you just doing it for neatness, the changelog isn't entirely clear?
The logic was crippled by this reset and the activation of both functions each time .. at the time I may have over overlooked that the fifo still might still need flushing when cleaning that mess up. One such situation I can think of right now might be when calling snd_pcm_release_substream: it does 'hw_free' before 'close' so pcm won't flush cpu_dai fifo and we end up with 'garbage' in the fifo and one spurious irq. Anyways being unsure of the sequence of events it might well be uncary to get rid of these, will get back to you.
If I recall correctly flushing the fifo works only when the corresponding function is enabled, making the calls in shutdown/suspend useless, whence the overlook, will check.