Re: ASoC: pxa2xx-i2s

[Karl Beldan <karl.beldan@gmail.com>]

Brian Rhodes wrote:
> i2s replay is not being enabled properly on occasion on my pxa270 
> board.  Current code is touching SACR1 in hw_params which I believe is 
> not correct.  That should be configured in trigger before enabling i2s.  
> Record is not disabled for playback mode.  Any invalid data in the 
> transmit fifo causes deadlock and replay is being disabled.
> 
> I have been able to reproduce it in kernel 2.6.26 as well as Linus' git 
> tree using speaker-test.  (repeatedly starting and stopping).  I've 
> verified that this fix prevents (at least) this condition.
> 
> Please review this patch and let me know if there is anything I have 
> overlooked.
> 
> Thanks
> 
> diff --git a/sound/soc/pxa/pxa2xx-i2s.c b/sound/soc/pxa/pxa2xx-i2s.c
> index 517991f..6cda7f1 100644
> --- a/sound/soc/pxa/pxa2xx-i2s.c
> +++ b/sound/soc/pxa/pxa2xx-i2s.c
> @@ -211,12 +211,10 @@ static int pxa2xx_i2s_hw_params(struct snd_pcm_substream *substream,
>  	if (!(SACR0 & SACR0_ENB)) {
>  
>  		SACR0 = 0;
> -		SACR1 = 0;
>  		if (pxa_i2s.master)
>  			SACR0 |= SACR0_BCKD;
>  
>  		SACR0 |= SACR0_RFTH(14) | SACR0_TFTH(1);
> -		SACR1 |= pxa_i2s.fmt;
>  	}
>  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
>  		SAIMR |= SAIMR_TFS;
> @@ -257,6 +255,12 @@ static int pxa2xx_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
>  
>  	switch (cmd) {
>  	case SNDRV_PCM_TRIGGER_START:
> +		SACR1 = pxa_i2s.fmt | SACR1_DRPL | SACR1_DREC;

Here you are stopping any ongoing stream - removing SACR1 reset in
hw_params should be enough.

> +		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> +			SACR1 &= ~SACR1_DRPL;
> +		} else {
> +			SACR1 &= ~SACR1_DREC;
> +		}
>  		SACR0 |= SACR0_ENB;
>  		break;
>  	case SNDRV_PCM_TRIGGER_RESUME:
> 

If - I can - I will post my changes, unless you are finished by then ;)

-- 
Karl