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

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

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?

> > 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.