[alsa-devel] [PATCH 2/4] pxa2xx-i2s: Handle SACR1_DRPL and SACR1_DREC separately

[Mark Brown]

On Fri, May 08, 2009 at 01:53:47AM +0200, Karl Beldan wrote:

> - hw_params enables both RPL and REC functions each time,
> enable the appropriate function in pxa2xx_i2s_trigger.
> - pxa2xx_i2s_shutdown disables i2s anytime one of RPL or REC
> function is off, turn it off only when both functions are off.
> - do not put the clk_i2s for no reason in pxa2xx_i2s_shutdown.

> Signed-off-by: Karl Beldan <karl.beldan at mobile-devices.fr>

I'm still seeing the behaviour I was before now I've applied patch 1.
To reproduce I'm looking at the clocks with a scope.  After startup I'm
starting a playback (aplay and mplayer have the same effect).  Once the
playback is finished (either normally or by killing the application) the
clocks are still present.  Restarting playback causes the clock to stop
immediately and the generation of a DMA error.  Subsequent iterations
repeat the same behaviour, as does recording.

>  	case SNDRV_PCM_TRIGGER_RESUME:
> @@ -252,13 +254,11 @@ static void pxa2xx_i2s_shutdown(struct snd_pcm_substream *substream,
>  		SAIMR &= ~SAIMR_RFS;
>  	}
>  
> -	if (SACR1 & (SACR1_DREC | SACR1_DRPL)) {
> +	if ((SACR1 & (SACR1_DREC | SACR1_DRPL)) == (SACR1_DREC | SACR1_DRPL)) {
>  		SACR0 &= ~SACR0_ENB;
>  		pxa_i2s_wait();
>  		clk_disable(clk_i2s);
>  	}

The problem happens because this code no longer triggers - since the
port is still being reset on startup if the DAI is inactive the first
patch will have no effect on DREC and DRPL, they'll be reset to their
power on default of enabled when startup() is first called.  Applying
your third patch which removes the port reset avoids that issue.

Unfortunately there's still the outstanding issue with the third patch
removing the FIFO flushes - looking at the datasheet I think we do need
to clear the FIFOs, especially in the case where the PXA is running as
slave and might've had clocks removed.  Section 14.4.7.2 explicitly says
there are situations where the FIFO might not be drained and I'd really
not be surprised if there were others.

I'm out of time to look at this today - I expect that the fix is
probably to move some version of the port reset change into this patch.