[alsa-devel] [PATCH 3/4] pxa2xx-i2s: Cleaner use of the FIFOs

[Karl Beldan]

On Fri, May 8, 2009 at 12:57 PM, Mark Brown <broonie at 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.
I will be able to test this monday or so.

BTW, thx for the review.


-- 
Karl