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).
+/*
- 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?
Yes.
+/* 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?
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.
Liam