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

[Karl Beldan]

On Fri, May 8, 2009 at 10:07 PM, Karl Beldan <karl.beldan at gmail.com> wrote:
> 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.

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.

-- 
Karl