[alsa-devel] [PATCH 1/3] ASoC: omap-mcpdm: Replace legacy driver

[Liam Girdwood]

On 07/07/11 17:53, Mark Brown wrote:
> On Thu, Jul 07, 2011 at 05:32:42PM +0100, Liam Girdwood wrote:
>> On 07/07/11 16:57, Mark Brown wrote:
>>> On Thu, Jul 07, 2011 at 03:27:50PM +0300, Peter Ujfalusi wrote:
> 
>>>> The current McPDM driver design is not suitable to support both
>>>> the ABE and Legacy DMA operating modes. Therefore remove most
> 
>>> In what way is it not suitable?
> 
>> It cant support both the ABE and Legacy DMA modes without adding some
>> unnecessary and complicated logic. My preference is 1 driver to
>> support both modes of operation and this is the easiest way to do so
>> (also keeping the maintenance easier too).
> 
> My comment was more that the changelog should say why it's not suitable
> given that it involves a near total rewrite of the driver which is a
> pretty big step, I'm fine with the actual change.  I guess that the
> current McPDM driver configures some things that should be moved to the
> DMA driver?
> 

I cant think of anything thing atm that could be moved to DMA driver,
McPDM mostly uses ABE for moving PCM data around.

>>> It occurs to me that it'd be much simpler to implement this by doing the
>>> cleanup in your runtime suspend callback, it looks like you're working
>>> around the pm_runtime framework rather than using it.  If you need to do
>>> some cleanup when the device goes idle and you can't do it within a
>>> framework designed to suspend the device when it goes idle then there's
>>> an issue there.
> 
>>> Alternatively, why is this deferred?
> 
>> There are some power, clock and pop dependencies here between the
>> CODEC, ABE and McPDM interface and this deferred work allows us to
>> shutwdown McPDM (in a pop free manner) and satisfy the dependencies
>> without causing a data abort and/or locking the ABE firmware.
> 
> Sounds like runtime PM ought to be able to handle this, though?  If you
> need to sync with the ABE can the ABE take PM references to the DAIs
> it's talking to?  I guess the ABE will be happier if everything it's
> talking to runs for longer than it does.  Or the DAI could switch into
> autosuspend mode when it goes idle to do the delay.

It's not that simple in this situation. We also have a PM dependency on
the CODEC here too, it supplies our interface clock via the DAI so we
have to be very careful how we interact with the ABE and CODEC. The
critical thing here is the pop reduction and a subsequent ABE patch will
also introduce some hard timing constraints here too.

Liam