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

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

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?

> +/*
> + * Enables the transfer through the PDM interface to/from the Phoenix
> + * codec by enabling the corresponding UP or DN channels.
> + */
> +static void omap_mcpdm_start(struct omap_mcpdm *mcpdm)
> +{
> +	u32 ctrl = omap_mcpdm_read(mcpdm, MCPDM_REG_CTRL);
> +
> +	ctrl |= (MCPDM_SW_DN_RST | MCPDM_SW_UP_RST);
> +	omap_mcpdm_write(mcpdm, MCPDM_REG_CTRL, ctrl);
> +
> +	ctrl |= mcpdm->dn_channels | mcpdm->up_channels;
> +	omap_mcpdm_write(mcpdm, MCPDM_REG_CTRL, ctrl);
> +
> +	ctrl &= ~(MCPDM_SW_DN_RST | MCPDM_SW_UP_RST);
> +	omap_mcpdm_write(mcpdm, MCPDM_REG_CTRL, ctrl);
> +}

Presumably this works with any PDM input/output?

> +/* work to delay McPDM shutdown */
> +static void playback_work(struct work_struct *work)
> +{
> +	struct omap_mcpdm *mcpdm = container_of(work,
> +					struct omap_mcpdm, delayed_work.work);
> +
> +	if (!mcpdm->active && omap_mcpdm_active(mcpdm)) {
> +		omap_mcpdm_stop(mcpdm);
> +		omap_mcpdm_close_streams(mcpdm);
> +	}
> +
> +	if (!omap_mcpdm_active(mcpdm))
> +		pm_runtime_put_sync(mcpdm->dev);
> +}

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?